Thread (26 messages) 26 messages, 6 authors, 2022-11-21

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

From: Rob Herring <robh@kernel.org>
Date: 2022-11-20 16:13:28
Also in: linux-devicetree, linux-mediatek, linux-pci, linux-phy, linux-usb, lkml

On Sat, Nov 19, 2022 at 08:19:52AM +0100, Frank Wunderlich wrote:
Am 8. November 2022 15:45:49 MEZ schrieb Rob Herring [off-list ref]:
quoted
On Fri, Nov 18, 2022 at 4:05 PM Frank Wunderlich
[off-list ref] wrote:
quoted
Am 18. November 2022 22:39:52 MEZ schrieb Rob Herring [off-list ref]:
quoted
On Fri, Nov 18, 2022 at 1:01 PM Frank Wunderlich [off-list ref] wrote:
quoted
From: Frank Wunderlich <redacted>

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.

quoted
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?
quoted
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?
It will be easier,but requires dts for all
 combinations,we have have sd/emmc
 combination twice (once for nand
 enabling,once for nor) and we have then 4
 full dts instead of smaller overlays in fit.
No, I mean can't you have 1 dtb with everything, but nand, nor, emmc, 
and sd are all disabled. Then at boot change 'status' for what's 
enabled.

So i should add spi subnodes both disabled
 in base dtsi and create 4 dts (sd-nand,sd-nor,emmc-nand,emmc-nor) with
 mmc node and enabling the right spi node?
quoted
quoted
quoted
quoted
Signed-off-by: Frank Wunderlich <redacted>
---
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.
Do this if still using overlays,will test new way.

Maybe we can apply parts 1-9 first?
Sure.
quoted
quoted
quoted
quoted
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.
This is a third way if i understand correctly

Make all of them as overlay (dtso?) but
 build dtb by combining them in makefile.

This looks the best way because it avoids
 redundand code for mmc node and allows
 my current spi config (not the status way
 which may break due to same unit address).

I guess my base dtsi is then a dts too?
Yes, it can be.
Or should these overlays only duplicated and either include sd dts or emmc dts (but this creates again redundant code)?
quoted
quoted
quoted
quoted
--- 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).
Oh,need to check,copied partitions from my
 uboot dts...maybe there is a linux version
 for marking it as fit partition,else i drop
 completely.
You just need to document it. But the first thing I'm going to say, is 
'u-boot' is the vendor, not 'denx'. So 'u-boot,fit'.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help