Re: [PATCH 1/2] smsc95xx: add module parameter to turn off NIC status leds

Re: [PATCH 1/2] smsc95xx: add module parameter to turn off NIC status leds

[Peter Meerwald]

Hello David,

> > add module parameter to allow to turn off NIC status leds (link, 
> > speed, activity); blinking LEDs are annoying outside the server room :)

> No.  Create a generic mechanism, perhaps via ethtool, for users to
> configure something like this.
 
> Otherwise the next driver that wants to provide this kind of knob
> will add yet another module parameter with yet another name, and
> this kind of ad-hoc set of interfaces absolutely sucks for users.

I am not sure how many NICs provide that functionality, so a specific knob 
seemed reasonable

I had a look at ethtool, there are ETHTOOL_GFLAGS and ETHTOOL_GPFLAGS (and 
the respective setters) which could be used -- but noone seems to use 
those for any purpose 

a private flag for LED control (via ETHTOOL_GPFLAGS) is pretty much the 
same knob as a module parameter, so not an option

so the remaining option is to add something like ETHTOOL_GLEDCONTROL / 
ETHTOOL_SLEDCONTROL with likely only one user (smsc95xx)

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)

Re: [PATCH 1/2] smsc95xx: add module parameter to turn off NIC status leds

[Ben Hutchings]

On Thu, 2012-02-09 at 10:38 +0100, Peter Meerwald wrote:
> Hello David,
> 
> > > add module parameter to allow to turn off NIC status leds (link, 
> > > speed, activity); blinking LEDs are annoying outside the server room :)
> 
> > No.  Create a generic mechanism, perhaps via ethtool, for users to
> > configure something like this.
>  
> > Otherwise the next driver that wants to provide this kind of knob
> > will add yet another module parameter with yet another name, and
> > this kind of ad-hoc set of interfaces absolutely sucks for users.
> 
> I am not sure how many NICs provide that functionality, so a specific knob 
> seemed reasonable

I think this has been controllable on all the PHYs I've dealt with.
Generally they need to be adaptable to the requirements for different
boards.  So in theory this is generic.

However I just don't see this being a popular enough option for many
driver maintainers to implement it.  Those status LEDs are useful.  Put
a piece of tape over them if you don't like them. :-)  Or if you're
designing a board then put them somewhere out of the way (but your users
may hate you for it).

> I had a look at ethtool, there are ETHTOOL_GFLAGS and ETHTOOL_GPFLAGS (and 
> the respective setters) which could be used -- but noone seems to use 
> those for any purpose 
> 
> a private flag for LED control (via ETHTOOL_GPFLAGS) is pretty much the 
> same knob as a module parameter, so not an option

In my view it's slightly preferable because private flags are
per-device.  Not sure whether David would agree.

Ben.

> so the remaining option is to add something like ETHTOOL_GLEDCONTROL / 
> ETHTOOL_SLEDCONTROL with likely only one user (smsc95xx)
> 
> regards, p.
> 

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [PATCH 2/2] smsc95xx: allow to re-use MAC address already programmed

[Peter Meerwald]

Hello David,

> > patch adds a module parameter which allows to keep the MAC address already 
> > set (e.g. by the boot loader) if it is valid (otherwise the MAC address is read 
> > from an optional EEPROM or randomly generated)

> No way, for the same reason as your other patch.
> Module parameters suck, use something generic.

is there a generic solution (yet)?

the goal is to support tftp-ing a kernel image from u-boot and doing 
a rootnfs mount in the kernel (with the same MAC address)

it was suggested to set the MAC address from userspace/initrd which seems 
to be a lot of overhead

I am aware of Danny Kukawka's work in a similar direction

the embedded approach would probably be to hand some board-specific 
callback to the 'network layer' in order to obtain a MAC whenever it is 
needed; this could replace/supplement random_ether_addr() found in many 
drivers

regards, p.

-- 

Peter Meerwald
+43-664-2444418 (mobile)