lkml.org 
[lkml]   [2022]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH v3 03/13] coresight: stm: Update STM driver to use Trace ID API
From
On 09/08/2022 23:33, Mike Leach wrote:
> Updates the STM driver to use the trace ID allocation API.
> This uses the _system_id calls to allocate an ID on device poll,
> and release on device remove.
>
> The sysfs access to the STMTRACEIDR register has been changed from RW
> to RO. Having this value as writable is not appropriate for the new
> Trace ID scheme - and had potential to cause errors in the previous
> scheme if values clashed with other sources.
>
> Signed-off-by: Mike Leach <mike.leach@linaro.org>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> ---
> drivers/hwtracing/coresight/coresight-stm.c | 41 +++++++--------------
> 1 file changed, 14 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-stm.c b/drivers/hwtracing/coresight/coresight-stm.c
> index bb14a3a8a921..9ef3e923a930 100644
> --- a/drivers/hwtracing/coresight/coresight-stm.c
> +++ b/drivers/hwtracing/coresight/coresight-stm.c
> @@ -31,6 +31,7 @@
> #include <linux/stm.h>
>
> #include "coresight-priv.h"
> +#include "coresight-trace-id.h"
>
> #define STMDMASTARTR 0xc04
> #define STMDMASTOPR 0xc08
> @@ -615,24 +616,7 @@ static ssize_t traceid_show(struct device *dev,
> val = drvdata->traceid;
> return sprintf(buf, "%#lx\n", val);
> }
> -
> -static ssize_t traceid_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t size)
> -{
> - int ret;
> - unsigned long val;
> - struct stm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> -
> - ret = kstrtoul(buf, 16, &val);
> - if (ret)
> - return ret;
> -
> - /* traceid field is 7bit wide on STM32 */
> - drvdata->traceid = val & 0x7f;
> - return size;
> -}
> -static DEVICE_ATTR_RW(traceid);
> +static DEVICE_ATTR_RO(traceid);
>
> #define coresight_stm_reg(name, offset) \
> coresight_simple_reg32(struct stm_drvdata, name, offset)
> @@ -819,14 +803,6 @@ static void stm_init_default_data(struct stm_drvdata *drvdata)
> */
> drvdata->stmsper = ~0x0;
>
> - /*
> - * The trace ID value for *ETM* tracers start at CPU_ID * 2 + 0x10 and
> - * anything equal to or higher than 0x70 is reserved. Since 0x00 is
> - * also reserved the STM trace ID needs to be higher than 0x00 and
> - * lowner than 0x10.
> - */
> - drvdata->traceid = 0x1;
> -
> /* Set invariant transaction timing on all channels */
> bitmap_clear(drvdata->chs.guaranteed, 0, drvdata->numsp);
> }
> @@ -854,7 +830,7 @@ static void stm_init_generic_data(struct stm_drvdata *drvdata,
>
> static int stm_probe(struct amba_device *adev, const struct amba_id *id)
> {
> - int ret;
> + int ret, trace_id;
> void __iomem *base;
> struct device *dev = &adev->dev;
> struct coresight_platform_data *pdata = NULL;
> @@ -938,12 +914,22 @@ static int stm_probe(struct amba_device *adev, const struct amba_id *id)
> goto stm_unregister;
> }
>
> + trace_id = coresight_trace_id_get_system_id();
> + if (trace_id < 0) {

The above API returns "INVALID_ID" and not a negative error status.
I think it is better to fix the API to return:

ret < 0 - If there is any error
- Otherwise a positive integer
And the users should be kept unaware of which ID is valid or invalid.

Suzuki

\
 
 \ /
  Last update: 2022-10-03 11:08    [W:0.201 / U:0.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site