netdev - Re: [PATCH] zd1211rw-d80211: Use LED class

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 11 Dec 2006 20:32:03 -0500
From:	Michael Wu <flamingice@...rmilk.net>
To:	Ulrich Kunitz <kune@...ne-taler.de>
Cc:	Daniel Drake <dsd@...too.org>, netdev@...r.kernel.org,
	Johannes Berg <johannes@...solutions.net>
Subject: Re: [PATCH] zd1211rw-d80211: Use LED class

On Monday 11 December 2006 17:55, Ulrich Kunitz wrote:
> I cannot accknowledge this patch. Michael, the LED does not only
> show the link status, but also indicate that packets are sent,
> which is done by the firmware. However the LED flags have to be
> reset every second, because if an odd number of packets have been
> transmitted, the LED stays dark.
The d80211 TX trigger has the same behavior. It's probably worse on single LED 
devices since it may look like nothing is on. Even then, I think having it 
reset every second is overkill. Having an "TX LED" watchdog run when we 
haven't TXed anything for the last second or sounds better to me. This would 
force the LEDs to a reasonable state when we know nothing has been TXed 
recently. This could be for STA mode only.. I dunno if the LED blinks when 
sending hardware generated/filled beacons.

> I even don't think, that the upper d80211 layers should care for
> LED controlling. The LEDs are part of the device and should be
> controlled by the device driver. I have a ZD1211 device, which has
> two LEDs. Now suddenly the upper layer must know about the two
> LEDs. But there could be WLAN devices who would have 6 LEDs for
> whatever reason. Should the upper layer know about all those LEDs
> and their semantics? The better design would be, that the driver
> could register for upper layer events and control it's LEDs
> accordingly.
>
The upper layer does not know about the LEDs of individual devices. It merely 
provides a number of LED triggers which device drivers can choose to listen 
to - which sounds like what you're suggesting.

> If you would also have taken the time to look at my out-of-tree
> driver, you would have seen the other housekeeping/workqueue
> functions for handling strong signals. These changes are still
> missing from the kernel, because I have still to resolve issues
> with AL2230 RFs. This again can only be implemented in workqueues,
> because the register accesses require process context.
>
Switching to using LED class makes the housekeeping & workqueue code not do 
anything useful, so I removed it. I am not against the housekeeping & 
workqueue code - but right now this is dead code that will unnecessarily 
cause zd1211rw-d80211 not to compile when wireless-dev gets the workqueue api 
changes. (of course, d80211 doesn't compile either with the workqueue 
changes, but I'm working out a patch for that..)

If you are testing your new RF code with zd1211rw-d80211 also, I can add the 
workqueue & housekeeping code back as soon as things are sorted out in 
wireless-dev. I will also probably port the rts/cts code after wireless-dev 
deals with the workqueue changes.

Sound tolerable to you?

-Michael Wu

Content of type "application/pgp-signature" skipped