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 - To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
