linux-kernel - Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to Tegra PMC bindings

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 21 Aug 2014 09:38:29 -0600
From:	Stephen Warren <swarren@...dotorg.org>
To:	Thierry Reding <thierry.reding@...il.com>
CC:	Mikko Perttunen <mperttunen@...dia.com>,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-tegra@...r.kernel.org, wni@...dia.com
Subject: Re: [PATCH v2 1/5] of: Add descriptions of thermtrip properties to
 Tegra PMC bindings

On 08/21/2014 12:58 AM, Thierry Reding wrote:
> On Wed, Aug 20, 2014 at 02:16:49PM -0600, Stephen Warren wrote:
>> On 08/13/2014 06:41 AM, Mikko Perttunen wrote:
>>> Hardware-triggered thermal reset requires configuring the I2C
>>> reset procedure. This configuration is read from the device tree,
>>> so document the relevant properties in the binding documentation.
>>
>>> diff --git a/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt b/Documentation/devicetree/bindings/arm/tegra/nvidia,tegra20-pmc.txt
>>
>>> +Hardware-triggered thermal reset:
>>> +On Tegra30, Tegra114 and Tegra124, if the 'i2c-thermtrip' subnode exists,
>>> +hardware-triggered thermal reset will be enabled.
>>
>> "will be enabled" sounds like SW behaviour, whereas DT is suppose to
>> describe HW, and leave SW to define its own behaviour. I would suggest:
>>
>> Optional sub-nodes:
>> i2c-thermtrip: Describes how to power off the system in the event of a
>>    thermal emergency.
>>
>>> +Required properties for hardware-triggered thermal reset (inside 'i2c-thermtrip'):
>>
>> Simpler might be:
>>
>> Required properties for i2c-thermtrip node:
>>
>>> +- nvidia,pmu : Phandle to power management unit / PMIC handling poweroff
>>> +- nvidia,reg-addr : I2C register address to write poweroff command to
>>> +- nvidia,reg-data : Poweroff command to write to PMU
>>
>> Why are both the PMU/PMIC phandle and the register address/data required? I
>> thought the purpose of having the phandle was to allow the register address
>> and data to be queried from the PMU/PMIC driver.
>>
>> To me, it seems much simpler to get rid of the phandle and just hard-code
>> the I2C bus number, address, and data into this node, rather than having to
>> go query it from the PMU/PMIC driver, then find the I2C controller, then
>> query it for its ID (and hope that all HW modules that talk to I2C
>> controllers directly use the same numbering scheme...)
>
> I originally requested this to be changed. It seems wrong to duplicate
> information about the PMIC in both the PMIC device tree node and the
> i2c-thermtrip node if we can get the same information from the driver
> directly (via the phandle). It certainly requires a little more code,
> but at the advantage of not having to figure out the I2C controller
> hardware number and I2C slave addresses when writing the i2c-thermtrip
> node.

I cant see that argument, but surely the PMIC driver can also supply the 
"reg-addr" and "reg-data" values too, if it's already being queried for 
the I2C device address and bus number? The binding above appears to 
duplicate part of the information, while requiring querying of the other 
part.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/