[PATCH v4 6/9] coresight: etm3x: change drvdata->spinlock type to raw_spin_lock_t
Jie Gan
jie.gan at oss.qualcomm.com
Wed Apr 15 05:05:07 PDT 2026
On 4/13/2026 10:19 PM, Yeoreum Yun wrote:
> etm_starting_cpu()/etm_dying_cpu() are called in not sleepable context.
> This poses an issue in PREEMPT_RT kernel where spinlock_t is sleepable.
>
> To address this, change etm3's drvdata->spinlock type to raw_spin_lock_t.
> This will be good to align with etm4x.
>
LGTM
Reviewed-by: Jie Gan <jie.gan at oss.qualcomm.com>
> Signed-off-by: Yeoreum Yun <yeoreum.yun at arm.com>
> ---
> drivers/hwtracing/coresight/coresight-etm.h | 2 +-
> .../coresight/coresight-etm3x-core.c | 18 +--
> .../coresight/coresight-etm3x-sysfs.c | 130 +++++++++---------
> 3 files changed, 75 insertions(+), 75 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-etm.h b/drivers/hwtracing/coresight/coresight-etm.h
> index 1d753cca2943..40f20daded4f 100644
> --- a/drivers/hwtracing/coresight/coresight-etm.h
> +++ b/drivers/hwtracing/coresight/coresight-etm.h
> @@ -232,7 +232,7 @@ struct etm_drvdata {
> struct csdev_access csa;
> struct clk *atclk;
> struct coresight_device *csdev;
> - spinlock_t spinlock;
> + raw_spinlock_t spinlock;
> int cpu;
> int port_size;
> u8 arch;
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-core.c b/drivers/hwtracing/coresight/coresight-etm3x-core.c
> index a547a6d2e0bd..4a702b515733 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-core.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-core.c
> @@ -511,7 +511,7 @@ static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_pat
> struct etm_enable_arg arg = { };
> int ret;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
>
> drvdata->traceid = path->trace_id;
>
> @@ -534,7 +534,7 @@ static int etm_enable_sysfs(struct coresight_device *csdev, struct coresight_pat
> if (ret)
> etm_release_trace_id(drvdata);
>
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> if (!ret)
> dev_dbg(&csdev->dev, "ETM tracing enabled\n");
> @@ -634,7 +634,7 @@ static void etm_disable_sysfs(struct coresight_device *csdev)
> * DYING hotplug callback is serviced by the ETM driver.
> */
> cpus_read_lock();
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
>
> /*
> * Executing etm_disable_hw on the cpu whose ETM is being disabled
> @@ -643,7 +643,7 @@ static void etm_disable_sysfs(struct coresight_device *csdev)
> smp_call_function_single(drvdata->cpu, etm_disable_sysfs_smp_call,
> drvdata, 1);
>
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> cpus_read_unlock();
>
> /*
> @@ -709,7 +709,7 @@ static int etm_starting_cpu(unsigned int cpu)
> if (!etmdrvdata[cpu])
> return 0;
>
> - spin_lock(&etmdrvdata[cpu]->spinlock);
> + raw_spin_lock(&etmdrvdata[cpu]->spinlock);
> if (!etmdrvdata[cpu]->os_unlock) {
> etm_os_unlock(etmdrvdata[cpu]);
> etmdrvdata[cpu]->os_unlock = true;
> @@ -717,7 +717,7 @@ static int etm_starting_cpu(unsigned int cpu)
>
> if (coresight_get_mode(etmdrvdata[cpu]->csdev))
> etm_enable_hw(etmdrvdata[cpu]);
> - spin_unlock(&etmdrvdata[cpu]->spinlock);
> + raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
> return 0;
> }
>
> @@ -726,10 +726,10 @@ static int etm_dying_cpu(unsigned int cpu)
> if (!etmdrvdata[cpu])
> return 0;
>
> - spin_lock(&etmdrvdata[cpu]->spinlock);
> + raw_spin_lock(&etmdrvdata[cpu]->spinlock);
> if (coresight_get_mode(etmdrvdata[cpu]->csdev))
> etm_disable_hw(etmdrvdata[cpu]);
> - spin_unlock(&etmdrvdata[cpu]->spinlock);
> + raw_spin_unlock(&etmdrvdata[cpu]->spinlock);
> return 0;
> }
>
> @@ -856,7 +856,7 @@ static int etm_probe(struct amba_device *adev, const struct amba_id *id)
>
> desc.access = drvdata->csa = CSDEV_ACCESS_IOMEM(base);
>
> - spin_lock_init(&drvdata->spinlock);
> + raw_spin_lock_init(&drvdata->spinlock);
>
> drvdata->atclk = devm_clk_get_optional_enabled(dev, "atclk");
> if (IS_ERR(drvdata->atclk))
> diff --git a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> index 762109307b86..42b12c33516b 100644
> --- a/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-etm3x-sysfs.c
> @@ -49,13 +49,13 @@ static ssize_t etmsr_show(struct device *dev,
> struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>
> pm_runtime_get_sync(dev->parent);
> - spin_lock_irqsave(&drvdata->spinlock, flags);
> + raw_spin_lock_irqsave(&drvdata->spinlock, flags);
> CS_UNLOCK(drvdata->csa.base);
>
> val = etm_readl(drvdata, ETMSR);
>
> CS_LOCK(drvdata->csa.base);
> - spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> pm_runtime_put(dev->parent);
>
> return sprintf(buf, "%#lx\n", val);
> @@ -76,7 +76,7 @@ static ssize_t reset_store(struct device *dev,
> return ret;
>
> if (val) {
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> memset(config, 0, sizeof(struct etm_config));
> config->mode = ETM_MODE_EXCLUDE;
> config->trigger_event = ETM_DEFAULT_EVENT_VAL;
> @@ -86,7 +86,7 @@ static ssize_t reset_store(struct device *dev,
>
> etm_set_default(config);
> etm_release_trace_id(drvdata);
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> }
>
> return size;
> @@ -117,7 +117,7 @@ static ssize_t mode_store(struct device *dev,
> if (ret)
> return ret;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> config->mode = val & ETM_MODE_ALL;
>
> if (config->mode & ETM_MODE_EXCLUDE)
> @@ -168,12 +168,12 @@ static ssize_t mode_store(struct device *dev,
> if (config->mode & (ETM_MODE_EXCL_KERN | ETM_MODE_EXCL_USER))
> etm_config_trace_mode(config);
>
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
>
> err_unlock:
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return ret;
> }
> static DEVICE_ATTR_RW(mode);
> @@ -299,9 +299,9 @@ static ssize_t addr_idx_store(struct device *dev,
> * Use spinlock to ensure index doesn't change while it gets
> * dereferenced multiple times within a spinlock block elsewhere.
> */
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> config->addr_idx = val;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -315,16 +315,16 @@ static ssize_t addr_single_show(struct device *dev,
> struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> struct etm_config *config = &drvdata->config;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> idx = config->addr_idx;
> if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
> config->addr_type[idx] == ETM_ADDR_TYPE_SINGLE)) {
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return -EINVAL;
> }
>
> val = config->addr_val[idx];
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return sprintf(buf, "%#lx\n", val);
> }
> @@ -343,17 +343,17 @@ static ssize_t addr_single_store(struct device *dev,
> if (ret)
> return ret;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> idx = config->addr_idx;
> if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
> config->addr_type[idx] == ETM_ADDR_TYPE_SINGLE)) {
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return -EINVAL;
> }
>
> config->addr_val[idx] = val;
> config->addr_type[idx] = ETM_ADDR_TYPE_SINGLE;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -367,23 +367,23 @@ static ssize_t addr_range_show(struct device *dev,
> struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> struct etm_config *config = &drvdata->config;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> idx = config->addr_idx;
> if (idx % 2 != 0) {
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return -EPERM;
> }
> if (!((config->addr_type[idx] == ETM_ADDR_TYPE_NONE &&
> config->addr_type[idx + 1] == ETM_ADDR_TYPE_NONE) ||
> (config->addr_type[idx] == ETM_ADDR_TYPE_RANGE &&
> config->addr_type[idx + 1] == ETM_ADDR_TYPE_RANGE))) {
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return -EPERM;
> }
>
> val1 = config->addr_val[idx];
> val2 = config->addr_val[idx + 1];
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return sprintf(buf, "%#lx %#lx\n", val1, val2);
> }
> @@ -403,17 +403,17 @@ static ssize_t addr_range_store(struct device *dev,
> if (val1 > val2)
> return -EINVAL;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> idx = config->addr_idx;
> if (idx % 2 != 0) {
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return -EPERM;
> }
> if (!((config->addr_type[idx] == ETM_ADDR_TYPE_NONE &&
> config->addr_type[idx + 1] == ETM_ADDR_TYPE_NONE) ||
> (config->addr_type[idx] == ETM_ADDR_TYPE_RANGE &&
> config->addr_type[idx + 1] == ETM_ADDR_TYPE_RANGE))) {
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return -EPERM;
> }
>
> @@ -422,7 +422,7 @@ static ssize_t addr_range_store(struct device *dev,
> config->addr_val[idx + 1] = val2;
> config->addr_type[idx + 1] = ETM_ADDR_TYPE_RANGE;
> config->enable_ctrl1 |= (1 << (idx/2));
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -436,16 +436,16 @@ static ssize_t addr_start_show(struct device *dev,
> struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> struct etm_config *config = &drvdata->config;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> idx = config->addr_idx;
> if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
> config->addr_type[idx] == ETM_ADDR_TYPE_START)) {
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return -EPERM;
> }
>
> val = config->addr_val[idx];
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return sprintf(buf, "%#lx\n", val);
> }
> @@ -464,11 +464,11 @@ static ssize_t addr_start_store(struct device *dev,
> if (ret)
> return ret;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> idx = config->addr_idx;
> if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
> config->addr_type[idx] == ETM_ADDR_TYPE_START)) {
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return -EPERM;
> }
>
> @@ -476,7 +476,7 @@ static ssize_t addr_start_store(struct device *dev,
> config->addr_type[idx] = ETM_ADDR_TYPE_START;
> config->startstop_ctrl |= (1 << idx);
> config->enable_ctrl1 |= ETMTECR1_START_STOP;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -490,16 +490,16 @@ static ssize_t addr_stop_show(struct device *dev,
> struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> struct etm_config *config = &drvdata->config;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> idx = config->addr_idx;
> if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
> config->addr_type[idx] == ETM_ADDR_TYPE_STOP)) {
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return -EPERM;
> }
>
> val = config->addr_val[idx];
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return sprintf(buf, "%#lx\n", val);
> }
> @@ -518,11 +518,11 @@ static ssize_t addr_stop_store(struct device *dev,
> if (ret)
> return ret;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> idx = config->addr_idx;
> if (!(config->addr_type[idx] == ETM_ADDR_TYPE_NONE ||
> config->addr_type[idx] == ETM_ADDR_TYPE_STOP)) {
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return -EPERM;
> }
>
> @@ -530,7 +530,7 @@ static ssize_t addr_stop_store(struct device *dev,
> config->addr_type[idx] = ETM_ADDR_TYPE_STOP;
> config->startstop_ctrl |= (1 << (idx + 16));
> config->enable_ctrl1 |= ETMTECR1_START_STOP;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -543,9 +543,9 @@ static ssize_t addr_acctype_show(struct device *dev,
> struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> struct etm_config *config = &drvdata->config;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> val = config->addr_acctype[config->addr_idx];
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return sprintf(buf, "%#lx\n", val);
> }
> @@ -563,9 +563,9 @@ static ssize_t addr_acctype_store(struct device *dev,
> if (ret)
> return ret;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> config->addr_acctype[config->addr_idx] = val;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -601,9 +601,9 @@ static ssize_t cntr_idx_store(struct device *dev,
> * Use spinlock to ensure index doesn't change while it gets
> * dereferenced multiple times within a spinlock block elsewhere.
> */
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> config->cntr_idx = val;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -616,9 +616,9 @@ static ssize_t cntr_rld_val_show(struct device *dev,
> struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> struct etm_config *config = &drvdata->config;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> val = config->cntr_rld_val[config->cntr_idx];
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return sprintf(buf, "%#lx\n", val);
> }
> @@ -636,9 +636,9 @@ static ssize_t cntr_rld_val_store(struct device *dev,
> if (ret)
> return ret;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> config->cntr_rld_val[config->cntr_idx] = val;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -651,9 +651,9 @@ static ssize_t cntr_event_show(struct device *dev,
> struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> struct etm_config *config = &drvdata->config;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> val = config->cntr_event[config->cntr_idx];
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return sprintf(buf, "%#lx\n", val);
> }
> @@ -671,9 +671,9 @@ static ssize_t cntr_event_store(struct device *dev,
> if (ret)
> return ret;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> config->cntr_event[config->cntr_idx] = val & ETM_EVENT_MASK;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -686,9 +686,9 @@ static ssize_t cntr_rld_event_show(struct device *dev,
> struct etm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> struct etm_config *config = &drvdata->config;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> val = config->cntr_rld_event[config->cntr_idx];
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return sprintf(buf, "%#lx\n", val);
> }
> @@ -706,9 +706,9 @@ static ssize_t cntr_rld_event_store(struct device *dev,
> if (ret)
> return ret;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> config->cntr_rld_event[config->cntr_idx] = val & ETM_EVENT_MASK;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -723,11 +723,11 @@ static ssize_t cntr_val_show(struct device *dev,
> struct etm_config *config = &drvdata->config;
>
> if (!coresight_get_mode(drvdata->csdev)) {
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> for (i = 0; i < drvdata->nr_cntr; i++)
> ret += sprintf(buf, "counter %d: %x\n",
> i, config->cntr_val[i]);
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
> return ret;
> }
>
> @@ -752,9 +752,9 @@ static ssize_t cntr_val_store(struct device *dev,
> if (ret)
> return ret;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> config->cntr_val[config->cntr_idx] = val;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -947,13 +947,13 @@ static ssize_t seq_curr_state_show(struct device *dev,
> }
>
> pm_runtime_get_sync(dev->parent);
> - spin_lock_irqsave(&drvdata->spinlock, flags);
> + raw_spin_lock_irqsave(&drvdata->spinlock, flags);
>
> CS_UNLOCK(drvdata->csa.base);
> val = (etm_readl(drvdata, ETMSQR) & ETM_SQR_MASK);
> CS_LOCK(drvdata->csa.base);
>
> - spin_unlock_irqrestore(&drvdata->spinlock, flags);
> + raw_spin_unlock_irqrestore(&drvdata->spinlock, flags);
> pm_runtime_put(dev->parent);
> out:
> return sprintf(buf, "%#lx\n", val);
> @@ -1012,9 +1012,9 @@ static ssize_t ctxid_idx_store(struct device *dev,
> * Use spinlock to ensure index doesn't change while it gets
> * dereferenced multiple times within a spinlock block elsewhere.
> */
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> config->ctxid_idx = val;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
> @@ -1034,9 +1034,9 @@ static ssize_t ctxid_pid_show(struct device *dev,
> if (task_active_pid_ns(current) != &init_pid_ns)
> return -EINVAL;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> val = config->ctxid_pid[config->ctxid_idx];
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return sprintf(buf, "%#lx\n", val);
> }
> @@ -1066,9 +1066,9 @@ static ssize_t ctxid_pid_store(struct device *dev,
> if (ret)
> return ret;
>
> - spin_lock(&drvdata->spinlock);
> + raw_spin_lock(&drvdata->spinlock);
> config->ctxid_pid[config->ctxid_idx] = pid;
> - spin_unlock(&drvdata->spinlock);
> + raw_spin_unlock(&drvdata->spinlock);
>
> return size;
> }
More information about the linux-arm-kernel
mailing list