Re: [PATCH v4 11/11] coresight-tpdm: Add nodes for dsb msr support

[Tao Zhang]

On 6/5/2023 6:24 PM, Suzuki K Poulose wrote:
> On 27/04/2023 10:00, Tao Zhang wrote:
> > Add the nodes for DSB subunit MSR(mux select register) support.
> > The TPDM MSR (mux select register) interface is an optional
> > interface and associated bank of registers per TPDM subunit.
> > The intent of mux select registers is to control muxing structures
> > driving the TPDM’s’ various subunit interfaces.
> >
> > Signed-off-by: Tao Zhang <quic_taozha@xxxxxxxxxxx>
> > ---
> >   .../ABI/testing/sysfs-bus-coresight-devices-tpdm   | 15 ++++++
> >   drivers/hwtracing/coresight/coresight-tpdm.c       | 53 
> > ++++++++++++++++++++++
> >   drivers/hwtracing/coresight/coresight-tpdm.h       |  3 ++
> >   3 files changed, 71 insertions(+)
> >
> > diff --git 
> > a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm 
> > b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > index 639b6fb8..f746f25 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > @@ -170,3 +170,18 @@ Description:
> >           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.
> > +
> > +What: /sys/bus/coresight/devices/<tpdm-name>/dsb_msr
> > +Date:        March 2023
> > +KernelVersion    6.3
> > +Contact:    Jinlong Mao (QUIC) <quic_jinlmao@xxxxxxxxxxx>, Tao Zhang 
> > (QUIC) <quic_taozha@xxxxxxxxxxx>
> > +Description:
> > +        (Write) Set the MSR(mux select register) of DSB tpdm. Read
> > +        the MSR(mux select register) of DSB tpdm.
> > +
> > +        Expected format is the following:
> > +        <integer1> <integer2>
> > +
> > +        Where:
> > +        <integer1> : Index number of MSR register
> > +        <integer2> : The value need to be written
> > diff --git a/drivers/hwtracing/coresight/coresight-tpdm.c 
> > b/drivers/hwtracing/coresight/coresight-tpdm.c
> > index 627de36..5fe0bd5c 100644
> > --- a/drivers/hwtracing/coresight/coresight-tpdm.c
> > +++ b/drivers/hwtracing/coresight/coresight-tpdm.c
> > @@ -240,6 +240,14 @@ static int tpdm_datasets_setup(struct 
> > tpdm_drvdata *drvdata)
> >               if (!drvdata->dsb)
> >                   return -ENOMEM;
> >           }
> > +        if (!of_property_read_u32(drvdata->dev->of_node,
> > +               "qcom,dsb_msr_num", &drvdata->dsb->msr_num)) {
> > +            drvdata->dsb->msr = devm_kzalloc(drvdata->dev,
> > +                   (drvdata->dsb->msr_num * 
> > sizeof(*drvdata->dsb->msr)),
> > +                   GFP_KERNEL);
> > +            if (!drvdata->dsb->msr)
> > +                return -ENOMEM;
> > +        }
> >       }
> >         return 0;
> > @@ -765,6 +773,50 @@ static ssize_t dsb_trig_ts_store(struct device 
> > *dev,
> >   }
> >   static DEVICE_ATTR_RW(dsb_trig_ts);
> >   +static ssize_t dsb_msr_show(struct device *dev,
> > +                 struct device_attribute *attr,
> > +                 char *buf)
> > +{
> > +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > +    unsigned int i;
> > +    ssize_t size = 0;
> > +
> > +    if (drvdata->dsb->msr_num == 0)
> > +        return -EINVAL;
> > +
> > +    spin_lock(&drvdata->spinlock);
> > +    for (i = 0; i < TPDM_DSB_MAX_PATT; i++) {
>
> Shouldn't this be  "i < drvdata->dsb->msr_num" ?
Yes, I will update it to the next patch series.
> > +        size += sysfs_emit_at(buf, size,
> > +                  "%u 0x%x\n", i, drvdata->dsb->msr[i]);
> > +    }
> > +    spin_unlock(&drvdata->spinlock);
> > +
> > +    return size;
> > +}
> > +
> > +static ssize_t dsb_msr_store(struct device *dev,
> > +                  struct device_attribute *attr,
> > +                  const char *buf,
> > +                  size_t size)
> > +{
> > +    struct tpdm_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > +    unsigned int num, val;
> > +    int nval;
> > +
> > +    if (drvdata->dsb->msr_num == 0)
> > +        return -EINVAL;
> > +
> > +    nval = sscanf(buf, "%u %x", &num, &val);
> > +    if ((nval != 2) || (num >= (drvdata->dsb->msr_num - 1)))
>
> (num >= drvdata->dsb->msr_num) ?
Yes, I will update it to the next patch series.
> > +        return -EINVAL;
> > +
> > +    spin_lock(&drvdata->spinlock);
> > +    drvdata->dsb->msr[num] = val;
> > +    spin_unlock(&drvdata->spinlock);
> > +    return size;
> > +}
> > +static DEVICE_ATTR_RW(dsb_msr);
> > +
> >   static struct attribute *tpdm_dsb_attrs[] = {
> >       &dev_attr_dsb_mode.attr,
> >       &dev_attr_dsb_edge_ctrl.attr,
> > @@ -777,6 +829,7 @@ static struct attribute *tpdm_dsb_attrs[] = {
> >       &dev_attr_dsb_trig_patt_mask.attr,
> >       &dev_attr_dsb_trig_ts.attr,
> >       &dev_attr_dsb_trig_type.attr,
> > +    &dev_attr_dsb_msr.attr,
> >       NULL,
> >   };
> >   diff --git a/drivers/hwtracing/coresight/coresight-tpdm.h 
> > b/drivers/hwtracing/coresight/coresight-tpdm.h
> > index 9ad32a6..05e9f8e 100644
> > --- a/drivers/hwtracing/coresight/coresight-tpdm.h
> > +++ b/drivers/hwtracing/coresight/coresight-tpdm.h
> > @@ -18,6 +18,7 @@
> >   #define TPDM_DSB_XPMR(n)    (0x7E8 + (n * 4))
> >   #define TPDM_DSB_EDCR(n)    (0x808 + (n * 4))
> >   #define TPDM_DSB_EDCMR(n)    (0x848 + (n * 4))
> > +#define TPDM_DSB_MSR(n)        (0x980 + (n * 4))
> >     /* Enable bit for DSB subunit */
> >   #define TPDM_DSB_CR_ENA        BIT(0)
> > @@ -113,6 +114,8 @@ struct dsb_dataset {
> >       u32                trig_patt_mask[TPDM_DSB_MAX_PATT];
> >       bool            trig_ts;
> >       bool            trig_type;
> > +    u32                msr_num;
> > +    u32                *msr;
> >   };
> >     /**
>
>
> Where/when do we write to these registers in the DSB ?

DSB MSR registers should be written in the DSB TPDM enablement function.

I will update this to the next patch series.


Best

Tao

> Suzuki