[RESEND PATCH 0/3] RFKill airplane-mode indicator

[RESEND PATCH 0/3] 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. By default the trigger fires on RFKILL_OP_CHANGE_ALL,
but this policy can be overwritten by userspace using the new operations
_AIRPLANE_MODE_INDICATOR_ACQUIRE and _AIRPLANE_MODE_INDICATOR_CHANGE. When the
airplane-mode indicator state changes, userspace gets notifications through the
RFKill control misc device (/dev/rfkill). I also have patches to the rfkill
userspace tool that makes use of this interface, so it can serve as API usage
example and to do quick checks on a running system, which I'll send in a
separate series.

After some hiatus I found the time to get back to this, and this time I've used
Jouni's hwsim suite to test the patches on top of mac80211-next/master. Lines
containing rfkill are pasted bellow:

START wext_rfkill 14/1880
PASS wext_rfkill 4.100002 2016-04-29 12:30:23.792682
START rfkill_wpas 571/1880
PASS rfkill_wpas 1.245307 2016-04-29 12:48:51.804344
START rfkill_autogo 572/1880
PASS rfkill_autogo 1.154174 2016-04-29 12:48:52.959605
START rfkill_p2p_discovery 573/1880
PASS rfkill_p2p_discovery 0.534903 2016-04-29 12:48:53.495547
START rfkill_open 574/1880
PASS rfkill_open 0.34073 2016-04-29 12:48:53.836963
START rfkill_p2p_discovery_p2p_dev 575/1880
PASS rfkill_p2p_discovery_p2p_dev 1.159446 2016-04-29 12:48:54.997555
START rfkill_hostapd 576/1880
PASS rfkill_hostapd 3.686868 2016-04-29 12:48:58.685162
START rfkill_wpa2_psk 577/1880
PASS rfkill_wpa2_psk 0.330014 2016-04-29 12:48:59.016711

João Paulo Rechi Vita (3):
  rfkill: Create "rfkill-airplane-mode" LED trigger
  rfkill: Userspace control for airplane mode
  rfkill: Notify userspace of airplane-mode state changes

 Documentation/rfkill.txt    |  15 +++++++
 include/uapi/linux/rfkill.h |   6 +++
 net/rfkill/core.c           | 100 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 119 insertions(+), 2 deletions(-)

-- 
2.5.0

[RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[João Paulo Rechi Vita]

From: João Paulo Rechi Vita <jprvita@gmail.com>

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 884027f..9adf95e 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
@@ -1262,15 +1300,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;
 	}
 
@@ -1279,6 +1324,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
@@ -1295,5 +1341,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: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Pavel Machek]

Hi!

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

If userspace wants to control the manually, it can do just that --
control it manually. There should not be a need to "override the
default policy".
								Pavel
								
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Johannes Berg]

On Wed, 2016-05-04 at 09:29 +0200, Pavel Machek wrote:
> 
> If userspace wants to control the manually, it can do just that --
> control it manually. There should not be a need to "override the
> default policy".

I'm still not buying this.

In the original situation, without these patches, userspace has to have
a list of all LEDs that are supposed to indicate airplane mode.

With this patch only (without patch 2/3), userspace can look up the
default trigger, but then has to change it, causing the necessary
information to be lost immediately when you actually use it - that also
seems like a bad idea.

With the patches, the userspace that cares can also concentrate on
something it already *does* - i.e. dealing with the rfkill API - and
let the rest of the situation be sorted out in itself.


Now, if the LED subsystem had a really good way of specifying LED
intent, and it was widely used, and rfkill didn't already concern
itself with the rfkill status of all devices ... yeah maybe this
wouldn't be needed. As it stands, I still think this is the best way
forward.

johannes

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Johannes Berg]

On Wed, 2016-05-04 at 09:29 +0200, Pavel Machek wrote:
> 
> If userspace wants to control the manually, it can do just that --
> control it manually. There should not be a need to "override the
> default policy".

I'm still not buying this.

In the original situation, without these patches, userspace has to have
a list of all LEDs that are supposed to indicate airplane mode.

With this patch only (without patch 2/3), userspace can look up the
default trigger, but then has to change it, causing the necessary
information to be lost immediately when you actually use it - that also
seems like a bad idea.

With the patches, the userspace that cares can also concentrate on
something it already *does* - i.e. dealing with the rfkill API - and
let the rest of the situation be sorted out in itself.


Now, if the LED subsystem had a really good way of specifying LED
intent, and it was widely used, and rfkill didn't already concern
itself with the rfkill status of all devices ... yeah maybe this
wouldn't be needed. As it stands, I still think this is the best way
forward.

johannes

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Pavel Machek]

On Thu 2016-05-12 11:32:52, Johannes Berg wrote:
> On Wed, 2016-05-04 at 09:29 +0200, Pavel Machek wrote:
> > 
> > If userspace wants to control the manually, it can do just that --
> > control it manually. There should not be a need to "override the
> > default policy".
> 
> I'm still not buying this.
> 
> In the original situation, without these patches, userspace has to have
> a list of all LEDs that are supposed to indicate airplane mode.

Well, that's situation for many LEDs.

> With this patch only (without patch 2/3), userspace can look up the
> default trigger, but then has to change it, causing the necessary
> information to be lost immediately when you actually use it - that also
> seems like a bad idea.

We should not store "what kind of led this is" in a trigger. LED
subsystem seems to use suffix of LED name to do that. So if we
standartize, lets say "::rfkill" suffix for this, it should work and
follow existing practice.

> Now, if the LED subsystem had a really good way of specifying LED
> intent, and it was widely used, and rfkill didn't already concern
> itself with the rfkill status of all devices ... yeah maybe this
> wouldn't be needed. As it stands, I still think this is the best way
> forward.

There is one -- suffix in the LED name.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Johannes Berg]

On Thu, 2016-05-19 at 09:16 +0200, Pavel Machek wrote:

> > In the original situation, without these patches, userspace has to
> > have a list of all LEDs that are supposed to indicate airplane
> > mode.
> Well, that's situation for many LEDs.

That doesn't make it a *good* situation though.

> > With this patch only (without patch 2/3), userspace can look up the
> > default trigger, but then has to change it, causing the necessary
> > information to be lost immediately when you actually use it - that
> > also seems like a bad idea.
> We should not store "what kind of led this is" in a trigger. 

That's pretty much what I'm arguing though.

> LED
> subsystem seems to use suffix of LED name to do that. So if we
> standartize, lets say "::rfkill" suffix for this, it should work and
> follow existing practice.
[...]
> There is one -- suffix in the LED name.

I don't really think that's a good way, and it doesn't seem to be used
universally, but I suppose it's good enough.

João, that means you should send a patch to add the ::rfkill suffix.

And Pavel should send a patch to document the practice and the existing
suffixes with their meaning ;-)

johannes

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Johannes Berg]

On Thu, 2016-05-19 at 09:16 +0200, Pavel Machek wrote:

> > In the original situation, without these patches, userspace has to
> > have a list of all LEDs that are supposed to indicate airplane
> > mode.
> Well, that's situation for many LEDs.

That doesn't make it a *good* situation though.

> > With this patch only (without patch 2/3), userspace can look up the
> > default trigger, but then has to change it, causing the necessary
> > information to be lost immediately when you actually use it - that
> > also seems like a bad idea.
> We should not store "what kind of led this is" in a trigger. 

That's pretty much what I'm arguing though.

> LED
> subsystem seems to use suffix of LED name to do that. So if we
> standartize, lets say "::rfkill" suffix for this, it should work and
> follow existing practice.
[...]
> There is one -- suffix in the LED name.

I don't really think that's a good way, and it doesn't seem to be used
universally, but I suppose it's good enough.

João, that means you should send a patch to add the ::rfkill suffix.

And Pavel should send a patch to document the practice and the existing
suffixes with their meaning ;-)

johannes

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[João Paulo Rechi Vita]

On 9 June 2016 at 08:43, Johannes Berg <johannes@sipsolutions.net> wrote:
> On Thu, 2016-05-19 at 09:16 +0200, Pavel Machek wrote:
>

(...)

>> LED
>> subsystem seems to use suffix of LED name to do that. So if we
>> standartize, lets say "::rfkill" suffix for this, it should work and
>> follow existing practice.
> [...]
>> There is one -- suffix in the LED name.
>
> I don't really think that's a good way, and it doesn't seem to be used
> universally, but I suppose it's good enough.
>

The main practical drawback of this approach IMO is that we can't
guarantee that userspace processes will not step on each other's toes
trying to control the LED concurrently. But I guess that is something
that userspace will have to solve for now, I rather get this moving
without the trigger than not moving at all.

> João, that means you should send a patch to add the ::rfkill suffix.
>

IMO "airplane" (or maybe "airplane-mode") is a better suffix, as it
reflects the label on the machine's chassis. I'll name it
"asus-wireless::airplane" and send this through platform-drivers-x86,
as this is now contained in the platform-drivers-x86 subsystem. Thanks
Johannes for your patience and help designing and reviewing the rfkill
changes, even if not all of them made it through in the end. And
thanks everyone else involved for the feedback.

Best regards,

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

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Pavel Machek]

Hi!

> > João, that means you should send a patch to add the ::rfkill suffix.
> >
> 
> IMO "airplane" (or maybe "airplane-mode") is a better suffix, as it
> reflects the label on the machine's chassis. I'll name it
> "asus-wireless::airplane" and send this through platform-drivers-x86,
> as this is now contained in the platform-drivers-x86 subsystem. Thanks
> Johannes for your patience and help designing and reviewing the rfkill
> changes, even if not all of them made it through in the end. And
> thanks everyone else involved for the feedback.

Actually, I'd do '::rfkill', for consistency with other places in
/sys.

/sys/devices/platform/thinkpad_acpi/rfkill/rfkill1/name
/sys/class/rfkill
/sys/module/rfkill

Thanks for patience,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[João Paulo Rechi Vita]

On 13 June 2016 at 15:00, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> > João, that means you should send a patch to add the ::rfkill suffix.
>> >
>>
>> IMO "airplane" (or maybe "airplane-mode") is a better suffix, as it
>> reflects the label on the machine's chassis. I'll name it
>> "asus-wireless::airplane" and send this through platform-drivers-x86,
>> as this is now contained in the platform-drivers-x86 subsystem. Thanks
>> Johannes for your patience and help designing and reviewing the rfkill
>> changes, even if not all of them made it through in the end. And
>> thanks everyone else involved for the feedback.
>
> Actually, I'd do '::rfkill', for consistency with other places in
> /sys.
>
> /sys/devices/platform/thinkpad_acpi/rfkill/rfkill1/name
> /sys/class/rfkill
> /sys/module/rfkill
>

If we use "rfkill" as a suffix, how do you expect userspace to be able
to differentiate between a LED that indicates airplane-mode (LED ON
when all radios are OFF) and a LED that indicates the state of a
specific radio like WiFi or Bluetooth (LED ON when that specific radio
is ON)? If we're going this route we should provide meaningful
information here.

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[João Paulo Rechi Vita]

On 13 June 2016 at 15:00, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> > João, that means you should send a patch to add the ::rfkill suffix.
>> >
>>
>> IMO "airplane" (or maybe "airplane-mode") is a better suffix, as it
>> reflects the label on the machine's chassis. I'll name it
>> "asus-wireless::airplane" and send this through platform-drivers-x86,
>> as this is now contained in the platform-drivers-x86 subsystem. Thanks
>> Johannes for your patience and help designing and reviewing the rfkill
>> changes, even if not all of them made it through in the end. And
>> thanks everyone else involved for the feedback.
>
> Actually, I'd do '::rfkill', for consistency with other places in
> /sys.
>
> /sys/devices/platform/thinkpad_acpi/rfkill/rfkill1/name
> /sys/class/rfkill
> /sys/module/rfkill
>

If we use "rfkill" as a suffix, how do you expect userspace to be able
to differentiate between a LED that indicates airplane-mode (LED ON
when all radios are OFF) and a LED that indicates the state of a
specific radio like WiFi or Bluetooth (LED ON when that specific radio
is ON)? If we're going this route we should provide meaningful
information here.

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

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Pavel Machek]

On Mon 2016-06-13 15:59:35, João Paulo Rechi Vita wrote:
> On 13 June 2016 at 15:00, Pavel Machek <pavel@ucw.cz> wrote:
> > Hi!
> >
> >> > João, that means you should send a patch to add the ::rfkill suffix.
> >> >
> >>
> >> IMO "airplane" (or maybe "airplane-mode") is a better suffix, as it
> >> reflects the label on the machine's chassis. I'll name it
> >> "asus-wireless::airplane" and send this through platform-drivers-x86,
> >> as this is now contained in the platform-drivers-x86 subsystem. Thanks
> >> Johannes for your patience and help designing and reviewing the rfkill
> >> changes, even if not all of them made it through in the end. And
> >> thanks everyone else involved for the feedback.
> >
> > Actually, I'd do '::rfkill', for consistency with other places in
> > /sys.
> >
> > /sys/devices/platform/thinkpad_acpi/rfkill/rfkill1/name
> > /sys/class/rfkill
> > /sys/module/rfkill
> >
> 
> If we use "rfkill" as a suffix, how do you expect userspace to be able
> to differentiate between a LED that indicates airplane-mode (LED ON
> when all radios are OFF) and a LED that indicates the state of a
> specific radio like WiFi or Bluetooth (LED ON when that specific radio
> is ON)? If we're going this route we should provide meaningful
> information here.

'::airplane' has same problem, no?

If you want to distinguish that, maybe you can do '::rfkill' for
everything vs '::rfkill-wifi' for wifi-only and '::rfkill-bt' for
bluetooth...

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[João Paulo Rechi Vita]

On 13 June 2016 at 17:01, Pavel Machek <pavel@ucw.cz> wrote:
> On Mon 2016-06-13 15:59:35, João Paulo Rechi Vita wrote:
>> On 13 June 2016 at 15:00, Pavel Machek <pavel@ucw.cz> wrote:
>> > Hi!
>> >
>> >> > João, that means you should send a patch to add the ::rfkill suffix.
>> >> >
>> >>
>> >> IMO "airplane" (or maybe "airplane-mode") is a better suffix, as it
>> >> reflects the label on the machine's chassis. I'll name it
>> >> "asus-wireless::airplane" and send this through platform-drivers-x86,
>> >> as this is now contained in the platform-drivers-x86 subsystem. Thanks
>> >> Johannes for your patience and help designing and reviewing the rfkill
>> >> changes, even if not all of them made it through in the end. And
>> >> thanks everyone else involved for the feedback.
>> >
>> > Actually, I'd do '::rfkill', for consistency with other places in
>> > /sys.
>> >
>> > /sys/devices/platform/thinkpad_acpi/rfkill/rfkill1/name
>> > /sys/class/rfkill
>> > /sys/module/rfkill
>> >
>>
>> If we use "rfkill" as a suffix, how do you expect userspace to be able
>> to differentiate between a LED that indicates airplane-mode (LED ON
>> when all radios are OFF) and a LED that indicates the state of a
>> specific radio like WiFi or Bluetooth (LED ON when that specific radio
>> is ON)? If we're going this route we should provide meaningful
>> information here.
>
> '::airplane' has same problem, no?
>

No, because in this case we would not use "airplane" as a suffix for a
LED associated with an individual radio.

> If you want to distinguish that, maybe you can do '::rfkill' for
> everything vs '::rfkill-wifi' for wifi-only and '::rfkill-bt' for
> bluetooth...
>

The problem here is that the "rfkill" name is already associated with
individual rfkill switches under /sys/class/rfkill,
/sys/devices/platform/*/rfkill etc, so I think we're better off
distinguishing "airplane" vs "wifi" vs "bluetooth" etc, to avoid
confusion.

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[João Paulo Rechi Vita]

On 13 June 2016 at 17:01, Pavel Machek <pavel@ucw.cz> wrote:
> On Mon 2016-06-13 15:59:35, João Paulo Rechi Vita wrote:
>> On 13 June 2016 at 15:00, Pavel Machek <pavel@ucw.cz> wrote:
>> > Hi!
>> >
>> >> > João, that means you should send a patch to add the ::rfkill suffix.
>> >> >
>> >>
>> >> IMO "airplane" (or maybe "airplane-mode") is a better suffix, as it
>> >> reflects the label on the machine's chassis. I'll name it
>> >> "asus-wireless::airplane" and send this through platform-drivers-x86,
>> >> as this is now contained in the platform-drivers-x86 subsystem. Thanks
>> >> Johannes for your patience and help designing and reviewing the rfkill
>> >> changes, even if not all of them made it through in the end. And
>> >> thanks everyone else involved for the feedback.
>> >
>> > Actually, I'd do '::rfkill', for consistency with other places in
>> > /sys.
>> >
>> > /sys/devices/platform/thinkpad_acpi/rfkill/rfkill1/name
>> > /sys/class/rfkill
>> > /sys/module/rfkill
>> >
>>
>> If we use "rfkill" as a suffix, how do you expect userspace to be able
>> to differentiate between a LED that indicates airplane-mode (LED ON
>> when all radios are OFF) and a LED that indicates the state of a
>> specific radio like WiFi or Bluetooth (LED ON when that specific radio
>> is ON)? If we're going this route we should provide meaningful
>> information here.
>
> '::airplane' has same problem, no?
>

No, because in this case we would not use "airplane" as a suffix for a
LED associated with an individual radio.

> If you want to distinguish that, maybe you can do '::rfkill' for
> everything vs '::rfkill-wifi' for wifi-only and '::rfkill-bt' for
> bluetooth...
>

The problem here is that the "rfkill" name is already associated with
individual rfkill switches under /sys/class/rfkill,
/sys/devices/platform/*/rfkill etc, so I think we're better off
distinguishing "airplane" vs "wifi" vs "bluetooth" etc, to avoid
confusion.

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

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Pavel Machek]

On Mon 2016-06-13 17:10:02, João Paulo Rechi Vita wrote:
> On 13 June 2016 at 17:01, Pavel Machek <pavel@ucw.cz> wrote:
> > On Mon 2016-06-13 15:59:35, João Paulo Rechi Vita wrote:
> >> On 13 June 2016 at 15:00, Pavel Machek <pavel@ucw.cz> wrote:
> >> > Hi!
> >> >
> >> >> > João, that means you should send a patch to add the ::rfkill suffix.
> >> >> >
> >> >>
> >> >> IMO "airplane" (or maybe "airplane-mode") is a better suffix, as it
> >> >> reflects the label on the machine's chassis. I'll name it
> >> >> "asus-wireless::airplane" and send this through platform-drivers-x86,
> >> >> as this is now contained in the platform-drivers-x86 subsystem. Thanks
> >> >> Johannes for your patience and help designing and reviewing the rfkill
> >> >> changes, even if not all of them made it through in the end. And
> >> >> thanks everyone else involved for the feedback.
> >> >
> >> > Actually, I'd do '::rfkill', for consistency with other places in
> >> > /sys.
> >> >
> >> > /sys/devices/platform/thinkpad_acpi/rfkill/rfkill1/name
> >> > /sys/class/rfkill
> >> > /sys/module/rfkill
> >> >
> >>
> >> If we use "rfkill" as a suffix, how do you expect userspace to be able
> >> to differentiate between a LED that indicates airplane-mode (LED ON
> >> when all radios are OFF) and a LED that indicates the state of a
> >> specific radio like WiFi or Bluetooth (LED ON when that specific radio
> >> is ON)? If we're going this route we should provide meaningful
> >> information here.
> >
> > '::airplane' has same problem, no?
> >
> 
> No, because in this case we would not use "airplane" as a suffix for a
> LED associated with an individual radio.
> 
> > If you want to distinguish that, maybe you can do '::rfkill' for
> > everything vs '::rfkill-wifi' for wifi-only and '::rfkill-bt' for
> > bluetooth...
> >
> 
> The problem here is that the "rfkill" name is already associated with
> individual rfkill switches under /sys/class/rfkill,
> /sys/devices/platform/*/rfkill etc, so I think we're better off
> distinguishing "airplane" vs "wifi" vs "bluetooth" etc, to avoid
> confusion.

(Actually, "::wifi" is super confusing, I'd expect that to be a led
that blinks when wifi is active.)

Well... "airplane" is quite confusing. I'd we use "rfkill" for
disabling radios, and I believe we should stick with that.

But small problem might be polarity. You may need both "::rfkill" and
"::not-rfkill".

Best regards,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Johannes Berg]

On Mon, 2016-06-13 at 23:21 +0200, Pavel Machek wrote:
> 
> (Actually, "::wifi" is super confusing, I'd expect that to be a led
> that blinks when wifi is active.)

Agree with that, yeah, that'd be confusing.

> Well... "airplane" is quite confusing. I'd we use "rfkill" for
> disabling radios, and I believe we should stick with that.
> 
> But small problem might be polarity. You may need both "::rfkill" and
> "::not-rfkill".

I'd argue that "airplane" matches better what you're likely to find on
the chassis of the system.

In either case I think the suffixes should be documented, for both
future kernel and application developers.

johannes

Re: [RESEND PATCH 1/3] rfkill: Create "rfkill-airplane-mode" LED trigger

[Johannes Berg]

On Mon, 2016-06-13 at 23:21 +0200, Pavel Machek wrote:
> 
> (Actually, "::wifi" is super confusing, I'd expect that to be a led
> that blinks when wifi is active.)

Agree with that, yeah, that'd be confusing.

> Well... "airplane" is quite confusing. I'd we use "rfkill" for
> disabling radios, and I believe we should stick with that.
> 
> But small problem might be polarity. You may need both "::rfkill" and
> "::not-rfkill".

I'd argue that "airplane" matches better what you're likely to find on
the chassis of the system.

In either case I think the suffixes should be documented, for both
future kernel and application developers.

johannes

[RESEND PATCH 2/3] rfkill: Userspace control for airplane mode

[João Paulo Rechi Vita]

From: João Paulo Rechi Vita <jprvita@gmail.com>

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

To change state of the indicator, the
RFKILL_OP_AIRPLANE_MODE_INDICATOR_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 |  6 ++++++
 net/rfkill/core.c           | 40 ++++++++++++++++++++++++++++++++++++++--
 3 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index b13025a..9dbe3fc 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_INDICATOR_ACQUIRE, and is only available for one
+userspace application at a time. Closing the fd 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_INDICATOR_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..36e0770 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -61,12 +61,18 @@ enum rfkill_type {
  * @RFKILL_OP_CHANGE_ALL: userspace changes all devices (of a type, or all)
  *	into a state, also updating the default state used for devices that
  *	are hot-plugged later.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE: userspace acquires control of
+ * 	the airplane-mode indicator.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: userspace changes the
+ * 	airplane-mode indicator state.
  */
 enum rfkill_operation {
 	RFKILL_OP_ADD = 0,
 	RFKILL_OP_DEL,
 	RFKILL_OP_CHANGE,
 	RFKILL_OP_CHANGE_ALL,
+	RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE,
+	RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE,
 };
 
 /**
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 9adf95e..95824b3 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
@@ -1174,9 +1176,23 @@ 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;
@@ -1216,6 +1232,25 @@ static ssize_t rfkill_fop_write(struct file *file, const char __user *buf,
 				rfkill_set_block(rfkill, ev.soft);
 		ret = 0;
 		break;
+	case RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE:
+		if (rfkill_apm_owned && !data->is_apm_owner) {
+			ret = -EACCES;
+			break;
+		}
+
+		rfkill_apm_owned = true;
+		data->is_apm_owner = true;
+		ret = 0;
+		break;
+	case RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE:
+		if (!rfkill_apm_owned || !data->is_apm_owner) {
+			ret = -EACCES;
+			break;
+		}
+
+		rfkill_apm_led_trigger_event(ev.soft);
+		ret = 0;
+		break;
 	default:
 		ret = -EINVAL;
 		break;
@@ -1232,6 +1267,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

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

[João Paulo Rechi Vita]

From: João Paulo Rechi Vita <jprvita@gmail.com>

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

diff --git a/Documentation/rfkill.txt b/Documentation/rfkill.txt
index 9dbe3fc..588b4bf 100644
--- a/Documentation/rfkill.txt
+++ b/Documentation/rfkill.txt
@@ -133,5 +133,8 @@ applications to take control. Changes to the airplane-mode indicator state can
 be made using RFKILL_OP_AIRPLANE_MODE_INDICATOR_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/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h
index 36e0770..2ccb02f 100644
--- a/include/uapi/linux/rfkill.h
+++ b/include/uapi/linux/rfkill.h
@@ -63,8 +63,8 @@ enum rfkill_type {
  *	are hot-plugged later.
  * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_ACQUIRE: userspace acquires control of
  * 	the airplane-mode indicator.
- * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: userspace changes the
- * 	airplane-mode indicator state.
+ * @RFKILL_OP_AIRPLANE_MODE_INDICATOR_CHANGE: the airplane-mode indicator state
+ * 	changed -- userspace changes the airplane-mode indicator state.
  */
 enum rfkill_operation {
 	RFKILL_OP_ADD = 0,
diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index 95824b3..c4bbd19 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_INDICATOR_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