Re: [PATCH V4 5/8] tools: binman: section: replace @ with -

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

On 20/05/2022 17:10, Peng Fan (OSS) wrote:
> From: Peng Fan <peng.fan@nxp.com>
> 
> In arch/arm/dts/imx8mp-u-boot.dtsi, there are blob-ext@1, blob-ext@2 and
> etc which is for packing ddr phy firmware. However we could not declare
> symbol name such as 'binman_sym_declare(ulong, blob_ext@1, image_pos)',
> because '@' is not allowed, so we choose to declare the symbol
> 'binman_sym_declare(ulong, blob_ext_1, image_pos);' with '@' replaced with
> '_'. It does not impact if there is no '@' in section name.
> 
> Tested-by: Tim Harvey <tharvey@gateworks.com> #imx8m[m,n,p]-venice
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Signed-off-by: Peng Fan <peng.fan@nxp.com>
> ---
>  tools/binman/etype/section.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This shouldn't be necessary if you keep the old names for the binman
entries and use `binman_sym_declare(ulong, imem_1d, image_pos);` etc.
for the symbols.

> diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
> index bd67238b919..e3f362b442b 100644
> --- a/tools/binman/etype/section.py
> +++ b/tools/binman/etype/section.py
> @@ -875,7 +875,7 @@ class Entry_section(Entry):
>                  entries[entry.GetPath()] = entry
>              for entry in to_add.values():
>                  self._CollectEntries(entries, entries_by_name, entry)
> -        entries_by_name[add_entry.name] = add_entry
> +        entries_by_name[add_entry.name.replace('@', '-')] = add_entry

The correct place to do this would be LookupSymbol() in
binman/etype/section.py, but I'm not convinced this should be done at
all. I'd say if an entry is important enough to have a symbol for it, it
should have a unique, descriptive, non-@ name.

>  
>      def MissingArgs(self, entry, missing):
>          """Report a missing argument, if enabled