On 1/31/2024 6:02 PM, Suzuki K Poulose wrote:
On 31/01/2024 01:39, Tao Zhang wrote:
On 1/30/2024 8:35 PM, Suzuki K Poulose wrote:
On 30/01/2024 09:02, 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 | 123 +++++++++++-------- drivers/hwtracing/coresight/coresight-tpda.h | 6 + 2 files changed, 79 insertions(+), 50 deletions(-)
diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c index 4ac954f4bc13..fcddff3ded81 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,57 @@ 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) +{
+ if (drvdata->dsb_esize == 64) + *val |= TPDA_Pn_CR_DSBSIZE;
We don't seem to be clearing the fields we modify, before updating them. This may be OK in real world where the device connected to TPDA port may not change. But it is always safer to clear the bits and set it.
e.g.: *val &= ~(TPDA_Pn_CR_DSBSIZE | TPDA_Pn_CR_CMBSIZE);
+ 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);
Similarly here ^^^. I am happy to fix it up if you are OK with it (unless there are other changes that need a respin)
Thank you. I would be very grateful if you could help for this.
Given, you need to respin, please incorporate this change too.
Sure.
Is it OK if I modify the code as follow and update to this patch directly?
*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;
Best,
Tao
Suzuki