enable_req_count is only ever accessed inside the spinlock, so to avoid
confusion that there are concurrent accesses and simplify the code,
change it to an int.
One access outside of the spinlock is in enable_show() which appears to
allow partially written data to be displayed between enable_req_count,
powered and enabled so move this one inside the spin lock too.
Signed-off-by: James Clark <[email protected]>
---
drivers/hwtracing/coresight/coresight-cti-core.c | 14 +++++++-------
drivers/hwtracing/coresight/coresight-cti-sysfs.c | 2 +-
drivers/hwtracing/coresight/coresight-cti.h | 2 +-
3 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
index 838872f2484d..277c890a1f1f 100644
--- a/drivers/hwtracing/coresight/coresight-cti-core.c
+++ b/drivers/hwtracing/coresight/coresight-cti-core.c
@@ -107,12 +107,12 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
cti_write_all_hw_regs(drvdata);
config->hw_enabled = true;
- atomic_inc(&drvdata->config.enable_req_count);
+ drvdata->config.enable_req_count++;
spin_unlock_irqrestore(&drvdata->spinlock, flags);
return rc;
cti_state_unchanged:
- atomic_inc(&drvdata->config.enable_req_count);
+ drvdata->config.enable_req_count++;
/* cannot enable due to error */
cti_err_not_enabled:
@@ -129,7 +129,7 @@ static void cti_cpuhp_enable_hw(struct cti_drvdata *drvdata)
config->hw_powered = true;
/* no need to do anything if no enable request */
- if (!atomic_read(&drvdata->config.enable_req_count))
+ if (!drvdata->config.enable_req_count)
goto cti_hp_not_enabled;
/* try to claim the device */
@@ -156,13 +156,13 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
spin_lock(&drvdata->spinlock);
/* don't allow negative refcounts, return an error */
- if (!atomic_read(&drvdata->config.enable_req_count)) {
+ if (!drvdata->config.enable_req_count) {
ret = -EINVAL;
goto cti_not_disabled;
}
/* check refcount - disable on 0 */
- if (atomic_dec_return(&drvdata->config.enable_req_count) > 0)
+ if (--drvdata->config.enable_req_count > 0)
goto cti_not_disabled;
/* no need to do anything if disabled or cpu unpowered */
@@ -239,7 +239,7 @@ static void cti_set_default_config(struct device *dev,
/* Most regs default to 0 as zalloc'ed except...*/
config->trig_filter_enable = true;
config->ctigate = GENMASK(config->nr_ctm_channels - 1, 0);
- atomic_set(&config->enable_req_count, 0);
+ config->enable_req_count = 0;
}
/*
@@ -696,7 +696,7 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
drvdata->config.hw_enabled = false;
/* check enable reference count to enable HW */
- if (atomic_read(&drvdata->config.enable_req_count)) {
+ if (drvdata->config.enable_req_count) {
/* check we can claim the device as we re-power */
if (coresight_claim_device(csdev))
goto cti_notify_exit;
diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
index 71e7a8266bb3..e528cff9d4e2 100644
--- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
+++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
@@ -84,8 +84,8 @@ static ssize_t enable_show(struct device *dev,
bool enabled, powered;
struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
- enable_req = atomic_read(&drvdata->config.enable_req_count);
spin_lock(&drvdata->spinlock);
+ enable_req = drvdata->config.enable_req_count;
powered = drvdata->config.hw_powered;
enabled = drvdata->config.hw_enabled;
spin_unlock(&drvdata->spinlock);
diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
index acf7b545e6b9..8b106b13a244 100644
--- a/drivers/hwtracing/coresight/coresight-cti.h
+++ b/drivers/hwtracing/coresight/coresight-cti.h
@@ -141,7 +141,7 @@ struct cti_config {
int nr_trig_max;
/* cti enable control */
- atomic_t enable_req_count;
+ int enable_req_count;
bool hw_enabled;
bool hw_powered;
--
2.25.1
On Tue, 10 Jan 2023 at 11:08, James Clark <[email protected]> wrote:
>
> enable_req_count is only ever accessed inside the spinlock, so to avoid
> confusion that there are concurrent accesses and simplify the code,
> change it to an int.
>
> One access outside of the spinlock is in enable_show() which appears to
> allow partially written data to be displayed between enable_req_count,
> powered and enabled so move this one inside the spin lock too.
>
> Signed-off-by: James Clark <[email protected]>
> ---
> drivers/hwtracing/coresight/coresight-cti-core.c | 14 +++++++-------
> drivers/hwtracing/coresight/coresight-cti-sysfs.c | 2 +-
> drivers/hwtracing/coresight/coresight-cti.h | 2 +-
> 3 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-cti-core.c b/drivers/hwtracing/coresight/coresight-cti-core.c
> index 838872f2484d..277c890a1f1f 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-core.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-core.c
> @@ -107,12 +107,12 @@ static int cti_enable_hw(struct cti_drvdata *drvdata)
> cti_write_all_hw_regs(drvdata);
>
> config->hw_enabled = true;
> - atomic_inc(&drvdata->config.enable_req_count);
> + drvdata->config.enable_req_count++;
> spin_unlock_irqrestore(&drvdata->spinlock, flags);
> return rc;
>
> cti_state_unchanged:
> - atomic_inc(&drvdata->config.enable_req_count);
> + drvdata->config.enable_req_count++;
>
> /* cannot enable due to error */
> cti_err_not_enabled:
> @@ -129,7 +129,7 @@ static void cti_cpuhp_enable_hw(struct cti_drvdata *drvdata)
> config->hw_powered = true;
>
> /* no need to do anything if no enable request */
> - if (!atomic_read(&drvdata->config.enable_req_count))
> + if (!drvdata->config.enable_req_count)
> goto cti_hp_not_enabled;
>
> /* try to claim the device */
> @@ -156,13 +156,13 @@ static int cti_disable_hw(struct cti_drvdata *drvdata)
> spin_lock(&drvdata->spinlock);
>
> /* don't allow negative refcounts, return an error */
> - if (!atomic_read(&drvdata->config.enable_req_count)) {
> + if (!drvdata->config.enable_req_count) {
> ret = -EINVAL;
> goto cti_not_disabled;
> }
>
> /* check refcount - disable on 0 */
> - if (atomic_dec_return(&drvdata->config.enable_req_count) > 0)
> + if (--drvdata->config.enable_req_count > 0)
> goto cti_not_disabled;
>
> /* no need to do anything if disabled or cpu unpowered */
> @@ -239,7 +239,7 @@ static void cti_set_default_config(struct device *dev,
> /* Most regs default to 0 as zalloc'ed except...*/
> config->trig_filter_enable = true;
> config->ctigate = GENMASK(config->nr_ctm_channels - 1, 0);
> - atomic_set(&config->enable_req_count, 0);
> + config->enable_req_count = 0;
> }
>
> /*
> @@ -696,7 +696,7 @@ static int cti_cpu_pm_notify(struct notifier_block *nb, unsigned long cmd,
> drvdata->config.hw_enabled = false;
>
> /* check enable reference count to enable HW */
> - if (atomic_read(&drvdata->config.enable_req_count)) {
> + if (drvdata->config.enable_req_count) {
> /* check we can claim the device as we re-power */
> if (coresight_claim_device(csdev))
> goto cti_notify_exit;
> diff --git a/drivers/hwtracing/coresight/coresight-cti-sysfs.c b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> index 71e7a8266bb3..e528cff9d4e2 100644
> --- a/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> +++ b/drivers/hwtracing/coresight/coresight-cti-sysfs.c
> @@ -84,8 +84,8 @@ static ssize_t enable_show(struct device *dev,
> bool enabled, powered;
> struct cti_drvdata *drvdata = dev_get_drvdata(dev->parent);
>
> - enable_req = atomic_read(&drvdata->config.enable_req_count);
> spin_lock(&drvdata->spinlock);
> + enable_req = drvdata->config.enable_req_count;
> powered = drvdata->config.hw_powered;
> enabled = drvdata->config.hw_enabled;
> spin_unlock(&drvdata->spinlock);
> diff --git a/drivers/hwtracing/coresight/coresight-cti.h b/drivers/hwtracing/coresight/coresight-cti.h
> index acf7b545e6b9..8b106b13a244 100644
> --- a/drivers/hwtracing/coresight/coresight-cti.h
> +++ b/drivers/hwtracing/coresight/coresight-cti.h
> @@ -141,7 +141,7 @@ struct cti_config {
> int nr_trig_max;
>
> /* cti enable control */
> - atomic_t enable_req_count;
> + int enable_req_count;
> bool hw_enabled;
> bool hw_powered;
>
> --
> 2.25.1
>
Reviewed-by: Mike Leach <[email protected]>
--
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK