Re: [PATCH 7/8] binman_sym: guard with CONFIG_IS_ENABLED(BINMAN_SYMBOLS)

Peng Fan (OSS)
2022-06-13T02:31:29Z

在 2022/6/11 18:32, Peng Fan (OSS) 写道:
>
> 在 2022/6/11 0:47, Alper Nebi Yasak 写道:
>> On 04/06/2022 14:50, Alper Nebi Yasak wrote:
>>> On 03/06/2022 10:17, Peng Fan (OSS) wrote:
>>>> From: Peng Fan <peng.fan@nxp.com>
>>>>
>>>> There is case that CONFIG_BINMAN is defined, but
>>>> CONFIG_SPL_BINMAN_SYMBOLS is not defined. In that case, there will be
>>>> build failure. So use CONFIG_SPL_BINMAN_SYMBOLS to guard the macros, and
>>>> define CONFIG_SPL_BINMAN_SYMBOLS in binman syms test.
>>>>
>>>> Tested-by: Tim Harvey <tharvey@gateworks.com> #imx8m[m,n,p]-venice
>>>> Signed-off-by: Peng Fan <peng.fan@nxp.com>
>>>> ---
>>>> include/binman_sym.h | 2 +-
>>>> tools/binman/test/Makefile | 2 +-
>>>> tools/binman/test/generated/autoconf.h | 3 +++
>>>> tools/binman/test/u_boot_binman_syms.c | 2 +-
>>>> tools/binman/test/u_boot_binman_syms_size.c | 2 +-
>>>> 5 files changed, 7 insertions(+), 4 deletions(-)
>>>> create mode 100644 tools/binman/test/generated/autoconf.h
>>> Reviewed-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>> Looks like I have misunderstood things here a bit. CONFIG_BINMAN enables
>> you to declare and use symbols. CONFIG_SPL/TPL_BINMAN_SYMBOLS declares
>> certain symbols ('u_boot_any'). The name is a bit misleading, as if it
>> enables support for using symbols, and that confused me.
> I am not sure, but I think CONFIG_SPL/TPL_BINMAN_SYMBOLS are only for
> certain symbols saying u_boot_any.

I mean I not think BINMAN_SYMBOLS are only for certtain symbols.

>
> Maybe Simon could comment?
>
>> I have sent a patch [1] that fixes the build error mentioned here, which
>> should be used instead of this patch. Please:
>>
>> - Rebase on top of that series [1]
>> - Maybe drop config changes in 1/8 and 2/8 (they're now unnecessary)
>> - Disable CONFIG_SPL/TPL/VPL_BINMAN_SYMBOLS for i.MX8M boards
>> - Change the if statement to if (IS_ENABLED(CONFIG_BINMAN)) in patch 5/8
> This not work, BINMAN has been enabled for packing images, but we have
> not generated some i.MX binman symbols
> for getting exact blob size. So I use binman symbols to get exact blob
> size in this patchset.
>
> Thanks,
> Peng.
>
>> - Drop this patch 7/8
>>
>> Sorry for the confusion.
>>
>> [1] spl: binman: Fix use of undeclared u_boot_any symbols
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fu-boot%2F20220610105806.27177-2-alpernebiyasak%40gmail.com%2F&amp;data=05%7C01%7Cpeng.fan%40nxp.com%7C4e7220d3cf724ca71a8e08da4b00e7fa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637904764505471459%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=L9jmVdbr1RUL8gKZa8QWwxFnPhmxjW5bYZKwq8eDsH4%3D&amp;reserved=0