[U-Boot] [PATCH v2 4/5] lib: Import hexdump.c from Linux

[Mario Six]

Hi Alexey,

On Mon, Jun 4, 2018 at 6:05 PM, Alexey Brodkin
<Alexey.Brodkin at synopsys.com> wrote:
> Hi Mario,
>
> On Wed, 2018-05-23 at 14:09 +0200, Mario Six wrote:
>> Especially for commands, it is useful to be able to turn a hexadecimal
>> string into its binary representation.
>>
>> Hence, import the hex_to_bin, bin2hex, and hex2bin functions from the
>> Linux kernel.
>>
>> Signed-off-by: Mario Six <mario.six at gdsys.cc>
>>
>> ---
>>
>> v1 -> v2:
>> New in v2
>
> Something is missing?
>
> Note there was a similar discussion some time ago here:
> https://patchwork.ozlabs.org/patch/633733/, might worth checking.
>
> If of any interest you may pick up my earlier patch and do
> fix-ups mentioned by Tom:
>  1. Move hexdump.h away from common.h
>  2. Update existing users of print_hex_dump() in U-Boot
>     such that they don't use debug level (i.e. drop the first argument)
>
> Or I may do the same re-spin sometime soon.
>

Thanks for your feedback! I saw that you posted a re-spin of your patch;
Thank you, that's very helpful.

> Still read-on for a couple of comments for your patch.
>
> [snip]
>
>>  /*
>>   * min()/max()/clamp() macros that also do
>>   * strict type-checking.. See the
>> diff --git a/lib/Makefile b/lib/Makefile
>> index d531ea54b31..0f6d744579f 100644
>> --- a/lib/Makefile
>> +++ b/lib/Makefile
>> @@ -29,6 +29,7 @@ obj-$(CONFIG_FIT) += fdtdec_common.o
>>  obj-$(CONFIG_TEST_FDTDEC) += fdtdec_test.o
>>  obj-$(CONFIG_GZIP_COMPRESSED) += gzip.o
>>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += smbios.o
>> +obj-y += hexdump.o
>
> U-Boot might be used on targets with limited memory
> so having ability to include hexdump or not might be
> beneficial here. Especially in production builds why would you need hexdump?
>

Yes, it's probably better to have it deactivatable, true. But as for why
production builds need hexdump: It's not so much the hexdump function, but the
bin2hex function, which can be used in a number of U-Boot commands that read
hexadecimal data. We use one such command to initialize a TPM on one of our
boards, for example.

> [snip]
>
>> +#ifdef CONFIG_PRINTK
>
> Why PRINTK in U-Boot? It's purely kernel's thing.
>
>> +#if !defined(CONFIG_DYNAMIC_DEBUG)
>
> Ditto, CONFIG_DYNAMIC_DEBUG has nothing to do with U-Boot.
>

Those were copied verbatim from the kernel, I'm pretty sure I just got them by
mistake.

> -Alexey
>

Best regards,
Mario