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

From: Cristian Marussi <cristian.marussi@arm.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, sudeep.holla@arm.com,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	linux-hwmon@vger.kernel.org
Subject: Re: [PATCH 7/8] hwmon: (scmi) Register explicitly with Thermal Framework
Date: Fri, 28 Oct 2022 17:15:41 +0100	[thread overview]
Message-ID: <Y1wAHyV/tLKQmo7l@e120937-lin> (raw)
In-Reply-To: <e4040686-851c-d8b0-b274-ac71d38685e1@roeck-us.net>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel