Message ID | 1662626705-13097-8-git-send-email-quic_taozha@quicinc.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | Add support to configure TPDM DSB subunit | expand |
On 08/09/2022 09:45, Tao Zhang wrote: > Add the nodes to set value for DSB edge control and DSB edge > control mask. Each DSB subunit TPDM has n(n<16) EDCR resgisters > to configure edge control. And each DSB subunit TPDM has m(m<8) > ECDMR registers to configure edge control mask. > Please could you describe what these values represent ? Size of the "value". Without the specs, it is really difficult to comprehend the code. Please add comments > Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> > --- > drivers/hwtracing/coresight/coresight-tpdm.c | 130 ++++++++++++++++++++++++++- > drivers/hwtracing/coresight/coresight-tpdm.h | 11 +++ > 2 files changed, 140 insertions(+), 1 deletion(-) > > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c > index 7265793..14bcf2b 100644 > --- a/drivers/hwtracing/coresight/coresight-tpdm.c > +++ b/drivers/hwtracing/coresight/coresight-tpdm.c > @@ -22,7 +22,14 @@ DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm"); > > static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) > { > - u32 val, mode; > + u32 val, mode, i; > + > + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) > + writel_relaxed(drvdata->dsb->edge_ctrl[i], > + drvdata->base + TPDM_DSB_EDCR(i)); > + for (i = 0; i < TPDM_DSB_MAX_EDCR / 2; i++) Please use "TPDM_DSB_MAX_EDMCR" for consistency and ease of following the code. > + writel_relaxed(drvdata->dsb->edge_ctrl_mask[i], > + drvdata->base + TPDM_DSB_EDCMR(i)); > > val = readl_relaxed(drvdata->base + TPDM_DSB_TIER); > /* Set trigger timestamp */ > @@ -278,6 +285,125 @@ static ssize_t dsb_mode_store(struct device *dev, > } > static DEVICE_ATTR_RW(dsb_mode); > > +static ssize_t dsb_edge_ctrl_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; > + > + if (!(drvdata->datasets & TPDM_PIDR0_DS_DSB)) > + return -EPERM; > + > + spin_lock(&drvdata->spinlock); > + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) { > + size += scnprintf(buf + size, PAGE_SIZE - size, > + "Index:0x%x Val:0x%x\n", i, > + drvdata->dsb->edge_ctrl[i]); > + } > + spin_unlock(&drvdata->spinlock); > + return size; > +} > + > +/* > + * value 1: Start EDCR register number > + * value 2: End EDCR register number > + * value 3: The value need to be written What are the constraints on these inputs ? Please add a comment here. And may be also add a user scenario if possible to describe the values and its effects on the unit ? > + */ > +static ssize_t dsb_edge_ctrl_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 start, end, edge_ctrl; > + uint32_t val; > + int i, bit, reg; > + > + if (sscanf(buf, "%lx %lx %lx", &start, &end, &edge_ctrl) != 3) > + return -EINVAL; > + if (!(drvdata->datasets & TPDM_PIDR0_DS_DSB) || > + (start >= TPDM_DSB_MAX_LINES) || (end >= TPDM_DSB_MAX_LINES) || > + edge_ctrl > 0x2) > + return -EPERM; > + > + spin_lock(&drvdata->spinlock); > + for (i = start; i <= end; i++) { > + reg = i / (NUM_OF_BITS / 2); > + bit = i % (NUM_OF_BITS / 2); NUM_OF_BITS in what ? That is too generic. > + bit = bit * 2; minor nit: Name "bit" is confusing here. s/bit/{index or even field}? > + > + val = drvdata->dsb->edge_ctrl[reg]; > + val = val & ~GENMASK((bit + 1), bit); > + val = val | (edge_ctrl << bit); > + drvdata->dsb->edge_ctrl[reg] = val; > + } > + spin_unlock(&drvdata->spinlock); > + > + return size; > +} > +static DEVICE_ATTR_RW(dsb_edge_ctrl); > + > +static ssize_t dsb_edge_ctrl_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; > + > + if (!(drvdata->datasets & TPDM_PIDR0_DS_DSB)) > + return -EPERM; > + > + spin_lock(&drvdata->spinlock); > + for (i = 0; i < TPDM_DSB_MAX_EDCR / 2; i++) { > + size += scnprintf(buf + size, PAGE_SIZE - size, > + "Index:0x%x Val:0x%x\n", i, > + drvdata->dsb->edge_ctrl_mask[i]); > + } > + spin_unlock(&drvdata->spinlock); > + return size; > +} > + > +/* > + * value 1: Start EDCMR register number > + * value 2: End EDCMR register number > + * value 3: The value need to be written > + */ > +static ssize_t dsb_edge_ctrl_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 start, end, val; > + u32 set; > + int i, bit, reg; > + > + if (sscanf(buf, "%lx %lx %lx", &start, &end, &val) != 3) > + return -EINVAL; > + if (!(drvdata->datasets & TPDM_PIDR0_DS_DSB) || Please use is_visible() hook. > + (start >= TPDM_DSB_MAX_LINES) || (end >= TPDM_DSB_MAX_LINES)) > + return -EPERM; No constraints on "val"? 0 or 1 ? > + > + spin_lock(&drvdata->spinlock); > + for (i = start; i <= end; i++) { > + reg = i / NUM_OF_BITS; > + bit = (i % NUM_OF_BITS); > + > + set = drvdata->dsb->edge_ctrl_mask[reg]; > + if (val) > + set = set | BIT(bit); minor nit: set |= .. > + else > + set = set & ~BIT(bit); set &= .. ? > + drvdata->dsb->edge_ctrl_mask[reg] = set; > + } > + spin_unlock(&drvdata->spinlock); > + return size; > +} > +static DEVICE_ATTR_RW(dsb_edge_ctrl_mask); > + > static ssize_t dsb_trig_type_show(struct device *dev, > struct device_attribute *attr, > char *buf) > @@ -359,6 +485,8 @@ 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_edge_ctrl.attr, > + &dev_attr_dsb_edge_ctrl_mask.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 4d57488..ed03c68 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_EDCR(n) (0x808 + (n * 4)) > +#define TPDM_DSB_EDCMR(n) (0x848 + (n * 4)) > > /* Enable bit for DSB subunit */ > #define TPDM_DSB_CR_ENA BIT(0) > @@ -28,6 +30,8 @@ > #define TPDM_DSB_MODE_HPBYTESEL(val) BMVAL(val, 4, 8) > #define TPDM_MODE_ALL (0xFFFFFFF) > > +#define NUM_OF_BITS 32 > + > /* TPDM integration test registers */ > #define TPDM_ITATBCNTRL (0xEF0) > #define TPDM_ITCNTRL (0xF00) > @@ -54,14 +58,21 @@ > #define TPDM_PIDR0_DS_IMPDEF BIT(0) > #define TPDM_PIDR0_DS_DSB BIT(1) > > +#define TPDM_DSB_MAX_EDCR 16 > +#define TPDM_DSB_MAX_LINES 256 A comment here would help : e.g, EDCR has 2 bits for each lines. Thus we can support upto (16 * 32) / 2 lines. May be, even use that math above instead of hard coding 256 ? And explain what each bits would do ? > + > /** > * struct dsb_dataset - specifics associated to dsb dataset > * @mode: DSB programming mode > + * @edge_ctrl: Save value for edge control > + * @edge_ctrl_mask: Save value for edge control mask > * @trig_ts: Enable/Disable trigger timestamp. > * @trig_type: Enable/Disable trigger type. > */ > struct dsb_dataset { > u32 mode; > + u32 edge_ctrl[TPDM_DSB_MAX_EDCR]; > + u32 edge_ctrl_mask[TPDM_DSB_MAX_EDCR / 2]; Please use TPDM_DSB_MAX_EDCMR instead. Suzuki > bool trig_ts; > bool trig_type; > };
diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c b/drivers/hwtracing/coresight/coresight-tpdm.c index 7265793..14bcf2b 100644 --- a/drivers/hwtracing/coresight/coresight-tpdm.c +++ b/drivers/hwtracing/coresight/coresight-tpdm.c @@ -22,7 +22,14 @@ DEFINE_CORESIGHT_DEVLIST(tpdm_devs, "tpdm"); static void tpdm_enable_dsb(struct tpdm_drvdata *drvdata) { - u32 val, mode; + u32 val, mode, i; + + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) + writel_relaxed(drvdata->dsb->edge_ctrl[i], + drvdata->base + TPDM_DSB_EDCR(i)); + for (i = 0; i < TPDM_DSB_MAX_EDCR / 2; i++) + writel_relaxed(drvdata->dsb->edge_ctrl_mask[i], + drvdata->base + TPDM_DSB_EDCMR(i)); val = readl_relaxed(drvdata->base + TPDM_DSB_TIER); /* Set trigger timestamp */ @@ -278,6 +285,125 @@ static ssize_t dsb_mode_store(struct device *dev, } static DEVICE_ATTR_RW(dsb_mode); +static ssize_t dsb_edge_ctrl_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; + + if (!(drvdata->datasets & TPDM_PIDR0_DS_DSB)) + return -EPERM; + + spin_lock(&drvdata->spinlock); + for (i = 0; i < TPDM_DSB_MAX_EDCR; i++) { + size += scnprintf(buf + size, PAGE_SIZE - size, + "Index:0x%x Val:0x%x\n", i, + drvdata->dsb->edge_ctrl[i]); + } + spin_unlock(&drvdata->spinlock); + return size; +} + +/* + * value 1: Start EDCR register number + * value 2: End EDCR register number + * value 3: The value need to be written + */ +static ssize_t dsb_edge_ctrl_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 start, end, edge_ctrl; + uint32_t val; + int i, bit, reg; + + if (sscanf(buf, "%lx %lx %lx", &start, &end, &edge_ctrl) != 3) + return -EINVAL; + if (!(drvdata->datasets & TPDM_PIDR0_DS_DSB) || + (start >= TPDM_DSB_MAX_LINES) || (end >= TPDM_DSB_MAX_LINES) || + edge_ctrl > 0x2) + return -EPERM; + + spin_lock(&drvdata->spinlock); + for (i = start; i <= end; i++) { + reg = i / (NUM_OF_BITS / 2); + bit = i % (NUM_OF_BITS / 2); + bit = bit * 2; + + val = drvdata->dsb->edge_ctrl[reg]; + val = val & ~GENMASK((bit + 1), bit); + val = val | (edge_ctrl << bit); + drvdata->dsb->edge_ctrl[reg] = val; + } + spin_unlock(&drvdata->spinlock); + + return size; +} +static DEVICE_ATTR_RW(dsb_edge_ctrl); + +static ssize_t dsb_edge_ctrl_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; + + if (!(drvdata->datasets & TPDM_PIDR0_DS_DSB)) + return -EPERM; + + spin_lock(&drvdata->spinlock); + for (i = 0; i < TPDM_DSB_MAX_EDCR / 2; i++) { + size += scnprintf(buf + size, PAGE_SIZE - size, + "Index:0x%x Val:0x%x\n", i, + drvdata->dsb->edge_ctrl_mask[i]); + } + spin_unlock(&drvdata->spinlock); + return size; +} + +/* + * value 1: Start EDCMR register number + * value 2: End EDCMR register number + * value 3: The value need to be written + */ +static ssize_t dsb_edge_ctrl_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 start, end, val; + u32 set; + int i, bit, reg; + + if (sscanf(buf, "%lx %lx %lx", &start, &end, &val) != 3) + return -EINVAL; + if (!(drvdata->datasets & TPDM_PIDR0_DS_DSB) || + (start >= TPDM_DSB_MAX_LINES) || (end >= TPDM_DSB_MAX_LINES)) + return -EPERM; + + spin_lock(&drvdata->spinlock); + for (i = start; i <= end; i++) { + reg = i / NUM_OF_BITS; + bit = (i % NUM_OF_BITS); + + set = drvdata->dsb->edge_ctrl_mask[reg]; + if (val) + set = set | BIT(bit); + else + set = set & ~BIT(bit); + drvdata->dsb->edge_ctrl_mask[reg] = set; + } + spin_unlock(&drvdata->spinlock); + return size; +} +static DEVICE_ATTR_RW(dsb_edge_ctrl_mask); + static ssize_t dsb_trig_type_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -359,6 +485,8 @@ 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_edge_ctrl.attr, + &dev_attr_dsb_edge_ctrl_mask.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 4d57488..ed03c68 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_EDCR(n) (0x808 + (n * 4)) +#define TPDM_DSB_EDCMR(n) (0x848 + (n * 4)) /* Enable bit for DSB subunit */ #define TPDM_DSB_CR_ENA BIT(0) @@ -28,6 +30,8 @@ #define TPDM_DSB_MODE_HPBYTESEL(val) BMVAL(val, 4, 8) #define TPDM_MODE_ALL (0xFFFFFFF) +#define NUM_OF_BITS 32 + /* TPDM integration test registers */ #define TPDM_ITATBCNTRL (0xEF0) #define TPDM_ITCNTRL (0xF00) @@ -54,14 +58,21 @@ #define TPDM_PIDR0_DS_IMPDEF BIT(0) #define TPDM_PIDR0_DS_DSB BIT(1) +#define TPDM_DSB_MAX_EDCR 16 +#define TPDM_DSB_MAX_LINES 256 + /** * struct dsb_dataset - specifics associated to dsb dataset * @mode: DSB programming mode + * @edge_ctrl: Save value for edge control + * @edge_ctrl_mask: Save value for edge control mask * @trig_ts: Enable/Disable trigger timestamp. * @trig_type: Enable/Disable trigger type. */ struct dsb_dataset { u32 mode; + u32 edge_ctrl[TPDM_DSB_MAX_EDCR]; + u32 edge_ctrl_mask[TPDM_DSB_MAX_EDCR / 2]; bool trig_ts; bool trig_type; };
Add the nodes to set value for DSB edge control and DSB edge control mask. Each DSB subunit TPDM has n(n<16) EDCR resgisters to configure edge control. And each DSB subunit TPDM has m(m<8) ECDMR registers to configure edge control mask. Signed-off-by: Tao Zhang <quic_taozha@quicinc.com> --- drivers/hwtracing/coresight/coresight-tpdm.c | 130 ++++++++++++++++++++++++++- drivers/hwtracing/coresight/coresight-tpdm.h | 11 +++ 2 files changed, 140 insertions(+), 1 deletion(-)