Re: [PATCH v7 11/13] coresight-tpdm: Add nodes for timestamp request

On 25/07/2023 08:15, Tao Zhang wrote:

Add nodes to configure the timestamp request based on input pattern match. Each TPDM that support DSB subunit has maximum of n(n<7) TPR registers to configure value for timestamp request based on input pattern match. Eight 32 bit registers providing DSB interface timestamp request pattern match comparison. And each TPDM that support DSB subunit has maximum of m(m<7) TPMR registers to configure pattern mask for timestamp request. Eight 32 bit registers providing DSB interface timestamp request pattern match mask generation. Add nodes to enable/disable pattern timestamp and set pattern timestamp type.

Signed-off-by: Tao Zhang quic_taozha@quicinc.com

.../ABI/testing/sysfs-bus-coresight-devices-tpdm | 57 +++++- drivers/hwtracing/coresight/coresight-tpdm.c | 205 ++++++++++++++++++++- drivers/hwtracing/coresight/coresight-tpdm.h | 16 ++ 3 files changed, 272 insertions(+), 6 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm index 66f9582..74a0126 100644 --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm @@ -130,4 +130,59 @@ Description: to the index number. Before writing data to this sysfs file, "dsb_trig_patt_idx" should be written first to configure the index number of the trigger pattern mask which needs to be

• configured.
  

\ No newline at end of file

• configured.
  

+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_idx +Date: March 2023 +KernelVersion 6.5 +Contact: Jinlong Mao (QUIC) quic_jinlmao@quicinc.com, Tao Zhang (QUIC) quic_taozha@quicinc.com +Description:

• Read/Write the index number of the DSB tpdm pattern. Since
  

• there are at most 8 TPR and TPMR registers for the parttern,
  

• this value ranges from 0 to 7.
  

If it is only 0-7 and we read/write full registers, why not expose individual registers as mentioned in the previous comments and avoid multi-line output for "show" routines. You may even group the registers under a directory structure to avoid confusion.

e.g, :

dsb_patt/ - mask0 ... mask7 - ts0 ... ts7 - type0 ... type7

dsb_edge/ - edcr0 ... edcr15 - edcmr0 ... edcmr7 - ctrl_idx - ctrl_idx_val ....

+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_val +Date: March 2023 +KernelVersion 6.5 +Contact: Jinlong Mao (QUIC) quic_jinlmao@quicinc.com, Tao Zhang (QUIC) quic_taozha@quicinc.com +Description:

• Read a set of the pattern values of the DSB TPDM. Write a data
  

• to configure the pattern corresponding to the index number.
  

• Before writing data to this sysfs file, "dsb_patt_idx" should be
  

• written first to configure the index number of the pattern which
  

• needs to be configured.
  

+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_mask +Date: March 2023 +KernelVersion 6.5 +Contact: Jinlong Mao (QUIC) quic_jinlmao@quicinc.com, Tao Zhang (QUIC) quic_taozha@quicinc.com +Description:

• Read a set of the pattern mask values of the DSB TPDM. Write a
  

• data to configure the pattern mask corresponding to the index
  

• number. Before writing data to this sysfs file, "dsb_patt_idx"
  

• should be written first to configure the index number of the
  

• pattern mask which needs to be configured.
  

+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_ts +Date: March 2023 +KernelVersion 6.5 +Contact: Jinlong Mao (QUIC) quic_jinlmao@quicinc.com, Tao Zhang (QUIC) quic_taozha@quicinc.com +Description:

• (Write) Set the pattern timestamp of DSB tpdm. Read
  

• the pattern timestamp of DSB tpdm.
  

• Accepts only one of the 2 values -  0 or 1.
  

• 0 : Disable DSB pattern timestamp.
  

• 1 : Enable DSB pattern timestamp.
  

+What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_type +Date: March 2023 +KernelVersion 6.5 +Contact: Jinlong Mao (QUIC) quic_jinlmao@quicinc.com, Tao Zhang (QUIC) quic_taozha@quicinc.com +Description:

• (Write) Set the pattern type of DSB tpdm. Read
  

• the pattern type of DSB tpdm.
  

• Accepts only one of the 2 values -  0 or 1.
  

• 0 : Set the DSB pattern type to value.
  

• 1 : Set the DSB pattern type to toggle.
  

\ No newline at end of file diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c index 1c32d27..f9e5a1d 100644 --- a/drivers/hwtracing/coresight/coresight-tpdm.c +++ b/drivers/hwtracing/coresight/coresight-tpdm.c @@ -69,6 +69,27 @@ static void set_dsb_perf_mode(struct tpdm_drvdata *drvdata, u32 *val) *val &= ~TPDM_DSB_CR_MODE; } +static void set_dsb_tier(struct tpdm_drvdata *drvdata, u32 *val) +{

• /* Set pattern timestamp type and enablement */
• if (drvdata->dsb->patt_ts) {

• *val |= TPDM_DSB_TIER_PATT_TSENAB;
  

• if (drvdata->dsb->patt_type)
  

• 	*val |= TPDM_DSB_TIER_PATT_TYPE;
  

• else
  

• 	*val &= ~TPDM_DSB_TIER_PATT_TYPE;
  

• } else {

• *val &= ~TPDM_DSB_TIER_PATT_TSENAB;
  

• }
• /* Set trigger timestamp */
• if (drvdata->dsb->trig_ts)

• *val |= TPDM_DSB_TIER_XTRIG_TSENAB;
  

• else

• *val &= ~TPDM_DSB_TIER_XTRIG_TSENAB;
  

+}

• static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) { u32 val, i;

@@ -81,6 +102,10 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) drvdata->base + TPDM_DSB_EDCMR(i)); for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {

• writel_relaxed(drvdata->dsb->patt_val[i],
  

• 	    drvdata->base + TPDM_DSB_TPR(i));
  

• writel_relaxed(drvdata->dsb->patt_mask[i],
  

• 	    drvdata->base + TPDM_DSB_TPMR(i));
  

  writel_relaxed(drvdata->dsb->trig_patt[i], drvdata->base + TPDM_DSB_XPR(i)); writel_relaxed(drvdata->dsb->trig_patt_mask[i],

@@ -88,11 +113,7 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) } val = readl_relaxed(drvdata->base + TPDM_DSB_TIER);

• /* Set trigger timestamp */
• if (drvdata->dsb->trig_ts)

• val |= TPDM_DSB_TIER_XTRIG_TSENAB;
  

• else

• val &= ~TPDM_DSB_TIER_XTRIG_TSENAB;
  

• set_dsb_tier(drvdata, &val); writel_relaxed(val, drvdata->base + TPDM_DSB_TIER);

val = readl_relaxed(drvdata->base + TPDM_DSB_CR); @@ -462,6 +483,175 @@ static ssize_t dsb_edge_ctrl_mask_store(struct device *dev, } static DEVICE_ATTR_RW(dsb_edge_ctrl_mask); +static ssize_t dsb_patt_idx_show(struct device *dev,

• 		struct device_attribute *attr,
  

• 		char *buf)
  

+{

• struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
• return sysfs_emit(buf, "%u\n",

• 	 (unsigned int)drvdata->dsb->patt_idx);
  

+}

+static ssize_t dsb_patt_idx_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 index;
• if ((kstrtoul(buf, 0, &index)) || (index >= TPDM_DSB_MAX_PATT))

• return -EINVAL;
  

• spin_lock(&drvdata->spinlock);
• drvdata->dsb->patt_idx = index;
• spin_unlock(&drvdata->spinlock);
• return size;

+} +static DEVICE_ATTR_RW(dsb_patt_idx);

+static ssize_t dsb_patt_val_show(struct device *dev,

• 		struct device_attribute *attr,
  

• 		char *buf)
  

+{

• struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
• ssize_t size = 0;
• unsigned long bytes;
• int i = 0;
• spin_lock(&drvdata->spinlock);
• for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {

• bytes = sysfs_emit_at(buf, size,
  

• 		  "Value: 0x%x\n", drvdata->dsb->patt_val[i]);
  

Please see my comment on the other patches.

• if (bytes <= 0)
  

• 	break;
  

• size += bytes;
  

• }
• spin_unlock(&drvdata->spinlock);
• return size;

+}

+static ssize_t dsb_patt_val_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, 0, &val))

You have used the base for all such conversions inconsistently, throughout the series. You can always stick to 0.

$ git grep kstrtoul drivers/hwtracing/coresight/coresight-tpdm.c drivers/hwtracing/coresight/coresight-tpdm.c: ret = kstrtoul(buf, 10, &val); drivers/hwtracing/coresight/coresight-tpdm.c: ret = kstrtoul(buf, 10, &val); drivers/hwtracing/coresight/coresight-tpdm.c: if ((kstrtoul(buf, 0, &val)) || val < 0) drivers/hwtracing/coresight/coresight-tpdm.c: if ((kstrtoul(buf, 0, &val)) || (val >= TPDM_DSB_MAX_LINES)) drivers/hwtracing/coresight/coresight-tpdm.c: if ((kstrtoul(buf, 0, &edge_ctrl)) || (edge_ctrl > 0x2)) drivers/hwtracing/coresight/coresight-tpdm.c: if ((kstrtoul(buf, 0, &val)) || (val & ~1UL)) drivers/hwtracing/coresight/coresight-tpdm.c: if ((kstrtoul(buf, 0, &val)) || (val & ~1UL)) drivers/hwtracing/coresight/coresight-tpdm.c: if ((kstrtoul(buf, 0, &val)) || (val & ~1UL))

Suzuki