Re: [PATCH 7/8] hwmon: (scmi) Register explicitly with Thermal Framework

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

 



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".

Thanks,
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