[PATCH 1/3] binman: Allow to define custom arguments

[Jan Kiszka]

On 20.06.23 12:11, Simon Glass wrote:
> Hi Jan,
> 
> On Mon, 19 Jun 2023 at 16:09, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>>
>> On 19.06.23 16:09, Simon Glass wrote:
>>> Hi Jan,
>>>
>>> On Mon, 19 Jun 2023 at 14:28, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>>>>
>>>> On 19.06.23 14:36, Simon Glass wrote:
>>>>> Hi Jan,
>>>>>
>>>>> On Fri, 16 Jun 2023 at 16:42, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>>>>>>
>>>>>> On 15.06.23 13:46, Jan Kiszka wrote:
>>>>>>> On 15.06.23 13:38, Simon Glass wrote:
>>>>>>>> Hi Jan,
>>>>>>>>
>>>>>>>> On Thu, 15 Jun 2023 at 12:21, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>>>>>>>>>
>>>>>>>>> On 15.06.23 13:19, Simon Glass wrote:
>>>>>>>>>> Hi Jan,
>>>>>>>>>>
>>>>>>>>>> On Thu, 15 Jun 2023 at 12:09, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> On 15.06.23 12:55, Simon Glass wrote:
>>>>>>>>>>>> Hi Jan,
>>>>>>>>>>>>
>>>>>>>>>>>> On Thu, 15 Jun 2023 at 11:26, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 12.06.23 23:17, Simon Glass wrote:
>>>>>>>>>>>>>> Hi Jan,
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Mon, 5 Jun 2023 at 15:39, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka at siemens.com>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Introduce BINMAN_EXTRA_ARGS that can be set per board, e.g., to inject
>>>>>>>>>>>>>>> specific settings. Will be used by IOT2050 first to define multiple
>>>>>>>>>>>>>>> of-lists.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Signed-off-by: Jan Kiszka <jan.kiszka at siemens.com>
>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>> CC: Simon Glass <sjg at chromium.org>
>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>  Makefile | 1 +
>>>>>>>>>>>>>>>  1 file changed, 1 insertion(+)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> I'm really not keen on this, since it means that the Makefile (or some
>>>>>>>>>>>>>> vars it sets) are again involved in controlling the image generation.
>>>>>>>>>>>>>> It should really all be in the binman image description / .dtsi file
>>>>>>>>>>>>>
>>>>>>>>>>>>> binman does not allow me to unrole of-list inside the dts file, does it?
>>>>>>>>>>>>> With such a feature, I wouldn't need any custom -a of-list-X switches
>>>>>>>>>>>>> and, thus, no such EXTRA_ARGS.
>>>>>>>>>>>>
>>>>>>>>>>>> Can you explain a bit more about what you mean by 'unrole'? It is just
>>>>>>>>>>>> software, so anything should be possible.
>>>>>>>>>>>
>>>>>>>>>>> To use a DT sequence, I need to specify fit,ftd-list. But I can say
>>>>>>>>>>>
>>>>>>>>>>> fit,ftd-list = "first.dtb second.dtb"
>>>>>>>>>>>
>>>>>>>>>>> I rather need to go via the EntryArg indirection of the binman command line.
>>>>>>>>>>
>>>>>>>>>> Do you mean you would rather not use '-a CONFIG_OF_LIST'. Or are you
>>>>>>>>>> wanting to filter that list based on something else?
>>>>>>>>>>
>>>>>>>>>> I'm afraid I am really not following this at all.
>>>>>>>>>
>>>>>>>>> CONFIG_OF_LIST is not working here as we have two different boards with
>>>>>>>>> two different lists.
>>>>>>>>
>>>>>>>> But we only build one board at a time, don't we?
>>>>>>>
>>>>>>> No, this is about building two flash images for two different board
>>>>>>> generations in the same binman run (see patch 3).
>>>>>>>
>>>>>>>>
>>>>>>>> We could provide a way to select between different lists, but how does
>>>>>>>> Makefile get access to them?
>>>>>>>
>>>>>>> See patch 3: known lists, now put into board config.mk.
>>>>>>>
>>>>>>
>>>>>> Any better suggestions for this issue? Otherwise, I will have to keep
>>>>>> binman args extension for now.
>>>>>
>>>>> I've dug into this a bit. The use of #defines and macros looks to be
>>>>> unnecessary, unless I am missing something.
>>>>>
>>>>> You can do things like this:
>>>>>
>>>>> fit_node: fit {
>>>>>     // base definition of FIT
>>>>>     };
>>>>>
>>>>> the later on:
>>>>>
>>>>> fit_node: {
>>>>> #ifdef xxx
>>>>>    // override a few things
>>>>>    fit,fdt-list = ...
>>>>> #else
>>>>>
>>>>> #endif
>>>>
>>>> As I wrote already, I have a solution for that by using a template dtsi.
>>>>
>>>>>
>>>>> There is no need to specify the properties all at once. You can update
>>>>> a property later on just by referring to its node as above.
>>>>
>>>> Does not help when the anchor name needs to vary as well. That's where I
>>>> will use a #define to customize the template on include.
>>>>
>>>>>
>>>>> I think with this you should be above to eliminate BINMAN_EXTRA_ARGS
>>>>> and all the #define stuff.
>>>>
>>>> You still didn't address my question. Should I share my version to make
>>>> the problem clearer? But I thought I explained this in various shades
>>>> already.
>>>
>>> Yes, if I am looking at the wrong patches, please point me to the
>>> correct series, or push a tree somewhere.
>>>
>>
>> Please have a look at
>> https://github.com/siemens/u-boot/commit/393ce2b78cee9214edda7f7e04f6ca2ce144195e
> 
> OK that looks a lot better to me. We can go with that until we come up
> with something better.
> 
> There has been a long-standing request to support common parts of images.
> 
> I am thinking something like this:
> 
> common_part: common-part {
>     template;   // indicates this is not a real image
>     entry1 {
>         ...
>     };
>     entry2 {
>         ...
>    };
> };
> 
> image1 {
>    add-entries = <&common_part>;
>    entry1 {
>       fit,fdt-list = "something";
>    };
> };
> 
> image2 {
>    add-entries = <&common_part>;
> 
>    // override a few things from entry1
>    entry1 {
>       fit,fdt-list = "something else";
>    };
> };
> 
> This would allow a common part to be included but still be adjusted
> depending on what each image needs. What do you think?

Ok, that saves us from the less ugly but still ugly template inclusion -
starting to understand. Will play with that.

But it still does not resolve the need to customize the bitman command
line for multiple of-lists.

Jan

-- 
Siemens AG, Technology
Competence Center Embedded Linux