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

[Simon Glass]

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.

Regards,
Simon