* Re: [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays
2022-03-17 11:06 [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays Sassen, Rutger
@ 2022-03-17 18:03 ` Peter Seiderer
2022-03-17 19:02 ` Bartosz Bilas
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Peter Seiderer @ 2022-03-17 18:03 UTC (permalink / raw)
To: Sassen, Rutger; +Cc: Martin Bark, Mahyar Koshkouei, buildroot
Hello Rutger,
On Thu, 17 Mar 2022 11:06:22 +0000, "Sassen, Rutger" <rsassen@comecer.com> wrote:
> 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>
Thanks for new patch iteration...
Reviewed-by: Peter Seiderer <ps.report@gmx.net>
Regards,
Peter
> ---
> 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
> + 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/
> +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))
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays
2022-03-17 11:06 [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays Sassen, Rutger
2022-03-17 18:03 ` Peter Seiderer
@ 2022-03-17 19:02 ` Bartosz Bilas
2022-03-21 21:56 ` Arnout Vandecappelle
2022-03-23 21:40 ` Peter Korsgaard
3 siblings, 0 replies; 9+ messages in thread
From: Bartosz Bilas @ 2022-03-17 19:02 UTC (permalink / raw)
To: Sassen, Rutger; +Cc: Peter Seiderer, buildroot, Martin Bark, Mahyar Koshkouei
[-- Attachment #1.1: Type: text/plain, Size: 3420 bytes --]
Hello,
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.
>
> 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
> + bool "Install DTB overlay map"
> + default n
bool symbols implicitly default to n.
Best
Bartek
> + 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/
> +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
[-- Attachment #1.2: Type: text/html, Size: 5492 bytes --]
[-- Attachment #2: Type: text/plain, Size: 150 bytes --]
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays
2022-03-17 11:06 [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays Sassen, Rutger
2022-03-17 18:03 ` Peter Seiderer
2022-03-17 19:02 ` Bartosz Bilas
@ 2022-03-21 21:56 ` Arnout Vandecappelle
2022-03-22 8:16 ` [Buildroot] [External] " Sassen, Rutger
2022-03-23 19:35 ` [Buildroot] " Peter Seiderer
2022-03-23 21:40 ` Peter Korsgaard
3 siblings, 2 replies; 9+ messages in thread
From: Arnout Vandecappelle @ 2022-03-21 21:56 UTC (permalink / raw)
To: Sassen, Rutger, buildroot; +Cc: Peter Seiderer, Martin Bark, Mahyar Koshkouei
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...
> + 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.
Please check if the commit [1] does what you need, if not I can recover the
original patch with the Config.in option.
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Buildroot] [External] Re: [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays
2022-03-21 21:56 ` Arnout Vandecappelle
@ 2022-03-22 8:16 ` Sassen, Rutger
2022-03-23 19:35 ` [Buildroot] " Peter Seiderer
1 sibling, 0 replies; 9+ messages in thread
From: Sassen, Rutger @ 2022-03-22 8:16 UTC (permalink / raw)
To: Arnout Vandecappelle, buildroot
Cc: Peter Seiderer, Martin Bark, Mahyar Koshkouei
[-- Attachment #1.1: Type: text/plain, Size: 1788 bytes --]
Hi Arnout,
> > 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 <mailto: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...
...
> ... reducing the patch to just this single line.
Almost back to the first iteration of my patch, where overlay_map.dtb and README were copied unconditionally 😉
> Please check if the commit [1] does what you need, if not I can recover the
> original patch with the http://Config.in option.
I've checked, and it works for me. Thanks.
> Applied to master, thanks.
>
> Regards,
> Arnout
>
> [1]
> https://git.buildroot.org/buildroot/commit/?id=da38cdead909aa133b9c468ddebf3f67c8d198f2
Regards,
Rutger
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.
[-- Attachment #1.2: Type: text/html, Size: 2574 bytes --]
[-- Attachment #2: Type: text/plain, Size: 150 bytes --]
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays
2022-03-21 21:56 ` Arnout Vandecappelle
2022-03-22 8:16 ` [Buildroot] [External] " Sassen, Rutger
@ 2022-03-23 19:35 ` Peter Seiderer
2022-03-23 19:49 ` Peter Seiderer
2022-03-23 20:06 ` Arnout Vandecappelle
1 sibling, 2 replies; 9+ messages in thread
From: Peter Seiderer @ 2022-03-23 19:35 UTC (permalink / raw)
To: Arnout Vandecappelle
Cc: Martin Bark, Sassen, Rutger, Mahyar Koshkouei, buildroot
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays
2022-03-23 19:35 ` [Buildroot] " Peter Seiderer
@ 2022-03-23 19:49 ` Peter Seiderer
2022-03-23 20:06 ` Arnout Vandecappelle
1 sibling, 0 replies; 9+ messages in thread
From: Peter Seiderer @ 2022-03-23 19:49 UTC (permalink / raw)
To: Arnout Vandecappelle
Cc: buildroot, Martin Bark, Mahyar Koshkouei, Sassen, Rutger
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays
2022-03-23 19:35 ` [Buildroot] " Peter Seiderer
2022-03-23 19:49 ` Peter Seiderer
@ 2022-03-23 20:06 ` Arnout Vandecappelle
1 sibling, 0 replies; 9+ messages in thread
From: Arnout Vandecappelle @ 2022-03-23 20:06 UTC (permalink / raw)
To: Peter Seiderer; +Cc: Martin Bark, Sassen, Rutger, Mahyar Koshkouei, buildroot
On 23/03/2022 20:35, Peter Seiderer 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])...
But AFAIU the presence of the overlay map doesn't stop you from using the
other mechanisms, right? It's similar to how we install all the dtbo files even
though maybe only one of them is relevant for you.
If the presence of the overlay map does have an effect (i.e. if you'd need to
remove it in the post-build script to get a correctly booting system), then the
option indeed does make sense.
Context: we want to avoid adding too many Config.in options, to make the
complexity of the menus manageable. We make something a Config.in option only if
it has a significant impact on rootfs size, or if it makes the difference
between working and non-working system.
Regards,
Arnout
>
>>
>>> + 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays
2022-03-17 11:06 [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays Sassen, Rutger
` (2 preceding siblings ...)
2022-03-21 21:56 ` Arnout Vandecappelle
@ 2022-03-23 21:40 ` Peter Korsgaard
3 siblings, 0 replies; 9+ messages in thread
From: Peter Korsgaard @ 2022-03-23 21:40 UTC (permalink / raw)
To: Sassen, Rutger; +Cc: Peter Seiderer, Martin Bark, Mahyar Koshkouei, buildroot
>>>>> "Sassen," == Sassen, Rutger <rsassen@comecer.com> writes:
> 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>
> ---
> 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)
Committed to 2021.02.x, 2021.11.x and 2022.02.x, thanks.
--
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot
^ permalink raw reply [flat|nested] 9+ messages in thread