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

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 09/05/2022 14:39, Mao Jinlong 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/tpdm0/enable_source
> echo 1 > /sys/bus/coresight/devices/tpdm0/integration_test
> echo 2 > /sys/bus/coresight/devices/tpdm0/integration_test
> cat /dev/tmc_etf0 > /data/etf-tpdm0.bin
> 
> Signed-off-by: Tao Zhang <quic_taozha@quicinc.com>
> Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>

Please could we stick this under a sub-Kconfig entry, like
we did for the CTI ?

CONFIG_CORESIGHT_TPMD_INTEGRATION_TEST

> ---
>   drivers/hwtracing/coresight/coresight-tpdm.c | 54 ++++++++++++++++++++
>   drivers/hwtracing/coresight/coresight-tpdm.h | 14 +++++
>   2 files changed, 68 insertions(+)
> 
> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c
> index 70df888ac565..57e38aa7d2bd 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> @@ -123,6 +123,59 @@ static void tpdm_init_default_data(struct tpdm_drvdata *drvdata)
>   	CS_LOCK(drvdata->base);
>   }
>   
> +/*
> + * 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++)

super minor nit : It is a bit un-natural, not to have i = 0;

Rest looks fine to me .

Suzuki


> +		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);
> +
> +static struct attribute *tpdm_attrs[] = {
> +	&dev_attr_integration_test.attr,
> +	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;
> @@ -157,6 +210,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 f95aaad9c653..4aa880794383 100644
> --- a/drivers/hwtracing/coresight/coresight-tpdm.h
> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
> @@ -14,6 +14,20 @@
>   /* Enable bit for DSB subunit */
>   #define TPDM_DSB_CR_ENA		BIT(0)
>   
> +/* 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