RE: [PATCH v19 1/2] ARM: dts: aspeed: yosemite4: Revise i2c-mux devices
From: Delphine_CC_Chiu/WYHQ/Wiwynn
Date: Wed Sep 25 2024 - 21:43:14 EST
> -----Original Message-----
> From: Andrew Jeffery <andrew@xxxxxxxxxxxxxxxxxxxx>
> Sent: Monday, September 23, 2024 9:02 AM
> To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@xxxxxxxxxx>;
> patrick@xxxxxxxxx; Rob Herring <robh@xxxxxxxxxx>; Krzysztof Kozlowski
> <krzk+dt@xxxxxxxxxx>; Conor Dooley <conor+dt@xxxxxxxxxx>; Joel Stanley
> <joel@xxxxxxxxx>
> Cc: Ricky CX Wu <ricky.cx.wu.wiwynn@xxxxxxxxx>;
> devicetree@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> linux-aspeed@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v19 1/2] ARM: dts: aspeed: yosemite4: Revise i2c-mux
> devices
>
> [External Sender]
>
> [External Sender]
>
> On Fri, 2024-09-20 at 15:04 +0800, Delphine CC Chiu wrote:
> > From: Ricky CX Wu <ricky.cx.wu.wiwynn@xxxxxxxxx>
> >
> > Revise Yosemite 4 devicetree for devices behind i2c-mux
> > - Add gpio and eeprom behind i2c-mux
> > - Remove redundant idle-state setting for i2c-mux
> > - Revise address of max31790 devices after i2c-mux.
> > - Fix warnings reporting by dts checking tool.
>
> Can you be more specific here? If you've had to fix existing warnings that the
> devicetree produces you should split those out into commits separate from
> adding further nodes. Instead, if you're just fixing warnings that this patch
> itself introduced then you should drop this comment as it's just a process issue.
> Process issues are better described in the patch notes or series cover letter, not
> in the commit message.
>
> Generally if you feel you have to list the changes you're making in the commit
> message for a single patch, it's a sign that you patch should be split into
> multiple commits anyway.
>
Hi Andrew,
I'll split the patch to multiple patches according to the I2C bus.
> > - Remove led_gpio pca9552 since there is no binding document.
> >
> > Signed-off-by: Ricky CX Wu <ricky.cx.wu.wiwynn@xxxxxxxxx>
> > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@xxxxxxxxxx>
> > ---
> > .../aspeed/aspeed-bmc-facebook-yosemite4.dts | 539
> > +++++++++++++++---
> > 1 file changed, 466 insertions(+), 73 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..6b4efb5fbd83 100644
> > --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
> > +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
> >
>
> ...
>
> >
> > &i2c11 {
> > status = "okay";
> > power-sensor@10 {
> > - compatible = "adi, adm1272";
> > + compatible = "adi,adm1272";
> > reg = <0x10>;
> > };
> >
> > power-sensor@12 {
> > - compatible = "adi, adm1272";
> > + compatible = "adi,adm1272";
> > reg = <0x12>;
> > };
>
> Can you please split the adm1272 compatible changes out to a separate patch
> and include an appropriate Fixes: tag?
>
> Andrew
Sure, I sent another patch to correct the compatible string of adm1272.