[PATCH RFC 1/3] tools: binman: Enable getting file from specific directory

Neha Malcom Francis n-francis at ti.com
Fri Jul 28 14:40:03 CEST 2023


Hi Simon

On 28/07/23 08:05, Simon Glass wrote:
> Hi Neha,
> 
> On Thu, 27 Jul 2023 at 06:12, Neha Malcom Francis <n-francis at ti.com> wrote:
>>
>> While we have the option of using '-a <file-dir> to grab a file from a
>> specific directory, this is problematic when searching for a filename
>> shared by multiple files across directories. An example would be FDT
>> generated in SPL_BUILD vs. non-SPL_BUILD that is present in spl/dts and
>> arch/arm/dts respectively with the same DTB name. Maybe running binman
> 
> Do you mean spl/arch/arm/dts and arch/arch/dts ?
> 

No I mean spl/dts, that's what is generated.

>> twice could also solve this issue, but this seems like a valid
>> implementation.
>>
>> Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
>> ---
>>   tools/binman/etype/blob.py  | 12 ++++++++++--
>>   tools/binman/etype/fit.py   | 15 ++++++++++++++-
>>   tools/u_boot_pylib/tools.py |  7 ++++++-
>>   3 files changed, 30 insertions(+), 4 deletions(-)
>>
> 
> I don't fully understand this, but perhaps it would make sense to give
> the indirs names, like:
> 
> binman -I spl:path/to/spl  -I name2:path/to/somewhere-else
> 

Hm yes that could be something, I'll try working on that.

> then you could use the names below, instead of the paths. It provide
> at least a little separation.
> 
> Perhaps if you listed out the two dirs and the files they contain,
> that would help me understand better.
> 

Yes sure:

arch/arm/dts:
k3-j7200-common-proc-board.dtb     k3-j721e-r5-common-proc-board.dtb
k3-j7200-r5-common-proc-board.dtb  k3-j721e-r5-sk.dtb
k3-j721e-common-proc-board.dtb     k3-j721e-sk.dtb

spl/dts:
built-in.o  k3-j721e-common-proc-board.dtb  k3-j721e-sk.dtb

>> diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py
>> index 064fae5036..eccf6f87ac 100644
>> --- a/tools/binman/etype/blob.py
>> +++ b/tools/binman/etype/blob.py
>> @@ -19,6 +19,8 @@ class Entry_blob(Entry):
>>
>>       Properties / Entry arguments:
>>           - filename: Filename of file to read into entry
>> +        - indir-path: Specific directory to fetch file from, if not provided
>> +        default indir paths will be searched
>>           - compress: Compression algorithm to use:
>>               none: No compression
>>               lz4: Use lz4 compression (via 'lz4' command-line utility)
>> @@ -35,6 +37,7 @@ class Entry_blob(Entry):
>>           super().__init__(section, etype, node,
>>                            auto_write_symbols=auto_write_symbols)
>>           self._filename = fdt_util.GetString(self._node, 'filename', self.etype)
>> +        self._indir_path = fdt_util.GetString(self._node, 'indir-path', None)
>>           self.elf_fname = fdt_util.GetString(self._node, 'elf-filename',
>>                                               self.elf_fname)
>>           self.elf_base_sym = fdt_util.GetString(self._node, 'elf-base-sym')
>> @@ -44,8 +47,13 @@ class Entry_blob(Entry):
>>
>>       def ObtainContents(self, fake_size=0):
>>           self._filename = self.GetDefaultFilename()
>> -        self._pathname = tools.get_input_filename(self._filename,
>> -            self.external and (self.optional or self.section.GetAllowMissing()))
>> +        if self._indir_path:
>> +            self._pathname = tools.get_input_filename(self._filename,
>> +                self.external and (self.optional or self.section.GetAllowMissing()),
>> +                self._indir_path)
> 
> I think this might as well just read the file, rather than calling
> tools.get_input_filename()
> 
>> +        else:
>> +            self._pathname = tools.get_input_filename(self._filename,
>> +                self.external and (self.optional or self.section.GetAllowMissing()))
>>           # Allow the file to be missing
>>           if not self._pathname:
>>               self._pathname, faked = self.check_fake_fname(self._filename,
>> diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
>> index ef4d066757..46db816370 100644
>> --- a/tools/binman/etype/fit.py
>> +++ b/tools/binman/etype/fit.py
>> @@ -87,6 +87,15 @@ class Entry_fit(Entry_section):
>>
>>                   fit,fdt-list-val = "dtb1", "dtb2";
>>
>> +        fit,fdt-indir
>> +            Indicates the specific directory (if any) to be used to pick up fdts
>> +            generated in the gen-fdt-nodes property. For example, picking fdt from
>> +            spl/dts instead of arch/arm/dts. This is equivalent to '-a spl/dts' but
>> +            helps when multiple directories contain same named fdt to choose a
>> +            specific one
>> +
>> +                fit,fdt-indir = "spl/dts"
> 
> Should this be a named indir? Otherwise you have to use the actual
> dirs and that could be come quite brittle when things change.
> 

Yes let me try working with a named indir

>> +
>>       Substitutions
>>       ~~~~~~~~~~~~~
>>
>> @@ -362,6 +371,7 @@ class Entry_fit(Entry_section):
>>               if pname.startswith('fit,'):
>>                   self._fit_props[pname] = prop
>>           self._fit_list_prop = self._fit_props.get('fit,fdt-list')
>> +        self._fit_indir = fdt_util.GetString(self._node, 'fit,fdt-indir', None)
>>           if self._fit_list_prop:
>>               fdts, = self.GetEntryArgsOrProps(
>>                   [EntryArg(self._fit_list_prop.value, str)])
>> @@ -583,7 +593,10 @@ class Entry_fit(Entry_section):
>>                   # Generate nodes for each FDT
>>                   for seq, fdt_fname in enumerate(self._fdts):
>>                       node_name = node.name[1:].replace('SEQ', str(seq + 1))
>> -                    fname = tools.get_input_filename(fdt_fname + '.dtb')
>> +                    if self._fit_indir:
>> +                        fname = tools.get_input_filename(fdt_fname + '.dtb', specific_indir=self._fit_indir)
>> +                    else:
>> +                        fname = tools.get_input_filename(fdt_fname + '.dtb')
>>                       with fsw.add_node(node_name):
>>                           for pname, prop in node.props.items():
>>                               if pname == 'fit,firmware':
>> diff --git a/tools/u_boot_pylib/tools.py b/tools/u_boot_pylib/tools.py
>> index 187725b501..1b6f6b71d8 100644
>> --- a/tools/u_boot_pylib/tools.py
>> +++ b/tools/u_boot_pylib/tools.py
>> @@ -123,12 +123,13 @@ def set_input_dirs(dirname):
>>       indir = dirname
>>       tout.debug("Using input directories %s" % indir)
>>
>> -def get_input_filename(fname, allow_missing=False):
>> +def get_input_filename(fname, allow_missing=False, specific_indir=None):
>>       """Return a filename for use as input.
>>
>>       Args:
>>           fname: Filename to use for new file
>>           allow_missing: True if the filename can be missing
>> +        specific_indir: Directory to use to get file
>>
>>       Returns:
>>           fname, if indir is None;
>> @@ -140,6 +141,10 @@ def get_input_filename(fname, allow_missing=False):
>>       """
>>       if not indir or fname[:1] == '/':
>>           return fname
>> +    if specific_indir:
>> +        pathname = os.path.join(specific_indir, fname)
>> +        if os.path.exists(pathname):
>> +            return pathname
>>       for dirname in indir:
>>           pathname = os.path.join(dirname, fname)
>>           if os.path.exists(pathname):
>> --
>> 2.34.1
>>
> 
> Regards,
> Simon

-- 
Thanking You
Neha Malcom Francis


More information about the U-Boot mailing list