linux-kernel - Re: [PATCH] arm64: dts: ti: k3-am625-sk: Add power/temperature sensors

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <20250507120104.4mhuaabe5auukarn@banter>
Date: Wed, 7 May 2025 07:01:04 -0500
From: Nishanth Menon <nm@...com>
To: Sascha Hauer <s.hauer@...gutronix.de>
CC: Vignesh Raghavendra <vigneshr@...com>, Tero Kristo <kristo@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] arm64: dts: ti: k3-am625-sk: Add power/temperature
 sensors

On 15:24-20250505, Sascha Hauer wrote:
> The AM625-SK has six power sensors and two temperature sensors connected
> to I2C. Add them to the device tree.
> 
> Signed-off-by: Sascha Hauer <s.hauer@...gutronix.de>
> ---
> The AM625-SK has six power sensors and two temperature sensors connected
> to I2C. Add them to the device tree.

Sascha,

I suggest making this as overlay. The reason is as follows: AM625-SK
among other TI evms do have automated power measurement capability from
XDS110 (accessible via USB port for jtag - appears as a rudimentary
menu option). The way this works is that it uses TM4C1294NCPDT to use
I2C commands to control the INA226/231 depending on the evm.

This firmware should be flashed by default on production boards (if
not, starting up CCS[1], autodetects older firmware and updates - at
least to my understanding) - by the way, this firmware also does test
automation, such as boot mode switch control, reset control etc.

This is the primary framework meant to be used by test automation and
indeed it is the default inside TI.

Challenge here is this: if we make this default in Linux, the test
automation system configures the INA226/231 in a different sampling
mode depending on usecase etc Vs what Linux does (even though the
shunt and the bus voltage for a given INA is the same). And just like
Linux, the firmware power measurement logic has changed over the
years.

Anyways, while I know that the SoC and TM4C can both handle
multi-master, the challenge is the same INA controlled and
mix-configured by two masters (and there is no synchronization between
the two).

To avoid this entire conflict and headache, I suggest adding it as
overlay that can be applied depending on the preference of measurement
desired.

[1] https://www.ti.com/tool/CCSTUDIO

> ---
>  arch/arm64/boot/dts/ti/k3-am625-sk.dts | 68 ++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/ti/k3-am625-sk.dts b/arch/arm64/boot/dts/ti/k3-am625-sk.dts
> index 2fbfa371934575efc4e9118a705f062bdea55f4f..e900d3134c72dc2616e3820b273d84b0db64bed5 100644
> --- a/arch/arm64/boot/dts/ti/k3-am625-sk.dts
> +++ b/arch/arm64/boot/dts/ti/k3-am625-sk.dts
> @@ -193,6 +193,74 @@ exp1: gpio@22 {
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&main_gpio1_ioexp_intr_pins_default>;
>  	};
> +
> +	power-sensor@40 {
> +		compatible = "ti,ina231";
> +		reg = <0x40>;
> +		#io-channel-cells = <1>;
> +		label = "vdd_core";
> +		shunt-resistor = <10000>;
> +		vs-supply = <&vcc_3v3_sys>;
> +	};
> +
> +	power-sensor@41 {
> +		compatible = "ti,ina231";
> +		reg = <0x41>;
> +		#io-channel-cells = <1>;
> +		label = "vddr_core";
> +		shunt-resistor = <10000>;
> +		vs-supply = <&vcc_3v3_sys>;
> +	};
> +
> +	power-sensor@45 {
> +		compatible = "ti,ina231";
> +		reg = <0x45>;
> +		#io-channel-cells = <1>;
> +		label = "dvdd_1v8";
> +		shunt-resistor = <10000>;
> +		vs-supply = <&vcc_3v3_sys>;
> +	};
> +
> +	power-sensor@47 {
> +		compatible = "ti,ina231";
> +		reg = <0x47>;
> +		#io-channel-cells = <1>;
> +		label = "vdd_ddr";
> +		shunt-resistor = <10000>;
> +		vs-supply = <&vcc_3v3_sys>;
> +	};
> +
> +	temperature-sensor@48 {
> +		compatible = "ti,tmp100";
> +		reg = <0x48>;
> +		label = "soc";
> +		vs-supply = <&vcc_3v3_sys>;
> +	};
> +
> +	temperature-sensor@49 {
> +		compatible = "ti,tmp100";
> +		reg = <0x49>;
> +		label = "ddr";
> +		vs-supply = <&vcc_3v3_sys>;
> +	};
> +
> +	power-sensor@4c {
> +		compatible = "ti,ina231";
> +		reg = <0x4c>;
> +		#io-channel-cells = <1>;
> +		label = "dvdd_3v3";
> +		shunt-resistor = <10000>;
> +		vs-supply = <&vcc_3v3_sys>;
> +	};
> +
> +	power-sensor@4d {
> +		compatible = "ti,ina231";
> +		reg = <0x4d>;
> +		#io-channel-cells = <1>;
> +		label = "vdda_1v8";
> +		shunt-resistor = <10000>;
> +		vs-supply = <&vcc_3v3_sys>;
> +	};
>  };
>  
>  &sdhci1 {
> 
> ---
> base-commit: 92a09c47464d040866cf2b4cd052bc60555185fb
> change-id: 20250505-am625-sk-sensors-bb255e2baa47
> 
> Best regards,
> -- 
> Sascha Hauer <s.hauer@...gutronix.de>
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D