Re: [PATCH v2] thermal: add temperature sensor support for tango SoC

[Mason <slash.tmp@free.fr>]

Hello Eduardo,

On 08/03/2016 22:48, Eduardo Valentin wrote:

> On Fri, Mar 04, 2016 at 04:52:47PM +0100, Mason wrote:
>
>>  drivers/thermal/Kconfig         |   6 +++
>>  drivers/thermal/Makefile        |   1 +
>>  drivers/thermal/tango_thermal.c | 113 ++++++++++++++++++++++++++++++++++++++++
> 
> Can you please add a device tree binding entry under
> Documentation/devicetree/bindings/thermal/

OK.

>> +config TANGO_THERMAL
>> +	tristate "Tango4 thermal management"
>> +	depends on ARCH_TANGO || COMPILE_TEST
>> +	help
>> +	  Enable SMP8758 temperature sensor driver.
> 
> checkpatch.pl --strict complains about this, could you please improve
> the description of your drivers help entry?

Is checkpatch.pl complaining that the help text is only one line long?
("please write a paragraph that describes the config symbol fully")
There is nothing more to say. TANGO_THERMAL=Y will just enable support
for the temperature sensors on the 8758... I guess I could say that I
plan to use it for passive cooling with cpufreq?

> Also, can you please add a comment on the logic here? Where the 41
> constant came from?

I've now changed the algorithm, based on a colleague's suggestion.
I'll try documenting the new algorithm (a trivial linear search).

>> +	priv->zone = thermal_zone_of_sensor_register(dev, 0, priv->base, &ops);
> 
> Why registering only for id 0 when you mentioned this device has three
> sensors?

I plan to define 3 distinct thermal zones (each containing a single sensor).

e.g. for the time being, I've only defined the CPU thermal zone:

	soc {
		cpu_temp: sensor@920100 {
			compatible = "sigma,smp8758-sensor";
			#thermal-sensor-cells = <0>;
			reg = <0x920100 12>;
		};
	};

	thermal-zones {
		cpu_thermal: cpu-thermal {
			polling-delay-passive = <2000>;	/* ms */
			polling-delay = <1000>;		/* ms */
			thermal-sensors = <&cpu_temp>;
		};
	};


Regards.