Re: [RFC] d80211 LED handling

[Johannes Berg <johannes@sipsolutions.net>]

On Thu, 2006-08-17 at 17:30 +0200, Jiri Benc wrote:

> How is the driver supposed to add its led handlers?

The driver just adds an LED device. And then we only need to set the
default trigger correctly.

> Using generic led triggers is probably interesting for cases when system
> has some separate led. It won't help drivers which need to handle leds
> on the card itself. In the former case you may want all of wireless
> cards in the system to be connected to one led. I don't see how this is
> easily possible with this patch.

No, that's not possible at all. But we could have a generic
'd80211-all-cards' trigger I suppose.

>  In the latter case, you want to call
> callback provided by the driver and not to use generic led triggers at
> all.

Why not use the generic trigger anyway? With the right default...

> > [...]
> > +config D80211_LEDS
> > +	bool "Enable LED triggers"
> > +	select LEDS_TRIGGERS
> 
> Is it really necessary to have this as a configurable option?

Probably not.

> > [...]
> > +	name = (char*) local->rx_led+1;
> 
> This doesn't seem correct.

Hm, yeah, it probably needs to be (char*) (local->rx_led+1).

> > +	snprintf(name, IFNAMSIZ + 2, "%srx", local->mdev->name);
> 
> Name of interface may change at any time. Using it for fixed identifier
> is not a good idea. In addition, nothing prevents you from ending up
> with two different led triggers with the same name:
> 
> 1. modprobe card1
> 2. ieee80211_led_init(card1)	<- card1 is "wmaster0"
> 3. ip link set wmaster0 name mysupercard
> 4. modprobe card2
> 5. ieee80211_led_init(card2)	<- card2 is "wmaster0"

Good point. I guess we'll want to have the wiphy in there instead.

johannes