Re: [Buildroot] [PATCH 1/1] package/rpi-firmware: fix missing files in overlays

[Peter Seiderer <ps.report@gmx.net>]

Hello Rutger,

On Wed, 9 Mar 2022 10:25:24 +0000, "Sassen, Rutger" <rsassen@comecer.com> wrote:

> Hi Peter,
>
> > > Not all files in boot/overlays are copied to the image, which may result
> > > in overlays not being loaded at all, or a wrong overlay being loaded.
> > >
> > > When using os_prefix and/or overlay_prefix in the raspberry pi
> > > config.txt, it checks for the existence of README in the overlays
> > > directory. If it is missing, overlays will not be loaded.
>
> > Any prove for this behavior (could not find anything about it in
> > the docs)?
>
> Yes, it is documented in the raspberry pi docs for config.txt [3], 2nd paragraph:
>

Thanks for the link, but as I read it (and I am pretty sure the given overlays
are loaded from the default location without the README file) the README file is
only needed in case os_prefix is is (overlays stored in a non default location)?

> I've also tested it myself and can confirm that e.g. when using os_prefix=custom/ , and /custom/overlays/README does not exist, that overlays will be loaded from the root /overlays instead, or when /overlays does not exist, no overlays will be loaded at all.
>

Seems to validate the above..., a README file in the default boot partition overlays directory is
not needed (the default buildroot/rpi-firmware/genimage-configs location)..., and the special
cases (os_prefix use cases) are not supported by buildroot out-of-the-box, so I would omit
the README case...

>
> > > The overlay_map.dtb file contains rules for overlays that require
> > > special handling depending on the specific type of raspberry pi
> > > hardware. When this file is missing, the wrong overlay may be loaded.
> >
> > Only if you use this feature, see [1], [2] (building multiple rpi targeting sdcard)
> > instead of determining the right overlay (exactly targeting one rpi flavour sdcard,
> > as the buildroot rpi example config files do)...
>
> That is correct. You have to be careful however when e.g. switching to a new rpi target. When no overlay map is present, you'll have to manually change config.txt to use the correct overlays for your platform instead of relying on the overlay map.

As the buildroot default configs are per raspberry pi model this is no use-case/problem...., but optional
support for multiple targeting sdcards is welcome ;-)

>
> > Would prefer to only/explicitly add the overlay_map.dtb file:
> >
> > $(foreach ovldtb,$(wildcard $(@D)/boot/overlays/*.dtbo) $(@D)/boot/overlays/overlay_map.dtb, \
> >
> > or
> >
> > $(foreach ovldtb,$(wildcard $(@D)/boot/overlays/*.dtbo $(@D)/boot/overlays/*.dtb), \
> >
> > or as it is a (maybe unwanted target feature) add a new buildroot rpi-firmware
> > configure option for adding the overlay_map.dtb file...
>
> I had also considered adding only the README and overlay_map.dtb. But I figured if the raspberry pi firmware adds extra files, they would probably be important. Not copying them might introduce unexpected behavior. When not copying all files, each time when bumping rpi-firmware to a new version one should check if files were added, and if they are worth copying.
>
> I could add a configure option to also copy overlay_map.dtb and README like you suggested (probably two separate options then; default on?). E.g.
>
> BR2_PACKAGE_RPI_FIRMWARE_INSTALL_DTB_OVERLAY_MAP

Yes (default off)..., as it is a easy/simple option to enable one implementation
of basic multi-target sd-cards (the other option would be the use of 'Model Filters'
([4]) in config.txt).

> BR2_PACKAGE_RPI_FIRMWARE_INSTALL_DTB_OVERLAY_README

Omit (see above)...

>
> How about instead adding a configure option for specifying a list of files to be copied instead of all files? That way you could also specify exactly which dtbo files should be copied, instead of copying all of them. Currently there are a lot of overlays copied that we don't use for our system configuration. And if in the future new files are added to the rpi-firmware overlays, we don't have to introduce yet another configure option for it. E.g.
>
> BR2_PACKAGE_RPI_FIRMWARE_INSTALL_DTB_OVERLAY_FILES

Matter of taste (as with many options available in buildroot)...., I believe the _FILES option is
overkill/too-complex for 1.1M overlays files (and will not fix/enable all possibles use-cases,
see os_prefix above..., very special cases needs special hand-crafted solutions/scripts/genimage-configs,
etc.)...

Regards,
Peter

[4] https://www.raspberrypi.com/documentation/computers/config_txt.html#model-filters


>
> Thanks for the feedback, regards,
>
> Rutger
>
> [3] https://www.raspberrypi.com/documentation/computers/config_txt.html#os_prefix
>
> This email communication is CONFIDENTIAL. If you are not the intended recipient, you may not use, copy or disclose to anyone the message or any information contained in the message and I ask that you please notify me by return email and delete this communication immediately. Thank you.

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot