Re: [PATCH v6 11/11] arm64: dts: mt7986: add BPI-R3 nand/nor overlays

[Rob Herring <robh+dt@kernel.org>]

On Fri, Nov 18, 2022 at 4:05 PM Frank Wunderlich
<frank-w@public-files.de> wrote:
>
> Am 18. November 2022 22:39:52 MEZ schrieb Rob Herring <robh+dt@kernel.org>:
> >On Fri, Nov 18, 2022 at 1:01 PM Frank Wunderlich <linux@fw-web.de> wrote:
> >>
> >> From: Frank Wunderlich <frank-w@public-files.de>
> >>
> >> Add devicetree overlays for using nand and nor on BPI-R3.
> >
> >Can you not tell at runtime which one you booted from? If not, how
> >does one choose which overlay to apply? If you can, why not populate
> >both nodes and enable the right one? IMO, if all h/w is present, it
> >should all be in the DT. Selecting what h/w to use is a separate
> >problem and overlays aren't a great solution for that.
>
> It is not the decision about bootdevice,more available devices.
>
> Only 1 spi device (nand OR nor) is available
> at boottime as they share same spi bus and
> chipselect is set via hw jumper.
> Both nodes have reg 0,which is imho not
> supported in linux.

As long as one is set to disabled, it should be fine.


> I choosed overlays to add the right spi
> device on the right mmc device where
> similar selection happens (see patch 10).
> Either sd OR emmc can be used (1 mmc
> controller,first 4bits from bus switched by
> hardware jumper).But for mmc i use it as
> base fdt because i see mmc as primary
> device which holds rootfs too. Nand/nor is
> imho helping device for accessing emmc or
> like rescue system (only uboot).

No way to read the jumper state or know what you booted from I gues?

> I probe in uboot if emmc is available (mmc
>  partconf) and choose emmc else sd. For
>  spi i try with sf command to check for nor,if
>  this does not work i apply nand overlay.

Instead of applying overlays, wouldn't just changing 'status' be easier?

>
> >> Signed-off-by: Frank Wunderlich <frank-w@public-files.de>
> >> ---
> >> maybe rename to dtso?
> >>
> >> "kbuild: Allow DTB overlays to built from .dtso named source files"
> >> https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git/commit/?h=dt/next&id=363547d2191cbc32ca954ba75d72908712398ff2
>
> Should i do this?

Yes. .dts -> .dtbo is going to be removed.

> >> more comments about the dt overlay-support:
> >>
> >> https://patchwork.kernel.org/comment/25092116/
> >> https://patchwork.kernel.org/comment/25085681/
>
> Daniel suggest define sd/emmc as overlay too...with way you mention below we could create 4 full fdt without applying overlays in uboot.

Yes, but if you are going to do that, then you can just do all this
with includes.

> >> --- a/arch/arm64/boot/dts/mediatek/Makefile
> >> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> >> @@ -8,6 +8,8 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
> >>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
> >>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
> >>  dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtb
> >> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo
> >> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo
> >
> >These need rules to apply them to the base dtb(s). You just need:
> >
> >full.dtb := base.dtb overlay.dtb
> >dtb-y += full.dtb
>
> I would prefer to do this in bootloader to allow all 4 possible configurations:
>
> Sd+nand
> Sd+nor
> Emmc+nand
> Emmc+nor

That's fine. The purpose here is to document what the overlays apply
to, check that they actually apply, and validate them when applied
(unless someone wants to figure out all the issues with validating
just an overlay and make that work). You for example have an
undocumented compatible in yours (denx,fit).

Rob