Subject: Re: [PATCH v6 4/6] arm64: dts: aspeed: Add initial AST2700 SoC device
tree

On Thu, Oct 23, 2025, at 09:37, Ryan Chen wrote:

quoted

quoted

quoted

+	aliases {
+		serial0 = &uart0;
+		serial1 = &uart1;
+		serial2 = &uart2;
+		serial3 = &uart3;
+		serial4 = &uart4;
+		serial5 = &uart5;
+		serial6 = &uart6;
+		serial7 = &uart7;
+		serial8 = &uart8;
+		serial9 = &uart9;
+		serial10 = &uart10;
+		serial11 = &uart11;
+		serial12 = &uart12;
+		serial13 = &uart13;
+		serial14 = &uart14;
+	};

This looks like you just list all the uarts that are present on the
chip, which is not how the aliases are meant to be used. Move this
block into the board specific file and only list the ones that are
actually enabled on that particular board.

In particular, the alias names are meant to be local to the board and
don't usually correspond to the numbering inside of the chip. In the
defconfig, we currently set CONFIG_SERIAL_8250_NR_UARTS=8, which is
enough for any board we support so far, but that means only the first
8 aliases in the list will actually work.

Understood. I'll move the aliases block from the SoC dtsi into the EVB
board dts. For the EVB, UART12 is used as the default console, and the
board labels match the SoC numbering, so I plan to keep:

Does that look acceptable?
ast2700-evb.dts
	aliases {
		serial0 = &uart0;
		serial1 = &uart1;
		serial2 = &uart2;
		serial3 = &uart3;
		serial4 = &uart4;
		serial5 = &uart5;
		serial6 = &uart6;
		serial7 = &uart7;
		serial8 = &uart8;
		serial9 = &uart9;
		serial10 = &uart10;
		serial11 = &uart11;
		serial12 = &uart12;
		serial13 = &uart13;
		serial14 = &uart14;
}

I think this would be broken for the defconfig if the consol is on serial12. I
would recommend using serial0 as the console, like

aliases {
       serial0 = &uart12;
}

in this case. If additional uarts are enabled, add those as further aliases.

Understood, I will update.

quoted

quoted

quoted

+	soc1: soc@14000000 {
+		compatible = "simple-bus";
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges = <0x0 0x0 0x0 0x14000000 0x0 0x10000000>;

This probably needs some explanation: why are there two 'soc@...'
devices? Is this literally two chips in the system, or are you
describing two buses inside of the same SoC?

The AST2700 is two soc connection with a property bus.
Sharing some decode registers. Each have it own ahb bus.

I don't understand your explanation,

Let me clarify more clearly:
The AST2700 is a dual-SoC architecture, consisting of two interconnected SoCs,
referred to as SoC0 and SoC1. Each SoC has its own clock/reset domains. 
They are connected through an internal "property bus", 
which is Aspeed's internal interconnect providing shared
address decoding and communication between the two SoCs.

quoted

quoted

quoted

+
+		mdio0: mdio@14040000 {
+			compatible = "aspeed,ast2600-mdio";
+			reg = <0 0x14040000 0 0x8>;
+			resets = <&syscon1 SCU1_RESET_MII>;
+			status = "disabled";
+		};

I see that you use the old compatible="aspeed,ast2600-mdio" string
exclusively here. While this works, I would suggest you list both a
more specific "aspeed,ast2700-mdio" string to refer to the version in
this chip as well as the fallback "aspeed,ast2600-mdio" string as the generic

identifier.

quoted

quoted

The binding obviously has to describe both in that case, but the
driver does not need to be modified as long as both behave the same way.

Thanks, will submit ast2700-mdio.
Question, should I add in here patch series?
Or go for another patch thread?

Since there is no corresponding driver change, I would keep the binding change
as a patch in this series.

Sorry, I am wondering, I will follow Andrew advice. 
Submit ast2700-mdio to net-next go out another thread.
And put submit link in cover-letter in next version.
Is it ok?

quoted

quoted

quoted

+
+		syscon1: syscon@14c02000 {
+			compatible = "aspeed,ast2700-scu1", "syscon",

"simple-mfd";

quoted

quoted

quoted

+			reg = <0x0 0x14c02000 0x0 0x1000>;
+			ranges = <0x0 0x0 0x14c02000 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			#clock-cells = <1>;
+			#reset-cells = <1>;
+
+			scu_ic2: interrupt-controller@100 {
+				compatible = "aspeed,ast2700-scu-ic2";
+				reg = <0x100 0x8>;
+				#interrupt-cells = <1>;
+				interrupts-extended = <&intc1_5 0>;
+				interrupt-controller;
+			};
+
+			scu_ic3: interrupt-controller@108 {
+				compatible = "aspeed,ast2700-scu-ic3";
+				reg = <0x108 0x8>;
+				#interrupt-cells = <1>;
+				interrupts-extended = <&intc1_5 26>;
+				interrupt-controller;
+			};

This looks a bit silly to be honest: you have two separate devices
that each have a single register and a different compatible string?

Yes, it have difference register define in each scu-ic#. That is
compatible with design.
https://github.com/torvalds/linux/blob/master/drivers/irqchip/irq-aspe
ed-scu-ic.c#L45-L48

Right, if the driver already has this design, it does make sense to not change it
for the new generation. For a newly added driver I would probably do it
differently.

Thanks

quoted

quoted

Also you claim to be compatible with "syscon" but nothing actually
refers to the syscon node in that form?

There is another submit ongoing in pinctrl which will use syscon.
https://lwn.net/ml/all/20250829073030.2749482-2-billy_tsai@aspeedtech.
com/

Could I keep it? or I should remove it?

The version of the driver you are linking does not appear to use syscon, maybe
this is an artifact from a previous version?

If so, you can drop it. On the other hand, this does seem to be a classic syscon
device and keeping it marked that way is not harmful, just redundant if you
actually use the more specific compatible string.

Sorry, I may not point right link
https://patchwork.ozlabs.org/project/linux-aspeed/patch/20250829073030.2749482-4-billy_tsai@aspeedtech.com/
aspeed_g7_soc0_pinctrl_probe -> aspeed_pinctrl_probe
https://github.com/torvalds/linux/blob/master/drivers/pinctrl/aspeed/pinctrl-aspeed.c#L456

That will use syscon to regmap.

     Arnd