[PATCH] platform/x86: think-lmi: Fix issues with duplicate attributes

[PATCH] platform/x86: think-lmi: Fix issues with duplicate attributes

[Mario Limonciello]

On an AMD based Lenovo T14, I find that the module doesn't work at
all, and instead has a traceback with messages like:

```
sysfs: cannot create duplicate filename '/devices/virtual/firmware-attributes/thinklmi/attributes/Reserved'
```

Check for duplicates before adding any attributes.

Fixes: a40cd7ef22fb ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/think-lmi.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
index d2644230b91f..b029d4a5bc3c 100644
--- a/drivers/platform/x86/think-lmi.c
+++ b/drivers/platform/x86/think-lmi.c
@@ -691,6 +691,13 @@ static int tlmi_sysfs_init(void)
 		if (!tlmi_priv.setting[i])
 			continue;
 
+		/* check for duplicate */
+		if (kset_find_obj(tlmi_priv.attribute_kset, tlmi_priv.setting[i]->display_name)) {
+			pr_debug("duplicate attribute name found - %s\n",
+				tlmi_priv.setting[i]->display_name);
+			continue;
+		}
+
 		/* Build attribute */
 		tlmi_priv.setting[i]->kobj.kset = tlmi_priv.attribute_kset;
 		ret = kobject_init_and_add(&tlmi_priv.setting[i]->kobj, &tlmi_attr_setting_ktype,
-- 
2.25.1

Re: [External] [PATCH] platform/x86: think-lmi: Fix issues with duplicate attributes

[Mark Pearson]

On 2021-06-22 1:55 p.m., Mario Limonciello wrote:
> On an AMD based Lenovo T14, I find that the module doesn't work at
> all, and instead has a traceback with messages like:
> 
> ```
> sysfs: cannot create duplicate filename '/devices/virtual/firmware-attributes/thinklmi/attributes/Reserved'
> ```
> 
> Check for duplicates before adding any attributes.
> 
> Fixes: a40cd7ef22fb ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/platform/x86/think-lmi.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
> index d2644230b91f..b029d4a5bc3c 100644
> --- a/drivers/platform/x86/think-lmi.c
> +++ b/drivers/platform/x86/think-lmi.c
> @@ -691,6 +691,13 @@ static int tlmi_sysfs_init(void)
>  		if (!tlmi_priv.setting[i])
>  			continue;
>  
> +		/* check for duplicate */
> +		if (kset_find_obj(tlmi_priv.attribute_kset, tlmi_priv.setting[i]->display_name)) {
> +			pr_debug("duplicate attribute name found - %s\n",
> +				tlmi_priv.setting[i]->display_name);
> +			continue;
> +		}
> +
>  		/* Build attribute */
>  		tlmi_priv.setting[i]->kobj.kset = tlmi_priv.attribute_kset;
>  		ret = kobject_init_and_add(&tlmi_priv.setting[i]->kobj, &tlmi_attr_setting_ktype,
> 
Thanks Mario - I don't think I'd tested it on the T14 AMD yet.

Change looks good to me
Mark

Re: [External] [PATCH] platform/x86: think-lmi: Fix issues with duplicate attributes

[Limonciello, Mario]

On 6/22/2021 14:55, Mark Pearson wrote:
> 
> On 2021-06-22 1:55 p.m., Mario Limonciello wrote:
>> On an AMD based Lenovo T14, I find that the module doesn't work at
>> all, and instead has a traceback with messages like:
>>
>> ```
>> sysfs: cannot create duplicate filename '/devices/virtual/firmware-attributes/thinklmi/attributes/Reserved'
>> ```
>>
>> Check for duplicates before adding any attributes.
>>
>> Fixes: a40cd7ef22fb ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/platform/x86/think-lmi.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
>> index d2644230b91f..b029d4a5bc3c 100644
>> --- a/drivers/platform/x86/think-lmi.c
>> +++ b/drivers/platform/x86/think-lmi.c
>> @@ -691,6 +691,13 @@ static int tlmi_sysfs_init(void)
>>   		if (!tlmi_priv.setting[i])
>>   			continue;
>>   
>> +		/* check for duplicate */
>> +		if (kset_find_obj(tlmi_priv.attribute_kset, tlmi_priv.setting[i]->display_name)) {
>> +			pr_debug("duplicate attribute name found - %s\n",
>> +				tlmi_priv.setting[i]->display_name);
>> +			continue;
>> +		}
>> +
>>   		/* Build attribute */
>>   		tlmi_priv.setting[i]->kobj.kset = tlmi_priv.attribute_kset;
>>   		ret = kobject_init_and_add(&tlmi_priv.setting[i]->kobj, &tlmi_attr_setting_ktype,
>>
> Thanks Mario - I don't think I'd tested it on the T14 AMD yet.
> 
> Change looks good to me
> Mark
> 

In further testing this is causing problems on unload (or there was 
already another problem). So Hans please hold off, I'll work out what's 
happening and send a follow up v2.

Mark - something I'm wondering though what does "Reserved" even mean? 
Should that really be exported?  Or should it be part of a dis-allow list?

Re: [External] [PATCH] platform/x86: think-lmi: Fix issues with duplicate attributes

[Mark Pearson]

On 2021-06-22 3:58 p.m., Limonciello, Mario wrote:
> On 6/22/2021 14:55, Mark Pearson wrote:
>>
>> On 2021-06-22 1:55 p.m., Mario Limonciello wrote:
>>> On an AMD based Lenovo T14, I find that the module doesn't work at
>>> all, and instead has a traceback with messages like:
>>>
>>> ```
>>> sysfs: cannot create duplicate filename
>>> '/devices/virtual/firmware-attributes/thinklmi/attributes/Reserved'
>>> ```
>>>
>>> Check for duplicates before adding any attributes.
>>>
>>> Fixes: a40cd7ef22fb ("platform/x86: think-lmi: Add WMI interface
>>> support on Lenovo platforms")
>>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>>> ---
>>>   drivers/platform/x86/think-lmi.c | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/think-lmi.c
>>> b/drivers/platform/x86/think-lmi.c
>>> index d2644230b91f..b029d4a5bc3c 100644
>>> --- a/drivers/platform/x86/think-lmi.c
>>> +++ b/drivers/platform/x86/think-lmi.c
>>> @@ -691,6 +691,13 @@ static int tlmi_sysfs_init(void)
>>>           if (!tlmi_priv.setting[i])
>>>               continue;
>>>   +        /* check for duplicate */
>>> +        if (kset_find_obj(tlmi_priv.attribute_kset,
>>> tlmi_priv.setting[i]->display_name)) {
>>> +            pr_debug("duplicate attribute name found - %s\n",
>>> +                tlmi_priv.setting[i]->display_name);
>>> +            continue;
>>> +        }
>>> +
>>>           /* Build attribute */
>>>           tlmi_priv.setting[i]->kobj.kset = tlmi_priv.attribute_kset;
>>>           ret = kobject_init_and_add(&tlmi_priv.setting[i]->kobj,
>>> &tlmi_attr_setting_ktype,
>>>
>> Thanks Mario - I don't think I'd tested it on the T14 AMD yet.
>>
>> Change looks good to me
>> Mark
>>
> 
> In further testing this is causing problems on unload (or there was
> already another problem). So Hans please hold off, I'll work out what's
> happening and send a follow up v2.
> 
> Mark - something I'm wondering though what does "Reserved" even mean?
> Should that really be exported?  Or should it be part of a dis-allow list?
> 
> 
As an aside, I did test unload so it was working previously.

I'll need to check this out to see what's going on - it's quite possible
the T14 AMD is giving you duplicate items. I'll do some digging. My
guess is "Reserved" is probably not something we're supposed to
configure - but it's just a guess

Mark