Messages in this thread | | | Date | Tue, 6 Jun 2023 18:55:28 +0800 | Subject | Re: [PATCH v4 09/11] coresight-tpdm: Add nodes for timestamp request | From | Tao Zhang <> |
| |
On 6/5/2023 6:19 PM, Suzuki K Poulose wrote: > On 27/04/2023 10:00, 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 | 48 ++++++ >> drivers/hwtracing/coresight/coresight-tpdm.c | 182 >> ++++++++++++++++++++- >> drivers/hwtracing/coresight/coresight-tpdm.h | 14 ++ >> 3 files changed, 239 insertions(+), 5 deletions(-) >> >> diff --git >> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >> index c04c735..639b6fb8 100644 >> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >> @@ -122,3 +122,51 @@ Description: >> Where: >> <integer1> : Index number of XPMR register, the range is 0 >> to 7 >> <integer2> : The value need to be written >> + >> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_val >> +Date: March 2023 >> +KernelVersion 6.3 > > Similar to the previous comments, please update this. Sure, I will update this in the next patch series. > >> +Contact: Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang >> (QUIC) <quic_taozha@quicinc.com> >> +Description: >> + (Write) Set the pattern value of DSB tpdm. Read >> + the pattern value of DSB tpdm. >> + >> + Accepts the following two values. >> + value 1: Index number of TPR register >> + value 2: The value need to be written >> + >> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_mask >> +Date: March 2023 >> +KernelVersion 6.3 >> +Contact: Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang >> (QUIC) <quic_taozha@quicinc.com> >> +Description: >> + (Write) Set the pattern mask of DSB tpdm. Read >> + the pattern mask of DSB tpdm. >> + >> + Accepts the following two values. >> + value 1: Index number of TPMR register >> + value 2: The value need to be written >> + >> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_ts >> +Date: March 2023 >> +KernelVersion 6.3 >> +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.3 >> +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. >> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c >> b/drivers/hwtracing/coresight/coresight-tpdm.c >> index 9387bdf..627de36 100644 >> --- a/drivers/hwtracing/coresight/coresight-tpdm.c >> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c >> @@ -78,6 +78,27 @@ static void set_trigger_type(struct tpdm_drvdata >> *drvdata, u32 *val) >> *val &= ~TPDM_DSB_CR_TRIG_TYPE; >> } >> +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; >> @@ -90,6 +111,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_val[i], >> drvdata->base + TPDM_DSB_XPR(i)); >> writel_relaxed(drvdata->dsb->trig_patt_mask[i], >> @@ -97,11 +122,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); >> @@ -451,6 +472,153 @@ static ssize_t dsb_edge_ctrl_mask_store(struct >> device *dev, >> } >> static DEVICE_ATTR_RW(dsb_edge_ctrl_mask); >> +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; >> + int i = 0; >> + >> + spin_lock(&drvdata->spinlock); >> + for (i = 0; i < TPDM_DSB_MAX_PATT; i++) { >> + size += sysfs_emit_at(buf, size, >> + "Index: 0x%x Value: 0x%x\n", i, >> + drvdata->dsb->patt_val[i]); >> + } > > Similarly here, please check for overflows and stop in case. Sure, I will update this in the next patch series. > >> + spin_unlock(&drvdata->spinlock); >> + return size; >> +} >> + >> +/* >> + * value 1: Index of TPR register >> + * value 2: Value need to be written >> + */ >> +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 index, val; >> + >> + if (sscanf(buf, "%lx %lx", &index, &val) != 2) >> + return -EINVAL; >> + if (index >= TPDM_DSB_MAX_PATT) >> + return -EPERM; >> + >> + spin_lock(&drvdata->spinlock); >> + drvdata->dsb->patt_val[index] = val; >> + spin_unlock(&drvdata->spinlock); >> + return size; >> +} >> +static DEVICE_ATTR_RW(dsb_patt_val); >> + >> +static ssize_t dsb_patt_mask_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) >> +{ >> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); >> + ssize_t size = 0; >> + int i = 0; >> + >> + spin_lock(&drvdata->spinlock); >> + for (i = 0; i < TPDM_DSB_MAX_PATT; i++) { >> + size += sysfs_emit_at(buf, size, >> + "Index: 0x%x Value: 0x%x\n", i, >> + drvdata->dsb->patt_mask[i]); > > Same here Sure, I will update this in the next patch series. > >> + } >> + spin_unlock(&drvdata->spinlock); >> + return size; >> +} >> + >> +/* >> + * value 1: Index of TPMR register >> + * value 2: Value need to be written >> + */ >> +static ssize_t dsb_patt_mask_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, val; >> + >> + if (sscanf(buf, "%lx %lx", &index, &val) != 2) >> + return -EINVAL; >> + if (index >= TPDM_DSB_MAX_PATT) >> + return -EPERM; >> + >> + spin_lock(&drvdata->spinlock); >> + drvdata->dsb->patt_mask[index] = val; >> + spin_unlock(&drvdata->spinlock); >> + return size; >> +} >> +static DEVICE_ATTR_RW(dsb_patt_mask); >> + >> +static ssize_t dsb_patt_ts_show(struct device *dev, >> + struct device_attribute *attr, >> + char *buf) > > minor nit: alignment ?
Is there a criteria for the alignment?
I edit the file by Notepad++, and seems like these lines have been aligned.
> >> +{ >> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); >> + >> + return sysfs_emit(buf, "%u\n", >> + (unsigned int)drvdata->dsb->patt_ts); >> +} >> + >> +/* >> + * value 1: Enable/Disable DSB pattern timestamp >> + */ >> +static ssize_t dsb_patt_ts_store(struct device *dev, >> + struct device_attribute *attr, >> + const char *buf, >> + size_t size) > > minor nit: Alignmnet Same as my previous comment. > >> +{ >> + struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent); >> + unsigned long val; >> + >> + if ((kstrtoul(buf, 0, &val)) || (val & ~1UL)) >> + return -EINVAL; >> + >> + spin_lock(&drvdata->spinlock); >> + if (val) >> + drvdata->dsb->patt_ts = true; >> + else >> + drvdata->dsb->patt_ts = false; > > ultra minor nit: > > drvdata->dsb->patt_ts = !!val; > > >> + spin_unlock(&drvdata->spinlock); >> + return size; >> +} >> +static DEVICE_ATTR_RW(dsb_patt_ts); >> + >> +static ssize_t dsb_patt_type_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_type); >> +} >> + >> +/* >> + * value 1: Set DSB pattern type >> + */ >> +static ssize_t dsb_patt_type_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)) || (val & ~1UL)) >> + return -EINVAL; >> + >> + spin_lock(&drvdata->spinlock); >> + drvdata->dsb->patt_type = val; >> + spin_unlock(&drvdata->spinlock); >> + return size; >> +} >> +static DEVICE_ATTR_RW(dsb_patt_type); >> + >> static ssize_t dsb_trig_patt_val_show(struct device *dev, >> struct device_attribute *attr, >> char *buf) >> @@ -601,6 +769,10 @@ static struct attribute *tpdm_dsb_attrs[] = { >> &dev_attr_dsb_mode.attr, >> &dev_attr_dsb_edge_ctrl.attr, >> &dev_attr_dsb_edge_ctrl_mask.attr, >> + &dev_attr_dsb_patt_val.attr, >> + &dev_attr_dsb_patt_mask.attr, >> + &dev_attr_dsb_patt_ts.attr, >> + &dev_attr_dsb_patt_type.attr, >> &dev_attr_dsb_trig_patt_val.attr, >> &dev_attr_dsb_trig_patt_mask.attr, >> &dev_attr_dsb_trig_ts.attr, >> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h >> b/drivers/hwtracing/coresight/coresight-tpdm.h >> index 55c620f..9ad32a6 100644 >> --- a/drivers/hwtracing/coresight/coresight-tpdm.h >> +++ b/drivers/hwtracing/coresight/coresight-tpdm.h >> @@ -12,6 +12,8 @@ >> /* DSB Subunit Registers */ >> #define TPDM_DSB_CR (0x780) >> #define TPDM_DSB_TIER (0x784) >> +#define TPDM_DSB_TPR(n) (0x788 + (n * 4)) >> +#define TPDM_DSB_TPMR(n) (0x7A8 + (n * 4)) >> #define TPDM_DSB_XPR(n) (0x7C8 + (n * 4)) >> #define TPDM_DSB_XPMR(n) (0x7E8 + (n * 4)) >> #define TPDM_DSB_EDCR(n) (0x808 + (n * 4)) >> @@ -24,8 +26,12 @@ >> /* Enable bit for DSB subunit trigger type */ >> #define TPDM_DSB_CR_TRIG_TYPE BIT(12) >> +/* Enable bit for DSB subunit pattern timestamp */ >> +#define TPDM_DSB_TIER_PATT_TSENAB BIT(0) >> /* Enable bit for DSB subunit trigger timestamp */ >> #define TPDM_DSB_TIER_XTRIG_TSENAB BIT(1) >> +/* Bit for DSB subunit pattern type */ >> +#define TPDM_DSB_TIER_PATT_TYPE BIT(2) >> /* DSB programming modes */ >> /* Test mode control bit*/ >> @@ -86,6 +92,10 @@ >> * @mode: DSB programming mode >> * @edge_ctrl: Save value for edge control >> * @edge_ctrl_mask: Save value for edge control mask >> + * @patt_val: Save value for pattern >> + * @patt_mask: Save value for pattern mask >> + * @patt_ts: Enable/Disable pattern timestamp >> + * @patt_type: Set pattern type >> * @trig_patt_val: Save value for trigger pattern >> * @trig_patt_mask: Save value for trigger pattern mask >> * @trig_ts: Enable/Disable trigger timestamp. >> @@ -95,6 +105,10 @@ struct dsb_dataset { >> u32 mode; >> u32 edge_ctrl[TPDM_DSB_MAX_EDCR]; >> u32 edge_ctrl_mask[TPDM_DSB_MAX_EDCMR]; >> + u32 patt_val[TPDM_DSB_MAX_PATT]; >> + u32 patt_mask[TPDM_DSB_MAX_PATT]; >> + bool patt_ts; >> + bool patt_type; > > minor nit: Alignment of the bool fields ? > > Also, it may be good to move the bool fields together, for better > packing of the structure. Sure, I will update in the next patch series. > >> u32 trig_patt_val[TPDM_DSB_MAX_PATT]; >> u32 trig_patt_mask[TPDM_DSB_MAX_PATT]; >> bool trig_ts; > > Suzuki
| |