[PATCH v2 3/3] ARM: dts: aspeed: yosemite4: Add power module and ADC on Medusa Board

[Andrew Jeffery]

Hi Ricky,

On Wed, 2024-09-18 at 17:54 +0800, Delphine CC Chiu wrote:
> From: Ricky CX Wu <ricky.cx.wu.wiwynn at gmail.com>
> 
> Add RTQ6056 as 2nd source ADC sensor on Medusa Board.

Can you unpack why this is related in the commit message? I assume it's
for something like battery monitoring? An explanation would help
though.

> Add power sensors on Medusa board:
> - Add XDP710 as 2nd source HSC to monitor P48V PSU power.
> - Add MP5023 as P12V efuse (Driver exists but un-documented).
> - Add PMBUS sensors as P12V Delta Module.

Generally if you're listing multiple things the change does in the
commit message you should have split the patch up accordingly.

There's some good advice here:

https://docs.kernel.org/process/5.Posting.html#patch-preparation

and here:

https://github.com/axboe/liburing/blob/master/CONTRIBUTING.md?plain=1#L21-L32

> 
> Signed-off-by: Ricky CX Wu <ricky.cx.wu.wiwynn at gmail.com>
> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu at wiwynn.com>
> ---
>  .../aspeed/aspeed-bmc-facebook-yosemite4.dts  | 45 ++++++++++++++++++-
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
> index 98477792aa00..e486b9d78f61 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
> @@ -284,15 +284,25 @@ &i2c10 {
>  &i2c11 {
>  	status = "okay";
>  	power-sensor at 10 {
> -		compatible = "adi, adm1272";
> +		compatible = "adi,adm1272";
>  		reg = <0x10>;
>  	};

This was outright busted. The hunk is a fix, as is the second instance
below. Please separate these out into their own patch and add a Fixes:
tag to it.

>  
> +	power-sensor at 11 {
> +		compatible = "infineon,xdp710";
> +		reg = <0x11>;
> +	};
> +
>  	power-sensor at 12 {
> -		compatible = "adi, adm1272";
> +		compatible = "adi,adm1272";

(i.e. this one also)

>  		reg = <0x12>;
>  	};
>  
> +	power-sensor at 13 {
> +		compatible = "infineon,xdp710";
> +		reg = <0x13>;
> +	};
> +
>  	gpio at 20 {
>  		compatible = "nxp,pca9555";
>  		reg = <0x20>;
> @@ -321,6 +331,17 @@ gpio at 23 {
>  		#gpio-cells = <2>;
>  	};
>  
> +	power-sensor at 40 {
> +		compatible = "mps,mp5023";
> +		reg = <0x40>;
> +	};
> +
> +	adc at 41 {
> +		compatible = "richtek,rtq6056";
> +		reg = <0x41>;
> +		#io-channel-cells = <1>;
> +	};
> +
>  	temperature-sensor at 48 {
>  		compatible = "ti,tmp75";
>  		reg = <0x48>;
> @@ -345,6 +366,26 @@ eeprom at 54 {
>  		compatible = "atmel,24c256";
>  		reg = <0x54>;
>  	};
> +
> +	power-sensor at 62 {
> +		compatible = "pmbus";
> +		reg = <0x62>;
> +	};
> +
> +	power-sensor at 64 {
> +		compatible = "pmbus";
> +		reg = <0x64>;
> +	};
> +
> +	power-sensor at 65 {
> +		compatible = "pmbus";
> +		reg = <0x65>;
> +	};
> +
> +	power-sensor at 68 {
> +		compatible = "pmbus";
> +		reg = <0x68>;
> +	};

See the discussion on your proposed DT binding document; I expect these
will need to change.

Andrew