Re: [PATCH v5 05/10] coresight-tpdm: Add integration test support

[Jinlong Mao <quic_jinlmao@quicinc.com>]

Hi Mike,

Thank you for review.

On 4/13/2022 9:57 PM, Mike Leach wrote:
> Hi
>
> On Tue, 12 Apr 2022 at 13:51, Mao Jinlong <quic_jinlmao@quicinc.com> wrote:
>> Integration test for tpdm can help to generate the data for
>> verification of the topology during TPDM software bring up.
>>
>> Sample:
>> echo 1 > /sys/bus/coresight/devices/tmc_etf0/enable_sink
>> echo 1 > /sys/bus/coresight/devices/tpdm1/enable_source
>> echo 1 > /sys/bus/coresight/devices/tpdm1/integration_test
>> echo 2 > /sys/bus/coresight/devices/tpdm1/integration_test
>> cat /dev/tmc_etf0 > /data/etf-tpdm1.bin
>>
>> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
>> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
>> ---
>>   drivers/hwtracing/coresight/Kconfig          |  9 +++
>>   drivers/hwtracing/coresight/coresight-tpdm.c | 64 ++++++++++++++++++++
>>   drivers/hwtracing/coresight/coresight-tpdm.h | 14 +++++
>>   3 files changed, 87 insertions(+)
>>
>> diff --git a/drivers/hwtracing/coresight/Kconfig b/drivers/hwtracing/coresight/Kconfig
>> index 5c506a1cd08f..60248fef4089 100644
>> --- a/drivers/hwtracing/coresight/Kconfig
>> +++ b/drivers/hwtracing/coresight/Kconfig
>> @@ -214,4 +214,13 @@ config CORESIGHT_TPDM
>>            To compile this driver as a module, choose M here: the module will be
>>            called coresight-tpdm.
>>
>> +config CORESIGHT_TPDM_INTEGRATION_TEST
>> +       bool "Enable CoreSight Integration Test For TPDM"
>> +       depends on CORESIGHT_TPDM
>> +       help
>> +         This option adds support for the CoreSight integration test on this
>> +         devie. Coresight architecture provides integration control modes of
>> +         operation to facilitate integration testing and software bringup
>> +         and/or to instrument topology discovery. The TPDM utilizes integration
>> +         mode to accomplish integration testing and software bringup.
>>   endif
> For the last patchset you mentioned that you were making this
> configurable because the CTI intgration tests were also configurable.
> The reason that the CTI intergration test registers were done in this
> way is that some of the CoreSight components were not guaranteed to
> return to a usable state once integration test was disabled.
> Thus after use of the integration test, a complete board reset was
> recommended. Therefore we ensured that these features would only be
> used by those specifically configuring them and who were hopefully
> aware of the potentail limitations
>
> If your hardware can reliably enable and disable integration test
> without adverse effects, then you may wish to consider making the
> integration test a permanent feature of the driver.
>
> Regards
>
> Mike
TPDM has integration mode control register. The register can control the 
component to enable/disable integration mode.
There is no adverse effects which we have do many tests with integration 
mode for TPDM.
I will remove this config.

Thanks

Jinlong Mao

>
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
>> index d7b970cdcf51..14bccbff467d 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
>> @@ -126,6 +126,69 @@ static void tpdm_init_default_data(struct tpdm_drvdata *drvdata)
>>          CS_LOCK(drvdata->base);
>>   }
>>
>> +/*
>> + * Define CONFIG_CORESIGHT_TPDM_INTEGRATION_TEST to enable
>> + * integration_test sysfs nodes. It will help to generate
>> + * tpdm data to make sure that the trace path is enabled
>> + * and the funnel configurations are fine.
>> + */
>> +#ifdef CONFIG_CORESIGHT_TPDM_INTEGRATION_TEST
>> +/*
>> + * value 1: 64 bits test data
>> + * value 2: 32 bits test data
>> + */
>> +static ssize_t integration_test_store(struct device *dev,
>> +                                         struct device_attribute *attr,
>> +                                         const char *buf,
>> +                                         size_t size)
>> +{
>> +       int i, ret = 0;
>> +       unsigned long val;
>> +       struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
>> +
>> +       ret = kstrtoul(buf, 10, &val);
>> +       if (ret)
>> +               return ret;
>> +
>> +       if (val != 1 && val != 2)
>> +               return -EINVAL;
>> +
>> +       if (!drvdata->enable)
>> +               return -EINVAL;
>> +
>> +       if (val == 1)
>> +               val = ATBCNTRL_VAL_64;
>> +       else
>> +               val = ATBCNTRL_VAL_32;
>> +       CS_UNLOCK(drvdata->base);
>> +       writel_relaxed(0x1, drvdata->base + TPDM_ITCNTRL);
>> +
>> +       for (i = 1; i < INTEGRATION_TEST_CYCLE; i++)
>> +               writel_relaxed(val, drvdata->base + TPDM_ITATBCNTRL);
>> +
>> +       writel_relaxed(0, drvdata->base + TPDM_ITCNTRL);
>> +       CS_LOCK(drvdata->base);
>> +       return size;
>> +}
>> +static DEVICE_ATTR_WO(integration_test);
>> +#endif /* CORESIGHT_TPDM_INTEGRATION_TEST */
>> +
>> +static struct attribute *tpdm_attrs[] = {
>> +#ifdef CONFIG_CORESIGHT_TPDM_INTEGRATION_TEST
>> +       &dev_attr_integration_test.attr,
>> +#endif /* CORESIGHT_TPDM_INTEGRATION_TEST */
>> +       NULL,
>> +};
>> +
>> +static struct attribute_group tpdm_attr_grp = {
>> +       .attrs = tpdm_attrs,
>> +};
>> +
>> +static const struct attribute_group *tpdm_attr_grps[] = {
>> +       &tpdm_attr_grp,
>> +       NULL,
>> +};
>> +
>>   static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
>>   {
>>          struct device *dev = &adev->dev;
>> @@ -160,6 +223,7 @@ static int tpdm_probe(struct amba_device *adev, const struct amba_id *id)
>>          desc.ops = &tpdm_cs_ops;
>>          desc.pdata = adev->dev.platform_data;
>>          desc.dev = &adev->dev;
>> +       desc.groups = tpdm_attr_grps;
>>          drvdata->csdev = coresight_register(&desc);
>>          if (IS_ERR(drvdata->csdev))
>>                  return PTR_ERR(drvdata->csdev);
>> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h
>> index 8f05070879c4..ea457ba5434e 100644
>> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
>> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
>> @@ -12,6 +12,20 @@
>>   /* DSB Subunit Registers */
>>   #define TPDM_DSB_CR            (0x780)
>>
>> +/* TPDM integration test registers */
>> +#define TPDM_ITATBCNTRL                (0xEF0)
>> +#define TPDM_ITCNTRL           (0xF00)
>> +
>> +/* Register value for integration test */
>> +#define ATBCNTRL_VAL_32                0xC00F1409
>> +#define ATBCNTRL_VAL_64                0xC01F1409
>> +
>> +/*
>> + * Number of cycles to write value when
>> + * integration test.
>> + */
>> +#define INTEGRATION_TEST_CYCLE 10
>> +
>>   /**
>>    * This enum is for PERIPHIDR0 register of TPDM.
>>    * The fields [6:0] of PERIPHIDR0 are used to determine what
>> --
>> 2.17.1
>>
>