Re: [ptxdist] [PATCH v2] tf-a: new package for ARM trusted firmware A

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

On 2/13/20 3:35 PM, Guillermo Rodriguez Garcia wrote:
> El jue., 13 feb. 2020 a las 15:32, Ahmad Fatoum
> (<a.fatoum@pengutronix.de>) escribió:
>>
>> On 2/13/20 3:28 PM, Ahmad Fatoum wrote:
>>> On 2/13/20 3:25 PM, Guillermo Rodriguez Garcia wrote:
>>>> El jue., 13 feb. 2020 a las 14:30, Ahmad Fatoum
>>>> (<a.fatoum@pengutronix.de>) escribió:
>>>
>>>>>>> +# ----------------------------------------------------------------------------
>>>>>>> +# Install
>>>>>>> +# ----------------------------------------------------------------------------
>>>>>>> +
>>>>>>> +$(STATEDIR)/tf-a.install:
>>>>>>> +       @$(call targetinfo)
>>>>>>> +ifeq ($(TF_A_ARTIFACTS_SRC),)
>>>>>>> +       $(warning TF_A_ARTIFACTS is empty. nothing to install.)
>>>>>>> +else
>>>>>>> +       @install -m644 -D \
>>>>>>> +               --target-directory=$(PTXCONF_SYSROOT_TARGET)/usr/lib/firmware \
>>>>>>
>>>>>> This doesn't look right.
>>>>>> Shouldn't the install stage install things to the package install
>>>>>> directory only?
>>>>>> And, in case you want to install something somewhere else, shouldn't
>>>>>> the actual target directory at least be configurable?
>>>>>> For example there is no /usr/lib/firmware in my platform.
>>>>>
>>>>> That's only in the sysroot for use by other rules (e.g. barebox embedding TF-A
>>>>> on some i.MX8). It's not installed in the target rootfs.
>>>>
>>>> Ok but that's specific to that particular configuration, and the
>>>> choice of directory also seems to be arbitrary.
>>>
>>> You got to standardize on something. If this is good, a $(PTXCONF_SYSROOT_FIRMWARE)
>>> might be a good thing to agree on.
>>>
>>>> Does this belong in a generic rules file ?
>>>
>>> How would that look like?
>>
>> Ah, you mean this is too specific that it shouldn't be in tf-a.make?
> 
> Yes.
> 
>>
>> Well, Some Zynqs do this as well. If you are embedding OPTEE in TF-A, you will need
>> to save optee somewhere too. It's not a niche use case. The split between install
> 
> For cases like this (optee) you're going to need to amend tf-a.in
> anyway because you need to add dependencies (you mentioned this
> yourself in a previous email)
> 
>> and target-install is just because so many vendors have their own idea of where the TF-A
>> comes from. If most of these can be made to work by installing the file to sysroot along
>> with the IMAGEDIR, so why not do so?
> 
> If it really covers "most of these" use cases, then I guess it may make sense.

if it's consumed by image rules, it's in IMAGEDIR. If it's consumed by "normal" rules,
it's in the sysroot at a fixed place. Not sure if there is another use case I am missing.

> 
> Still -- shouldn't it at least be optional?

Why should the .install stage be behind an option, but the .targetinstall not?
Should there be options for enabling/disabling each one of them?

And why? Just to save a few hundred kilobytes extra in the _build_ directory?
Doesn't sound like a good enough reason to me.

Cheers
Ahmad

> 
> Guillermo
> 
>>
>> Cheers
>> Ahmad
>>
>>>
>>>
>>> Cheers
>>> Ahmad
>>>
>>
>>
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | https://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                       | https://www.pengutronix.de/ |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de