lkml.org 
[lkml]   [2023]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v3 09/11] coresight-tpdm: Add nodes for timestamp request
    From
    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;
    >

    \
     
     \ /
      Last update: 2023-03-27 10:42    [W:5.222 / U:0.220 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site