Re: [patch 55/66] hwtracing: coresight-etm4x: Convert to hotplug state machine

[Mathieu Poirier]

On 11 July 2016 at 06:29, Anna-Maria Gleixner <anna-maria@xxxxxxxxxxxxx> wrote:
> From: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
>
> This driver has an asymmetry of ONLINE code without any corresponding tear
> down code. Otherwise, this is a straightforward conversion.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> Cc: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Signed-off-by: Anna-Maria Gleixner <anna-maria@xxxxxxxxxxxxx>
> ---
>  drivers/hwtracing/coresight/coresight-etm4x.c |   93 ++++++++++++++------------
>  include/linux/cpuhotplug.h                    |    1
>  2 files changed, 53 insertions(+), 41 deletions(-)
>
> --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> @@ -673,53 +673,52 @@ void etm4_config_trace_mode(struct etmv4
>         config->addr_acc[ETM_DEFAULT_ADDR_COMP + 1] = addr_acc;
>  }
>
> -static int etm4_cpu_callback(struct notifier_block *nfb, unsigned long action,
> -                           void *hcpu)
> +static int etm4_online_cpu(unsigned int cpu)
>  {
> -       unsigned int cpu = (unsigned long)hcpu;
> +       if (!etmdrvdata[cpu])
> +               return 0;
> +
> +       if (etmdrvdata[cpu]->boot_enable && !etmdrvdata[cpu]->sticky_enable)
> +               coresight_enable(etmdrvdata[cpu]->csdev);
> +       return 0;
> +}
> +
> +static int etm4_starting_cpu(unsigned int cpu)
> +{
> +       if (!etmdrvdata[cpu])
> +               return 0;
> +
> +       spin_lock(&etmdrvdata[cpu]->spinlock);
> +       if (!etmdrvdata[cpu]->os_unlock) {
> +               etm4_os_unlock(etmdrvdata[cpu]);
> +               etmdrvdata[cpu]->os_unlock = true;
> +       }
>
> +       if (local_read(&etmdrvdata[cpu]->mode))
> +               etm4_enable_hw(etmdrvdata[cpu]);
> +       spin_unlock(&etmdrvdata[cpu]->spinlock);
> +       return 0;
> +}
> +
> +static int etm4_dying_cpu(unsigned int cpu)
> +{
>         if (!etmdrvdata[cpu])
> -               goto out;
> +               return 0;
>
> -       switch (action & (~CPU_TASKS_FROZEN)) {
> -       case CPU_STARTING:
> -               spin_lock(&etmdrvdata[cpu]->spinlock);
> -               if (!etmdrvdata[cpu]->os_unlock) {
> -                       etm4_os_unlock(etmdrvdata[cpu]);
> -                       etmdrvdata[cpu]->os_unlock = true;
> -               }
> -
> -               if (local_read(&etmdrvdata[cpu]->mode))
> -                       etm4_enable_hw(etmdrvdata[cpu]);
> -               spin_unlock(&etmdrvdata[cpu]->spinlock);
> -               break;
> -
> -       case CPU_ONLINE:
> -               if (etmdrvdata[cpu]->boot_enable &&
> -                       !etmdrvdata[cpu]->sticky_enable)
> -                       coresight_enable(etmdrvdata[cpu]->csdev);
> -               break;
> -
> -       case CPU_DYING:
> -               spin_lock(&etmdrvdata[cpu]->spinlock);
> -               if (local_read(&etmdrvdata[cpu]->mode))
> -                       etm4_disable_hw(etmdrvdata[cpu]);
> -               spin_unlock(&etmdrvdata[cpu]->spinlock);
> -               break;
> -       }
> -out:
> -       return NOTIFY_OK;
> -}
> -
> -static struct notifier_block etm4_cpu_notifier = {
> -       .notifier_call = etm4_cpu_callback,
> -};
> +       spin_lock(&etmdrvdata[cpu]->spinlock);
> +       if (local_read(&etmdrvdata[cpu]->mode))
> +               etm4_disable_hw(etmdrvdata[cpu]);
> +       spin_unlock(&etmdrvdata[cpu]->spinlock);
> +       return 0;
> +}
>
>  static void etm4_init_trace_id(struct etmv4_drvdata *drvdata)
>  {
>         drvdata->trcid = coresight_get_trace_id(drvdata->cpu);
>  }
>
> +static enum cpuhp_state hp_online;
> +

Same comment as with ETMv3 - I'd like to see this at the top of the
file with the rest of the global declaration.

>  static int etm4_probe(struct amba_device *adev, const struct amba_id *id)
>  {
>         int ret;
> @@ -767,8 +766,17 @@ static int etm4_probe(struct amba_device
>                                 etm4_init_arch_data,  drvdata, 1))
>                 dev_err(dev, "ETM arch init failed\n");
>
> -       if (!etm4_count++)
> -               register_hotcpu_notifier(&etm4_cpu_notifier);
> +       if (!etm4_count++) {
> +               cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT4_STARTING,
> +                                         "AP_ARM_CORESIGHT4_STARTING",
> +                                         etm4_starting_cpu, etm4_dying_cpu);
> +               ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN,
> +                                               "AP_ARM_CORESIGHT4_ONLINE",
> +                                               etm4_online_cpu, NULL);
> +               if (ret < 0)
> +                       goto err_arch_supported;
> +               hp_online = ret;
> +       }
>
>         put_online_cpus();
>
> @@ -809,8 +817,11 @@ static int etm4_probe(struct amba_device
>         return 0;
>
>  err_arch_supported:
> -       if (--etm4_count == 0)
> -               unregister_hotcpu_notifier(&etm4_cpu_notifier);
> +       if (--etm4_count == 0) {
> +               cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT4_STARTING);
> +               if (hp_online)
> +                       cpuhp_remove_state_nocalls(hp_online);
> +       }
>         return ret;
>  }
>
> --- a/include/linux/cpuhotplug.h
> +++ b/include/linux/cpuhotplug.h
> @@ -55,6 +55,7 @@ enum cpuhp_state {
>         CPUHP_AP_KVM_ARM_TIMER_STARTING,
>         CPUHP_AP_ARM_XEN_STARTING,
>         CPUHP_AP_ARM_CORESIGHT_STARTING,
> +       CPUHP_AP_ARM_CORESIGHT4_STARTING,
>         CPUHP_AP_LEDTRIG_STARTING,
>         CPUHP_AP_NOTIFY_STARTING,
>         CPUHP_AP_ONLINE,
>
>

With the above change,

Acked-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>