[<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
Powered by blists - more mailing lists