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

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

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 |