Re: [PATCH 2/9] coresight-tpda: Add DSB dataset support

Suzuki K Poulose
2022-09-15T10:14:30Z

Hi Tao

On 08/09/2022 09:44, Tao Zhang wrote:
> Read the DSB element size from the device tree. Set the register
> bit that controls the DSB element size of the corresponding port.
>
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> ---
> drivers/hwtracing/coresight/coresight-tpda.c | 62 ++++++++++++++++++++++++++++
> drivers/hwtracing/coresight/coresight-tpda.h | 4 ++
> 2 files changed, 66 insertions(+)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
> index c8bbc75..76636a1 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.c
> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> @@ -37,6 +37,15 @@ static void tpda_enable_port(struct tpda_drvdata *drvdata, int port)
> u32 val;
>
> val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
> + /*
> + * Configure aggregator port n DSB data set element size
> + * Set the bit to 0 if the size is 32
> + * Set the bit to 1 if the size is 64
> + */
> + if (drvdata->dsb_esize[port] == 32)
> + val &= ~TPDA_Pn_CR_DSBSIZE;
> + else if (drvdata->dsb_esize[port] == 64)
> + val |= TPDA_Pn_CR_DSBSIZE;
> /* Enable the port */
> val |= TPDA_Pn_CR_ENA;
> writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
> @@ -105,6 +114,55 @@ static const struct coresight_ops tpda_cs_ops = {
> .link_ops = &tpda_link_ops,
> };
>
> +static int tpda_parse_dsb(struct tpda_drvdata *drvdata)
> +{
> + int len, port, i;
> + const __be32 *prop;
> + struct device_node *node = drvdata->dev->of_node;
> +
> + /* Read the size of DSB element */
> + prop = of_get_property(node, "qcom,dsb-elem-size", &len);
> + if (prop) {
> + len /= sizeof(__be32);
> + /*
> + * The read set of data is port and size, so the number of data
> + * is a multiple of two. And the number of data will not exceed
> + * two times that of the TPDA inpurts number.
> + */
> + if (len < 2 || len >= (2 * TPDA_MAX_INPORTS) || len % 2 != 0) {
> + dev_err(drvdata->dev,
> + "Dataset DSB width entries are wrong\n");
> + return -EINVAL;
> + }
> +
> + for (i = 0; i < len; i++) {

Please could we be explicit here that we are dealing with 2 entries
in an iteration. i.e,

for (i = 0; i < len; i += 2) {
> + port = be32_to_cpu(prop[i++]);

port = be32_to_cpu(prop[i]);

> + if (port >= TPDA_MAX_INPORTS) {
> + dev_err(drvdata->dev,
> + "Wrong port specified for DSB\n");
> + return -EINVAL;
> + }
> + /* Set DSB element size for corresponding port to dsb_esize*/
> + drvdata->dsb_esize[port] = be32_to_cpu(prop[i]);

drvdata->dsb_esize[port] = be32_to_cpu(prop[i + 1]);

> + }
> + }
> +
> + return 0;
> +}
> +
> +static int tpda_parse_of_data(struct tpda_drvdata *drvdata)
> +{
> + int ret;
> +
> + ret = tpda_parse_dsb(drvdata);
> + if (ret) {
> + dev_err(drvdata->dev, "Fail to get DSB data set element size\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> static int tpda_init_default_data(struct tpda_drvdata *drvdata)
> {
> int atid;
> @@ -148,6 +206,10 @@ static int tpda_probe(struct amba_device *adev, const struct amba_id *id)
>
> spin_lock_init(&drvdata->spinlock);
>
> + ret = tpda_parse_of_data(drvdata);
> + if (ret)
> + return ret;
> +
> ret = tpda_init_default_data(drvdata);
> if (ret)
> return ret;
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h
> index 4beb332..ecc7869 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.h
> +++ b/drivers/hwtracing/coresight/coresight-tpda.h
> @@ -10,6 +10,8 @@
> #define TPDA_Pn_CR(n) (0x004 + (n * 4))
> /* Aggregator port enable bit */
> #define TPDA_Pn_CR_ENA BIT(0)
> +/* Aggregator port DSB data set element size bit */
> +#define TPDA_Pn_CR_DSBSIZE BIT(8)
>
> #define TPDA_MAX_INPORTS 32
>
> @@ -23,6 +25,7 @@
> * @csdev: component vitals needed by the framework.
> * @spinlock: lock for the drvdata value.
> * @enable: enable status of the component.
> + * @dsb_esize DSB element size

super minor nit: Missing ":", consistent with the other fields.

> */
> struct tpda_drvdata {
> void __iomem *base;
> @@ -30,6 +33,7 @@ struct tpda_drvdata {
> struct coresight_device *csdev;
> spinlock_t spinlock;
> u8 atid;
> + u32 dsb_esize[TPDA_MAX_INPORTS];
> };
>
> #endif /* _CORESIGHT_CORESIGHT_TPDA_H */

Suzuki