All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH v3 1/1] package/rpi-firmware: fix missing files in overlays
@ 2022-03-17 11:06 Sassen, Rutger
  2022-03-17 18:03 ` Peter Seiderer
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Sassen, Rutger @ 2022-03-17 11:06 UTC (permalink / raw)
  To: buildroot; +Cc: Peter Seiderer, Martin Bark, Mahyar Koshkouei


[-- Attachment #1.1: Type: text/plain, Size: 3116 bytes --]

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

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: 3733 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 related	[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
                   ` (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

end of thread, other threads:[~2022-03-23 21:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 19:49     ` Peter Seiderer
2022-03-23 20:06     ` Arnout Vandecappelle
2022-03-23 21:40 ` Peter Korsgaard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.