Hi Tom,
On Wed, 13 Dec 2023 at 13:59, Tom Rini <tr...@konsulko.com> wrote:
>
> On Wed, Dec 13, 2023 at 01:51:00PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 13 Dec 2023 at 13:42, Tom Rini <tr...@konsulko.com> wrote:
> > >
> > > On Wed, Dec 13, 2023 at 12:50:02PM -0700, Simon Glass wrote:
> > > > 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?
> > >
> > > I don't follow you here, sorry.
> >
> > I mean that we share a lot of code already, code which contains CONFIG
> > options. So does it matter avoiding adding one more?
>
> It's adding <linux/kconfig.h> to files, we must not do that. And we
> don't need to if we don't have <watchdog.h> (and that in turn,
> cyclic.h) included outside of USE_HOSTCC :)
$ git grep kconfig.h
.azure-pipelines.yml: :^include/linux/kconfig.h
:^tools/ && exit 1 || exit 0
.gitlab-ci.yml: :^include/linux/kconfig.h :^tools/ && exit 1 || exit 0
Makefile: -include $(srctree)/include/linux/kconfig.h
Makefile: -include linux/kconfig.h -include
include/config.h \
arch/arm/include/asm/arch-fsl-layerscape/config.h:#include <linux/kconfig.h>
arch/arm/mach-rockchip/tpl.c:#include <linux/kconfig.h>
arch/arm/mach-sunxi/dram_sun50i_h6.c:#include <linux/kconfig.h>
arch/arm/mach-sunxi/dram_sun50i_h616.c:#include <linux/kconfig.h>
arch/arm/mach-sunxi/dram_sunxi_dw.c:#include <linux/kconfig.h>
boot/image-fit.c:#include <linux/kconfig.h>
boot/image.c:#include <linux/kconfig.h>
These are the sorts of ones which are a real pain to remove.
common/hash.c:#include <linux/kconfig.h>
drivers/timer/dw-apb-timer.c:#include <linux/kconfig.h>
env/embedded.c:#include <linux/kconfig.h>
include/bootstage.h:#include <linux/kconfig.h>
include/configs/at91-sama5_common.h:#include <linux/kconfig.h>
include/configs/tqma6.h:#include <linux/kconfig.h>
include/env_internal.h:#include <linux/kconfig.h>
include/hash.h:#include <linux/kconfig.h>
include/image.h:#include <linux/kconfig.h>
include/u-boot/ecdsa.h:#include <linux/kconfig.h>
lib/rsa/rsa-verify.c:#include <linux/kconfig.h>
scripts/Makefile.autoconf: -include
$(srctree)/include/linux/kconfig.h
scripts/Makefile.autoconf: echo \#include \<linux/kconfig.h\>;
\
test/dm/scmi.c:#include <linux/kconfig.h>
test/lib/kconfig.c: * Test of linux/kconfig.h macros
test/lib/kconfig_spl.c: * Test of linux/kconfig.h macros for SPL
tools/binman/test/blob_syms.c:#include <linux/kconfig.h>
tools/binman/test/u_boot_binman_syms.c:#include <linux/kconfig.h>
tools/binman/test/u_boot_binman_syms_size.c:#include <linux/kconfig.h>
tools/env/fw_env_private.h:#include <linux/kconfig.h>
tools/envcrc.c:#include <linux/kconfig.h>
tools/mkeficapsule.c:#include <linux/kconfig.h>
tools/printinitialenv.c:#include <linux/kconfig.h>
I don't really mind what we do with sha, and it seems I am a bit too
rabid on the anti-#ifdef thing compared to others.
Regards,
Simon
