diff mbox series

u-boot: add hook to prepare binaries before copying

Message ID 20250409221851.1005988-1-dmitry.baryshkov@oss.qualcomm.com
State New
Headers show
Series u-boot: add hook to prepare binaries before copying | expand

Commit Message

Dmitry Baryshkov April 9, 2025, 10:18 p.m. UTC
BSP layers might need to perform additional steps on the U-Boot binary,
which are not a part of the oe_runmake invocation. For example, the
binary might need to be signed or converted to the Android boot image.

In order to let BSP recipes to perform such tasks without duplicating
u-boot.inc functions, add a special hook to be called before
uboot_compile_config_copy_binary().

Note: it is of course possible to push necessary steps into
uboot_compile_config_copy_binary() itself, but it doesn't look as clean
as just adding a hook.

Cc: Koen Kooi <koen.kooi@oss.qualcomm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 meta/recipes-bsp/u-boot/u-boot.inc | 8 ++++++++
 1 file changed, 8 insertions(+)

Comments

Joshua Watt April 14, 2025, 5 p.m. UTC | #1
On Wed, Apr 9, 2025 at 4:18 PM Dmitry Baryshkov via
lists.openembedded.org
<dmitry.baryshkov=oss.qualcomm.com@lists.openembedded.org> wrote:
>
> BSP layers might need to perform additional steps on the U-Boot binary,
> which are not a part of the oe_runmake invocation. For example, the
> binary might need to be signed or converted to the Android boot image.
>
> In order to let BSP recipes to perform such tasks without duplicating
> u-boot.inc functions, add a special hook to be called before
> uboot_compile_config_copy_binary().
>
> Note: it is of course possible to push necessary steps into
> uboot_compile_config_copy_binary() itself, but it doesn't look as clean
> as just adding a hook.

It's unclear why a _prepare is necessary, when it passes the exact
same argument to _copy, and _copy is only one line (so you could do
the same thing by making your _copy contain what you want in _prepare
and the one `cp` line from _copy)

Is there some distinct advantage to _prepare, beyond allowing keeping
the one line default _copy?

>
> Cc: Koen Kooi <koen.kooi@oss.qualcomm.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  meta/recipes-bsp/u-boot/u-boot.inc | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/meta/recipes-bsp/u-boot/u-boot.inc b/meta/recipes-bsp/u-boot/u-boot.inc
> index 9464736b8418..fe86ba0ad011 100644
> --- a/meta/recipes-bsp/u-boot/u-boot.inc
> +++ b/meta/recipes-bsp/u-boot/u-boot.inc
> @@ -81,6 +81,7 @@ uboot_compile_config () {
>      for binary in ${UBOOT_BINARIES}; do
>          k=$(expr $k + 1);
>          if [ $k -eq $i ]; then
> +            uboot_compile_config_prepare_binary $config $type $binary
>              uboot_compile_config_copy_binary $config $type $binary
>          fi
>      done
> @@ -93,6 +94,13 @@ uboot_compile_config () {
>      fi
>  }
>
> +# Do nothing by default, this is a hook to be extended by BSP recipes
> +uboot_compile_config_prepare_binary () {
> +    config=$1
> +    type=$2
> +    binary=$3
> +}
> +
>  uboot_compile_config_copy_binary () {
>      config=$1
>      type=$2
> --
> 2.47.2
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#214598): https://lists.openembedded.org/g/openembedded-core/message/214598
> Mute This Topic: https://lists.openembedded.org/mt/112181580/3616693
> Group Owner: openembedded-core+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [JPEWhacker@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
Dmitry Baryshkov April 15, 2025, 8:58 a.m. UTC | #2
On 14/04/2025 20:00, Joshua Watt wrote:
> On Wed, Apr 9, 2025 at 4:18 PM Dmitry Baryshkov via
> lists.openembedded.org
> <dmitry.baryshkov=oss.qualcomm.com@lists.openembedded.org> wrote:
>>
>> BSP layers might need to perform additional steps on the U-Boot binary,
>> which are not a part of the oe_runmake invocation. For example, the
>> binary might need to be signed or converted to the Android boot image.
>>
>> In order to let BSP recipes to perform such tasks without duplicating
>> u-boot.inc functions, add a special hook to be called before
>> uboot_compile_config_copy_binary().
>>
>> Note: it is of course possible to push necessary steps into
>> uboot_compile_config_copy_binary() itself, but it doesn't look as clean
>> as just adding a hook.
> 
> It's unclear why a _prepare is necessary, when it passes the exact
> same argument to _copy, and _copy is only one line (so you could do
> the same thing by making your _copy contain what you want in _prepare
> and the one `cp` line from _copy)
> 
> Is there some distinct advantage to _prepare, beyond allowing keeping
> the one line default _copy?

Well. It makes a distinction logical: first you build something, then 
you copy it. Also, if we prepend the copy hook, we also need to parse 
the arguments, which then get parsed again, so the code looks untidy.

 From my POV it's just a matter of preference and I'd be happy to follow 
any maintainer's decision on this topic.

> 
>>
>> Cc: Koen Kooi <koen.kooi@oss.qualcomm.com>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> ---
>>   meta/recipes-bsp/u-boot/u-boot.inc | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/meta/recipes-bsp/u-boot/u-boot.inc b/meta/recipes-bsp/u-boot/u-boot.inc
>> index 9464736b8418..fe86ba0ad011 100644
>> --- a/meta/recipes-bsp/u-boot/u-boot.inc
>> +++ b/meta/recipes-bsp/u-boot/u-boot.inc
>> @@ -81,6 +81,7 @@ uboot_compile_config () {
>>       for binary in ${UBOOT_BINARIES}; do
>>           k=$(expr $k + 1);
>>           if [ $k -eq $i ]; then
>> +            uboot_compile_config_prepare_binary $config $type $binary
>>               uboot_compile_config_copy_binary $config $type $binary
>>           fi
>>       done
>> @@ -93,6 +94,13 @@ uboot_compile_config () {
>>       fi
>>   }
>>
>> +# Do nothing by default, this is a hook to be extended by BSP recipes
>> +uboot_compile_config_prepare_binary () {
>> +    config=$1
>> +    type=$2
>> +    binary=$3
>> +}
>> +
>>   uboot_compile_config_copy_binary () {
>>       config=$1
>>       type=$2
>> --
>> 2.47.2
>>
>>
>> -=-=-=-=-=-=-=-=-=-=-=-
>> Links: You receive all messages sent to this group.
>> View/Reply Online (#214598): https://lists.openembedded.org/g/openembedded-core/message/214598
>> Mute This Topic: https://lists.openembedded.org/mt/112181580/3616693
>> Group Owner: openembedded-core+owner@lists.openembedded.org
>> Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [JPEWhacker@gmail.com]
>> -=-=-=-=-=-=-=-=-=-=-=-
>>
diff mbox series

Patch

diff --git a/meta/recipes-bsp/u-boot/u-boot.inc b/meta/recipes-bsp/u-boot/u-boot.inc
index 9464736b8418..fe86ba0ad011 100644
--- a/meta/recipes-bsp/u-boot/u-boot.inc
+++ b/meta/recipes-bsp/u-boot/u-boot.inc
@@ -81,6 +81,7 @@  uboot_compile_config () {
     for binary in ${UBOOT_BINARIES}; do
         k=$(expr $k + 1);
         if [ $k -eq $i ]; then
+            uboot_compile_config_prepare_binary $config $type $binary
             uboot_compile_config_copy_binary $config $type $binary
         fi
     done
@@ -93,6 +94,13 @@  uboot_compile_config () {
     fi
 }
 
+# Do nothing by default, this is a hook to be extended by BSP recipes
+uboot_compile_config_prepare_binary () {
+    config=$1
+    type=$2
+    binary=$3
+}
+
 uboot_compile_config_copy_binary () {
     config=$1
     type=$2