[PATCH 14/42] mmc: dw_mmc: Use CONFIG_IS_ENABLED() to check config options
Sam Protsenko
semen.protsenko at linaro.org
Mon Jun 10 00:33:00 CEST 2024
On Thu, May 23, 2024 at 10:03 AM Quentin Schulz
<quentin.schulz at 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
More information about the U-Boot
mailing list