[PATCH] hwmon: (scpi) Add slope and offset to SCP sensor readings

Carlo Caione carlo at endlessm.com
Wed Mar 1 09:09:29 PST 2017


On Wed, Mar 1, 2017 at 5:57 PM, Punit Agrawal <punit.agrawal at arm.com> wrote:
> Carlo Caione <carlo at endlessm.com> writes:
>>> Are you saying that SCPI does not specify or provide the units to be used
>>> when reading values, and thus effectively just reports a more or less
>>> random number ?
>>
>> AFAICT the standard does not specify the units to be used for the
>> values or a way to get that information. For the Amlogic case the SCP
>> returns the value of the temperature in degree celsius and I'm not
>> sure how common is that.
>
> The standard does specify the units, but the way it is written seems to
> suggest that the units are part of the platform implementation rather
> than part of the standard [0].
>
> [0] http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.dui0922g/CABBCJGH.html

Oh, I missed that, odd indeed. Fantastic, one more non-compliancy.

>> [cut]
>>>> -       *temp = value;
>>>> +       slope = thermal_zone_get_slope(zone->z);
>>>> +       offset = thermal_zone_get_offset(zone->z);
>>>
>>>
>>> This is conceptually wrong. The functions return -ENODEV if thermal is
>>> disabled.
>>> While a negative slope does not make sense, a negative offset does.
>>
>> Yeah, but in that case we would have already failed to register the
>> thermal zone at all in devm_thermal_zone_of_sensor_register().
>>
>
> In addition to the thermal sub-subsystem, hwmon sysfs interface also
> expects temperature in millidegree Celsius. Ideally, any change should
> fix the reporting there as well. More below.

[cut]

> Another way to fix this would be to add optional properties to the scpi
> sensors binding and use them instead. This could then be used to fixup
> values reported to both thermal and hwmon.

Yeah, if we want to fix also the hwmon side probably it's the best solution.

Cheers,

-- 
Carlo Caione  |  +39.340.80.30.096  |  Endless



More information about the linux-amlogic mailing list