On Thu, May 23, 2024 at 10:03 AM Quentin Schulz <quentin.sch...@cherry.de> wrote: > > Hi Sam, > > On 5/23/24 1:31 AM, Sam Protsenko wrote: > > Use CONFIG_IS_ENABLED() macro to check config options as recommended by > > checkpatch, instead of checking those with just #ifdef CONFIG_... > > > > No functional change. > > > > There are actual functional changes in here. > > defined(CONFIG_DM_MMC) != CONFIG_IS_ENABLED(DM_MMC) > > Currently, if one has SPL_MMC and MMC_DW_ROCKCHIP defined, it'll build > the SPL variant of MMC_DW_ROCKCHIP but guarding only with the U-Boot > proper symbols, meaning, essentially the SPL and proper variant of > rockchip_dw_mmc.o are more or less identical. > > I think we can argue this isn't proper and should be fixed. I think we > need to migrate all the MMC_DW drivers to use $(SPL_TPL_) in there and > create the symbols in Kconfig with the appropriate dependencies. We'll > likely also need to modify a bunch of defconfigs or make > SPL_MMC_DW_ROCKCHIP default y if MMC_DW_ROCKCHIP for example, so that we > don't break current support (it's pretty much expected to have MMC > support in SPL). > > I may have misinterpreted this, so please let me know where I am wrong > in my assumptions here, but this does look like more work than just this. >
Thanks for noticing this! I completely forgot about SPL as it's not built for my board. I tried to check with buildman and it seems like replacing #ifdefs with CONFIG_IS_ENABLED() doesn't change anything, but I'm not sure if buildman actually checks SPL at all (it probably only checks the U-Boot proper binary?). In my v2 I'll stick to #ifdef and avoid using CONFIG_IS_ENABLED(). As you said, correct rework probably takes much more than just replacing #ifdefs. It can be done later in a separate series. > Cheers, > Quentin