On Fri, Oct 28, 2022 at 08:58:58AM -0700, Guenter Roeck wrote: > On 10/28/22 08:35, Cristian Marussi wrote: > [ ... ] > > > > + /* > > > > + * Try to register a temperature sensor with the Thermal Framework: > > > > + * skip sensors not defined as part of any thermal zone (-ENODEV) but > > > > + * report any other errors related to misconfigured zones/sensors. > > > > + */ > > > > + tzd = devm_thermal_of_zone_register(dev, th_sensor->info->id, th_sensor, > > > > + &scmi_hwmon_thermal_ops); > > > > + if (IS_ERR(tzd)) { > > > > + devm_kfree(dev, th_sensor); > > > > + > > > > + if (PTR_ERR(tzd) != -ENODEV) > > > > + return PTR_ERR(tzd); > > > > + > > > > + dev_info(dev, "Sensor '%s' not attached to any thermal zone.\n", > > > > + sensor->name); > > > > > > There were complaints about this message as it is noisy. If you send > > > another version, please drop it unless attaching each sensor to a thermal > > > zone is strongly expected. If you don't send another version, I'll drop it > > > while applying. > > > > > > > Ok fine for me. I am waiting to have some feedback from Sudeep too, but > > I do not have plan for another version as of now. > > > > As a side note, though, I understand the 'noisiness' argument, but, > > sincerely this same message in the original HWMON code was the only > > reason why I spotted that something was wrong with the SCMI/HWMON > > interactions and discovered the indexes/ids mismatch...if not for > > that it would have gone un-noticed that a perfectly configured > > ThermalZone/Sensor was not working properly... > > (un-noticed at least until something would have been burnt to fire > > in my house .. joking :P) > > > > Good point. > > Did you ever check the returned error code ? Maybe we could use it to > distinguish "it is not attached to a thermal zone because it is not > associated with one" from "attaching to a thermal zone failed because > its configuration is bad/incomplete". > Yes, it is what I do already indeed, in this regards I mimicked what the hwmon-thermal bridge was doing. In scmi_thermal_sensor_register() this message is printed out only if Thermal registration returned -ENODEV and no err is reported (which means teh specified sensor was not found attached to any TZ), while in the caller of scmi_thermal_sensor_register() for any error returned but -ENOMEM I print: "Thermal zone misconfigured for %s. err=%d\n", since any error reported by Thermal other than ENODEV and ENOMEM means the DT parsing unveiled some configuration anomaly. Thanks, Cristian