linux-kernel - Re: [PATCH v4 5/7] ARM: dts: aspeed: yosemite5: Correct power monitor shunt resistor

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <73c63bc2f5eaae2c931a4ced1462024bd5a24237.camel@codeconstruct.com.au>
Date: Mon, 09 Feb 2026 16:51:00 +1030
From: Andrew Jeffery <andrew@...econstruct.com.au>
To: Kevin Tung <kevin.tung.openbmc@...il.com>, Rob Herring
 <robh@...nel.org>,  Krzysztof Kozlowski	 <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Joel Stanley	 <joel@....id.au>
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-aspeed@...ts.ozlabs.org, linux-kernel@...r.kernel.org, Amithash
 Prasasd	 <amithash@...a.com>, Kevin Tung <Kevin.Tung@...ntatw.com>, Ken
 Chen	 <Ken.Chen@...ntatw.com>, Leo Yang <Leo-Yang@...ntatw.com>, Jackson
 Liu	 <Jackson.Liu@...ntatw.com>
Subject: Re: [PATCH v4 5/7] ARM: dts: aspeed: yosemite5: Correct power
 monitor shunt resistor

Hi Kevin,

On Fri, 2026-01-30 at 16:20 +0800, Kevin Tung wrote:
> The shunt resistor value defined in the DTS was incorrect and did not
> reflect the resistor value populated on the board. As a result, power
> and current readings derived from this value were inaccurate.

Okay, but how did it come to be incorrect in the devicetree? Was the
documentation wrong and so someone entered the wrong value? Was the
documentation correct but there was a misunderstanding? Did the
hardware design change and the devicetree is now just inappropriate for
your needs?

Essentially, what's preventing this from happening again?

> 
> This change 
> 

I'd rather you rephrase this to avoid using "This change ...".
Hopefully the commit message always refers to its change.

Generally, please write in the imperative mood (see [1])

[1]: https://docs.kernel.org/process/submitting-patches.html#describe-your-changes

> updates the DTS to use the correct shunt resistor value
> according to the hardware design, restoring accurate power and current
> measurements.
> 
> Signed-off-by: Kevin Tung <kevin.tung.openbmc@...il.com>
> ---
>  arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts
> index 84d3731b17f7c7c87338672bbcc859de2b89b722..524597a81365ef10cd03b67d35eeb88a965cbe0a 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts
> @@ -415,7 +415,7 @@ power-sensor@42 {
>  	power-monitor@43 {
>  		compatible = "lltc,ltc4287";
>  		reg = <0x43>;
> -		shunt-resistor-micro-ohms = <250>;
> +		shunt-resistor-micro-ohms = <100>;
>  	};
>  
>  	power-sensor@44 {
> @@ -461,25 +461,25 @@ eeprom@57 {
>  	power-monitor@58 {
>  		compatible = "renesas,isl28022";
>  		reg = <0x58>;
> -		shunt-resistor-micro-ohms = <1000>;
> +		shunt-resistor-micro-ohms = <10000>;
>  	};
>  
>  	power-monitor@59 {
>  		compatible = "renesas,isl28022";
>  		reg = <0x59>;
> -		shunt-resistor-micro-ohms = <1000>;
> +		shunt-resistor-micro-ohms = <10000>;
>  	};
>  
>  	power-monitor@5a {
>  		compatible = "renesas,isl28022";
>  		reg = <0x5a>;
> -		shunt-resistor-micro-ohms = <1000>;
> +		shunt-resistor-micro-ohms = <10000>;
>  	};
>  
>  	power-monitor@5b {
>  		compatible = "renesas,isl28022";
>  		reg = <0x5b>;
> -		shunt-resistor-micro-ohms = <1000>;
> +		shunt-resistor-micro-ohms = <10000>;
>  	};
>  
>  	psu@5c {
> @@ -723,13 +723,13 @@ gpio-expander@21 {
>  	power-sensor@40 {
>  		compatible = "ti,ina230";
>  		reg = <0x40>;
> -		shunt-resistor = <2000>;
> +		shunt-resistor = <1000>;
>  	};
>  
>  	power-sensor@41 {
>  		compatible = "ti,ina230";
>  		reg = <0x41>;
> -		shunt-resistor = <2000>;
> +		shunt-resistor = <1000>;
>  	};
>  
>  	power-sensor@42 {

The way the commit message is written suggests it was just one shunt
resistor, but that's clearly not the case.

Andrew