diff mbox series

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

Message ID 1662626705-13097-3-git-send-email-quic_taozha@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Add support to configure TPDM DSB subunit | expand

Commit Message

Tao Zhang Sept. 8, 2022, 8:44 a.m. UTC
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(+)

Comments

Suzuki K Poulose Sept. 15, 2022, 10:14 a.m. UTC | #1
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
Tao Zhang Sept. 20, 2022, 1:40 p.m. UTC | #2
Hi Suzuki,


Thanks a lot for your code review.

I will update the patch with your suggestions in the next release.

On 9/15/2022 6:14 PM, Suzuki K Poulose wrote:
> 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]);
>
Sure, I will update this part of the code in the next release.
>> +        }
>> +    }
>> +
>> +    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.
>
I will update in the next release.
>>    */
>>   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

Regards,

Tao
diff mbox series

Patch

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++) {
+			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]);
+		}
+	}
+
+	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
  */
 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 */