Re: [PATCH] leds/trigger: system can't enter suspend.

[Jacek Anaszewski <jacek.anaszewski@gmail.com>]

Hi Zhang,

Thanks for the patch.

I'd rather keep sending uevent on setting trigger to "none".

We could get rid of the problem you're trying to fix,
by modifying heartbeat_pm_notifier() so that it didn't go
through the whole procedure of unregistering/registering
the trigger on suspend/resume.

Instead of that we would need to add suspend/resume
ops to the struct led_trigger, that could be implemented
by triggers similarly to activate/deactivate.

We'd need also the led_trigger_suspend{resume}() API
that would iterate through all LED class devices registered
on given trigger and call their suspend/resume ops.

In this specifc case of ledtrig-heartbeat, that suspend
op would need only to call

del_timer_sync(&led_cdev->trigger_data->timer);

and resume would need to call setup_timer() accordingly.

led_trigger_suspend{resume} would replace
led_trigger_unregister{register} in heartbeat_pm_notifier().

I hope this rough design makes sense :-)

Best regards,
Jacek Anaszewski


On 05/07/2017 04:01 AM, sanshan zhang wrote:
> From: zhang sanshan <sanshan.zhang@nxp.com>
> 
> system can't enter suspend when enable led heartbeat.
> 
> pm will call heartbeat_pm_notifier when suspend.
> system will prepare led states, and led_trigger_unregister
> will use led_trigger_set to set trigger.
> kobject_uevent_env will send event,it will call ep_poll_callback
> to hold a wakeup source if we enable wakelock in kernel.
> So that system can't enter suspend.
> 
> If we set led device tigger to NONE, there is not need to
> send notification to user space.
> 
> Change-Id: I9f7ee5764d7e31b9a225dae5517cb36137675f9d
> Signed-off-by: zhang sanshan <sanshan.zhang@nxp.com>
> Signed-off-by: Zhang Bo <bo.zhang@nxp.com>
> ---
>  drivers/leds/led-triggers.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/leds/led-triggers.c b/drivers/leds/led-triggers.c
> index 431123b..f03c2bd1 100644
> --- a/drivers/leds/led-triggers.c
> +++ b/drivers/leds/led-triggers.c
> @@ -108,13 +108,10 @@ void led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
>  	unsigned long flags;
>  	char *event = NULL;
>  	char *envp[2];
> -	const char *name;
>  
>  	if (!led_cdev->trigger && !trig)
>  		return;
>  
> -	name = trig ? trig->name : "none";
> -	event = kasprintf(GFP_KERNEL, "TRIGGER=%s", name);
>  
>  	/* Remove any existing trigger */
>  	if (led_cdev->trigger) {
> @@ -138,6 +135,10 @@ void led_trigger_set(struct led_classdev *led_cdev, struct led_trigger *trig)
>  			trig->activate(led_cdev);
>  	}
>  
> +	if (NULL != trig)
> +		event = kasprintf(GFP_KERNEL, "TRIGGER=%s", trig->name);
> +	else
> +		return;
>  	if (event) {
>  		envp[0] = event;
>  		envp[1] = NULL;
>