Re: [PATCH 1/4] arm64:?==?utf-8?q? dts: rockchip: Move eeprom correct bus for Radxa ROCK 5A

[Dragan Simic]

Hello Naoki,

Thanks for this patch.  Please, see a couple of remarks below.

On Tuesday, November 11, 2025 06:09 CET, FUKAUMI Naoki <naoki at radxa.com> wrote:
> BL24C16 EEPROM chip found on Radxa ROCK 5A is connected on i2c0 bus.
> Move eeprom node from i2c2 to i2c0.

I'd suggest that the patch subject is reworded a bit like this:

  arm64: dts: rockchip: Move the EEPROM to correct I2C bus on Radxa ROCK 5A

Also, the patch description would benefit from becoming reworded
like this:

  The BL24C16 EEPROM chip found on Radxa ROCK 5A is connected
  to the i2c0 bus, [1] so move the eeprom node from the i2c2 bus
  to the i2c0 bus.

  [1] https://dl.radxa.com/rock5/5a/docs/hw/radxa_rock5a_V1.1_sch.pdf, pp. 19

> Fixes: 89c880808cff8 ("arm64: dts: rockchip: add I2C EEPROM to rock-5a")
> Link: https://dl.radxa.com/rock5/5a/docs/hw/radxa_rock5a_V1.1_sch.pdf p.19

The suggestion above also shows how it would be better to use
explicitly placed references, instead of just placing them into
the Links lines, because that places the refernces closer to
the backed contents.  This isn't critical in this case, because
there isn't much contents, but it's a good practice nonetheless.

> Signed-off-by: FUKAUMI Naoki <naoki at radxa.com>
> ---
>  arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> index 19a08f7794e67..428c6f0232a34 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-rock-5a.dts
> @@ -228,6 +228,12 @@ regulator-state-mem {
>  			regulator-off-in-suspend;
>  		};
>  	};
> +
> +	eeprom: eeprom at 50 {
> +		compatible = "belling,bl24c16a", "atmel,24c16";
> +		reg = <0x50>;
> +		pagesize = <16>;
> +	};
>  };
>  
>  &i2c2 {
> @@ -249,12 +255,6 @@ regulator-state-mem {
>  			regulator-off-in-suspend;
>  		};
>  	};
> -
> -	eeprom: eeprom at 50 {
> -		compatible = "belling,bl24c16a", "atmel,24c16";
> -		reg = <0x50>;
> -		pagesize = <16>;
> -	};
>  };
>  
>  &i2c3 {