Hi Suzuki,
在 10/24/2022 10:49 PM, Suzuki K Poulose 写道:
On 08/09/2022 09:45, Tao Zhang wrote:
Add node to set and show programming mode for TPDM DSB subunit. Once the DSB programming mode is set, it will be written to the register DSB_CR. Bit[10:9] of the DSB_CR register is used to set the DSB test mode.
Signed-off-by: Tao Zhang quic_taozha@quicinc.com
drivers/hwtracing/coresight/coresight-tpdm.c | 49 +++++++++++++++++++++++++++- drivers/hwtracing/coresight/coresight-tpdm.h | 10 ++++++ 2 files changed, 58 insertions(+), 1 deletion(-)
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c index fae9963..7265793 100644 --- a/drivers/hwtracing/coresight/coresight-tpdm.c +++ b/drivers/hwtracing/coresight/coresight-tpdm.c @@ -22,7 +22,7 @@ DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm"); static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) { - u32 val; + u32 val, mode; val = readl_relaxed(drvdata->base + TPDM_DSB_TIER); /* Set trigger timestamp */ @@ -42,6 +42,19 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) /* Set the enable bit of DSB control register to 1 */ val = readl_relaxed(drvdata->base + TPDM_DSB_CR); + /* Set the cycle accurate mode */ + mode = TPDM_DSB_MODE_CYCACC(drvdata->dsb->mode); + val = val & ~(0x7 << 9); + val = val | (mode << 9);
Please do not hard code numbers like that above. Please could you define proper masks for the fields in DSB_CR and use FIELD_GET, FIELD_PREP for setting the values.
Sure, I will update this in the next patch series.
+ /* Set the byte lane for high-performance mode */ + mode = TPDM_DSB_MODE_HPBYTESEL(drvdata->dsb->mode); + val = val & ~(0x1F << 2); + val = val | (mode << 2); + /* Set the performance mode */
Same as above.
Sure, I will update this in the next patch series.
+ if (drvdata->dsb->mode & TPDM_DSB_MODE_PERF) + val |= TPDM_DSB_MODE; + else + val &= ~TPDM_DSB_MODE; val |= TPDM_DSB_CR_ENA; writel_relaxed(val, drvdata->base + TPDM_DSB_CR); } @@ -232,6 +245,39 @@ static struct attribute_group tpdm_attr_grp = { .attrs = tpdm_attrs, }; +static ssize_t dsb_mode_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
+ if (!(drvdata->datasets & TPDM_PIDR0_DS_DSB)) + return -EPERM;
As mentioned earlier, use is_visble() instead of hard coding this in every function.
Sure, I will update this in the next patch series.
+ return scnprintf(buf, PAGE_SIZE, "%lx\n", + (unsigned long)drvdata->dsb->mode); +}
+static ssize_t dsb_mode_store(struct device *dev, + struct device_attribute *attr, + const char *buf, + size_t size) +{ + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); + unsigned long val;
+ if ((kstrtoul(buf, 16, &val)) || val < 0 || val > 3)
Why not leave kstrtoul to detect the base instead of always forcing 16 ? Some other code had forced to base of 10. Please be flexible and avoid the inconsistencies.
Sure, I will update this in the next patch series.
+ return -EINVAL; + if (!(drvdata->datasets & TPDM_PIDR0_DS_DSB)) + return -EPERM;
+ spin_lock(&drvdata->spinlock); + drvdata->dsb->mode = val & TPDM_MODE_ALL; + spin_unlock(&drvdata->spinlock); + return size; +} +static DEVICE_ATTR_RW(dsb_mode);
static ssize_t dsb_trig_type_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -312,6 +358,7 @@ static ssize_t dsb_trig_ts_store(struct device *dev, } static DEVICE_ATTR_RW(dsb_trig_ts); static struct attribute *tpdm_dsb_attrs[] = { + &dev_attr_dsb_mode.attr, &dev_attr_dsb_trig_ts.attr, &dev_attr_dsb_trig_type.attr, NULL, diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h index dd4a013..4d57488 100644 --- a/drivers/hwtracing/coresight/coresight-tpdm.h +++ b/drivers/hwtracing/coresight/coresight-tpdm.h @@ -19,6 +19,14 @@ #define TPDM_DSB_XTRIG_TSENAB BIT(1) /* Enable bit for DSB subunit trigger type */ #define TPDM_DSB_TRIG_TYPE BIT(12) +/* Enable bit for DSB subunit perfmance mode */ +#define TPDM_DSB_MODE BIT(1)
+/* DSB programming modes */ +#define TPDM_DSB_MODE_CYCACC(val) BMVAL(val, 0, 2) +#define TPDM_DSB_MODE_PERF BIT(3) +#define TPDM_DSB_MODE_HPBYTESEL(val) BMVAL(val, 4, 8)
Please could we use GENMASK to define the masks and use FIELD_GET/FILED_PREP macros for the dealing with the fields ?
Similarly for the DSB_CR register please.
Sure, I will update this in the next patch series.
Suzuki
Best Regards Tao