[GIT PATCH] rfkill updates

[GIT PATCH] rfkill updates

[Henrique de Moraes Holschuh]

This patchset has the current stack of changes I am comfortable enough to
propose for merging.

There is some other stuff in the burner, but since they are userspace-ABI
related and I am not sure I am happy with it yet, I want to think a bit
more before sending them to the list as an RFC patchset.

I don't really care if this patch set make it to 2.6.28 merge window or
not, they're mostly enhancements... still, I would appreciate comments
(and if they are good enough already, ACKs and maybe even a merge into
wireless-testing).

Shortlog:
Henrique de Moraes Holschuh (5):
  rfkill: use killable locks instead of interruptible
  rfkill: export global states to rfkill-input
  rfkill: add master_switch_mode and EPO lock to rfkill and rfkill-input
  rfkill: honour EPO state when resuming a rfkill controller
  rfkill: rate-limit rfkill-input workqueue usage (v3)

Thanks!

-- 
  "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

Re: [GIT PATCH] rfkill updates

[Ivo van Doorn]

Hi,

> This patchset has the current stack of changes I am comfortable enough to
> propose for merging.
> 
> There is some other stuff in the burner, but since they are userspace-ABI
> related and I am not sure I am happy with it yet, I want to think a bit
> more before sending them to the list as an RFC patchset.
> 
> I don't really care if this patch set make it to 2.6.28 merge window or
> not, they're mostly enhancements... still, I would appreciate comments
> (and if they are good enough already, ACKs and maybe even a merge into
> wireless-testing).

2.6.27 has been released so these will be for 2.6.29,
it seems they were already merged into linux-wireless,
but for completeness here are my acks.

Ivo

[GIT PATCH] rfkill updates

[Henrique de Moraes Holschuh]

While looking at the rfkill userspace API stuff that was bitrotting on
my git tree, I found this small set of minor patches for rfkill.

One of them will clash with a patch just sent to the list that removes
commented-out code.  If both patches get Ack'd, I will take care of
the clash by collecting them all in a single set and reposting the
entire set with all Acks, etc.

Please consider for inclusion in wireless-testing.  The patches are
based on wireless-testing master branch.

shortlog:

Henrique de Moraes Holschuh (4):
      rfkill: minor locking fix
      rfkill: status initialization paranoia
      rfkill: defensive use of test_bit
      rfkill: allow embedded config without an userspace API

[PATCH 1/4] rfkill: minor locking fix

[Henrique de Moraes Holschuh]

The API requires that we lock rfkill->mutex before we call
rfkill_toggle_radio().  We weren't doing it in rfkill_add_switch(),
which is part of the rfkill_register() code flow.

It is higly unlikely that something would access the rfkill struct
while it is not even registered yet, so this is not a bug I would ever
expect to see triggered.

Still, there is no reason to not fix anyway and thus always enforce
the API.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Ivo van Doorn <IvDoorn@gmail.com>
---
 net/rfkill/rfkill.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 3eaa394..f00df8c 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -629,9 +629,11 @@ static int rfkill_add_switch(struct rfkill *rfkill)
 			rfkill_global_states[rfkill->type].default_state;
 	}
 
+	mutex_lock(&rfkill->mutex);
 	rfkill_toggle_radio(rfkill,
 			    rfkill_global_states[rfkill->type].current_state,
 			    0);
+	mutex_unlock(&rfkill->mutex);
 
 	list_add_tail(&rfkill->node, &rfkill_list);
 
-- 
1.6.2.1

[PATCH 2/4] rfkill: status initialization paranoia

[Henrique de Moraes Holschuh]

Track, and detect if rfkill->status is unitialized at
rfkill_register() time.

Note that if rfkill->get_status() is defined, the status will be
initialized by the core at rfkill_register() time.  This is nothing
new, it already happened in the rfkill_toggle_radio() call done by
rfkill_add_switch().

Also, update relevant docs and comments.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Ivo van Doorn <IvDoorn@gmail.com>
---
 Documentation/rfkill.txt |    3 ++-
 net/rfkill/rfkill.c      |   26 ++++++++++++++++++++++----
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 4d3ee31..8c611ec 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -483,7 +483,8 @@ You should:
 	- rfkill_allocate()
 	- modify rfkill fields (flags, name)
 	- modify state to the current hardware state (THIS IS THE ONLY TIME
-	  YOU CAN ACCESS state DIRECTLY)
+	  YOU CAN ACCESS state DIRECTLY) if you don't provide a get_state()
+	  hook
 	- rfkill_register()
 
 The only way to set a device to the RFKILL_STATE_HARD_BLOCKED state is through
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index f00df8c..16db5cc 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -662,10 +662,10 @@ static void rfkill_remove_switch(struct rfkill *rfkill)
  *
  * This function should be called by the network driver when it needs
  * rfkill structure.  Once the structure is allocated the driver should
- * finish its initialization by setting the name, private data, enable_radio
- * and disable_radio methods and then register it with rfkill_register().
+ * finish its initialization by setting the name, private data, hooks,
+ * and other relevant fields... and then register it with rfkill_register().
  *
- * NOTE: If registration fails the structure shoudl be freed by calling
+ * NOTE: If registration fails the structure should be freed by calling
  * rfkill_free() otherwise rfkill_unregister() should be used.
  */
 struct rfkill * __must_check rfkill_allocate(struct device *parent,
@@ -687,6 +687,7 @@ struct rfkill * __must_check rfkill_allocate(struct device *parent,
 	mutex_init(&rfkill->mutex);
 	INIT_LIST_HEAD(&rfkill->node);
 	rfkill->type = type;
+	rfkill->state = RFKILL_STATE_MAX; /* pre-init to illegal value */
 
 	dev = &rfkill->dev;
 	dev->class = &rfkill_class;
@@ -754,7 +755,7 @@ int __must_check rfkill_register(struct rfkill *rfkill)
 
 	if (WARN((!rfkill || !rfkill->toggle_radio ||
 			rfkill->type >= RFKILL_TYPE_MAX ||
-			rfkill->state >= RFKILL_STATE_MAX),
+			rfkill->state > RFKILL_STATE_MAX),
 			KERN_WARNING
 			"rfkill: attempt to register a "
 			"badly initialized rfkill struct\n"))
@@ -764,6 +765,23 @@ int __must_check rfkill_register(struct rfkill *rfkill)
 
 	rfkill_led_trigger_register(rfkill);
 
+	/* rfkill->state initialization */
+	if (rfkill->get_state) {
+		mutex_lock(&rfkill->mutex);
+		error = rfkill->get_state(rfkill->data, &rfkill->state);
+		mutex_unlock(&rfkill->mutex);
+		if (error < 0) {
+			printk(KERN_WARNING
+				"rfkill: get_state() hook failed "
+				"during registration...\n");
+			return error;
+		}
+	}
+	if (WARN((rfkill->state >= RFKILL_STATE_MAX),
+			KERN_WARNING
+			"rfkill: rfkill->state not properly initialized\n"))
+		return -EINVAL;
+
 	error = rfkill_add_switch(rfkill);
 	if (error) {
 		rfkill_led_trigger_unregister(rfkill);
-- 
1.6.2.1

[PATCH 3/4] rfkill: defensive use of test_bit

[Henrique de Moraes Holschuh]

test_bit can return negative values for true, not just 0 or 1.  This
has already caused a bug in rfkill once, so it is best to take a more
defensive position in its use.

While at it, also make some booleans more explicit.

This change is for future-proofing, the code works as-is.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Ivo van Doorn <IvDoorn@gmail.com>
---
 net/rfkill/rfkill-input.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
index 84efde9..d8a9b70 100644
--- a/net/rfkill/rfkill-input.c
+++ b/net/rfkill/rfkill-input.c
@@ -173,11 +173,11 @@ static void rfkill_task_handler(struct work_struct *work)
 					bool c;
 #ifdef RFKILL_NEED_SWSET
 					bool sp, s;
-					sp = test_and_clear_bit(i,
+					sp = !!test_and_clear_bit(i,
 							task->sw_setpending);
-					s = test_bit(i, task->sw_newstate);
+					s = !!test_bit(i, task->sw_newstate);
 #endif
-					c = test_and_clear_bit(i,
+					c = !!test_and_clear_bit(i,
 							task->sw_togglestate);
 					spin_unlock_irq(&task->lock);
 
@@ -280,7 +280,7 @@ static void rfkill_schedule_toggle(enum rfkill_type type)
 	spin_unlock_irqrestore(&rfkill_task.lock, flags);
 }
 
-static void rfkill_schedule_evsw_rfkillall(int state)
+static void rfkill_schedule_evsw_rfkillall(bool state)
 {
 	if (state) {
 		switch (rfkill_master_switch_mode) {
@@ -332,7 +332,7 @@ static void rfkill_event(struct input_handle *handle, unsigned int type,
 	} else if (type == EV_SW) {
 		switch (code) {
 		case SW_RFKILL_ALL:
-			rfkill_schedule_evsw_rfkillall(data);
+			rfkill_schedule_evsw_rfkillall(!!data);
 			return;
 		default:
 			return;
@@ -381,7 +381,7 @@ static void rfkill_start(struct input_handle *handle)
 
 	if (test_bit(EV_SW, handle->dev->evbit)) {
 		if (test_bit(SW_RFKILL_ALL, handle->dev->swbit))
-			rfkill_schedule_evsw_rfkillall(test_bit(SW_RFKILL_ALL,
+			rfkill_schedule_evsw_rfkillall(!!test_bit(SW_RFKILL_ALL,
 							handle->dev->sw));
 		/* add resync for further EV_SW events here */
 	}
-- 
1.6.2.1

[PATCH 4/4] rfkill: allow embedded config without an userspace API

[Henrique de Moraes Holschuh]

Allow rfkill to be configured without any of the sysfs APIs, on embedded
systems.

Systems using rfkill-input exclusively and who could care less for userland
feedback do not need the userspace APIs, and can get rid of a reasonable
ammount of code and data that way.

Some code is moved around, but there are no code changes other than the
inclusion of some #ifndef/#else/#endif clauses.

Signed-off-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Ivo van Doorn <IvDoorn@gmail.com>
Cc: Dmitry Torokhov <dtor@mail.ru>
---
 net/rfkill/Kconfig  |   10 +++++
 net/rfkill/rfkill.c |   94 +++++++++++++++++++++++++++++++--------------------
 2 files changed, 67 insertions(+), 37 deletions(-)

diff --git a/net/rfkill/Kconfig b/net/rfkill/Kconfig
index 7f807b3..ae0181f 100644
--- a/net/rfkill/Kconfig
+++ b/net/rfkill/Kconfig
@@ -10,6 +10,16 @@ menuconfig RFKILL
 	  To compile this driver as a module, choose M here: the
 	  module will be called rfkill.
 
+config RFKILL_NO_CORE_UAPI
+	bool "Remove the rfkill core userspace APIs"
+	depends on RFKILL
+	depends on EMBEDDED
+	default n
+	help
+	  Say Y here if you want to remove the entire rfkill userspace
+	  API.  Use this on highly constrained systems when no userspace
+	  feedback or userspace-based rfkill control is needed.
+
 config RFKILL_INPUT
 	tristate "Input layer to RF switch connector"
 	depends on RFKILL && INPUT
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 16db5cc..80f0fa0 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -53,6 +53,7 @@ static struct rfkill_gsw_state rfkill_global_states[RFKILL_TYPE_MAX];
 static unsigned long rfkill_states_lockdflt[BITS_TO_LONGS(RFKILL_TYPE_MAX)];
 static bool rfkill_epo_lock_active;
 
+static void rfkill_uevent(struct rfkill *rfkill);
 
 #ifdef CONFIG_RFKILL_LEDS
 static void rfkill_led_trigger(struct rfkill *rfkill,
@@ -77,27 +78,6 @@ static void rfkill_led_trigger_activate(struct led_classdev *led)
 }
 #endif /* CONFIG_RFKILL_LEDS */
 
-static void rfkill_uevent(struct rfkill *rfkill)
-{
-	kobject_uevent(&rfkill->dev.kobj, KOBJ_CHANGE);
-}
-
-static void update_rfkill_state(struct rfkill *rfkill)
-{
-	enum rfkill_state newstate, oldstate;
-
-	if (rfkill->get_state) {
-		mutex_lock(&rfkill->mutex);
-		if (!rfkill->get_state(rfkill->data, &newstate)) {
-			oldstate = rfkill->state;
-			rfkill->state = newstate;
-			if (oldstate != newstate)
-				rfkill_uevent(rfkill);
-		}
-		mutex_unlock(&rfkill->mutex);
-	}
-}
-
 /**
  * rfkill_toggle_radio - wrapper for toggle_radio hook
  * @rfkill: the rfkill struct to use
@@ -363,6 +343,29 @@ int rfkill_force_state(struct rfkill *rfkill, enum rfkill_state state)
 }
 EXPORT_SYMBOL(rfkill_force_state);
 
+
+/*
+ * rfkill core sysfs interface
+ */
+
+#ifndef CONFIG_RFKILL_NO_CORE_UAPI
+
+static void update_rfkill_state(struct rfkill *rfkill)
+{
+	enum rfkill_state newstate, oldstate;
+
+	if (rfkill->get_state) {
+		mutex_lock(&rfkill->mutex);
+		if (!rfkill->get_state(rfkill->data, &newstate)) {
+			oldstate = rfkill->state;
+			rfkill->state = newstate;
+			if (oldstate != newstate)
+				rfkill_uevent(rfkill);
+		}
+		mutex_unlock(&rfkill->mutex);
+	}
+}
+
 static ssize_t rfkill_name_show(struct device *dev,
 				struct device_attribute *attr,
 				char *buf)
@@ -504,6 +507,39 @@ static struct device_attribute rfkill_dev_attrs[] = {
 	__ATTR_NULL
 };
 
+
+/*
+ * rfkill core UEVENT API
+ */
+
+static void rfkill_uevent(struct rfkill *rfkill)
+{
+	kobject_uevent(&rfkill->dev.kobj, KOBJ_CHANGE);
+}
+
+static int rfkill_dev_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	struct rfkill *rfkill = to_rfkill(dev);
+	int error;
+
+	error = add_uevent_var(env, "RFKILL_NAME=%s", rfkill->name);
+	if (error)
+		return error;
+	error = add_uevent_var(env, "RFKILL_TYPE=%s",
+				rfkill_get_type_str(rfkill->type));
+	if (error)
+		return error;
+	error = add_uevent_var(env, "RFKILL_STATE=%d", rfkill->state);
+	return error;
+}
+
+#else
+#define rfkill_dev_attrs NULL
+#define rfkill_dev_uevent NULL
+static void rfkill_uevent(struct rfkill *rfkill) {};
+#endif	/* ifndef CONFIG_RFKILL_NO_CORE_UAPI */
+
+
 static void rfkill_release(struct device *dev)
 {
 	struct rfkill *rfkill = to_rfkill(dev);
@@ -568,22 +604,6 @@ static int rfkill_resume(struct device *dev)
 #define rfkill_resume NULL
 #endif
 
-static int rfkill_dev_uevent(struct device *dev, struct kobj_uevent_env *env)
-{
-	struct rfkill *rfkill = to_rfkill(dev);
-	int error;
-
-	error = add_uevent_var(env, "RFKILL_NAME=%s", rfkill->name);
-	if (error)
-		return error;
-	error = add_uevent_var(env, "RFKILL_TYPE=%s",
-				rfkill_get_type_str(rfkill->type));
-	if (error)
-		return error;
-	error = add_uevent_var(env, "RFKILL_STATE=%d", rfkill->state);
-	return error;
-}
-
 static struct class rfkill_class = {
 	.name		= "rfkill",
 	.dev_release	= rfkill_release,
-- 
1.6.2.1

Re: [GIT PATCH] rfkill updates

[Johannes Berg]

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

Henrique,

Can we drop all this? I'm rewriting the entire rfkill core to better
support hard/soft kills, and all this code pretty much goes away anyway.

It's very frustrating to see a subsystem stagnate forever, and then once
somebody else starts working on it see a bunch of patches that break all
that work being posted.

Clearly those "bugs" are minor enough that nobody has even found them
yet, so it's not worth fixing them.

johannes


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

Re: [GIT PATCH] rfkill updates

[Henrique de Moraes Holschuh]

On Fri, 27 Mar 2009, Johannes Berg wrote:
> Can we drop all this? I'm rewriting the entire rfkill core to better
> support hard/soft kills, and all this code pretty much goes away anyway.

Sure.

-- 
  "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