Messages in this thread | | | Date | Mon, 27 Mar 2023 16:37:24 +0800 | Subject | Re: [PATCH v3 09/11] coresight-tpdm: Add nodes for timestamp request | From | Tao Zhang <> |
| |
Hi Suzuki,
On 3/24/2023 2:41 AM, Suzuki K Poulose wrote: > On 23/03/2023 06:04, 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 | 172 >> +++++++++++++++++++++ >> drivers/hwtracing/coresight/coresight-tpdm.h | 14 ++ >> 3 files changed, 234 insertions(+) >> >> diff --git >> a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >> b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >> index c06374f..60ff660 100644 >> --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >> +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm >> @@ -95,3 +95,51 @@ Description: >> Accepts the following two values. >> value 1: Index number of XPMR register >> value 2: The value need to be written >> + >> +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_val >> +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 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 : Set the DSB pattern timestamp to false >> + 1 : Set the DSB pattern timestamp to true >> + >> +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. >> + > > Sounds a bit strange for "pattern type" to be bool. What does this do ? > Does it enable/disable something ?
I think there are two pattern types and controlled by 1 bit in the register. It doesn't
really make sense to use Boolean values. I will confirm this and update it in the next
patch series.
> >> + Accepts only one of the 2 values - 0 or 1. >> + 0 : Set the DSB pattern type to false >> + 1 : Set the DSB pattern type to true >> diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c >> b/drivers/hwtracing/coresight/coresight-tpdm.c >> index d6cc6b5..c740681 100644 >> --- a/drivers/hwtracing/coresight/coresight-tpdm.c >> +++ b/drivers/hwtracing/coresight/coresight-tpdm.c >> @@ -98,6 +98,13 @@ 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)); >> + } >> + >> + for (i = 0; i < TPDM_DSB_MAX_PATT; i++) { > > Why not do all the pattern related writing in one shot, instead > of two loops ? I will combine these two loops to one. > >> writel_relaxed(drvdata->dsb->trig_patt_val[i], >> drvdata->base + TPDM_DSB_XPR(i)); >> writel_relaxed(drvdata->dsb->trig_patt_mask[i], >> @@ -105,6 +112,16 @@ static void tpdm_enable_dsb(struct tpdm_drvdata >> *drvdata) >> } >> val = readl_relaxed(drvdata->base + TPDM_DSB_TIER); >> + /* 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_dsb_pattern_ts() in line with the other helper functions ? > Rest looks fine to me. > Sure, I will update this in the next patch series.
Tao
> Suzuki > > >> /* Set trigger timestamp */ >> if (drvdata->dsb->trig_ts) >> val |= TPDM_DSB_TIER_XTRIG_TSENAB; >> @@ -455,6 +472,157 @@ static ssize_t dsb_edge_ctrl_mask_store(struct >> device *dev, >> return size; >> } >> 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]); >> + } >> + 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]); >> + } >> + 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) >> +{ >> + 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) >> +{ >> + 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; >> + 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); >> + if (val) >> + drvdata->dsb->patt_type = true; >> + else >> + drvdata->dsb->patt_type = false; >> + 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) >> @@ -605,6 +773,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 2e8020e..f9d4dd9 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 */ >> #define TPDM_DSB_MODE_CYCACC(val) (val & GENMASK(2, 0)) >> @@ -76,6 +82,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. >> @@ -85,6 +95,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; >> u32 trig_patt_val[TPDM_DSB_MAX_PATT]; >> u32 trig_patt_mask[TPDM_DSB_MAX_PATT]; >> bool trig_ts; >
| |