Messages in this thread | | | From | Mathieu Poirier <> | Date | Tue, 12 Jul 2016 09:19:29 -0600 | Subject | Re: [patch 54/66] hwtracing: coresight-etm3x: Convert to hotplug state machine |
| |
On 11 July 2016 at 06:29, Anna-Maria Gleixner <anna-maria@linutronix.de> wrote: > From: Richard Cochran <rcochran@linutronix.de> > > This driver has an asymmetry of ONLINE code without any corresponding tear > down code. Otherwise, this is a straightforward conversion. > > Signed-off-by: Richard Cochran <rcochran@linutronix.de> > Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Cc: Mathieu Poirier <mathieu.poirier@linaro.org> > Cc: linux-arm-kernel@lists.infradead.org > Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de> > > --- > drivers/hwtracing/coresight/coresight-etm3x.c | 90 ++++++++++++++------------ > include/linux/cpuhotplug.h | 1 > 2 files changed, 50 insertions(+), 41 deletions(-) > > --- a/drivers/hwtracing/coresight/coresight-etm3x.c > +++ b/drivers/hwtracing/coresight/coresight-etm3x.c > @@ -481,8 +481,7 @@ static int etm_enable_sysfs(struct cores > > /* > * Configure the ETM only if the CPU is online. If it isn't online > - * hw configuration will take place when 'CPU_STARTING' is received > - * in @etm_cpu_callback. > + * hw configuration will take place on the local CPU during bring up. > */ > if (cpu_online(drvdata->cpu)) { > ret = smp_call_function_single(drvdata->cpu, > @@ -641,47 +640,44 @@ static const struct coresight_ops etm_cs > .source_ops = &etm_source_ops, > }; > > -static int etm_cpu_callback(struct notifier_block *nfb, unsigned long action, > - void *hcpu) > +static int etm_online_cpu(unsigned int cpu) > { > - unsigned int cpu = (unsigned long)hcpu; > - > 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) { > - etm_os_unlock(etmdrvdata[cpu]); > - etmdrvdata[cpu]->os_unlock = true; > - } > - > - if (local_read(&etmdrvdata[cpu]->mode)) > - etm_enable_hw(etmdrvdata[cpu]); > - spin_unlock(&etmdrvdata[cpu]->spinlock); > - break; > + if (etmdrvdata[cpu]->boot_enable && !etmdrvdata[cpu]->sticky_enable) > + coresight_enable(etmdrvdata[cpu]->csdev); > + return 0; > +} > > - case CPU_ONLINE: > - if (etmdrvdata[cpu]->boot_enable && > - !etmdrvdata[cpu]->sticky_enable) > - coresight_enable(etmdrvdata[cpu]->csdev); > - break; > +static int etm_starting_cpu(unsigned int cpu) > +{ > + if (!etmdrvdata[cpu]) > + return 0; > > - case CPU_DYING: > - spin_lock(&etmdrvdata[cpu]->spinlock); > - if (local_read(&etmdrvdata[cpu]->mode)) > - etm_disable_hw(etmdrvdata[cpu]); > - spin_unlock(&etmdrvdata[cpu]->spinlock); > - break; > + spin_lock(&etmdrvdata[cpu]->spinlock); > + if (!etmdrvdata[cpu]->os_unlock) { > + etm_os_unlock(etmdrvdata[cpu]); > + etmdrvdata[cpu]->os_unlock = true; > } > -out: > - return NOTIFY_OK; > + > + if (local_read(&etmdrvdata[cpu]->mode)) > + etm_enable_hw(etmdrvdata[cpu]); > + spin_unlock(&etmdrvdata[cpu]->spinlock); > + return 0; > } > > -static struct notifier_block etm_cpu_notifier = { > - .notifier_call = etm_cpu_callback, > -}; > +static int etm_dying_cpu(unsigned int cpu) > +{ > + if (!etmdrvdata[cpu]) > + return 0; > + > + spin_lock(&etmdrvdata[cpu]->spinlock); > + if (local_read(&etmdrvdata[cpu]->mode)) > + etm_disable_hw(etmdrvdata[cpu]); > + spin_unlock(&etmdrvdata[cpu]->spinlock); > + return 0; > +} > > static bool etm_arch_supported(u8 arch) > { > @@ -750,6 +746,8 @@ static void etm_init_trace_id(struct etm > drvdata->traceid = coresight_get_trace_id(drvdata->cpu); > } > > +static enum cpuhp_state hp_online; > +
Can we put this at the top of the file with the other global declaration?
> static int etm_probe(struct amba_device *adev, const struct amba_id *id) > { > int ret; > @@ -806,9 +804,17 @@ static int etm_probe(struct amba_device > etm_init_arch_data, drvdata, 1)) > dev_err(dev, "ETM arch init failed\n"); > > - if (!etm_count++) > - register_hotcpu_notifier(&etm_cpu_notifier); > - > + if (!etm_count++) { > + cpuhp_setup_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING, > + "AP_ARM_CORESIGHT_STARTING", > + etm_starting_cpu, etm_dying_cpu); > + ret = cpuhp_setup_state_nocalls(CPUHP_AP_ONLINE_DYN, > + "AP_ARM_CORESIGHT_ONLINE", > + etm_online_cpu, NULL); > + if (ret < 0) > + goto err_arch_supported; > + hp_online = ret; > + } > put_online_cpus(); > > if (etm_arch_supported(drvdata->arch) == false) { > @@ -839,7 +845,6 @@ static int etm_probe(struct amba_device > > pm_runtime_put(&adev->dev); > dev_info(dev, "%s initialized\n", (char *)id->data); > - > if (boot_enable) { > coresight_enable(drvdata->csdev); > drvdata->boot_enable = true; > @@ -848,8 +853,11 @@ static int etm_probe(struct amba_device > return 0; > > err_arch_supported: > - if (--etm_count == 0) > - unregister_hotcpu_notifier(&etm_cpu_notifier); > + if (--etm_count == 0) { > + cpuhp_remove_state_nocalls(CPUHP_AP_ARM_CORESIGHT_STARTING); > + if (hp_online) > + cpuhp_remove_state_nocalls(hp_online); > + } > return ret; > } > > --- a/include/linux/cpuhotplug.h > +++ b/include/linux/cpuhotplug.h > @@ -54,6 +54,7 @@ enum cpuhp_state { > CPUHP_AP_KVM_ARM_VGIC_STARTING, > CPUHP_AP_KVM_ARM_TIMER_STARTING, > CPUHP_AP_ARM_XEN_STARTING, > + CPUHP_AP_ARM_CORESIGHT_STARTING, > CPUHP_AP_LEDTRIG_STARTING, > CPUHP_AP_NOTIFY_STARTING, > CPUHP_AP_ONLINE, > >
This looks good to me - thanks for contribution. With the above change,
Acked-by: Mathieu Poirier <mathieu.poirier@linaro.org>
| |