[PATCH 1/3] binman: Allow to define custom arguments
Simon Glass
sjg at chromium.org
Tue Jun 20 19:12:57 CEST 2023
Hi Jan,
On Tue, 20 Jun 2023 at 18:05, Jan Kiszka <jan.kiszka at 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 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.
>
> Yep, that is what would help as said before.
OK I will take a look.
Regards,
Simon
More information about the U-Boot
mailing list