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

["Arnd Bergmann" <arnd@arndb.de>]

On Fri, Oct 24, 2025, at 05:54, Ryan Chen wrote:
>> 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:
>> >> > +	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.

Makes sense. Since this is a fairly unusual design, I would suggest
you add that explanation into the patch description for this
patch as well, so readers have a chance to find it when they look
at the file in the git history at a later point.

>> 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?

Yes

>> 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.

Right, if that is the documented binding, I think keeping syscon in
the compatible list makes sense.

      Arnd