[v6,05/10] coresight-tpda: Add support to configure CMB element
Commit Message
Read the CMB element size from the device tree. Set the register
bit that controls the CMB element size of the corresponding port.
Reviewed-by: James Clark <james.clark@arm.com>
Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
---
drivers/hwtracing/coresight/coresight-tpda.c | 125 +++++++++++--------
drivers/hwtracing/coresight/coresight-tpda.h | 6 +
2 files changed, 81 insertions(+), 50 deletions(-)
Comments
On 02/02/2024 09:32, Tao Zhang wrote:
> Read the CMB element size from the device tree. Set the register
> bit that controls the CMB element size of the corresponding port.
>
> Reviewed-by: James Clark <james.clark@arm.com>
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
> ---
> drivers/hwtracing/coresight/coresight-tpda.c | 125 +++++++++++--------
> drivers/hwtracing/coresight/coresight-tpda.h | 6 +
> 2 files changed, 81 insertions(+), 50 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c
> index 4ac954f4bc13..27d567f4c8bf 100644
> --- a/drivers/hwtracing/coresight/coresight-tpda.c
> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> @@ -18,6 +18,7 @@
> #include "coresight-priv.h"
> #include "coresight-tpda.h"
> #include "coresight-trace-id.h"
> +#include "coresight-tpdm.h"
>
> DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
>
> @@ -28,24 +29,59 @@ static bool coresight_device_is_tpdm(struct coresight_device *csdev)
> CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM);
> }
>
> +static void tpdm_clear_element_size(struct coresight_device *csdev)
I just noticed this anomaly. This is supposed to be :
tpda_clear_element_size() ? I can fix it up locally.
Suzuki
> +{
> + struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
> +
> + drvdata->dsb_esize = 0;
> + drvdata->cmb_esize = 0;
> +}
> +
> +static void tpda_set_element_size(struct tpda_drvdata *drvdata, u32 *val)
> +{
> + /* Clear all relevant fields */
> + *val &= ~(TPDA_Pn_CR_DSBSIZE | TPDA_Pn_CR_CMBSIZE);
> +
> + if (drvdata->dsb_esize == 64)
> + *val |= TPDA_Pn_CR_DSBSIZE;
> + else if (drvdata->dsb_esize == 32)
> + *val &= ~TPDA_Pn_CR_DSBSIZE;
> +
> + if (drvdata->cmb_esize == 64)
> + *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x2);
> + else if (drvdata->cmb_esize == 32)
> + *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x1);
> + else if (drvdata->cmb_esize == 8)
> + *val &= ~TPDA_Pn_CR_CMBSIZE;
> +}
> +
> /*
> - * Read the DSB element size from the TPDM device
> + * Read the element size from the TPDM device. One TPDM must have at least one of the
> + * element size property.
> * Returns
> - * The dsb element size read from the devicetree if available.
> - * 0 - Otherwise, with a warning once.
> + * 0 - The element size property is read
> + * Others - Cannot read the property of the element size
> */
> -static int tpdm_read_dsb_element_size(struct coresight_device *csdev)
> +static int tpdm_read_element_size(struct tpda_drvdata *drvdata,
> + struct coresight_device *csdev)
> {
> - int rc = 0;
> - u8 size = 0;
> + int rc = -EINVAL;
> + struct tpdm_drvdata *tpdm_data = dev_get_drvdata(csdev->dev.parent);
> +
> + if (tpdm_has_dsb_dataset(tpdm_data)) {
> + rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent),
> + "qcom,dsb-element-size", &drvdata->dsb_esize);
> + }
> + if (tpdm_has_cmb_dataset(tpdm_data)) {
> + rc = fwnode_property_read_u32(dev_fwnode(csdev->dev.parent),
> + "qcom,cmb-element-bits", &drvdata->cmb_esize);
> + }
>
> - rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent),
> - "qcom,dsb-element-size", &size);
> if (rc)
> dev_warn_once(&csdev->dev,
> - "Failed to read TPDM DSB Element size: %d\n", rc);
> + "Failed to read TPDM Element size: %d\n", rc);
>
> - return size;
> + return rc;
> }
>
> /*
> @@ -56,11 +92,12 @@ static int tpdm_read_dsb_element_size(struct coresight_device *csdev)
> * Parameter "inport" is used to pass in the input port number
> * of TPDA, and it is set to -1 in the recursize call.
> */
> -static int tpda_get_element_size(struct coresight_device *csdev,
> +static int tpda_get_element_size(struct tpda_drvdata *drvdata,
> + struct coresight_device *csdev,
> int inport)
> {
> - int dsb_size = -ENOENT;
> - int i, size;
> + int rc = 0;
> + int i;
> struct coresight_device *in;
>
> for (i = 0; i < csdev->pdata->nr_inconns; i++) {
> @@ -69,30 +106,26 @@ static int tpda_get_element_size(struct coresight_device *csdev,
> continue;
>
> /* Ignore the paths that do not match port */
> - if (inport > 0 &&
> + if (inport >= 0 &&
> csdev->pdata->in_conns[i]->dest_port != inport)
> continue;
>
> if (coresight_device_is_tpdm(in)) {
> - size = tpdm_read_dsb_element_size(in);
> + if (drvdata->dsb_esize || drvdata->cmb_esize)
> + return -EEXIST;
> + rc = tpdm_read_element_size(drvdata, in);
> + if (rc)
> + return rc;
> } else {
> /* Recurse down the path */
> - size = tpda_get_element_size(in, -1);
> - }
> -
> - if (size < 0)
> - return size;
> -
> - if (dsb_size < 0) {
> - /* Found a size, save it. */
> - dsb_size = size;
> - } else {
> - /* Found duplicate TPDMs */
> - return -EEXIST;
> + rc = tpda_get_element_size(drvdata, in, -1);
> + if (rc)
> + return rc;
> }
> }
>
> - return dsb_size;
> +
> + return rc;
> }
>
> /* Settings pre enabling port control register */
> @@ -109,7 +142,7 @@ static void tpda_enable_pre_port(struct tpda_drvdata *drvdata)
> static int tpda_enable_port(struct tpda_drvdata *drvdata, int port)
> {
> u32 val;
> - int size;
> + int rc;
>
> val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
> /*
> @@ -117,29 +150,21 @@ static int tpda_enable_port(struct tpda_drvdata *drvdata, int port)
> * Set the bit to 0 if the size is 32
> * Set the bit to 1 if the size is 64
> */
> - size = tpda_get_element_size(drvdata->csdev, port);
> - switch (size) {
> - case 32:
> - val &= ~TPDA_Pn_CR_DSBSIZE;
> - break;
> - case 64:
> - val |= TPDA_Pn_CR_DSBSIZE;
> - break;
> - case 0:
> - return -EEXIST;
> - case -EEXIST:
> + tpdm_clear_element_size(drvdata->csdev);
> + rc = tpda_get_element_size(drvdata, drvdata->csdev, port);
> + if (!rc && (drvdata->dsb_esize || drvdata->cmb_esize)) {
> + tpda_set_element_size(drvdata, &val);
> + /* Enable the port */
> + val |= TPDA_Pn_CR_ENA;
> + writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
> + } else if (rc == -EEXIST)
> dev_warn_once(&drvdata->csdev->dev,
> - "Detected multiple TPDMs on port %d", -EEXIST);
> - return -EEXIST;
> - default:
> - return -EINVAL;
> - }
> -
> - /* Enable the port */
> - val |= TPDA_Pn_CR_ENA;
> - writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
> + "Detected multiple TPDMs on port %d", port);
> + else
> + dev_warn_once(&drvdata->csdev->dev,
> + "Didn't find TPDM element size");
>
> - return 0;
> + return rc;
> }
>
> static int __tpda_enable(struct tpda_drvdata *drvdata, int port)
> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h
> index b3b38fd41b64..19af64120fcf 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 CMB data set element size bit */
> +#define TPDA_Pn_CR_CMBSIZE GENMASK(7, 6)
> /* Aggregator port DSB data set element size bit */
> #define TPDA_Pn_CR_DSBSIZE BIT(8)
>
> @@ -25,6 +27,8 @@
> * @csdev: component vitals needed by the framework.
> * @spinlock: lock for the drvdata value.
> * @enable: enable status of the component.
> + * @dsb_esize Record the DSB element size.
> + * @cmb_esize Record the CMB element size.
> */
> struct tpda_drvdata {
> void __iomem *base;
> @@ -32,6 +36,8 @@ struct tpda_drvdata {
> struct coresight_device *csdev;
> spinlock_t spinlock;
> u8 atid;
> + u8 dsb_esize;
> + u32 cmb_esize;
> };
>
> #endif /* _CORESIGHT_CORESIGHT_TPDA_H */
On 02/02/2024 09:45, Suzuki K Poulose wrote:
> On 02/02/2024 09:32, Tao Zhang wrote:
>> Read the CMB element size from the device tree. Set the register
>> bit that controls the CMB element size of the corresponding port.
>>
>> Reviewed-by: James Clark <james.clark@arm.com>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
>> ---
>> drivers/hwtracing/coresight/coresight-tpda.c | 125 +++++++++++--------
>> drivers/hwtracing/coresight/coresight-tpda.h | 6 +
>> 2 files changed, 81 insertions(+), 50 deletions(-)
>>
>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c
>> b/drivers/hwtracing/coresight/coresight-tpda.c
>> index 4ac954f4bc13..27d567f4c8bf 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpda.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>> @@ -18,6 +18,7 @@
>> #include "coresight-priv.h"
>> #include "coresight-tpda.h"
>> #include "coresight-trace-id.h"
>> +#include "coresight-tpdm.h"
>> DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
>> @@ -28,24 +29,59 @@ static bool coresight_device_is_tpdm(struct
>> coresight_device *csdev)
>> CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM);
>> }
>> +static void tpdm_clear_element_size(struct coresight_device *csdev)
>
> I just noticed this anomaly. This is supposed to be :
>
> tpda_clear_element_size() ? I can fix it up locally.
>
>
> Suzuki
>
>
>> +{
>> + struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>> +
>> + drvdata->dsb_esize = 0;
>> + drvdata->cmb_esize = 0;
>> +}
>> +
>> +static void tpda_set_element_size(struct tpda_drvdata *drvdata, u32
>> *val)
>> +{
>> + /* Clear all relevant fields */
>> + *val &= ~(TPDA_Pn_CR_DSBSIZE | TPDA_Pn_CR_CMBSIZE);
>> +
>> + if (drvdata->dsb_esize == 64)
>> + *val |= TPDA_Pn_CR_DSBSIZE;
>> + else if (drvdata->dsb_esize == 32)
>> + *val &= ~TPDA_Pn_CR_DSBSIZE;
>> +
>> + if (drvdata->cmb_esize == 64)
>> + *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x2);
>> + else if (drvdata->cmb_esize == 32)
>> + *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x1);
>> + else if (drvdata->cmb_esize == 8)
>> + *val &= ~TPDA_Pn_CR_CMBSIZE;
>> +}
>> +
>> /*
>> - * Read the DSB element size from the TPDM device
>> + * Read the element size from the TPDM device. One TPDM must have at
>> least one of the
>> + * element size property.
>> * Returns
>> - * The dsb element size read from the devicetree if available.
>> - * 0 - Otherwise, with a warning once.
>> + * 0 - The element size property is read
>> + * Others - Cannot read the property of the element size
>> */
>> -static int tpdm_read_dsb_element_size(struct coresight_device *csdev)
>> +static int tpdm_read_element_size(struct tpda_drvdata *drvdata,
>> + struct coresight_device *csdev)
>> {
>> - int rc = 0;
>> - u8 size = 0;
>> + int rc = -EINVAL;
>> + struct tpdm_drvdata *tpdm_data = dev_get_drvdata(csdev->dev.parent);
>> +
>> + if (tpdm_has_dsb_dataset(tpdm_data)) {
>> + rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent),
>> + "qcom,dsb-element-size", &drvdata->dsb_esize);
>> + }
>> + if (tpdm_has_cmb_dataset(tpdm_data)) {
>> + rc = fwnode_property_read_u32(dev_fwnode(csdev->dev.parent),
>> + "qcom,cmb-element-bits", &drvdata->cmb_esize);
>> + }
>> - rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent),
>> - "qcom,dsb-element-size", &size);
>> if (rc)
>> dev_warn_once(&csdev->dev,
>> - "Failed to read TPDM DSB Element size: %d\n", rc);
>> + "Failed to read TPDM Element size: %d\n", rc);
>> - return size;
>> + return rc;
>> }
>> /*
>> @@ -56,11 +92,12 @@ static int tpdm_read_dsb_element_size(struct
>> coresight_device *csdev)
>> * Parameter "inport" is used to pass in the input port number
>> * of TPDA, and it is set to -1 in the recursize call.
>> */
>> -static int tpda_get_element_size(struct coresight_device *csdev,
>> +static int tpda_get_element_size(struct tpda_drvdata *drvdata,
>> + struct coresight_device *csdev,
>> int inport)
>> {
>> - int dsb_size = -ENOENT;
>> - int i, size;
>> + int rc = 0;
>> + int i;
>> struct coresight_device *in;
>> for (i = 0; i < csdev->pdata->nr_inconns; i++) {
>> @@ -69,30 +106,26 @@ static int tpda_get_element_size(struct
>> coresight_device *csdev,
>> continue;
>> /* Ignore the paths that do not match port */
>> - if (inport > 0 &&
>> + if (inport >= 0 &&
>> csdev->pdata->in_conns[i]->dest_port != inport)
>> continue;
>> if (coresight_device_is_tpdm(in)) {
>> - size = tpdm_read_dsb_element_size(in);
>> + if (drvdata->dsb_esize || drvdata->cmb_esize)
>> + return -EEXIST;
>> + rc = tpdm_read_element_size(drvdata, in);
>> + if (rc)
>> + return rc;
>> } else {
>> /* Recurse down the path */
>> - size = tpda_get_element_size(in, -1);
>> - }
>> -
>> - if (size < 0)
>> - return size;
>> -
>> - if (dsb_size < 0) {
>> - /* Found a size, save it. */
>> - dsb_size = size;
>> - } else {
>> - /* Found duplicate TPDMs */
>> - return -EEXIST;
>> + rc = tpda_get_element_size(drvdata, in, -1);
>> + if (rc)
>> + return rc;
>> }
>> }
>> - return dsb_size;
>> +
>> + return rc;
>> }
>> /* Settings pre enabling port control register */
>> @@ -109,7 +142,7 @@ static void tpda_enable_pre_port(struct
>> tpda_drvdata *drvdata)
>> static int tpda_enable_port(struct tpda_drvdata *drvdata, int port)
>> {
>> u32 val;
>> - int size;
>> + int rc;
>> val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
>> /*
>> @@ -117,29 +150,21 @@ static int tpda_enable_port(struct tpda_drvdata
>> *drvdata, int port)
>> * Set the bit to 0 if the size is 32
>> * Set the bit to 1 if the size is 64
>> */
The comment above is stale, you need to remove it. I noticed it
after I applied the series for my build tests.
Please could you respin the series with the two issues above fixed ?
Suzuki
>> - size = tpda_get_element_size(drvdata->csdev, port);
>> - switch (size) {
>> - case 32:
>> - val &= ~TPDA_Pn_CR_DSBSIZE;
>> - break;
>> - case 64:
>> - val |= TPDA_Pn_CR_DSBSIZE;
>> - break;
>> - case 0:
>> - return -EEXIST;
>> - case -EEXIST:
>> + tpdm_clear_element_size(drvdata->csdev);
>> + rc = tpda_get_element_size(drvdata, drvdata->csdev, port);
>> + if (!rc && (drvdata->dsb_esize || drvdata->cmb_esize)) {
>> + tpda_set_element_size(drvdata, &val);
>> + /* Enable the port */
>> + val |= TPDA_Pn_CR_ENA;
>> + writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
>> + } else if (rc == -EEXIST)
>> dev_warn_once(&drvdata->csdev->dev,
>> - "Detected multiple TPDMs on port %d", -EEXIST);
>> - return -EEXIST;
>> - default:
>> - return -EINVAL;
>> - }
>> -
>> - /* Enable the port */
>> - val |= TPDA_Pn_CR_ENA;
>> - writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
>> + "Detected multiple TPDMs on port %d", port);
>> + else
>> + dev_warn_once(&drvdata->csdev->dev,
>> + "Didn't find TPDM element size");
>> - return 0;
>> + return rc;
>> }
>> static int __tpda_enable(struct tpda_drvdata *drvdata, int port)
>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h
>> b/drivers/hwtracing/coresight/coresight-tpda.h
>> index b3b38fd41b64..19af64120fcf 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 CMB data set element size bit */
>> +#define TPDA_Pn_CR_CMBSIZE GENMASK(7, 6)
>> /* Aggregator port DSB data set element size bit */
>> #define TPDA_Pn_CR_DSBSIZE BIT(8)
>> @@ -25,6 +27,8 @@
>> * @csdev: component vitals needed by the framework.
>> * @spinlock: lock for the drvdata value.
>> * @enable: enable status of the component.
>> + * @dsb_esize Record the DSB element size.
>> + * @cmb_esize Record the CMB element size.
>> */
>> struct tpda_drvdata {
>> void __iomem *base;
>> @@ -32,6 +36,8 @@ struct tpda_drvdata {
>> struct coresight_device *csdev;
>> spinlock_t spinlock;
>> u8 atid;
>> + u8 dsb_esize;
>> + u32 cmb_esize;
>> };
>> #endif /* _CORESIGHT_CORESIGHT_TPDA_H */
>
On 2/2/2024 6:06 PM, Suzuki K Poulose wrote:
> On 02/02/2024 09:45, Suzuki K Poulose wrote:
>> On 02/02/2024 09:32, Tao Zhang wrote:
>>> Read the CMB element size from the device tree. Set the register
>>> bit that controls the CMB element size of the corresponding port.
>>>
>>> Reviewed-by: James Clark <james.clark@arm.com>
>>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>>> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
>>> ---
>>> drivers/hwtracing/coresight/coresight-tpda.c | 125
>>> +++++++++++--------
>>> drivers/hwtracing/coresight/coresight-tpda.h | 6 +
>>> 2 files changed, 81 insertions(+), 50 deletions(-)
>>>
>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.c
>>> b/drivers/hwtracing/coresight/coresight-tpda.c
>>> index 4ac954f4bc13..27d567f4c8bf 100644
>>> --- a/drivers/hwtracing/coresight/coresight-tpda.c
>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c
>>> @@ -18,6 +18,7 @@
>>> #include "coresight-priv.h"
>>> #include "coresight-tpda.h"
>>> #include "coresight-trace-id.h"
>>> +#include "coresight-tpdm.h"
>>> DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
>>> @@ -28,24 +29,59 @@ static bool coresight_device_is_tpdm(struct
>>> coresight_device *csdev)
>>> CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM);
>>> }
>>> +static void tpdm_clear_element_size(struct coresight_device *csdev)
>>
>> I just noticed this anomaly. This is supposed to be :
>>
>> tpda_clear_element_size() ? I can fix it up locally.
>>
>>
>> Suzuki
>>
>>
>>> +{
>>> + struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
>>> +
>>> + drvdata->dsb_esize = 0;
>>> + drvdata->cmb_esize = 0;
>>> +}
>>> +
>>> +static void tpda_set_element_size(struct tpda_drvdata *drvdata, u32
>>> *val)
>>> +{
>>> + /* Clear all relevant fields */
>>> + *val &= ~(TPDA_Pn_CR_DSBSIZE | TPDA_Pn_CR_CMBSIZE);
>>> +
>>> + if (drvdata->dsb_esize == 64)
>>> + *val |= TPDA_Pn_CR_DSBSIZE;
>>> + else if (drvdata->dsb_esize == 32)
>>> + *val &= ~TPDA_Pn_CR_DSBSIZE;
>>> +
>>> + if (drvdata->cmb_esize == 64)
>>> + *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x2);
>>> + else if (drvdata->cmb_esize == 32)
>>> + *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x1);
>>> + else if (drvdata->cmb_esize == 8)
>>> + *val &= ~TPDA_Pn_CR_CMBSIZE;
>>> +}
>>> +
>>> /*
>>> - * Read the DSB element size from the TPDM device
>>> + * Read the element size from the TPDM device. One TPDM must have
>>> at least one of the
>>> + * element size property.
>>> * Returns
>>> - * The dsb element size read from the devicetree if available.
>>> - * 0 - Otherwise, with a warning once.
>>> + * 0 - The element size property is read
>>> + * Others - Cannot read the property of the element size
>>> */
>>> -static int tpdm_read_dsb_element_size(struct coresight_device *csdev)
>>> +static int tpdm_read_element_size(struct tpda_drvdata *drvdata,
>>> + struct coresight_device *csdev)
>>> {
>>> - int rc = 0;
>>> - u8 size = 0;
>>> + int rc = -EINVAL;
>>> + struct tpdm_drvdata *tpdm_data =
>>> dev_get_drvdata(csdev->dev.parent);
>>> +
>>> + if (tpdm_has_dsb_dataset(tpdm_data)) {
>>> + rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent),
>>> + "qcom,dsb-element-size", &drvdata->dsb_esize);
>>> + }
>>> + if (tpdm_has_cmb_dataset(tpdm_data)) {
>>> + rc = fwnode_property_read_u32(dev_fwnode(csdev->dev.parent),
>>> + "qcom,cmb-element-bits", &drvdata->cmb_esize);
>>> + }
>>> - rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent),
>>> - "qcom,dsb-element-size", &size);
>>> if (rc)
>>> dev_warn_once(&csdev->dev,
>>> - "Failed to read TPDM DSB Element size: %d\n", rc);
>>> + "Failed to read TPDM Element size: %d\n", rc);
>>> - return size;
>>> + return rc;
>>> }
>>> /*
>>> @@ -56,11 +92,12 @@ static int tpdm_read_dsb_element_size(struct
>>> coresight_device *csdev)
>>> * Parameter "inport" is used to pass in the input port number
>>> * of TPDA, and it is set to -1 in the recursize call.
>>> */
>>> -static int tpda_get_element_size(struct coresight_device *csdev,
>>> +static int tpda_get_element_size(struct tpda_drvdata *drvdata,
>>> + struct coresight_device *csdev,
>>> int inport)
>>> {
>>> - int dsb_size = -ENOENT;
>>> - int i, size;
>>> + int rc = 0;
>>> + int i;
>>> struct coresight_device *in;
>>> for (i = 0; i < csdev->pdata->nr_inconns; i++) {
>>> @@ -69,30 +106,26 @@ static int tpda_get_element_size(struct
>>> coresight_device *csdev,
>>> continue;
>>> /* Ignore the paths that do not match port */
>>> - if (inport > 0 &&
>>> + if (inport >= 0 &&
>>> csdev->pdata->in_conns[i]->dest_port != inport)
>>> continue;
>>> if (coresight_device_is_tpdm(in)) {
>>> - size = tpdm_read_dsb_element_size(in);
>>> + if (drvdata->dsb_esize || drvdata->cmb_esize)
>>> + return -EEXIST;
>>> + rc = tpdm_read_element_size(drvdata, in);
>>> + if (rc)
>>> + return rc;
>>> } else {
>>> /* Recurse down the path */
>>> - size = tpda_get_element_size(in, -1);
>>> - }
>>> -
>>> - if (size < 0)
>>> - return size;
>>> -
>>> - if (dsb_size < 0) {
>>> - /* Found a size, save it. */
>>> - dsb_size = size;
>>> - } else {
>>> - /* Found duplicate TPDMs */
>>> - return -EEXIST;
>>> + rc = tpda_get_element_size(drvdata, in, -1);
>>> + if (rc)
>>> + return rc;
>>> }
>>> }
>>> - return dsb_size;
>>> +
>>> + return rc;
>>> }
>>> /* Settings pre enabling port control register */
>>> @@ -109,7 +142,7 @@ static void tpda_enable_pre_port(struct
>>> tpda_drvdata *drvdata)
>>> static int tpda_enable_port(struct tpda_drvdata *drvdata, int port)
>>> {
>>> u32 val;
>>> - int size;
>>> + int rc;
>>> val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
>>> /*
>>> @@ -117,29 +150,21 @@ static int tpda_enable_port(struct
>>> tpda_drvdata *drvdata, int port)
>>> * Set the bit to 0 if the size is 32
>>> * Set the bit to 1 if the size is 64
>>> */
>
> The comment above is stale, you need to remove it. I noticed it
> after I applied the series for my build tests.
>
> Please could you respin the series with the two issues above fixed ?
Sure, I will update them to the new patch series soon.
Best,
Tao
>
> Suzuki
>
>
>>> - size = tpda_get_element_size(drvdata->csdev, port);
>>> - switch (size) {
>>> - case 32:
>>> - val &= ~TPDA_Pn_CR_DSBSIZE;
>>> - break;
>>> - case 64:
>>> - val |= TPDA_Pn_CR_DSBSIZE;
>>> - break;
>>> - case 0:
>>> - return -EEXIST;
>>> - case -EEXIST:
>>> + tpdm_clear_element_size(drvdata->csdev);
>>> + rc = tpda_get_element_size(drvdata, drvdata->csdev, port);
>>> + if (!rc && (drvdata->dsb_esize || drvdata->cmb_esize)) {
>>> + tpda_set_element_size(drvdata, &val);
>>> + /* Enable the port */
>>> + val |= TPDA_Pn_CR_ENA;
>>> + writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
>>> + } else if (rc == -EEXIST)
>>> dev_warn_once(&drvdata->csdev->dev,
>>> - "Detected multiple TPDMs on port %d", -EEXIST);
>>> - return -EEXIST;
>>> - default:
>>> - return -EINVAL;
>>> - }
>>> -
>>> - /* Enable the port */
>>> - val |= TPDA_Pn_CR_ENA;
>>> - writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
>>> + "Detected multiple TPDMs on port %d", port);
>>> + else
>>> + dev_warn_once(&drvdata->csdev->dev,
>>> + "Didn't find TPDM element size");
>>> - return 0;
>>> + return rc;
>>> }
>>> static int __tpda_enable(struct tpda_drvdata *drvdata, int port)
>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h
>>> b/drivers/hwtracing/coresight/coresight-tpda.h
>>> index b3b38fd41b64..19af64120fcf 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 CMB data set element size bit */
>>> +#define TPDA_Pn_CR_CMBSIZE GENMASK(7, 6)
>>> /* Aggregator port DSB data set element size bit */
>>> #define TPDA_Pn_CR_DSBSIZE BIT(8)
>>> @@ -25,6 +27,8 @@
>>> * @csdev: component vitals needed by the framework.
>>> * @spinlock: lock for the drvdata value.
>>> * @enable: enable status of the component.
>>> + * @dsb_esize Record the DSB element size.
>>> + * @cmb_esize Record the CMB element size.
>>> */
>>> struct tpda_drvdata {
>>> void __iomem *base;
>>> @@ -32,6 +36,8 @@ struct tpda_drvdata {
>>> struct coresight_device *csdev;
>>> spinlock_t spinlock;
>>> u8 atid;
>>> + u8 dsb_esize;
>>> + u32 cmb_esize;
>>> };
>>> #endif /* _CORESIGHT_CORESIGHT_TPDA_H */
>>
>
@@ -18,6 +18,7 @@
#include "coresight-priv.h"
#include "coresight-tpda.h"
#include "coresight-trace-id.h"
+#include "coresight-tpdm.h"
DEFINE_CORESIGHT_DEVLIST(tpda_devs, "tpda");
@@ -28,24 +29,59 @@ static bool coresight_device_is_tpdm(struct coresight_device *csdev)
CORESIGHT_DEV_SUBTYPE_SOURCE_TPDM);
}
+static void tpdm_clear_element_size(struct coresight_device *csdev)
+{
+ struct tpda_drvdata *drvdata = dev_get_drvdata(csdev->dev.parent);
+
+ drvdata->dsb_esize = 0;
+ drvdata->cmb_esize = 0;
+}
+
+static void tpda_set_element_size(struct tpda_drvdata *drvdata, u32 *val)
+{
+ /* Clear all relevant fields */
+ *val &= ~(TPDA_Pn_CR_DSBSIZE | TPDA_Pn_CR_CMBSIZE);
+
+ if (drvdata->dsb_esize == 64)
+ *val |= TPDA_Pn_CR_DSBSIZE;
+ else if (drvdata->dsb_esize == 32)
+ *val &= ~TPDA_Pn_CR_DSBSIZE;
+
+ if (drvdata->cmb_esize == 64)
+ *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x2);
+ else if (drvdata->cmb_esize == 32)
+ *val |= FIELD_PREP(TPDA_Pn_CR_CMBSIZE, 0x1);
+ else if (drvdata->cmb_esize == 8)
+ *val &= ~TPDA_Pn_CR_CMBSIZE;
+}
+
/*
- * Read the DSB element size from the TPDM device
+ * Read the element size from the TPDM device. One TPDM must have at least one of the
+ * element size property.
* Returns
- * The dsb element size read from the devicetree if available.
- * 0 - Otherwise, with a warning once.
+ * 0 - The element size property is read
+ * Others - Cannot read the property of the element size
*/
-static int tpdm_read_dsb_element_size(struct coresight_device *csdev)
+static int tpdm_read_element_size(struct tpda_drvdata *drvdata,
+ struct coresight_device *csdev)
{
- int rc = 0;
- u8 size = 0;
+ int rc = -EINVAL;
+ struct tpdm_drvdata *tpdm_data = dev_get_drvdata(csdev->dev.parent);
+
+ if (tpdm_has_dsb_dataset(tpdm_data)) {
+ rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent),
+ "qcom,dsb-element-size", &drvdata->dsb_esize);
+ }
+ if (tpdm_has_cmb_dataset(tpdm_data)) {
+ rc = fwnode_property_read_u32(dev_fwnode(csdev->dev.parent),
+ "qcom,cmb-element-bits", &drvdata->cmb_esize);
+ }
- rc = fwnode_property_read_u8(dev_fwnode(csdev->dev.parent),
- "qcom,dsb-element-size", &size);
if (rc)
dev_warn_once(&csdev->dev,
- "Failed to read TPDM DSB Element size: %d\n", rc);
+ "Failed to read TPDM Element size: %d\n", rc);
- return size;
+ return rc;
}
/*
@@ -56,11 +92,12 @@ static int tpdm_read_dsb_element_size(struct coresight_device *csdev)
* Parameter "inport" is used to pass in the input port number
* of TPDA, and it is set to -1 in the recursize call.
*/
-static int tpda_get_element_size(struct coresight_device *csdev,
+static int tpda_get_element_size(struct tpda_drvdata *drvdata,
+ struct coresight_device *csdev,
int inport)
{
- int dsb_size = -ENOENT;
- int i, size;
+ int rc = 0;
+ int i;
struct coresight_device *in;
for (i = 0; i < csdev->pdata->nr_inconns; i++) {
@@ -69,30 +106,26 @@ static int tpda_get_element_size(struct coresight_device *csdev,
continue;
/* Ignore the paths that do not match port */
- if (inport > 0 &&
+ if (inport >= 0 &&
csdev->pdata->in_conns[i]->dest_port != inport)
continue;
if (coresight_device_is_tpdm(in)) {
- size = tpdm_read_dsb_element_size(in);
+ if (drvdata->dsb_esize || drvdata->cmb_esize)
+ return -EEXIST;
+ rc = tpdm_read_element_size(drvdata, in);
+ if (rc)
+ return rc;
} else {
/* Recurse down the path */
- size = tpda_get_element_size(in, -1);
- }
-
- if (size < 0)
- return size;
-
- if (dsb_size < 0) {
- /* Found a size, save it. */
- dsb_size = size;
- } else {
- /* Found duplicate TPDMs */
- return -EEXIST;
+ rc = tpda_get_element_size(drvdata, in, -1);
+ if (rc)
+ return rc;
}
}
- return dsb_size;
+
+ return rc;
}
/* Settings pre enabling port control register */
@@ -109,7 +142,7 @@ static void tpda_enable_pre_port(struct tpda_drvdata *drvdata)
static int tpda_enable_port(struct tpda_drvdata *drvdata, int port)
{
u32 val;
- int size;
+ int rc;
val = readl_relaxed(drvdata->base + TPDA_Pn_CR(port));
/*
@@ -117,29 +150,21 @@ static int tpda_enable_port(struct tpda_drvdata *drvdata, int port)
* Set the bit to 0 if the size is 32
* Set the bit to 1 if the size is 64
*/
- size = tpda_get_element_size(drvdata->csdev, port);
- switch (size) {
- case 32:
- val &= ~TPDA_Pn_CR_DSBSIZE;
- break;
- case 64:
- val |= TPDA_Pn_CR_DSBSIZE;
- break;
- case 0:
- return -EEXIST;
- case -EEXIST:
+ tpdm_clear_element_size(drvdata->csdev);
+ rc = tpda_get_element_size(drvdata, drvdata->csdev, port);
+ if (!rc && (drvdata->dsb_esize || drvdata->cmb_esize)) {
+ tpda_set_element_size(drvdata, &val);
+ /* Enable the port */
+ val |= TPDA_Pn_CR_ENA;
+ writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
+ } else if (rc == -EEXIST)
dev_warn_once(&drvdata->csdev->dev,
- "Detected multiple TPDMs on port %d", -EEXIST);
- return -EEXIST;
- default:
- return -EINVAL;
- }
-
- /* Enable the port */
- val |= TPDA_Pn_CR_ENA;
- writel_relaxed(val, drvdata->base + TPDA_Pn_CR(port));
+ "Detected multiple TPDMs on port %d", port);
+ else
+ dev_warn_once(&drvdata->csdev->dev,
+ "Didn't find TPDM element size");
- return 0;
+ return rc;
}
static int __tpda_enable(struct tpda_drvdata *drvdata, int port)
@@ -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 CMB data set element size bit */
+#define TPDA_Pn_CR_CMBSIZE GENMASK(7, 6)
/* Aggregator port DSB data set element size bit */
#define TPDA_Pn_CR_DSBSIZE BIT(8)
@@ -25,6 +27,8 @@
* @csdev: component vitals needed by the framework.
* @spinlock: lock for the drvdata value.
* @enable: enable status of the component.
+ * @dsb_esize Record the DSB element size.
+ * @cmb_esize Record the CMB element size.
*/
struct tpda_drvdata {
void __iomem *base;
@@ -32,6 +36,8 @@ struct tpda_drvdata {
struct coresight_device *csdev;
spinlock_t spinlock;
u8 atid;
+ u8 dsb_esize;
+ u32 cmb_esize;
};
#endif /* _CORESIGHT_CORESIGHT_TPDA_H */