CoreSight August 2025

coresight@lists.linaro.org

7 participants
42 discussions

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

by James Clark

On 26/08/2025 1:11 pm, Jie Gan wrote: > > > On 8/26/2025 5:54 PM, James Clark wrote: >> >> >> On 26/08/2025 10:39 am, Jie Gan wrote: >>> >>> >>> On 8/26/2025 5:27 PM, James Clark wrote: >>>> >>>> >>>> On 26/08/2025 8:01 am, Jie Gan wrote: >>>>> From: Tao Zhang <tao.zhang(a)oss.qualcomm.com> >>>>> >>>>> 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(a)oss.qualcomm.com> >>>>> Co-developed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> >>>>> Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> >>>>> --- >>>>> .../testing/sysfs-bus-coresight-devices-tpda | 7 +++ >>>>> drivers/hwtracing/coresight/coresight-tpda.c | 45 ++++++++++++++ >>>>> + ++++ >>>>> drivers/hwtracing/coresight/coresight-tpda.h | 1 + >>>>> 3 files changed, 53 insertions(+) >>>>> >>>>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices- >>>>> tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda >>>>> index e827396a0fa1..8803158ba42f 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(a)oss.qualcomm.com>, Tao Zhang <tao.zhang(a)oss.qu >>>>> 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: August 2025 >>>>> +KernelVersion: 6.17 >>>>> +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Tao Zhang >>>>> <tao.zhang(a)oss.qualcomm.com>, Jie Gan <jie.gan(a)oss.qualcomm.com> >>>>> +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 9e623732d1e7..c5f169facc51 100644 >>>>> --- a/drivers/hwtracing/coresight/coresight-tpda.c >>>>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c >>>>> @@ -509,6 +509,50 @@ static ssize_t cmbchan_mode_store(struct >>>>> device *dev, >>>>> } >>>>> static DEVICE_ATTR_RW(cmbchan_mode); >>>>> +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; >>>>> + >>>>> + guard(spinlock)(&drvdata->spinlock); >>>>> + if (!drvdata->csdev->refcnt) >>>>> + return -EPERM; >>>>> + >>>>> + val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR); >>>>> + return sysfs_emit(buf, "%lx\n", val); >>>> >>>> Decimal would be better for a port number that goes from 0 - 127. If >>>> you really want to use hex then don't you need to prefix it with 0x? >>>> Otherwise you can't tell the difference between decimal 10 and hex >>>> 10, and it's not documented that it's hex either. >>>> >>> >>> Got it. I will fix the code here, and update the description in >>> document. >>> >>>>> +} >>>>> + >>>>> +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); >>>>> + unsigned long val; >>>>> + >>>>> + if (kstrtoul(buf, 0, &val)) >>>>> + return -EINVAL; >>>>> + >>>>> + /* The valid value ranges from 0 to 127 */ >>>>> + if (val > 127) >>>>> + return -EINVAL; >>>>> + >>>>> + guard(spinlock)(&drvdata->spinlock); >>>>> + if (!drvdata->csdev->refcnt) >>>>> + return -EPERM; >>>>> + >>>>> + if (val) { >>>> >>>> If 0 - 127 are valid don't you want to write 0 too? >>> >>> It's 1-127 here. 0 may leads to an unexpected issue here. >>> >>> Thanks, >>> Jie >>> >> >> Then can't the above be this: >> >> /* The valid value ranges from 1 to 127 */ >> if (val < 1 || val > 127) >> return -EINVAL; >> >> But I'm wondering how you flush port 0? >> > > BIT(0) represents port 0 with value 1 and the default value 0 means > nothing will be triggered here. > >> Isn't the default value 0? So if you never write to port_flush_req >> then you'd flush port 0, but why can't you change it back to 0 after >> writing a different value? > > We can change the value back to 0 but I think we shouldn't do this > although I haven't suffer issue after I changed it back to 0(for bit). > Because the document mentioned: "Once set, the bit remains set until the > flush operation on port i completes and the bit then clears to 0". So I > think we should let the flush operation finish as expected and clear the > bit by itself? Or may suffer unexpected error when try to interrupt the > flush operation? > > Thanks, > Jie Oh I see, I thought this was a port number, not a bit for each port. That changes this and my other comment about changing the output to be decimal then. Hex is probably better but it needs the 0x prefix. I would also treat 0 as EINVAL. It doesn't do anything different to any other out of range request so it should be treated the same way. Then comparing to 127 isn't that obvious either. Something like FIELD_FITS() more clearly states that values have to fit into a bitfield rather than be less than some value: if (!val || !FIELD_FIT(TPDA_FLUSH_CR_PORTNUM, val)) return -EINVAL; > >>> >>>>> + 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_attrs[] = { >>>>> &dev_attr_trig_async_enable.attr, >>>>> &dev_attr_trig_flag_ts_enable.attr, >>>>> @@ -516,6 +560,7 @@ static struct attribute *tpda_attrs[] = { >>>>> &dev_attr_freq_ts_enable.attr, >>>>> &dev_attr_global_flush_req.attr, >>>>> &dev_attr_cmbchan_mode.attr, >>>>> + &dev_attr_port_flush_req.attr, >>>>> NULL, >>>>> }; >>>>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/ >>>>> drivers/ hwtracing/coresight/coresight-tpda.h >>>>> index 00d146960d81..55a18d718357 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 FREQ packets timestamp bit */ >>>>> #define TPDA_CR_FREQTS BIT(2) >>>> >>>> >>> >> >> >

2 weeks, 3 days

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

by James Clark

On 26/08/2025 10:39 am, Jie Gan wrote: > > > On 8/26/2025 5:27 PM, James Clark wrote: >> >> >> On 26/08/2025 8:01 am, Jie Gan wrote: >>> From: Tao Zhang <tao.zhang(a)oss.qualcomm.com> >>> >>> 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(a)oss.qualcomm.com> >>> Co-developed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> >>> Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> >>> --- >>> .../testing/sysfs-bus-coresight-devices-tpda | 7 +++ >>> drivers/hwtracing/coresight/coresight-tpda.c | 45 +++++++++++++++++++ >>> drivers/hwtracing/coresight/coresight-tpda.h | 1 + >>> 3 files changed, 53 insertions(+) >>> >>> diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices- >>> tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda >>> index e827396a0fa1..8803158ba42f 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(a)oss.qualcomm.com>, Tao Zhang <tao.zhang(a)oss.qu >>> 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: August 2025 >>> +KernelVersion: 6.17 >>> +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Tao Zhang >>> <tao.zhang(a)oss.qualcomm.com>, Jie Gan <jie.gan(a)oss.qualcomm.com> >>> +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 9e623732d1e7..c5f169facc51 100644 >>> --- a/drivers/hwtracing/coresight/coresight-tpda.c >>> +++ b/drivers/hwtracing/coresight/coresight-tpda.c >>> @@ -509,6 +509,50 @@ static ssize_t cmbchan_mode_store(struct device >>> *dev, >>> } >>> static DEVICE_ATTR_RW(cmbchan_mode); >>> +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; >>> + >>> + guard(spinlock)(&drvdata->spinlock); >>> + if (!drvdata->csdev->refcnt) >>> + return -EPERM; >>> + >>> + val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR); >>> + return sysfs_emit(buf, "%lx\n", val); >> >> Decimal would be better for a port number that goes from 0 - 127. If >> you really want to use hex then don't you need to prefix it with 0x? >> Otherwise you can't tell the difference between decimal 10 and hex 10, >> and it's not documented that it's hex either. >> > > Got it. I will fix the code here, and update the description in document. > >>> +} >>> + >>> +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); >>> + unsigned long val; >>> + >>> + if (kstrtoul(buf, 0, &val)) >>> + return -EINVAL; >>> + >>> + /* The valid value ranges from 0 to 127 */ >>> + if (val > 127) >>> + return -EINVAL; >>> + >>> + guard(spinlock)(&drvdata->spinlock); >>> + if (!drvdata->csdev->refcnt) >>> + return -EPERM; >>> + >>> + if (val) { >> >> If 0 - 127 are valid don't you want to write 0 too? > > It's 1-127 here. 0 may leads to an unexpected issue here. > > Thanks, > Jie > Then can't the above be this: /* The valid value ranges from 1 to 127 */ if (val < 1 || val > 127) return -EINVAL; But I'm wondering how you flush port 0? Isn't the default value 0? So if you never write to port_flush_req then you'd flush port 0, but why can't you change it back to 0 after writing a different value? >> >>> + 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_attrs[] = { >>> &dev_attr_trig_async_enable.attr, >>> &dev_attr_trig_flag_ts_enable.attr, >>> @@ -516,6 +560,7 @@ static struct attribute *tpda_attrs[] = { >>> &dev_attr_freq_ts_enable.attr, >>> &dev_attr_global_flush_req.attr, >>> &dev_attr_cmbchan_mode.attr, >>> + &dev_attr_port_flush_req.attr, >>> NULL, >>> }; >>> diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/ >>> hwtracing/coresight/coresight-tpda.h >>> index 00d146960d81..55a18d718357 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 FREQ packets timestamp bit */ >>> #define TPDA_CR_FREQTS BIT(2) >> >> >

2 weeks, 3 days

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

by James Clark

On 26/08/2025 8:01 am, Jie Gan wrote: > From: Tao Zhang <tao.zhang(a)oss.qualcomm.com> > > 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(a)oss.qualcomm.com> > Co-developed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../testing/sysfs-bus-coresight-devices-tpda | 7 +++ > drivers/hwtracing/coresight/coresight-tpda.c | 45 +++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tpda.h | 1 + > 3 files changed, 53 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda > index e827396a0fa1..8803158ba42f 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(a)oss.qualcomm.com>, Tao Zhang <tao.zhang(a)oss.qu > 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: August 2025 > +KernelVersion: 6.17 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Tao Zhang <tao.zhang(a)oss.qualcomm.com>, Jie Gan <jie.gan(a)oss.qualcomm.com> > +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 9e623732d1e7..c5f169facc51 100644 > --- a/drivers/hwtracing/coresight/coresight-tpda.c > +++ b/drivers/hwtracing/coresight/coresight-tpda.c > @@ -509,6 +509,50 @@ static ssize_t cmbchan_mode_store(struct device *dev, > } > static DEVICE_ATTR_RW(cmbchan_mode); > > +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; > + > + guard(spinlock)(&drvdata->spinlock); > + if (!drvdata->csdev->refcnt) > + return -EPERM; > + > + val = readl_relaxed(drvdata->base + TPDA_FLUSH_CR); > + return sysfs_emit(buf, "%lx\n", val); Decimal would be better for a port number that goes from 0 - 127. If you really want to use hex then don't you need to prefix it with 0x? Otherwise you can't tell the difference between decimal 10 and hex 10, and it's not documented that it's hex either. > +} > + > +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); > + unsigned long val; > + > + if (kstrtoul(buf, 0, &val)) > + return -EINVAL; > + > + /* The valid value ranges from 0 to 127 */ > + if (val > 127) > + return -EINVAL; > + > + guard(spinlock)(&drvdata->spinlock); > + if (!drvdata->csdev->refcnt) > + return -EPERM; > + > + if (val) { If 0 - 127 are valid don't you want to write 0 too? > + 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_attrs[] = { > &dev_attr_trig_async_enable.attr, > &dev_attr_trig_flag_ts_enable.attr, > @@ -516,6 +560,7 @@ static struct attribute *tpda_attrs[] = { > &dev_attr_freq_ts_enable.attr, > &dev_attr_global_flush_req.attr, > &dev_attr_cmbchan_mode.attr, > + &dev_attr_port_flush_req.attr, > NULL, > }; > > diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h > index 00d146960d81..55a18d718357 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 FREQ packets timestamp bit */ > #define TPDA_CR_FREQTS BIT(2)

2 weeks, 3 days

Re: [PATCH v1 2/3] coresight: tpda: add function to configure TPDA_SYNCR register

by James Clark

On 26/08/2025 8:01 am, Jie Gan wrote: > From: Tao Zhang <tao.zhang(a)oss.qualcomm.com> > > The TPDA_SYNCR register defines the frequency at which TPDA generates > ASYNC packets, enabling userspace tools to accurately parse each valid > packet. > > Signed-off-by: Tao Zhang <tao.zhang(a)oss.qualcomm.com> > Co-developed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > drivers/hwtracing/coresight/coresight-tpda.c | 15 +++++++++++++++ > drivers/hwtracing/coresight/coresight-tpda.h | 1 + > 2 files changed, 16 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c > index cc254d53b8ec..9e623732d1e7 100644 > --- a/drivers/hwtracing/coresight/coresight-tpda.c > +++ b/drivers/hwtracing/coresight/coresight-tpda.c > @@ -189,6 +189,18 @@ static void tpda_enable_pre_port(struct tpda_drvdata *drvdata) > writel_relaxed(0x0, drvdata->base + TPDA_FPID_CR); > } > > +static void tpda_enable_post_port(struct tpda_drvdata *drvdata) > +{ > + uint32_t val; Minor nit: this is inconsistent with u32 used elsewhere in this file. > + > + val = readl_relaxed(drvdata->base + TPDA_SYNCR); > + /* Clear the mode */ > + val = val & ~TPDA_MODE_CTRL; &= > + /* Program the counter value */ > + val = val | 0xFFF; |= Defining a field would be a bit nicer here. Like: val |= FIELD_PREP(TPDA_SYNCR_COUNTER, UINT32_MAX); Assuming you wanted to set all bits, and 0xFFF isn't some specific value. > + writel_relaxed(val, drvdata->base + TPDA_SYNCR); > +} > + > static int tpda_enable_port(struct tpda_drvdata *drvdata, int port) > { > u32 val; > @@ -227,6 +239,9 @@ static int __tpda_enable(struct tpda_drvdata *drvdata, int port) > tpda_enable_pre_port(drvdata); > > ret = tpda_enable_port(drvdata, port); > + if (!drvdata->csdev->refcnt) > + tpda_enable_post_port(drvdata); Any reason this can't be done on tpda_enable_pre_port()? It has the same logic where it's only done once for the first port. If it can't be done there you should add a comment saying why it must be done after enabling the first port. > + > CS_LOCK(drvdata->base); > > return ret; > diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h > index b651372d4c88..00d146960d81 100644 > --- a/drivers/hwtracing/coresight/coresight-tpda.h > +++ b/drivers/hwtracing/coresight/coresight-tpda.h > @@ -9,6 +9,7 @@ > #define TPDA_CR (0x000) > #define TPDA_Pn_CR(n) (0x004 + (n * 4)) > #define TPDA_FPID_CR (0x084) > +#define TPDA_SYNCR (0x08C) > > /* Cross trigger FREQ packets timestamp bit */ > #define TPDA_CR_FREQTS BIT(2)

2 weeks, 3 days

Re: [PATCH v1 1/3] coresight: tpda: Add sysfs node for tpda cross-trigger configuration

by James Clark

2 weeks, 3 days

Re: [PATCH v1 1/3] coresight: tpda: Add sysfs node for tpda cross-trigger configuration

by James Clark

On 26/08/2025 8:01 am, Jie Gan wrote: > From: Tao Zhang <tao.zhang(a)oss.qualcomm.com> > > Introduce sysfs nodes to configure cross-trigger parameters for TPDA. > These registers define the characteristics of cross-trigger packets, > including generation frequency and flag values. > > Signed-off-by: Tao Zhang <tao.zhang(a)oss.qualcomm.com> > Co-developed-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > Signed-off-by: Jie Gan <jie.gan(a)oss.qualcomm.com> > --- > .../testing/sysfs-bus-coresight-devices-tpda | 43 ++++ > drivers/hwtracing/coresight/coresight-tpda.c | 241 ++++++++++++++++++ > drivers/hwtracing/coresight/coresight-tpda.h | 27 ++ > 3 files changed, 311 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda > > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda > new file mode 100644 > index 000000000000..e827396a0fa1 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpda > @@ -0,0 +1,43 @@ > +What: /sys/bus/coresight/devices/<tpda-name>/trig_async_enable > +Date: August 2025 > +KernelVersion: 6.17 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Tao Zhang <tao.zhang(a)oss.qualcomm.com>, Jie Gan <jie.gan(a)oss.qualcomm.com> > +Description: > + (RW) Enable/disable cross trigger synchronization sequence interface. > + > +What: /sys/bus/coresight/devices/<tpda-name>/trig_flag_ts_enable > +Date: August 2025 > +KernelVersion: 6.17 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Tao Zhang <tao.zhang(a)oss.qualcomm.com>, Jie Gan <jie.gan(a)oss.qualcomm.com> > +Description: > + (RW) Enable/disable cross trigger FLAG packet request interface. > + > +What: /sys/bus/coresight/devices/<tpda-name>/trig_freq_enable > +Date: August 2025 > +KernelVersion: 6.17 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Tao Zhang <tao.zhang(a)oss.qualcomm.com>, Jie Gan <jie.gan(a)oss.qualcomm.com> > +Description: > + (RW) Enable/disable cross trigger FREQ packet request interface. > + > +What: /sys/bus/coresight/devices/<tpda-name>/freq_ts_enable > +Date: August 2025 > +KernelVersion: 6.17 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Tao Zhang <tao.zhang(a)oss.qualcomm.com>, Jie Gan <jie.gan(a)oss.qualcomm.com> > +Description: > + (RW) Enable/disable the timestamp for all FREQ packets. > + > +What: /sys/bus/coresight/devices/<tpda-name>/global_flush_req > +Date: August 2025 > +KernelVersion: 6.17 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Tao Zhang <tao.zhang(a)oss.qualcomm.com>, Jie Gan <jie.gan(a)oss.qualcomm.com> > +Description: > + (RW) Set/unset global (all ports) flush request bit. The bit remains set until a I don't think you can unset? global_flush_req_store() only does something for set. > + global flush request sequence completes. > + > +What: /sys/bus/coresight/devices/<tpda-name>/cmbchan_mode > +Date: August 2025 > +KernelVersion: 6.17 > +Contact: Jinlong Mao <jinlong.mao(a)oss.qualcomm.com>, Tao Zhang <tao.zhang(a)oss.qualcomm.com>, Jie Gan <jie.gan(a)oss.qualcomm.com> > +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. > diff --git a/drivers/hwtracing/coresight/coresight-tpda.c b/drivers/hwtracing/coresight/coresight-tpda.c > index 4e93fa5bace4..cc254d53b8ec 100644 > --- a/drivers/hwtracing/coresight/coresight-tpda.c > +++ b/drivers/hwtracing/coresight/coresight-tpda.c > @@ -156,9 +156,37 @@ static void tpda_enable_pre_port(struct tpda_drvdata *drvdata) > u32 val; > > val = readl_relaxed(drvdata->base + TPDA_CR); > + val &= ~TPDA_CR_MID; > val &= ~TPDA_CR_ATID; > val |= FIELD_PREP(TPDA_CR_ATID, drvdata->atid); > + if (drvdata->trig_async) > + val = val | TPDA_CR_SRIE; > + else > + val = val & ~TPDA_CR_SRIE; val |= val &= > + if (drvdata->trig_flag_ts) > + val = val | TPDA_CR_FLRIE; > + else > + val = val & ~TPDA_CR_FLRIE; > + if (drvdata->trig_freq) > + val = val | TPDA_CR_FRIE; > + else > + val = val & ~TPDA_CR_FRIE; > + if (drvdata->freq_ts) > + val = val | TPDA_CR_FREQTS; > + else > + val = val & ~TPDA_CR_FREQTS; > + if (drvdata->cmbchan_mode) > + val = val | TPDA_CR_CMBCHANMODE; > + else > + val = val & ~TPDA_CR_CMBCHANMODE; > writel_relaxed(val, drvdata->base + TPDA_CR); > + > + /* > + * If FLRIE bit is set, set the master and channel > + * id as zero > + */ > + if (drvdata->trig_flag_ts) > + writel_relaxed(0x0, drvdata->base + TPDA_FPID_CR); > } > > static int tpda_enable_port(struct tpda_drvdata *drvdata, int port) > @@ -274,6 +302,217 @@ static const struct coresight_ops tpda_cs_ops = { > .link_ops = &tpda_link_ops, > }; > > +static ssize_t trig_async_enable_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); > + > + return sysfs_emit(buf, "%u\n", (unsigned int)drvdata->trig_async); > +} > + > +static ssize_t trig_async_enable_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t size) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); > + unsigned long val; > + > + if (kstrtoul(buf, 0, &val)) > + return -EINVAL; > + > + guard(spinlock)(&drvdata->spinlock); > + if (val) > + drvdata->trig_async = true; > + else > + drvdata->trig_async = false; > + drvdata->trig_async = !!val same with all the following ones too > + return size; > +} > +static DEVICE_ATTR_RW(trig_async_enable); > + > +static ssize_t trig_flag_ts_enable_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); > + > + return sysfs_emit(buf, "%u\n", (unsigned int)drvdata->trig_flag_ts); > +} > + > +static ssize_t trig_flag_ts_enable_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t size) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); > + unsigned long val; > + > + if (kstrtoul(buf, 0, &val)) > + return -EINVAL; > + > + guard(spinlock)(&drvdata->spinlock); > + if (val) > + drvdata->trig_flag_ts = true; > + else > + drvdata->trig_flag_ts = false; > + > + return size; > +} > +static DEVICE_ATTR_RW(trig_flag_ts_enable); > + > +static ssize_t trig_freq_enable_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); > + > + return sysfs_emit(buf, "%u\n", (unsigned int)drvdata->trig_freq); > +} > + > +static ssize_t trig_freq_enable_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t size) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); > + unsigned long val; > + > + if (kstrtoul(buf, 0, &val)) > + return -EINVAL; > + > + guard(spinlock)(&drvdata->spinlock); > + if (val) > + drvdata->trig_freq = true; > + else > + drvdata->trig_freq = false; > + > + return size; > +} > +static DEVICE_ATTR_RW(trig_freq_enable); > + > +static ssize_t freq_ts_enable_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); > + > + return sysfs_emit(buf, "%u\n", (unsigned int)drvdata->freq_ts); > +} > + > +static ssize_t freq_ts_enable_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t size) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); > + unsigned long val; > + > + if (kstrtoul(buf, 0, &val)) > + return -EINVAL; > + > + guard(spinlock)(&drvdata->spinlock); > + if (val) > + drvdata->freq_ts = true; > + else > + drvdata->freq_ts = false; > + > + return size; > +} > +static DEVICE_ATTR_RW(freq_ts_enable); > + > +static ssize_t global_flush_req_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); > + unsigned long val; > + > + guard(spinlock)(&drvdata->spinlock); > + if (!drvdata->csdev->refcnt) > + return -EPERM; -EPERM doesn't seem right, maybe EBUSY or EINVAL? Also don't you need CS_UNLOCK() for reading? I'm not 100% sure but I found one example of it in debug_init_arch_data(). > + > + val = readl_relaxed(drvdata->base + TPDA_CR); > + return sysfs_emit(buf, "%lx\n", val); > +} > + > +static ssize_t global_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); > + unsigned long val; > + > + if (kstrtoul(buf, 0, &val)) > + return -EINVAL; > + if (!val) return size; Check this first, no point in taking the spinlock or checking the refcount if you aren't going to do anything. > + guard(spinlock)(&drvdata->spinlock); > + if (!drvdata->csdev->refcnt) > + return -EPERM; ditto for -EPERM > + > + if (val) { > + CS_UNLOCK(drvdata->base); > + val = readl_relaxed(drvdata->base + TPDA_CR); > + val = val | BIT(0); > + writel_relaxed(val, drvdata->base + TPDA_CR); > + CS_LOCK(drvdata->base); > + } > + > + return size; > +} > +static DEVICE_ATTR_RW(global_flush_req); > + > +static ssize_t cmbchan_mode_show(struct device *dev, > + struct device_attribute *attr, > + char *buf) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); > + > + return sysfs_emit(buf, "%u\n", (unsigned int)drvdata->cmbchan_mode); > +} > + > +static ssize_t cmbchan_mode_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, > + size_t size) > +{ > + struct tpda_drvdata *drvdata = dev_get_drvdata(dev->parent); > + bool val; > + > + if (kstrtobool(buf, &val)) > + return -EINVAL; > + > + guard(spinlock)(&drvdata->spinlock); > + if (val) > + drvdata->cmbchan_mode = true; > + else > + drvdata->cmbchan_mode = false; > + > + return size; > +} > +static DEVICE_ATTR_RW(cmbchan_mode); > + > +static struct attribute *tpda_attrs[] = { > + &dev_attr_trig_async_enable.attr, > + &dev_attr_trig_flag_ts_enable.attr, > + &dev_attr_trig_freq_enable.attr, > + &dev_attr_freq_ts_enable.attr, > + &dev_attr_global_flush_req.attr, > + &dev_attr_cmbchan_mode.attr, > + NULL, > +}; > + > +static struct attribute_group tpda_attr_grp = { > + .attrs = tpda_attrs, > +}; > + > +static const struct attribute_group *tpda_attr_grps[] = { > + &tpda_attr_grp, > + NULL, > +}; > + > static int tpda_init_default_data(struct tpda_drvdata *drvdata) > { > int atid; > @@ -289,6 +528,7 @@ static int tpda_init_default_data(struct tpda_drvdata *drvdata) > return atid; > > drvdata->atid = atid; > + drvdata->freq_ts = true; > return 0; > } > > @@ -332,6 +572,7 @@ static int tpda_probe(struct amba_device *adev, const struct amba_id *id) > desc.ops = &tpda_cs_ops; > desc.pdata = adev->dev.platform_data; > desc.dev = &adev->dev; > + desc.groups = tpda_attr_grps; > desc.access = CSDEV_ACCESS_IOMEM(base); > drvdata->csdev = coresight_register(&desc); > if (IS_ERR(drvdata->csdev)) > diff --git a/drivers/hwtracing/coresight/coresight-tpda.h b/drivers/hwtracing/coresight/coresight-tpda.h > index c6af3d2da3ef..b651372d4c88 100644 > --- a/drivers/hwtracing/coresight/coresight-tpda.h > +++ b/drivers/hwtracing/coresight/coresight-tpda.h > @@ -8,17 +8,34 @@ > > #define TPDA_CR (0x000) > #define TPDA_Pn_CR(n) (0x004 + (n * 4)) > +#define TPDA_FPID_CR (0x084) > + > +/* Cross trigger FREQ packets timestamp bit */ > +#define TPDA_CR_FREQTS BIT(2) > +/* Cross trigger FREQ packet request bit */ > +#define TPDA_CR_FRIE BIT(3) > +/* Cross trigger FLAG packet request interface bit */ > +#define TPDA_CR_FLRIE BIT(4) > +/* Cross trigger synchronization bit */ > +#define TPDA_CR_SRIE BIT(5) > +/* Packetize CMB/MCMB traffic bit */ > +#define TPDA_CR_CMBCHANMODE BIT(20) > + > /* Aggregator port enable bit */ > #define TPDA_Pn_CR_ENA BIT(0) > /* Aggregator port CMB data set element size bit */ > #define TPDA_Pn_CR_CMBSIZE GENMASK(7, 6) > /* Aggregator port DSB data set element size bit */ > #define TPDA_Pn_CR_DSBSIZE BIT(8) > +/* Mode control bit */ > +#define TPDA_MODE_CTRL BIT(12) > > #define TPDA_MAX_INPORTS 32 > > /* Bits 6 ~ 12 is for atid value */ > #define TPDA_CR_ATID GENMASK(12, 6) > +/* Bits 13 ~ 19 is for mid value */ > +#define TPDA_CR_MID GENMASK(19, 13) > > /** > * struct tpda_drvdata - specifics associated to an TPDA component > @@ -29,6 +46,11 @@ > * @enable: enable status of the component. > * @dsb_esize Record the DSB element size. > * @cmb_esize Record the CMB element size. > + * @trig_async: Enable/disable cross trigger synchronization sequence interface. > + * @trig_flag_ts: Enable/disable cross trigger FLAG packet request interface. > + * @trig_freq: Enable/disable cross trigger FREQ packet request interface. > + * @freq_ts: Enable/disable the timestamp for all FREQ packets. > + * @cmbchan_mode: Configure the CMB/MCMB channel mode. > */ > struct tpda_drvdata { > void __iomem *base; > @@ -38,6 +60,11 @@ struct tpda_drvdata { > u8 atid; > u32 dsb_esize; > u32 cmb_esize; > + bool trig_async; > + bool trig_flag_ts; > + bool trig_freq; > + bool freq_ts; > + bool cmbchan_mode; > }; > > #endif /* _CORESIGHT_CORESIGHT_TPDA_H */

2 weeks, 3 days

Re: [PATCH v2 0/3] coresight-tnoc: Add support for Interconnect TNOC

by yuanfang zhang

On 8/19/2025 7:30 PM, Krzysztof Kozlowski wrote: > On 19/08/2025 12:27, Yuanfang Zhang wrote: >> This patch series adds support for the Qualcomm CoreSight Interconnect TNOC >> (Trace Network On Chip) block, which acts as a CoreSight graph link forwarding >> trace data from subsystems to the Aggregator TNOC. Unlike the Aggregator TNOC, >> this block does not support aggregation or ATID assignment. >> >> Signed-off-by: Yuanfang Zhang <yuanfang.zhang(a)oss.qualcomm.com> >> --- >> Changes in v2: >> - Refactor the dt-binding file. > > Everything is rafactor. What changed specifically? > > Or you just ignored prevous feedback and did other changes? > The feedback has not been ignored, i will update the detail in next patch. > > > Best regards, > Krzysztof

3 weeks, 3 days

Re: [PATCH v2 1/3] dt-bindings: arm: qcom: Add Coresight Interconnect TNOC

by yuanfang zhang

On 8/20/2025 3:45 AM, Rob Herring wrote: > On Tue, Aug 19, 2025 at 03:27:43AM -0700, Yuanfang Zhang wrote: >> Add device tree binding for Qualcomm Coresight Interconnect Trace >> Network On Chip (ITNOC). This TNOC acts as a CoreSight >> graph link that forwards trace data from a subsystem to the >> Aggregator TNOC, without aggregation or ATID functionality. >> >> Signed-off-by: Yuanfang Zhang <yuanfang.zhang(a)oss.qualcomm.com> >> --- >> .../bindings/arm/qcom,coresight-itnoc.yaml | 96 ++++++++++++++++++++++ >> 1 file changed, 96 insertions(+) >> >> diff --git a/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml b/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml >> new file mode 100644 >> index 0000000000000000000000000000000000000000..a3b5f2b949f69617a014d0ae2831c9c767178f8c >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/arm/qcom,coresight-itnoc.yaml >> @@ -0,0 +1,96 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/arm/qcom,coresight-itnoc.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Qualcomm Interconnect Trace Network On Chip - ITNOC >> + >> +maintainers: >> + - Yuanfang Zhang <yuanfang.zhang(a)oss.qualcomm.com> >> + >> +description: >> + The Interconnect TNOC is a CoreSight graph link that forwards trace data >> + from a subsystem to the Aggregator TNOC. Compared to Aggregator TNOC, it >> + does not have aggregation and ATID functionality. >> + >> +properties: >> + $nodename: >> + pattern: "^itnoc(@[0-9a-f]+)?$" >> + >> + compatible: >> + const: qcom,coresight-itnoc >> + >> + reg: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> + clock-names: >> + items: >> + - const: apb >> + >> + in-ports: >> + $ref: /schemas/graph.yaml#/properties/ports > >> + properties: >> + '#address-cells': >> + const: 1 >> + '#size-cells': >> + const: 0 > > Drop these. Already defined by the above ref. > sure, will update. >> + >> + patternProperties: >> + '^port(@[0-9a-f]{1,2})?$': >> + description: Input connections from CoreSight Trace Bus >> + $ref: /schemas/graph.yaml#/properties/port >> + >> + out-ports: >> + $ref: /schemas/graph.yaml#/properties/ports >> + additionalProperties: false >> + >> + properties: >> + port: >> + description: out connections to aggregator TNOC >> + $ref: /schemas/graph.yaml#/properties/port >> + >> +required: >> + - compatible >> + - reg >> + - clocks >> + - clock-names >> + - in-ports >> + - out-ports >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + itnoc@109ac000 { >> + compatible = "qcom,coresight-itnoc"; >> + reg = <0x109ac000 0x1000>; >> + >> + clocks = <&aoss_qmp>; >> + clock-names = "apb"; >> + >> + in-ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + port@0 { >> + reg = <0>; >> + tn_ic_in_tpdm_dcc: endpoint { >> + remote-endpoint = <&tpdm_dcc_out_tn_ic>; >> + }; >> + }; >> + }; >> + >> + out-ports { >> + port { >> + tn_ic_out_tnoc_aggr: endpoint { >> + /* to Aggregator TNOC input */ >> + remote-endpoint = <&tn_ag_in_tn_ic>; >> + }; >> + }; >> + }; >> + }; >> +... >> >> -- >> 2.34.1 >>

3 weeks, 3 days

Re: [PATCH v2 1/3] dt-bindings: arm: qcom: Add Coresight Interconnect TNOC

by yuanfang zhang

On 8/19/2025 8:32 PM, Rob Herring (Arm) wrote: > > On Tue, 19 Aug 2025 03:27:43 -0700, Yuanfang Zhang wrote: >> Add device tree binding for Qualcomm Coresight Interconnect Trace >> Network On Chip (ITNOC). This TNOC acts as a CoreSight >> graph link that forwards trace data from a subsystem to the >> Aggregator TNOC, without aggregation or ATID functionality. >> >> Signed-off-by: Yuanfang Zhang <yuanfang.zhang(a)oss.qualcomm.com> >> --- >> .../bindings/arm/qcom,coresight-itnoc.yaml | 96 ++++++++++++++++++++++ >> 1 file changed, 96 insertions(+) >> > > My bot found errors running 'make dt_binding_check' on your patch: > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > > > doc reference errors (make refcheckdocs): > > See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20250819-itn… > > The base for the series is generally the latest rc1. A different dependency > should be noted in *this* patch. > > If you already ran 'make dt_binding_check' and didn't see the above > error(s), then make sure 'yamllint' is installed and dt-schema is up to > date: > > pip3 install dtschema --upgrade > > Please check and re-submit after running the above command yourself. Note > that DT_SCHEMA_FILES can be set to your schema file to speed up checking > your schema. However, it must be unset to test all examples with your schema. > Below is my dtschema and yamllint version, They should already be the latest version. Name: dtschema Version: 2025.8 Name: yamllint Version: 1.37.1 I ran below 'make dt_binding_check', don't get any error/warnings. make dt_binding_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/arm/qcom,coresight-tnoc.yaml

3 weeks, 3 days

[PATCH v2 0/3] coresight-tnoc: Add support for Interconnect TNOC

by Yuanfang Zhang

This patch series adds support for the Qualcomm CoreSight Interconnect TNOC (Trace Network On Chip) block, which acts as a CoreSight graph link forwarding trace data from subsystems to the Aggregator TNOC. Unlike the Aggregator TNOC, this block does not support aggregation or ATID assignment. Signed-off-by: Yuanfang Zhang <yuanfang.zhang(a)oss.qualcomm.com> --- Changes in v2: - Refactor the dt-binding file. - Change "atid" type from u32 to int, set it as "-EOPNOTSUPP" for non-AMBA device. - Link to v1: https://lore.kernel.org/r/20250815-itnoc-v1-0-62c8e4f7ad32@oss.qualcomm.com --- Yuanfang Zhang (3): dt-bindings: arm: qcom: Add Coresight Interconnect TNOC coresight-tnoc: add platform driver to support Interconnect TNOC coresight-tnoc: Add runtime PM support for Interconnect TNOC .../bindings/arm/qcom,coresight-itnoc.yaml | 96 ++++++++++++++ drivers/hwtracing/coresight/coresight-tnoc.c | 147 ++++++++++++++++++--- 2 files changed, 226 insertions(+), 17 deletions(-) --- base-commit: 2b52cf338d39d684a1c6af298e8204902c026aca change-id: 20250815-itnoc-460273d1b80c Best regards, -- Yuanfang Zhang <yuanfang.zhang(a)oss.qualcomm.com>

3 weeks, 3 days

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

CoreSight August 2025