On Mon, Mar 13, 2023 at 02:31:22PM -0700, Troy Kisky wrote: > This patch set gets ready to checks the usage of > CONFIG_IS_ENABLED/IS_ENABLED. > > After the set has been applied, you can delete > test/usage_of_is_enabled_todo.txt > and run test/usage_of_is_enabled_commit.sh > > The script test/usage_of_is_enabled_check.sh > checks for new questionable uses of > CONFIG_IS_ENABLED/IS_ENABLED and is added > to .azure-pipelines.yml, and > .gitlab-ci.yml > > version 3 changes: > Dropped changes to puma-rk3399 and ringneck-px30 > in favor of Quentin Schulz's patch > https://patchwork.ozlabs.org/project/uboot/patch/20230301-tsd-env-nowhere-kconfig-v1-1-d5c08e096...@theobroma-systems.com/ > https://patchwork.ozlabs.org/project/uboot/patch/20230301-tsd-env-nowhere-kconfig-v1-2-d5c08e096...@theobroma-systems.com/ > Please apply them before this series. > > Drop gateworks: venice: Always define setup_fec and setup_eqos > Drop arm: cpu: armv7: ls102xa: fdt: remove eth_device support > Simon's patches took care of these > > Add to todo list, these new questionable uses that snuck into the code. > PARTITION_TYPE_GUID > PHY_ATHEROS > RTC_SANDBOX > > Changes in v3: > remove error entirely and prevent with Kconfig > new patch to address Tom's concerns > - Rebase on Simon's s/CMD_SATA/SATA/ change > - commit message updated > > Changes in v2: > - new patch > - delay include of linux/kconfig.h to do from Makefile > - as suggested by Simon > - delay include of linux/kconfig.h to do from Makefile > - as suggested by Simon > - delay include of linux/kconfig.h to do from Makefile > - as suggested by Simon > - delay include of linux/kconfig.h to do from Makefile > - as suggested by Simon > - delay include of linux/kconfig.h to do from Makefile > - as suggested by Simon > - include linux/kconfig.h from tools/Makefile > - as suggested by Simon > - changed condition of when to include field bdf > - added protection to another instance of bdf in uart.c > - Thanks to Simon for getting this corrected > - use normal if, not preprocessor > - new in series > - use an accessor function gd_set_pci_ram_top
So, I've provided some feedback on a few of these in particular, and applied a number of others. The biggest problems I see are that I'm not sure that overall the changes make the code more readable. When I ran the whole conversion series there were a few places where the resulting code changed because of how things are used today. Not as a problem for you to jump off and solve, but, looking harder at this makes me think that going for CONFIG_(SPL|TPL|VPL|etc)_FOO makes it harder to use the macros people expect from the kernel, where things are done as a suffix and so we could just use IS_ENABLED(CONFIG_FOO) and have things expand to test for CONFIG_FOO || (CONFIG_FOO_SPL && CONFIG_SPL_BUILD) || .. -- Tom
signature.asc
Description: PGP signature