[PATCH v3 00/23] CONFIG_IS_ENABLED vs IS_ENABLED

[Tom Rini]

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-d5c08e096906@theobroma-systems.com/
> https://patchwork.ozlabs.org/project/uboot/patch/20230301-tsd-env-nowhere-kconfig-v1-2-d5c08e096906@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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20230511/20681988/attachment.sig>