Re: [patch V2 43/67] leds/trigger/cpu: Convert to hotplug state machine

[Jacek Anaszewski]

Hi Anna,

On 07/13/2016 07:16 PM, Anna-Maria Gleixner wrote:

> -------- Original Message --------
> Subject: [patch V2 43/67] leds/trigger/cpu: Convert to hotplug state 
machine
> Date: Wed, 13 Jul 2016 17:16:45 +0000
> From: Anna-Maria Gleixner <anna-maria@xxxxxxxxxxxxx>
> To: LKML <linux-kernel@xxxxxxxxxxxxxxx>
> CC: Peter Zijlstra <peterz@xxxxxxxxxxxxx>, Ingo Molnar 
<mingo@xxxxxxxxxx>, Thomas Gleixner <tglx@xxxxxxxxxxxxx>, 
rt@xxxxxxxxxxxxx, Richard Cochran <rcochran@xxxxxxxxxxxxx>, Sebastian 
Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>, Jacek Anaszewski 
<j.anaszewski@xxxxxxxxxxx>, Linus Torvalds 
<torvalds@xxxxxxxxxxxxxxxxxxxx>, Linus Walleij 
<linus.walleij@xxxxxxxxxx>, Paul Gortmaker 
<paul.gortmaker@xxxxxxxxxxxxx>, Richard Purdie <rpurdie@xxxxxxxxx>, 
linux-leds@xxxxxxxxxxxxxxx, Anna-Maria Gleixner <anna-maria@xxxxxxxxxxxxx>
>
> From: Richard Cochran <rcochran@xxxxxxxxxxxxx>
>
> This is a straightforward conversion. We place this callback last
> in the list so that the LED illuminates only after a successful
> bring up sequence.
>
> Signed-off-by: Richard Cochran <rcochran@xxxxxxxxxxxxx>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> Cc: Jacek Anaszewski <j.anaszewski@xxxxxxxxxxx>
> Cc: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx>
> Cc: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>
> Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> Cc: Richard Purdie <rpurdie@xxxxxxxxx>
> Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> Cc: linux-leds@xxxxxxxxxxxxxxx
> Signed-off-by: Anna-Maria Gleixner <anna-maria@xxxxxxxxxxxxx>
> ---
>  drivers/leds/trigger/ledtrig-cpu.c | 32 +++++++++++++++-----------------
>  include/linux/cpuhotplug.h         |  1 +
>  2 files changed, 16 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/leds/trigger/ledtrig-cpu.c 
b/drivers/leds/trigger/ledtrig-cpu.c
> index 938467f..4a6a182 100644
> --- a/drivers/leds/trigger/ledtrig-cpu.c
> +++ b/drivers/leds/trigger/ledtrig-cpu.c
> @@ -92,25 +92,17 @@ static struct syscore_ops ledtrig_cpu_syscore_ops = {
>       .resume         = ledtrig_cpu_syscore_resume,
>  };
>
> -static int ledtrig_cpu_notify(struct notifier_block *self,
> -                                        unsigned long action, void 
*hcpu)
> +static int ledtrig_starting_cpu(unsigned int cpu)
>  {
> -     switch (action & ~CPU_TASKS_FROZEN) {
> -     case CPU_STARTING:
> -             ledtrig_cpu(CPU_LED_START);
> -             break;
> -             break;
> -     }
> -
> -     return NOTIFY_OK;
> +     ledtrig_cpu(CPU_LED_START);
> +     return 0;
>  }
>
> -
> -static struct notifier_block ledtrig_cpu_nb = {
> -     .notifier_call = ledtrig_cpu_notify,
> -};
> +static int ledtrig_dying_cpu(unsigned int cpu)
> +{
> +     ledtrig_cpu(CPU_LED_STOP);
> +     return 0;
> +}
>
>  static int __init ledtrig_cpu_init(void)
>  {
> @@ -133,7 +125,13 @@ static int __init ledtrig_cpu_init(void)
>       }
>
>       register_syscore_ops(&ledtrig_cpu_syscore_ops);
> -     register_cpu_notifier(&ledtrig_cpu_nb);
> +
> +     /*
> +      * FIXME: Why needs this to happen in the interrupt disabled
> +      * low level bringup phase of a cpu?
> +      */

Why wasn't it possible to clarify the issue before the
submission?

> +     cpuhp_setup_state(CPUHP_AP_LEDTRIG_STARTING, "AP_LEDTRIG_STARTING",
> +                       ledtrig_starting_cpu, ledtrig_dying_cpu);
>
>       pr_info("ledtrig-cpu: registered to indicate activity on CPUs\n");
>
> diff --git a/include/linux/cpuhotplug.h b/include/linux/cpuhotplug.h
> index c052b04..ef7bfa6 100644
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -43,6 +43,7 @@ enum cpuhp_state {
>       CPUHP_AP_QCOM_TIMER_STARTING,
>       CPUHP_AP_MIPS_GIC_TIMER_STARTING,
>       CPUHP_AP_KVM_STARTING,
> +     CPUHP_AP_LEDTRIG_STARTING,
>       CPUHP_AP_NOTIFY_STARTING,
>       CPUHP_AP_ONLINE,
>       CPUHP_TEARDOWN_CPU,
> --


--
Best regards,
Jacek Anaszewski