Re: [PATCH 1/1] arm: dts: Enable device-tree overlay support for sun8i-h3 pi devices

["Jernej Škrabec" <jernej.skrabec@gmail.com>]

Dne sreda, 19. julij 2023 ob 14:26:13 CEST je Moessbauer Felix napisal(a):
> On Tue, 2023-07-11 at 21:42 +0200, Jernej Škrabec wrote:
> > Dne torek, 27. junij 2023 ob 15:37:03 CEST je Felix Moessbauer
> > 
> > napisal(a):
> > > Add the '-@' DTC option for the sun8i-h3 pi-class devices. This
> > > option
> > > populates the '__symbols__' node that contains all the necessary
> > > symbols
> > > for supporting device-tree overlays (for instance from the firmware
> > > or
> > > the bootloader) on these devices.
> > > 
> > > These devices allow various modules to be connected and this
> > > enables
> > > users to create out-of-tree device-tree overlays for these modules.
> > > 
> > > Please note that this change does increase the size of the
> > > resulting DTB
> > > by ~30%. For example, with v6.4 increase in size is as follows:
> > > 
> > > 22909 -> 29564 sun8i-h3-orangepi-lite.dtb
> > > 24214 -> 30935 sun8i-h3-bananapi-m2-plus.dtb
> > > 23915 -> 30664 sun8i-h3-nanopi-m1-plus.dtb
> > > 22969 -> 29537 sun8i-h3-nanopi-m1.dtb
> > > 24157 -> 30836 sun8i-h3-nanopi-duo2.dtb
> > > 24110 -> 30845 sun8i-h3-orangepi-plus2e.dtb
> > > 23472 -> 30037 sun8i-h3-orangepi-one.dtb
> > > 24600 -> 31410 sun8i-h3-orangepi-plus.dtb
> > > 23618 -> 30230 sun8i-h3-orangepi-2.dtb
> > > 22170 -> 28548 sun8i-h3-orangepi-zero-plus2.dtb
> > > 23258 -> 29795 sun8i-h3-nanopi-neo-air.dtb
> > > 23113 -> 29699 sun8i-h3-zeropi.dtb
> > > 22803 -> 29270 sun8i-h3-nanopi-neo.dtb
> > > 24674 -> 31318 sun8i-h3-nanopi-r1.dtb
> > > 23477 -> 30038 sun8i-h3-orangepi-pc.dtb
> > > 24622 -> 31380 sun8i-h3-bananapi-m2-plus-v1.2.dtb
> > > 23750 -> 30366 sun8i-h3-orangepi-pc-plus.dtb
> > > 
> > > Signed-off-by: Felix Moessbauer <felix.moessbauer@siemens.com>
> > > ---
> > > Please note that I only tested the overlay on the sun8i-h3-nanopi-
> > > neo
> > > device. However, the devices are quite similar and in general the
> > > change to add symbols should be pretty safe. Similar patches have
> > > been
> > > applied to various other devices in the past without any negative
> > > effect (except for the increased size).
> > 
> > I'm not a fan of this approach. What's wrong with letting kernel
> > builders
> > specify this flag on their own at compile time? That way size is
> > still
> > completely in domain of builder.
> 
> That's easier said than done. These symbols are only needed for some
> targets (that are commonly used with overlays). But how should a
> general purpose kernel builder like Debian know which DT to build with
> symbols and which without? In Yocto the overlays are not really needed,
> as often the original dts is just patched directly.

LibreELEC (distro I help with) has support for DT overlays for some time and 
it was easy to enable symbols for all DTs with simple "DTC_FLAGS=-@" addition 
to build command [1]. Do you see any downside except knowing you have to 
include it? On the upside, there is no need to patch each and every file and 
kernel builder has a choice if overlays should be supported or not. Note that 
in some cases overlays can be needed even in the case when board doesn't have 
expansion ports.

Best regards,
Jernej

[1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/packages/linux/
package.mk#L213

> 
> There has been a discussion in 2017 to improve the situation in
> general, but without any final decision or implementation [1]. By that,
> these patches got more and more common. I recently sent a very similar
> patch for a riscv board that got accepted [2].
> 
> IMHO it is better to have these patches in the kernel instead of
> carrying different variants of this in each Linux distro. IMHO, the
> board vendors themselves should add this when they add a board.
> 
> [1]
> https://lore.kernel.org/lkml/1502831736-28282-1-git-send-email-trini@konsulk
> o.com/ [2]
> https://lore.kernel.org/linux-devicetree/20230627080620.329873-1-felix.moess
> bauer@siemens.com/
> 
> Best regards,
> Felix
> 
> > Best regards,
> > Jernej
> > 
> > > Felix Moessbauer
> > > Siemens AG
> > > 
> > >  arch/arm/boot/dts/allwinner/Makefile | 19 +++++++++++++++++++
> > >  1 file changed, 19 insertions(+)
> > > 
> > > diff --git a/arch/arm/boot/dts/allwinner/Makefile
> > > b/arch/arm/boot/dts/allwinner/Makefile index
> > > 589a1ce1120a..eebb5a0c873a
> > > 100644
> > > --- a/arch/arm/boot/dts/allwinner/Makefile
> > > +++ b/arch/arm/boot/dts/allwinner/Makefile
> > > @@ -179,6 +179,25 @@ dtb-$(CONFIG_MACH_SUN7I) += \
> > >         sun7i-a20-pcduino3-nano.dtb \
> > >         sun7i-a20-wexler-tab7200.dtb \
> > >         sun7i-a20-wits-pro-a20-dkt.dtb
> > > +
> > > +# Enables support for device-tree overlays for all pis
> > > +DTC_FLAGS_sun8i-h3-orangepi-lite := -@
> > > +DTC_FLAGS_sun8i-h3-bananapi-m2-plus := -@
> > > +DTC_FLAGS_sun8i-h3-nanopi-m1-plus := -@
> > > +DTC_FLAGS_sun8i-h3-nanopi-m1 := -@
> > > +DTC_FLAGS_sun8i-h3-nanopi-duo2 := -@
> > > +DTC_FLAGS_sun8i-h3-orangepi-plus2e := -@
> > > +DTC_FLAGS_sun8i-h3-orangepi-one := -@
> > > +DTC_FLAGS_sun8i-h3-orangepi-plus := -@
> > > +DTC_FLAGS_sun8i-h3-orangepi-2 := -@
> > > +DTC_FLAGS_sun8i-h3-orangepi-zero-plus2 := -@
> > > +DTC_FLAGS_sun8i-h3-nanopi-neo-air := -@
> > > +DTC_FLAGS_sun8i-h3-zeropi := -@
> > > +DTC_FLAGS_sun8i-h3-nanopi-neo := -@
> > > +DTC_FLAGS_sun8i-h3-nanopi-r1 := -@
> > > +DTC_FLAGS_sun8i-h3-orangepi-pc := -@
> > > +DTC_FLAGS_sun8i-h3-bananapi-m2-plus-v1.2 := -@
> > > +DTC_FLAGS_sun8i-h3-orangepi-pc-plus := -@
> > >  dtb-$(CONFIG_MACH_SUN8I) += \
> > >         sun8i-a23-evb.dtb \
> > >         sun8i-a23-gt90h-v4.dtb \