Hi Alexey, On Mon, Jun 4, 2018 at 6:05 PM, Alexey Brodkin <alexey.brod...@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....@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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
