From: Jinlong Mao <quic_jinlmao@quicinc.com> To: Suzuki K Poulose <suzuki.poulose@arm.com>, Mathieu Poirier <mathieu.poirier@linaro.org>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Konrad Dybcio <konradybcio@gmail.com>, Mike Leach <mike.leach@linaro.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, <coresight@lists.linaro.org>, <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>, Tingwei Zhang <quic_tingweiz@quicinc.com>, Yuanfang Zhang <quic_yuanfang@quicinc.com>, Tao Zhang <quic_taozha@quicinc.com>, Trilok Soni <quic_tsoni@quicinc.com>, Hao Zhang <quic_hazha@quicinc.com>, <linux-arm-msm@vger.kernel.org>, Bjorn Andersson <bjorn.andersson@linaro.org> Subject: Re: [PATCH v7 05/10] coresight-tpdm: Add integration test support Date: Tue, 24 May 2022 15:14:38 +0800 [thread overview] Message-ID: <6f44aa9e-ba5e-3c94-501c-32e0b35ade56@quicinc.com> (raw) In-Reply-To: <7fd884ed-1255-5976-6fcc-7a19ad0eb04e@arm.com> Good afternoon. On 5/23/2022 5:17 PM, Suzuki K Poulose wrote: > 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 This is comment in V5 patch. There is no adverse effects for all tpdms with integration test enabled. So i removed this config. " 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 " > >> --- >> 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; This should be 0. I will update. > > 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 > > _______________________________________________ > CoreSight mailing list -- coresight@lists.linaro.org > To unsubscribe send an email to coresight-leave@lists.linaro.org
WARNING: multiple messages have this Message-ID (diff)
From: Jinlong Mao <quic_jinlmao@quicinc.com> To: Suzuki K Poulose <suzuki.poulose@arm.com>, Mathieu Poirier <mathieu.poirier@linaro.org>, Alexander Shishkin <alexander.shishkin@linux.intel.com>, Konrad Dybcio <konradybcio@gmail.com>, Mike Leach <mike.leach@linaro.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>, <coresight@lists.linaro.org>, <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>, Tingwei Zhang <quic_tingweiz@quicinc.com>, Yuanfang Zhang <quic_yuanfang@quicinc.com>, Tao Zhang <quic_taozha@quicinc.com>, Trilok Soni <quic_tsoni@quicinc.com>, Hao Zhang <quic_hazha@quicinc.com>, <linux-arm-msm@vger.kernel.org>, Bjorn Andersson <bjorn.andersson@linaro.org> Subject: Re: [PATCH v7 05/10] coresight-tpdm: Add integration test support Date: Tue, 24 May 2022 15:14:38 +0800 [thread overview] Message-ID: <6f44aa9e-ba5e-3c94-501c-32e0b35ade56@quicinc.com> (raw) In-Reply-To: <7fd884ed-1255-5976-6fcc-7a19ad0eb04e@arm.com> Good afternoon. On 5/23/2022 5:17 PM, Suzuki K Poulose wrote: > 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 This is comment in V5 patch. There is no adverse effects for all tpdms with integration test enabled. So i removed this config. " 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 " > >> --- >> 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; This should be 0. I will update. > > 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 > > _______________________________________________ > CoreSight mailing list -- coresight@lists.linaro.org > To unsubscribe send an email to coresight-leave@lists.linaro.org _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-05-24 7:14 UTC|newest] Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-05-09 13:39 [PATCH v7 00/10] Coresight: Add support for TPDM and TPDA Mao Jinlong 2022-05-09 13:39 ` Mao Jinlong 2022-05-09 13:39 ` [PATCH v7 01/10] coresight: core: Use IDR for non-cpu bound sources' paths Mao Jinlong 2022-05-09 13:39 ` Mao Jinlong 2022-05-09 13:39 ` [PATCH v7 02/10] Coresight: Add coresight TPDM source driver Mao Jinlong 2022-05-09 13:39 ` Mao Jinlong 2022-05-23 8:57 ` Suzuki K Poulose 2022-05-23 8:57 ` Suzuki K Poulose 2022-05-24 7:00 ` Jinlong Mao 2022-05-24 7:00 ` Jinlong Mao 2022-06-01 9:21 ` Jinlong Mao 2022-06-01 9:21 ` Jinlong Mao 2022-06-01 9:30 ` Suzuki K Poulose 2022-06-01 9:30 ` Suzuki K Poulose 2022-06-01 9:56 ` Jinlong Mao 2022-06-01 9:56 ` Jinlong Mao 2022-06-02 3:14 ` Jinlong Mao 2022-06-02 3:14 ` Jinlong Mao 2022-05-09 13:39 ` [PATCH v7 03/10] dt-bindings: arm: Adds CoreSight TPDM hardware definitions Mao Jinlong 2022-05-09 13:39 ` Mao Jinlong 2022-05-23 9:02 ` Suzuki K Poulose 2022-05-23 9:02 ` Suzuki K Poulose 2022-05-09 13:39 ` [PATCH v7 04/10] coresight-tpdm: Add DSB dataset support Mao Jinlong 2022-05-09 13:39 ` Mao Jinlong 2022-05-23 9:11 ` Suzuki K Poulose 2022-05-23 9:11 ` Suzuki K Poulose 2022-05-23 9:24 ` Suzuki K Poulose 2022-05-23 9:24 ` Suzuki K Poulose 2022-05-24 7:15 ` Jinlong Mao 2022-05-24 7:15 ` Jinlong Mao 2022-05-24 7:07 ` Jinlong Mao 2022-05-24 7:07 ` Jinlong Mao 2022-05-09 13:39 ` [PATCH v7 05/10] coresight-tpdm: Add integration test support Mao Jinlong 2022-05-09 13:39 ` Mao Jinlong 2022-05-23 9:17 ` Suzuki K Poulose 2022-05-23 9:17 ` Suzuki K Poulose 2022-05-24 7:14 ` Jinlong Mao [this message] 2022-05-24 7:14 ` Jinlong Mao 2022-05-09 13:39 ` [PATCH v7 06/10] docs: sysfs: coresight: Add sysfs ABI documentation for TPDM Mao Jinlong 2022-05-09 13:39 ` Mao Jinlong 2022-05-09 13:39 ` [PATCH v7 07/10] Coresight: Add TPDA link driver Mao Jinlong 2022-05-09 13:39 ` Mao Jinlong 2022-05-23 9:40 ` Suzuki K Poulose 2022-05-23 9:40 ` Suzuki K Poulose 2022-05-24 7:32 ` Jinlong Mao 2022-05-24 7:32 ` Jinlong Mao 2022-05-09 13:39 ` [PATCH v7 08/10] dt-bindings: arm: Adds CoreSight TPDA hardware definitions Mao Jinlong 2022-05-09 13:39 ` Mao Jinlong 2022-05-23 9:44 ` Suzuki K Poulose 2022-05-23 9:44 ` Suzuki K Poulose 2022-05-23 14:24 ` Rob Herring 2022-05-23 14:24 ` Rob Herring 2022-05-24 7:34 ` Jinlong Mao 2022-05-24 7:34 ` Jinlong Mao 2022-05-09 13:39 ` [PATCH v7 09/10] arm64: dts: qcom: sm8250: Add coresight components Mao Jinlong 2022-05-09 13:39 ` Mao Jinlong 2022-05-09 13:39 ` [PATCH v7 10/10] arm64: dts: qcom: sm8250: Add tpdm mm/prng Mao Jinlong 2022-05-09 13:39 ` Mao Jinlong
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=6f44aa9e-ba5e-3c94-501c-32e0b35ade56@quicinc.com \ --to=quic_jinlmao@quicinc.com \ --cc=alexander.shishkin@linux.intel.com \ --cc=bjorn.andersson@linaro.org \ --cc=coresight@lists.linaro.org \ --cc=gregkh@linuxfoundation.org \ --cc=konradybcio@gmail.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=mathieu.poirier@linaro.org \ --cc=mike.leach@linaro.org \ --cc=quic_hazha@quicinc.com \ --cc=quic_taozha@quicinc.com \ --cc=quic_tingweiz@quicinc.com \ --cc=quic_tsoni@quicinc.com \ --cc=quic_yuanfang@quicinc.com \ --cc=suzuki.poulose@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.