Re: [PATCH v8 2/2] coresight: Add label sysfs node support

[Jinlong Mao <quic_jinlmao@quicinc.com>]

On 2025/7/3 22:19, Leo Yan wrote:
> On Thu, Jul 03, 2025 at 09:04:53PM +0800, Mao Jinlong wrote:
> 
> [...]
> 
>> +static ssize_t label_show(struct device *dev,
>> +		struct device_attribute *attr, char *buf)
>> +{
>> +
>> +	const char *str;
>> +	int ret = 0;
> 
> No need to init ret to 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);
>> +
> 
> This change adds a 'label' entry for source, link, helper, and sink
> components, but the documentation has only updated for three components:
> CTI, funnel, and TPDM.
> 
> Should we also update the documentation for all relevant components,
> such as ETM, ETR, etc.?
> 
> Additionally, patch 01 is missing the update to the ETM yaml file for
> the new property. I checked patch v4 [1], which includes a change to
> etm.yaml, but this change was dropped since v5. I briefly read the
> v4 discussion thread and didn't see any mention of removing the ETM
> related change. Did you see any particular issue when add label for
> ETM devices?
> 
> Overall, this series is fine for me. Just please ensure that all
> relevant components are covered for completeness.
> 
> Thanks,
> Leo
> 

I will update all coresight docs.

Thanks
Jinlong Mao

> [1] https://patchwork.kernel.org/project/linux-arm-msm/cover/20240703122340.26864-1-quic_jinlmao@quicinc.com/
> 
>>   const struct device_type coresight_dev_type[] = {
>>   	[CORESIGHT_DEV_TYPE_SINK] = {
>>   		.name = "sink",
>> @@ -390,6 +420,7 @@ const struct device_type coresight_dev_type[] = {
>>   	},
>>   	[CORESIGHT_DEV_TYPE_LINK] = {
>>   		.name = "link",
>> +		.groups = coresight_link_groups,
>>   	},
>>   	[CORESIGHT_DEV_TYPE_LINKSINK] = {
>>   		.name = "linksink",
>> @@ -401,6 +432,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 */
>> -- 
>> 2.17.1
>>
>> _______________________________________________
>> CoreSight mailing list -- coresight@lists.linaro.org
>> To unsubscribe send an email to coresight-leave@lists.linaro.org