Re: [PATCH 6/6] hwmon: (lm90) Fix sysfs and udev notifications

[Dmitry Osipenko <digetx@gmail.com>]

16.01.2022 00:11, Guenter Roeck пишет:
> On 1/15/22 12:41 PM, Dmitry Osipenko wrote:
>> 15.01.2022 23:33, Dmitry Osipenko пишет:
>>> 11.01.2022 19:51, Guenter Roeck пишет:
>>>> sysfs and udev notifications need to be sent to the _alarm
>>>> attributes, not to the value attributes.
>>>>
>>>> Fixes: 94dbd23ed88c ("hwmon: (lm90) Use hwmon_notify_event()")
>>>> Cc: Dmitry Osipenko <digetx@gmail.com>
>>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>>> ---
>>>>   drivers/hwmon/lm90.c | 12 ++++++------
>>>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/hwmon/lm90.c b/drivers/hwmon/lm90.c
>>>> index ba01127c1deb..1c9493c70813 100644
>>>> --- a/drivers/hwmon/lm90.c
>>>> +++ b/drivers/hwmon/lm90.c
>>>> @@ -1808,22 +1808,22 @@ static bool lm90_is_tripped(struct
>>>> i2c_client *client, u16 *status)
>>>>         if (st & LM90_STATUS_LLOW)
>>>>           hwmon_notify_event(data->hwmon_dev, hwmon_temp,
>>>> -                   hwmon_temp_min, 0);
>>>> +                   hwmon_temp_min_alarm, 0);
>>>>       if (st & LM90_STATUS_RLOW)
>>>>           hwmon_notify_event(data->hwmon_dev, hwmon_temp,
>>>> -                   hwmon_temp_min, 1);
>>>> +                   hwmon_temp_min_alarm, 1);
>>>>       if (st2 & MAX6696_STATUS2_R2LOW)
>>>>           hwmon_notify_event(data->hwmon_dev, hwmon_temp,
>>>> -                   hwmon_temp_min, 2);
>>>> +                   hwmon_temp_min_alarm, 2);
>>>>       if (st & LM90_STATUS_LHIGH)
>>>>           hwmon_notify_event(data->hwmon_dev, hwmon_temp,
>>>> -                   hwmon_temp_max, 0);
>>>> +                   hwmon_temp_max_alarm, 0);
>>>>       if (st & LM90_STATUS_RHIGH)
>>>>           hwmon_notify_event(data->hwmon_dev, hwmon_temp,
>>>> -                   hwmon_temp_max, 1);
>>>> +                   hwmon_temp_max_alarm, 1);
>>>>       if (st2 & MAX6696_STATUS2_R2HIGH)
>>>>           hwmon_notify_event(data->hwmon_dev, hwmon_temp,
>>>> -                   hwmon_temp_max, 2);
>>>> +                   hwmon_temp_max_alarm, 2);
>>>
>>>
>>> IIUC, "alarm" is about the T_CRIT output line. While these attributes
>>> are about the ALERT line. Hence why "alert" notifications need to be
>>> sent to the unrelated "alarm" attributes? This change doesn't look
>>> right.
>>>
>>
>> Although, no. I see now that the "alarm_bits" in the code are about the
>> alerts. Should be okay then.
>>
> 
> Alarm attributes are really neither about interrupts nor about alerts.
> Alarm attributes alert userspace that a limit has been exceeded.
> How and if the driver notices that a limit has been exceeded is
> an implementation detail. In a specific implementation, alerts
> or interrupts can be used to notify a driver that a notable event
> has occurred on a given device, but technically that is not
> necessary. Polling would do just as well.

Datasheet refers to T_CRIT using the "alarm" term, this confused me a tad.

BTW, we don't have events wired for the temp_crit_alarm attribute.