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

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

Hello *,

On Wed, 23 Mar 2022 20:35:35 +0100, Peter Seiderer <ps.report@gmx.net> wrote:

> 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])...

Missed to add the link

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

Regards,
Peter

>
> >
> > > + 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

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