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

[Simon Glass]

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?

Regards,
Simon