[PATCH 0/9] RFKill airplane-mode indicator

[PATCH 0/9] RFKill airplane-mode indicator

[João Paulo Rechi Vita]

This series implements an airplane-mode indicator LED trigger, which can be
used by platform drivers. The default policy have have airplane-mode set when
all the radios known by RFKill are OFF, and unset otherwise. This policy can be
overwritten by userspace using the new operations _AIRPLANE_MODE_ACQUIRE,
_AIRPLANE_MODE_RELEASE, and _AIRPLANE_MODE_CHANGE. When the airplane-mode
indicator state changes, userspace gets notifications through the RFKill
control misc device (/dev/rfkill).

The series also contains a few general fixes and improvements to the subsystem.

João Paulo Rechi Vita (9):
  rfkill: Improve documentation language
  rfkill: Remove extra blank line
  rfkill: Point to the correct deprecated doc location
  rfkill: Move "state" sysfs file back to stable
  rfkill: Factor rfkill_global_states[].cur assignments
  rfkill: Add documentation about LED triggers
  rfkill: Create "rfkill-airplane_mode" LED trigger
  rfkill: Userspace control for airplane mode
  rfkill: Notify userspace of airplane-mode state changes

 Documentation/ABI/obsolete/sysfs-class-rfkill |  20 ----
 Documentation/ABI/stable/sysfs-class-rfkill   |  27 ++++-
 Documentation/rfkill.txt                      |  17 +++
 include/uapi/linux/rfkill.h                   |   3 +
 net/rfkill/core.c                             | 144 +++++++++++++++++++++-----
 5 files changed, 164 insertions(+), 47 deletions(-)
 delete mode 100644 Documentation/ABI/obsolete/sysfs-class-rfkill

-- 
2.5.0

[PATCH 1/9] rfkill: Improve documentation language

[João Paulo Rechi Vita]

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

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index a805831..ffbc375 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -282,8 +282,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, which 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)
-- 
2.5.0

Re: [PATCH 1/9] rfkill: Improve documentation language

[Julian Calaby]

Hi João,

On Tue, Feb 9, 2016 at 2:41 AM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>

Looks right to me.

Reviewed-by: Julian Calaby <julian.calaby@gmail.com>

> ---
>  net/rfkill/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index a805831..ffbc375 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -282,8 +282,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, which 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)
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 1/9] rfkill: Improve documentation language

[Julian Calaby]

Hi João,

On Tue, Feb 9, 2016 at 2:41 AM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
> Signed-off-by: João Paulo Rechi Vita <jprvita-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

Looks right to me.

Reviewed-by: Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> ---
>  net/rfkill/core.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index a805831..ffbc375 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -282,8 +282,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, which 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)
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/

[PATCH 2/9] rfkill: Remove extra blank line

[João Paulo Rechi Vita]

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

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index ffbc375..56d79cb 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -455,7 +455,6 @@ bool rfkill_get_global_sw_state(const enum rfkill_type type)
 }
 #endif
 
-
 bool rfkill_set_hw_state(struct rfkill *rfkill, bool blocked)
 {
 	unsigned long flags;
-- 
2.5.0

Re: [PATCH 2/9] rfkill: Remove extra blank line

[Julian Calaby]

Hi João,

On Tue, Feb 9, 2016 at 2:41 AM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>

Looks sane to me.

Reviewed-by: Julian Calaby <julian.calaby@gmail.com>

> ---
>  net/rfkill/core.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index ffbc375..56d79cb 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -455,7 +455,6 @@ bool rfkill_get_global_sw_state(const enum rfkill_type type)
>  }
>  #endif
>
> -
>  bool rfkill_set_hw_state(struct rfkill *rfkill, bool blocked)
>  {
>         unsigned long flags;
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

[PATCH 3/9] rfkill: Point to the correct deprecated doc location

[João Paulo Rechi Vita]

The "claim" sysfs interface has been removed, so its documentation now
lives in the "removed" folder.

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
 Documentation/ABI/stable/sysfs-class-rfkill | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-class-rfkill b/Documentation/ABI/stable/sysfs-class-rfkill
index 097f522..e51571e 100644
--- a/Documentation/ABI/stable/sysfs-class-rfkill
+++ b/Documentation/ABI/stable/sysfs-class-rfkill
@@ -2,8 +2,10 @@ rfkill - radio frequency (RF) connector kill switch support
 
 For details to this subsystem look at Documentation/rfkill.txt.
 
-For the deprecated /sys/class/rfkill/*/state and
-/sys/class/rfkill/*/claim knobs of this interface look in
+For the deprecated /sys/class/rfkill/*/claim knobs of this interface look in
+Documentation/ABI/removed/sysfs-class-rfkill.
+
+For the deprecated /sys/class/rfkill/*/state knobs of this interface look in
 Documentation/ABI/obsolete/sysfs-class-rfkill.
 
 What: 		/sys/class/rfkill
-- 
2.5.0

Re: [PATCH 3/9] rfkill: Point to the correct deprecated doc location

[Julian Calaby]

Hi João,

On Tue, Feb 9, 2016 at 2:41 AM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
> The "claim" sysfs interface has been removed, so its documentation now
> lives in the "removed" folder.
>
> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>

Looks right to me.

Reviewed-by: Julian Calaby <julian.calaby@gmail.com>

> ---
>  Documentation/ABI/stable/sysfs-class-rfkill | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/stable/sysfs-class-rfkill b/Documentation/ABI/stable/sysfs-class-rfkill
> index 097f522..e51571e 100644
> --- a/Documentation/ABI/stable/sysfs-class-rfkill
> +++ b/Documentation/ABI/stable/sysfs-class-rfkill
> @@ -2,8 +2,10 @@ rfkill - radio frequency (RF) connector kill switch support
>
>  For details to this subsystem look at Documentation/rfkill.txt.
>
> -For the deprecated /sys/class/rfkill/*/state and
> -/sys/class/rfkill/*/claim knobs of this interface look in
> +For the deprecated /sys/class/rfkill/*/claim knobs of this interface look in
> +Documentation/ABI/removed/sysfs-class-rfkill.
> +
> +For the deprecated /sys/class/rfkill/*/state knobs of this interface look in
>  Documentation/ABI/obsolete/sysfs-class-rfkill.
>
>  What:          /sys/class/rfkill
> --
> 2.5.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

[PATCH 4/9] rfkill: Move "state" sysfs file back to stable

[João Paulo Rechi Vita]

There is still quite a bit of code using this interface, so we can't
just remove it. Hopefully it will be possible in the future, but since
its scheduled removal date is past 2 years already, we are better having
the documentation reflecting the current state of things.

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
 Documentation/ABI/obsolete/sysfs-class-rfkill | 20 --------------------
 Documentation/ABI/stable/sysfs-class-rfkill   | 25 ++++++++++++++++++++++---
 2 files changed, 22 insertions(+), 23 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-wireless@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/stable/sysfs-class-rfkill b/Documentation/ABI/stable/sysfs-class-rfkill
index e51571e..e1ba4a1 100644
--- a/Documentation/ABI/stable/sysfs-class-rfkill
+++ b/Documentation/ABI/stable/sysfs-class-rfkill
@@ -5,9 +5,6 @@ For details to this subsystem look at Documentation/rfkill.txt.
 For the deprecated /sys/class/rfkill/*/claim knobs of this interface look in
 Documentation/ABI/removed/sysfs-class-rfkill.
 
-For the deprecated /sys/class/rfkill/*/state knobs of this interface look in
-Documentation/ABI/obsolete/sysfs-class-rfkill.
-
 What: 		/sys/class/rfkill
 Date:		09-Jul-2007
 KernelVersion:	v2.6.22
@@ -44,6 +41,28 @@ Values: 	A numeric value.
 		1: true
 
 
+What:		/sys/class/rfkill/rfkill[0-9]+/state
+Date:		09-Jul-2007
+KernelVersion	v2.6.22
+Contact:	linux-wireless@vger.kernel.org
+Description: 	Current state of the transmitter.
+		This file was scheduled to be removed in 2014, but due to its
+		large number of users it will be sticking around for a bit
+		longer. Despite it being marked as stabe, the newer "hard" and
+		"soft" interfaces should be preffered, since it is not possible
+		to express the 'soft and hard block' state of the rfkill driver
+		through this interface. There will likely be another attempt to
+		remove it in the future.
+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.
+
+
 What:		/sys/class/rfkill/rfkill[0-9]+/hard
 Date:		12-March-2010
 KernelVersion	v2.6.34
-- 
2.5.0

[PATCH 5/9] rfkill: Factor rfkill_global_states[].cur assignments

[João Paulo Rechi Vita]

Factor all assignments to rfkill_global_states[].cur into a single
function rfkill_update_global_state().

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

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 56d79cb..8b96869 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -302,6 +302,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);
 
@@ -319,15 +332,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;
@@ -1164,15 +1169,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)
@@ -1261,10 +1259,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

Re: [PATCH 5/9] rfkill: Factor rfkill_global_states[].cur assignments

[Julian Calaby]

Hi João,

On Tue, Feb 9, 2016 at 2:41 AM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
> Factor all assignments to rfkill_global_states[].cur into a single
> function rfkill_update_global_state().
>
> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>

Looks sane to me.

Reviewed-by: Julian Calaby <julian.calaby@gmail.com>

> ---
>  net/rfkill/core.c | 38 +++++++++++++++++---------------------
>  1 file changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index 56d79cb..8b96869 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -302,6 +302,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);
>
> @@ -319,15 +332,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;
> @@ -1164,15 +1169,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)
> @@ -1261,10 +1259,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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 5/9] rfkill: Factor rfkill_global_states[].cur assignments

[Julian Calaby]

Hi João,

On Tue, Feb 9, 2016 at 2:41 AM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
> Factor all assignments to rfkill_global_states[].cur into a single
> function rfkill_update_global_state().
>
> Signed-off-by: João Paulo Rechi Vita <jprvita-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>

Looks sane to me.

Reviewed-by: Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

> ---
>  net/rfkill/core.c | 38 +++++++++++++++++---------------------
>  1 file changed, 17 insertions(+), 21 deletions(-)
>
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index 56d79cb..8b96869 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -302,6 +302,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);
>
> @@ -319,15 +332,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;
> @@ -1164,15 +1169,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)
> @@ -1261,10 +1259,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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/

[PATCH 6/9] rfkill: Add documentation about LED triggers

[João Paulo Rechi Vita]

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
 Documentation/rfkill.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 2ee6ef9..1f0c270 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -83,6 +83,8 @@ rfkill drivers that control devices that can be hard-blocked unless they also
 assign the poll_hw_block() callback (then the rfkill core will poll the
 device). Don't do this unless you cannot get the event in any other way.
 
+RFKill provides per-switch LED triggers, which can be used to drive LEDs
+according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
 
 
 5. Userspace support
-- 
2.5.0

[PATCH 7/9] rfkill: Create "rfkill-airplane_mode" LED trigger

[João Paulo Rechi Vita]

This creates a new LED trigger to be used by platform drivers as a
default trigger for airplane-mode indicator LEDs.

By default this trigger will fire when RFKILL_OP_CHANGE_ALL is called
for all types (RFKILL_TYPE_ALL), setting the LED brightness to LED_FULL
when the changing the state to blocked, and to LED_OFF when the changing
the state to unblocked. In the future there will be a mechanism for
userspace to override the default policy, so it can implement its own.

This trigger will be used by the asus-wireless x86 platform driver.

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
 Documentation/rfkill.txt |  2 ++
 net/rfkill/core.c        | 49 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 1f0c270..b13025a 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -85,6 +85,8 @@ device). Don't do this unless you cannot get the event in any other way.
 
 RFKill provides per-switch LED triggers, which can be used to drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
+An airplane-mode indicator LED trigger is also available, which triggers
+LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
 
 
 5. Userspace support
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 8b96869..fb11547 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -126,6 +126,30 @@ static bool rfkill_epo_lock_active;
 
 
 #ifdef CONFIG_RFKILL_LEDS
+static struct led_trigger rfkill_apm_led_trigger;
+
+static void rfkill_apm_led_trigger_event(bool state)
+{
+	led_trigger_event(&rfkill_apm_led_trigger, state ? LED_FULL : LED_OFF);
+}
+
+static void rfkill_apm_led_trigger_activate(struct led_classdev *led)
+{
+	rfkill_apm_led_trigger_event(!rfkill_default_state);
+}
+
+static int rfkill_apm_led_trigger_register(void)
+{
+	rfkill_apm_led_trigger.name = "rfkill-airplane_mode";
+	rfkill_apm_led_trigger.activate = rfkill_apm_led_trigger_activate;
+	return led_trigger_register(&rfkill_apm_led_trigger);
+}
+
+static void rfkill_apm_led_trigger_unregister(void)
+{
+	led_trigger_unregister(&rfkill_apm_led_trigger);
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
 	struct led_trigger *trigger;
@@ -177,6 +201,19 @@ static void rfkill_led_trigger_unregister(struct rfkill *rfkill)
 	led_trigger_unregister(&rfkill->led_trigger);
 }
 #else
+static void rfkill_apm_led_trigger_event(bool state)
+{
+}
+
+static int rfkill_apm_led_trigger_register(void)
+{
+	return 0;
+}
+
+static void rfkill_apm_led_trigger_unregister(void)
+{
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
 }
@@ -313,6 +350,7 @@ static void rfkill_update_global_state(enum rfkill_type type, bool blocked)
 
 	for (i = 0; i < NUM_RFKILL_TYPES; i++)
 		rfkill_global_states[i].cur = blocked;
+	rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1260,15 +1298,22 @@ static int __init rfkill_init(void)
 {
 	int error;
 
+	error = rfkill_apm_led_trigger_register();
+	if (error)
+		goto out;
+
 	rfkill_update_global_state(RFKILL_TYPE_ALL, !rfkill_default_state);
 
 	error = class_register(&rfkill_class);
-	if (error)
+	if (error) {
+		rfkill_apm_led_trigger_unregister();
 		goto out;
+	}
 
 	error = misc_register(&rfkill_miscdev);
 	if (error) {
 		class_unregister(&rfkill_class);
+		rfkill_apm_led_trigger_unregister();
 		goto out;
 	}
 
@@ -1277,6 +1322,7 @@ static int __init rfkill_init(void)
 	if (error) {
 		misc_deregister(&rfkill_miscdev);
 		class_unregister(&rfkill_class);
+		rfkill_apm_led_trigger_unregister();
 		goto out;
 	}
 #endif
@@ -1293,5 +1339,6 @@ static void __exit rfkill_exit(void)
 #endif
 	misc_deregister(&rfkill_miscdev);
 	class_unregister(&rfkill_class);
+	rfkill_apm_led_trigger_unregister();
 }
 module_exit(rfkill_exit);
-- 
2.5.0

Re: [PATCH 7/9] rfkill: Create "rfkill-airplane_mode" LED trigger

[Johannes Berg]

On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
> This creates a new LED trigger to be used by platform drivers as a
> default trigger for airplane-mode indicator LEDs.
> 
> By default this trigger will fire when RFKILL_OP_CHANGE_ALL is called
> for all types (RFKILL_TYPE_ALL), setting the LED brightness to
> LED_FULL
> when the changing the state to blocked, and to LED_OFF when the
> changing
> the state to unblocked. In the future there will be a mechanism for
> userspace to override the default policy, so it can implement its
> own.
> 
> This trigger will be used by the asus-wireless x86 platform driver.

Just one comment - I think you should be consistent with the _ vs - and
just use "rfkill-airplane-mode" as the name.

johannes

Re: [PATCH 7/9] rfkill: Create "rfkill-airplane_mode" LED trigger

[João Paulo Rechi Vita]

On 18 February 2016 at 15:08, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
>> This creates a new LED trigger to be used by platform drivers as a
>> default trigger for airplane-mode indicator LEDs.
>>
>> By default this trigger will fire when RFKILL_OP_CHANGE_ALL is called
>> for all types (RFKILL_TYPE_ALL), setting the LED brightness to
>> LED_FULL
>> when the changing the state to blocked, and to LED_OFF when the
>> changing
>> the state to unblocked. In the future there will be a mechanism for
>> userspace to override the default policy, so it can implement its
>> own.
>>
>> This trigger will be used by the asus-wireless x86 platform driver.
>
> Just one comment - I think you should be consistent with the _ vs - and
> just use "rfkill-airplane-mode" as the name.
>

Ok, no problem. I'll change for the next version.

--
João Paulo Rechi Vita
http://about.me/jprvita

[PATCH 8/9] rfkill: Userspace control for airplane mode

[João Paulo Rechi Vita]

Provide an interface for the airplane-mode indicator be controlled from
userspace. User has to first acquire the control through
RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
it wants to be in control of the indicator. Closing the fd or using
RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.

To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
operation is used, passing the value on "struct rfkill_event.soft". If
the caller has not acquired the airplane-mode control beforehand, the
operation fails.

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
 Documentation/rfkill.txt    | 10 ++++++++++
 include/uapi/linux/rfkill.h |  3 +++
 net/rfkill/core.c           | 47 ++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index b13025a..aa6e014 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -87,6 +87,7 @@ RFKill provides per-switch LED triggers, which can be used to drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
 An airplane-mode indicator LED trigger is also available, which triggers
 LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
+The airplane-mode indicator LED trigger policy can be overridden by userspace.
 
 
 5. Userspace support
@@ -123,5 +124,14 @@ RFKILL_TYPE
 The contents of these variables corresponds to the "name", "state" and
 "type" sysfs files explained above.
 
+Userspace can also override the default airplane-mode indicator policy through
+/dev/rfkill. Control of the airplane mode indicator has to be acquired first,
+using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one userspace
+application at a time. Closing the fd or using RFKILL_OP_AIRPLANE_MODE_RELEASE
+reverts the airplane-mode indicator back to the default kernel policy and makes
+it available for other applications to take control. Changes to the
+airplane-mode indicator state can be made using RFKILL_OP_AIRPLANE_MODE_CHANGE,
+passing the new value in the 'soft' field of 'struct rfkill_event'.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 2e00dce..9cb999b 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -67,6 +67,9 @@ enum rfkill_operation {
 	RFKILL_OP_DEL,
 	RFKILL_OP_CHANGE,
 	RFKILL_OP_CHANGE_ALL,
+	RFKILL_OP_AIRPLANE_MODE_ACQUIRE,
+	RFKILL_OP_AIRPLANE_MODE_RELEASE,
+	RFKILL_OP_AIRPLANE_MODE_CHANGE,
 };
 
 /**
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index fb11547..8067701 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -89,6 +89,7 @@ struct rfkill_data {
 	struct mutex		mtx;
 	wait_queue_head_t	read_wait;
 	bool			input_handler;
+	bool			is_apm_owner;
 };
 
 
@@ -123,7 +124,7 @@ static struct {
 } rfkill_global_states[NUM_RFKILL_TYPES];
 
 static bool rfkill_epo_lock_active;
-
+static bool rfkill_apm_owned;
 
 #ifdef CONFIG_RFKILL_LEDS
 static struct led_trigger rfkill_apm_led_trigger;
@@ -350,7 +351,8 @@ static void rfkill_update_global_state(enum rfkill_type type, bool blocked)
 
 	for (i = 0; i < NUM_RFKILL_TYPES; i++)
 		rfkill_global_states[i].cur = blocked;
-	rfkill_apm_led_trigger_event(blocked);
+	if (!rfkill_apm_owned)
+		rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1183,6 +1185,7 @@ static ssize_t rfkill_fop_read(struct file *file, char __user *buf,
 static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 				size_t count, loff_t *pos)
 {
+	struct rfkill_data *data = file->private_data;
 	struct rfkill *rfkill;
 	struct rfkill_event ev;
 
@@ -1199,7 +1202,7 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 	if (copy_from_user(&ev, buf, count))
 		return -EFAULT;
 
-	if (ev.op != RFKILL_OP_CHANGE && ev.op != RFKILL_OP_CHANGE_ALL)
+	if (ev.op < RFKILL_OP_CHANGE)
 		return -EINVAL;
 
 	if (ev.type >= NUM_RFKILL_TYPES)
@@ -1207,6 +1210,34 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 
 	mutex_lock(&rfkill_global_mutex);
 
+	if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
+		if (rfkill_apm_owned && !data->is_apm_owner) {
+			count = -EACCES;
+		} else {
+			rfkill_apm_owned = true;
+			data->is_apm_owner = true;
+		}
+	}
+
+	if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {
+		if (rfkill_apm_owned && !data->is_apm_owner) {
+			count = -EACCES;
+		} else {
+			bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
+
+			rfkill_apm_owned = false;
+			data->is_apm_owner = false;
+			rfkill_apm_led_trigger_event(state);
+		}
+	}
+
+	if (ev.op == RFKILL_OP_AIRPLANE_MODE_CHANGE) {
+		if (rfkill_apm_owned && data->is_apm_owner)
+			rfkill_apm_led_trigger_event(ev.soft);
+		else
+			count = -EACCES;
+	}
+
 	if (ev.op == RFKILL_OP_CHANGE_ALL)
 		rfkill_update_global_state(ev.type, ev.soft);
 
@@ -1230,7 +1261,17 @@ static int rfkill_fop_release(struct inode *inode, struct file *file)
 	struct rfkill_int_event *ev, *tmp;
 
 	mutex_lock(&rfkill_global_mutex);
+
+	if (data->is_apm_owner) {
+		bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
+
+		rfkill_apm_owned = false;
+		data->is_apm_owner = false;
+		rfkill_apm_led_trigger_event(state);
+	}
+
 	list_del(&data->list);
+
 	mutex_unlock(&rfkill_global_mutex);
 
 	mutex_destroy(&data->mtx);
-- 
2.5.0

[PATCH 8/9] rfkill: Userspace control for airplane mode

[João Paulo Rechi Vita]

Provide an interface for the airplane-mode indicator be controlled from
userspace. User has to first acquire the control through
RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
it wants to be in control of the indicator. Closing the fd or using
RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.

To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
operation is used, passing the value on "struct rfkill_event.soft". If
the caller has not acquired the airplane-mode control beforehand, the
operation fails.

Signed-off-by: João Paulo Rechi Vita <jprvita-6IF/jdPJHihWk0Htik3J/w@public.gmane.org>
---
 Documentation/rfkill.txt    | 10 ++++++++++
 include/uapi/linux/rfkill.h |  3 +++
 net/rfkill/core.c           | 47 ++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index b13025a..aa6e014 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -87,6 +87,7 @@ RFKill provides per-switch LED triggers, which can be used to drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
 An airplane-mode indicator LED trigger is also available, which triggers
 LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
+The airplane-mode indicator LED trigger policy can be overridden by userspace.
 
 
 5. Userspace support
@@ -123,5 +124,14 @@ RFKILL_TYPE
 The contents of these variables corresponds to the "name", "state" and
 "type" sysfs files explained above.
 
+Userspace can also override the default airplane-mode indicator policy through
+/dev/rfkill. Control of the airplane mode indicator has to be acquired first,
+using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one userspace
+application at a time. Closing the fd or using RFKILL_OP_AIRPLANE_MODE_RELEASE
+reverts the airplane-mode indicator back to the default kernel policy and makes
+it available for other applications to take control. Changes to the
+airplane-mode indicator state can be made using RFKILL_OP_AIRPLANE_MODE_CHANGE,
+passing the new value in the 'soft' field of 'struct rfkill_event'.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 2e00dce..9cb999b 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -67,6 +67,9 @@ enum rfkill_operation {
 	RFKILL_OP_DEL,
 	RFKILL_OP_CHANGE,
 	RFKILL_OP_CHANGE_ALL,
+	RFKILL_OP_AIRPLANE_MODE_ACQUIRE,
+	RFKILL_OP_AIRPLANE_MODE_RELEASE,
+	RFKILL_OP_AIRPLANE_MODE_CHANGE,
 };
 
 /**
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index fb11547..8067701 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -89,6 +89,7 @@ struct rfkill_data {
 	struct mutex		mtx;
 	wait_queue_head_t	read_wait;
 	bool			input_handler;
+	bool			is_apm_owner;
 };
 
 
@@ -123,7 +124,7 @@ static struct {
 } rfkill_global_states[NUM_RFKILL_TYPES];
 
 static bool rfkill_epo_lock_active;
-
+static bool rfkill_apm_owned;
 
 #ifdef CONFIG_RFKILL_LEDS
 static struct led_trigger rfkill_apm_led_trigger;
@@ -350,7 +351,8 @@ static void rfkill_update_global_state(enum rfkill_type type, bool blocked)
 
 	for (i = 0; i < NUM_RFKILL_TYPES; i++)
 		rfkill_global_states[i].cur = blocked;
-	rfkill_apm_led_trigger_event(blocked);
+	if (!rfkill_apm_owned)
+		rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1183,6 +1185,7 @@ static ssize_t rfkill_fop_read(struct file *file, char __user *buf,
 static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 				size_t count, loff_t *pos)
 {
+	struct rfkill_data *data = file->private_data;
 	struct rfkill *rfkill;
 	struct rfkill_event ev;
 
@@ -1199,7 +1202,7 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 	if (copy_from_user(&ev, buf, count))
 		return -EFAULT;
 
-	if (ev.op != RFKILL_OP_CHANGE && ev.op != RFKILL_OP_CHANGE_ALL)
+	if (ev.op < RFKILL_OP_CHANGE)
 		return -EINVAL;
 
 	if (ev.type >= NUM_RFKILL_TYPES)
@@ -1207,6 +1210,34 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 
 	mutex_lock(&rfkill_global_mutex);
 
+	if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
+		if (rfkill_apm_owned && !data->is_apm_owner) {
+			count = -EACCES;
+		} else {
+			rfkill_apm_owned = true;
+			data->is_apm_owner = true;
+		}
+	}
+
+	if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {
+		if (rfkill_apm_owned && !data->is_apm_owner) {
+			count = -EACCES;
+		} else {
+			bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
+
+			rfkill_apm_owned = false;
+			data->is_apm_owner = false;
+			rfkill_apm_led_trigger_event(state);
+		}
+	}
+
+	if (ev.op == RFKILL_OP_AIRPLANE_MODE_CHANGE) {
+		if (rfkill_apm_owned && data->is_apm_owner)
+			rfkill_apm_led_trigger_event(ev.soft);
+		else
+			count = -EACCES;
+	}
+
 	if (ev.op == RFKILL_OP_CHANGE_ALL)
 		rfkill_update_global_state(ev.type, ev.soft);
 
@@ -1230,7 +1261,17 @@ static int rfkill_fop_release(struct inode *inode, struct file *file)
 	struct rfkill_int_event *ev, *tmp;
 
 	mutex_lock(&rfkill_global_mutex);
+
+	if (data->is_apm_owner) {
+		bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
+
+		rfkill_apm_owned = false;
+		data->is_apm_owner = false;
+		rfkill_apm_led_trigger_event(state);
+	}
+
 	list_del(&data->list);
+
 	mutex_unlock(&rfkill_global_mutex);
 
 	mutex_destroy(&data->mtx);
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Dan Williams]

On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
> Provide an interface for the airplane-mode indicator be controlled
> from
> userspace. User has to first acquire the control through
> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole
> time
> it wants to be in control of the indicator. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
> 
> To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
> operation is used, passing the value on "struct rfkill_event.soft".
> If
> the caller has not acquired the airplane-mode control beforehand, the
> operation fails.

I'd like to clarify a bit, so tell me if I'm correct or not.  Using
RFKILL_OP_AIRPLANE_MODE_CHANGE does not actually change any device
state. It's just an indicator with no relationship to any of the
registered rfkill switches, right?

I wonder if setting RFKILL_OP_AIRPLANE_MODE_CHANGE(true) shouldn't also
softblock all switches, otherwise you can set airplane mode all day
long with RFKILL_OP_AIRPLANE_MODE_CHANGE and it doesn't actually enable
airplane mode at all?

Dan

> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
> ---
>  Documentation/rfkill.txt    | 10 ++++++++++
>  include/uapi/linux/rfkill.h |  3 +++
>  net/rfkill/core.c           | 47
> ++++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 57 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
> index b13025a..aa6e014 100644
> --- a/Documentation/rfkill.txt
> +++ b/Documentation/rfkill.txt
> @@ -87,6 +87,7 @@ RFKill provides per-switch LED triggers, which can
> be used to drive LEDs
>  according to the switch state (LED_FULL when blocked, LED_OFF
> otherwise).
>  An airplane-mode indicator LED trigger is also available, which
> triggers
>  LED_FULL when all radios known by RFKill are blocked, and LED_OFF
> otherwise.
> +The airplane-mode indicator LED trigger policy can be overridden by
> userspace.
>  
>  
>  5. Userspace support
> @@ -123,5 +124,14 @@ RFKILL_TYPE
>  The contents of these variables corresponds to the "name", "state"
> and
>  "type" sysfs files explained above.
>  
> +Userspace can also override the default airplane-mode indicator
> policy through
> +/dev/rfkill. Control of the airplane mode indicator has to be
> acquired first,
> +using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one
> userspace
> +application at a time. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE
> +reverts the airplane-mode indicator back to the default kernel
> policy and makes
> +it available for other applications to take control. Changes to the
> +airplane-mode indicator state can be made using
> RFKILL_OP_AIRPLANE_MODE_CHANGE,
> +passing the new value in the 'soft' field of 'struct rfkill_event'.
> +
>  
>  For further details consult Documentation/ABI/stable/sysfs-class
> -rfkill.
> diff --git a/include/uapi/linux/rfkill.h
> b/include/uapi/linux/rfkill.h
> index 2e00dce..9cb999b 100644
> --- a/include/uapi/linux/rfkill.h
> +++ b/include/uapi/linux/rfkill.h
> @@ -67,6 +67,9 @@ enum rfkill_operation {
>  	RFKILL_OP_DEL,
>  	RFKILL_OP_CHANGE,
>  	RFKILL_OP_CHANGE_ALL,
> +	RFKILL_OP_AIRPLANE_MODE_ACQUIRE,
> +	RFKILL_OP_AIRPLANE_MODE_RELEASE,
> +	RFKILL_OP_AIRPLANE_MODE_CHANGE,
>  };
>  
>  /**
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index fb11547..8067701 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -89,6 +89,7 @@ struct rfkill_data {
>  	struct mutex		mtx;
>  	wait_queue_head_t	read_wait;
>  	bool			input_handler;
> +	bool			is_apm_owner;
>  };
>  
>  
> @@ -123,7 +124,7 @@ static struct {
>  } rfkill_global_states[NUM_RFKILL_TYPES];
>  
>  static bool rfkill_epo_lock_active;
> -
> +static bool rfkill_apm_owned;
>  
>  #ifdef CONFIG_RFKILL_LEDS
>  static struct led_trigger rfkill_apm_led_trigger;
> @@ -350,7 +351,8 @@ static void rfkill_update_global_state(enum
> rfkill_type type, bool blocked)
>  
>  	for (i = 0; i < NUM_RFKILL_TYPES; i++)
>  		rfkill_global_states[i].cur = blocked;
> -	rfkill_apm_led_trigger_event(blocked);
> +	if (!rfkill_apm_owned)
> +		rfkill_apm_led_trigger_event(blocked);
>  }
>  
>  #ifdef CONFIG_RFKILL_INPUT
> @@ -1183,6 +1185,7 @@ static ssize_t rfkill_fop_read(struct file
> *file, char __user *buf,
>  static ssize_t rfkill_fop_write(struct file *file, const char __user
> *buf,
>  				size_t count, loff_t *pos)
>  {
> +	struct rfkill_data *data = file->private_data;
>  	struct rfkill *rfkill;
>  	struct rfkill_event ev;
>  
> @@ -1199,7 +1202,7 @@ static ssize_t rfkill_fop_write(struct file
> *file, const char __user *buf,
>  	if (copy_from_user(&ev, buf, count))
>  		return -EFAULT;
>  
> -	if (ev.op != RFKILL_OP_CHANGE && ev.op !=
> RFKILL_OP_CHANGE_ALL)
> +	if (ev.op < RFKILL_OP_CHANGE)
>  		return -EINVAL;
>  
>  	if (ev.type >= NUM_RFKILL_TYPES)
> @@ -1207,6 +1210,34 @@ static ssize_t rfkill_fop_write(struct file
> *file, const char __user *buf,
>  
>  	mutex_lock(&rfkill_global_mutex);
>  
> +	if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
> +		if (rfkill_apm_owned && !data->is_apm_owner) {
> +			count = -EACCES;
> +		} else {
> +			rfkill_apm_owned = true;
> +			data->is_apm_owner = true;
> +		}
> +	}
> +
> +	if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {
> +		if (rfkill_apm_owned && !data->is_apm_owner) {
> +			count = -EACCES;
> +		} else {
> +			bool state =
> rfkill_global_states[RFKILL_TYPE_ALL].cur;
> +
> +			rfkill_apm_owned = false;
> +			data->is_apm_owner = false;
> +			rfkill_apm_led_trigger_event(state);
> +		}
> +	}
> +
> +	if (ev.op == RFKILL_OP_AIRPLANE_MODE_CHANGE) {
> +		if (rfkill_apm_owned && data->is_apm_owner)
> +			rfkill_apm_led_trigger_event(ev.soft);
> +		else
> +			count = -EACCES;
> +	}
> +
>  	if (ev.op == RFKILL_OP_CHANGE_ALL)
>  		rfkill_update_global_state(ev.type, ev.soft);
>  
> @@ -1230,7 +1261,17 @@ static int rfkill_fop_release(struct inode
> *inode, struct file *file)
>  	struct rfkill_int_event *ev, *tmp;
>  
>  	mutex_lock(&rfkill_global_mutex);
> +
> +	if (data->is_apm_owner) {
> +		bool state =
> rfkill_global_states[RFKILL_TYPE_ALL].cur;
> +
> +		rfkill_apm_owned = false;
> +		data->is_apm_owner = false;
> +		rfkill_apm_led_trigger_event(state);
> +	}
> +
>  	list_del(&data->list);
> +
>  	mutex_unlock(&rfkill_global_mutex);
>  
>  	mutex_destroy(&data->mtx);

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[João Paulo Rechi Vita]

On 8 February 2016 at 11:11, Dan Williams <dcbw@redhat.com> wrote:
> On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
>> Provide an interface for the airplane-mode indicator be controlled
>> from
>> userspace. User has to first acquire the control through
>> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole
>> time
>> it wants to be in control of the indicator. Closing the fd or using
>> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
>>
>> To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
>> operation is used, passing the value on "struct rfkill_event.soft".
>> If
>> the caller has not acquired the airplane-mode control beforehand, the
>> operation fails.
>
> I'd like to clarify a bit, so tell me if I'm correct or not.  Using
> RFKILL_OP_AIRPLANE_MODE_CHANGE does not actually change any device
> state. It's just an indicator with no relationship to any of the
> registered rfkill switches, right?
>

Yes, that's correct, and it is the intended behavior.

> I wonder if setting RFKILL_OP_AIRPLANE_MODE_CHANGE(true) shouldn't also
> softblock all switches, otherwise you can set airplane mode all day
> long with RFKILL_OP_AIRPLANE_MODE_CHANGE and it doesn't actually enable
> airplane mode at all?
>

The idea behind it is to just provide the mechanism for userspace to
decide what exactly being in airplane mode means, so it would
set/unset the indicator in the right moment.

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Johannes Berg]

On Mon, 2016-02-08 at 10:11 -0600, Dan Williams wrote:
> I'd like to clarify a bit, so tell me if I'm correct or not.  Using
> RFKILL_OP_AIRPLANE_MODE_CHANGE does not actually change any device
> state. It's just an indicator with no relationship to any of the
> registered rfkill switches, right?

Yes. I'm not sure I'm totally happy with this userspace API.

> I wonder if setting RFKILL_OP_AIRPLANE_MODE_CHANGE(true) shouldn't
> also
> softblock all switches, otherwise you can set airplane mode all day
> long with RFKILL_OP_AIRPLANE_MODE_CHANGE and it doesn't actually
> enable
> airplane mode at all?

No, this is kinda the point that you could implement your policy in
userspace now.

johannes

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Johannes Berg]

On Mon, 2016-02-08 at 10:11 -0600, Dan Williams wrote:
> I'd like to clarify a bit, so tell me if I'm correct or not.  Using
> RFKILL_OP_AIRPLANE_MODE_CHANGE does not actually change any device
> state. It's just an indicator with no relationship to any of the
> registered rfkill switches, right?

Yes. I'm not sure I'm totally happy with this userspace API.

> I wonder if setting RFKILL_OP_AIRPLANE_MODE_CHANGE(true) shouldn't
> also
> softblock all switches, otherwise you can set airplane mode all day
> long with RFKILL_OP_AIRPLANE_MODE_CHANGE and it doesn't actually
> enable
> airplane mode at all?

No, this is kinda the point that you could implement your policy in
userspace now.

johannes

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Dan Williams]

On Wed, 2016-02-10 at 17:07 +0100, Johannes Berg wrote:
> On Mon, 2016-02-08 at 10:11 -0600, Dan Williams wrote:
> > I'd like to clarify a bit, so tell me if I'm correct or not.  Using
> > RFKILL_OP_AIRPLANE_MODE_CHANGE does not actually change any device
> > state. It's just an indicator with no relationship to any of the
> > registered rfkill switches, right?
> 
> Yes. I'm not sure I'm totally happy with this userspace API.
> 
> > I wonder if setting RFKILL_OP_AIRPLANE_MODE_CHANGE(true) shouldn't
> > also
> > softblock all switches, otherwise you can set airplane mode all day
> > long with RFKILL_OP_AIRPLANE_MODE_CHANGE and it doesn't actually
> > enable
> > airplane mode at all?
> 
> No, this is kinda the point that you could implement your policy in
> userspace now.

Yeah, I get that now.  It's just that to me, something called
"AIRPLANE_MODE_CHANGE" seems like it should actually change airplane
mode on/off, which implies killing radios.  I wouldn't have had the
problem if it was named AIRPLANE_MODE_INDICATOR_CHANGE, which makes it
clear this is simply an indicator and has no actual effect on anything
other than a LED.

Dan

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Johannes Berg]

On 2016-02-10 17:53, Dan Williams wrote:
> Yeah, I get that now.  It's just that to me, something called
> "AIRPLANE_MODE_CHANGE" seems like it should actually change airplane
> mode on/off, which implies killing radios.  I wouldn't have had the
> problem if it was named AIRPLANE_MODE_INDICATOR_CHANGE, which makes it
> clear this is simply an indicator and has no actual effect on anything
> other than a LED.

Yeah, I agree. I'm not even sure that it's a good idea to subsume this 
into the regular operations in the API, although that's obviously the 
easiest extension. I'll need to think about that a bit more with the 
code at hand though.

johannes

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[João Paulo Rechi Vita]

On 10 February 2016 at 12:12, Johannes Berg <johannes@sipsolutions.net> wrote:
> On 2016-02-10 17:53, Dan Williams wrote:
>>
>> Yeah, I get that now.  It's just that to me, something called
>> "AIRPLANE_MODE_CHANGE" seems like it should actually change airplane
>> mode on/off, which implies killing radios.  I wouldn't have had the
>> problem if it was named AIRPLANE_MODE_INDICATOR_CHANGE, which makes it
>> clear this is simply an indicator and has no actual effect on anything
>> other than a LED.
>

I also agree the AIRPLANE_MODE_INDICATOR_* prefix makes things more
clear here. Thanks for the feedback, Dan!

>
> Yeah, I agree. I'm not even sure that it's a good idea to subsume this into
> the regular operations in the API, although that's obviously the easiest
> extension. I'll need to think about that a bit more with the code at hand
> though.
>

Initially I have thought about creating and additional RFKill switch
for that, but I think it would be a bit hackish since we would have to
treat it specially in sysfs attributes and probably other places, and
userspace would also need a special case when going through the list
of RFKill switches in the system. The proposed solution has equivalent
semantics to an RFKill switch, is backward-compatible (users would
only ignore the unknown operations and evens -- although gsd has a
"default:" case to abort execution on an unknown event) and does not
extend the RFKill event struct.

One alternative would be to move the control point to a separate
device, like /dev/rfkill-airplane-mode, but I'm not sure this is a
very elegant approach. Anyway, I'm open to work on changes to the API,
but it would be great if you could at least pick or reject the
non-polemical patches of the series, so I don't need to carry them
anymore.

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Marcel Holtmann]

Hi Joa Paulo,

> Provide an interface for the airplane-mode indicator be controlled from
> userspace. User has to first acquire the control through
> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
> it wants to be in control of the indicator. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
> 
> To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
> operation is used, passing the value on "struct rfkill_event.soft". If
> the caller has not acquired the airplane-mode control beforehand, the
> operation fails.

as explained in an earlier response, the concept Airplane Mode seems to be imposing policy into the kernel. Do we really want have this as a kernel exposed API.

Regards

Marcel

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Marcel Holtmann]

Hi Joa Paulo,

> Provide an interface for the airplane-mode indicator be controlled from
> userspace. User has to first acquire the control through
> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
> it wants to be in control of the indicator. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
> 
> To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
> operation is used, passing the value on "struct rfkill_event.soft". If
> the caller has not acquired the airplane-mode control beforehand, the
> operation fails.

as explained in an earlier response, the concept Airplane Mode seems to be imposing policy into the kernel. Do we really want have this as a kernel exposed API.

Regards

Marcel

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Johannes Berg]

On 2016-02-08 17:20, Marcel Holtmann wrote:

> as explained in an earlier response, the concept Airplane Mode seems
> to be imposing policy into the kernel. Do we really want have this as
> a kernel exposed API.

This patch is the one that *solves* that problem ... please read it more 
carefully?

johannes

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[João Paulo Rechi Vita]

Hello Marcel,

On 8 February 2016 at 11:20, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Joa Paulo,
>
>> Provide an interface for the airplane-mode indicator be controlled from
>> userspace. User has to first acquire the control through
>> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
>> it wants to be in control of the indicator. Closing the fd or using
>> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
>>
>> To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
>> operation is used, passing the value on "struct rfkill_event.soft". If
>> the caller has not acquired the airplane-mode control beforehand, the
>> operation fails.
>
> as explained in an earlier response, the concept Airplane Mode seems to be imposing policy into the kernel. Do we really want have this as a kernel exposed API.
>

On that very same thread we decided to keep using the "airplane mode"
name both for when having the default policy of "set it when all
radios are off" or when allowing userspace to override the default.
Please see the following message from Johannes
http://www.spinics.net/lists/linux-wireless/msg146069.html and let me
know if that covers your concern.

Thanks!

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Julian Calaby]

Hi João,

On Tue, Feb 9, 2016 at 2:41 AM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
> Provide an interface for the airplane-mode indicator be controlled from
> userspace. User has to first acquire the control through
> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
> it wants to be in control of the indicator. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
>
> To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
> operation is used, passing the value on "struct rfkill_event.soft". If
> the caller has not acquired the airplane-mode control beforehand, the
> operation fails.
>
> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
> ---
>  Documentation/rfkill.txt    | 10 ++++++++++
>  include/uapi/linux/rfkill.h |  3 +++
>  net/rfkill/core.c           | 47 ++++++++++++++++++++++++++++++++++++++++++---
>  3 files changed, 57 insertions(+), 3 deletions(-)
>
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index fb11547..8067701 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -1207,6 +1210,34 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
>
>         mutex_lock(&rfkill_global_mutex);
>
> +       if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
> +               if (rfkill_apm_owned && !data->is_apm_owner) {
> +                       count = -EACCES;
> +               } else {
> +                       rfkill_apm_owned = true;
> +                       data->is_apm_owner = true;
> +               }
> +       }
> +
> +       if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {
> +               if (rfkill_apm_owned && !data->is_apm_owner) {

Are you sure this is correct?

In the case that airplane mode isn't owned, the
rfkill_apm_led_trigger_event() call will be a noop, so we should
arguably not be calling it.

Also, should we just fail silently if we're not the owner? I.e. what
does userspace learn from this op failing and is that useful?

> +                       count = -EACCES;
> +               } else {
> +                       bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
> +
> +                       rfkill_apm_owned = false;
> +                       data->is_apm_owner = false;
> +                       rfkill_apm_led_trigger_event(state);
> +               }
> +       }
> +
> +       if (ev.op == RFKILL_OP_AIRPLANE_MODE_CHANGE) {
> +               if (rfkill_apm_owned && data->is_apm_owner)
> +                       rfkill_apm_led_trigger_event(ev.soft);
> +               else
> +                       count = -EACCES;
> +       }
> +
>         if (ev.op == RFKILL_OP_CHANGE_ALL)
>                 rfkill_update_global_state(ev.type, ev.soft);
>
> @@ -1230,7 +1261,17 @@ static int rfkill_fop_release(struct inode *inode, struct file *file)
>         struct rfkill_int_event *ev, *tmp;
>
>         mutex_lock(&rfkill_global_mutex);
> +
> +       if (data->is_apm_owner) {
> +               bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
> +
> +               rfkill_apm_owned = false;
> +               data->is_apm_owner = false;
> +               rfkill_apm_led_trigger_event(state);

Also, this code is duplicated from the _RELEASE op above. Would it
make sense to factor it out into a separate function?

> +       }
> +
>         list_del(&data->list);
> +

(extra line)

>         mutex_unlock(&rfkill_global_mutex);
>
>         mutex_destroy(&data->mtx);

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[João Paulo Rechi Vita]

On 8 February 2016 at 17:53, Julian Calaby <julian.calaby@gmail.com> wrote:

>> +       if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {
>> +               if (rfkill_apm_owned && !data->is_apm_owner) {
>
> Are you sure this is correct?
>
> In the case that airplane mode isn't owned, the
> rfkill_apm_led_trigger_event() call will be a noop, so we should
> arguably not be calling it.
>

Ok, I'm changing the check to be consistent with _CHANGE, so the call
only succeeds if (rfkill_apm_owned && data->is_apm_owner), and return
an error otherwise.

> Also, should we just fail silently if we're not the owner? I.e. what
> does userspace learn from this op failing and is that useful?
>

I think it is better to return an error every time userspace is trying
to call an operation that it was not supposed to call at a certain
state (without acquiring control of the airplane-mode indicator). If a
program has a logic error that makes it call _RELEASE without calling
_ACQUIRE first, it's easier for the programmer to spot the problem if
we return an error here.

>> +                       count = -EACCES;
>> +               } else {
>> +                       bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
>> +
>> +                       rfkill_apm_owned = false;
>> +                       data->is_apm_owner = false;
>> +                       rfkill_apm_led_trigger_event(state);
>> +               }
>> +       }
>> +
>> +       if (ev.op == RFKILL_OP_AIRPLANE_MODE_CHANGE) {
>> +               if (rfkill_apm_owned && data->is_apm_owner)
>> +                       rfkill_apm_led_trigger_event(ev.soft);
>> +               else
>> +                       count = -EACCES;
>> +       }
>> +
>>         if (ev.op == RFKILL_OP_CHANGE_ALL)
>>                 rfkill_update_global_state(ev.type, ev.soft);
>>
>> @@ -1230,7 +1261,17 @@ static int rfkill_fop_release(struct inode *inode, struct file *file)
>>         struct rfkill_int_event *ev, *tmp;
>>
>>         mutex_lock(&rfkill_global_mutex);
>> +
>> +       if (data->is_apm_owner) {
>> +               bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
>> +
>> +               rfkill_apm_owned = false;
>> +               data->is_apm_owner = false;
>> +               rfkill_apm_led_trigger_event(state);
>
> Also, this code is duplicated from the _RELEASE op above. Would it
> make sense to factor it out into a separate function?
>

Yes, makes sense. This also made me notice I was assigning a negative
value to a size_t variable (count).

>> +       }
>> +
>>         list_del(&data->list);
>> +
>
> (extra line)
>

After factoring out the _RELEASE code it looks better without this
additional line.

>>         mutex_unlock(&rfkill_global_mutex);
>>
>>         mutex_destroy(&data->mtx);
>
> Thanks,
>

Thanks for the review, Julian. I'm sending an updated version.

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[João Paulo Rechi Vita]

On 8 February 2016 at 17:53, Julian Calaby <julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

>> +       if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {
>> +               if (rfkill_apm_owned && !data->is_apm_owner) {
>
> Are you sure this is correct?
>
> In the case that airplane mode isn't owned, the
> rfkill_apm_led_trigger_event() call will be a noop, so we should
> arguably not be calling it.
>

Ok, I'm changing the check to be consistent with _CHANGE, so the call
only succeeds if (rfkill_apm_owned && data->is_apm_owner), and return
an error otherwise.

> Also, should we just fail silently if we're not the owner? I.e. what
> does userspace learn from this op failing and is that useful?
>

I think it is better to return an error every time userspace is trying
to call an operation that it was not supposed to call at a certain
state (without acquiring control of the airplane-mode indicator). If a
program has a logic error that makes it call _RELEASE without calling
_ACQUIRE first, it's easier for the programmer to spot the problem if
we return an error here.

>> +                       count = -EACCES;
>> +               } else {
>> +                       bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
>> +
>> +                       rfkill_apm_owned = false;
>> +                       data->is_apm_owner = false;
>> +                       rfkill_apm_led_trigger_event(state);
>> +               }
>> +       }
>> +
>> +       if (ev.op == RFKILL_OP_AIRPLANE_MODE_CHANGE) {
>> +               if (rfkill_apm_owned && data->is_apm_owner)
>> +                       rfkill_apm_led_trigger_event(ev.soft);
>> +               else
>> +                       count = -EACCES;
>> +       }
>> +
>>         if (ev.op == RFKILL_OP_CHANGE_ALL)
>>                 rfkill_update_global_state(ev.type, ev.soft);
>>
>> @@ -1230,7 +1261,17 @@ static int rfkill_fop_release(struct inode *inode, struct file *file)
>>         struct rfkill_int_event *ev, *tmp;
>>
>>         mutex_lock(&rfkill_global_mutex);
>> +
>> +       if (data->is_apm_owner) {
>> +               bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
>> +
>> +               rfkill_apm_owned = false;
>> +               data->is_apm_owner = false;
>> +               rfkill_apm_led_trigger_event(state);
>
> Also, this code is duplicated from the _RELEASE op above. Would it
> make sense to factor it out into a separate function?
>

Yes, makes sense. This also made me notice I was assigning a negative
value to a size_t variable (count).

>> +       }
>> +
>>         list_del(&data->list);
>> +
>
> (extra line)
>

After factoring out the _RELEASE code it looks better without this
additional line.

>>         mutex_unlock(&rfkill_global_mutex);
>>
>>         mutex_destroy(&data->mtx);
>
> Thanks,
>

Thanks for the review, Julian. I'm sending an updated version.

--
João Paulo Rechi Vita
http://about.me/jprvita

[PATCH 8/9] rfkill: Userspace control for airplane mode

[João Paulo Rechi Vita]

Provide an interface for the airplane-mode indicator be controlled from
userspace. User has to first acquire the control through
RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
it wants to be in control of the indicator. Closing the fd or using
RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.

To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
operation is used, passing the value on "struct rfkill_event.soft". If
the caller has not acquired the airplane-mode control beforehand, the
operation fails.

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
 Documentation/rfkill.txt    | 10 ++++++++++
 include/uapi/linux/rfkill.h |  3 +++
 net/rfkill/core.c           | 45 +++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index b13025a..aa6e014 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -87,6 +87,7 @@ RFKill provides per-switch LED triggers, which can be used to drive LEDs
 according to the switch state (LED_FULL when blocked, LED_OFF otherwise).
 An airplane-mode indicator LED trigger is also available, which triggers
 LED_FULL when all radios known by RFKill are blocked, and LED_OFF otherwise.
+The airplane-mode indicator LED trigger policy can be overridden by userspace.
 
 
 5. Userspace support
@@ -123,5 +124,14 @@ RFKILL_TYPE
 The contents of these variables corresponds to the "name", "state" and
 "type" sysfs files explained above.
 
+Userspace can also override the default airplane-mode indicator policy through
+/dev/rfkill. Control of the airplane mode indicator has to be acquired first,
+using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one userspace
+application at a time. Closing the fd or using RFKILL_OP_AIRPLANE_MODE_RELEASE
+reverts the airplane-mode indicator back to the default kernel policy and makes
+it available for other applications to take control. Changes to the
+airplane-mode indicator state can be made using RFKILL_OP_AIRPLANE_MODE_CHANGE,
+passing the new value in the 'soft' field of 'struct rfkill_event'.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 2e00dce..9cb999b 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -67,6 +67,9 @@ enum rfkill_operation {
 	RFKILL_OP_DEL,
 	RFKILL_OP_CHANGE,
 	RFKILL_OP_CHANGE_ALL,
+	RFKILL_OP_AIRPLANE_MODE_ACQUIRE,
+	RFKILL_OP_AIRPLANE_MODE_RELEASE,
+	RFKILL_OP_AIRPLANE_MODE_CHANGE,
 };
 
 /**
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index fb11547..c0da716 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -89,6 +89,7 @@ struct rfkill_data {
 	struct mutex		mtx;
 	wait_queue_head_t	read_wait;
 	bool			input_handler;
+	bool			is_apm_owner;
 };
 
 
@@ -123,7 +124,7 @@ static struct {
 } rfkill_global_states[NUM_RFKILL_TYPES];
 
 static bool rfkill_epo_lock_active;
-
+static bool rfkill_apm_owned;
 
 #ifdef CONFIG_RFKILL_LEDS
 static struct led_trigger rfkill_apm_led_trigger;
@@ -350,7 +351,8 @@ static void rfkill_update_global_state(enum rfkill_type type, bool blocked)
 
 	for (i = 0; i < NUM_RFKILL_TYPES; i++)
 		rfkill_global_states[i].cur = blocked;
-	rfkill_apm_led_trigger_event(blocked);
+	if (!rfkill_apm_owned)
+		rfkill_apm_led_trigger_event(blocked);
 }
 
 #ifdef CONFIG_RFKILL_INPUT
@@ -1180,11 +1182,26 @@ static ssize_t rfkill_fop_read(struct file *file, char __user *buf,
 	return ret;
 }
 
+static int rfkill_airplane_mode_release(struct rfkill_data *data)
+{
+	bool state = rfkill_global_states[RFKILL_TYPE_ALL].cur;
+
+	if (rfkill_apm_owned && data->is_apm_owner) {
+		rfkill_apm_owned = false;
+		data->is_apm_owner = false;
+		rfkill_apm_led_trigger_event(state);
+		return 0;
+	}
+	return -EACCES;
+}
+
 static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 				size_t count, loff_t *pos)
 {
+	struct rfkill_data *data = file->private_data;
 	struct rfkill *rfkill;
 	struct rfkill_event ev;
+	int ret = 0;
 
 	/* we don't need the 'hard' variable but accept it */
 	if (count < RFKILL_EVENT_SIZE_V1 - 1)
@@ -1199,7 +1216,7 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 	if (copy_from_user(&ev, buf, count))
 		return -EFAULT;
 
-	if (ev.op != RFKILL_OP_CHANGE && ev.op != RFKILL_OP_CHANGE_ALL)
+	if (ev.op < RFKILL_OP_CHANGE)
 		return -EINVAL;
 
 	if (ev.type >= NUM_RFKILL_TYPES)
@@ -1207,6 +1224,25 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 
 	mutex_lock(&rfkill_global_mutex);
 
+	if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
+		if (rfkill_apm_owned && !data->is_apm_owner) {
+			ret = -EACCES;
+		} else {
+			rfkill_apm_owned = true;
+			data->is_apm_owner = true;
+		}
+	}
+
+	if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE)
+		ret = rfkill_airplane_mode_release(data);
+
+	if (ev.op == RFKILL_OP_AIRPLANE_MODE_CHANGE) {
+		if (rfkill_apm_owned && data->is_apm_owner)
+			rfkill_apm_led_trigger_event(ev.soft);
+		else
+			ret = -EACCES;
+	}
+
 	if (ev.op == RFKILL_OP_CHANGE_ALL)
 		rfkill_update_global_state(ev.type, ev.soft);
 
@@ -1221,7 +1257,7 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 	}
 	mutex_unlock(&rfkill_global_mutex);
 
-	return count;
+	return ret ? ret : count;
 }
 
 static int rfkill_fop_release(struct inode *inode, struct file *file)
@@ -1230,6 +1266,7 @@ static int rfkill_fop_release(struct inode *inode, struct file *file)
 	struct rfkill_int_event *ev, *tmp;
 
 	mutex_lock(&rfkill_global_mutex);
+	rfkill_airplane_mode_release(data);
 	list_del(&data->list);
 	mutex_unlock(&rfkill_global_mutex);
 
-- 
2.5.0

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Julian Calaby]

Hi All,

On Wed, Feb 10, 2016 at 8:36 AM, João Paulo Rechi Vita
<jprvita@gmail.com> wrote:
> Provide an interface for the airplane-mode indicator be controlled from
> userspace. User has to first acquire the control through
> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole time
> it wants to be in control of the indicator. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
>
> To change state of the indicator, the RFKILL_OP_AIRPLANE_MODE_CHANGE
> operation is used, passing the value on "struct rfkill_event.soft". If
> the caller has not acquired the airplane-mode control beforehand, the
> operation fails.
>
> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>

This looks sane to me.

Reviewed-by: Julian Calaby <julian.calaby@gmail.com>

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[Johannes Berg]

Hi,

Sorry for the delay reviewing this.



On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
> Provide an interface for the airplane-mode indicator be controlled
> from
> userspace. User has to first acquire the control through
> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole
> time
> it wants to be in control of the indicator. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.

I've come to the conclusion that the new ops are probably the best
thing to do here.

> +Userspace can also override the default airplane-mode indicator
> policy through
> +/dev/rfkill. Control of the airplane mode indicator has to be
> acquired first,
> +using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one
> userspace
> +application at a time. Closing the fd or using
> RFKILL_OP_AIRPLANE_MODE_RELEASE
> +reverts the airplane-mode indicator back to the default kernel
> policy and makes
> +it available for other applications to take control. Changes to the
> +airplane-mode indicator state can be made using
> RFKILL_OP_AIRPLANE_MODE_CHANGE,
> +passing the new value in the 'soft' field of 'struct rfkill_event'.

I don't really see any value in _RELEASE, since an application can just
close the fd? I'd prefer not having the duplicate functionality
and force us to exercise the single code path every time.

>  For further details consult Documentation/ABI/stable/sysfs-class-
> rfkill.
> diff --git a/include/uapi/linux/rfkill.h
> b/include/uapi/linux/rfkill.h
> index 2e00dce..9cb999b 100644
> --- a/include/uapi/linux/rfkill.h
> +++ b/include/uapi/linux/rfkill.h
> @@ -67,6 +67,9 @@ enum rfkill_operation {
>  	RFKILL_OP_DEL,
>  	RFKILL_OP_CHANGE,
>  	RFKILL_OP_CHANGE_ALL,
> +	RFKILL_OP_AIRPLANE_MODE_ACQUIRE,
> +	RFKILL_OP_AIRPLANE_MODE_RELEASE,
> +	RFKILL_OP_AIRPLANE_MODE_CHANGE,
>  };
 
> @@ -1199,7 +1202,7 @@ static ssize_t rfkill_fop_write(struct file
> *file, const char __user *buf,
>  	if (copy_from_user(&ev, buf, count))
>  		return -EFAULT;
>  
> -	if (ev.op != RFKILL_OP_CHANGE && ev.op !=
> RFKILL_OP_CHANGE_ALL)
> +	if (ev.op < RFKILL_OP_CHANGE)
>  		return -EINVAL;

You need to also reject invalid high values, like 27.

>  	mutex_lock(&rfkill_global_mutex);
>  
> +	if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
> +		if (rfkill_apm_owned && !data->is_apm_owner) {
> +			count = -EACCES;
> +		} else {
> +			rfkill_apm_owned = true;
> +			data->is_apm_owner = true;
> +		}
> +	}
> +
> +	if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {

It would probably be better to simply use "switch (ev.op)" and make the
default case do a reject.

>  	if (ev.op == RFKILL_OP_CHANGE_ALL)
>  		rfkill_update_global_state(ev.type, ev.soft);

Also moving the existing code inside the switch, of course.

johannes

Re: [PATCH 8/9] rfkill: Userspace control for airplane mode

[João Paulo Rechi Vita]

On 18 February 2016 at 15:12, Johannes Berg <johannes@sipsolutions.net> wrote:
> Hi,
>
> Sorry for the delay reviewing this.
>

No problems!

>
> On Mon, 2016-02-08 at 10:41 -0500, João Paulo Rechi Vita wrote:
>> Provide an interface for the airplane-mode indicator be controlled
>> from
>> userspace. User has to first acquire the control through
>> RFKILL_OP_AIRPLANE_MODE_ACQUIRE and keep the fd open for the whole
>> time
>> it wants to be in control of the indicator. Closing the fd or using
>> RFKILL_OP_AIRPLANE_MODE_RELEASE restores the default policy.
>
> I've come to the conclusion that the new ops are probably the best
> thing to do here.
>

Nice.

>> +Userspace can also override the default airplane-mode indicator
>> policy through
>> +/dev/rfkill. Control of the airplane mode indicator has to be
>> acquired first,
>> +using RFKILL_OP_AIRPLANE_MODE_ACQUIRE, and is only available for one
>> userspace
>> +application at a time. Closing the fd or using
>> RFKILL_OP_AIRPLANE_MODE_RELEASE
>> +reverts the airplane-mode indicator back to the default kernel
>> policy and makes
>> +it available for other applications to take control. Changes to the
>> +airplane-mode indicator state can be made using
>> RFKILL_OP_AIRPLANE_MODE_CHANGE,
>> +passing the new value in the 'soft' field of 'struct rfkill_event'.
>
> I don't really see any value in _RELEASE, since an application can just
> close the fd? I'd prefer not having the duplicate functionality
> and force us to exercise the single code path every time.
>

I actually added this op only for completion, I couldn't think of a
use-case where simply closing the fd wouldn't be enough. I'll remove
it for the next revision.

>>  For further details consult Documentation/ABI/stable/sysfs-class-
>> rfkill.
>> diff --git a/include/uapi/linux/rfkill.h
>> b/include/uapi/linux/rfkill.h
>> index 2e00dce..9cb999b 100644
>> --- a/include/uapi/linux/rfkill.h
>> +++ b/include/uapi/linux/rfkill.h
>> @@ -67,6 +67,9 @@ enum rfkill_operation {
>>       RFKILL_OP_DEL,
>>       RFKILL_OP_CHANGE,
>>       RFKILL_OP_CHANGE_ALL,
>> +     RFKILL_OP_AIRPLANE_MODE_ACQUIRE,
>> +     RFKILL_OP_AIRPLANE_MODE_RELEASE,
>> +     RFKILL_OP_AIRPLANE_MODE_CHANGE,
>>  };
>
>> @@ -1199,7 +1202,7 @@ static ssize_t rfkill_fop_write(struct file
>> *file, const char __user *buf,
>>       if (copy_from_user(&ev, buf, count))
>>               return -EFAULT;
>>
>> -     if (ev.op != RFKILL_OP_CHANGE && ev.op !=
>> RFKILL_OP_CHANGE_ALL)
>> +     if (ev.op < RFKILL_OP_CHANGE)
>>               return -EINVAL;
>
> You need to also reject invalid high values, like 27.
>

Yes, sorry for missing this.

>>       mutex_lock(&rfkill_global_mutex);
>>
>> +     if (ev.op == RFKILL_OP_AIRPLANE_MODE_ACQUIRE) {
>> +             if (rfkill_apm_owned && !data->is_apm_owner) {
>> +                     count = -EACCES;
>> +             } else {
>> +                     rfkill_apm_owned = true;
>> +                     data->is_apm_owner = true;
>> +             }
>> +     }
>> +
>> +     if (ev.op == RFKILL_OP_AIRPLANE_MODE_RELEASE) {
>
> It would probably be better to simply use "switch (ev.op)" and make the
> default case do a reject.
>

Sounds better indeed.

>>       if (ev.op == RFKILL_OP_CHANGE_ALL)
>>               rfkill_update_global_state(ev.type, ev.soft);
>
> Also moving the existing code inside the switch, of course.
>

Sure.

--
João Paulo Rechi Vita
http://about.me/jprvita

[PATCH 9/9] rfkill: Notify userspace of airplane-mode state changes

[João Paulo Rechi Vita]

Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
---
 Documentation/rfkill.txt |  3 +++
 net/rfkill/core.c        | 13 +++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index aa6e014..5248812 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -133,5 +133,8 @@ it available for other applications to take control. Changes to the
 airplane-mode indicator state can be made using RFKILL_OP_AIRPLANE_MODE_CHANGE,
 passing the new value in the 'soft' field of 'struct rfkill_event'.
 
+This same API is also used to provide userspace with notifications of changes
+to airplane-mode indicator state.
+
 
 For further details consult Documentation/ABI/stable/sysfs-class-rfkill.
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 8067701..abbb8f7 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -131,7 +131,20 @@ static struct led_trigger rfkill_apm_led_trigger;
 
 static void rfkill_apm_led_trigger_event(bool state)
 {
+	struct rfkill_data *data;
+	struct rfkill_int_event *ev;
+
 	led_trigger_event(&rfkill_apm_led_trigger, state ? LED_FULL : LED_OFF);
+
+	list_for_each_entry(data, &rfkill_fds, list) {
+		ev = kzalloc(sizeof(*ev), GFP_KERNEL);
+		if (!ev)
+			continue;
+		ev->ev.op = RFKILL_OP_AIRPLANE_MODE_CHANGE;
+		ev->ev.soft = state;
+		list_add_tail(&ev->list, &data->events);
+		wake_up_interruptible(&data->read_wait);
+	}
 }
 
 static void rfkill_apm_led_trigger_activate(struct led_classdev *led)
-- 
2.5.0