RE: [PATCH v6 2/2] coresight: Add label sysfs node support

Mike Leach
2024-12-18T09:58:42Z

Hi

> -----Original Message-----
> From: Suzuki K Poulose <suzuki.poulose@arm.com>
> Sent: Wednesday, December 18, 2024 9:38 AM
> To: Mao Jinlong <quic_jinlmao@quicinc.com>; Mike Leach
> <mike.leach@linaro.org>; James Clark <James.Clark@arm.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Kozlowski <krzk+dt@kernel.org>; Conor Dooley
> <conor+dt@kernel.org>; Mathieu Poirier <mathieu.poirier@linaro.org>; Bjorn
> Andersson <andersson@kernel.org>; Konrad Dybcio
> <konradybcio@kernel.org>
> Cc: coresight@lists.linaro.org; linux-arm-kernel@lists.infradead.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> msm@vger.kernel.org
> Subject: Re: [PATCH v6 2/2] coresight: Add label sysfs node support
>
> On 17/12/2024 06:33, Mao Jinlong wrote:
> > For some coresight components like CTI and TPDM, there could be
> > numerous of them. From the node name, we can only get the type and
> > register address of the component. We can't identify the HW or the
> > system the component belongs to. Add label sysfs node support for
> > showing the intuitive name of the device.
> >
> > Signed-off-by: Mao Jinlong <quic_jinlmao@quicinc.com>
> > ---
> > .../testing/sysfs-bus-coresight-devices-cti | 6 ++++
> > .../sysfs-bus-coresight-devices-funnel | 6 ++++
> > .../testing/sysfs-bus-coresight-devices-tpdm | 6 ++++
> > drivers/hwtracing/coresight/coresight-sysfs.c | 32
> +++++++++++++++++++
> > 4 files changed, 50 insertions(+)
>
> Do you think we need to name the devices using the label ?
>

No - absolutely not. If we use label to name devices then we have to validate that the string is a correctly formed device name and that it has not been previously used.

Using the canonical driver selected names works best as we are guaranteed a unique name and the information label can be used to provide flexible context information that best matches the users requirements.

Mike

> Or is this enough ?

> Suzuki
>
>
> >
> > diff --git a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> > b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> > index bf2869c413e7..909670e0451a 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-cti
> > @@ -239,3 +239,9 @@ Date: March 2020
> > KernelVersion 5.7
> > Contact: Mike Leach or Mathieu Poirier
> > Description: (Write) Clear all channel / trigger programming.
> > +
> > +What: /sys/bus/coresight/devices/<cti-name>/label
> > +Date: Dec 2024
> > +KernelVersion 6.14
> > +Contact: Mao Jinlong <quic_jinlmao@quicinc.com>
> > +Description: (Read) Show hardware context information of device.
> > diff --git
> > a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> > b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> > index d75acda5e1b3..944aad879aeb 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-funnel
> > @@ -10,3 +10,9 @@ Date: November 2014
> > KernelVersion: 3.19
> > Contact: Mathieu Poirier <mathieu.poirier@linaro.org>
> > Description: (RW) Defines input port priority order.
> > +
> > +What: /sys/bus/coresight/devices/<memory_map>.funnel/label
> > +Date: Dec 2024
> > +KernelVersion 6.14
> > +Contact: Mao Jinlong <quic_jinlmao@quicinc.com>
> > +Description: (Read) Show hardware context information of device.
> > diff --git
> > a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > index bf710ea6e0ef..309802246398 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > +++ b/Documentation/ABI/testing/sysfs-bus-coresight-devices-tpdm
> > @@ -257,3 +257,9 @@ Contact: Jinlong Mao (QUIC)
> <quic_jinlmao@quicinc.com>, Tao Zhang (QUIC) <quic_t
> > Description:
> > (RW) Set/Get the MSR(mux select register) for the CMB
> subunit
> > TPDM.
> > +
> > +What: /sys/bus/coresight/devices/<tpdm-name>/label
> > +Date: Dec 2024
> > +KernelVersion 6.14
> > +Contact: Mao Jinlong <quic_jinlmao@quicinc.com>
> > +Description: (Read) Show hardware context information of device.
> > diff --git a/drivers/hwtracing/coresight/coresight-sysfs.c
> > b/drivers/hwtracing/coresight/coresight-sysfs.c
> > index a01c9e54e2ed..4af40cd7d75a 100644
> > --- a/drivers/hwtracing/coresight/coresight-sysfs.c
> > +++ b/drivers/hwtracing/coresight/coresight-sysfs.c
> > @@ -7,6 +7,7 @@
> > #include <linux/device.h>
> > #include <linux/idr.h>
> > #include <linux/kernel.h>
> > +#include <linux/property.h>
> >
> > #include "coresight-priv.h"
> > #include "coresight-trace-id.h"
> > @@ -366,18 +367,47 @@ static ssize_t enable_source_store(struct device
> *dev,
> > }
> > static DEVICE_ATTR_RW(enable_source);
> >
> > +static ssize_t label_show(struct device *dev,
> > + struct device_attribute *attr, char *buf) {
> > +
> > + const char *str;
> > + int ret = 0;
> > +
> > + ret = fwnode_property_read_string(dev_fwnode(dev), "label", &str);
> > + if (ret == 0)
> > + return scnprintf(buf, PAGE_SIZE, "%s\n", str);
> > + else
> > + return ret;
> > +}
> > +static DEVICE_ATTR_RO(label);
> > +
> > static struct attribute *coresight_sink_attrs[] = {
> > &dev_attr_enable_sink.attr,
> > + &dev_attr_label.attr,
> > NULL,
> > };
> > ATTRIBUTE_GROUPS(coresight_sink);
> >
> > static struct attribute *coresight_source_attrs[] = {
> > &dev_attr_enable_source.attr,
> > + &dev_attr_label.attr,
> > NULL,
> > };
> > ATTRIBUTE_GROUPS(coresight_source);
> >
> > +static struct attribute *coresight_link_attrs[] = {
> > + &dev_attr_label.attr,
> > + NULL,
> > +};
> > +ATTRIBUTE_GROUPS(coresight_link);
> > +
> > +static struct attribute *coresight_helper_attrs[] = {
> > + &dev_attr_label.attr,
> > + NULL,
> > +};
> > +ATTRIBUTE_GROUPS(coresight_helper);
> > +
> > const struct device_type coresight_dev_type[] = {
> > [CORESIGHT_DEV_TYPE_SINK] = {
> > .name = "sink",
> > @@ -385,6 +415,7 @@ const struct device_type coresight_dev_type[] = {
> > },
> > [CORESIGHT_DEV_TYPE_LINK] = {
> > .name = "link",
> > + .groups = coresight_link_groups,
> > },
> > [CORESIGHT_DEV_TYPE_LINKSINK] = {
> > .name = "linksink",
> > @@ -396,6 +427,7 @@ const struct device_type coresight_dev_type[] = {
> > },
> > [CORESIGHT_DEV_TYPE_HELPER] = {
> > .name = "helper",
> > + .groups = coresight_helper_groups,
> > }
> > };
> > /* Ensure the enum matches the names and groups */
>
> _______________________________________________
> CoreSight mailing list -- coresight@lists.linaro.org To unsubscribe send an
> email to coresight-leave@lists.linaro.org