Re: [PATCH V3 3/6] thermal: Register hwmon in thermal_zone_of_sensor_register_param()

[Marek Vasut <marek.vasut@gmail.com>]

On 2/6/19 12:24 AM, Eduardo Valentin wrote:
> On Mon, Jan 28, 2019 at 01:10:11PM +0100, Marek Vasut wrote:
>> On 1/15/19 1:35 AM, Marek Vasut wrote:
>>> On 12/22/18 3:19 AM, Marek Vasut wrote:
>>>> On 12/18/2018 10:44 PM, Eduardo Valentin wrote:
>>>>> On Mon, Dec 17, 2018 at 04:56:41PM +0100, marek.vasut@gmail.com wrote:
>>>>>> From: Marek Vasut <marek.vasut@gmail.com>
>>>>>>
>>>>>> Register hwmon sysfs interface in thermal_zone_of_sensor_register_param()
>>>>>> in case thermal_zone_params->no_hwmon is set to false. This behavior is
>>>>>> the same as thermal_zone_device_register().
>>>>>>
>>>>>> From: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>>>> Cc: Daniel Lezcano <daniel.lezcano@linaro.org>
>>>>>> Cc: Eduardo Valentin <edubezval@gmail.com>
>>>>>> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>
>>>>>> Cc: Zhang Rui <rui.zhang@intel.com>
>>>>>> Cc: linux-renesas-soc@vger.kernel.org
>>>>>> To: linux-pm@vger.kernel.org
>>>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com>
>>>>>> ---
>>>>>> V2: No change
>>>>>> V3: - Work around the From line and SoB line checkpatch warning
>>>>>>     - Reorder the SoB line at the end
>>>>>> ---
>>>>>>  drivers/thermal/of-thermal.c | 12 +++++++++++-
>>>>>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
>>>>>> index e1a303a5698c..5ccff7b678de 100644
>>>>>> --- a/drivers/thermal/of-thermal.c
>>>>>> +++ b/drivers/thermal/of-thermal.c
>>>>>> @@ -15,6 +15,7 @@
>>>>>>  #include <linux/string.h>
>>>>>>  
>>>>>>  #include "thermal_core.h"
>>>>>> +#include "thermal_hwmon.h"
>>>>>>  
>>>>>>  /***   Private data structures to represent thermal device tree data ***/
>>>>>>  
>>>>>> @@ -521,8 +522,15 @@ thermal_zone_of_sensor_register_params(struct device *dev, int sensor_id,
>>>>>>  		if (sensor_specs.np == sensor_np && id == sensor_id) {
>>>>>>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>>>>>>  							 data, ops);
>>>>>> -			if (!IS_ERR(tzd))
>>>>>> +			if (!IS_ERR(tzd)) {
>>>>>> +				tzd->tzp = tzp;
>>>>>
>>>>> So, here you will overwrite what was done in of_parse_thermal_zones().
>>>>> That means, after this point, property like sustainable power, slope and
>>>>> offset are gone.
>>>>
>>>> Hmmmmm, that was rather inobvious, indeed.
>>>>
>>>> Do you have some suggestion how to pass in the no_hwmon = false then ?
>>>> Since tzp->no_hwmon is set to true in of_parse_thermal_zones(), the
>>>> three drivers (stm32, rcar, rcar_gen3) seem to hack around it. I'd like
>>>> to clean that up.
>>>
> 
> Yeah, that is an issue.
> 
>>> Bump ?
>>
>> Bump again, any suggestions ?
> 
> Yeah, a couple of ideas have been proposed for this issue.
> 
> First most tempting one is to have a DT property per thermal zone.
> Making it linux specific, something prefixed by linux,<property>. I
> recall Amit Kutcheria trying something similar to this, but dont
> remember where that went. Frankly, this is a Linux thing, I am not
> convinced DT is really the right place to fix this.

DT is not the right place for this, DT describes hardware and this is
policy, so it shouldn't be in DT.

> Another hack that could be written is a module parameter for of-thermal
> that would reflect the no_hwmon value, globally. The down side here is
> you have to make sure all drivers match that no_hwmon value, right?

Indeed, it should be the driver which decides whether or not to register
a HWMON interface for the thermal zone.

I guess for now, I'll just discard this entire series and hack the data
structure like other drivers do, since I don't see any reasonable solution.

-- 
Best regards,
Marek Vasut