All of lore.kernel.org
 help / color / mirror / Atom feed
From: Claudiu Manoil <claudiu.manoil@freescale.com>
To: David Miller <davem@davemloft.net>
Cc: <netdev@vger.kernel.org>
Subject: Re: [PATCH net-next 2/6] gianfar: Replace sysfs stubs with module params (fix)
Date: Mon, 17 Feb 2014 12:20:52 +0200	[thread overview]
Message-ID: <5301E284.3080309@freescale.com> (raw)
In-Reply-To: <20140216.234843.10302057405564080.davem@davemloft.net>

On 2/17/2014 6:48 AM, David Miller wrote:
> From: Claudiu Manoil <claudiu.manoil@freescale.com>
> Date: Fri, 14 Feb 2014 14:04:01 +0200
>
>> Removing the sysfs stubs for the Tx FIFOCFG and ATTRELI
>> (stashing) config registers, as these registers may only
>> be configured after a MAC reset, with the controller stopped
>> (i.e. during hw init, at probe() time). The current sysfs
>> stubs allow on-the-fly updates of these registers (the locking
>> measures are useless and only add unecessary code).
>>
>> Changing these registers on-the-fly is strogly discouraged.
>> In this regard, this patch may be seen as a security fix.
>>
>> To address this issue and not lose entirely these config
>> params, they are now accessible as driver module parameters,
>> and their names and default values have been preserved.
>>
>> Moreover, the stasing configuration options were effectively
>> disabled (didn't get to the hw anyway if changed) because
>> the stashing device_flags (HAS_BD_STASHING|HAS_BUF_STASHING)
>> were "accidentally" cleared during probe(). The patch fixes
>> this bug as well.
>>
>> Signed-off-by: Claudiu Manoil <claudiu.manoil@freescale.com>
>
> Sorry, no new module parameters.
>
> And as you state these never actually reached the hardware so they
> _never worked_.  Please just remove them.
>
>

Sounds reasonable.
Only the stashing parameters (ATTRELI) didn't reach the HW.
The FIFO cfg ones did, but changing these defaults is also
discouraged (as documented in gianfar.txt).
Will send a V2.

Thanks.
Claudiu

  reply	other threads:[~2014-02-17 10:21 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-14 12:03 [PATCH net-next 0/6] gianfar: Device configuration fixes Claudiu Manoil
2014-02-14 12:04 ` [PATCH net-next 1/6] gianfar: Cleanup/Fix gfar_probe and the hw init code Claudiu Manoil
2014-02-14 12:04 ` [PATCH net-next 2/6] gianfar: Replace sysfs stubs with module params (fix) Claudiu Manoil
2014-02-17  4:48   ` David Miller
2014-02-17 10:20     ` Claudiu Manoil [this message]
2014-02-14 12:04 ` [PATCH net-next 3/6] gianfar: Remove useless HAS_PADDING device flag Claudiu Manoil
2014-02-14 12:04 ` [PATCH net-next 4/6] gianfar: Factor out enabling/disabling of hw interrupts Claudiu Manoil
2014-02-14 12:04 ` [PATCH net-next 5/6] gianfar: Add missing graceful reset steps and fixes Claudiu Manoil
2014-02-14 12:04 ` [PATCH net-next 6/6] gianfar: Remove clean_rx_ring race from gfar_ethtool Claudiu Manoil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5301E284.3080309@freescale.com \
    --to=claudiu.manoil@freescale.com \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.