[PATCH v3] rfkill: Add rfkill-any LED trigger

[Mike Krinkin]

On Wed, Dec 21, 2016 at 09:45:33AM +0100, MichaÅ KÄpieÅ wrote:
> Add a new "global" (i.e. not per-rfkill device) LED trigger, rfkill-any,
> which may be useful on laptops with a single "radio LED" and multiple
> radio transmitters.  The trigger is meant to turn a LED on whenever
> there is at least one radio transmitter active and turn it off
> otherwise.
> 
> This requires taking rfkill_global_mutex before calling
> rfkill_set_block() in rfkill_resume(): since
> rfkill_any_led_trigger_event(true) is called from rfkill_set_block()
> unconditionally, each caller of the latter needs to take care of locking
> rfkill_global_mutex.
> 
> Signed-off-by: MichaÅ KÄpieÅ <kernel@xxxxxxxxxx>
> ---
> Jonathan, I refrained from resending patch 1/2 from v2 as part of this
> series as it is currently applied in mac80211-next/master along with
> Arnd's fix.  Please let me know if you would like me to handle this
> differently.
> 
> Mike, could you please test whether this version works fine on your
> machine?  Thanks!

Sorry for the delay, patch works fine for me.

> 
> Changes from v2:
> 
>   - Handle the global mutex properly when rfkill_set_{hw,sw}_state() or
>     rfkill_set_states() is called from within an rfkill callback.  v2
>     always tried to lock the global mutex in such a case, which led to a
>     deadlock when an rfkill driver called one of the above functions
>     from its query or set_block callback.  This is solved by defining a
>     new bitfield, RFKILL_BLOCK_SW_HASLOCK, which is set before the above
>     callbacks are invoked and cleared afterwards; the functions listed
>     above use this bitfield to tell rfkill_any_led_trigger_event()
>     whether the global mutex is currently held or not.
>     RFKILL_BLOCK_SW_SETCALL cannot be reused for this purpose as setting
>     it before invoking the query callback would cause any calls to
>     rfkill_set_sw_state() made from within that callback to work on
>     RFKILL_BLOCK_SW_PREV instead of RFKILL_BLOCK_SW and thus change the
>     way rfkill_set_block() behaves.
> 
>   - As rfkill_any_led_trigger_event() now takes a boolean argument which
>     tells it whether the global mutex was already taken by the caller,
>     all calls to __rfkill_any_led_trigger_event() outside
>     rfkill_any_led_trigger_event() have been replaced with calls to
>     rfkill_any_led_trigger_event(true).
> 
>  net/rfkill/core.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 86 insertions(+), 4 deletions(-)
> 
> diff --git a/net/rfkill/core.c b/net/rfkill/core.c
> index afa4f71b4c7b..688eac7b97ef 100644
> --- a/net/rfkill/core.c
> +++ b/net/rfkill/core.c
> @@ -44,6 +44,7 @@
>  #define RFKILL_BLOCK_ANY	(RFKILL_BLOCK_HW |\
>  				 RFKILL_BLOCK_SW |\
>  				 RFKILL_BLOCK_SW_PREV)
> +#define RFKILL_BLOCK_SW_HASLOCK	BIT(30)
>  #define RFKILL_BLOCK_SW_SETCALL	BIT(31)
>  
>  struct rfkill {
> @@ -176,6 +177,51 @@ static void rfkill_led_trigger_unregister(struct rfkill *rfkill)
>  {
>  	led_trigger_unregister(&rfkill->led_trigger);
>  }
> +
> +static struct led_trigger rfkill_any_led_trigger;
> +
> +static void __rfkill_any_led_trigger_event(void)
> +{
> +	enum led_brightness brightness = LED_OFF;
> +	struct rfkill *rfkill;
> +
> +	list_for_each_entry(rfkill, &rfkill_list, node) {
> +		if (!(rfkill->state & RFKILL_BLOCK_ANY)) {
> +			brightness = LED_FULL;
> +			break;
> +		}
> +	}
> +
> +	led_trigger_event(&rfkill_any_led_trigger, brightness);
> +}
> +
> +static void rfkill_any_led_trigger_event(bool global_locked)
> +{
> +	if (global_locked) {
> +		__rfkill_any_led_trigger_event();
> +	} else {
> +		mutex_lock(&rfkill_global_mutex);
> +		__rfkill_any_led_trigger_event();
> +		mutex_unlock(&rfkill_global_mutex);
> +	}
> +}
> +
> +static void rfkill_any_led_trigger_activate(struct led_classdev *led_cdev)
> +{
> +	rfkill_any_led_trigger_event(false);
> +}
> +
> +static int rfkill_any_led_trigger_register(void)
> +{
> +	rfkill_any_led_trigger.name = "rfkill-any";
> +	rfkill_any_led_trigger.activate = rfkill_any_led_trigger_activate;
> +	return led_trigger_register(&rfkill_any_led_trigger);
> +}
> +
> +static void rfkill_any_led_trigger_unregister(void)
> +{
> +	led_trigger_unregister(&rfkill_any_led_trigger);
> +}
>  #else
>  static void rfkill_led_trigger_event(struct rfkill *rfkill)
>  {
> @@ -189,6 +235,19 @@ static inline int rfkill_led_trigger_register(struct rfkill *rfkill)
>  static inline void rfkill_led_trigger_unregister(struct rfkill *rfkill)
>  {
>  }
> +
> +static void rfkill_any_led_trigger_event(bool global_locked)
> +{
> +}
> +
> +static int rfkill_any_led_trigger_register(void)
> +{
> +	return 0;
> +}
> +
> +static void rfkill_any_led_trigger_unregister(void)
> +{
> +}
>  #endif /* CONFIG_RFKILL_LEDS */
>  
>  static void rfkill_fill_event(struct rfkill_event *ev, struct rfkill *rfkill,
> @@ -253,6 +312,10 @@ static void rfkill_set_block(struct rfkill *rfkill, bool blocked)
>  	if (unlikely(rfkill->dev.power.power_state.event & PM_EVENT_SLEEP))
>  		return;
>  
> +	spin_lock_irqsave(&rfkill->lock, flags);
> +	rfkill->state |= RFKILL_BLOCK_SW_HASLOCK;
> +	spin_unlock_irqrestore(&rfkill->lock, flags);
> +
>  	/*
>  	 * Some platforms (...!) generate input events which affect the
>  	 * _hard_ kill state -- whenever something tries to change the
> @@ -292,11 +355,13 @@ static void rfkill_set_block(struct rfkill *rfkill, bool blocked)
>  			rfkill->state &= ~RFKILL_BLOCK_SW;
>  	}
>  	rfkill->state &= ~RFKILL_BLOCK_SW_SETCALL;
> +	rfkill->state &= ~RFKILL_BLOCK_SW_HASLOCK;
>  	rfkill->state &= ~RFKILL_BLOCK_SW_PREV;
>  	curr = rfkill->state & RFKILL_BLOCK_SW;
>  	spin_unlock_irqrestore(&rfkill->lock, flags);
>  
>  	rfkill_led_trigger_event(rfkill);
> +	rfkill_any_led_trigger_event(true);
>  
>  	if (prev != curr)
>  		rfkill_event(rfkill);
> @@ -463,7 +528,7 @@ bool rfkill_get_global_sw_state(const enum rfkill_type type)
>  bool rfkill_set_hw_state(struct rfkill *rfkill, bool blocked)
>  {
>  	unsigned long flags;
> -	bool ret, prev;
> +	bool ret, prev, global_locked;
>  
>  	BUG_ON(!rfkill);
>  
> @@ -474,9 +539,11 @@ bool rfkill_set_hw_state(struct rfkill *rfkill, bool blocked)
>  	else
>  		rfkill->state &= ~RFKILL_BLOCK_HW;
>  	ret = !!(rfkill->state & RFKILL_BLOCK_ANY);
> +	global_locked = !!(rfkill->state & RFKILL_BLOCK_SW_HASLOCK);
>  	spin_unlock_irqrestore(&rfkill->lock, flags);
>  
>  	rfkill_led_trigger_event(rfkill);
> +	rfkill_any_led_trigger_event(global_locked);
>  
>  	if (rfkill->registered && prev != blocked)
>  		schedule_work(&rfkill->uevent_work);
> @@ -502,7 +569,7 @@ static void __rfkill_set_sw_state(struct rfkill *rfkill, bool blocked)
>  bool rfkill_set_sw_state(struct rfkill *rfkill, bool blocked)
>  {
>  	unsigned long flags;
> -	bool prev, hwblock;
> +	bool prev, hwblock, global_locked;
>  
>  	BUG_ON(!rfkill);
>  
> @@ -511,6 +578,7 @@ bool rfkill_set_sw_state(struct rfkill *rfkill, bool blocked)
>  	__rfkill_set_sw_state(rfkill, blocked);
>  	hwblock = !!(rfkill->state & RFKILL_BLOCK_HW);
>  	blocked = blocked || hwblock;
> +	global_locked = !!(rfkill->state & RFKILL_BLOCK_SW_HASLOCK);
>  	spin_unlock_irqrestore(&rfkill->lock, flags);
>  
>  	if (!rfkill->registered)
> @@ -520,6 +588,7 @@ bool rfkill_set_sw_state(struct rfkill *rfkill, bool blocked)
>  		schedule_work(&rfkill->uevent_work);
>  
>  	rfkill_led_trigger_event(rfkill);
> +	rfkill_any_led_trigger_event(global_locked);
>  
>  	return blocked;
>  }
> @@ -542,7 +611,7 @@ EXPORT_SYMBOL(rfkill_init_sw_state);
>  void rfkill_set_states(struct rfkill *rfkill, bool sw, bool hw)
>  {
>  	unsigned long flags;
> -	bool swprev, hwprev;
> +	bool swprev, hwprev, global_locked;
>  
>  	BUG_ON(!rfkill);
>  
> @@ -559,6 +628,7 @@ void rfkill_set_states(struct rfkill *rfkill, bool sw, bool hw)
>  		rfkill->state |= RFKILL_BLOCK_HW;
>  	else
>  		rfkill->state &= ~RFKILL_BLOCK_HW;
> +	global_locked = !!(rfkill->state & RFKILL_BLOCK_SW_HASLOCK);
>  
>  	spin_unlock_irqrestore(&rfkill->lock, flags);
>  
> @@ -569,6 +639,7 @@ void rfkill_set_states(struct rfkill *rfkill, bool sw, bool hw)
>  			schedule_work(&rfkill->uevent_work);
>  
>  		rfkill_led_trigger_event(rfkill);
> +		rfkill_any_led_trigger_event(global_locked);
>  	}
>  }
>  EXPORT_SYMBOL(rfkill_set_states);
> @@ -812,8 +883,10 @@ static int rfkill_resume(struct device *dev)
>  	rfkill->suspended = false;
>  
>  	if (!rfkill->persistent) {
> +		mutex_lock(&rfkill_global_mutex);
>  		cur = !!(rfkill->state & RFKILL_BLOCK_SW);
>  		rfkill_set_block(rfkill, cur);
> +		mutex_unlock(&rfkill_global_mutex);
>  	}
>  
>  	if (rfkill->ops->poll && !rfkill->polling_paused)
> @@ -985,6 +1058,7 @@ int __must_check rfkill_register(struct rfkill *rfkill)
>  #endif
>  	}
>  
> +	rfkill_any_led_trigger_event(true);
>  	rfkill_send_events(rfkill, RFKILL_OP_ADD);
>  
>  	mutex_unlock(&rfkill_global_mutex);
> @@ -1017,6 +1091,7 @@ void rfkill_unregister(struct rfkill *rfkill)
>  	mutex_lock(&rfkill_global_mutex);
>  	rfkill_send_events(rfkill, RFKILL_OP_DEL);
>  	list_del_init(&rfkill->node);
> +	rfkill_any_led_trigger_event(true);
>  	mutex_unlock(&rfkill_global_mutex);
>  
>  	rfkill_led_trigger_unregister(rfkill);
> @@ -1269,6 +1344,10 @@ static int __init rfkill_init(void)
>  	if (error)
>  		goto error_misc;
>  
> +	error = rfkill_any_led_trigger_register();
> +	if (error)
> +		goto error_led_trigger;
> +
>  #ifdef CONFIG_RFKILL_INPUT
>  	error = rfkill_handler_init();
>  	if (error)
> @@ -1279,8 +1358,10 @@ static int __init rfkill_init(void)
>  
>  #ifdef CONFIG_RFKILL_INPUT
>  error_input:
> -	misc_deregister(&rfkill_miscdev);
> +	rfkill_any_led_trigger_unregister();
>  #endif
> +error_led_trigger:
> +	misc_deregister(&rfkill_miscdev);
>  error_misc:
>  	class_unregister(&rfkill_class);
>  error_class:
> @@ -1293,6 +1374,7 @@ static void __exit rfkill_exit(void)
>  #ifdef CONFIG_RFKILL_INPUT
>  	rfkill_handler_exit();
>  #endif
> +	rfkill_any_led_trigger_unregister();
>  	misc_deregister(&rfkill_miscdev);
>  	class_unregister(&rfkill_class);
>  }
> -- 
> 2.11.0
>