[DistroKit] [PATCH] v8a: barebox: add missing dependency on TF_A for i.MX

[DistroKit] [PATCH] v8a: barebox: add missing dependency on TF_A for i.MX

[Ahmad Fatoum]

Unlike firmware-rockchip, firmware-imx doesn't ship BL31 (TF-A). Instead
we configure the upstream TF-A rule to build images for i.MX8M Quad,
Mini and Plus.

With the switch to the upstream barebox rule, this dependency on TF-A
got lost breaking less-parallel fresh builds, so let's add it back.

Preferably, TF_A would be selected directly from the barebox rule's
Kconfig. There's a BAREBOX_DEPENDENCIES option for that, but I don't
know if it's possible to extend barebox.in without having to duplicate
it fully, so adding it to firmware-imx should suffice for now.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 configs/platform-v8a/rules/firmware-imx.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/platform-v8a/rules/firmware-imx.in b/configs/platform-v8a/rules/firmware-imx.in
index 5f06a44416fa..c756b963e5dc 100644
--- a/configs/platform-v8a/rules/firmware-imx.in
+++ b/configs/platform-v8a/rules/firmware-imx.in
@@ -10,6 +10,7 @@ menuconfig FIRMWARE_IMX
 config BAREBOX_DEPENDENCIES
 	tristate
 	select FIRMWARE_IMX
+	select TF_A
 
 if FIRMWARE_IMX
 
-- 
2.39.2

Re: [DistroKit] [PATCH] v8a: barebox: add missing dependency on TF_A for i.MX

[Ahmad Fatoum]

Hello Michael,

On 01.06.23 08:48, Ahmad Fatoum wrote:
> Unlike firmware-rockchip, firmware-imx doesn't ship BL31 (TF-A). Instead
> we configure the upstream TF-A rule to build images for i.MX8M Quad,
> Mini and Plus.
> 
> With the switch to the upstream barebox rule, this dependency on TF-A
> got lost breaking less-parallel fresh builds, so let's add it back.
> 
> Preferably, TF_A would be selected directly from the barebox rule's
> Kconfig. There's a BAREBOX_DEPENDENCIES option for that, but I don't
> know if it's possible to extend barebox.in without having to duplicate
> it fully, so adding it to firmware-imx should suffice for now.

Is there something like post/barebox-fixup.make, but for Kconfig files
that I should be using instead?

> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  configs/platform-v8a/rules/firmware-imx.in | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/configs/platform-v8a/rules/firmware-imx.in b/configs/platform-v8a/rules/firmware-imx.in
> index 5f06a44416fa..c756b963e5dc 100644
> --- a/configs/platform-v8a/rules/firmware-imx.in
> +++ b/configs/platform-v8a/rules/firmware-imx.in
> @@ -10,6 +10,7 @@ menuconfig FIRMWARE_IMX
>  config BAREBOX_DEPENDENCIES
>  	tristate
>  	select FIRMWARE_IMX
> +	select TF_A
>  
>  if FIRMWARE_IMX
>  

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [DistroKit] [PATCH] v8a: barebox: add missing dependency on TF_A for i.MX

[Michael Olbrich]

On Thu, Jun 01, 2023 at 08:49:38AM +0200, Ahmad Fatoum wrote:
> On 01.06.23 08:48, Ahmad Fatoum wrote:
> > Unlike firmware-rockchip, firmware-imx doesn't ship BL31 (TF-A). Instead
> > we configure the upstream TF-A rule to build images for i.MX8M Quad,
> > Mini and Plus.
> > 
> > With the switch to the upstream barebox rule, this dependency on TF-A
> > got lost breaking less-parallel fresh builds, so let's add it back.
> > 
> > Preferably, TF_A would be selected directly from the barebox rule's
> > Kconfig. There's a BAREBOX_DEPENDENCIES option for that, but I don't
> > know if it's possible to extend barebox.in without having to duplicate
> > it fully, so adding it to firmware-imx should suffice for now.
> 
> Is there something like post/barebox-fixup.make, but for Kconfig files
> that I should be using instead?

This looks fine. We need the special stuff in make, because you cannot
extend a make target in another makefile. It's easier with Kconfig:
All occurrences of a symbol are merged so you just need to make sure that
the file with you're new dependency is actually loaded.

Michael

> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > ---
> >  configs/platform-v8a/rules/firmware-imx.in | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/configs/platform-v8a/rules/firmware-imx.in b/configs/platform-v8a/rules/firmware-imx.in
> > index 5f06a44416fa..c756b963e5dc 100644
> > --- a/configs/platform-v8a/rules/firmware-imx.in
> > +++ b/configs/platform-v8a/rules/firmware-imx.in
> > @@ -10,6 +10,7 @@ menuconfig FIRMWARE_IMX
> >  config BAREBOX_DEPENDENCIES
> >  	tristate
> >  	select FIRMWARE_IMX
> > +	select TF_A
> >  
> >  if FIRMWARE_IMX
> >  
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [DistroKit] [PATCH] v8a: barebox: add missing dependency on TF_A for i.MX

[Ahmad Fatoum]

On 01.06.23 09:05, Michael Olbrich wrote:
> On Thu, Jun 01, 2023 at 08:49:38AM +0200, Ahmad Fatoum wrote:
>> On 01.06.23 08:48, Ahmad Fatoum wrote:
>>> Unlike firmware-rockchip, firmware-imx doesn't ship BL31 (TF-A). Instead
>>> we configure the upstream TF-A rule to build images for i.MX8M Quad,
>>> Mini and Plus.
>>>
>>> With the switch to the upstream barebox rule, this dependency on TF-A
>>> got lost breaking less-parallel fresh builds, so let's add it back.
>>>
>>> Preferably, TF_A would be selected directly from the barebox rule's
>>> Kconfig. There's a BAREBOX_DEPENDENCIES option for that, but I don't
>>> know if it's possible to extend barebox.in without having to duplicate
>>> it fully, so adding it to firmware-imx should suffice for now.
>>
>> Is there something like post/barebox-fixup.make, but for Kconfig files
>> that I should be using instead?
> 
> This looks fine. We need the special stuff in make, because you cannot
> extend a make target in another makefile. It's easier with Kconfig:
> All occurrences of a symbol are merged so you just need to make sure that
> the file with you're new dependency is actually loaded.

It still seems out of place, because TF-A isn't a dependency of firmware-imx,
but of barebox, so it should be selected from a file that's related to barebox,
even if the end result is the same and both ends up as a select from
BAREBOX_DEPENDENCIES. There seems to be no such way though, right?

> 
> Michael
> 
>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>> ---
>>>  configs/platform-v8a/rules/firmware-imx.in | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/configs/platform-v8a/rules/firmware-imx.in b/configs/platform-v8a/rules/firmware-imx.in
>>> index 5f06a44416fa..c756b963e5dc 100644
>>> --- a/configs/platform-v8a/rules/firmware-imx.in
>>> +++ b/configs/platform-v8a/rules/firmware-imx.in
>>> @@ -10,6 +10,7 @@ menuconfig FIRMWARE_IMX
>>>  config BAREBOX_DEPENDENCIES
>>>  	tristate
>>>  	select FIRMWARE_IMX
>>> +	select TF_A
>>>  
>>>  if FIRMWARE_IMX
>>>  
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [DistroKit] [PATCH] v8a: barebox: add missing dependency on TF_A for i.MX

[Michael Olbrich]

On Thu, Jun 01, 2023 at 09:11:40AM +0200, Ahmad Fatoum wrote:
> On 01.06.23 09:05, Michael Olbrich wrote:
> > On Thu, Jun 01, 2023 at 08:49:38AM +0200, Ahmad Fatoum wrote:
> >> On 01.06.23 08:48, Ahmad Fatoum wrote:
> >>> Unlike firmware-rockchip, firmware-imx doesn't ship BL31 (TF-A). Instead
> >>> we configure the upstream TF-A rule to build images for i.MX8M Quad,
> >>> Mini and Plus.
> >>>
> >>> With the switch to the upstream barebox rule, this dependency on TF-A
> >>> got lost breaking less-parallel fresh builds, so let's add it back.
> >>>
> >>> Preferably, TF_A would be selected directly from the barebox rule's
> >>> Kconfig. There's a BAREBOX_DEPENDENCIES option for that, but I don't
> >>> know if it's possible to extend barebox.in without having to duplicate
> >>> it fully, so adding it to firmware-imx should suffice for now.
> >>
> >> Is there something like post/barebox-fixup.make, but for Kconfig files
> >> that I should be using instead?
> > 
> > This looks fine. We need the special stuff in make, because you cannot
> > extend a make target in another makefile. It's easier with Kconfig:
> > All occurrences of a symbol are merged so you just need to make sure that
> > the file with you're new dependency is actually loaded.
> 
> It still seems out of place, because TF-A isn't a dependency of firmware-imx,
> but of barebox, so it should be selected from a file that's related to barebox,
> even if the end result is the same and both ends up as a select from
> BAREBOX_DEPENDENCIES. There seems to be no such way though, right?

Then create configs/platform-v8a/platforms/firmware-tfa.in or whatever you want
to call it, with

## SECTION=bootloader

config BAREBOX_DEPENDENCIES
	select TF_A

As I said, just make sure it's included somewhere and Kconfig will do the
rest.

> >>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >>> ---
> >>>  configs/platform-v8a/rules/firmware-imx.in | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/configs/platform-v8a/rules/firmware-imx.in b/configs/platform-v8a/rules/firmware-imx.in
> >>> index 5f06a44416fa..c756b963e5dc 100644
> >>> --- a/configs/platform-v8a/rules/firmware-imx.in
> >>> +++ b/configs/platform-v8a/rules/firmware-imx.in

Patch opportunities:

Menu files that are only used for the platform belong in
configs/platform-v8a/platforms/

And there is a non_free_firmware section which should be a better fit for
firmware-imx.

> >>> @@ -10,6 +10,7 @@ menuconfig FIRMWARE_IMX
> >>>  config BAREBOX_DEPENDENCIES
> >>>  	tristate

You don't need the 'tristate' here.

Michael

> >>>  	select FIRMWARE_IMX
> >>> +	select TF_A
> >>>  
> >>>  if FIRMWARE_IMX
> >>>  
> >>
> >> -- 
> >> Pengutronix e.K.                           |                             |
> >> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> >> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >>
> >>
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [DistroKit] [PATCH] v8a: barebox: add missing dependency on TF_A for i.MX

[Ahmad Fatoum]

On 01.06.23 11:15, Michael Olbrich wrote:
> On Thu, Jun 01, 2023 at 09:11:40AM +0200, Ahmad Fatoum wrote:
>> On 01.06.23 09:05, Michael Olbrich wrote:
>>> On Thu, Jun 01, 2023 at 08:49:38AM +0200, Ahmad Fatoum wrote:
>>>> On 01.06.23 08:48, Ahmad Fatoum wrote:
>>>>> Unlike firmware-rockchip, firmware-imx doesn't ship BL31 (TF-A). Instead
>>>>> we configure the upstream TF-A rule to build images for i.MX8M Quad,
>>>>> Mini and Plus.
>>>>>
>>>>> With the switch to the upstream barebox rule, this dependency on TF-A
>>>>> got lost breaking less-parallel fresh builds, so let's add it back.
>>>>>
>>>>> Preferably, TF_A would be selected directly from the barebox rule's
>>>>> Kconfig. There's a BAREBOX_DEPENDENCIES option for that, but I don't
>>>>> know if it's possible to extend barebox.in without having to duplicate
>>>>> it fully, so adding it to firmware-imx should suffice for now.
>>>>
>>>> Is there something like post/barebox-fixup.make, but for Kconfig files
>>>> that I should be using instead?
>>>
>>> This looks fine. We need the special stuff in make, because you cannot
>>> extend a make target in another makefile. It's easier with Kconfig:
>>> All occurrences of a symbol are merged so you just need to make sure that
>>> the file with you're new dependency is actually loaded.
>>
>> It still seems out of place, because TF-A isn't a dependency of firmware-imx,
>> but of barebox, so it should be selected from a file that's related to barebox,
>> even if the end result is the same and both ends up as a select from
>> BAREBOX_DEPENDENCIES. There seems to be no such way though, right?
> 
> Then create configs/platform-v8a/platforms/firmware-tfa.in or whatever you want
> to call it, with
> 
> ## SECTION=bootloader
> 
> config BAREBOX_DEPENDENCIES
> 	select TF_A
> 
> As I said, just make sure it's included somewhere and Kconfig will do the
> rest.

That's the things I am asking about. Where would I include that from, if
both tf-a and barebox are upstream rules, whose .in is not in DistroKit.

@rsc, you can pick this patch and I will send a follow-up if I find a better way.

>>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>>> ---
>>>>>  configs/platform-v8a/rules/firmware-imx.in | 1 +
>>>>>  1 file changed, 1 insertion(+)
>>>>>
>>>>> diff --git a/configs/platform-v8a/rules/firmware-imx.in b/configs/platform-v8a/rules/firmware-imx.in
>>>>> index 5f06a44416fa..c756b963e5dc 100644
>>>>> --- a/configs/platform-v8a/rules/firmware-imx.in
>>>>> +++ b/configs/platform-v8a/rules/firmware-imx.in
> 
> Patch opportunities:

Thanks. I will keep that in mind next time I touch this.

> 
> Menu files that are only used for the platform belong in
> configs/platform-v8a/platforms/
> 
> And there is a non_free_firmware section which should be a better fit for
> firmware-imx.
> 
>>>>> @@ -10,6 +10,7 @@ menuconfig FIRMWARE_IMX
>>>>>  config BAREBOX_DEPENDENCIES
>>>>>  	tristate
> 
> You don't need the 'tristate' here.
> 
> Michael
> 
>>>>>  	select FIRMWARE_IMX
>>>>> +	select TF_A
>>>>>  
>>>>>  if FIRMWARE_IMX
>>>>>  
>>>>
>>>> -- 
>>>> Pengutronix e.K.                           |                             |
>>>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>>>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>>>
>>>>
>>>
>>
>> -- 
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
>>
>>
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [DistroKit] [PATCH] v8a: barebox: add missing dependency on TF_A for i.MX

[Michael Olbrich]

On Thu, Jun 01, 2023 at 11:31:43AM +0200, Ahmad Fatoum wrote:
> On 01.06.23 11:15, Michael Olbrich wrote:
> > On Thu, Jun 01, 2023 at 09:11:40AM +0200, Ahmad Fatoum wrote:
> >> On 01.06.23 09:05, Michael Olbrich wrote:
> >>> On Thu, Jun 01, 2023 at 08:49:38AM +0200, Ahmad Fatoum wrote:
> >>>> On 01.06.23 08:48, Ahmad Fatoum wrote:
> >>>>> Unlike firmware-rockchip, firmware-imx doesn't ship BL31 (TF-A). Instead
> >>>>> we configure the upstream TF-A rule to build images for i.MX8M Quad,
> >>>>> Mini and Plus.
> >>>>>
> >>>>> With the switch to the upstream barebox rule, this dependency on TF-A
> >>>>> got lost breaking less-parallel fresh builds, so let's add it back.
> >>>>>
> >>>>> Preferably, TF_A would be selected directly from the barebox rule's
> >>>>> Kconfig. There's a BAREBOX_DEPENDENCIES option for that, but I don't
> >>>>> know if it's possible to extend barebox.in without having to duplicate
> >>>>> it fully, so adding it to firmware-imx should suffice for now.
> >>>>
> >>>> Is there something like post/barebox-fixup.make, but for Kconfig files
> >>>> that I should be using instead?
> >>>
> >>> This looks fine. We need the special stuff in make, because you cannot
> >>> extend a make target in another makefile. It's easier with Kconfig:
> >>> All occurrences of a symbol are merged so you just need to make sure that
> >>> the file with you're new dependency is actually loaded.
> >>
> >> It still seems out of place, because TF-A isn't a dependency of firmware-imx,
> >> but of barebox, so it should be selected from a file that's related to barebox,
> >> even if the end result is the same and both ends up as a select from
> >> BAREBOX_DEPENDENCIES. There seems to be no such way though, right?
> > 
> > Then create configs/platform-v8a/platforms/firmware-tfa.in or whatever you want
> > to call it, with
> > 
> > ## SECTION=bootloader
> > 
> > config BAREBOX_DEPENDENCIES
> > 	select TF_A
> > 
> > As I said, just make sure it's included somewhere and Kconfig will do the
> > rest.
> 
> That's the things I am asking about. Where would I include that from, if
> both tf-a and barebox are upstream rules, whose .in is not in DistroKit.

You don't need to include it explicitly anywhere. By adding the
"## SECTION=bootloader" header it will be included automatically.

Michael

> @rsc, you can pick this patch and I will send a follow-up if I find a better way.
> 
> >>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >>>>> ---
> >>>>>  configs/platform-v8a/rules/firmware-imx.in | 1 +
> >>>>>  1 file changed, 1 insertion(+)
> >>>>>
> >>>>> diff --git a/configs/platform-v8a/rules/firmware-imx.in b/configs/platform-v8a/rules/firmware-imx.in
> >>>>> index 5f06a44416fa..c756b963e5dc 100644
> >>>>> --- a/configs/platform-v8a/rules/firmware-imx.in
> >>>>> +++ b/configs/platform-v8a/rules/firmware-imx.in
> > 
> > Patch opportunities:
> 
> Thanks. I will keep that in mind next time I touch this.
> 
> > 
> > Menu files that are only used for the platform belong in
> > configs/platform-v8a/platforms/
> > 
> > And there is a non_free_firmware section which should be a better fit for
> > firmware-imx.
> > 
> >>>>> @@ -10,6 +10,7 @@ menuconfig FIRMWARE_IMX
> >>>>>  config BAREBOX_DEPENDENCIES
> >>>>>  	tristate
> > 
> > You don't need the 'tristate' here.
> > 
> > Michael
> > 
> >>>>>  	select FIRMWARE_IMX
> >>>>> +	select TF_A
> >>>>>  
> >>>>>  if FIRMWARE_IMX
> >>>>>  
> >>>>
> >>>> -- 
> >>>> Pengutronix e.K.                           |                             |
> >>>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> >>>> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >>>> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >>>>
> >>>>
> >>>
> >>
> >> -- 
> >> Pengutronix e.K.                           |                             |
> >> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> >> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> >> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> >>
> >>
> >>
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |