Re: [PATCH 2/3] lm90: initialize parameters from devicetree

Stéphan Kochen
2016-01-22T22:32:39Z

On Fri, Jan 22, 2016 at 06:53:44AM -0800, Guenter Roeck wrote:
> On 01/21/2016 11:34 AM, Stéphan Kochen wrote:
> >Allow a device tree to set initial temperature sensor parameters.
>
> This very much smells like configuration, not hardware description.
>
> Having said that, the thermal subsystem does something similar. This raises even more
> questions, though. Since patch 3 of the series introduces registration with the thermal
> subsystem, it seems to me that the thermal subsystem should provide any limits used
> to program the chips, and that there should be a mechanism for the thermal subsystem
> to interact with the driver to both set the limits and to be informed if a limit
> is exceeded.
>
> Also, _if_ such a set of properties is introduced and accepted by the devicetree
> reviewers, it should probably be a set of properties which applies to _all_ hardware
> monitoring drivers, not just to one. Even if support is not implemented immediately
> in the hwmon core, the properties should be usable in a generic way, and not
> reflect sysfs attribute names used by the hwmon subsystem.

The original kernel for Ouya is a 3.1 kernel from nVidia's old
Linux4Tegra branches. This kernel had its own Tegra-specific thermal
zone support which seems to do more or less that: the thermal zone
system actually uses the sensor alert interrupts to detect when it needs
to act, and reconfigures alert bounds as the temperature slides to
another range it wants to monitor.

Thermal zones in current master only have the ability to check sensor
values at an interval.

I'd agree that update-interval and the low/high bounds could be
controlled by the thermal zone system. I'm less certain what to do with
the critical/emergency bounds, and the remote-offset.

Thing about Ouya is that, even with the thermal zone currently working,
the alert bounds set when the system comes up seem to be crippling the
system with interrupts shortly after the sensor is activated. So they
have to be set to something sane. Hence why I added both.

I'm not sure if you're expecting me to do the work of extending thermal
zones. I guess I'd need to figure out how a sensor would notify a
thermal zone of an alert, and implement that functionality while staying
backwards compatible.

Thanks again,

--
Stéphan Kochen