Re: [PATCH v5 3/3] coresight: tpda: add sysfs node to flush specific port

[Jie Gan]

On 12/19/2025 6:33 PM, Suzuki K Poulose wrote:
> On 19/12/2025 10:04, Jie Gan wrote:
> > From: Tao Zhang <tao.zhang@xxxxxxxxxxxxxxxx>
> >
> > Setting bit i in the TPDA_FLUSH_CR register initiates a flush request
> > for port i, forcing the data to synchronize and be transmitted to the
> > sink device.
> >
> > Signed-off-by: Tao Zhang <tao.zhang@xxxxxxxxxxxxxxxx>
> > Reviewed-by: James Clark <james.clark@xxxxxxxxxx>
> > Co-developed-by: Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
> > ---
> >   .../ABI/testing/sysfs-bus-coresight-devices-tpda   |  7 ++++
> >   drivers/hwtracing/coresight/coresight-tpda.c       | 49 ++++++++++++ 
> > ++++++++++
> >   drivers/hwtracing/coresight/coresight-tpda.h       |  1 +
> >   3 files changed, 57 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices- 
> > tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
> > index a7855922328e..ef2804f87d1e 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
> > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda
> > @@ -41,3 +41,10 @@ Contact:    Jinlong Mao 
> > <jinlong.mao@xxxxxxxxxxxxxxxx>, Tao Zhang <tao.zhang@xxxxxx
> >   Description:
> >           (RW) Configure the CMB/MCMB channel mode for all enabled ports.
> >           Value 0 means raw channel mapping mode. Value 1 means 
> > channel pair marking mode.
> > +
> > +What:        /sys/bus/coresight/devices/<tpda-name>/port_flush_req
> > +Date:        December 2025
> > +KernelVersion:    6.19
> > +Contact:    Jinlong Mao <jinlong.mao@xxxxxxxxxxxxxxxx>, Tao Zhang 
> > <tao.zhang@xxxxxxxxxxxxxxxx>, Jie Gan <jie.gan@xxxxxxxxxxxxxxxx>
> > +Description:
> > +        (RW) Configure the bit i to requests a flush operation of 
> > port i on the TPDA.
> > diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/ 
> > hwtracing/coresight/coresight-tpda.c
> > index d378ff8ad77d..5b4a9e41b067 100644
> > --- a/drivers/hwtracing/coresight/coresight-tpda.c
> > +++ b/drivers/hwtracing/coresight/coresight-tpda.c
> > @@ -375,6 +375,45 @@ static ssize_t tpda_trig_sysfs_store(struct 
> > device *dev,
> >       return ret;
> >   }
> > +static ssize_t port_flush_req_show(struct device *dev,
> > +                   struct device_attribute *attr,
> > +                   char *buf)
> > +{
> > +    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > +    unsigned long val;
> > +
> > +    if (!drvdata->csdev->refcnt)
> > +        return -EINVAL;
> > +
> > +    guard(spinlock)(&drvdata->spinlock);
> > +    val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR);
> > +
> > +    return sysfs_emit(buf, "0x%lx\n", val);
> > +}
> > +
> > +static ssize_t port_flush_req_store(struct device *dev,
> > +                    struct device_attribute *attr,
> > +                    const char *buf,
> > +                    size_t size)
> > +{
> > +    struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent);
> > +    u32 val;
> > +
> > +    if (kstrtou32(buf, 0, &val))
> > +        return -EINVAL;
> > +
> > +    if (!drvdata->csdev->refcnt || !val)
> > +        return -EINVAL;
> > +
> > +    guard(spinlock)(&drvdata->spinlock);
> > +    CS_UNLOCK(drvdata->base);
> > +    writel_relaxed(val, drvdata->base + TPDA_FLUSH_CR);
> > +    CS_LOCK(drvdata->base);
> > +
> > +    return size;
> > +}
> > +static DEVICE_ATTR_RW(port_flush_req);
> > +
> >   static struct attribute *tpda_global_cr_attrs[] = {
> >       tpda_trig_sysfs_rw(global_flush_req, FLREQ),
> >       tpda_trig_sysfs_rw(freq_ts_enable, FREQTS),
> > @@ -385,13 +424,23 @@ static struct attribute *tpda_global_cr_attrs[] = {
> >       NULL,
> >   };
> > +static struct attribute *tpda_attrs[] = {
> > +    &dev_attr_port_flush_req.attr,
> > +    NULL,
> > +};
> > +
>
> But why ? Why can't this be a part of the tpda_global_cr_attrs[] ?
> For that matter, why is tpda_global_cr_attrs named as such and why not
> tpda_attrs ?

will move all attributes to tpda_attrs[].

Thanks,
Jie

> Suzuki
>
> >   static struct attribute_group tpda_global_cr_attr_grp = {
> >       .attrs    = tpda_global_cr_attrs,
> >       .name    = "global_cr",
> >   };
> > +static struct attribute_group tpda_attr_grp = {
> > +    .attrs    = tpda_attrs,
> > +};
> > +
> >   static const struct attribute_group *tpda_attr_grps[] = {
> >       &tpda_global_cr_attr_grp,
> > +    &tpda_attr_grp,
> >       NULL,
> >   };
> > diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/ 
> > hwtracing/coresight/coresight-tpda.h
> > index 97e2729c15c9..ec93f9cb9648 100644
> > --- a/drivers/hwtracing/coresight/coresight-tpda.h
> > +++ b/drivers/hwtracing/coresight/coresight-tpda.h
> > @@ -10,6 +10,7 @@
> >   #define TPDA_Pn_CR(n)        (0x004 + (n * 4))
> >   #define TPDA_FPID_CR        (0x084)
> >   #define TPDA_SYNCR        (0x08C)
> > +#define TPDA_FLUSH_CR        (0x090)
> >   /* Cross trigger Global (all ports) flush request bit */
> >   #define TPDA_CR_FLREQ        BIT(0)