Re: [PATCH 20/24] binman: Support splitting an ELF file into multiple nodes

[Alper Nebi Yasak <alpernebiyasak@gmail.com>]

On 24/02/2022 01:59, Simon Glass wrote:
> On Tue, 15 Feb 2022 at 04:53, Alper Nebi Yasak <alpernebiyasak@gmail.com> wrote:
>> On 08/02/2022 21:50, Simon Glass wrote:
>>> +    fit,load
>>> +        Generates a `load = <...>` property with the load address of the
>>> +        segmnet
>>> +
>>> +    fit,entry
>>> +        Generates a `entry = <...>` property with the entry address of the
>>> +        ELF. This is only produced for the first entry
>>> +
>>> +    fit,data
>>> +        Generates a `data = <...>` property with the contents of the segment
>>
>> I think all these should be done by default. I don't see the point of
>> not setting the properties, or setting them manually to values that will
>> be the same for multiple nodes.
> 
> My intent is to make things discoverable and obvious, so that magic
> processing is explicit.

OK then.

>> Instead of putting these into the images subnode, we could have
>> images-level subnodes for the operations. For example, instead of the above:
>>
>>            images@gen-fdt-nodes {
>>                fdt-list = "of-list";
>>
>>                fdt {
>>                    type = "flat_dt";
>>                    compression = "none";
>>                };
>>            };
> 
> What does that mean, though? I presume it creates a second images {}
> node, which is fine as dtc will merge them. But what about ordering?

I imagined images@oper, configurations@oper would be binman-only control
nodes that generate and append arbitrary nodes/entries inside whatever
node name precedes the @oper. The meaning of what's inside the @oper
nodes and what's generated would be solely defined by the operation.

Honestly, I didn't think of ordering because I assumed it didn't matter
inside FIT. It's a shortcoming of this design if it's important.

> I certainly prefer this in terms of elegance. I'm just not convinced
> that people will understand it as well.
> 
>>
>> We can remove the -SEQ if we always append a sequence number, and we can
>> set "description" to "NAME.dtb" when it's missing, or do the replacement
>> when it's given. We can go further and use "%s", "%(name)s" or "{name}"
>> instead of "NAME" for python-ish formatting and likewise for the
>> sequence number.
> 
> Yes, but again this adds more magic. For the Python formatting, we
> still need to restrict what is put in there - e.g. we cannot just eval
> an arbitrary varaible.

Python only formats with what you give it (except for f-strings):

    >>> val = "test"
    >>> vals = {"name": "rk3399-gru-kevin", "seq": 1}

    >>> "conf-%(seq)d: %(name)s.dtb" % vals
    'conf-1: rk3399-gru-kevin.dtb'

    >>> "%(val)s" % vals
    KeyError: 'val'

    >>> "conf-{seq}: {name}.dtb".format(**vals)
    'conf-1: rk3399-gru-kevin.dtb'

    >>> "{val}".format(**vals)
    KeyError: 'val'

But neither format style can go in a node name, so if you want the
sequence number explicit in those I guess we're stuck with -SEQ and
everything consistent with it.

>>> [...]
> 
> We should talk about this some more though. I'm a bit worried it will
> get complaints about too much magic. I am trying to make it obvious
> that nodes get generated in certain places.

OK. I know I'm too biased towards magic, so won't insist much on those
parts.

> [...]
> 
> Thanks for all the comments and ideas. I think this all need a little
> more thought...

I'll try replying to the v2 for the things changed in v2.