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

[Mike Leach <mike.leach@linaro.org>]

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

> 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
>


-- 
Mike Leach
Principal Engineer, ARM Ltd.
Manchester Design Centre. UK