Hi Delphine,
On Wed, 2024-01-31 at 16:41 +0800, Delphine CC Chiu wrote:
> Revise Yosemite 4 devicetree for devices behind i2c-mux
> - Add gpio and eeprom behind i2c-mux
> - Remove redundant idle-state setting for i2c-mux
Generally if you find yourself listing things the patch does in the
commit message it's an indicator you should split the patch up.
It looks like there's a lot of stuff to be fixed, but it doesn't need
to all be fixed in the one commit (as 01/21 suggests I guess). The
devicetree is already inaccurate, it's okay if a subset of the
inaccuracies survive for another patch or so.
Otherwise, if they must be changed together, it would be good to have a
description of *why*. Broadly, the commit message should explain *why*
the change is need regardless, not discuss *what* the patch changes
(that's evident from the patch itself).
>
> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
> ---
> .../aspeed/aspeed-bmc-facebook-yosemite4.dts | 381 ++++++++++++++++--
> 1 file changed, 347 insertions(+), 34 deletions(-)
>
>
> - i2c-mux@71 {
> - compatible = "nxp,pca9846";
> + i2c-mux@74 {
> + compatible = "nxp,pca9546";
Aside from splitting the patch on adding more devices and removing the
redundant idle-state settings, things like this should probably be
separate too.
Why was the address changed? Was it always wrong? Or has there been a
new revision of the board? A separate commit with some explanation here
would be useful.
> #address-cells = <1>;
> #size-cells = <0>;
> -
> - idle-state = <0>;
> i2c-mux-idle-disconnect;
> - reg = <0x71>;
> + reg = <0x74>;
>
> - i2c@0 {
> + imux30: i2c@0 {
> #address-cells = <1>;
> #size-cells = <0>;
> reg = <0>;
> @@ -450,26 +726,26 @@ i2c@0 {
> adc@1f {
> compatible = "ti,adc128d818";
> reg = <0x1f>;
> - ti,mode = /bits/ 8 <2>;
> + ti,mode = /bits/ 8 <1>;
This isn't discussed anywhere. There should probably be a separate
change for anything adc128d818-related that explains what's going on
here.
> };
>
> pwm@20{
> - compatible = "max31790";
> + compatible = "maxim,max31790";
> + pwm-as-tach = <4 5>;
> reg = <0x20>;
> - #address-cells = <1>;
> - #size-cells = <0>;
This also isn't discussed anywhere. There should probably be a separate
change for anything max31790-related that explains what's going on
here.
> };
>
> gpio@22{
> compatible = "ti,tca6424";
> reg = <0x22>;
> + gpio-controller;
> + #gpio-cells = <2>;
Also not discussed. Separate change for anything tca6424-related that
explains what's going on here.
> };
>
> - pwm@23{
> - compatible = "max31790";
> - reg = <0x23>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> + pwm@2f{
> + compatible = "maxim,max31790";
> + pwm-as-tach = <4 5>;
> + reg = <0x2f>;
> };
Should go in the max31790-related patch.
>
> adc@33 {
> @@ -492,34 +768,34 @@ gpio@61 {
> };
> };
>
> - i2c@1 {
> + imux31: i2c@1 {
> #address-cells = <1>;
> #size-cells = <0>;
> - reg = <0>;
> + reg = <1>;
>
> adc@1f {
> compatible = "ti,adc128d818";
> reg = <0x1f>;
> - ti,mode = /bits/ 8 <2>;
> + ti,mode = /bits/ 8 <1>;
Should go in the adc128d818 patch
> };
>
> pwm@20{
> - compatible = "max31790";
> + compatible = "maxim,max31790";
> + pwm-as-tach = <4 5>;
> reg = <0x20>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> };
Should go in the max31790 patch
>
> gpio@22{
> compatible = "ti,tca6424";
> reg = <0x22>;
> + gpio-controller;
> + #gpio-cells = <2>;
Should go in the tca6424 patch
> };
>
> - pwm@23{
> - compatible = "max31790";
> - reg = <0x23>;
> - #address-cells = <1>;
> - #size-cells = <0>;
> + pwm@2f{
> + compatible = "maxim,max31790";
> + pwm-as-tach = <4 5>;
> + reg = <0x2f>;
Should go in the max31790 patch
Andrew
@@ -17,6 +17,25 @@ aliases {
serial6 = &uart7;
serial7 = &uart8;
serial8 = &uart9;
+
+ i2c16 = &imux16;
+ i2c17 = &imux17;
+ i2c18 = &imux18;
+ i2c19 = &imux19;
+ i2c20 = &imux20;
+ i2c21 = &imux21;
+ i2c22 = &imux22;
+ i2c23 = &imux23;
+ i2c24 = &imux24;
+ i2c25 = &imux25;
+ i2c26 = &imux26;
+ i2c27 = &imux27;
+ i2c28 = &imux28;
+ i2c29 = &imux29;
+ i2c30 = &imux30;
+ i2c31 = &imux31;
+ i2c32 = &imux32;
+ i2c33 = &imux33;
};
chosen {
@@ -259,9 +278,109 @@ &i2c8 {
bus-frequency = <400000>;
i2c-mux@70 {
compatible = "nxp,pca9544";
- idle-state = <0>;
i2c-mux-idle-disconnect;
reg = <0x70>;
+
+ imux16: i2c@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ gpio@49 {
+ compatible = "nxp,pca9537";
+ reg = <0x49>;
+ };
+
+ eeprom@50 {
+ compatible = "atmel,24c128";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c128";
+ reg = <0x51>;
+ };
+
+ eeprom@54 {
+ compatible = "atmel,24c128";
+ reg = <0x54>;
+ };
+ };
+
+ imux17: i2c@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ gpio@49 {
+ compatible = "nxp,pca9537";
+ reg = <0x49>;
+ };
+
+ eeprom@50 {
+ compatible = "atmel,24c128";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c128";
+ reg = <0x51>;
+ };
+
+ eeprom@54 {
+ compatible = "atmel,24c128";
+ reg = <0x54>;
+ };
+ };
+
+ imux18: i2c@2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+ gpio@49 {
+ compatible = "nxp,pca9537";
+ reg = <0x49>;
+ };
+
+ eeprom@50 {
+ compatible = "atmel,24c128";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c128";
+ reg = <0x51>;
+ };
+
+ eeprom@54 {
+ compatible = "atmel,24c128";
+ reg = <0x54>;
+ };
+ };
+
+ imux19: i2c@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ gpio@49 {
+ compatible = "nxp,pca9537";
+ reg = <0x49>;
+ };
+
+ eeprom@50 {
+ compatible = "atmel,24c128";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c128";
+ reg = <0x51>;
+ };
+
+ eeprom@54 {
+ compatible = "atmel,24c128";
+ reg = <0x54>;
+ };
+ };
};
};
@@ -270,15 +389,174 @@ &i2c9 {
bus-frequency = <400000>;
i2c-mux@71 {
compatible = "nxp,pca9544";
- idle-state = <0>;
i2c-mux-idle-disconnect;
reg = <0x71>;
+
+ imux20: i2c@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ gpio@49 {
+ compatible = "nxp,pca9537";
+ reg = <0x49>;
+ };
+
+ eeprom@50 {
+ compatible = "atmel,24c128";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c128";
+ reg = <0x51>;
+ };
+
+ eeprom@54 {
+ compatible = "atmel,24c128";
+ reg = <0x54>;
+ };
+ };
+
+ imux21: i2c@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ gpio@49 {
+ compatible = "nxp,pca9537";
+ reg = <0x49>;
+ };
+
+ eeprom@50 {
+ compatible = "atmel,24c128";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c128";
+ reg = <0x51>;
+ };
+
+ eeprom@54 {
+ compatible = "atmel,24c128";
+ reg = <0x54>;
+ };
+ };
+
+ imux22: i2c@2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+ gpio@49 {
+ compatible = "nxp,pca9537";
+ reg = <0x49>;
+ };
+
+ eeprom@50 {
+ compatible = "atmel,24c128";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c128";
+ reg = <0x51>;
+ };
+
+ eeprom@54 {
+ compatible = "atmel,24c128";
+ reg = <0x54>;
+ };
+ };
+
+ imux23: i2c@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+
+ gpio@49 {
+ compatible = "nxp,pca9537";
+ reg = <0x49>;
+ };
+
+ eeprom@50 {
+ compatible = "atmel,24c128";
+ reg = <0x50>;
+ };
+
+ eeprom@51 {
+ compatible = "atmel,24c128";
+ reg = <0x51>;
+ };
+
+ eeprom@54 {
+ compatible = "atmel,24c128";
+ reg = <0x54>;
+ };
+ };
};
};
&i2c10 {
status = "okay";
bus-frequency = <400000>;
+ i2c-mux@74 {
+ compatible = "nxp,pca9544";
+ i2c-mux-idle-disconnect;
+ reg = <0x74>;
+
+ imux28: i2c@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+
+ gpio@20 {
+ compatible = "nxp,pca9506";
+ reg = <0x20>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ gpio@21 {
+ compatible = "nxp,pca9506";
+ reg = <0x21>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ gpio@22 {
+ compatible = "nxp,pca9506";
+ reg = <0x22>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ gpio@23 {
+ compatible = "nxp,pca9506";
+ reg = <0x23>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+
+ gpio@24 {
+ compatible = "nxp,pca9506";
+ reg = <0x24>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-line-names =
+ "","","","",
+ "NIC0-MAIN-PWR-EN","NIC1-MAIN-PWR-EN",
+ "NIC2-MAIN-PWR-EN","NIC3-MAIN-PWR-EN",
+ "","","","","","","","",
+ "","","","","","","","",
+ "","","","","","","","";
+ };
+ };
+
+ imux29: i2c@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ };
+ };
};
&i2c11 {
@@ -433,16 +711,14 @@ eeprom@51 {
reg = <0x51>;
};
- i2c-mux@71 {
- compatible = "nxp,pca9846";
+ i2c-mux@74 {
+ compatible = "nxp,pca9546";
#address-cells = <1>;
#size-cells = <0>;
-
- idle-state = <0>;
i2c-mux-idle-disconnect;
- reg = <0x71>;
+ reg = <0x74>;
- i2c@0 {
+ imux30: i2c@0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
@@ -450,26 +726,26 @@ i2c@0 {
adc@1f {
compatible = "ti,adc128d818";
reg = <0x1f>;
- ti,mode = /bits/ 8 <2>;
+ ti,mode = /bits/ 8 <1>;
};
pwm@20{
- compatible = "max31790";
+ compatible = "maxim,max31790";
+ pwm-as-tach = <4 5>;
reg = <0x20>;
- #address-cells = <1>;
- #size-cells = <0>;
};
gpio@22{
compatible = "ti,tca6424";
reg = <0x22>;
+ gpio-controller;
+ #gpio-cells = <2>;
};
- pwm@23{
- compatible = "max31790";
- reg = <0x23>;
- #address-cells = <1>;
- #size-cells = <0>;
+ pwm@2f{
+ compatible = "maxim,max31790";
+ pwm-as-tach = <4 5>;
+ reg = <0x2f>;
};
adc@33 {
@@ -492,34 +768,34 @@ gpio@61 {
};
};
- i2c@1 {
+ imux31: i2c@1 {
#address-cells = <1>;
#size-cells = <0>;
- reg = <0>;
+ reg = <1>;
adc@1f {
compatible = "ti,adc128d818";
reg = <0x1f>;
- ti,mode = /bits/ 8 <2>;
+ ti,mode = /bits/ 8 <1>;
};
pwm@20{
- compatible = "max31790";
+ compatible = "maxim,max31790";
+ pwm-as-tach = <4 5>;
reg = <0x20>;
- #address-cells = <1>;
- #size-cells = <0>;
};
gpio@22{
compatible = "ti,tca6424";
reg = <0x22>;
+ gpio-controller;
+ #gpio-cells = <2>;
};
- pwm@23{
- compatible = "max31790";
- reg = <0x23>;
- #address-cells = <1>;
- #size-cells = <0>;
+ pwm@2f{
+ compatible = "maxim,max31790";
+ pwm-as-tach = <4 5>;
+ reg = <0x2f>;
};
adc@33 {
@@ -547,12 +823,10 @@ i2c-mux@73 {
compatible = "nxp,pca9544";
#address-cells = <1>;
#size-cells = <0>;
-
- idle-state = <0>;
i2c-mux-idle-disconnect;
reg = <0x73>;
- i2c@0 {
+ imux32: i2c@0 {
#address-cells = <1>;
#size-cells = <0>;
reg = <0>;
@@ -563,10 +837,10 @@ adc@35 {
};
};
- i2c@1 {
+ imux33: i2c@1 {
#address-cells = <1>;
#size-cells = <0>;
- reg = <0>;
+ reg = <1>;
adc@35 {
compatible = "maxim,max11617";
@@ -589,9 +863,48 @@ mctp@10 {
i2c-mux@72 {
compatible = "nxp,pca9544";
- idle-state = <0>;
i2c-mux-idle-disconnect;
reg = <0x72>;
+
+ imux24: i2c@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ temperature-sensor@1f {
+ compatible = "ti,tmp421";
+ reg = <0x1f>;
+ };
+ };
+
+ imux25: i2c@1 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <1>;
+ temperature-sensor@1f {
+ compatible = "ti,tmp421";
+ reg = <0x1f>;
+ };
+ };
+
+ imux26: i2c@2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <2>;
+ temperature-sensor@1f {
+ compatible = "ti,tmp421";
+ reg = <0x1f>;
+ };
+ };
+
+ imux27: i2c@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ temperature-sensor@1f {
+ compatible = "ti,tmp421";
+ reg = <0x1f>;
+ };
+ };
};
};