diff mbox series

[v1] ARM: dts: aspeed: yosemite4: Enable interrupt setting for pca9555

Message ID 20240910063952.3006665-1-Delphine_CC_Chiu@wiwynn.com
State New
Headers show
Series [v1] ARM: dts: aspeed: yosemite4: Enable interrupt setting for pca9555 | expand

Commit Message

Delphine CC Chiu Sept. 10, 2024, 6:39 a.m. UTC 
From: Ricky CX Wu <ricky.cx.wu.wiwynn@gmail.com>

Enable interrupt setting and add GPIO line name for pca9555 for the I/O
expanders on Medusa board.

Signed-off-by: Ricky CX Wu <ricky.cx.wu.wiwynn@gmail.com>
Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
---
 .../aspeed/aspeed-bmc-facebook-yosemite4.dts  | 52 +++++++++++++++++--
 1 file changed, 48 insertions(+), 4 deletions(-)

Comments

Krzysztof Kozlowski Sept. 10, 2024, 7:30 a.m. UTC | #1 
On 10/09/2024 08:39, Delphine CC Chiu wrote:
> From: Ricky CX Wu <ricky.cx.wu.wiwynn@gmail.com>
> 
> Enable interrupt setting and add GPIO line name for pca9555 for the I/O
> expanders on Medusa board.
> 
> Signed-off-by: Ricky CX Wu <ricky.cx.wu.wiwynn@gmail.com>
> Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
> ---
>  .../aspeed/aspeed-bmc-facebook-yosemite4.dts  | 52 +++++++++++++++++--
>  1 file changed, 48 insertions(+), 4 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..cb2436031181 100644
> --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
> +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
> @@ -295,30 +295,74 @@ power-sensor@12 {
>  
>  	gpio@20 {
>  		compatible = "nxp,pca9555";
> -		reg = <0x20>;
>  		gpio-controller;
>  		#gpio-cells = <2>;
> +		reg = <0x20>;

Hm? Why? The placement is after compatible.

> +		interrupt-parent = <&gpio0>;
> +		interrupts = <98 IRQ_TYPE_LEVEL_LOW>;
> +		gpio-line-names =
> +		"P48V_OCP_GPIO1","P48V_OCP_GPIO2",

Nothing improved here. I already commented about above and this.
Implement feedback for all your patches, not only one.

Best regards,
Krzysztof
Delphine CC Chiu Sept. 10, 2024, 8:20 a.m. UTC | #2 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Tuesday, September 10, 2024 3:31 PM
> To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@wiwynn.com>;
> patrick@stwcx.xyz; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Joel Stanley
> <joel@jms.id.au>; Andrew Jeffery <andrew@codeconstruct.com.au>
> Cc: Ricky CX Wu <ricky.cx.wu.wiwynn@gmail.com>;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v1] ARM: dts: aspeed: yosemite4: Enable interrupt setting
> for pca9555
> 
>  [External Sender]
> 
>  [External Sender]
> 
> On 10/09/2024 08:39, Delphine CC Chiu wrote:
> > From: Ricky CX Wu <ricky.cx.wu.wiwynn@gmail.com>
> >
> > Enable interrupt setting and add GPIO line name for pca9555 for the
> > I/O expanders on Medusa board.
> >
> > Signed-off-by: Ricky CX Wu <ricky.cx.wu.wiwynn@gmail.com>
> > Signed-off-by: Delphine CC Chiu <Delphine_CC_Chiu@wiwynn.com>
> > ---
> >  .../aspeed/aspeed-bmc-facebook-yosemite4.dts  | 52
> > +++++++++++++++++--
> >  1 file changed, 48 insertions(+), 4 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..cb2436031181 100644
> > --- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
> > +++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
> > @@ -295,30 +295,74 @@ power-sensor@12 {
> >
> >       gpio@20 {
> >               compatible = "nxp,pca9555";
> > -             reg = <0x20>;
> >               gpio-controller;
> >               #gpio-cells = <2>;
> > +             reg = <0x20>;
> 
> Hm? Why? The placement is after compatible.
> 
I will revise in v2. Thanks!
> > +             interrupt-parent = <&gpio0>;
> > +             interrupts = <98 IRQ_TYPE_LEVEL_LOW>;
> > +             gpio-line-names =
> > +             "P48V_OCP_GPIO1","P48V_OCP_GPIO2",
> 
> Nothing improved here. I already commented about above and this.
> Implement feedback for all your patches, not only one.
> 
> Best regards,
> Krzysztof
Sorry about that.
I saw you say "Broken alignment" in v15 patch.
Would like to ask if the following format meets your expectations?
+			gpio-line-names =
+				"P48V_OCP_GPIO1", "P48V_OCP_GPIO2",
+				"P48V_OCP_GPIO3", "FAN_BOARD_0_REVISION_0_R",
Krzysztof Kozlowski Sept. 16, 2024, 8:51 a.m. UTC | #3 
On 10/09/2024 10:20, Delphine_CC_Chiu/WYHQ/Wiwynn wrote:
> I will revise in v2. Thanks!
>>> +             interrupt-parent = <&gpio0>;
>>> +             interrupts = <98 IRQ_TYPE_LEVEL_LOW>;
>>> +             gpio-line-names =
>>> +             "P48V_OCP_GPIO1","P48V_OCP_GPIO2",
>>
>> Nothing improved here. I already commented about above and this.
>> Implement feedback for all your patches, not only one.
>>
>> Best regards,
>> Krzysztof
> Sorry about that.
> I saw you say "Broken alignment" in v15 patch.
> Would like to ask if the following format meets your expectations?
> +			gpio-line-names =
> +				"P48V_OCP_GPIO1", "P48V_OCP_GPIO2",
> +				"P48V_OCP_GPIO3", "FAN_BOARD_0_REVISION_0_R",

Please read DTS coding style before posting next version of the patch
(or any patch for DTS). This is still not aligned. There is (almost)
never a blank line after '='.

Best regards,
Krzysztof
Delphine CC Chiu Sept. 18, 2024, 3:12 a.m. UTC | #4 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Monday, September 16, 2024 4:51 PM
> To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@wiwynn.com>;
> patrick@stwcx.xyz; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Joel Stanley
> <joel@jms.id.au>; Andrew Jeffery <andrew@codeconstruct.com.au>
> Cc: Ricky CX Wu <ricky.cx.wu.wiwynn@gmail.com>;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v1] ARM: dts: aspeed: yosemite4: Enable interrupt setting
> for pca9555
> 
>  [External Sender]
> 
>  [External Sender]
> 
> On 10/09/2024 10:20, Delphine_CC_Chiu/WYHQ/Wiwynn wrote:
> > I will revise in v2. Thanks!
> >>> +             interrupt-parent = <&gpio0>;
> >>> +             interrupts = <98 IRQ_TYPE_LEVEL_LOW>;
> >>> +             gpio-line-names =
> >>> +             "P48V_OCP_GPIO1","P48V_OCP_GPIO2",
> >>
> >> Nothing improved here. I already commented about above and this.
> >> Implement feedback for all your patches, not only one.
> >>
> >> Best regards,
> >> Krzysztof
> > Sorry about that.
> > I saw you say "Broken alignment" in v15 patch.
> > Would like to ask if the following format meets your expectations?
> > +                     gpio-line-names =
> > +                             "P48V_OCP_GPIO1",
> "P48V_OCP_GPIO2",
> > +                             "P48V_OCP_GPIO3",
> > + "FAN_BOARD_0_REVISION_0_R",
> 
> Please read DTS coding style before posting next version of the patch (or any
> patch for DTS). This is still not aligned. There is (almost) never a blank line
> after '='.
> 
> Best regards,
> Krzysztof
Hi Krzysztof,
After checking the DTS coding style, I found the "Indentation" section mentioned that:
"For arrays spanning across lines, it is preferred to align the continued entries with opening < from the first line."

Should I align the code with following format? (No blank line after = and use two space to align the ")
Or would like to ask could you help to provide the dts file that I can follow?

gpio-line-names = "HSC1_ALERT1_R_N", "HSC2_ALERT1_R_N",
				  "HSC3_ALERT1_R_N", "HSC4_ALERT1_R_N",
				  "HSC5_ALERT1_R_N", "HSC6_ALERT1_R_N",
				  "HSC7_ALERT1_R_N", "HSC8_ALERT1_R_N",
				  "HSC1_ALERT2_R_N", "HSC2_ALERT2_R_N",
				  "HSC3_ALERT2_R_N", "HSC4_ALERT2_R_N",
				  "HSC5_ALERT2_R_N", "HSC6_ALERT2_R_N",
				  "HSC7_ALERT2_R_N", "HSC8_ALERT2_R_N";
Thanks.
Krzysztof Kozlowski Sept. 18, 2024, 8:16 a.m. UTC | #5 
On 18/09/2024 05:12, Delphine_CC_Chiu/WYHQ/Wiwynn wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzk@kernel.org>
>> Sent: Monday, September 16, 2024 4:51 PM
>> To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@wiwynn.com>;
>> patrick@stwcx.xyz; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
>> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Joel Stanley
>> <joel@jms.id.au>; Andrew Jeffery <andrew@codeconstruct.com.au>
>> Cc: Ricky CX Wu <ricky.cx.wu.wiwynn@gmail.com>;
>> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
>> linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH v1] ARM: dts: aspeed: yosemite4: Enable interrupt setting
>> for pca9555
>>
>>  [External Sender]
>>
>>  [External Sender]
>>
>> On 10/09/2024 10:20, Delphine_CC_Chiu/WYHQ/Wiwynn wrote:
>>> I will revise in v2. Thanks!
>>>>> +             interrupt-parent = <&gpio0>;
>>>>> +             interrupts = <98 IRQ_TYPE_LEVEL_LOW>;
>>>>> +             gpio-line-names =
>>>>> +             "P48V_OCP_GPIO1","P48V_OCP_GPIO2",
>>>>
>>>> Nothing improved here. I already commented about above and this.
>>>> Implement feedback for all your patches, not only one.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>> Sorry about that.
>>> I saw you say "Broken alignment" in v15 patch.
>>> Would like to ask if the following format meets your expectations?
>>> +                     gpio-line-names =
>>> +                             "P48V_OCP_GPIO1",
>> "P48V_OCP_GPIO2",
>>> +                             "P48V_OCP_GPIO3",
>>> + "FAN_BOARD_0_REVISION_0_R",
>>
>> Please read DTS coding style before posting next version of the patch (or any
>> patch for DTS). This is still not aligned. There is (almost) never a blank line
>> after '='.
>>
>> Best regards,
>> Krzysztof
> Hi Krzysztof,
> After checking the DTS coding style, I found the "Indentation" section mentioned that:
> "For arrays spanning across lines, it is preferred to align the continued entries with opening < from the first line."
> 
> Should I align the code with following format? (No blank line after = and use two space to align the ")
> Or would like to ask could you help to provide the dts file that I can follow?

Sure, e.g. qcom/sm8650.dtsi, line 765 or line 796.

Or arch/arm64/boot/dts/qcom/sm8450-sony-xperia-nagara.dtsi and
gpio-line-names.

Best regards,
Krzysztof
Delphine CC Chiu Sept. 18, 2024, 8:59 a.m. UTC | #6 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Wednesday, September 18, 2024 4:17 PM
> To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@wiwynn.com>;
> patrick@stwcx.xyz; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Joel Stanley
> <joel@jms.id.au>; Andrew Jeffery <andrew@codeconstruct.com.au>
> Cc: Ricky CX Wu <ricky.cx.wu.wiwynn@gmail.com>;
> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH v1] ARM: dts: aspeed: yosemite4: Enable interrupt setting
> for pca9555
> 
>  [External Sender]
> 
>  [External Sender]
> 
> On 18/09/2024 05:12, Delphine_CC_Chiu/WYHQ/Wiwynn wrote:
> >
> >
> >> -----Original Message-----
> >> From: Krzysztof Kozlowski <krzk@kernel.org>
> >> Sent: Monday, September 16, 2024 4:51 PM
> >> To: Delphine_CC_Chiu/WYHQ/Wiwynn <Delphine_CC_Chiu@wiwynn.com>;
> >> patrick@stwcx.xyz; Rob Herring <robh@kernel.org>; Krzysztof Kozlowski
> >> <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>; Joel
> >> Stanley <joel@jms.id.au>; Andrew Jeffery
> >> <andrew@codeconstruct.com.au>
> >> Cc: Ricky CX Wu <ricky.cx.wu.wiwynn@gmail.com>;
> >> devicetree@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> >> linux-aspeed@lists.ozlabs.org; linux-kernel@vger.kernel.org
> >> Subject: Re: [PATCH v1] ARM: dts: aspeed: yosemite4: Enable interrupt
> >> setting for pca9555
> >>
> >>  [External Sender]
> >>
> >>  [External Sender]
> >>
> >> On 10/09/2024 10:20, Delphine_CC_Chiu/WYHQ/Wiwynn wrote:
> >>> I will revise in v2. Thanks!
> >>>>> +             interrupt-parent = <&gpio0>;
> >>>>> +             interrupts = <98 IRQ_TYPE_LEVEL_LOW>;
> >>>>> +             gpio-line-names =
> >>>>> +             "P48V_OCP_GPIO1","P48V_OCP_GPIO2",
> >>>>
> >>>> Nothing improved here. I already commented about above and this.
> >>>> Implement feedback for all your patches, not only one.
> >>>>
> >>>> Best regards,
> >>>> Krzysztof
> >>> Sorry about that.
> >>> I saw you say "Broken alignment" in v15 patch.
> >>> Would like to ask if the following format meets your expectations?
> >>> +                     gpio-line-names =
> >>> +                             "P48V_OCP_GPIO1",
> >> "P48V_OCP_GPIO2",
> >>> +                             "P48V_OCP_GPIO3",
> >>> + "FAN_BOARD_0_REVISION_0_R",
> >>
> >> Please read DTS coding style before posting next version of the patch
> >> (or any patch for DTS). This is still not aligned. There is (almost)
> >> never a blank line after '='.
> >>
> >> Best regards,
> >> Krzysztof
> > Hi Krzysztof,
> > After checking the DTS coding style, I found the "Indentation" section
> mentioned that:
> > "For arrays spanning across lines, it is preferred to align the continued entries
> with opening < from the first line."
> >
> > Should I align the code with following format? (No blank line after =
> > and use two space to align the ") Or would like to ask could you help to
> provide the dts file that I can follow?
> 
> Sure, e.g. qcom/sm8650.dtsi, line 765 or line 796.
> 
> Or arch/arm64/boot/dts/qcom/sm8450-sony-xperia-nagara.dtsi and
> gpio-line-names.
> 
> Best regards,
> Krzysztof
Thanks for your help!
I'll correct the format in v2.
diff mbox series

Patch

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..cb2436031181 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite4.dts
@@ -295,30 +295,74 @@  power-sensor@12 {
 
 	gpio@20 {
 		compatible = "nxp,pca9555";
-		reg = <0x20>;
 		gpio-controller;
 		#gpio-cells = <2>;
+		reg = <0x20>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <98 IRQ_TYPE_LEVEL_LOW>;
+		gpio-line-names =
+		"P48V_OCP_GPIO1","P48V_OCP_GPIO2",
+		"P48V_OCP_GPIO3","FAN_BOARD_0_REVISION_0_R",
+		"FAN_BOARD_0_REVISION_1_R","FAN_BOARD_1_REVISION_0_R",
+		"FAN_BOARD_1_REVISION_1_R","RST_MUX_R_N",
+		"RST_LED_CONTROL_FAN_BOARD_0_N","RST_LED_CONTROL_FAN_BOARD_1_N",
+		"RST_IOEXP_FAN_BOARD_0_N","RST_IOEXP_FAN_BOARD_1_N",
+		"PWRGD_LOAD_SWITCH_FAN_BOARD_0_R","PWRGD_LOAD_SWITCH_FAN_BOARD_1_R",
+		"","";
 	};
 
 	gpio@21 {
 		compatible = "nxp,pca9555";
-		reg = <0x21>;
 		gpio-controller;
 		#gpio-cells = <2>;
+		reg = <0x21>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <98 IRQ_TYPE_LEVEL_LOW>;
+		gpio-line-names =
+		"HSC_OCP_SLOT_ODD_GPIO1","HSC_OCP_SLOT_ODD_GPIO2",
+		"HSC_OCP_SLOT_ODD_GPIO3","HSC_OCP_SLOT_EVEN_GPIO1",
+		"HSC_OCP_SLOT_EVEN_GPIO2","HSC_OCP_SLOT_EVEN_GPIO3",
+		"ADC_TYPE_0_R","ADC_TYPE_1_R",
+		"MEDUSA_BOARD_REV_0","MEDUSA_BOARD_REV_1",
+		"MEDUSA_BOARD_REV_2","MEDUSA_BOARD_TYPE",
+		"DELTA_MODULE_TYPE","P12V_HSC_TYPE",
+		"","";
 	};
 
 	gpio@22 {
 		compatible = "nxp,pca9555";
-		reg = <0x22>;
 		gpio-controller;
 		#gpio-cells = <2>;
+		reg = <0x22>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <98 IRQ_TYPE_LEVEL_LOW>;
+		gpio-line-names =
+		"CARD_TYPE_SLOT1","CARD_TYPE_SLOT2",
+		"CARD_TYPE_SLOT3","CARD_TYPE_SLOT4",
+		"CARD_TYPE_SLOT5","CARD_TYPE_SLOT6",
+		"CARD_TYPE_SLOT7","CARD_TYPE_SLOT8",
+		"OC_P48V_HSC_0_N","FLT_P48V_HSC_0_N",
+		"OC_P48V_HSC_1_N","FLT_P48V_HSC_1_N",
+		"EN_P48V_AUX_0","EN_P48V_AUX_1",
+		"PWRGD_P12V_AUX_0","PWRGD_P12V_AUX_1";
 	};
 
 	gpio@23 {
 		compatible = "nxp,pca9555";
-		reg = <0x23>;
 		gpio-controller;
 		#gpio-cells = <2>;
+		reg = <0x23>;
+		interrupt-parent = <&gpio0>;
+		interrupts = <98 IRQ_TYPE_LEVEL_LOW>;
+		gpio-line-names =
+		"HSC1_ALERT1_R_N","HSC2_ALERT1_R_N",
+		"HSC3_ALERT1_R_N","HSC4_ALERT1_R_N",
+		"HSC5_ALERT1_R_N","HSC6_ALERT1_R_N",
+		"HSC7_ALERT1_R_N","HSC8_ALERT1_R_N",
+		"HSC1_ALERT2_R_N","HSC2_ALERT2_R_N",
+		"HSC3_ALERT2_R_N","HSC4_ALERT2_R_N",
+		"HSC5_ALERT2_R_N","HSC6_ALERT2_R_N",
+		"HSC7_ALERT2_R_N","HSC8_ALERT2_R_N";
 	};
 
 	temperature-sensor@48 {