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

Simon Glass sjg at chromium.org
Tue Jun 20 16:36:50 CEST 2023


Hi Jan,

On Tue, 20 Jun 2023 at 11:37, Jan Kiszka <jan.kiszka at 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 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.

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


More information about the U-Boot mailing list