Re: [PATCH v5 05/10] coresight-tpda: Add support to configure CMB element

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxx>
Signed-off-by: Tao Zhang <quic_taozha@xxxxxxxxxxx>
Signed-off-by: Mao Jinlong <quic_jinlmao@xxxxxxxxxxx>
---
  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







[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux