Re: [RFC PATCH] arm64: broadcom: bcm2712: Rework rp1 overlay handling

[Andrea della Porta <andrea.porta@suse.com>]

Hi Rob,

On 14:58 Tue 09 Dec     , Andrea della Porta wrote:
> Hi Rob,
> sorry for the delay...
> 
> On 15:15 Mon 17 Nov     , Rob Herring (Arm) wrote:
> > It's a requirement that DT overlays be applied at build time in order to
> > validate them as overlays are not validated on their own.
> > 
> > Simply adding a build time target is possible, but it's also not desirable
> > to have targets which include the overlay contents and apply the same
> > overlay.
> > 
> > Rework the targets such that there are 2 overlays: the existing RP1 overlay
> > and the board specific configuration of the RP1 (bcm2712-rpi-5-b.dtso).
> > 
> > Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
> > ---
> > The missing applying of rp1.dtbo will soon be a warning[1].
> > 
> > The comment in bcm2712-rpi-5-b.dts(o) needs updating or removing. The RP1
> > driver or userspace handling of the overlay probably needs updating to
> > handle the 2 overlays. IIRC, I think we can apply overlays to overlays
> > which may be useful here. If not, adding that to the tools should be
> > possible.
> > 
> > [1] https://lore.kernel.org/all/20251114222759.4181152-1-robh@kernel.org/
> > ---
> >  arch/arm64/boot/dts/broadcom/Makefile                 | 10 ++++++++--
> >  ...m2712-d-rpi-5-b.dts => bcm2712-d-rpi-5-b-base.dts} |  2 +-
> >  .../{bcm2712-rpi-5-b.dts => bcm2712-rpi-5-b.dtso}     | 11 +++++------
> >  3 files changed, 14 insertions(+), 9 deletions(-)
> >  rename arch/arm64/boot/dts/broadcom/{bcm2712-d-rpi-5-b.dts => bcm2712-d-rpi-5-b-base.dts} (95%)
> >  rename arch/arm64/boot/dts/broadcom/{bcm2712-rpi-5-b.dts => bcm2712-rpi-5-b.dtso} (92%)
> > 
> > diff --git a/arch/arm64/boot/dts/broadcom/Makefile b/arch/arm64/boot/dts/broadcom/Makefile
> > index 83d45afc6588..4957edec9d9f 100644
> > --- a/arch/arm64/boot/dts/broadcom/Makefile
> > +++ b/arch/arm64/boot/dts/broadcom/Makefile
> > @@ -6,9 +6,9 @@ DTC_FLAGS := -@
> >  dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
> >  			      bcm2711-rpi-4-b.dtb \
> >  			      bcm2711-rpi-cm4-io.dtb \
> > -			      bcm2712-rpi-5-b.dtb \
> > +			      bcm2712-rpi-5-b.dtbo \
> >  			      bcm2712-rpi-5-b-ovl-rp1.dtb \
> > -			      bcm2712-d-rpi-5-b.dtb \
> > +			      bcm2712-d-rpi-5-b-base.dtb \
> >  			      bcm2837-rpi-2-b.dtb \
> >  			      bcm2837-rpi-3-a-plus.dtb \
> >  			      bcm2837-rpi-3-b.dtb \
> > @@ -17,6 +17,12 @@ dtb-$(CONFIG_ARCH_BCM2835) += bcm2711-rpi-400.dtb \
> >  			      bcm2837-rpi-zero-2-w.dtb \
> >  			      rp1.dtbo
> >  
> > +bcm2712-rpi-5-b-dtbs := bcm2712-rpi-5-b-ovl-rp1.dtb rp1.dtbo bcm2712-rpi-5-b.dtbo
> > +dtb-$(CONFIG_ARCH_BCM2835) += bcm2712-rpi-5-b.dtb
> > +
> > +bcm2712-d-rpi-5-b-dtbs := bcm2712-d-rpi-5-b-base.dtb rp1.dtbo bcm2712-rpi-5-b.dtbo
> > +dtb-$(CONFIG_ARCH_BCM2835) += bcm2712-d-rpi-5-b.dtb
> > +
> >  subdir-y	+= bcmbca
> >  subdir-y	+= northstar2
> >  subdir-y	+= stingray
> > diff --git a/arch/arm64/boot/dts/broadcom/bcm2712-d-rpi-5-b.dts b/arch/arm64/boot/dts/broadcom/bcm2712-d-rpi-5-b-base.dts
> > similarity index 95%
> > rename from arch/arm64/boot/dts/broadcom/bcm2712-d-rpi-5-b.dts
> > rename to arch/arm64/boot/dts/broadcom/bcm2712-d-rpi-5-b-base.dts
> > index 7de24d60bcd1..48a0ae118839 100644
> > --- a/arch/arm64/boot/dts/broadcom/bcm2712-d-rpi-5-b.dts
> > +++ b/arch/arm64/boot/dts/broadcom/bcm2712-d-rpi-5-b-base.dts
> > @@ -1,7 +1,7 @@
> >  // SPDX-License-Identifier: (GPL-2.0 OR MIT)
> >  /dts-v1/;
> >  
> > -#include "bcm2712-rpi-5-b.dts"
> > +#include "bcm2712-rpi-5-b-ovl-rp1.dts"
> >  
> >  &gio_aon {
> >  	brcm,gpio-bank-widths = <15 6>;
> > diff --git a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtso
> > similarity index 92%
> > rename from arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
> > rename to arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtso
> > index b8f256545022..89d099c00035 100644
> > --- a/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dts
> > +++ b/arch/arm64/boot/dts/broadcom/bcm2712-rpi-5-b.dtso
> > @@ -15,19 +15,18 @@
> >   */
> >  
> >  /dts-v1/;
> > +/plugin/;
> >  
> > -#include "bcm2712-rpi-5-b-ovl-rp1.dts"
> > -
> > -&pcie2 {
> > -	#include "rp1-nexus.dtsi"
> > -};
> > +#include <dt-bindings/gpio/gpio.h>
> 
> Sounds reasonable to me, please let me have a full round of tests to check for
> any issue with the driver and I'll come back with feedback.

Just a heads up, since you've posted this new revision [1] and that I'm currently
reworking RP1 overlay (in order to drop it), I assume that this patch is no longer
relevant and I'll avoid testing it.

Thanks,
Andrea

[1] - https://lore.kernel.org/all/aUQA5IKqqsrElzKP@apocalypse/

> 
> >  
> >  &rp1_eth {
> >  	status = "okay";
> >  	phy-mode = "rgmii-id";
> >  	phy-handle = <&phy1>;
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> 
> I think this should go in a separate patch.
> 
> >  
> > -	mdio {
> > +	mdio@1 {
> 
> Ditto.
> 
> Many thanks,
> Andrea
> 
> >  		reg = <0x1>;
> >  		reset-gpios = <&rp1_gpio 32 GPIO_ACTIVE_LOW>;
> >  		reset-delay-us = <5000>;
> > -- 
> > 2.51.0
> >