Messages in this thread Patch in this message | | | Date | Wed, 6 Sep 2023 10:09:38 +0100 | Subject | Re: [PATCH v8 11/13] coresight-tpdm: Add nodes for timestamp request | From | Suzuki K Poulose <> |
| |
On 22/08/2023 06:26, 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 | 40 ++++++- > drivers/hwtracing/coresight/coresight-tpdm.c | 133 ++++++++++++++++++++- > drivers/hwtracing/coresight/coresight-tpdm.h | 24 ++++ > 3 files changed, 191 insertions(+), 6 deletions(-) > > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm > index f5cd302..46a5535 100644 > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm > @@ -123,4 +123,42 @@ KernelVersion 6.5 > Contact: Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com> > Description: > (RW) Set/Get the mask of the trigger pattern for the DSB > - subunit TPDM. > \ No newline at end of file > + subunit TPDM. > + > +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt/tpr[0:7] > +Date: March 2023 > +KernelVersion 6.5 > +Contact: Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com> > +Description: > + (RW) Set/Get the value of the pattern for the DSB subunit TPDM. > + > +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt/tpmr[0:7] > +Date: March 2023 > +KernelVersion 6.5 > +Contact: Jinlong Mao (QUIC) <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_taozha@quicinc.com> > +Description: > + (RW) Set/Get the mask of the pattern for the DSB subunit TPDM. > + > +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_patt_ts
Given we have a dedicated "group" for dsb_patt, could we move this to dsb_patt and name this "enable_timestamp"
i.e.,
tpdm-name/dsb_patt/enable_timestamp
> +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
Similarly here.
> +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. > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c > index 6521019..9b0e060 100644 > --- a/drivers/hwtracing/coresight/coresight-tpdm.c > +++ b/drivers/hwtracing/coresight/coresight-tpdm.c > @@ -45,6 +45,12 @@ static ssize_t tpdm_simple_dataset_show(struct device *dev, > case DSB_TRIG_PATT_MASK: > return sysfs_emit(buf, "0x%x\n", > drvdata->dsb->trig_patt_mask[tpdm_attr->idx]); > + case DSB_PATT: > + return sysfs_emit(buf, "0x%x\n", > + drvdata->dsb->patt_val[tpdm_attr->idx]); > + case DSB_PATT_MASK: > + return sysfs_emit(buf, "0x%x\n", > + drvdata->dsb->patt_mask[tpdm_attr->idx]); > default: > return -EINVAL; > } > @@ -72,6 +78,12 @@ static ssize_t tpdm_simple_dataset_store(struct device *dev, > case DSB_TRIG_PATT_MASK: > drvdata->dsb->trig_patt_mask[tpdm_attr->idx] = val; > break; > + case DSB_PATT: > + drvdata->dsb->patt_val[tpdm_attr->idx] = val; > + break; > + case DSB_PATT_MASK: > + drvdata->dsb->patt_mask[tpdm_attr->idx] = val; > + break; > default: > spin_unlock(&drvdata->spinlock); > return -EINVAL; > @@ -129,6 +141,27 @@ static void set_dsb_mode(struct tpdm_drvdata *drvdata, u32 *val) > *val &= ~TPDM_DSB_CR_MODE; > } > > +static void set_dsb_tier(struct tpdm_drvdata *drvdata, u32 *val) > +{
Could we not Write to the DSB_TIER register from this function ? There are no other users of this function and keeping the read and write operations in the caller doesn't make much sense.
> + /* 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;
minor nit: Does it make sense to clear everything in one shot and set the required fields based on the fields ? That makes it a bit more reader friendly.
{ u32 val = readl_relaxed(drvdata.., TPDM_DSB_TIER);
/* Clear all relevant fields */ val &= ~(TPDM_DSB_TIER_PATT_TSENAB | TPDM_DSB_TIER_PATT_TYPE |\ TPDM_DSB_TIER_XTRIG_TSENAB)
/* Set the required fields */ if (drvdata->dsb->patt_ts) { val |= TPDM_DSB_TIER_PATT_TSENAB; if (drvdata->dsb->patt_type) val |= TPDM_DSB_TIER_PATT_TYPE; }
if (drvdata->dsb->trig_ts) val |= TPDM_DSB_TIER_XTRIG_TSENAB;
writel_relaxed(val, ... TPDM_DSB_TIER); }
> + > +} > + > static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) > { > u32 val, i; > @@ -140,17 +173,17 @@ static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) > writel_relaxed(drvdata->dsb->edge_ctrl_mask[i], > 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], > drvdata->base + TPDM_DSB_XPMR(i)); > } > 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);
See above
> > val = readl_relaxed(drvdata->base + TPDM_DSB_CR); > @@ -471,6 +504,67 @@ static ssize_t ctrl_mask_store(struct device *dev, > } > static DEVICE_ATTR_WO(ctrl_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); > + 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_type_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > @@ -593,8 +687,30 @@ static struct attribute *tpdm_dsb_trig_patt_attrs[] = { > NULL, > }; > > +static struct attribute *tpdm_dsb_patt_attrs[] = { > + DSB_PATT_ATTR(0), > + DSB_PATT_ATTR(1), > + DSB_PATT_ATTR(2), > + DSB_PATT_ATTR(3), > + DSB_PATT_ATTR(4), > + DSB_PATT_ATTR(5), > + DSB_PATT_ATTR(6), > + DSB_PATT_ATTR(7), > + DSB_PATT_MASK_ATTR(0), > + DSB_PATT_MASK_ATTR(1), > + DSB_PATT_MASK_ATTR(2), > + DSB_PATT_MASK_ATTR(3), > + DSB_PATT_MASK_ATTR(4), > + DSB_PATT_MASK_ATTR(5), > + DSB_PATT_MASK_ATTR(6), > + DSB_PATT_MASK_ATTR(7), > + NULL, > +}; > + > static struct attribute *tpdm_dsb_attrs[] = { > &dev_attr_dsb_mode.attr,
> + &dev_attr_dsb_patt_ts.attr, > + &dev_attr_dsb_patt_type.attr,
As mentioned above, could we move the above two to the dsb_patt_attrs ?
Suzuki
> &dev_attr_dsb_trig_ts.attr, > &dev_attr_dsb_trig_type.attr, > NULL, > @@ -617,11 +733,18 @@ static struct attribute_group tpdm_dsb_trig_patt_grp = { > .name = "dsb_trig_patt", > }; > > +static struct attribute_group tpdm_dsb_patt_grp = { > + .attrs = tpdm_dsb_patt_attrs, > + .is_visible = tpdm_dsb_is_visible, > + .name = "dsb_patt", > +}; > + > static const struct attribute_group *tpdm_attr_grps[] = { > &tpdm_attr_grp, > &tpdm_dsb_attrs_grp, > &tpdm_dsb_edge_grp, > &tpdm_dsb_trig_patt_grp, > + &tpdm_dsb_patt_grp, > NULL, > }; > > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h b/drivers/hwtracing/coresight/coresight-tpdm.h > index 9e1b0a4..9173e80 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)) > @@ -28,8 +30,12 @@ > /* Data bits for DSB test mode */ > #define TPDM_DSB_CR_TEST_MODE GENMASK(10, 9) > > +/* 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 */ > /* DSB mode bits mask */ > @@ -122,14 +128,26 @@ > tpdm_simple_dataset_rw(xpmr##nr, \ > DSB_TRIG_PATT_MASK, nr, TPDM_DSB_MAX_PATT) > > +#define DSB_PATT_ATTR(nr) \ > + tpdm_simple_dataset_rw(tpr##nr, \ > + DSB_PATT, nr, TPDM_DSB_MAX_PATT) > + > +#define DSB_PATT_MASK_ATTR(nr) \ > + tpdm_simple_dataset_rw(tpmr##nr, \ > + DSB_PATT_MASK, nr, TPDM_DSB_MAX_PATT) > + > /** > * struct dsb_dataset - specifics associated to dsb dataset > * @mode: DSB programming mode > * @edge_ctrl_idx Index number of the edge control > * @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 > * @trig_patt: Save value for trigger pattern > * @trig_patt_mask: Save value for trigger pattern mask > + * @patt_ts: Enable/Disable pattern timestamp > + * @patt_type: Set pattern type > * @trig_ts: Enable/Disable trigger timestamp. > * @trig_type: Enable/Disable trigger type. > */ > @@ -138,8 +156,12 @@ struct dsb_dataset { > u32 edge_ctrl_idx; > 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]; > u32 trig_patt[TPDM_DSB_MAX_PATT]; > u32 trig_patt_mask[TPDM_DSB_MAX_PATT]; > + bool patt_ts; > + bool patt_type; > bool trig_ts; > bool trig_type; > }; > @@ -171,6 +193,8 @@ enum dataset_mem { > DSB_EDGE_CTRL_MASK, > DSB_TRIG_PATT, > DSB_TRIG_PATT_MASK, > + DSB_PATT, > + DSB_PATT_MASK, > }; > > /**
| |