[PATCH v5 3/8] ARM: dts: aspeed: yosemite5: Add new SGPIO line names and rename signal

Kevin Tung posted 8 patches 1 month, 1 week ago
[PATCH v5 3/8] ARM: dts: aspeed: yosemite5: Add new SGPIO line names and rename signal
Posted by Kevin Tung 1 month, 1 week ago
Add new SGPIO line names for user space monitoring and event logging.

Also rename PADDLE_BD_IOEXP_INT to ALERT_IRQ_PMBUS_PWR2_N to match
hardware naming. The original PADDLE_BD_IOEXP_INT is unused, so this
change does not affect current system functionality.

Signed-off-by: Kevin Tung <kevin.tung.openbmc@gmail.com>
---
 .../dts/aspeed/aspeed-bmc-facebook-yosemite5.dts   | 31 ++++++++++++++++++----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts
index 45b8ac2e8c65a4f672e64571631b7f6944f26213..983aebc394d9159c7e3db2e7c39e963f7b64c855 100644
--- a/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts
+++ b/arch/arm/boot/dts/aspeed/aspeed-bmc-facebook-yosemite5.dts
@@ -978,7 +978,7 @@ &sgpiom0 {
 	"E1S_0_BD_IOEXP","",
 	"E1S_1_BD_IOEXP","",
 	/*bit88-bit95*/
-	"PADDLE_BD_IOEXP_INT","",
+	"ALERT_IRQ_PMBUS_PWR2_N","",
 	"FM_BOARD_REV_ID0","",
 	"FM_BOARD_REV_ID1","",
 	"FM_BOARD_REV_ID2","",
@@ -991,16 +991,37 @@ &sgpiom0 {
 	"PRSNT_BOOT_N_FF","",
 	"PRSNT_MCIO1A_N_FF","",
 	"NIC_PRSNT_N","",
-	"","",
+	"FM_CPU_BMC_RST_N","",
 	"","",
 	"","",
 	"","",
 	/*bit104-bit111*/
-	"","","","","","","","","","","","","","","","",
+	"MASTER_PWR_EN","",
+	"MASTER_PWR2_EN","",
+	"PRSNT_MCIO0A_E1S0_N","",
+	"","",
+	"PRSNT_MCIO0A_E1S1_N","",
+	"","",
+	"","",
+	"Fault","",
 	/*bit112-bit119*/
-	"","","","","","","","","","","","","","","","",
+	"FM_CPLD_RSVD_MCIO0A_SB1","",
+	"FM_CPLD_RSVD_MCIO0A_SB2","",
+	"","",
+	"","",
+	"","",
+	"","",
+	"","",
+	"","",
 	/*bit120-bit127*/
-	"","","","","","","","","","","","","","","","";
+	"","",
+	"","",
+	"","",
+	"","",
+	"","",
+	"","",
+	"","",
+	"","";
 	status = "okay";
 };
 

-- 
2.53.0
Re: [PATCH v5 3/8] ARM: dts: aspeed: yosemite5: Add new SGPIO line names and rename signal
Posted by Andrew Jeffery 4 weeks, 1 day ago
On Mon, 2026-02-23 at 19:17 +0800, Kevin Tung wrote:
> Add new SGPIO line names for user space monitoring and event logging.
> 
> Also rename PADDLE_BD_IOEXP_INT to ALERT_IRQ_PMBUS_PWR2_N to match
> hardware naming. The original PADDLE_BD_IOEXP_INT is unused, so this
> change does not affect current system functionality.

Why are these two problems being solved in the one patch?

https://docs.kernel.org/process/submitting-patches.html#split-changes

Essentially, your use of "Also" is a bit of a red flag here.

However, on the specifics, why was the PADDLE_BD_IOEXP_INT hardware
naming wrong to begin with? What changed?

Broadly, it feels a lot like you're revising platform designs, then
trying to make the one devicetree fit the current design, and are not
explicitly communicating that this is what you're doing.

If that _is_ what you're doing, then we can come up with much better
schemes to handle it that aren't a constant stream of compatibility
breaks.

I need you to engage with this concern.

From inspection, I only find patches 1, 4 and 7 of this series to be
something I'd consider applying without further discussion.

Andrew
Re: [PATCH v5 3/8] ARM: dts: aspeed: yosemite5: Add new SGPIO line names and rename signal
Posted by Kevin Tung 3 weeks, 3 days ago
On Tue, Mar 3, 2026 at 6:41 PM Andrew Jeffery
<andrew@codeconstruct.com.au> wrote:
>
> On Mon, 2026-02-23 at 19:17 +0800, Kevin Tung wrote:
> > Add new SGPIO line names for user space monitoring and event logging.
> >
> > Also rename PADDLE_BD_IOEXP_INT to ALERT_IRQ_PMBUS_PWR2_N to match
> > hardware naming. The original PADDLE_BD_IOEXP_INT is unused, so this
> > change does not affect current system functionality.
>
> Why are these two problems being solved in the one patch?
>
> https://docs.kernel.org/process/submitting-patches.html#split-changes
>
> Essentially, your use of "Also" is a bit of a red flag here.
>
Hi Andew, sorry for addressing two issues in a single patch. I will
split them into two separate patches.

> However, on the specifics, why was the PADDLE_BD_IOEXP_INT hardware
> naming wrong to begin with? What changed?
>
Originally the signal was named PADDLE_BD_IOEXP_INT by the hardware team,
but the name did not clearly reflect its actual function. After
discussion with the EE team,
it was renamed to ALERT_IRQ_PMBUS_PWR2_N to better match its use as
the PMBus PWR2 alert interrupt in the system.

> Broadly, it feels a lot like you're revising platform designs, then
> trying to make the one devicetree fit the current design, and are not
> explicitly communicating that this is what you're doing.
>
> If that _is_ what you're doing, then we can come up with much better
> schemes to handle it that aren't a constant stream of compatibility
> breaks.
>
> I need you to engage with this concern.
>
Thanks for your feedback. I realize there may be a lack of knowledge
on my side regarding the best practices here.
Could you kindly guide me on how we might implement a better approach
that avoids a constant stream of compatibility breaks?
I’d like to ensure we handle this correctly and align with the
expected workflow.

> From inspection, I only find patches 1, 4 and 7 of this series to be
> something I'd consider applying without further discussion.
>
Got it. Should I split patches 1, 4, and 7 into a separate series?
This would keep the current series shorter by excluding items that
don’t require further discussion.

Kevin
BR
Re: [PATCH v5 3/8] ARM: dts: aspeed: yosemite5: Add new SGPIO line names and rename signal
Posted by Andrew Jeffery 1 week ago
Hi Kevin,

On Mon, 2026-03-09 at 11:34 -0700, Kevin Tung wrote:
> > Broadly, it feels a lot like you're revising platform designs, then
> > trying to make the one devicetree fit the current design, and are not
> > explicitly communicating that this is what you're doing.
> > 
> > If that _is_ what you're doing, then we can come up with much better
> > schemes to handle it that aren't a constant stream of compatibility
> > breaks.
> > 
> > I need you to engage with this concern.
> > 
> Thanks for your feedback. I realize there may be a lack of knowledge
> on my side regarding the best practices here.
> Could you kindly guide me on how we might implement a better approach
> that avoids a constant stream of compatibility breaks?
> I’d like to ensure we handle this correctly and align with the
> expected workflow.

Sure, see the reply I just sent here:

https://lore.kernel.org/all/d7794f74b26bbc1ee0a70e39c5671acc018f80eb.camel@codeconstruct.com.au/

> 
> > From inspection, I only find patches 1, 4 and 7 of this series to be
> > something I'd consider applying without further discussion.
> > 
> Got it. Should I split patches 1, 4, and 7 into a separate series?
> This would keep the current series shorter by excluding items that
> don’t require further discussion.
> 

I am okay with that, so long as it makes sense in the context of the
discussion linked above.

Andrew