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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

> -----Original Message-----
> From: Suzuki K Poulose <suzuki.poulose@xxxxxxx>
> Sent: Wednesday, December 18, 2024 9:38 AM
> To: Mao Jinlong <quic_jinlmao@xxxxxxxxxxx>; Mike Leach
> <mike.leach@xxxxxxxxxx>; James Clark <James.Clark@xxxxxxx>; Rob Herring
> <robh@xxxxxxxxxx>; Krzysztof Kozlowski <krzk+dt@xxxxxxxxxx>; Conor Dooley
> <conor+dt@xxxxxxxxxx>; Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>; Bjorn
> Andersson <andersson@xxxxxxxxxx>; Konrad Dybcio
> <konradybcio@xxxxxxxxxx>
> Cc: coresight@xxxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> devicetree@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; linux-arm-
> msm@xxxxxxxxxxxxxxx
> 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@xxxxxxxxxxx>
> > ---
> >   .../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@xxxxxxxxxxx>
> > +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@xxxxxxxxxx>
> >   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@xxxxxxxxxxx>
> > +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@xxxxxxxxxxx>, 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@xxxxxxxxxxx>
> > +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@xxxxxxxxxxxxxxxx To unsubscribe send an
> email to coresight-leave@xxxxxxxxxxxxxxxx





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux