diff mbox series

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

Message ID 9e905abd369f1d8b182e6fc6e3c9ef4b1526bf76.1685975993.git.jan.kiszka@siemens.com
State Changes Requested
Delegated to: Simon Glass
Headers show
Series iot2050: Re-unify its configs and build processes | expand

Commit Message

Jan Kiszka June 5, 2023, 2:39 p.m. UTC
From: Jan Kiszka <jan.kiszka@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@siemens.com>
---
CC: Simon Glass <sjg@chromium.org>
---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

Comments

Simon Glass June 12, 2023, 9:17 p.m. UTC | #1
Hi Jan,

On Mon, 5 Jun 2023 at 15:39, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> From: Jan Kiszka <jan.kiszka@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@siemens.com>
> ---
> CC: Simon Glass <sjg@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

Regards,
Simon
Jan Kiszka June 15, 2023, 10:26 a.m. UTC | #2
On 12.06.23 23:17, Simon Glass wrote:
> Hi Jan,
> 
> On Mon, 5 Jun 2023 at 15:39, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
>> ---
>> CC: Simon Glass <sjg@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.

Jan
Simon Glass June 15, 2023, 10:55 a.m. UTC | #3
Hi Jan,

On Thu, 15 Jun 2023 at 11:26, Jan Kiszka <jan.kiszka@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@siemens.com> wrote:
> >>
> >> From: Jan Kiszka <jan.kiszka@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@siemens.com>
> >> ---
> >> CC: Simon Glass <sjg@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.

Regards,
Simon

>
> Jan
>
> --
> Siemens AG, Technology
> Competence Center Embedded Linux
>
Jan Kiszka June 15, 2023, 11:08 a.m. UTC | #4
On 15.06.23 12:55, Simon Glass wrote:
> Hi Jan,
> 
> On Thu, 15 Jun 2023 at 11:26, Jan Kiszka <jan.kiszka@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@siemens.com> wrote:
>>>>
>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
>>>> ---
>>>> CC: Simon Glass <sjg@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.

Jan
Simon Glass June 15, 2023, 11:19 a.m. UTC | #5
Hi Jan,

On Thu, 15 Jun 2023 at 12:09, Jan Kiszka <jan.kiszka@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@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@siemens.com> wrote:
> >>>>
> >>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
> >>>> ---
> >>>> CC: Simon Glass <sjg@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.

Regards,
Simon
Jan Kiszka June 15, 2023, 11:20 a.m. UTC | #6
On 15.06.23 13:19, Simon Glass wrote:
> Hi Jan,
> 
> On Thu, 15 Jun 2023 at 12:09, Jan Kiszka <jan.kiszka@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@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@siemens.com> wrote:
>>>>>>
>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
>>>>>> ---
>>>>>> CC: Simon Glass <sjg@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.

Jan
Simon Glass June 15, 2023, 11:38 a.m. UTC | #7
Hi Jan,

On Thu, 15 Jun 2023 at 12:21, Jan Kiszka <jan.kiszka@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@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@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@siemens.com> wrote:
> >>>>>>
> >>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
> >>>>>> ---
> >>>>>> CC: Simon Glass <sjg@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?

We could provide a way to select between different lists, but how does
Makefile get access to them?

Regards,
Simon
Jan Kiszka June 15, 2023, 11:46 a.m. UTC | #8
On 15.06.23 13:38, Simon Glass wrote:
> Hi Jan,
> 
> On Thu, 15 Jun 2023 at 12:21, Jan Kiszka <jan.kiszka@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@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@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@siemens.com> wrote:
>>>>>>>>
>>>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
>>>>>>>> ---
>>>>>>>> CC: Simon Glass <sjg@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.

Jan
Jan Kiszka June 16, 2023, 3:42 p.m. UTC | #9
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@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@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@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@siemens.com> wrote:
>>>>>>>>>
>>>>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
>>>>>>>>> ---
>>>>>>>>> CC: Simon Glass <sjg@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.

Jan
Simon Glass June 19, 2023, 12:36 p.m. UTC | #10
Hi Jan,

On Fri, 16 Jun 2023 at 16:42, Jan Kiszka <jan.kiszka@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@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@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@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@siemens.com> wrote:
> >>>>>>>>>
> >>>>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
> >>>>>>>>> ---
> >>>>>>>>> CC: Simon Glass <sjg@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

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.

I think with this you should be above to eliminate BINMAN_EXTRA_ARGS
and all the #define stuff.

Please take a look and let me know if it doesn't make sense.

Regards,
Simon
Jan Kiszka June 19, 2023, 1:28 p.m. UTC | #11
On 19.06.23 14:36, Simon Glass wrote:
> Hi Jan,
> 
> On Fri, 16 Jun 2023 at 16:42, Jan Kiszka <jan.kiszka@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@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@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@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@siemens.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
>>>>>>>>>>> ---
>>>>>>>>>>> CC: Simon Glass <sjg@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.

Jan
Simon Glass June 19, 2023, 2:09 p.m. UTC | #12
Hi Jan,

On Mon, 19 Jun 2023 at 14:28, Jan Kiszka <jan.kiszka@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@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@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@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@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@siemens.com> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
> >>>>>>>>>>> ---
> >>>>>>>>>>> CC: Simon Glass <sjg@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.

Regards,
Simon
Jan Kiszka June 19, 2023, 3:09 p.m. UTC | #13
On 19.06.23 16:09, Simon Glass wrote:
> Hi Jan,
> 
> On Mon, 19 Jun 2023 at 14:28, Jan Kiszka <jan.kiszka@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@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@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@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@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@siemens.com> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
>>>>>>>>>>>>> ---
>>>>>>>>>>>>> CC: Simon Glass <sjg@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

Thanks,
Jan
Simon Glass June 20, 2023, 10:11 a.m. UTC | #14
Hi Jan,

On Mon, 19 Jun 2023 at 16:09, Jan Kiszka <jan.kiszka@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@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@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@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@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@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@siemens.com> wrote:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
> >>>>>>>>>>>>> ---
> >>>>>>>>>>>>> CC: Simon Glass <sjg@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?

Regards,
Simon
Jan Kiszka June 20, 2023, 10:37 a.m. UTC | #15
On 20.06.23 12:11, Simon Glass wrote:
> Hi Jan,
> 
> On Mon, 19 Jun 2023 at 16:09, Jan Kiszka <jan.kiszka@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@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@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@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@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@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@siemens.com> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>> CC: Simon Glass <sjg@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
Simon Glass June 20, 2023, 2:36 p.m. UTC | #16
Hi Jan,

On Tue, 20 Jun 2023 at 11:37, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 20.06.23 12:11, Simon Glass wrote:
> > Hi Jan,
> >
> > On Mon, 19 Jun 2023 at 16:09, Jan Kiszka <jan.kiszka@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@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@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@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@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@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@siemens.com> wrote:
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
> >>>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>>> CC: Simon Glass <sjg@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.

Note that the above is not implemented yet! It is just an idea, so let
me know what you think.

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

At the moment you have:

fit,fdt-list = IOT2050_FLASH_DT_LIST;

with that defined as:

#define IOT2050_FLASH_DT_LIST "of-list-pg2"

and then you do:

-a of-list-pg1="k3-am6528-iot2050-basic k3-am6548-iot2050-advanced" \

I wonder if we should have something like:

fit,fdt-list-val = "k3-am6528-iot2050-basic", "k3-am6548-iot2050-advanced";

and avoid the entry-arg redirection? Then you can add the property in
as I showed above.

Regards,
Simon
Jan Kiszka June 20, 2023, 5:05 p.m. UTC | #17
On 20.06.23 16:36, Simon Glass wrote:
> Hi Jan,
> 
> On Tue, 20 Jun 2023 at 11:37, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>>
>> On 20.06.23 12:11, Simon Glass wrote:
>>> Hi Jan,
>>>
>>> On Mon, 19 Jun 2023 at 16:09, Jan Kiszka <jan.kiszka@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@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@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@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@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@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@siemens.com> wrote:
>>>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
>>>>>>>>>>>>>>>>> ---
>>>>>>>>>>>>>>>>> CC: Simon Glass <sjg@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.
> 
> Note that the above is not implemented yet! It is just an idea, so let
> me know what you think.
> 
>>
>> But it still does not resolve the need to customize the bitman command
>> line for multiple of-lists.
> 
> At the moment you have:
> 
> fit,fdt-list = IOT2050_FLASH_DT_LIST;
> 
> with that defined as:
> 
> #define IOT2050_FLASH_DT_LIST "of-list-pg2"
> 
> and then you do:
> 
> -a of-list-pg1="k3-am6528-iot2050-basic k3-am6548-iot2050-advanced" \
> 
> I wonder if we should have something like:
> 
> fit,fdt-list-val = "k3-am6528-iot2050-basic", "k3-am6548-iot2050-advanced";
> 
> and avoid the entry-arg redirection? Then you can add the property in
> as I showed above.

Yep, that is what would help as said before.

Jan
Simon Glass June 20, 2023, 5:12 p.m. UTC | #18
Hi Jan,

On Tue, 20 Jun 2023 at 18:05, Jan Kiszka <jan.kiszka@siemens.com> wrote:
>
> On 20.06.23 16:36, Simon Glass wrote:
> > Hi Jan,
> >
> > On Tue, 20 Jun 2023 at 11:37, Jan Kiszka <jan.kiszka@siemens.com> wrote:
> >>
> >> On 20.06.23 12:11, Simon Glass wrote:
> >>> Hi Jan,
> >>>
> >>> On Mon, 19 Jun 2023 at 16:09, Jan Kiszka <jan.kiszka@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@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@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@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@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@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@siemens.com> wrote:
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> From: Jan Kiszka <jan.kiszka@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@siemens.com>
> >>>>>>>>>>>>>>>>> ---
> >>>>>>>>>>>>>>>>> CC: Simon Glass <sjg@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.
> >
> > Note that the above is not implemented yet! It is just an idea, so let
> > me know what you think.
> >
> >>
> >> But it still does not resolve the need to customize the bitman command
> >> line for multiple of-lists.
> >
> > At the moment you have:
> >
> > fit,fdt-list = IOT2050_FLASH_DT_LIST;
> >
> > with that defined as:
> >
> > #define IOT2050_FLASH_DT_LIST "of-list-pg2"
> >
> > and then you do:
> >
> > -a of-list-pg1="k3-am6528-iot2050-basic k3-am6548-iot2050-advanced" \
> >
> > I wonder if we should have something like:
> >
> > fit,fdt-list-val = "k3-am6528-iot2050-basic", "k3-am6548-iot2050-advanced";
> >
> > and avoid the entry-arg redirection? Then you can add the property in
> > as I showed above.
>
> Yep, that is what would help as said before.

OK I will take a look.

Regards,
Simon
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 10bfaa52adf..2285ae26b9b 100644
--- a/Makefile
+++ b/Makefile
@@ -1345,6 +1345,7 @@  cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
 		-a spl-dtb=$(CONFIG_SPL_OF_REAL) \
 		-a tpl-dtb=$(CONFIG_TPL_OF_REAL) \
 		-a pre-load-key-path=${PRE_LOAD_KEY_PATH} \
+		$(BINMAN_EXTRA_ARGS) \
 		$(BINMAN_$(@F))
 
 OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex