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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <eed04042-ce69-491f-b42e-7c173afdb234@quicinc.com>
Date: Thu, 17 Jul 2025 11:03:52 +0800
From: Jinlong Mao <quic_jinlmao@...cinc.com>
To: Mike Leach <mike.leach@...aro.org>
CC: Leo Yan <leo.yan@....com>, Suzuki K Poulose <suzuki.poulose@....com>,
        James Clark <james.clark@....com>, Rob Herring <robh@...nel.org>,
        Krzysztof
 Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Mathieu
 Poirier <mathieu.poirier@...aro.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>, <coresight@...ts.linaro.org>,
        <linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-arm-msm@...r.kernel.org>
Subject: Re: [PATCH v8 2/2] coresight: Add label sysfs node support



On 2025/7/16 18:45, Mike Leach wrote:
> Hi,
> 
> On Wed, 16 Jul 2025 at 03:43, Jinlong Mao <quic_jinlmao@...cinc.com> wrote:
>>
>>
>>
>> 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@...ts.linaro.org
>>>> To unsubscribe send an email to coresight-leave@...ts.linaro.org
>>
> 
> Revisiting this - the label DT attribute is purely optional, and
> provides context for the hardware instance.
> This code as written appears to add a "label" file to all devices,
> irrespective of if the label is set in the DT.or not, with blank
> labels where  the attribute is not present.
> The visibility of the sysfs attribute should be controlled so that it
> only appears if label is present in the DT.
> 
I will follow this.

Thanks
Jinlong Mao
> Mike
>