Hi Tom, On Sat, 9 Dec 2023 at 08:48, Tom Rini <tr...@konsulko.com> wrote: > > On Sat, Dec 02, 2023 at 08:33:48AM -0700, Simon Glass wrote: > > > The cyclic subsystem is currently enabled either in all build phases > > or none. For tools this should not be enabled, but since lib/shc256.c > > and other files include watchdog.h in the host build, we must make > > sure that it is not enabled there. > > This part is what I see as the Wrong Direction. There's some code we > really need to share with our user space tools, in order to not > copy/paste the same code. In turn, this code must be as user-space > friendly as possible. Maybe even we re-factor things a little more, if > needed, so that we _just_ have the library functions in common files, > and u-boot or user space only files have the make use of logic. I don't > feel bad about tools/ needing: > void sha256_csum_wd(const unsigned char *input, unsigned int ilen, > unsigned char *output, unsigned int chunk_sz) > { > sha256_context ctx; > sha256_starts(&ctx); > sha256_update(&ctx, input, ilen); > sha256_finish(&ctx, output); > } > > (and so on for other algos) as a duplicate bit of code. Much less so > than I do about adding <linux/kconfig.h> to a direct include list (since > we should never be doing that) so that later on we can if > (IS_ENABLED(..)) the existing code.
Bear in mind that we have the CONFIG_TOOLS_... options entirely to deal with the need for tools to enable features in common code. This SHA thing is a very small part of the code, compared to common code in boot/ for example. So is this really a win? Regards, Simon