All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC V2] b43/legacy: port to cfg80211 rfkill
@ 2009-06-05 18:19 Larry Finger
  2009-06-05 19:01 ` Michael Buesch
  0 siblings, 1 reply; 16+ messages in thread
From: Larry Finger @ 2009-06-05 18:19 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Michael Buesch, linux-wireless

From: Johannes Berg <johannes@sipsolutions.net>

This ports the b43/legacy rfkill code to the new API offered
by cfg80211 and thus removes a lot of useless stuff.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Michael Buesch <mb@bu3sch.de>
---

V1 - Original by Johannes.
V2 - Modified in testing by Larry.
     The rfkill polling routine brings the interface back to the initialized
     state if it is found to be uninitialized. This way the rfkill switch
     may be interpreted. In addition, the radio LED is not turned on in the
     initialization routine unless the rfkill switch is on.

 b43/Kconfig           |    7 ---
 b43/Makefile          |    2
 b43/b43.h             |    3 -
 b43/leds.c            |    7 +--
 b43/main.c            |   30 ++++----------
 b43/main.h            |    2
 b43/phy_common.h      |    2
 b43/rfkill.c          |  102 ++++-------------------------------------------
 b43/rfkill.h          |   44 +-------------------
 b43legacy/Kconfig     |    8 ---
 b43legacy/Makefile    |    2
 b43legacy/b43legacy.h |    3 -
 b43legacy/leds.c      |    7 +--
 b43legacy/main.c      |   20 ++-------
 b43legacy/main.h      |    2
 b43legacy/rfkill.c    |  107 ++++----------------------------------------------
 b43legacy/rfkill.h    |   50 +----------------------
 17 files changed, 55 insertions(+), 343 deletions(-)

Index: wireless-testing/drivers/net/wireless/b43/Kconfig
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/Kconfig
+++ wireless-testing/drivers/net/wireless/b43/Kconfig
@@ -98,13 +98,6 @@ config B43_LEDS
 	depends on B43 && MAC80211_LEDS && (LEDS_CLASS = y || LEDS_CLASS = B43)
 	default y
 
-# This config option automatically enables b43 RFKILL support,
-# if it's possible.
-config B43_RFKILL
-	bool
-	depends on B43 && (RFKILL = y || RFKILL = B43)
-	default y
-
 # This config option automatically enables b43 HW-RNG support,
 # if the HW-RNG core is enabled.
 config B43_HWRNG
Index: wireless-testing/drivers/net/wireless/b43/b43.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/b43.h
+++ wireless-testing/drivers/net/wireless/b43/b43.h
@@ -631,9 +631,6 @@ struct b43_wl {
 	char rng_name[30 + 1];
 #endif /* CONFIG_B43_HWRNG */
 
-	/* The RF-kill button */
-	struct b43_rfkill rfkill;
-
 	/* List of all wireless devices on this chip */
 	struct list_head devlist;
 	u8 nr_devs;
Index: wireless-testing/drivers/net/wireless/b43/main.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/main.c
+++ wireless-testing/drivers/net/wireless/b43/main.c
@@ -285,7 +285,6 @@ static struct ieee80211_supported_band b
 };
 
 static void b43_wireless_core_exit(struct b43_wldev *dev);
-static int b43_wireless_core_init(struct b43_wldev *dev);
 static void b43_wireless_core_stop(struct b43_wldev *dev);
 static int b43_wireless_core_start(struct b43_wldev *dev);
 
@@ -4103,7 +4102,7 @@ static void b43_wireless_core_exit(struc
 }
 
 /* Initialize a wireless core */
-static int b43_wireless_core_init(struct b43_wldev *dev)
+int b43_wireless_core_init(struct b43_wldev *dev)
 {
 	struct b43_wl *wl = dev->wl;
 	struct ssb_bus *bus = dev->dev->bus;
@@ -4298,7 +4297,6 @@ static int b43_op_start(struct ieee80211
 	struct b43_wldev *dev = wl->current_dev;
 	int did_init = 0;
 	int err = 0;
-	bool do_rfkill_exit = 0;
 
 	/* Kill all old instance specific information to make sure
 	 * the card won't use it in the short timeframe between start
@@ -4312,18 +4310,12 @@ static int b43_op_start(struct ieee80211
 	wl->beacon1_uploaded = 0;
 	wl->beacon_templates_virgin = 1;
 
-	/* First register RFkill.
-	 * LEDs that are registered later depend on it. */
-	b43_rfkill_init(dev);
-
 	mutex_lock(&wl->mutex);
 
 	if (b43_status(dev) < B43_STAT_INITIALIZED) {
 		err = b43_wireless_core_init(dev);
-		if (err) {
-			do_rfkill_exit = 1;
+		if (err)
 			goto out_mutex_unlock;
-		}
 		did_init = 1;
 	}
 
@@ -4332,17 +4324,16 @@ static int b43_op_start(struct ieee80211
 		if (err) {
 			if (did_init)
 				b43_wireless_core_exit(dev);
-			do_rfkill_exit = 1;
 			goto out_mutex_unlock;
 		}
 	}
 
+	/* XXX: only do if device doesn't support rfkill irq */
+	wiphy_rfkill_start_polling(hw->wiphy);
+
  out_mutex_unlock:
 	mutex_unlock(&wl->mutex);
 
-	if (do_rfkill_exit)
-		b43_rfkill_exit(dev);
-
 	return err;
 }
 
@@ -4351,7 +4342,6 @@ static void b43_op_stop(struct ieee80211
 	struct b43_wl *wl = hw_to_b43_wl(hw);
 	struct b43_wldev *dev = wl->current_dev;
 
-	b43_rfkill_exit(dev);
 	cancel_work_sync(&(wl->beacon_update_trigger));
 
 	mutex_lock(&wl->mutex);
@@ -4433,6 +4423,7 @@ static const struct ieee80211_ops b43_hw
 	.sta_notify		= b43_op_sta_notify,
 	.sw_scan_start		= b43_op_sw_scan_start_notifier,
 	.sw_scan_complete	= b43_op_sw_scan_complete_notifier,
+	.rfkill_poll		= b43_rfkill_poll,
 };
 
 /* Hard-reset the chip. Do not call this directly.
@@ -4920,7 +4911,7 @@ static struct ssb_driver b43_ssb_driver
 static void b43_print_driverinfo(void)
 {
 	const char *feat_pci = "", *feat_pcmcia = "", *feat_nphy = "",
-		   *feat_leds = "", *feat_rfkill = "";
+		   *feat_leds = "";
 
 #ifdef CONFIG_B43_PCI_AUTOSELECT
 	feat_pci = "P";
@@ -4934,14 +4925,11 @@ static void b43_print_driverinfo(void)
 #ifdef CONFIG_B43_LEDS
 	feat_leds = "L";
 #endif
-#ifdef CONFIG_B43_RFKILL
-	feat_rfkill = "R";
-#endif
 	printk(KERN_INFO "Broadcom 43xx driver loaded "
-	       "[ Features: %s%s%s%s%s, Firmware-ID: "
+	       "[ Features: %s%s%s%s, Firmware-ID: "
 	       B43_SUPPORTED_FIRMWARE_ID " ]\n",
 	       feat_pci, feat_pcmcia, feat_nphy,
-	       feat_leds, feat_rfkill);
+	       feat_leds);
 }
 
 static int __init b43_init(void)
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
@@ -22,7 +22,6 @@
 
 */
 
-#include "rfkill.h"
 #include "b43.h"
 #include "phy_common.h"
 
@@ -30,7 +29,7 @@
 
 
 /* Returns TRUE, if the radio is enabled in hardware. */
-static bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
+bool b43_is_hw_radio_enabled(struct b43_wldev *dev)
 {
 	if (dev->phy.rev >= 3) {
 		if (!(b43_read32(dev, B43_MMIO_RADIO_HWENABLED_HI)
@@ -45,110 +44,27 @@ static bool b43_is_hw_radio_enabled(stru
 }
 
 /* The poll callback for the hardware button. */
-static void b43_rfkill_poll(struct rfkill *rfkill, void *data)
+void b43_rfkill_poll(struct ieee80211_hw *hw)
 {
-	struct b43_wldev *dev = data;
-	struct b43_wl *wl = dev->wl;
+	struct b43_wl *wl = hw_to_b43_wl(hw);
+	struct b43_wldev *dev = wl->current_dev;
 	bool enabled;
 
 	mutex_lock(&wl->mutex);
 	if (unlikely(b43_status(dev) < B43_STAT_INITIALIZED)) {
-		mutex_unlock(&wl->mutex);
-		return;
+		if (b43_wireless_core_init(dev)) {
+			mutex_unlock(&wl->mutex);
+			return;
+		}
 	}
 	enabled = b43_is_hw_radio_enabled(dev);
 	if (unlikely(enabled != dev->radio_hw_enable)) {
 		dev->radio_hw_enable = enabled;
 		b43info(wl, "Radio hardware status changed to %s\n",
 			enabled ? "ENABLED" : "DISABLED");
-		enabled = !rfkill_set_hw_state(rfkill, !enabled);
+		wiphy_rfkill_set_hw_state(hw->wiphy, !enabled);
 		if (enabled != dev->phy.radio_on)
 			b43_software_rfkill(dev, !enabled);
 	}
 	mutex_unlock(&wl->mutex);
 }
-
-/* Called when the RFKILL toggled in software. */
-static int b43_rfkill_soft_set(void *data, bool blocked)
-{
-	struct b43_wldev *dev = data;
-	struct b43_wl *wl = dev->wl;
-	int err = -EINVAL;
-
-	if (WARN_ON(!wl->rfkill.registered))
-		return -EINVAL;
-
-	mutex_lock(&wl->mutex);
-
-	if (b43_status(dev) < B43_STAT_INITIALIZED)
-		goto out_unlock;
-
-	if (!dev->radio_hw_enable)
-		goto out_unlock;
-
-	if (!blocked != dev->phy.radio_on)
-		b43_software_rfkill(dev, blocked);
-	err = 0;
-out_unlock:
-	mutex_unlock(&wl->mutex);
-	return err;
-}
-
-const char *b43_rfkill_led_name(struct b43_wldev *dev)
-{
-	struct b43_rfkill *rfk = &(dev->wl->rfkill);
-
-	if (!rfk->registered)
-		return NULL;
-	return rfkill_get_led_trigger_name(rfk->rfkill);
-}
-
-static const struct rfkill_ops b43_rfkill_ops = {
-	.set_block = b43_rfkill_soft_set,
-	.poll = b43_rfkill_poll,
-};
-
-void b43_rfkill_init(struct b43_wldev *dev)
-{
-	struct b43_wl *wl = dev->wl;
-	struct b43_rfkill *rfk = &(wl->rfkill);
-	int err;
-
-	rfk->registered = 0;
-
-	snprintf(rfk->name, sizeof(rfk->name),
-		 "b43-%s", wiphy_name(wl->hw->wiphy));
-
-	rfk->rfkill = rfkill_alloc(rfk->name,
-				   dev->dev->dev,
-				   RFKILL_TYPE_WLAN,
-				   &b43_rfkill_ops, dev);
-	if (!rfk->rfkill)
-		goto out_error;
-
-	err = rfkill_register(rfk->rfkill);
-	if (err)
-		goto err_free;
-
-	rfk->registered = 1;
-
-	return;
- err_free:
-	rfkill_destroy(rfk->rfkill);
- out_error:
-	rfk->registered = 0;
-	b43warn(wl, "RF-kill button init failed\n");
-}
-
-void b43_rfkill_exit(struct b43_wldev *dev)
-{
-	struct b43_rfkill *rfk = &(dev->wl->rfkill);
-
-	if (!rfk->registered)
-		return;
-	rfk->registered = 0;
-
-	rfkill_unregister(rfk->rfkill);
-	rfkill_destroy(rfk->rfkill);
-	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
@@ -1,49 +1,13 @@
 #ifndef B43_RFKILL_H_
 #define B43_RFKILL_H_
 
+struct ieee80211_hw;
 struct b43_wldev;
 
+void b43_rfkill_poll(struct ieee80211_hw *hw);
 
-#ifdef CONFIG_B43_RFKILL
+int b43_wireless_core_init(struct b43_wldev *dev);
 
-#include <linux/rfkill.h>
-
-
-struct b43_rfkill {
-	/* The RFKILL subsystem data structure */
-	struct rfkill *rfkill;
-	/* Did initialization succeed? Used for freeing. */
-	bool registered;
-	/* The unique name of this rfkill switch */
-	char name[sizeof("b43-phy4294967295")];
-};
-
-/* The init function returns void, because we are not interested
- * in failing the b43 init process when rfkill init failed. */
-void b43_rfkill_init(struct b43_wldev *dev);
-void b43_rfkill_exit(struct b43_wldev *dev);
-
-const char *b43_rfkill_led_name(struct b43_wldev *dev);
-
-
-#else /* CONFIG_B43_RFKILL */
-/* No RFKILL support. */
-
-struct b43_rfkill {
-	/* empty */
-};
-
-static inline void b43_rfkill_init(struct b43_wldev *dev)
-{
-}
-static inline void b43_rfkill_exit(struct b43_wldev *dev)
-{
-}
-static inline char * b43_rfkill_led_name(struct b43_wldev *dev)
-{
-	return NULL;
-}
-
-#endif /* CONFIG_B43_RFKILL */
+bool b43_is_hw_radio_enabled(struct b43_wldev *dev);
 
 #endif /* B43_RFKILL_H_ */
Index: wireless-testing/drivers/net/wireless/b43/Makefile
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/Makefile
+++ wireless-testing/drivers/net/wireless/b43/Makefile
@@ -13,7 +13,7 @@ b43-y				+= lo.o
 b43-y				+= wa.o
 b43-y				+= dma.o
 b43-$(CONFIG_B43_PIO)		+= pio.o
-b43-$(CONFIG_B43_RFKILL)	+= rfkill.o
+b43-y				+= rfkill.o
 b43-$(CONFIG_B43_LEDS)		+= leds.o
 b43-$(CONFIG_B43_PCMCIA)	+= pcmcia.o
 b43-$(CONFIG_B43_DEBUG)		+= debugfs.o
Index: wireless-testing/drivers/net/wireless/b43legacy/Kconfig
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/Kconfig
+++ wireless-testing/drivers/net/wireless/b43legacy/Kconfig
@@ -42,14 +42,6 @@ config B43LEGACY_LEDS
 	depends on B43LEGACY && MAC80211_LEDS && (LEDS_CLASS = y || LEDS_CLASS = B43LEGACY)
 	default y
 
-# RFKILL support
-# This config option automatically enables b43legacy RFKILL support,
-# if it's possible.
-config B43LEGACY_RFKILL
-	bool
-	depends on B43LEGACY && (RFKILL = y || RFKILL = B43LEGACY)
-	default y
-
 # This config option automatically enables b43 HW-RNG support,
 # if the HW-RNG core is enabled.
 config B43LEGACY_HWRNG
Index: wireless-testing/drivers/net/wireless/b43legacy/Makefile
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/Makefile
+++ wireless-testing/drivers/net/wireless/b43legacy/Makefile
@@ -6,7 +6,7 @@ b43legacy-y				+= radio.o
 b43legacy-y				+= sysfs.o
 b43legacy-y				+= xmit.o
 # b43 RFKILL button support
-b43legacy-$(CONFIG_B43LEGACY_RFKILL)	+= rfkill.o
+b43legacy-y				+= rfkill.o
 # b43legacy LED support
 b43legacy-$(CONFIG_B43LEGACY_LEDS)	+= leds.o
 # b43legacy debugging
Index: wireless-testing/drivers/net/wireless/b43legacy/main.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/main.c
+++ wireless-testing/drivers/net/wireless/b43legacy/main.c
@@ -159,7 +159,6 @@ static struct ieee80211_supported_band b
 };
 
 static void b43legacy_wireless_core_exit(struct b43legacy_wldev *dev);
-static int b43legacy_wireless_core_init(struct b43legacy_wldev *dev);
 static void b43legacy_wireless_core_stop(struct b43legacy_wldev *dev);
 static int b43legacy_wireless_core_start(struct b43legacy_wldev *dev);
 
@@ -3230,7 +3229,7 @@ static void prepare_phy_data_for_init(st
 }
 
 /* Initialize a wireless core */
-static int b43legacy_wireless_core_init(struct b43legacy_wldev *dev)
+int b43legacy_wireless_core_init(struct b43legacy_wldev *dev)
 {
 	struct b43legacy_wl *wl = dev->wl;
 	struct ssb_bus *bus = dev->dev->bus;
@@ -3431,11 +3430,6 @@ static int b43legacy_op_start(struct iee
 	struct b43legacy_wldev *dev = wl->current_dev;
 	int did_init = 0;
 	int err = 0;
-	bool do_rfkill_exit = 0;
-
-	/* First register RFkill.
-	 * LEDs that are registered later depend on it. */
-	b43legacy_rfkill_init(dev);
 
 	/* Kill all old instance specific information to make sure
 	 * the card won't use it in the short timeframe between start
@@ -3451,10 +3445,8 @@ static int b43legacy_op_start(struct iee
 
 	if (b43legacy_status(dev) < B43legacy_STAT_INITIALIZED) {
 		err = b43legacy_wireless_core_init(dev);
-		if (err) {
-			do_rfkill_exit = 1;
+		if (err)
 			goto out_mutex_unlock;
-		}
 		did_init = 1;
 	}
 
@@ -3463,17 +3455,15 @@ static int b43legacy_op_start(struct iee
 		if (err) {
 			if (did_init)
 				b43legacy_wireless_core_exit(dev);
-			do_rfkill_exit = 1;
 			goto out_mutex_unlock;
 		}
 	}
 
+	wiphy_rfkill_start_polling(hw->wiphy);
+
 out_mutex_unlock:
 	mutex_unlock(&wl->mutex);
 
-	if (do_rfkill_exit)
-		b43legacy_rfkill_exit(dev);
-
 	return err;
 }
 
@@ -3482,7 +3472,6 @@ static void b43legacy_op_stop(struct iee
 	struct b43legacy_wl *wl = hw_to_b43legacy_wl(hw);
 	struct b43legacy_wldev *dev = wl->current_dev;
 
-	b43legacy_rfkill_exit(dev);
 	cancel_work_sync(&(wl->beacon_update_trigger));
 
 	mutex_lock(&wl->mutex);
@@ -3518,6 +3507,7 @@ static const struct ieee80211_ops b43leg
 	.start			= b43legacy_op_start,
 	.stop			= b43legacy_op_stop,
 	.set_tim		= b43legacy_op_beacon_set_tim,
+	.rfkill_poll		= b43legacy_rfkill_poll,
 };
 
 /* Hard-reset the chip. Do not call this directly.
Index: wireless-testing/drivers/net/wireless/b43legacy/rfkill.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/rfkill.c
+++ wireless-testing/drivers/net/wireless/b43legacy/rfkill.c
@@ -22,7 +22,6 @@
 
 */
 
-#include "rfkill.h"
 #include "radio.h"
 #include "b43legacy.h"
 
@@ -30,7 +29,7 @@
 
 
 /* Returns TRUE, if the radio is enabled in hardware. */
-static bool b43legacy_is_hw_radio_enabled(struct b43legacy_wldev *dev)
+bool b43legacy_is_hw_radio_enabled(struct b43legacy_wldev *dev)
 {
 	if (dev->phy.rev >= 3) {
 		if (!(b43legacy_read32(dev, B43legacy_MMIO_RADIO_HWENABLED_HI)
@@ -45,23 +44,25 @@ static bool b43legacy_is_hw_radio_enable
 }
 
 /* The poll callback for the hardware button. */
-static void b43legacy_rfkill_poll(struct rfkill *rfkill, void *data)
+void b43legacy_rfkill_poll(struct ieee80211_hw *hw)
 {
-	struct b43legacy_wldev *dev = data;
-	struct b43legacy_wl *wl = dev->wl;
+	struct b43legacy_wl *wl = hw_to_b43legacy_wl(hw);
+	struct b43legacy_wldev *dev = wl->current_dev;
 	bool enabled;
 
 	mutex_lock(&wl->mutex);
 	if (unlikely(b43legacy_status(dev) < B43legacy_STAT_INITIALIZED)) {
-		mutex_unlock(&wl->mutex);
-		return;
+		if (b43legacy_wireless_core_init(dev)) {
+			mutex_unlock(&wl->mutex);
+			return;
+		}
 	}
 	enabled = b43legacy_is_hw_radio_enabled(dev);
 	if (unlikely(enabled != dev->radio_hw_enable)) {
 		dev->radio_hw_enable = enabled;
 		b43legacyinfo(wl, "Radio hardware status changed to %s\n",
 			enabled ? "ENABLED" : "DISABLED");
-		enabled = !rfkill_set_hw_state(rfkill, !enabled);
+		wiphy_rfkill_set_hw_state(hw->wiphy, !enabled);
 		if (enabled != dev->phy.radio_on) {
 			if (enabled)
 				b43legacy_radio_turn_on(dev);
@@ -71,93 +72,3 @@ static void b43legacy_rfkill_poll(struct
 	}
 	mutex_unlock(&wl->mutex);
 }
-
-/* Called when the RFKILL toggled in software.
- * This is called without locking. */
-static int b43legacy_rfkill_soft_set(void *data, bool blocked)
-{
-	struct b43legacy_wldev *dev = data;
-	struct b43legacy_wl *wl = dev->wl;
-	int ret = -EINVAL;
-
-	if (!wl->rfkill.registered)
-		return -EINVAL;
-
-	mutex_lock(&wl->mutex);
-	if (b43legacy_status(dev) < B43legacy_STAT_INITIALIZED)
-		goto out_unlock;
-
-	if (!dev->radio_hw_enable)
-		goto out_unlock;
-
-	if (!blocked != dev->phy.radio_on) {
-		if (!blocked)
-			b43legacy_radio_turn_on(dev);
-		else
-			b43legacy_radio_turn_off(dev, 0);
-	}
-	ret = 0;
-
-out_unlock:
-	mutex_unlock(&wl->mutex);
-	return ret;
-}
-
-const char *b43legacy_rfkill_led_name(struct b43legacy_wldev *dev)
-{
-	struct b43legacy_rfkill *rfk = &(dev->wl->rfkill);
-
-	if (!rfk->registered)
-		return NULL;
-	return rfkill_get_led_trigger_name(rfk->rfkill);
-}
-
-static const struct rfkill_ops b43legacy_rfkill_ops = {
-	.set_block = b43legacy_rfkill_soft_set,
-	.poll = b43legacy_rfkill_poll,
-};
-
-void b43legacy_rfkill_init(struct b43legacy_wldev *dev)
-{
-	struct b43legacy_wl *wl = dev->wl;
-	struct b43legacy_rfkill *rfk = &(wl->rfkill);
-	int err;
-
-	rfk->registered = 0;
-
-	snprintf(rfk->name, sizeof(rfk->name),
-		 "b43legacy-%s", wiphy_name(wl->hw->wiphy));
-	rfk->rfkill = rfkill_alloc(rfk->name,
-				   dev->dev->dev,
-				   RFKILL_TYPE_WLAN,
-				   &b43legacy_rfkill_ops, dev);
-	if (!rfk->rfkill)
-		goto out_error;
-
-	err = rfkill_register(rfk->rfkill);
-	if (err)
-		goto err_free;
-
-	rfk->registered = 1;
-
-	return;
- err_free:
-	rfkill_destroy(rfk->rfkill);
- out_error:
-	rfk->registered = 0;
-	b43legacywarn(wl, "RF-kill button init failed\n");
-}
-
-void b43legacy_rfkill_exit(struct b43legacy_wldev *dev)
-{
-	struct b43legacy_rfkill *rfk = &(dev->wl->rfkill);
-
-	if (!rfk->registered)
-		return;
-	rfk->registered = 0;
-
-	rfkill_unregister(rfk->rfkill);
-	rfkill_destroy(rfk->rfkill);
-	rfk->rfkill = NULL;
-}
-
Index: wireless-testing/drivers/net/wireless/b43legacy/rfkill.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/rfkill.h
+++ wireless-testing/drivers/net/wireless/b43legacy/rfkill.h
@@ -1,55 +1,13 @@
 #ifndef B43legacy_RFKILL_H_
 #define B43legacy_RFKILL_H_
 
+struct ieee80211_hw;
 struct b43legacy_wldev;
 
-#ifdef CONFIG_B43LEGACY_RFKILL
+void b43legacy_rfkill_poll(struct ieee80211_hw *hw);
 
-#include <linux/rfkill.h>
+int b43legacy_wireless_core_init(struct b43legacy_wldev *dev);
 
-
-
-struct b43legacy_rfkill {
-	/* The RFKILL subsystem data structure */
-	struct rfkill *rfkill;
-	/* Did initialization succeed? Used for freeing. */
-	bool registered;
-	/* The unique name of this rfkill switch */
-	char name[sizeof("b43legacy-phy4294967295")];
-};
-
-/* The init function returns void, because we are not interested
- * in failing the b43 init process when rfkill init failed. */
-void b43legacy_rfkill_init(struct b43legacy_wldev *dev);
-void b43legacy_rfkill_exit(struct b43legacy_wldev *dev);
-
-const char *b43legacy_rfkill_led_name(struct b43legacy_wldev *dev);
-
-
-#else /* CONFIG_B43LEGACY_RFKILL */
-/* No RFKILL support. */
-
-struct b43legacy_rfkill {
-	/* empty */
-};
-
-static inline void b43legacy_rfkill_alloc(struct b43legacy_wldev *dev)
-{
-}
-static inline void b43legacy_rfkill_free(struct b43legacy_wldev *dev)
-{
-}
-static inline void b43legacy_rfkill_init(struct b43legacy_wldev *dev)
-{
-}
-static inline void b43legacy_rfkill_exit(struct b43legacy_wldev *dev)
-{
-}
-static inline char *b43legacy_rfkill_led_name(struct b43legacy_wldev *dev)
-{
-	return NULL;
-}
-
-#endif /* CONFIG_B43LEGACY_RFKILL */
+bool b43legacy_is_hw_radio_enabled(struct b43legacy_wldev *dev);
 
 #endif /* B43legacy_RFKILL_H_ */
Index: wireless-testing/drivers/net/wireless/b43legacy/b43legacy.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/b43legacy.h
+++ wireless-testing/drivers/net/wireless/b43legacy/b43legacy.h
@@ -602,9 +602,6 @@ struct b43legacy_wl {
 	char rng_name[30 + 1];
 #endif
 
-	/* The RF-kill button */
-	struct b43legacy_rfkill rfkill;
-
 	/* List of all wireless devices on this chip */
 	struct list_head devlist;
 	u8 nr_devs;
Index: wireless-testing/drivers/net/wireless/b43/phy_common.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/phy_common.h
+++ wireless-testing/drivers/net/wireless/b43/phy_common.h
@@ -1,7 +1,7 @@
 #ifndef LINUX_B43_PHY_COMMON_H_
 #define LINUX_B43_PHY_COMMON_H_
 
-#include <linux/rfkill.h>
+#include <linux/types.h>
 
 struct b43_wldev;
 
Index: wireless-testing/drivers/net/wireless/b43/leds.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/leds.c
+++ wireless-testing/drivers/net/wireless/b43/leds.c
@@ -28,6 +28,7 @@
 
 #include "b43.h"
 #include "leds.h"
+#include "rfkill.h"
 
 
 static void b43_led_turn_on(struct b43_wldev *dev, u8 led_index,
@@ -164,10 +165,10 @@ static void b43_map_led(struct b43_wldev
 		snprintf(name, sizeof(name),
 			 "b43-%s::radio", wiphy_name(hw->wiphy));
 		b43_register_led(dev, &dev->led_radio, name,
-				 b43_rfkill_led_name(dev),
+				 ieee80211_get_radio_led_name(hw),
 				 led_index, activelow);
-		/* Sync the RF-kill LED state with the switch state. */
-		if (dev->radio_hw_enable)
+		/* Sync the RF-kill LED state with radio and switch states. */
+		if (dev->phy.radio_on && b43_is_hw_radio_enabled(dev))
 			b43_led_turn_on(dev, led_index, activelow);
 		break;
 	case B43_LED_WEIRD:
Index: wireless-testing/drivers/net/wireless/b43legacy/leds.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/leds.c
+++ wireless-testing/drivers/net/wireless/b43legacy/leds.c
@@ -28,6 +28,7 @@
 
 #include "b43legacy.h"
 #include "leds.h"
+#include "rfkill.h"
 
 
 static void b43legacy_led_turn_on(struct b43legacy_wldev *dev, u8 led_index,
@@ -164,10 +165,10 @@ static void b43legacy_map_led(struct b43
 		snprintf(name, sizeof(name),
 			 "b43legacy-%s::radio", wiphy_name(hw->wiphy));
 		b43legacy_register_led(dev, &dev->led_radio, name,
-				 b43legacy_rfkill_led_name(dev),
+				 ieee80211_get_radio_led_name(hw),
 				 led_index, activelow);
-		/* Sync the RF-kill LED state with the switch state. */
-		if (dev->radio_hw_enable)
+		/* Sync the RF-kill LED state with radio and switch states. */
+		if (dev->phy.radio_on && b43legacy_is_hw_radio_enabled(dev))
 			b43legacy_led_turn_on(dev, led_index, activelow);
 		break;
 	case B43legacy_LED_WEIRD:
Index: wireless-testing/drivers/net/wireless/b43/main.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43/main.h
+++ wireless-testing/drivers/net/wireless/b43/main.h
@@ -128,6 +128,8 @@ void b43_dummy_transmission(struct b43_w
 
 void b43_wireless_core_reset(struct b43_wldev *dev, u32 flags);
 
+int b43_wireless_core_init(struct b43_wldev *dev);
+
 void b43_controller_restart(struct b43_wldev *dev, const char *reason);
 
 #define B43_PS_ENABLED	(1 << 0)	/* Force enable hardware power saving */
Index: wireless-testing/drivers/net/wireless/b43legacy/main.h
===================================================================
--- wireless-testing.orig/drivers/net/wireless/b43legacy/main.h
+++ wireless-testing/drivers/net/wireless/b43legacy/main.h
@@ -118,6 +118,8 @@ void b43legacy_dummy_transmission(struct
 
 void b43legacy_wireless_core_reset(struct b43legacy_wldev *dev, u32 flags);
 
+int b43legacy_wireless_core_init(struct b43legacy_wldev *dev);
+
 void b43legacy_mac_suspend(struct b43legacy_wldev *dev);
 void b43legacy_mac_enable(struct b43legacy_wldev *dev);
 

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-05 18:19 [RFC V2] b43/legacy: port to cfg80211 rfkill Larry Finger
@ 2009-06-05 19:01 ` Michael Buesch
  2009-06-05 21:20   ` Larry Finger
  2009-06-06 17:42   ` Johannes Berg
  0 siblings, 2 replies; 16+ messages in thread
From: Michael Buesch @ 2009-06-05 19:01 UTC (permalink / raw)
  To: Larry Finger; +Cc: Johannes Berg, linux-wireless

On Friday 05 June 2009 20:19:19 Larry Finger wrote:
> From: Johannes Berg <johannes@sipsolutions.net>
> 
> This ports the b43/legacy rfkill code to the new API offered
> by cfg80211 and thus removes a lot of useless stuff.
> 
> Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Michael Buesch <mb@bu3sch.de>
> ---
> 
> V1 - Original by Johannes.
> V2 - Modified in testing by Larry.
>      The rfkill polling routine brings the interface back to the initialized
>      state if it is found to be uninitialized. This way the rfkill switch
>      may be interpreted. In addition, the radio LED is not turned on in the
>      initialization routine unless the rfkill switch is on.

This is pretty silly behavior IMO. Just to bring it to the point:
We initialize a huge wireless MAC, PHY and Radio that consume several watts of power
just to poll a silly RF-kill bit.

We can't we just accept that the RF-kill status is unknown while the device is down?

I really do hate all that rfkill crap and I'm still refusing to sign off on anything that's
related to rfkill (like I did for the past year or so). If people want this merged,
somebody else maintain and sign it off, please.

-- 
Greetings, Michael.

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-05 19:01 ` Michael Buesch
@ 2009-06-05 21:20   ` Larry Finger
  2009-06-05 21:38     ` Michael Buesch
                       ` (2 more replies)
  2009-06-06 17:42   ` Johannes Berg
  1 sibling, 3 replies; 16+ messages in thread
From: Larry Finger @ 2009-06-05 21:20 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Johannes Berg, linux-wireless

Michael Buesch wrote:
> 
> This is pretty silly behavior IMO. Just to bring it to the point:
> We initialize a huge wireless MAC, PHY and Radio that consume several watts of power
> just to poll a silly RF-kill bit.

That is what the driver already does.

> We can't we just accept that the RF-kill status is unknown while the device is down?

The problem is that while the interface is down the switch status
cannot be interrogated. If you try, you get a fatal SSB error. Thus
the only way to bring it back up is to flip the switch, then
rmmod/insmod the driver. If you want hardware rfkill to be one-way,
then take Johannes's patch. We would save a little power by calling
b43_wireless_exit() if we brought it up to test the switch, and the
switch was still off. That would leave everything off most of the time.

> I really do hate all that rfkill crap and I'm still refusing to sign off on anything that's
> related to rfkill (like I did for the past year or so). If people want this merged,
> somebody else maintain and sign it off, please.

I'm sick of rfkill as well and really detest the endless discussions
that have taken place; however, I do want the stuff to work.

As I see it, we have several options (presented in my order of
preference):

1. We switch to the cfg80211 rfkill and use this patch modified to
turn the interface back off if the switch is still off.

2. We continue to use the old rfkill mechanism. It works just fine,
but this method runs the risk of the old method being deprecated and
eliminated.

3. We get new callbacks that will only power down/up the radio when it
is blocked. That saves a little power.

Larry


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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-05 21:20   ` Larry Finger
@ 2009-06-05 21:38     ` Michael Buesch
  2009-06-05 22:32       ` Johannes Berg
  2009-06-05 22:38       ` Larry Finger
  2009-06-06 17:44     ` Johannes Berg
  2009-06-06 18:00     ` Michael Buesch
  2 siblings, 2 replies; 16+ messages in thread
From: Michael Buesch @ 2009-06-05 21:38 UTC (permalink / raw)
  To: Larry Finger; +Cc: Johannes Berg, linux-wireless

On Friday 05 June 2009 23:20:55 Larry Finger wrote:
> Michael Buesch wrote:
> > 
> > This is pretty silly behavior IMO. Just to bring it to the point:
> > We initialize a huge wireless MAC, PHY and Radio that consume several watts of power
> > just to poll a silly RF-kill bit.
> 
> That is what the driver already does.

No. If the device is down, then it _is_ down. There won't be an rfkill thing
trying to bring it up again although the interface is down.

> > We can't we just accept that the RF-kill status is unknown while the device is down?
> 
> The problem is that while the interface is down the switch status
> cannot be interrogated. If you try, you get a fatal SSB error. Thus
> the only way to bring it back up is to flip the switch, then
> rmmod/insmod the driver. If you want hardware rfkill to be one-way,
> then take Johannes's patch. We would save a little power by calling
> b43_wireless_exit() if we brought it up to test the switch, and the
> switch was still off. That would leave everything off most of the time.

Yeah well. We cannot read the rfkill status while the device is down. That is
a hardware limitation. I think we should _live_ with that limitation instead of
working around it by always keeping the device initialized.
Can't we teach the rfkill subsystem about an "unknown" state? Because that's what we're in.

> > I really do hate all that rfkill crap and I'm still refusing to sign off on anything that's
> > related to rfkill (like I did for the past year or so). If people want this merged,
> > somebody else maintain and sign it off, please.
> 
> I'm sick of rfkill as well and really detest the endless discussions
> that have taken place; however, I do want the stuff to work.

Yeah. But wasting huge amounts of power to keep polling a bit that's not even used
most of the time is not really what I like.

> 1. We switch to the cfg80211 rfkill and use this patch modified to
> turn the interface back off if the switch is still off.

+void b43_rfkill_poll(struct ieee80211_hw *hw)
 {
-       struct b43_wldev *dev = data;
-       struct b43_wl *wl = dev->wl;
+       struct b43_wl *wl = hw_to_b43_wl(hw);
+       struct b43_wldev *dev = wl->current_dev;
        bool enabled;
 
        mutex_lock(&wl->mutex);
        if (unlikely(b43_status(dev) < B43_STAT_INITIALIZED)) {
-               mutex_unlock(&wl->mutex);
-               return;
+               if (b43_wireless_core_init(dev)) {
+                       mutex_unlock(&wl->mutex);
+                       return;
+               }
        }

This is the part of the code which I really really really dislike.
Hell, just return a freaking error from b43_rfkill_poll(), if the interface
is down. If rfkill can't handle that, it should probably be taught to handle it.
Especially as there can be other errors as well, like memory allocation failures.

> 2. We continue to use the old rfkill mechanism. It works just fine,
> but this method runs the risk of the old method being deprecated and
> eliminated.

I agree that this is not really an option.

> 3. We get new callbacks that will only power down/up the radio when it
> is blocked. That saves a little power.

What is wrong with the current mechanism to power up the radio, if the interface is up
and powering it down if the interface is down? I think the power of the PHY/Radio should
not be affected by rfkill. It should work the other way around instead. Rfkill should
be tolerant to a radio that is down and simply live with an unknown switch state.

-- 
Greetings, Michael.

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-05 21:38     ` Michael Buesch
@ 2009-06-05 22:32       ` Johannes Berg
  2009-06-06  9:34         ` Michael Buesch
  2009-06-05 22:38       ` Larry Finger
  1 sibling, 1 reply; 16+ messages in thread
From: Johannes Berg @ 2009-06-05 22:32 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Larry Finger, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 732 bytes --]

On Fri, 2009-06-05 at 23:38 +0200, Michael Buesch wrote:

> Hell, just return a freaking error from b43_rfkill_poll(), if the interface
> is down. If rfkill can't handle that, it should probably be taught to handle it.
> Especially as there can be other errors as well, like memory allocation failures.

Be my guest. You'll notice eventually that it's not really easy or
possible to do. I've explained the currently broken scenario already.
And even if you introduce the 'unknown' state, you still end up having
to poll the bit when userspace tries to turn on the device... doesn't
help much.

Larry, thanks for your patch, I'll modify it to turn off the core again
after checking the rfkill bit tomorrow.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-05 21:38     ` Michael Buesch
  2009-06-05 22:32       ` Johannes Berg
@ 2009-06-05 22:38       ` Larry Finger
  2009-06-06  9:38         ` Michael Buesch
  2009-06-06 17:37         ` Johannes Berg
  1 sibling, 2 replies; 16+ messages in thread
From: Larry Finger @ 2009-06-05 22:38 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Johannes Berg, linux-wireless

Michael Buesch wrote:
> On Friday 05 June 2009 23:20:55 Larry Finger wrote:
>> That is what the driver already does.
> 
> No. If the device is down, then it _is_ down. There won't be an rfkill thing
> trying to bring it up again although the interface is down.

But in the current driver, turning the radio switch off only shuts
down the radio. The rest of the interface is still up.

>>> We can't we just accept that the RF-kill status is unknown while the device is down?
>> The problem is that while the interface is down the switch status
>> cannot be interrogated. If you try, you get a fatal SSB error. Thus
>> the only way to bring it back up is to flip the switch, then
>> rmmod/insmod the driver. If you want hardware rfkill to be one-way,
>> then take Johannes's patch. We would save a little power by calling
>> b43_wireless_exit() if we brought it up to test the switch, and the
>> switch was still off. That would leave everything off most of the time.
> 
> Yeah well. We cannot read the rfkill status while the device is down. That is
> a hardware limitation. I think we should _live_ with that limitation instead of
> working around it by always keeping the device initialized.
> Can't we teach the rfkill subsystem about an "unknown" state? Because that's what we're in.

An "unknown" state would be OK, but I don't know how to get the state
of the switch to the rfkill system.

> Yeah. But wasting huge amounts of power to keep polling a bit that's not even used
> most of the time is not really what I like.


> +void b43_rfkill_poll(struct ieee80211_hw *hw)
>  {
> -       struct b43_wldev *dev = data;
> -       struct b43_wl *wl = dev->wl;
> +       struct b43_wl *wl = hw_to_b43_wl(hw);
> +       struct b43_wldev *dev = wl->current_dev;
>         bool enabled;
>  
>         mutex_lock(&wl->mutex);
>         if (unlikely(b43_status(dev) < B43_STAT_INITIALIZED)) {
> -               mutex_unlock(&wl->mutex);
> -               return;
> +               if (b43_wireless_core_init(dev)) {
> +                       mutex_unlock(&wl->mutex);
> +                       return;
> +               }
>         }
> 
> This is the part of the code which I really really really dislike.
> Hell, just return a freaking error from b43_rfkill_poll(), if the interface
> is down. If rfkill can't handle that, it should probably be taught to handle it.
> Especially as there can be other errors as well, like memory allocation failures.

I understand that this is the part that you do not like. I don't like
it either, but unless we can read the switch-state bit, the interface
will never come back up. I do not find rmmod/insmod a suitable
recovery mechanism.

>> 2. We continue to use the old rfkill mechanism. It works just fine,
>> but this method runs the risk of the old method being deprecated and
>> eliminated.
> 
> I agree that this is not really an option.
> 
>> 3. We get new callbacks that will only power down/up the radio when it
>> is blocked. That saves a little power.
> 
> What is wrong with the current mechanism to power up the radio, if the interface is up
> and powering it down if the interface is down? I think the power of the PHY/Radio should
> not be affected by rfkill. It should work the other way around instead. Rfkill should
> be tolerant to a radio that is down and simply live with an unknown switch state.

I don't know what was intended in the new rfkill method. Remember, I
don't read those E-mails either, but what I observe is that cfg80211
calls the stop callback when the switch goes from on to off, but the
start callback is not called when the off to on transition occurs.
That may be a bug - I don't know. Johannes, is there a simple
explanation as to why the start routine is not called? If that were to
happen, the rfkill poll routine would happily exit immediately if the
interface were down.

I think rfkill should just be a means of letting drivers know that the
user wants the radio off, either by turning off a switch, or by
setting some software flag from userland, and the driver should honor
those wishes.

Larry


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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-05 22:32       ` Johannes Berg
@ 2009-06-06  9:34         ` Michael Buesch
  2009-06-06 17:38           ` Johannes Berg
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Buesch @ 2009-06-06  9:34 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Larry Finger, linux-wireless

On Saturday 06 June 2009 00:32:06 Johannes Berg wrote:
> On Fri, 2009-06-05 at 23:38 +0200, Michael Buesch wrote:
> 
> > Hell, just return a freaking error from b43_rfkill_poll(), if the interface
> > is down. If rfkill can't handle that, it should probably be taught to handle it.
> > Especially as there can be other errors as well, like memory allocation failures.
> 
> Be my guest. You'll notice eventually that it's not really easy or
> possible to do. I've explained the currently broken scenario already.
> And even if you introduce the 'unknown' state, you still end up having
> to poll the bit when userspace tries to turn on the device... doesn't
> help much.

It is OK to poll the bit while the device is supposed to be on. But it's not
OK to turn it on to check the bit.

> Larry, thanks for your patch, I'll modify it to turn off the core again
> after checking the rfkill bit tomorrow.

How frequent will that polling happen?

-- 
Greetings, Michael.

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-05 22:38       ` Larry Finger
@ 2009-06-06  9:38         ` Michael Buesch
  2009-06-06 17:38           ` Johannes Berg
  2009-06-06 17:37         ` Johannes Berg
  1 sibling, 1 reply; 16+ messages in thread
From: Michael Buesch @ 2009-06-06  9:38 UTC (permalink / raw)
  To: Larry Finger; +Cc: Johannes Berg, linux-wireless

On Saturday 06 June 2009 00:38:15 Larry Finger wrote:
> Michael Buesch wrote:
> > On Friday 05 June 2009 23:20:55 Larry Finger wrote:
> >> That is what the driver already does.
> > 
> > No. If the device is down, then it _is_ down. There won't be an rfkill thing
> > trying to bring it up again although the interface is down.
> 
> But in the current driver, turning the radio switch off only shuts
> down the radio. The rest of the interface is still up.

This is not what I'm talking about.
I was talking about bringing the device up just for the purpose of checking the bit.

> >>> We can't we just accept that the RF-kill status is unknown while the device is down?
> >> The problem is that while the interface is down the switch status
> >> cannot be interrogated. If you try, you get a fatal SSB error. Thus
> >> the only way to bring it back up is to flip the switch, then
> >> rmmod/insmod the driver. If you want hardware rfkill to be one-way,
> >> then take Johannes's patch. We would save a little power by calling
> >> b43_wireless_exit() if we brought it up to test the switch, and the
> >> switch was still off. That would leave everything off most of the time.
> > 
> > Yeah well. We cannot read the rfkill status while the device is down. That is
> > a hardware limitation. I think we should _live_ with that limitation instead of
> > working around it by always keeping the device initialized.
> > Can't we teach the rfkill subsystem about an "unknown" state? Because that's what we're in.
> 
> An "unknown" state would be OK, but I don't know how to get the state
> of the switch to the rfkill system.
> 
> > Yeah. But wasting huge amounts of power to keep polling a bit that's not even used
> > most of the time is not really what I like.
> 
> 
> > +void b43_rfkill_poll(struct ieee80211_hw *hw)
> >  {
> > -       struct b43_wldev *dev = data;
> > -       struct b43_wl *wl = dev->wl;
> > +       struct b43_wl *wl = hw_to_b43_wl(hw);
> > +       struct b43_wldev *dev = wl->current_dev;
> >         bool enabled;
> >  
> >         mutex_lock(&wl->mutex);
> >         if (unlikely(b43_status(dev) < B43_STAT_INITIALIZED)) {
> > -               mutex_unlock(&wl->mutex);
> > -               return;
> > +               if (b43_wireless_core_init(dev)) {
> > +                       mutex_unlock(&wl->mutex);
> > +                       return;
> > +               }
> >         }
> > 
> > This is the part of the code which I really really really dislike.
> > Hell, just return a freaking error from b43_rfkill_poll(), if the interface
> > is down. If rfkill can't handle that, it should probably be taught to handle it.
> > Especially as there can be other errors as well, like memory allocation failures.
> 
> I understand that this is the part that you do not like. I don't like
> it either, but unless we can read the switch-state bit, the interface
> will never come back up. I do not find rmmod/insmod a suitable
> recovery mechanism.

Huh? We cannot do a simple  ifconfig wlan0 up  anymore to bring the interface up?
Is that overridden by rfkill now?

-- 
Greetings, Michael.

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-05 22:38       ` Larry Finger
  2009-06-06  9:38         ` Michael Buesch
@ 2009-06-06 17:37         ` Johannes Berg
  1 sibling, 0 replies; 16+ messages in thread
From: Johannes Berg @ 2009-06-06 17:37 UTC (permalink / raw)
  To: Larry Finger; +Cc: Michael Buesch, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 1703 bytes --]

On Fri, 2009-06-05 at 17:38 -0500, Larry Finger wrote:

> > No. If the device is down, then it _is_ down. There won't be an rfkill thing
> > trying to bring it up again although the interface is down.
> 
> But in the current driver, turning the radio switch off only shuts
> down the radio. The rest of the interface is still up.

Correct. However, we have a lot of problems with this since we will try
to connect while the radio is off, etc. So I integrated it with cfg80211
to turn off interfaces while rfkilled.

> I don't know what was intended in the new rfkill method. Remember, I
> don't read those E-mails either, but what I observe is that cfg80211
> calls the stop callback when the switch goes from on to off, but the
> start callback is not called when the off to on transition occurs.
> That may be a bug - I don't know. Johannes, is there a simple
> explanation as to why the start routine is not called? If that were to
> happen, the rfkill poll routine would happily exit immediately if the
> interface were down.

Most devices don't require to be fully up to poll rfkill. There have
been bugs in iwlwifi wrt. this (not noticing rfkill presses while
interfaces down) that have been fixed a long time ago, just nobody cared
about b43 enough I guess.

> I think rfkill should just be a means of letting drivers know that the
> user wants the radio off, either by turning off a switch, or by
> setting some software flag from userland, and the driver should honor
> those wishes.

That's what we're really doing though. Except we're also doing a logical
interface down in addition so that users don't think they can still
connect to a network.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-06  9:38         ` Michael Buesch
@ 2009-06-06 17:38           ` Johannes Berg
  0 siblings, 0 replies; 16+ messages in thread
From: Johannes Berg @ 2009-06-06 17:38 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Larry Finger, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 285 bytes --]

On Sat, 2009-06-06 at 11:38 +0200, Michael Buesch wrote:

> Huh? We cannot do a simple  ifconfig wlan0 up  anymore to bring the interface up?
> Is that overridden by rfkill now?

Yes. Face it, it's the only logical thing: you cannot connect to a
network anyway etc.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-06  9:34         ` Michael Buesch
@ 2009-06-06 17:38           ` Johannes Berg
  0 siblings, 0 replies; 16+ messages in thread
From: Johannes Berg @ 2009-06-06 17:38 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Larry Finger, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 438 bytes --]

On Sat, 2009-06-06 at 11:34 +0200, Michael Buesch wrote:

> It is OK to poll the bit while the device is supposed to be on. But it's not
> OK to turn it on to check the bit.

Well, I'm going to do that anyway.

> > Larry, thanks for your patch, I'll modify it to turn off the core again
> > after checking the rfkill bit tomorrow.
> 
> How frequent will that polling happen?

Every 2 seconds, currently, I think.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-05 19:01 ` Michael Buesch
  2009-06-05 21:20   ` Larry Finger
@ 2009-06-06 17:42   ` Johannes Berg
  2009-06-06 17:49     ` Michael Buesch
  1 sibling, 1 reply; 16+ messages in thread
From: Johannes Berg @ 2009-06-06 17:42 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Larry Finger, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 1404 bytes --]

On Fri, 2009-06-05 at 21:01 +0200, Michael Buesch wrote:

> >      The rfkill polling routine brings the interface back to the initialized
> >      state if it is found to be uninitialized. This way the rfkill switch
> >      may be interpreted. In addition, the radio LED is not turned on in the
> >      initialization routine unless the rfkill switch is on.
> 
> This is pretty silly behavior IMO. Just to bring it to the point:
> We initialize a huge wireless MAC, PHY and Radio that consume several watts of power
> just to poll a silly RF-kill bit.

1) you're MUCH overstating the power consumption
2) we don't actually have to turn on the PHY, just the MAC core, if we
   can do that, but that requires some refactoring afaict

> We can't we just accept that the RF-kill status is unknown while the device is down?

No, because then we don't know whether we can allow users to bring up
the device. We could work around that and bring it up, check rfkill, and
bring it back down return -ERFKILL to the user then. But then guess what
will happen: userspace will have to poll rfkill again.

> I really do hate all that rfkill crap and I'm still refusing to sign off on anything that's
> related to rfkill (like I did for the past year or so). If people want this merged,
> somebody else maintain and sign it off, please.

Fine. Just don't stand in my way then.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-05 21:20   ` Larry Finger
  2009-06-05 21:38     ` Michael Buesch
@ 2009-06-06 17:44     ` Johannes Berg
  2009-06-06 18:00     ` Michael Buesch
  2 siblings, 0 replies; 16+ messages in thread
From: Johannes Berg @ 2009-06-06 17:44 UTC (permalink / raw)
  To: Larry Finger; +Cc: Michael Buesch, linux-wireless

[-- Attachment #1: Type: text/plain, Size: 1993 bytes --]

On Fri, 2009-06-05 at 16:20 -0500, Larry Finger wrote:

> The problem is that while the interface is down the switch status
> cannot be interrogated. If you try, you get a fatal SSB error. Thus
> the only way to bring it back up is to flip the switch, then
> rmmod/insmod the driver. If you want hardware rfkill to be one-way,
> then take Johannes's patch. We would save a little power by calling
> b43_wireless_exit() if we brought it up to test the switch, and the
> switch was still off. That would leave everything off most of the time.

Actually we should do that all the time, since after the user pushes the
button he might still not actually want to use the device.

> > I really do hate all that rfkill crap and I'm still refusing to sign off on anything that's
> > related to rfkill (like I did for the past year or so). If people want this merged,
> > somebody else maintain and sign it off, please.
> 
> I'm sick of rfkill as well and really detest the endless discussions
> that have taken place; however, I do want the stuff to work.
> 
> As I see it, we have several options (presented in my order of
> preference):
> 
> 1. We switch to the cfg80211 rfkill and use this patch modified to
> turn the interface back off if the switch is still off.

I'll do that.

> 2. We continue to use the old rfkill mechanism. It works just fine,
> but this method runs the risk of the old method being deprecated and
> eliminated.

It won't be, but you won't get proper userspace integration, NM will not
integrate properly and then we will continue having to rely on users
reading dmesg saying something like "rfkill off, press button before
wireless will connect" and mac80211 will at the same time try to connect
and do things unsuccessfully.

> 3. We get new callbacks that will only power down/up the radio when it
> is blocked. That saves a little power.

Less so than actually turning off most things most of the time, I'd
think.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-06 17:42   ` Johannes Berg
@ 2009-06-06 17:49     ` Michael Buesch
  0 siblings, 0 replies; 16+ messages in thread
From: Michael Buesch @ 2009-06-06 17:49 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Larry Finger, linux-wireless

On Saturday 06 June 2009 19:42:18 Johannes Berg wrote:
> On Fri, 2009-06-05 at 21:01 +0200, Michael Buesch wrote:
> 
> > >      The rfkill polling routine brings the interface back to the initialized
> > >      state if it is found to be uninitialized. This way the rfkill switch
> > >      may be interpreted. In addition, the radio LED is not turned on in the
> > >      initialization routine unless the rfkill switch is on.
> > 
> > This is pretty silly behavior IMO. Just to bring it to the point:
> > We initialize a huge wireless MAC, PHY and Radio that consume several watts of power
> > just to poll a silly RF-kill bit.
> 
> 1) you're MUCH overstating the power consumption

Care to measure it?

> Fine. Just don't stand in my way then.

I am not standing in your way. I'm just not going to maintain it.
In fact, I'd like to pass the whole maintainership to somebody else. But I guess there is nobody.

-- 
Greetings, Michael.

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-05 21:20   ` Larry Finger
  2009-06-05 21:38     ` Michael Buesch
  2009-06-06 17:44     ` Johannes Berg
@ 2009-06-06 18:00     ` Michael Buesch
  2009-06-07 12:55       ` Larry Finger
  2 siblings, 1 reply; 16+ messages in thread
From: Michael Buesch @ 2009-06-06 18:00 UTC (permalink / raw)
  To: Larry Finger; +Cc: Johannes Berg, linux-wireless

Larry, wouldn't it work to do

mutex_lock();
...
if (!initialized) {
	ssb_bus_powerup();
	ssb_device_enable();
	poll_the_bit;
	ssb_device_disable();
	ssb_bus_may_powerdown();
}
...
mutex_unlock();

That would be a _lot_ less heavyweight (at the order of 300 milliseconds less).
And from a power point of view, the PHY and Radio would stay turned off.

-- 
Greetings, Michael.

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

* Re: [RFC V2] b43/legacy: port to cfg80211 rfkill
  2009-06-06 18:00     ` Michael Buesch
@ 2009-06-07 12:55       ` Larry Finger
  0 siblings, 0 replies; 16+ messages in thread
From: Larry Finger @ 2009-06-07 12:55 UTC (permalink / raw)
  To: Michael Buesch; +Cc: Johannes Berg, linux-wireless

Michael Buesch wrote:
> Larry, wouldn't it work to do
> 
> mutex_lock();
> ...
> if (!initialized) {
> 	ssb_bus_powerup();
> 	ssb_device_enable();
> 	poll_the_bit;
> 	ssb_device_disable();
> 	ssb_bus_may_powerdown();
> }
> ...
> mutex_unlock();
> 
> That would be a _lot_ less heavyweight (at the order of 300 milliseconds less).
> And from a power point of view, the PHY and Radio would stay turned off.

Thanks for the suggestion. The method works well and should be a part
of the final patch.

Larry

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

end of thread, other threads:[~2009-06-07 12:55 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-06-05 18:19 [RFC V2] b43/legacy: port to cfg80211 rfkill Larry Finger
2009-06-05 19:01 ` Michael Buesch
2009-06-05 21:20   ` Larry Finger
2009-06-05 21:38     ` Michael Buesch
2009-06-05 22:32       ` Johannes Berg
2009-06-06  9:34         ` Michael Buesch
2009-06-06 17:38           ` Johannes Berg
2009-06-05 22:38       ` Larry Finger
2009-06-06  9:38         ` Michael Buesch
2009-06-06 17:38           ` Johannes Berg
2009-06-06 17:37         ` Johannes Berg
2009-06-06 17:44     ` Johannes Berg
2009-06-06 18:00     ` Michael Buesch
2009-06-07 12:55       ` Larry Finger
2009-06-06 17:42   ` Johannes Berg
2009-06-06 17:49     ` Michael Buesch

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.