| Message ID | 20221118190126.100895-12-linux@fw-web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add BananaPi R3 | expand |
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
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. > 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 > > more comments about the dt overlay-support: > > https://patchwork.kernel.org/comment/25092116/ > https://patchwork.kernel.org/comment/25085681/ > --- > v4: > - drop compile-comment from overlays > - add author-information to dt-overlays > --- > arch/arm64/boot/dts/mediatek/Makefile | 2 + > .../mediatek/mt7986a-bananapi-bpi-r3-nand.dts | 55 +++++++++++++++ > .../mediatek/mt7986a-bananapi-bpi-r3-nor.dts | 69 +++++++++++++++++++ > 3 files changed, 126 insertions(+) > create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-nand.dts > create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-nor.dts > > diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile > index e8902f2cc58f..d42208c4090d 100644 > --- 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 Rob
On Fri, Nov 18, 2022 at 3:39 PM Rob Herring <robh+dt@kernel.org> wrote: > > 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. > > > > 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 > > > > more comments about the dt overlay-support: > > > > https://patchwork.kernel.org/comment/25092116/ > > https://patchwork.kernel.org/comment/25085681/ > > --- > > v4: > > - drop compile-comment from overlays > > - add author-information to dt-overlays > > --- > > arch/arm64/boot/dts/mediatek/Makefile | 2 + > > .../mediatek/mt7986a-bananapi-bpi-r3-nand.dts | 55 +++++++++++++++ > > .../mediatek/mt7986a-bananapi-bpi-r3-nor.dts | 69 +++++++++++++++++++ > > 3 files changed, 126 insertions(+) > > create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-nand.dts > > create mode 100644 arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-nor.dts > > > > diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile > > index e8902f2cc58f..d42208c4090d 100644 > > --- 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 Oops, that should be: full-dtbs := base.dtb overlay.dtb dtb-y += full.dtb Just like multiple objs in a module. Rob
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. 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). 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. >> 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? >> 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. >> --- 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 >Rob Hi, regards Frank
Am 8. November 2022 15:45:49 MEZ schrieb 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? 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. 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? >> >> >> 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. Do this if still using overlays,will test new way. Maybe we can apply parts 1-9 first? >> >> 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? Or should these overlays only duplicated and either include sd dts or emmc dts (but this creates again redundant code)? >> >> --- 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. So i i stay with current way i have to rename overlays to dtso and add info about base dtb (bpi-r3-sd/emmc) to commit message or into the overlay itself? >Rob regards Frank
On Sat, Nov 19, 2022 at 08:19:52AM +0100, Frank Wunderlich wrote: > Am 8. November 2022 15:45:49 MEZ schrieb 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? > > 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? > >> > >> >> 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. > > Do this if still using overlays,will test new way. > > Maybe we can apply parts 1-9 first? Sure. > > >> >> 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)? > >> >> --- 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
Hi > Gesendet: Sonntag, 20. November 2022 um 17:12 Uhr > Von: "Rob Herring" <robh@kernel.org> > On Sat, Nov 19, 2022 at 08:19:52AM +0100, Frank Wunderlich wrote: > > Am 8. November 2022 15:45:49 MEZ schrieb 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? > > > > 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. at least for sd/emmc: no, as they share same mmc-node with different settings (bus-width,speed,...). for nand/nor i'm not sure if spi can have 2 childs with same unit-address/reg even if disabled. Then i do not know how to enable a dts-node in uboot...only know (not long time) how to apply overlay ;) > > 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? > > >> > > >> >> 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. > > > > Do this if still using overlays,will test new way. > > > > Maybe we can apply parts 1-9 first? > > Sure. i would wait for them applied before posting next version, i hope they can go into 6.2. Then it is easier because depencies are all in. overlay was only for discussion the right way...my preferred way was mmc in board dts and spi as overlay, daniel preferred base fdt without them all and 4 overlays. another way already discussed were 4 full fdt, but how to get all 4 combinations with less files and not defining mmc/spi device twice? i see only the overlay version and combining via Makefile. what is your preferred way (except all devices in 1 dts which is afair not possible, daniel please correct me if i'm wrong)? > > > > >> >> 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)? > > >> >> --- 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'. imho it is not needed in linux, so i drop it. regards Frank
> Gesendet: Sonntag, 20. November 2022 um 17:12 Uhr > Von: "Rob Herring" <robh@kernel.org> > > Or should these overlays only duplicated and either include sd dts or emmc dts (but this creates again redundant code)? > > >> >> --- 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 Hi, (removed most recipients to not spam all people with this) have now locally changed sd+emmc to overlays too and renamed to dtso... tried also this way to (arch/arm64/boot/dts/mediatek/Makefile): dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-sd.dtbo mt7986a-bananapi-bpi-r3-sd-nand.dtb := mt7986a-bananapi-bpi-r3.dtb mt7986a-bananapi-bpi-r3-sd.dtbo mt7986a-bananapi-bpi-r3-nand.dtbo dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-sd-nand.dtb does not work: make[3]: *** No rule to make target 'arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-sd-nand.dtb', needed by 'arch/arm64/boot/dts/mediatek/'. Stop. any thoughts? but this way i will need all possible combinations, i guess best way it to leave the overlays themselves. regards Frank
On Sat, Nov 26, 2022 at 9:00 AM Frank Wunderlich <frank-w@public-files.de> wrote: > > > Gesendet: Sonntag, 20. November 2022 um 17:12 Uhr > > Von: "Rob Herring" <robh@kernel.org> > > > > Or should these overlays only duplicated and either include sd dts or emmc dts (but this creates again redundant code)? > > > >> >> --- 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 > > Hi, > > (removed most recipients to not spam all people with this) > > have now locally changed sd+emmc to overlays too and renamed to dtso... > tried also this way to (arch/arm64/boot/dts/mediatek/Makefile): > > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-sd.dtbo > > mt7986a-bananapi-bpi-r3-sd-nand.dtb := mt7986a-bananapi-bpi-r3.dtb mt7986a-bananapi-bpi-r3-sd.dtbo mt7986a-bananapi-bpi-r3-nand.dtbo > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-sd-nand.dtb > > does not work: > > make[3]: *** No rule to make target 'arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-sd-nand.dtb', needed by 'arch/arm64/boot/dts/mediatek/'. Stop. > > any thoughts? but this way i will need all possible combinations, i guess best way it to leave the overlays themselves. You missed my correction: full-dtbs := base.dtb overlay.dtb dtb-y += full.dtb Rob
> Gesendet: Samstag, 26. November 2022 um 23:49 Uhr > Von: "Rob Herring" <robh@kernel.org> > An: "Frank Wunderlich" <frank-w@public-files.de> > Cc: "Frank Wunderlich" <linux@fw-web.de>, linux-mediatek@lists.infradead.org > Betreff: Re: Re: [PATCH v6 11/11] arm64: dts: mt7986: add BPI-R3 nand/nor overlays > > > > >> >These need rules to apply them to the base dtb(s). You just need: > > > > >> > > > > > >> >full.dtb := base.dtb overlay.dtb > > > > >> >dtb-y += full.dtb > > > > Hi, > > > > (removed most recipients to not spam all people with this) > > > > have now locally changed sd+emmc to overlays too and renamed to dtso... > > tried also this way to (arch/arm64/boot/dts/mediatek/Makefile): > > > > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb > > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-emmc.dtbo > > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nand.dtbo > > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-nor.dtbo > > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-sd.dtbo > > > > mt7986a-bananapi-bpi-r3-sd-nand.dtb := mt7986a-bananapi-bpi-r3.dtb mt7986a-bananapi-bpi-r3-sd.dtbo mt7986a-bananapi-bpi-r3-nand.dtbo > > dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-sd-nand.dtb > > > > does not work: > > > > make[3]: *** No rule to make target 'arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-sd-nand.dtb', needed by 'arch/arm64/boot/dts/mediatek/'. Stop. > > > > any thoughts? but this way i will need all possible combinations, i guess best way it to leave the overlays themselves. > > You missed my correction: > > full-dtbs := base.dtb overlay.dtb > dtb-y += full.dtb Thanks, using dash instead of dot and dtbs extension in full target works mt7986a-bananapi-bpi-r3-sd-nand-dtbs := mt7986a-bananapi-bpi-r3.dtb mt7986a-bananapi-bpi-r3-sd.dtbo mt7986a-bananapi-bpi-r3-nand.dtbo dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-sd-nand.dtb but i guess this is not the right way for mainline as this will add 5 more lines (10 in total for this single board) to makefile to compile all 4 combinations. i would send base dts and 4 overlays like above in next version if this is the right way. regards Frank
diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile index e8902f2cc58f..d42208c4090d 100644 --- 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 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3-sd.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-rfb.dtb dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986b-rfb.dtb diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-nand.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-nand.dts new file mode 100644 index 000000000000..15ee8c568f3c --- /dev/null +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-nand.dts @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */ +/* + * Authors: Daniel Golle <daniel@makrotopia.org> + * Frank Wunderlich <frank-w@public-files.de> + */ + +/dts-v1/; +/plugin/; + +/ { + compatible = "bananapi,bpi-r3", "mediatek,mt7986a"; + + fragment@0 { + target-path = "/soc/spi@1100a000"; + __overlay__ { + #address-cells = <1>; + #size-cells = <0>; + spi_nand: spi_nand@0 { + compatible = "spi-nand"; + reg = <0>; + spi-max-frequency = <10000000>; + spi-tx-buswidth = <4>; + spi-rx-buswidth = <4>; + + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + label = "bl2"; + reg = <0x0 0x80000>; + read-only; + }; + + partition@80000 { + label = "reserved"; + reg = <0x80000 0x300000>; + }; + + partition@380000 { + label = "fip"; + reg = <0x380000 0x200000>; + read-only; + }; + + partition@580000 { + label = "ubi"; + reg = <0x580000 0x7a80000>; + }; + }; + }; + }; + }; +}; diff --git a/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-nor.dts b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-nor.dts new file mode 100644 index 000000000000..f18643a9089c --- /dev/null +++ b/arch/arm64/boot/dts/mediatek/mt7986a-bananapi-bpi-r3-nor.dts @@ -0,0 +1,69 @@ +/* SPDX-License-Identifier: (GPL-2.0-only OR MIT) */ +/* + * Authors: Daniel Golle <daniel@makrotopia.org> + * Frank Wunderlich <frank-w@public-files.de> + */ + +/dts-v1/; +/plugin/; + +/ { + compatible = "bananapi,bpi-r3", "mediatek,mt7986a"; + + fragment@0 { + target-path = "/soc/spi@1100a000"; + __overlay__ { + #address-cells = <1>; + #size-cells = <0>; + flash@0 { + compatible = "jedec,spi-nor"; + reg = <0>; + spi-max-frequency = <10000000>; + + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + label = "bl2"; + reg = <0x0 0x20000>; + read-only; + }; + + partition@20000 { + label = "reserved"; + reg = <0x20000 0x20000>; + }; + + partition@40000 { + label = "u-boot-env"; + reg = <0x40000 0x40000>; + }; + + partition@80000 { + label = "reserved2"; + reg = <0x80000 0x80000>; + }; + + partition@100000 { + label = "fip"; + reg = <0x100000 0x80000>; + read-only; + }; + + partition@180000 { + label = "recovery"; + reg = <0x180000 0xa80000>; + }; + + partition@c00000 { + label = "fit"; + reg = <0xc00000 0x1400000>; + compatible = "denx,fit"; + }; + }; + }; + }; + }; +};