Re: [PATCH] net/rfkill: Create "airplane mode" LED trigger

[Johannes Berg]

On Tue, 2016-01-05 at 22:55 -0800, Marcel Holtmann wrote:
> 
> so I am not convinced the kernel should have the concept of airplane
> mode at all.

[snip long story]

This is true, but that doesn't mean the patch is bad, just the naming
could be different.

I think the patch could name this "rfkill-all" (or so) instead, and
replace all the "airplane_mode" identifiers as well.

Then the driver can still default to "rfkill-all" trigger, or a
suitably interested userspace could remove the trigger and manage the
LED state itself.

Then again - if I think about that more - perhaps the kernel *should*
have a concept of airplane mode, just one that's not necessarily tied
to the "rfkill_all" setting, but could be controlled by userspace. That
way, userspace wouldn't have to know about the LED, just about the
airplane mode indicator (for which rfkill would probably be an
appropriate place)

Two comments on the patch itself:

> +#ifdef CONFIG_RFKILL_LEDS
> +   led_trigger_register(airplane_mode_led_trigger);
> +#endif

Everything else uses inlines to avoid ifdefs, you can do the same here.
Also, error handling seems necessary.

johannes

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

Re: [PATCH] net/rfkill: Create "airplane mode" LED trigger

[Marcel Holtmann]

Hi Joao,

> For platform drivers to be able to correctly drive the "Airplane Mode"
> indicative LED there needs to be a RFKill LED trigger tied to the global
> state of RFKILL_TYPE_ALL (instead of to a specific RFKill) and that
> works in an inverted manner of regular RFKill LED triggers, that is, the
> LED is ON when the state is blocked, and OFF otherwise.
> 
> This commit implements such a trigger, which will be used by the
> asus-wireless x86 platform driver.
> 
> Signed-off-by: João Paulo Rechi Vita 
> ---
> net/rfkill/core.c | 30 ++
> 1 file changed, 30 insertions(+)
> 
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index b41e9ea..3effc29 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -124,6 +124,26 @@ static bool rfkill_epo_lock_active;
> 
> 
> #ifdef CONFIG_RFKILL_LEDS
> +static void airplane_mode_led_trigger_activate(struct led_classdev *led);
> +
> +static struct led_trigger airplane_mode_led_trigger = {
> + .name = "rfkill-airplane-mode",
> + .activate = airplane_mode_led_trigger_activate,
> +};

so I am not convinced the kernel should have the concept of airplane mode at 
all. It is kinda of a term that keeps changing since airlines do now allow WiFi 
and Bluetooth short range transmissions during flight. We stayed away from 
calling it airplane mode since by the nature of it being governed by local 
regulations it will change over time.

The RFKILL subsystem got away with not labeling it by just saying 
RFKILL_CHANGE_ALL and if we want a trigger for that action we better find a 
more general term to describe the fact that all RF devices are shut off.

Keep in mind that even with airplane mode on, you can re-activate Bluetooth and 
WiFi these days. So while you are in airplane mode, then RFKILL switches for 
these two technologies can be taken back off. If we wanted to model that in the 
kernel we would be putting policy in the kernel and I think that is a bad idea.

That is pretty much the main reason why ConnMan never tried to push the 
information about flight mode back into the kernel. It is not a policy that the 
kernel can determine in the first place.

Regards

Marcel

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

Re: [PATCH] net/rfkill: Create "airplane mode" LED trigger

[João Paulo Rechi Vita]

Hello Johannes,

On 26 December 2015 at 10:05, João Paulo Rechi Vita  wrote:
> For platform drivers to be able to correctly drive the "Airplane Mode"
> indicative LED there needs to be a RFKill LED trigger tied to the global
> state of RFKILL_TYPE_ALL (instead of to a specific RFKill) and that
> works in an inverted manner of regular RFKill LED triggers, that is, the
> LED is ON when the state is blocked, and OFF otherwise.
>
> This commit implements such a trigger, which will be used by the
> asus-wireless x86 platform driver.
>

The initial patches of the asus-wireless driver have been queue on the
platform-drivers-x86 tree. Do you have any comments on this one? It
would be great to have it in for 4.5, since the airplane mode LED
management in asus-wireless' pending patches depend on this one.

> Signed-off-by: João Paulo Rechi Vita 
> ---
>  net/rfkill/core.c | 30 ++
>  1 file changed, 30 insertions(+)
>
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index b41e9ea..3effc29 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -124,6 +124,26 @@ static bool rfkill_epo_lock_active;
>
>
>  #ifdef CONFIG_RFKILL_LEDS
> +static void airplane_mode_led_trigger_activate(struct led_classdev *led);
> +
> +static struct led_trigger airplane_mode_led_trigger = {
> +   .name = "rfkill-airplane-mode",
> +   .activate = airplane_mode_led_trigger_activate,
> +};
> +
> +static void airplane_mode_led_trigger_event(void)
> +{
> +   if (rfkill_global_states[RFKILL_TYPE_ALL].cur & RFKILL_BLOCK_ANY)
> +   led_trigger_event(_mode_led_trigger, LED_FULL);
> +   else
> +   led_trigger_event(_mode_led_trigger, LED_OFF);
> +}
> +
> +static void airplane_mode_led_trigger_activate(struct led_classdev *led)
> +{
> +   airplane_mode_led_trigger_event();
> +}
> +
>  static void rfkill_led_trigger_event(struct rfkill *rfkill)
>  {
> struct led_trigger *trigger;
> @@ -175,6 +195,10 @@ static void rfkill_led_trigger_unregister(struct rfkill 
> *rfkill)
> led_trigger_unregister(>led_trigger);
>  }
>  #else
> +static void airplane_mode_led_trigger_event(void)
> +{
> +}
> +
>  static void rfkill_led_trigger_event(struct rfkill *rfkill)
>  {
>  }
> @@ -346,6 +370,7 @@ static void __rfkill_switch_all(const enum rfkill_type 
> type, bool blocked)
>
> for (i = 0; i < NUM_RFKILL_TYPES; i++)
> rfkill_global_states[i].cur = blocked;
> +   airplane_mode_led_trigger_event();
> } else {
> rfkill_global_states[type].cur = blocked;
> }
> @@ -1177,6 +1202,7 @@ static ssize_t rfkill_fop_write(struct file *file, 
> const char __user *buf,
> enum rfkill_type i;
> for (i = 0; i < NUM_RFKILL_TYPES; i++)
> rfkill_global_states[i].cur = ev.soft;
> +   airplane_mode_led_trigger_event();
> } else {
> rfkill_global_states[ev.type].cur = ev.soft;
> }
> @@ -1293,6 +1319,10 @@ static int __init rfkill_init(void)
> }
>  #endif
>
> +#ifdef CONFIG_RFKILL_LEDS
> +   led_trigger_register(_mode_led_trigger);
> +#endif
> +
>   out:
> return error;
>  }
> --
> 2.5.0
>

Thanks and best regards,

--
João Paulo Rechi Vita
http://about.me/jprvita
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] net/rfkill: Create "airplane mode" LED trigger

[João Paulo Rechi Vita]

For platform drivers to be able to correctly drive the "Airplane Mode"
indicative LED there needs to be a RFKill LED trigger tied to the global
state of RFKILL_TYPE_ALL (instead of to a specific RFKill) and that
works in an inverted manner of regular RFKill LED triggers, that is, the
LED is ON when the state is blocked, and OFF otherwise.

This commit implements such a trigger, which will be used by the
asus-wireless x86 platform driver.

Signed-off-by: João Paulo Rechi Vita 
---
 net/rfkill/core.c | 30 ++
 1 file changed, 30 insertions(+)

diff --git a/net/rfkill/core.c b/net/rfkill/core.c
index b41e9ea..3effc29 100644
--- a/net/rfkill/core.c
+++ b/net/rfkill/core.c
@@ -124,6 +124,26 @@ static bool rfkill_epo_lock_active;
 
 
 #ifdef CONFIG_RFKILL_LEDS
+static void airplane_mode_led_trigger_activate(struct led_classdev *led);
+
+static struct led_trigger airplane_mode_led_trigger = {
+   .name = "rfkill-airplane-mode",
+   .activate = airplane_mode_led_trigger_activate,
+};
+
+static void airplane_mode_led_trigger_event(void)
+{
+   if (rfkill_global_states[RFKILL_TYPE_ALL].cur & RFKILL_BLOCK_ANY)
+   led_trigger_event(_mode_led_trigger, LED_FULL);
+   else
+   led_trigger_event(_mode_led_trigger, LED_OFF);
+}
+
+static void airplane_mode_led_trigger_activate(struct led_classdev *led)
+{
+   airplane_mode_led_trigger_event();
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
struct led_trigger *trigger;
@@ -175,6 +195,10 @@ static void rfkill_led_trigger_unregister(struct rfkill 
*rfkill)
led_trigger_unregister(>led_trigger);
 }
 #else
+static void airplane_mode_led_trigger_event(void)
+{
+}
+
 static void rfkill_led_trigger_event(struct rfkill *rfkill)
 {
 }
@@ -346,6 +370,7 @@ static void __rfkill_switch_all(const enum rfkill_type 
type, bool blocked)
 
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = blocked;
+   airplane_mode_led_trigger_event();
} else {
rfkill_global_states[type].cur = blocked;
}
@@ -1177,6 +1202,7 @@ static ssize_t rfkill_fop_write(struct file *file, const 
char __user *buf,
enum rfkill_type i;
for (i = 0; i < NUM_RFKILL_TYPES; i++)
rfkill_global_states[i].cur = ev.soft;
+   airplane_mode_led_trigger_event();
} else {
rfkill_global_states[ev.type].cur = ev.soft;
}
@@ -1293,6 +1319,10 @@ static int __init rfkill_init(void)
}
 #endif
 
+#ifdef CONFIG_RFKILL_LEDS
+   led_trigger_register(_mode_led_trigger);
+#endif
+
  out:
return error;
 }
-- 
2.5.0

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