linux-kernel - Re: [PATCH 2/2] hwmon: iio: Add labels from IIO channels

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1439bdda-0e01-42da-a8ec-7a51ee3a5a08@linux.dev>
Date: Fri, 21 Jun 2024 11:31:06 -0400
From: Sean Anderson <sean.anderson@...ux.dev>
To: Guenter Roeck <linux@...ck-us.net>, Jonathan Cameron <jic23@...nel.org>,
 Jean Delvare <jdelvare@...e.com>, linux-iio@...r.kernel.org,
 linux-hwmon@...r.kernel.org
Cc: Lars-Peter Clausen <lars@...afoo.de>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] hwmon: iio: Add labels from IIO channels

On 6/21/24 11:22, Sean Anderson wrote:
> On 6/21/24 11:08, Guenter Roeck wrote:
>> On 6/20/24 14:13, Sean Anderson wrote:
>>> Add labels from IIO channels to our channels. This allows userspace to
>>> display more meaningful names instead of "in0" or "temp5".
>>>
>>> Signed-off-by: Sean Anderson <sean.anderson@...ux.dev>
>>> ---
>>>
>>>   drivers/hwmon/iio_hwmon.c | 33 ++++++++++++++++++++++++++++++---
>>>   1 file changed, 30 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/iio_hwmon.c b/drivers/hwmon/iio_hwmon.c
>>> index 4c8a80847891..588b64c18e63 100644
>>> --- a/drivers/hwmon/iio_hwmon.c
>>> +++ b/drivers/hwmon/iio_hwmon.c
>>> @@ -33,6 +33,17 @@ struct iio_hwmon_state {
>>>       struct attribute **attrs;
>>>   };
>>>   +static ssize_t iio_hwmon_read_label(struct device *dev,
>>> +                  struct device_attribute *attr,
>>> +                  char *buf)
>>> +{
>>> +    struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
>>> +    struct iio_hwmon_state *state = dev_get_drvdata(dev);
>>> +    struct iio_channel *chan = &state->channels[sattr->index];
>>> +
>>> +    return iio_read_channel_label(chan, buf);
>> 
>> This can return -EINVAL if there is no label. Since the label attribute
>> is created unconditionally, every affected system would end up with
>> lots of error messages when running the "sensors" command.
>> This is not acceptable.
> 
> The sensors command gracefully handles this. There are no errors, and the label is unused.

For example, without IIO labels I get:

$ sensors hwmon_ams_ps-isa-0000
hwmon_ams_ps-isa-0000
Adapter: ISA adapter
in1:         825.00 mV 
in2:         826.00 mV 
in3:           1.81 V  
in4:           1.18 V  
in5:           1.80 V  
in6:           1.80 V  
in7:           3.27 V  
in8:           1.81 V  
in9:         825.00 mV 
in10:          1.81 V  
in11:          1.80 V  
temp1:        +79.8 C  
temp2:        +80.9 C  

and with labels I get

$ sensors hwmon_ams_ps-isa-0000
hwmon_ams_ps-isa-0000
Adapter: ISA adapter
VCC_PSINTLP: 824.00 mV 
VCC_PSINTFP: 822.00 mV 
VCC_PSAUX:     1.81 V  
VCC_PSDDR:     1.18 V  
VCC_PSIO3:     1.80 V  
VCC_PSIO0:     1.80 V  
VCC_PSIO1:     3.27 V  
VCC_PSIO2:     1.80 V  
PS_MGTRAVCC: 822.00 mV 
PS_MGTRAVTT:   1.81 V  
VCC_PSADC:     1.80 V  
Temp_LPD:     +79.5 C  
Temp_FPD:     +80.2 C  

I also was concerned about the same thing, but lm-sensors handles things
gracefully, allowing us to simplify the implementation.

--Sean