| Message ID | 1556023014-6159-1-git-send-email-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | hwmon: (max6650) Drop call to thermal_cdev_update | expand |
> On Apr 23, 2019, at 08:36, Guenter Roeck <linux@roeck-us.net> wrote: > > The call to thermal_cdev_update() causes any fan connected to the chip > to stop immediately. If the thermal subsystem is not set up to actually > handle the chip as cooling device, the remains stopped until is is > restarted manually with a write to a sysfs attribute. > > There is evidence that thermal_cdev_update() should only be called from > thermal governors, not from thermal cooling device drivers. Drop the call. > > Cc: Jean-Francois Dagenais <jeff.dagenais@gmail.com> > Signed-off-by: Guenter Roeck <linux@roeck-us.net> Tested-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com> > > --- > drivers/hwmon/max6650.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c > index e977c2f2d74a..939953240827 100644 > --- a/drivers/hwmon/max6650.c > +++ b/drivers/hwmon/max6650.c > @@ -801,8 +801,6 @@ static int max6650_probe(struct i2c_client *client, > dev_warn(&client->dev, > "thermal cooling device register failed: %ld\n", > PTR_ERR(data->cooling_dev)); > - else > - thermal_cdev_update(data->cooling_dev); > #endif > return 0; > } > -- > 2.7.4 > I was just testing exactly this, and coming to the same conclusion and about to send a v6! ;) No need now.
On 4/23/19 5:42 AM, Jean-Francois Dagenais wrote: > >> On Apr 23, 2019, at 08:36, Guenter Roeck <linux@roeck-us.net> wrote: >> >> The call to thermal_cdev_update() causes any fan connected to the chip >> to stop immediately. If the thermal subsystem is not set up to actually >> handle the chip as cooling device, the remains stopped until is is >> restarted manually with a write to a sysfs attribute. >> >> There is evidence that thermal_cdev_update() should only be called from >> thermal governors, not from thermal cooling device drivers. Drop the call. >> >> Cc: Jean-Francois Dagenais <jeff.dagenais@gmail.com> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> > Tested-by: Jean-Francois Dagenais <jeff.dagenais@gmail.com> Thanks a lot! I thought about merging the two patches, but then concluded that your Tested-by: and the thoughts behind removing the call are valuable, and decided to keep the patches separate after all. Guenter
diff --git a/drivers/hwmon/max6650.c b/drivers/hwmon/max6650.c index e977c2f2d74a..939953240827 100644 --- a/drivers/hwmon/max6650.c +++ b/drivers/hwmon/max6650.c @@ -801,8 +801,6 @@ static int max6650_probe(struct i2c_client *client, dev_warn(&client->dev, "thermal cooling device register failed: %ld\n", PTR_ERR(data->cooling_dev)); - else - thermal_cdev_update(data->cooling_dev); #endif return 0; }
The call to thermal_cdev_update() causes any fan connected to the chip to stop immediately. If the thermal subsystem is not set up to actually handle the chip as cooling device, the remains stopped until is is restarted manually with a write to a sysfs attribute. There is evidence that thermal_cdev_update() should only be called from thermal governors, not from thermal cooling device drivers. Drop the call. Cc: Jean-Francois Dagenais <jeff.dagenais@gmail.com> Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/hwmon/max6650.c | 2 -- 1 file changed, 2 deletions(-)