2008-01-16 20:24:22

by John W. Linville

[permalink] [raw]
Subject: Re: led support in mac80211 and drivers

I appreciate that this topic started with discussion of the (long
overdue) support for LEDs in the iwlwifi drivers. However at
this point I think it has strayed sufficiently into the mac80211
realm that it should be moved to the home of mac80211 development,
[email protected]

Thanks,

John

On Wed, Jan 16, 2008 at 08:38:48PM +0200, Tomas Winkler wrote:
> > > Currently we recognize 4 or 5 pasterns that are implemented and
> > > switching between them according laptop vendor.
> >
> > We = who?
>
> Intel
>
> Greping through the kernel source revealed only a single user
> > of ieee80211_get_*_led_name(), the broadcom B43 wireless driver.
>
> Correct
>
> That
> > driver switches between different modes based on bus->boardinfo.vendor.
> > But where are the other modules needing this flexibility? Are they being
> > developed in private by the companies that make the access points?
> >
>
> Different Laptop vendor and AP vendors. This information might be
> written in many places, PCI subvendor, in EEROM etc sometimes it has
> to be supplied from outside.
>
>
> > > So what is needed for led implementation are 3 decision chain
> > > - Activity Trigger (RX, TX, Association, Radio State/RF Kill)
> > > - Behavior Decision - What is done on each trigger
> > > - Behavior is easy to model - ON, OFF, BLINK(pattern)
> > > - Mapping to what led is this behavior mapped
> > > - One behavior might be mapped to single led. Then we need to know
> > > what trigger take precedences.
> >
> > Actually, the second and third point can be merged.
> >
>
> Agree Implementation wise might be simpler.
>
> > > Actually the major problem I have currently with mac80211 trigger
> > > implementation RX/TX trigger since Intel's led doesn't have to be
> > > triggered ON/OFF on each packet, led is capable of blinking itself.,
> > > therefore I would like to see your patch very much. We probably would
> > > have to work on flexibility of blinking patterns
> >
> > I thought about having an 'ieee80211_activity_listener' that could
> > subscribe to activity events of a ieee80211_hw device. One single
> > callback that would be called when there's activity or if the state of
> > the device has changed. Maybe something like this:
> >
> > int (*callback)(... *hw, u8 state, u8 type);
> >
> > where 'state' would be a bitmap of scanning/associated/etc and type the
> > 'type' of activity (transmitting/receiving a frame). The callback then
> > would decide which leds to activate and how (blinking or not etc). Does
> > that sound better? At least it would be better than having to register
> > three leds in each low-level driver ;)
>
> I'm not sure it's such big deal to subscribe to led more problem is
> that I cannot subscribe same led to different triggers
> so need another level
> mac80211 | driver
> Led Trigger ->| Logical Led -> (LED LOGIC/MAPPING) -> HW LED
> |
>
> The problem here is that logic is already defined in Led pattern is
> already defined int Led Trigger
>
> Problem with your approach I cannot reach LEDs that are not connected
> to NIC's PCIe pins.
>
> Other options that uses your idea is
> mac80211 | driver
> mac callback ->| (LED LOGIC) - Led Trigger - > Led
>
>
> > Btw, is there a function that lists all 'struct ieee80211_hw *'? How
> > does the led driver (external driver, where the leds are on a different
> > bus and the driver implemented in a different kernel module) get the led
> > triggers?
>
> by name - "wlan0:rx" for example
>
> > Oh, and I read (in the intel driver source) that there is no callback
> > from the low-level driver into the mac subsystem that would inform it
> > that rfkill is active... that limitation would be bad in case of the
> > external led drivers.
> >
> That's a gap.
>
> > tom
> >
> >
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> Ipw3945-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/ipw3945-devel

--
John W. Linville
[email protected]


2008-01-17 22:15:17

by Michael Büsch

[permalink] [raw]
Subject: Re: led support in mac80211 and drivers

On Thursday 17 January 2008 16:46:22 Johannes Berg wrote:
> > > > > Actually the major problem I have currently with mac80211 trigger
> > > > > implementation RX/TX trigger since Intel's led doesn't have to be
> > > > > triggered ON/OFF on each packet, led is capable of blinking itself.,
>
> I'm wondering if this shouldn't simply be a driver decision and mac80211
> exports an RXon LED that is always "on" for as long as receive is
> running (whatever that means, please specify!)

I read this thread multiple times, but I don't get where the problem is.
Why don't you simply implement this in the driver:
When LED didn't change software state for X jiffies, switch the auto-blinking
LED in the hardware off. Switch it on if it's off in hardware and there's
a software state transition.
Where's the problem? This is trivial to implement.

--
Greetings Michael.

2008-01-16 21:34:14

by Tomas Winkler

[permalink] [raw]
Subject: Re: led support in mac80211 and drivers

You are right, Thanks for bringing us home :)
Tomas

On Jan 16, 2008 10:01 PM, John W. Linville <[email protected]> wrote:
> I appreciate that this topic started with discussion of the (long
> overdue) support for LEDs in the iwlwifi drivers. However at
> this point I think it has strayed sufficiently into the mac80211
> realm that it should be moved to the home of mac80211 development,
> [email protected]
>
> Thanks,
>
> John
>
>
> On Wed, Jan 16, 2008 at 08:38:48PM +0200, Tomas Winkler wrote:
> > > > Currently we recognize 4 or 5 pasterns that are implemented and
> > > > switching between them according laptop vendor.
> > >
> > > We = who?
> >
> > Intel
> >
> > Greping through the kernel source revealed only a single user
> > > of ieee80211_get_*_led_name(), the broadcom B43 wireless driver.
> >
> > Correct
> >
> > That
> > > driver switches between different modes based on bus->boardinfo.vendor.
> > > But where are the other modules needing this flexibility? Are they being
> > > developed in private by the companies that make the access points?
> > >
> >
> > Different Laptop vendor and AP vendors. This information might be
> > written in many places, PCI subvendor, in EEROM etc sometimes it has
> > to be supplied from outside.
> >
> >
> > > > So what is needed for led implementation are 3 decision chain
> > > > - Activity Trigger (RX, TX, Association, Radio State/RF Kill)
> > > > - Behavior Decision - What is done on each trigger
> > > > - Behavior is easy to model - ON, OFF, BLINK(pattern)
> > > > - Mapping to what led is this behavior mapped
> > > > - One behavior might be mapped to single led. Then we need to know
> > > > what trigger take precedences.
> > >
> > > Actually, the second and third point can be merged.
> > >
> >
> > Agree Implementation wise might be simpler.
> >
> > > > Actually the major problem I have currently with mac80211 trigger
> > > > implementation RX/TX trigger since Intel's led doesn't have to be
> > > > triggered ON/OFF on each packet, led is capable of blinking itself.,
> > > > therefore I would like to see your patch very much. We probably would
> > > > have to work on flexibility of blinking patterns
> > >
> > > I thought about having an 'ieee80211_activity_listener' that could
> > > subscribe to activity events of a ieee80211_hw device. One single
> > > callback that would be called when there's activity or if the state of
> > > the device has changed. Maybe something like this:
> > >
> > > int (*callback)(... *hw, u8 state, u8 type);
> > >
> > > where 'state' would be a bitmap of scanning/associated/etc and type the
> > > 'type' of activity (transmitting/receiving a frame). The callback then
> > > would decide which leds to activate and how (blinking or not etc). Does
> > > that sound better? At least it would be better than having to register
> > > three leds in each low-level driver ;)
> >
> > I'm not sure it's such big deal to subscribe to led more problem is
> > that I cannot subscribe same led to different triggers
> > so need another level
> > mac80211 | driver
> > Led Trigger ->| Logical Led -> (LED LOGIC/MAPPING) -> HW LED
> > |
> >
> > The problem here is that logic is already defined in Led pattern is
> > already defined int Led Trigger
> >
> > Problem with your approach I cannot reach LEDs that are not connected
> > to NIC's PCIe pins.
> >
> > Other options that uses your idea is
> > mac80211 | driver
> > mac callback ->| (LED LOGIC) - Led Trigger - > Led
> >
> >
> > > Btw, is there a function that lists all 'struct ieee80211_hw *'? How
> > > does the led driver (external driver, where the leds are on a different
> > > bus and the driver implemented in a different kernel module) get the led
> > > triggers?
> >
> > by name - "wlan0:rx" for example
> >
> > > Oh, and I read (in the intel driver source) that there is no callback
> > > from the low-level driver into the mac subsystem that would inform it
> > > that rfkill is active... that limitation would be bad in case of the
> > > external led drivers.
> > >
> > That's a gap.
> >
> > > tom
> > >
> > >
> >
>
> > -------------------------------------------------------------------------
> > This SF.net email is sponsored by: Microsoft
> > Defy all challenges. Microsoft(R) Visual Studio 2008.
> > http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> > _______________________________________________
> > Ipw3945-devel mailing list
> > [email protected]
> > https://lists.sourceforge.net/lists/listinfo/ipw3945-devel
>
> --
> John W. Linville
> [email protected]
>

2008-01-18 03:56:08

by Jerone Young

[permalink] [raw]
Subject: Re: [ipw3945-devel] led support in mac80211 and drivers

Yes I see this more as a stop gap solution. The fact is that there is
a lot of contengency on how the light blinks (and it's
understandable). But the feature is so far over due that at this point
it's best to have something so the user can know that wireless is on
or off.

On Jan 17, 2008 6:35 PM, Tomas Winkler <[email protected]> wrote:
> On Jan 18, 2008 12:12 AM, Michael Buesch <[email protected]> wrote:
> > On Thursday 17 January 2008 16:46:22 Johannes Berg wrote:
> > > > > > > Actually the major problem I have currently with mac80211 trigger
> > > > > > > implementation RX/TX trigger since Intel's led doesn't have to be
> > > > > > > triggered ON/OFF on each packet, led is capable of blinking itself.,
> > >
> > > I'm wondering if this shouldn't simply be a driver decision and mac80211
> > > exports an RXon LED that is always "on" for as long as receive is
> > > running (whatever that means, please specify!)
> >
> > I read this thread multiple times, but I don't get where the problem is.
> > Why don't you simply implement this in the driver:
> > When LED didn't change software state for X jiffies, switch the auto-blinking
> > LED in the hardware off. Switch it on if it's off in hardware and there's
> > a software state transition.
> > Where's the problem? This is trivial to implement.
>
> Sure it's simple, even there are few implementations already in the
> mailing list.. just still missing the flexibility I seek.
> Tomas
>
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> Ipw3945-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/ipw3945-devel
>

2008-01-17 15:46:50

by Johannes Berg

[permalink] [raw]
Subject: Re: led support in mac80211 and drivers


> > > > So what is needed for led implementation are 3 decision chain
> > > > - Activity Trigger (RX, TX, Association, Radio State/RF Kill)

mac80211 exports multiple triggers already and could be amended to those
that are currently missing.

> > > > - Behavior Decision - What is done on each trigger
> > > > - Behavior is easy to model - ON, OFF, BLINK(pattern)

BLINK(pattern) is currently not defined. Actually, especially since you
say later:

> > > > Actually the major problem I have currently with mac80211 trigger
> > > > implementation RX/TX trigger since Intel's led doesn't have to be
> > > > triggered ON/OFF on each packet, led is capable of blinking itself.,

I'm wondering if this shouldn't simply be a driver decision and mac80211
exports an RXon LED that is always "on" for as long as receive is
running (whatever that means, please specify!)

Or you need to define blink patterns with the core LED framework.

> > > > - Mapping to what led is this behavior mapped

That's trivially specified in sysfs. b43 (since it was mentioned
already) has some special logic to set up defaults coming from EEPROM.

> > > > - One behavior might be mapped to single led. Then we need to know
> > > > what trigger take precedences.

That (multiple behaviours on a single LED) is impossible in the current
LED framework.

> > > I thought about having an 'ieee80211_activity_listener' that could
> > > subscribe to activity events of a ieee80211_hw device. One single
> > > callback that would be called when there's activity or if the state of
> > > the device has changed. Maybe something like this:
> > >
> > > int (*callback)(... *hw, u8 state, u8 type);
> > >
> > > where 'state' would be a bitmap of scanning/associated/etc and type the
> > > 'type' of activity (transmitting/receiving a frame). The callback then
> > > would decide which leds to activate and how (blinking or not etc). Does
> > > that sound better? At least it would be better than having to register
> > > three leds in each low-level driver ;)

No, that sounds like a bad idea. We actually want the flexibility here,
I could for example use the front LED on my powerbook as a wireless
association LED if I wanted. We want to use the LED framework and if
that means three LEDs in your driver then so be it.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2008-01-18 12:38:38

by Johannes Berg

[permalink] [raw]
Subject: Re: led support in mac80211 and drivers


> What we do is translate throughputs - to 8 patterns if I remember correctly.
> Only when the throughput cross threshold level NIC is disturbed with
> changing blinking pattern.

Huh ok. I suppose you want a throughput notification from mac80211 for
that. And you want to define blinking within the LED framework, rather
than just intensity.

> Not for traffic but for example some laptops vendors wants slow blink
> in unassociated state some wants led to be off.
>
> The problem is that led behavior in clean design would be defined by
> trigger. So I would prefer to put flexibility of LED behavior in that
> level.

I'm not so sure. I think it'd make more sense to define state triggers
(associated) and do whatever behaviour based on the state. Dunno.

> Most of todays laptop has only one led for Wifi that's a problem.

> I agree it has a lot of benefits to use LED framework, but it can be
> pushed to driver or some module. Unfortunately I'm not sure myself

Maybe an intermediate level in the LED framework could be written,
something that itself has multiple LED "inputs" (LED devices) and takes
multiple triggers, translating it into a new trigger that the single
laptop wifi LED can be bound to. Sounds like too much complexity
though...

I don't really care much, but I definitely don't want to add new
callbacks for LEDs to mac80211. If you need something special please do
it within the LED framework.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2008-01-17 16:40:35

by Tomas Winkler

[permalink] [raw]
Subject: Re: led support in mac80211 and drivers

On Jan 17, 2008 5:46 PM, Johannes Berg <[email protected]> wrote:

> I'm wondering if this shouldn't simply be a driver decision and mac80211
> exports an RXon LED that is always "on" for as long as receive is
> running (whatever that means, please specify!)

What we do is translate throughputs - to 8 patterns if I remember correctly.
Only when the throughput cross threshold level NIC is disturbed with
changing blinking pattern.

> Or you need to define blink patterns with the core LED framework.

Not for traffic but for example some laptops vendors wants slow blink
in unassociated state some wants led to be off.

The problem is that led behavior in clean design would be defined by
trigger. So I would prefer to put flexibility of LED behavior in that
level.

> > > > > - Mapping to what led is this behavior mapped
>
> That's trivially specified in sysfs. b43 (since it was mentioned
> already) has some special logic to set up defaults coming from EEPROM.
>
> > > > > - One behavior might be mapped to single led. Then we need to know
> > > > > what trigger take precedences.
>
> That (multiple behaviours on a single LED) is impossible in the current
> LED framework.

Most of todays laptop has only one led for Wifi that's a problem.

> > > > I thought about having an 'ieee80211_activity_listener' that could
> > > > subscribe to activity events of a ieee80211_hw device. One single
> > > > callback that would be called when there's activity or if the state of
> > > > the device has changed. Maybe something like this:
> > > >
> > > > int (*callback)(... *hw, u8 state, u8 type);
> > > >
> > > > where 'state' would be a bitmap of scanning/associated/etc and type the
> > > > 'type' of activity (transmitting/receiving a frame). The callback then
> > > > would decide which leds to activate and how (blinking or not etc). Does
> > > > that sound better? At least it would be better than having to register
> > > > three leds in each low-level driver ;)
>
> No, that sounds like a bad idea. We actually want the flexibility here,
> I could for example use the front LED on my powerbook as a wireless
> association LED if I wanted. We want to use the LED framework and if
> that means three LEDs in your driver then so be it.

I agree it has a lot of benefits to use LED framework, but it can be
pushed to driver or some module. Unfortunately I'm not sure myself

Tomas

> johannes
>

2008-01-17 01:13:25

by Tomas Carnecky

[permalink] [raw]
Subject: Re: led support in mac80211 and drivers

John W. Linville wrote:
> On Wed, Jan 16, 2008 at 08:38:48PM +0200, Tomas Winkler wrote:
>> That
>>> driver switches between different modes based on bus->boardinfo.vendor.
>>> But where are the other modules needing this flexibility? Are they being
>>> developed in private by the companies that make the access points?
>>>
>> Different Laptop vendor and AP vendors. This information might be
>> written in many places, PCI subvendor, in EEROM etc sometimes it has
>> to be supplied from outside.

The question rather was: since there is really no user of the led
infrastructure that needs the flexibility (the broadcom seems to have
the leds on the same bus as the device itself) in the official kernel..
where are these drivers? Are they publicly available? Or are they in
custom kernels? I can see the need for such infrastructure, but I don't
see any actual users of it...

>>> I thought about having an 'ieee80211_activity_listener' that could
>>> subscribe to activity events of a ieee80211_hw device. One single
>>> callback that would be called when there's activity or if the state of
>>> the device has changed. Maybe something like this:
>>>
>>> int (*callback)(... *hw, u8 state, u8 type);
>>>
>>> where 'state' would be a bitmap of scanning/associated/etc and type the
>>> 'type' of activity (transmitting/receiving a frame). The callback then
>>> would decide which leds to activate and how (blinking or not etc). Does
>>> that sound better? At least it would be better than having to register
>>> three leds in each low-level driver ;)
>> I'm not sure it's such big deal to subscribe to led more problem is
>> that I cannot subscribe same led to different triggers
>> so need another level
>> mac80211 | driver
>> Led Trigger ->| Logical Led -> (LED LOGIC/MAPPING) -> HW LED
>> |
>>
>> The problem here is that logic is already defined in Led pattern is
>> already defined int Led Trigger
>>
>> Problem with your approach I cannot reach LEDs that are not connected
>> to NIC's PCIe pins.
>>
>> Other options that uses your idea is
>> mac80211 | driver
>> mac callback ->| (LED LOGIC) - Led Trigger - > Led

I see that the whole wireless led interaction is a delicate matter. It's
not easy to understand it fully (in such short time - however hard I
try). You wouldn't believe how many times I've rewritten this email ;)

Do you have any numbers about how many platforms have leds on a
different bus than the wlan device? Are those notebooks or embedded
devices? In case of notebooks, I assume linux doesn't support the leds
there, since I haven't found any code in the kernel that would use the
mac80211 led subsystem to receive led activity notifications.

The 4965 chip supposedly supports three leds, but my notebook (thinkpad
x61t) only has one. Other notebooks/platforms may have two/three. I
imagine implementing different led behaviour based on how many leds
there are and other platform differences will require lots of code
(platform detection etc). And thus getting the led code right for
everyone is very difficult. Is that the reason why any of the iwlwifi
led patches hasn't been applied yet?

In the iwl4965 driver, implementing led support wholly inside the driver
itself seems possible. And platforms that have leds on a different bus
still can make their own led-driver and use the mac80211 subsystem to
receive led activity notifications. All but the broadcom wireless
drivers implement led support is such way. So I think that's the way to go.

In essence, on platforms that have the radio device and leds on the same
bus, it's actually easier to implement led support inside the driver and
not using the mac80211 led subsystem. The driver has all the needed info
to control the leds (device state etc). The mac80211 led subsystem is
only useful for platforms where the leds are on a different bus.

I must apologize, there's nothing wrong with the mac80211 led subsystem,
it does what it was designed for. It's the many iwlwifi led patches that
gave me a bad impression that the mac80211 led subsystem was somehow
needed for led support, what a shame :( I'm sorry for the noise.


tom


2008-01-18 12:35:52

by Johannes Berg

[permalink] [raw]
Subject: Re: led support in mac80211 and drivers


> I read this thread multiple times, but I don't get where the problem is.
> Why don't you simply implement this in the driver:
> When LED didn't change software state for X jiffies, switch the auto-blinking
> LED in the hardware off. Switch it on if it's off in hardware and there's
> a software state transition.
> Where's the problem? This is trivial to implement.

I think the point is that the software LED changes all the time but
that's not necessary for the hardware LED because the firmware will
blink it. Or something like that anyway.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2008-01-18 00:35:15

by Tomas Winkler

[permalink] [raw]
Subject: Re: led support in mac80211 and drivers

On Jan 18, 2008 12:12 AM, Michael Buesch <[email protected]> wrote:
> On Thursday 17 January 2008 16:46:22 Johannes Berg wrote:
> > > > > > Actually the major problem I have currently with mac80211 trigger
> > > > > > implementation RX/TX trigger since Intel's led doesn't have to be
> > > > > > triggered ON/OFF on each packet, led is capable of blinking itself.,
> >
> > I'm wondering if this shouldn't simply be a driver decision and mac80211
> > exports an RXon LED that is always "on" for as long as receive is
> > running (whatever that means, please specify!)
>
> I read this thread multiple times, but I don't get where the problem is.
> Why don't you simply implement this in the driver:
> When LED didn't change software state for X jiffies, switch the auto-blinking
> LED in the hardware off. Switch it on if it's off in hardware and there's
> a software state transition.
> Where's the problem? This is trivial to implement.

Sure it's simple, even there are few implementations already in the
mailing list.. just still missing the flexibility I seek.
Tomas