> On Apr 18, 2019, at 16:12, Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > That explains why the other drivers don't generate an error message. > You might want to reconsider the dev_err() above; it appears it adds zero > value. > >> If I am wrong, then pwm-fan.c should also be visited since my max6650_probe code >> was copy-pasted from it. >> > > You are correct, that should not fail either. But two wrongs don't make it right. So you think any fan chip driver probe function in hwmon should succeed even if (devm_)thermal_of_cooling_device_register fails?. Mmmh... perhaps I am missing something, but I tend to disagree. That means that for example my system boots normally without any errors, yet the thermal-zone I defined, which is there to prevent my system from burning out, would remain incomplete (without required cooling device) and yet no errors came out of the kernel. I just tested it and that's exactly what's happening! Yikes. > >> >> On a related note, I am worried about everyone having to progressively add >> thermal-zone support to each fan chip in drivers/hwmon. Would it not be possible >> for hwmon to detect, through `enum hwmon_sensor_types` set to `hwmon_fan` in >> `struct hwmon_channel_info` to be registered as cooling device in thermal ? Of >> course for max665x to work, it would have to be modified to use >> `devm_hwmon_device_register_with_info` instead of >> `devm_hwmon_device_register_with_groups` >> > > Yes, that would be desirable. Patches welcome. And, yes, the driver would have > to be reworked to use the new API. > >> Another possiblity would be to create a new driver in drivers/thermal for >> max665x as a thermal cooling device. Or even a > > Only if we also drop the existing driver from hwmon. Fine with me if others > agree and let you do it (one less hwmon driver to maintain). > > Note that I just submitted a patch series to introduce > devm_thermal_of_cooling_device_register(). We'll see if the thermal maintainers > accept it. Saw that. For a minute I really thought you pulled out a series which did exactly what I was talking about (i.e. auto-registering fans in thermal cooling)!! Since my problem is fixed in the short term, and am very stretched for time with our project, I will not have time to do this now. Perhaps one day... ;)
