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

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

Hello Arnout,

On Mon, 21 Mar 2022 22:56:41 +0100, Arnout Vandecappelle <arnout@mind.be> wrote:

> On 17/03/2022 12:06, Sassen, Rutger wrote:
> > When supporting multiple hardware targets, overlay_map.dtb might
> > be needed to map overlay names to one of several implementations.
>
>   I've added a reference to the documentation.
>
> >
> > Signed-off-by: Rutger Sassen <rsassen@comecer.com>
> > ---
> > Changes v2 -> v3:
> > - in Config.in use if statement instead of depends (suggested by Peter
> > Seiderer)
> > - mention the filename overlay_map.dtb in the help text (suggested by
> > Peter Seiderer)
> >
> > Changes v1 -> v2:
> > - instead of always copying overlay_map.dtb, make it configurable
> > (suggested by Peter Seiderer)
> > - do not copy README since it a special case, only needed when
> > installing overlays to a non-standard location (suggested by Peter
> > Seiderer)
> >
> > When supporting multiple hardware targets, overlay_map.dtb might
> > be needed to map overlay names to one of several implementations.
> >
> > Signed-off-by: Rutger Sassen <rsassen@comecer.com>
> > ---
> > package/rpi-firmware/Config.in | 12 ++++++++++++
> > package/rpi-firmware/rpi-firmware.mk | 7 +++++++
> > 2 files changed, 19 insertions(+)
> >
> > diff --git a/package/rpi-firmware/Config.in b/package/rpi-firmware/Config.in
> > index 8070dc3019..36054353f8 100644
> > --- a/package/rpi-firmware/Config.in
> > +++ b/package/rpi-firmware/Config.in
> > @@ -93,6 +93,18 @@ config BR2_PACKAGE_RPI_FIRMWARE_INSTALL_DTB_OVERLAYS
> > overlays, to support HATs (Hardware Attached on Top, add-on
> > modules).
> >
> > +if BR2_PACKAGE_RPI_FIRMWARE_INSTALL_DTB_OVERLAYS
> > +
> > +config BR2_PACKAGE_RPI_FIRMWARE_INSTALL_DTB_OVERLAY_MAP
>
>   In the end, I don't think this option is very useful. So I removed it...

Why to you think so?

I am still convinced the overlay map should be optional (in favor of explicit
overlay/feature loading/enabling instead of firmware magic) and the
overlay map is only one way to achieve a hardware dependent feature loading
(the other way is via Model Filters in config.txt, see [2])...

>
> > + bool "Install DTB overlay map"
> > + default n
> > + help
> > + Say 'y' here if you need to support multiple hardware targets
> > + and you need overlay_map.dtb for target dependent mapping of
> > + overlay names to one of several overlay implementations.
> > +
> > +endif # BR2_PACKAGE_RPI_FIRMWARE_INSTALL_DTB_OVERLAYS
> > +
> > config BR2_PACKAGE_RPI_FIRMWARE_INSTALL_VCDBG
> > bool "vcdbg"
> > depends on BR2_arm # prebuilt arm binary, rpi-userland
> > diff --git a/package/rpi-firmware/rpi-firmware.mk
> > b/package/rpi-firmware/rpi-firmware.mk
> > index b46a7f5270..800b560070 100644
> > --- a/package/rpi-firmware/rpi-firmware.mk
> > +++ b/package/rpi-firmware/rpi-firmware.mk
> > @@ -57,6 +57,12 @@ define RPI_FIRMWARE_INSTALL_DTB_OVERLAYS
> > endef
> > endif
> >
> > +ifeq ($(BR2_PACKAGE_RPI_FIRMWARE_INSTALL_DTB_OVERLAY_MAP),y)
> > +define RPI_FIRMWARE_INSTALL_DTB_OVERLAY_MAP
> > + $(INSTALL) -D -m 0644 $(@D)/boot/overlays/overlay_map.dtb
> > $(BINARIES_DIR)/rpi-firmware/overlays/
>
> ... reducing the patch to just this single line.

Which renders the review process to void (as the option was introduced by
Rutger on my request and this is what I reviewed/acknowledged by the Reviewed-by tag)...

>
>   Please check if the commit [1] does what you need, if not I can recover the
> original patch with the Config.in option.

+1 for the Config.in option ;-)

Regards,
Peter

>
>   Applied to master, thanks.
>
>   Regards,
>   Arnout
>
> [1]
> https://git.buildroot.org/buildroot/commit/?id=da38cdead909aa133b9c468ddebf3f67c8d198f2
>
>
>
> > +endef
> > +endif
> > +
> > # Install prebuilt libraries if RPI_USERLAND not enabled
> > ifneq ($(BR2_PACKAGE_RPI_USERLAND),y)
> > define RPI_FIRMWARE_INSTALL_TARGET_LIB
> > @@ -83,6 +89,7 @@ define RPI_FIRMWARE_INSTALL_IMAGES_CMDS
> > $(RPI_FIRMWARE_INSTALL_CONFIG)
> > $(RPI_FIRMWARE_INSTALL_DTB)
> > $(RPI_FIRMWARE_INSTALL_DTB_OVERLAYS)
> > + $(RPI_FIRMWARE_INSTALL_DTB_OVERLAY_MAP)
> > endef
> >
> > $(eval $(generic-package))
> > --
> > 2.20.1
> >
> >
> > *Disclaimer*
> >
> > 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

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