Discussion:
[PATCH 5/8] rfkill: Remove obsolete "state" sysfs interface
(too old to reply)
João Paulo Rechi Vita
2016-01-19 15:50:02 UTC
Permalink
This was schedule to be removed in 2014 by:

commit 69c86373c6ea1149aa559e6088362d58d8ec8835
Author: ***@mickler.org <***@mickler.org>
Date: Wed Feb 24 12:05:16 2010 +0100

Document the rfkill sysfs ABI

This moves sysfs ABI info from Documentation/rfkill.txt to the
ABI subfolder and reformats it.

This also schedules the deprecated sysfs parts to be removed in
2012 (claim file) and 2014 (state file).

Signed-off-by: Florian Mickler <***@mickler.org>
Signed-off-by: John W. Linville <***@tuxdriver.com>

Signed-off-by: João Paulo Rechi Vita <***@endlessm.com>
---
Documentation/ABI/obsolete/sysfs-class-rfkill | 20 ---------------
Documentation/ABI/removed/sysfs-class-rfkill | 17 +++++++++++++
net/rfkill/core.c | 35 ---------------------------
3 files changed, 17 insertions(+), 55 deletions(-)
delete mode 100644 Documentation/ABI/obsolete/sysfs-class-rfkill

diff --git a/Documentation/ABI/obsolete/sysfs-class-rfkill b/Documentation/ABI/obsolete/sysfs-class-rfkill
deleted file mode 100644
index e736d14..0000000
--- a/Documentation/ABI/obsolete/sysfs-class-rfkill
+++ /dev/null
@@ -1,20 +0,0 @@
-rfkill - radio frequency (RF) connector kill switch support
-
-For details to this subsystem look at Documentation/rfkill.txt.
-
-What: /sys/class/rfkill/rfkill[0-9]+/state
-Date: 09-Jul-2007
-KernelVersion v2.6.22
-Contact: linux-***@vger.kernel.org
-Description: Current state of the transmitter.
- This file is deprecated and scheduled to be removed in 2014,
- because its not possible to express the 'soft and hard block'
- state of the rfkill driver.
-Values: A numeric value.
- 0: RFKILL_STATE_SOFT_BLOCKED
- transmitter is turned off by software
- 1: RFKILL_STATE_UNBLOCKED
- transmitter is (potentially) active
- 2: RFKILL_STATE_HARD_BLOCKED
- transmitter is forced off by something outside of
- the driver's control.
diff --git a/Documentation/ABI/removed/sysfs-class-rfkill b/Documentation/ABI/removed/sysfs-class-rfkill
index 3ce6231..1b93139 100644
--- a/Documentation/ABI/removed/sysfs-class-rfkill
+++ b/Documentation/ABI/removed/sysfs-class-rfkill
@@ -11,3 +11,20 @@ Description: This file was deprecated because there no longer was a way to
This file was scheduled to be removed in 2012, and was removed
in 2016.
Values: 0: Kernel handles events
+
+What: /sys/class/rfkill/rfkill[0-9]+/state
+Date: 09-Jul-2007
+KernelVersion v2.6.22
+Contact: linux-***@vger.kernel.org
+Description: Current state of the transmitter.
+ This file was deprecated and scheduled to be removed in 2014,
+ because it was not possible to express the 'soft and hard block'
+ state of the rfkill driver. It was removed in 2016.
+Values: A numeric value.
+ 0: RFKILL_STATE_SOFT_BLOCKED
+ transmitter is turned off by software
+ 1: RFKILL_STATE_UNBLOCKED
+ transmitter is (potentially) active
+ 2: RFKILL_STATE_HARD_BLOCKED
+ transmitter is forced off by something outside of
+ the driver's control.
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index a05d1f1..26da4f0 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -683,46 +683,11 @@ static u8 user_state_from_blocked(unsigned long state)
return RFKILL_USER_STATE_UNBLOCKED;
}

-static ssize_t state_show(struct device *dev, struct device_attribute *attr,
- char *buf)
-{
- struct rfkill *rfkill = to_rfkill(dev);
-
- return sprintf(buf, "%d\n", user_state_from_blocked(rfkill->state));
-}
-
-static ssize_t state_store(struct device *dev, struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct rfkill *rfkill = to_rfkill(dev);
- unsigned long state;
- int err;
-
- if (!capable(CAP_NET_ADMIN))
- return -EPERM;
-
- err = kstrtoul(buf, 0, &state);
- if (err)
- return err;
-
- if (state != RFKILL_USER_STATE_SOFT_BLOCKED &&
- state != RFKILL_USER_STATE_UNBLOCKED)
- return -EINVAL;
-
- mutex_lock(&rfkill_global_mutex);
- rfkill_set_block(rfkill, state == RFKILL_USER_STATE_SOFT_BLOCKED);
- mutex_unlock(&rfkill_global_mutex);
-
- return count;
-}
-static DEVICE_ATTR_RW(state);
-
static struct attribute *rfkill_dev_attrs[] = {
&dev_attr_name.attr,
&dev_attr_type.attr,
&dev_attr_index.attr,
&dev_attr_persistent.attr,
- &dev_attr_state.attr,
&dev_attr_soft.attr,
&dev_attr_hard.attr,
NULL,
--
2.5.0
João Paulo Rechi Vita
2016-01-19 15:50:02 UTC
Permalink
This was schedule to be removed in 2012 by:

commit 69c86373c6ea1149aa559e6088362d58d8ec8835
Author: ***@mickler.org <***@mickler.org>
Date: Wed Feb 24 12:05:16 2010 +0100

Document the rfkill sysfs ABI

This moves sysfs ABI info from Documentation/rfkill.txt to the
ABI subfolder and reformats it.

This also schedules the deprecated sysfs parts to be removed in
2012 (claim file) and 2014 (state file).

Signed-off-by: Florian Mickler <***@mickler.org>
Signed-off-by: John W. Linville <***@tuxdriver.com>

Signed-off-by: João Paulo Rechi Vita <***@endlessm.com>
---
Documentation/ABI/obsolete/sysfs-class-rfkill | 9 ---------
Documentation/ABI/removed/sysfs-class-rfkill | 13 +++++++++++++
net/rfkill/core.c | 11 +----------
3 files changed, 14 insertions(+), 19 deletions(-)
create mode 100644 Documentation/ABI/removed/sysfs-class-rfkill

diff --git a/Documentation/ABI/obsolete/sysfs-class-rfkill b/Documentation/ABI/obsolete/sysfs-class-rfkill
index ff60ad9..e736d14 100644
--- a/Documentation/ABI/obsolete/sysfs-class-rfkill
+++ b/Documentation/ABI/obsolete/sysfs-class-rfkill
@@ -18,12 +18,3 @@ Values: A numeric value.
2: RFKILL_STATE_HARD_BLOCKED
transmitter is forced off by something outside of
the driver's control.
-
-What: /sys/class/rfkill/rfkill[0-9]+/claim
-Date: 09-Jul-2007
-KernelVersion v2.6.22
-Contact: linux-***@vger.kernel.org
-Description: This file is deprecated because there no longer is a way to
- claim just control over a single rfkill instance.
- This file is scheduled to be removed in 2012.
-Values: 0: Kernel handles events
diff --git a/Documentation/ABI/removed/sysfs-class-rfkill b/Documentation/ABI/removed/sysfs-class-rfkill
new file mode 100644
index 0000000..3ce6231
--- /dev/null
+++ b/Documentation/ABI/removed/sysfs-class-rfkill
@@ -0,0 +1,13 @@
+rfkill - radio frequency (RF) connector kill switch support
+
+For details to this subsystem look at Documentation/rfkill.txt.
+
+What: /sys/class/rfkill/rfkill[0-9]+/claim
+Date: 09-Jul-2007
+KernelVersion v2.6.22
+Contact: linux-***@vger.kernel.org
+Description: This file was deprecated because there no longer was a way to
+ claim just control over a single rfkill instance.
+ This file was scheduled to be removed in 2012, and was removed
+ in 2016.
+Values: 0: Kernel handles events
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 5406c76..a05d1f1 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -309,8 +309,7 @@ static atomic_t rfkill_input_disabled = ATOMIC_INIT(0);
* @blocked: the new state
*
* This function sets the state of all switches of given type,
- * unless a specific switch is claimed by userspace (in which case,
- * that switch is left alone) or suspended.
+ * unless a specific switch is suspended.
*
* Caller must have acquired rfkill_global_mutex.
*/
@@ -718,20 +717,12 @@ static ssize_t state_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RW(state);

-static ssize_t claim_show(struct device *dev, struct device_attribute *attr,
- char *buf)
-{
- return sprintf(buf, "%d\n", 0);
-}
-static DEVICE_ATTR_RO(claim);
-
static struct attribute *rfkill_dev_attrs[] = {
&dev_attr_name.attr,
&dev_attr_type.attr,
&dev_attr_index.attr,
&dev_attr_persistent.attr,
&dev_attr_state.attr,
- &dev_attr_claim.attr,
&dev_attr_soft.attr,
&dev_attr_hard.attr,
NULL,
--
2.5.0
João Paulo Rechi Vita
2016-01-19 15:50:02 UTC
Permalink
Factor all assignments to rfkill_global_states[].cur to
rfkill_update_global_state(), and also make accesses to
rfkill_global_states[].cur go through rfkill_get_global_sw_state().

Signed-off-by: João Paulo Rechi Vita <***@endlessm.com>
---
net/rfkill/core.c | 40 ++++++++++++++++++----------------------
1 file changed, 18 insertions(+), 22 deletions(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 0b9d5bf..ae3d15a 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -300,6 +300,19 @@ static void rfkill_set_block(struct rfkill *rfkill, bool blocked)
rfkill_event(rfkill);
}

+static void rfkill_update_global_state(enum rfkill_type type, bool blocked)
+{
+ int i;
+
+ if (type != RFKILL_TYPE_ALL) {
+ rfkill_global_states[type].cur = blocked;
+ return;
+ }
+
+ for (i = 0; i < NUM_RFKILL_TYPES; i++)
+ rfkill_global_states[i].cur = blocked;
+}
+
#ifdef CONFIG_RFKILL_INPUT
static atomic_t rfkill_input_disabled = ATOMIC_INIT(0);

@@ -317,15 +330,7 @@ static void __rfkill_switch_all(const enum rfkill_type type, bool blocked)
{
struct rfkill *rfkill;

- if (type == RFKILL_TYPE_ALL) {
- int i;
-
- for (i = 0; i < NUM_RFKILL_TYPES; i++)
- rfkill_global_states[i].cur = blocked;
- } else {
- rfkill_global_states[type].cur = blocked;
- }
-
+ rfkill_update_global_state(type, blocked);
list_for_each_entry(rfkill, &rfkill_list, node) {
if (rfkill->type != type && type != RFKILL_TYPE_ALL)
continue;
@@ -877,7 +882,7 @@ static void rfkill_sync_work(struct work_struct *work)
rfkill = container_of(work, struct rfkill, sync_work);

mutex_lock(&rfkill_global_mutex);
- cur = rfkill_global_states[rfkill->type].cur;
+ cur = rfkill_get_global_sw_state(rfkill->type);
rfkill_set_block(rfkill, cur);
mutex_unlock(&rfkill_global_mutex);
}
@@ -1116,15 +1121,8 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,

mutex_lock(&rfkill_global_mutex);

- if (ev.op == RFKILL_OP_CHANGE_ALL) {
- if (ev.type == RFKILL_TYPE_ALL) {
- enum rfkill_type i;
- for (i = 0; i < NUM_RFKILL_TYPES; i++)
- rfkill_global_states[i].cur = ev.soft;
- } else {
- rfkill_global_states[ev.type].cur = ev.soft;
- }
- }
+ if (ev.op == RFKILL_OP_CHANGE_ALL)
+ rfkill_update_global_state(ev.type, ev.soft);

list_for_each_entry(rfkill, &rfkill_list, node) {
if (rfkill->idx != ev.idx && ev.op != RFKILL_OP_CHANGE_ALL)
@@ -1213,10 +1211,8 @@ static struct miscdevice rfkill_miscdev = {
static int __init rfkill_init(void)
{
int error;
- int i;

- for (i = 0; i < NUM_RFKILL_TYPES; i++)
- rfkill_global_states[i].cur = !rfkill_default_state;
+ rfkill_update_global_state(RFKILL_TYPE_ALL, !rfkill_default_state);

error = class_register(&rfkill_class);
if (error)
--
2.5.0
kbuild test robot
2016-01-19 17:50:01 UTC
Permalink
Hi João,

[auto build test ERROR on mac80211-next/master]
[also build test ERROR on v4.4 next-20160119]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url: https://github.com/0day-ci/linux/commits/Jo-o-Paulo-Rechi-Vita/General-RFKill-improvements/20160119-234934
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: i386-randconfig-s0-201603 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
core.c:(.text+0x13d849): undefined reference to `rfkill_get_global_sw_state'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
João Paulo Rechi Vita
2016-01-19 20:40:02 UTC
Permalink
Post by kbuild test robot
Hi João,
[auto build test ERROR on mac80211-next/master]
[also build test ERROR on v4.4 next-20160119]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
url: https://github.com/0day-ci/linux/commits/Jo-o-Paulo-Rechi-Vita/General-RFKill-improvements/20160119-234934
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: i386-randconfig-s0-201603 (attached as .config)
# save the attached .config to linux build tree
make ARCH=i386
core.c:(.text+0x13d849): undefined reference to `rfkill_get_global_sw_state'
I've overlooked the fact that rfkill_get_global_sw_state() is inside a
#ifdef block -- I'll fix this in an updated version.

--
João Paulo Rechi Vita
http://about.me/jprvita
kbuild test robot
2016-01-19 18:20:03 UTC
Permalink
Hi João,

[auto build test ERROR on mac80211-next/master]
[also build test ERROR on v4.4 next-20160119]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url: https://github.com/0day-ci/linux/commits/Jo-o-Paulo-Rechi-Vita/General-RFKill-improvements/20160119-234934
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: x86_64-randconfig-a0-01200034 (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
ERROR: "rfkill_get_global_sw_state" [net/rfkill/rfkill.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
João Paulo Rechi Vita
2016-01-19 15:50:02 UTC
Permalink
__rfkill_set_hw_state() is only one used in rfkill_set_hw_state(), and
none of them are long or complicated, so merging the two makes the code
easier to read.

Signed-off-by: João Paulo Rechi Vita <***@endlessm.com>
---
net/rfkill/core.c | 41 +++++++++++++++--------------------------
1 file changed, 15 insertions(+), 26 deletions(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 838e869..5406c76 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -233,29 +233,6 @@ static void rfkill_event(struct rfkill *rfkill)
rfkill_send_events(rfkill, RFKILL_OP_CHANGE);
}

-static bool __rfkill_set_hw_state(struct rfkill *rfkill,
- bool blocked, bool *change)
-{
- unsigned long flags;
- bool prev, any;
-
- BUG_ON(!rfkill);
-
- spin_lock_irqsave(&rfkill->lock, flags);
- prev = !!(rfkill->state & RFKILL_BLOCK_HW);
- if (blocked)
- rfkill->state |= RFKILL_BLOCK_HW;
- else
- rfkill->state &= ~RFKILL_BLOCK_HW;
- *change = prev != blocked;
- any = !!(rfkill->state & RFKILL_BLOCK_ANY);
- spin_unlock_irqrestore(&rfkill->lock, flags);
-
- rfkill_led_trigger_event(rfkill);
-
- return any;
-}
-
/**
* rfkill_set_block - wrapper for set_block method
*
@@ -479,14 +456,26 @@ bool rfkill_get_global_sw_state(const enum rfkill_type type)

bool rfkill_set_hw_state(struct rfkill *rfkill, bool blocked)
{
- bool ret, change;
+ unsigned long flags;
+ bool ret, prev;
+
+ BUG_ON(!rfkill);
+
+ spin_lock_irqsave(&rfkill->lock, flags);
+ prev = !!(rfkill->state & RFKILL_BLOCK_HW);
+ if (blocked)
+ rfkill->state |= RFKILL_BLOCK_HW;
+ else
+ rfkill->state &= ~RFKILL_BLOCK_HW;
+ ret = !!(rfkill->state & RFKILL_BLOCK_ANY);
+ spin_unlock_irqrestore(&rfkill->lock, flags);

- ret = __rfkill_set_hw_state(rfkill, blocked, &change);
+ rfkill_led_trigger_event(rfkill);

if (!rfkill->registered)
return ret;

- if (change)
+ if (prev != blocked)
schedule_work(&rfkill->uevent_work);

return ret;
--
2.5.0
João Paulo Rechi Vita
2016-01-19 15:50:02 UTC
Permalink
With the removal of the state attribute there is only one user of
user_state_from_blocked() left. Moving this function close to its only
user makes the code more organized and easier to read.

Signed-off-by: João Paulo Rechi Vita <***@endlessm.com>
---
net/rfkill/core.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 26da4f0..0b9d5bf 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -673,16 +673,6 @@ static ssize_t soft_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_RW(soft);

-static u8 user_state_from_blocked(unsigned long state)
-{
- if (state & RFKILL_BLOCK_HW)
- return RFKILL_USER_STATE_HARD_BLOCKED;
- if (state & RFKILL_BLOCK_SW)
- return RFKILL_USER_STATE_SOFT_BLOCKED;
-
- return RFKILL_USER_STATE_UNBLOCKED;
-}
-
static struct attribute *rfkill_dev_attrs[] = {
&dev_attr_name.attr,
&dev_attr_type.attr,
@@ -701,6 +691,16 @@ static void rfkill_release(struct device *dev)
kfree(rfkill);
}

+static u8 user_state_from_blocked(unsigned long state)
+{
+ if (state & RFKILL_BLOCK_HW)
+ return RFKILL_USER_STATE_HARD_BLOCKED;
+ if (state & RFKILL_BLOCK_SW)
+ return RFKILL_USER_STATE_SOFT_BLOCKED;
+
+ return RFKILL_USER_STATE_UNBLOCKED;
+}
+
static int rfkill_dev_uevent(struct device *dev, struct kobj_uevent_env *env)
{
struct rfkill *rfkill = to_rfkill(dev);
--
2.5.0
João Paulo Rechi Vita
2016-01-19 15:50:03 UTC
Permalink
Add a note to userspace on the effect of RFKILL_OP_CHANGE_ALL also
updating the default state for hotplugged devices.

Signed-off-by: João Paulo Rechi Vita <***@endlessm.com>
---
include/uapi/linux/rfkill.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 058757f..c15fcfc 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -59,6 +59,7 @@ enum rfkill_type {
* @RFKILL_OP_DEL: a device was removed
* @RFKILL_OP_CHANGE: a device's state changed -- userspace changes one device
* @RFKILL_OP_CHANGE_ALL: userspace changes all devices (of a type, or all)
+ * into a state which also updates the default state for hotplugged devices.
*/
enum rfkill_operation {
RFKILL_OP_ADD = 0,
--
2.5.0
João Paulo Rechi Vita
2016-01-19 15:50:03 UTC
Permalink
RFKILL_BLOCK_SW value have just been saved to prev, no need to check it
again in the if expression. This makes code a little bit easier to read.

Signed-off-by: João Paulo Rechi Vita <***@endlessm.com>
---
net/rfkill/core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 4d6d726..838e869 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -285,7 +285,7 @@ static void rfkill_set_block(struct rfkill *rfkill, bool blocked)
spin_lock_irqsave(&rfkill->lock, flags);
prev = rfkill->state & RFKILL_BLOCK_SW;

- if (rfkill->state & RFKILL_BLOCK_SW)
+ if (prev)
rfkill->state |= RFKILL_BLOCK_SW_PREV;
else
rfkill->state &= ~RFKILL_BLOCK_SW_PREV;
--
2.5.0
João Paulo Rechi Vita
2016-01-19 15:50:03 UTC
Permalink
Fixes some small typos, punctuation, and copy & paste issues. Also
removes an extra blank line.

Signed-off-by: João Paulo Rechi Vita <***@endlessm.com>
---
include/linux/rfkill.h | 7 ++++---
net/rfkill/core.c | 5 ++---
2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
index d901078..3dcbaf6 100644
--- a/include/linux/rfkill.h
+++ b/include/linux/rfkill.h
@@ -104,16 +104,17 @@ int __must_check rfkill_register(struct rfkill *rfkill);
*
* Pause polling -- say transmitter is off for other reasons.
* NOTE: not necessary for suspend/resume -- in that case the
- * core stops polling anyway
+ * core stops polling anyway.
*/
void rfkill_pause_polling(struct rfkill *rfkill);

/**
* rfkill_resume_polling(struct rfkill *rfkill)
*
- * Pause polling -- say transmitter is off for other reasons.
+ * Resume polling previously paused with rfkill_pause_polling.
* NOTE: not necessary for suspend/resume -- in that case the
- * core stops polling anyway
+ * core restarts polling anyway, even if was explicitly paused
+ * before suspending.
*/
void rfkill_resume_polling(struct rfkill *rfkill);

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index f53bf3b6..4d6d726 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -303,8 +303,8 @@ static void rfkill_set_block(struct rfkill *rfkill, bool blocked)
spin_lock_irqsave(&rfkill->lock, flags);
if (err) {
/*
- * Failed -- reset status to _prev, this may be different
- * from what set set _PREV to earlier in this function
+ * Failed -- reset status to _PREV. This may be different
+ * from what we have set _PREV to earlier in this function
* if rfkill_set_sw_state was invoked.
*/
if (rfkill->state & RFKILL_BLOCK_SW_PREV)
@@ -477,7 +477,6 @@ bool rfkill_get_global_sw_state(const enum rfkill_type type)
}
#endif

-
bool rfkill_set_hw_state(struct rfkill *rfkill, bool blocked)
{
bool ret, change;
--
2.5.0
Johannes Berg
2016-01-19 20:20:02 UTC
Permalink
 /**
  * rfkill_resume_polling(struct rfkill *rfkill)
  *
- * Pause polling -- say transmitter is off for other reasons.
+ * Resume polling previously paused with rfkill_pause_polling.
  * NOTE: not necessary for suspend/resume -- in that case the
- * core stops polling anyway
+ * core restarts polling anyway, even if was explicitly paused
+ * before suspending.
  */
If this is true, that's a bug, no? Drivers would call pause/resume when
their status changes, and shouldn't be required to check status at
resume time?

johannes
João Paulo Rechi Vita
2016-01-19 20:30:02 UTC
Permalink
Post by Johannes Berg
Post by João Paulo Rechi Vita
/**
* rfkill_resume_polling(struct rfkill *rfkill)
*
- * Pause polling -- say transmitter is off for other reasons.
+ * Resume polling previously paused with rfkill_pause_polling.
* NOTE: not necessary for suspend/resume -- in that case the
- * core stops polling anyway
+ * core restarts polling anyway, even if was explicitly paused
+ * before suspending.
*/
If this is true, that's a bug, no? Drivers would call pause/resume when
their status changes, and shouldn't be required to check status at
resume time?
I did not dive too much into the logic here, but
rfkill_resume_polling() is called unconditionally on rfkill_resume(),
so it seems that if a driver call rfkill_pause_polling() before
suspend, on resume polling will be "un-paused". That indeed looks
strange.

--
João Paulo Rechi Vita
http://about.me/jprvita
Johannes Berg
2016-01-19 20:20:02 UTC
Permalink
Post by João Paulo Rechi Vita
-For details to this subsystem look at Documentation/rfkill.txt.
-
-What: /sys/class/rfkill/rfkill[0-9]+/state
-Date: 09-Jul-2007
-KernelVersion v2.6.22
-Description:  Current state of the transmitter.
- This file is deprecated and scheduled to be removed
in 2014,
- because its not possible to express the 'soft and
hard block'
- state of the rfkill driver.
-Values:  A numeric value.
- 0: RFKILL_STATE_SOFT_BLOCKED
- transmitter is turned off by software
- 1: RFKILL_STATE_UNBLOCKED
- transmitter is (potentially) active
- 2: RFKILL_STATE_HARD_BLOCKED
- transmitter is forced off by something
outside of
- the driver's control.
I suspect that nevertheless, we can't just remove it - Googling around
shows quite a bit of (e.g. Android) code using it.

johannes
João Paulo Rechi Vita
2016-01-19 20:40:03 UTC
Permalink
Post by Johannes Berg
Post by João Paulo Rechi Vita
-For details to this subsystem look at Documentation/rfkill.txt.
-
-What: /sys/class/rfkill/rfkill[0-9]+/state
-Date: 09-Jul-2007
-KernelVersion v2.6.22
-Description: Current state of the transmitter.
- This file is deprecated and scheduled to be removed
in 2014,
- because its not possible to express the 'soft and
hard block'
- state of the rfkill driver.
-Values: A numeric value.
- 0: RFKILL_STATE_SOFT_BLOCKED
- transmitter is turned off by software
- 1: RFKILL_STATE_UNBLOCKED
- transmitter is (potentially) active
- 2: RFKILL_STATE_HARD_BLOCKED
- transmitter is forced off by something
outside of
- the driver's control.
I suspect that nevertheless, we can't just remove it - Googling around
shows quite a bit of (e.g. Android) code using it.
Should I update the expected removal date to 2018, or some other time
in the future? Or if this is not going to be removed at all, shouldn't
it be moved back to stable?

--
João Paulo Rechi Vita
http://about.me/jprvita
Johannes Berg
2016-01-19 20:50:02 UTC
Permalink
Post by João Paulo Rechi Vita
 
Post by Johannes Berg
I suspect that nevertheless, we can't just remove it - Googling
around shows quite a bit of (e.g. Android) code using it.
Should I update the expected removal date to 2018, or some other time
in the future? Or if this is not going to be removed at all,
shouldn't it be moved back to stable?
Yeah, maybe we should just move it back. The problem with it is that it
doesn't fully reflect the state (both sw+hw blocked can't be reflected)
but if the current users are happy with it then that's not really a
problem.

johannes
Johannes Berg
2016-01-26 13:10:02 UTC
Permalink
Hi,
This series contains a few general improvements to the RFKill code,
found while
I was writing the rfkill-all / airplane mode LED trigger. All were
points where
I had to read twice or do some other kind of extra effort to fully
understand
it, so I think merging these changes will benefit someone trying to
understand
the RFKill subsystem in the future.
I applied some of this. I think we need to keep the "state" sysfs
interface, perhaps you want to document it more clearly. I also didn't
apply the first patch since I'd fixed the suspend/pause issue, please
see if any of your patches remain relevant and resend that.

Thanks,
johannes

Continue reading on narkive:
Loading...