linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC V2] b43: A patch for control of the radio LED using rfkill
@ 2008-09-18 14:07 Larry Finger
  2008-09-18 14:24 ` Henrique de Moraes Holschuh
  0 siblings, 1 reply; 13+ messages in thread
From: Larry Finger @ 2008-09-18 14:07 UTC (permalink / raw)
  To: ivdoorn; +Cc: bcm43xx-dev, linux-wireless

The changes below, along with Henriques patch "[PATCH] rfkill: update
LEDs for all state changes", and the reversion of commit bc19d6e make
the wireless LED toggle correctly.

This version passes UNBLOCKED to rfkill when the hardware switch enables the radio.

Most of this material comes from Matthew Garrett.

Larry

Index: wireless-testing/drivers/net/wireless/b43/rfkill.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/rfkill.c
+++ wireless-testing/drivers/net/wireless/b43/rfkill.c
@@ -44,13 +44,11 @@ static bool b43_is_hw_radio_enabled(stru
 	return 0;
 }
 
-/* The poll callback for the hardware button. */
-static void b43_rfkill_poll(struct input_polled_dev *poll_dev)
+static void b43_rfkill_update(struct b43_wldev *dev)
 {
-	struct b43_wldev *dev = poll_dev->private;
 	struct b43_wl *wl = dev->wl;
 	bool enabled;
-	bool report_change = 0;
+	struct b43_rfkill *rfk = &(dev->wl->rfkill);
 
 	mutex_lock(&wl->mutex);
 	if (unlikely(b43_status(dev) < B43_STAT_INITIALIZED)) {
@@ -60,18 +58,35 @@ static void b43_rfkill_poll(struct input
 	enabled = b43_is_hw_radio_enabled(dev);
 	if (unlikely(enabled != dev->radio_hw_enable)) {
 		dev->radio_hw_enable = enabled;
-		report_change = 1;
 		b43info(wl, "Radio hardware status changed to %s\n",
 			enabled ? "ENABLED" : "DISABLED");
+		if (!enabled)
+			rfkill_force_state(rfk->rfkill,
+					   RFKILL_STATE_HARD_BLOCKED);
+		else
+			rfkill_force_state(rfk->rfkill,
+					   RFKILL_STATE_UNBLOCKED);
 	}
 	mutex_unlock(&wl->mutex);
+}
 
-	/* send the radio switch event to the system - note both a key press
-	 * and a release are required */
-	if (unlikely(report_change)) {
-		input_report_key(poll_dev->input, KEY_WLAN, 1);
-		input_report_key(poll_dev->input, KEY_WLAN, 0);
-	}
+static void b43_rfkill_poll(unsigned long data)
+{
+	struct b43_rfkill *rfkill = (struct b43_rfkill *)data;
+	schedule_work(&rfkill->poll_queue);
+}
+
+static void b43_rfkill_work(struct work_struct *work)
+{
+	struct b43_rfkill *rfk = container_of(work, struct b43_rfkill,
+					      poll_queue);
+	struct b43_wl *wl = container_of(rfk, struct b43_wl, rfkill);
+	struct b43_wldev *dev = wl->current_dev;
+
+	b43_rfkill_update(dev);
+	rfk->poll_timer.function = b43_rfkill_poll;
+	rfk->poll_timer.expires = round_jiffies(jiffies + HZ);
+	add_timer(&rfk->poll_timer);
 }
 
 /* Called when the RFKILL toggled in software. */
@@ -141,48 +156,23 @@ void b43_rfkill_init(struct b43_wldev *d
 	rfk->rfkill->toggle_radio = b43_rfkill_soft_toggle;
 	rfk->rfkill->user_claim_unsupported = 1;
 
-	rfk->poll_dev = input_allocate_polled_device();
-	if (!rfk->poll_dev) {
-		rfkill_free(rfk->rfkill);
-		goto err_freed_rfk;
-	}
-
-	rfk->poll_dev->private = dev;
-	rfk->poll_dev->poll = b43_rfkill_poll;
-	rfk->poll_dev->poll_interval = 1000; /* msecs */
-
-	rfk->poll_dev->input->name = rfk->name;
-	rfk->poll_dev->input->id.bustype = BUS_HOST;
-	rfk->poll_dev->input->id.vendor = dev->dev->bus->boardinfo.vendor;
-	rfk->poll_dev->input->evbit[0] = BIT(EV_KEY);
-	set_bit(KEY_WLAN, rfk->poll_dev->input->keybit);
-
 	err = rfkill_register(rfk->rfkill);
 	if (err)
-		goto err_free_polldev;
+		goto err_free_rfk;
 
-#ifdef CONFIG_RFKILL_INPUT_MODULE
-	/* B43 RF-kill isn't useful without the rfkill-input subsystem.
-	 * Try to load the module. */
-	err = request_module("rfkill-input");
-	if (err)
-		b43warn(wl, "Failed to load the rfkill-input module. "
-			"The built-in radio LED will not work.\n");
-#endif /* CONFIG_RFKILL_INPUT */
+	rfk->registered = 1;
 
-	err = input_register_polled_device(rfk->poll_dev);
-	if (err)
-		goto err_unreg_rfk;
+	INIT_WORK(&rfk->poll_queue, b43_rfkill_work);
 
-	rfk->registered = 1;
+	init_timer(&rfk->poll_timer);
+	rfk->poll_timer.function = b43_rfkill_poll;
+	rfk->poll_timer.expires = round_jiffies(jiffies + HZ);
+	rfk->poll_timer.data = (unsigned long)rfk;
+	add_timer(&rfk->poll_timer);
 
 	return;
-err_unreg_rfk:
-	rfkill_unregister(rfk->rfkill);
-err_free_polldev:
-	input_free_polled_device(rfk->poll_dev);
-	rfk->poll_dev = NULL;
-err_freed_rfk:
+err_free_rfk:
+	rfkill_free(rfk->rfkill);
 	rfk->rfkill = NULL;
 out_error:
 	rfk->registered = 0;
@@ -197,9 +187,8 @@ void b43_rfkill_exit(struct b43_wldev *d
 		return;
 	rfk->registered = 0;
 
-	input_unregister_polled_device(rfk->poll_dev);
+	del_timer(&rfk->poll_timer);
+
 	rfkill_unregister(rfk->rfkill);
-	input_free_polled_device(rfk->poll_dev);
-	rfk->poll_dev = NULL;
 	rfk->rfkill = NULL;
 }
Index: wireless-testing/drivers/net/wireless/b43/rfkill.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/rfkill.h
+++ wireless-testing/drivers/net/wireless/b43/rfkill.h
@@ -7,14 +7,14 @@ struct b43_wldev;
 #ifdef CONFIG_B43_RFKILL
 
 #include <linux/rfkill.h>
-#include <linux/input-polldev.h>
-
 
 struct b43_rfkill {
 	/* The RFKILL subsystem data structure */
 	struct rfkill *rfkill;
-	/* The poll device for the RFKILL input button */
-	struct input_polled_dev *poll_dev;
+	/* The timer list for the RFKILL polling */
+	struct timer_list poll_timer;
+	/* Workqueue for the RFKILL polling */
+	struct work_struct poll_queue;
 	/* Did initialization succeed? Used for freeing. */
 	bool registered;
 	/* The unique name of this rfkill switch */

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 14:07 [RFC V2] b43: A patch for control of the radio LED using rfkill Larry Finger
@ 2008-09-18 14:24 ` Henrique de Moraes Holschuh
  2008-09-18 14:28   ` Ivo van Doorn
  2008-09-18 17:14   ` Michael Buesch
  0 siblings, 2 replies; 13+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-09-18 14:24 UTC (permalink / raw)
  To: Larry Finger; +Cc: ivdoorn, bcm43xx-dev, linux-wireless

On Thu, 18 Sep 2008, Larry Finger wrote:
> The changes below, along with Henriques patch "[PATCH] rfkill: update
> LEDs for all state changes", and the reversion of commit bc19d6e make
> the wireless LED toggle correctly.
> 
> This version passes UNBLOCKED to rfkill when the hardware switch enables the radio.

As I said in an email I just sent (unfortunately, after you already sent
this patch), whatever goes to rfkill_force_state must be the CURRENT state
of the hardware.

So, depending on what happens to b43 hardware when the hardware switch
enables it (i.e. does it enable for real, ignoring the software rfkill input
lines?), this patch might or might not be correct.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 14:24 ` Henrique de Moraes Holschuh
@ 2008-09-18 14:28   ` Ivo van Doorn
  2008-09-18 14:48     ` Henrique de Moraes Holschuh
  2008-09-18 17:14   ` Michael Buesch
  1 sibling, 1 reply; 13+ messages in thread
From: Ivo van Doorn @ 2008-09-18 14:28 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh; +Cc: Larry Finger, bcm43xx-dev, linux-wireless

On Thursday 18 September 2008, Henrique de Moraes Holschuh wrote:
> On Thu, 18 Sep 2008, Larry Finger wrote:
> > The changes below, along with Henriques patch "[PATCH] rfkill: update
> > LEDs for all state changes", and the reversion of commit bc19d6e make
> > the wireless LED toggle correctly.
> > 
> > This version passes UNBLOCKED to rfkill when the hardware switch enables the radio.
> 
> As I said in an email I just sent (unfortunately, after you already sent
> this patch), whatever goes to rfkill_force_state must be the CURRENT state
> of the hardware.
> 
> So, depending on what happens to b43 hardware when the hardware switch
> enables it (i.e. does it enable for real, ignoring the software rfkill input
> lines?), this patch might or might not be correct.

As far as I can see the v2 patch is correct, it sends the BLOCK event when the
rfkill key was pressed to block the radio and the UNBLOCK event when the rfkill
key was pressed to unblock the radio.

The change compared to v1 was needed to prevent BLOCK events to be send
when user has manually disabled the radio for the interface through another
interface then rfkill (and thus wasn't a rfkill event).

Ivo

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 14:28   ` Ivo van Doorn
@ 2008-09-18 14:48     ` Henrique de Moraes Holschuh
  2008-09-18 15:17       ` Larry Finger
  2008-09-18 17:46       ` Michael Buesch
  0 siblings, 2 replies; 13+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-09-18 14:48 UTC (permalink / raw)
  To: Ivo van Doorn; +Cc: Larry Finger, bcm43xx-dev, linux-wireless

On Thu, 18 Sep 2008, Ivo van Doorn wrote:
> On Thursday 18 September 2008, Henrique de Moraes Holschuh wrote:
> > On Thu, 18 Sep 2008, Larry Finger wrote:
> > > The changes below, along with Henriques patch "[PATCH] rfkill: update
> > > LEDs for all state changes", and the reversion of commit bc19d6e make
> > > the wireless LED toggle correctly.
> > > 
> > > This version passes UNBLOCKED to rfkill when the hardware switch enables the radio.
> > 
> > As I said in an email I just sent (unfortunately, after you already sent
> > this patch), whatever goes to rfkill_force_state must be the CURRENT state
> > of the hardware.
> > 
> > So, depending on what happens to b43 hardware when the hardware switch
> > enables it (i.e. does it enable for real, ignoring the software rfkill input
> > lines?), this patch might or might not be correct.
> 
> As far as I can see the v2 patch is correct, it sends the BLOCK event when the
> rfkill key was pressed to block the radio and the UNBLOCK event when the rfkill
> key was pressed to unblock the radio.

If rfkill_force_state() is being used, you MUST send to it the *real* radio
state.  Not the cached radio state, not "desired" radio state, not "user
requested" radio state.  It has to be the *real* radio state.

If a device driver is not doing so, it is incorrect and either it must send
the real radio state, or it should be doing something else than calling
rfkill_force_state().

> The change compared to v1 was needed to prevent BLOCK events to be send
> when user has manually disabled the radio for the interface through another
> interface then rfkill (and thus wasn't a rfkill event).

IMHO if tx power off is handled by the wireless device driver through the
software rfkill line, it DOES MEAN the radio goes into rfkill SOFT_BLOCK.

As long as the rfkill class is kept syncronized with reality through the use
of rfkill_force_state(), this WILL work just fine, because no input events
that change any other devices are ever sent by the rfkill core.

Now, if any input event generation (by the wireless device driver, since
rfkill core NEVER does it) is in the picture, it could be more complicated
(or not... after all, an *INPUT DEVICE* switch would simply *always* match
the *particular* hardware rfkill input line it is tied to, regardless of
radio state -- the input device does not CARE at all about the software
rfkill lines, other hardware rfkill lines, wireless tx power state, or phase
of the moon).

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 14:48     ` Henrique de Moraes Holschuh
@ 2008-09-18 15:17       ` Larry Finger
  2008-09-18 16:24         ` Henrique de Moraes Holschuh
  2008-09-18 17:46       ` Michael Buesch
  1 sibling, 1 reply; 13+ messages in thread
From: Larry Finger @ 2008-09-18 15:17 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh; +Cc: Ivo van Doorn, bcm43xx-dev, linux-wireless

Henrique de Moraes Holschuh wrote:
> 
> IMHO if tx power off is handled by the wireless device driver through the
> software rfkill line, it DOES MEAN the radio goes into rfkill SOFT_BLOCK.
> 
> As long as the rfkill class is kept syncronized with reality through the use
> of rfkill_force_state(), this WILL work just fine, because no input events
> that change any other devices are ever sent by the rfkill core.
> 
> Now, if any input event generation (by the wireless device driver, since
> rfkill core NEVER does it) is in the picture, it could be more complicated
> (or not... after all, an *INPUT DEVICE* switch would simply *always* match
> the *particular* hardware rfkill input line it is tied to, regardless of
> radio state -- the input device does not CARE at all about the software
> rfkill lines, other hardware rfkill lines, wireless tx power state, or phase
> of the moon).

OK, now I'm totally confused. I realize that English is probably not
your first language, but simple declarative sentences would be nice.

The situation with b43 is as follows:

(a) When the hardware switch is off, the radio is hardware blocked.

(b) When the hardware switch is on, the radio should follow whatever
mac80211 and userland wants.

What should b43 do to make this happen? Does V2 do it right?

Larry

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 15:17       ` Larry Finger
@ 2008-09-18 16:24         ` Henrique de Moraes Holschuh
  2008-09-18 16:49           ` Larry Finger
  0 siblings, 1 reply; 13+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-09-18 16:24 UTC (permalink / raw)
  To: Larry Finger; +Cc: Ivo van Doorn, bcm43xx-dev, linux-wireless

On Thu, 18 Sep 2008, Larry Finger wrote:
> Henrique de Moraes Holschuh wrote:
> > IMHO if tx power off is handled by the wireless device driver through the
> > software rfkill line, it DOES MEAN the radio goes into rfkill SOFT_BLOCK.
> > 
> > As long as the rfkill class is kept syncronized with reality through the use
> > of rfkill_force_state(), this WILL work just fine, because no input events
> > that change any other devices are ever sent by the rfkill core.
> > 
> > Now, if any input event generation (by the wireless device driver, since
> > rfkill core NEVER does it) is in the picture, it could be more complicated
> > (or not... after all, an *INPUT DEVICE* switch would simply *always* match
> > the *particular* hardware rfkill input line it is tied to, regardless of
> > radio state -- the input device does not CARE at all about the software
> > rfkill lines, other hardware rfkill lines, wireless tx power state, or phase
> > of the moon).
> 
> OK, now I'm totally confused. I realize that English is probably not
> your first language, but simple declarative sentences would be nice.

I will try :)

> The situation with b43 is as follows:
> 
> (a) When the hardware switch is off, the radio is hardware blocked.
>
> (b) When the hardware switch is on, the radio should follow whatever
> mac80211 and userland wants.

The problem is in the "should".

Maybe something else than b43 (like firmware) changed the radio software
rfkill bit, and it does not match what mac80211 and userland requested
anymore.

What you need to pass to rfkill_force_state() is what the radio is currently
doing.  So, the state you need to send to rfkill_force_state has to be based
on the real state of the hardware's "soft switch bit".

If the hardware "soft switch" bit is write-only, you have no choice but to
use the mac80211 value like your V2 patch does, BTW.

> What should b43 do to make this happen? Does V2 do it right?

Read the software switch state from the hardware instead of using what
mac80211/user wants.  That's all.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 16:24         ` Henrique de Moraes Holschuh
@ 2008-09-18 16:49           ` Larry Finger
  2008-09-18 17:20             ` Michael Buesch
  0 siblings, 1 reply; 13+ messages in thread
From: Larry Finger @ 2008-09-18 16:49 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh; +Cc: Ivo van Doorn, bcm43xx-dev, linux-wireless

Henrique de Moraes Holschuh wrote:
> 
> The problem is in the "should".
> 
> Maybe something else than b43 (like firmware) changed the radio software
> rfkill bit, and it does not match what mac80211 and userland requested
> anymore.
> 
> What you need to pass to rfkill_force_state() is what the radio is currently
> doing.  So, the state you need to send to rfkill_force_state has to be based
> on the real state of the hardware's "soft switch bit".

The hardware does not have such a bit. Once it is initialized and
mac80211 sends it a packet, it will try to send it. That is true even
if the hardware switch is off. It just will not succeed.

> If the hardware "soft switch" bit is write-only, you have no choice but to
> use the mac80211 value like your V2 patch does, BTW.

Since we have only a read-only hardware switch bit, I think you are
saying that V2 is correct.

Larry


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 14:24 ` Henrique de Moraes Holschuh
  2008-09-18 14:28   ` Ivo van Doorn
@ 2008-09-18 17:14   ` Michael Buesch
  1 sibling, 0 replies; 13+ messages in thread
From: Michael Buesch @ 2008-09-18 17:14 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Larry Finger, ivdoorn, bcm43xx-dev, linux-wireless

On Thursday 18 September 2008 16:24:05 Henrique de Moraes Holschuh wrote:
> On Thu, 18 Sep 2008, Larry Finger wrote:
> > The changes below, along with Henriques patch "[PATCH] rfkill: update
> > LEDs for all state changes", and the reversion of commit bc19d6e make
> > the wireless LED toggle correctly.
> > 
> > This version passes UNBLOCKED to rfkill when the hardware switch enables the radio.
> 
> As I said in an email I just sent (unfortunately, after you already sent
> this patch), whatever goes to rfkill_force_state must be the CURRENT state
> of the hardware.
> 
> So, depending on what happens to b43 hardware when the hardware switch
> enables it (i.e. does it enable for real, ignoring the software rfkill input
> lines?), this patch might or might not be correct.

It does enable it, but software might still be blocking it.
sw and hw rfkill are two unrelated things in b43.

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 16:49           ` Larry Finger
@ 2008-09-18 17:20             ` Michael Buesch
  2008-09-18 20:21               ` Henrique de Moraes Holschuh
  0 siblings, 1 reply; 13+ messages in thread
From: Michael Buesch @ 2008-09-18 17:20 UTC (permalink / raw)
  To: bcm43xx-dev
  Cc: Larry Finger, Henrique de Moraes Holschuh, Ivo van Doorn,
	linux-wireless

On Thursday 18 September 2008 18:49:01 Larry Finger wrote:
> Henrique de Moraes Holschuh wrote:
> > 
> > The problem is in the "should".
> > 
> > Maybe something else than b43 (like firmware) changed the radio software
> > rfkill bit, and it does not match what mac80211 and userland requested
> > anymore.
> > 
> > What you need to pass to rfkill_force_state() is what the radio is currently
> > doing.  So, the state you need to send to rfkill_force_state has to be based
> > on the real state of the hardware's "soft switch bit".
> 
> The hardware does not have such a bit. Once it is initialized and
> mac80211 sends it a packet, it will try to send it. That is true even
> if the hardware switch is off. It just will not succeed.

Right, b43 does have two "bits". One read-only bit that tells the hardware
block state. This state can only be changed by physically pressing rfkill
button. The other "bit" is a read/write "bit" to turn off the radio in software.
If _either_ one bit is blocking the radio, it will be physically blocked.

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 14:48     ` Henrique de Moraes Holschuh
  2008-09-18 15:17       ` Larry Finger
@ 2008-09-18 17:46       ` Michael Buesch
  2008-09-18 20:24         ` Henrique de Moraes Holschuh
  1 sibling, 1 reply; 13+ messages in thread
From: Michael Buesch @ 2008-09-18 17:46 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Ivo van Doorn, Larry Finger, bcm43xx-dev, linux-wireless

On Thursday 18 September 2008 16:48:36 Henrique de Moraes Holschuh wrote:
> On Thu, 18 Sep 2008, Ivo van Doorn wrote:
> > On Thursday 18 September 2008, Henrique de Moraes Holschuh wrote:
> > > On Thu, 18 Sep 2008, Larry Finger wrote:
> > > > The changes below, along with Henriques patch "[PATCH] rfkill: update
> > > > LEDs for all state changes", and the reversion of commit bc19d6e make
> > > > the wireless LED toggle correctly.
> > > > 
> > > > This version passes UNBLOCKED to rfkill when the hardware switch enables the radio.
> > > 
> > > As I said in an email I just sent (unfortunately, after you already sent
> > > this patch), whatever goes to rfkill_force_state must be the CURRENT state
> > > of the hardware.
> > > 
> > > So, depending on what happens to b43 hardware when the hardware switch
> > > enables it (i.e. does it enable for real, ignoring the software rfkill input
> > > lines?), this patch might or might not be correct.
> > 
> > As far as I can see the v2 patch is correct, it sends the BLOCK event when the
> > rfkill key was pressed to block the radio and the UNBLOCK event when the rfkill
> > key was pressed to unblock the radio.
> 
> If rfkill_force_state() is being used, you MUST send to it the *real* radio
> state.  Not the cached radio state, not "desired" radio state, not "user
> requested" radio state.  It has to be the *real* radio state.
> 
> If a device driver is not doing so, it is incorrect and either it must send
> the real radio state, or it should be doing something else than calling
> rfkill_force_state().

So well. You tell me this way, and Ivo tells me the other way around to not
announce SW_BLOCKED state ever. ;)
What's actually right?

-- 
Greetings Michael.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 17:20             ` Michael Buesch
@ 2008-09-18 20:21               ` Henrique de Moraes Holschuh
  2008-09-18 20:32                 ` Henrique de Moraes Holschuh
  0 siblings, 1 reply; 13+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-09-18 20:21 UTC (permalink / raw)
  To: Michael Buesch; +Cc: bcm43xx-dev, Larry Finger, Ivo van Doorn, linux-wireless

On Thu, 18 Sep 2008, Michael Buesch wrote:
> On Thursday 18 September 2008 18:49:01 Larry Finger wrote:
> > Henrique de Moraes Holschuh wrote:
> > > The problem is in the "should".
> > > 
> > > Maybe something else than b43 (like firmware) changed the radio software
> > > rfkill bit, and it does not match what mac80211 and userland requested
> > > anymore.
> > > 
> > > What you need to pass to rfkill_force_state() is what the radio is currently
> > > doing.  So, the state you need to send to rfkill_force_state has to be based
> > > on the real state of the hardware's "soft switch bit".
> > 
> > The hardware does not have such a bit. Once it is initialized and
> > mac80211 sends it a packet, it will try to send it. That is true even
> > if the hardware switch is off. It just will not succeed.
> 
> Right, b43 does have two "bits". One read-only bit that tells the hardware
> block state. This state can only be changed by physically pressing rfkill
> button. The other "bit" is a read/write "bit" to turn off the radio in software.
> If _either_ one bit is blocking the radio, it will be physically blocked.

A software-emulated rfkill line is still a SW rfkill line, and the rfkill
core needs to know about it [but you don't want to have set txpower off
state mixed in whatever you send back to the rfkill core].

Since nothing can change an emulater rfkill line behind your driver's back
(bugs and memory corruption don't count :p), you can just use the desired
state of that rfkill line, since it will always match reality.

So yes, V2 is correct, as long as whatever you are reading from mac80211
doesn't have "set txpower off" stuff mixed in it (as I said, I don't know
mac80211).

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 17:46       ` Michael Buesch
@ 2008-09-18 20:24         ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 13+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-09-18 20:24 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Ivo van Doorn, Larry Finger, bcm43xx-dev, linux-wireless

On Thu, 18 Sep 2008, Michael Buesch wrote:
> On Thursday 18 September 2008 16:48:36 Henrique de Moraes Holschuh wrote:
> > On Thu, 18 Sep 2008, Ivo van Doorn wrote:
> > > On Thursday 18 September 2008, Henrique de Moraes Holschuh wrote:
> > > > On Thu, 18 Sep 2008, Larry Finger wrote:
> > > > > The changes below, along with Henriques patch "[PATCH] rfkill: update
> > > > > LEDs for all state changes", and the reversion of commit bc19d6e make
> > > > > the wireless LED toggle correctly.
> > > > > 
> > > > > This version passes UNBLOCKED to rfkill when the hardware switch enables the radio.
> > > > 
> > > > As I said in an email I just sent (unfortunately, after you already sent
> > > > this patch), whatever goes to rfkill_force_state must be the CURRENT state
> > > > of the hardware.
> > > > 
> > > > So, depending on what happens to b43 hardware when the hardware switch
> > > > enables it (i.e. does it enable for real, ignoring the software rfkill input
> > > > lines?), this patch might or might not be correct.
> > > 
> > > As far as I can see the v2 patch is correct, it sends the BLOCK event when the
> > > rfkill key was pressed to block the radio and the UNBLOCK event when the rfkill
> > > key was pressed to unblock the radio.
> > 
> > If rfkill_force_state() is being used, you MUST send to it the *real* radio
> > state.  Not the cached radio state, not "desired" radio state, not "user
> > requested" radio state.  It has to be the *real* radio state.
> > 
> > If a device driver is not doing so, it is incorrect and either it must send
> > the real radio state, or it should be doing something else than calling
> > rfkill_force_state().
> 
> So well. You tell me this way, and Ivo tells me the other way around to not
> announce SW_BLOCKED state ever. ;)
> What's actually right?

Let me actually understand what Ivo means, and I will get back to you.

What he DOES NOT want is to have any RADIO state (as in stuff that takes
into account set txpower off, etc) being fed back into the rfkill core.

And I have no idea of that stuff you read from mac80211 has them mixed in,
or is exclusive for rfkill.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2] b43: A patch for control of the radio LED using rfkill
  2008-09-18 20:21               ` Henrique de Moraes Holschuh
@ 2008-09-18 20:32                 ` Henrique de Moraes Holschuh
  0 siblings, 0 replies; 13+ messages in thread
From: Henrique de Moraes Holschuh @ 2008-09-18 20:32 UTC (permalink / raw)
  To: Michael Buesch; +Cc: bcm43xx-dev, Larry Finger, Ivo van Doorn, linux-wireless

On Thu, 18 Sep 2008, Henrique de Moraes Holschuh wrote:
> On Thu, 18 Sep 2008, Michael Buesch wrote:
> > On Thursday 18 September 2008 18:49:01 Larry Finger wrote:
> > > Henrique de Moraes Holschuh wrote:
> > > > The problem is in the "should".
> > > > 
> > > > Maybe something else than b43 (like firmware) changed the radio software
> > > > rfkill bit, and it does not match what mac80211 and userland requested
> > > > anymore.
> > > > 
> > > > What you need to pass to rfkill_force_state() is what the radio is currently
> > > > doing.  So, the state you need to send to rfkill_force_state has to be based
> > > > on the real state of the hardware's "soft switch bit".
> > > 
> > > The hardware does not have such a bit. Once it is initialized and
> > > mac80211 sends it a packet, it will try to send it. That is true even
> > > if the hardware switch is off. It just will not succeed.
> > 
> > Right, b43 does have two "bits". One read-only bit that tells the hardware
> > block state. This state can only be changed by physically pressing rfkill
> > button. The other "bit" is a read/write "bit" to turn off the radio in software.
> > If _either_ one bit is blocking the radio, it will be physically blocked.
> 
> A software-emulated rfkill line is still a SW rfkill line, and the rfkill
> core needs to know about it [but you don't want to have set txpower off
> state mixed in whatever you send back to the rfkill core].

I forgot to add that you don't always need to do a rfkill_force_state() just
because a SW rfkill line changed.

If that change can only come through the rfkill core (i.e. through the
toggle_radio() hook), the rfkill core already knows about it.

You *still* take the sw rfkill line into account when you do
rfkill_force_state().  It just might not be reason enough to actually call
rfkill_force_state().

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2008-09-18 20:32 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-18 14:07 [RFC V2] b43: A patch for control of the radio LED using rfkill Larry Finger
2008-09-18 14:24 ` Henrique de Moraes Holschuh
2008-09-18 14:28   ` Ivo van Doorn
2008-09-18 14:48     ` Henrique de Moraes Holschuh
2008-09-18 15:17       ` Larry Finger
2008-09-18 16:24         ` Henrique de Moraes Holschuh
2008-09-18 16:49           ` Larry Finger
2008-09-18 17:20             ` Michael Buesch
2008-09-18 20:21               ` Henrique de Moraes Holschuh
2008-09-18 20:32                 ` Henrique de Moraes Holschuh
2008-09-18 17:46       ` Michael Buesch
2008-09-18 20:24         ` Henrique de Moraes Holschuh
2008-09-18 17:14   ` Michael Buesch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).