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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 1/16/22 12:14 AM, Dmitry Osipenko wrote:
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@xxxxxxxxx>
Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
   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.

And not for for emergency alarms either. I have a patch prepared to address
this, as part of a larger patch series. Unlike the patches in this series,
I considered adding events for those attributes an enhancement and not
a bug fix.

Guenter



[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux