Re: CONFIG_IS_ENABLED vs IS_ENABLED

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 2485 bytes --]

On Tue, Jan 24, 2023 at 06:36:00PM -0700, Simon Glass wrote:
> Hi Troy,
> 
> On Tue, 24 Jan 2023 at 16:31, Troy Kisky <Troy.Kisky@lairdconnect.com> wrote:
> >
> >
> >
> > ________________________________
> > From: Troy Kisky
> > Sent: Tuesday, January 24, 2023 2:52 PM
> > To: u-boot@lists.denx.de <u-boot@lists.denx.de>; sbabic@denx.de <sbabic@denx.de>; trini@konsulko.com <trini@konsulko.com>; festevam@gmail.com <festevam@gmail.com>
> > Cc: sjg@chromium.org <sjg@chromium.org>; marex@denx.de <marex@denx.de>
> > Subject: CONFIG_IS_ENABLED vs IS_ENABLED
> >
> > Hi Guys
> >
> > In a recent debugging session, I stumbled across this line
> > drivers/mmc/mmc.c:      if (CONFIG_IS_ENABLED(MMC_QUIRKS) && mmc->quirks & quirk)
> >
> > which prevents retries in SPL code, and was causing booting from an SD card to fail.
> > So I wrote a little script to print uses of
> > CONFIG_IS_ENABLED(x) which might need to be
> > IS_ENABLED(CONFIG_x) like the above one.
> >
> > Here it is if you want to try it out.
> >
> > git grep CONFIG_IS_ENABLED|sed -n -e "s/\(CONFIG_IS_ENABLED([0-9a-zA-Z_]*)\)/\n\1\n/gp"| \
> > sed -n -r "s/CONFIG_IS_ENABLED\(([0-9a-zA-Z_]+)\)/\1/p" |sort -u|xargs -I {} \
> > sh -c "git grep -E 'config [ST]PL_{}' | grep -q -E -w '[ST]PL_{}' || git grep 'CONFIG_IS_ENABLED({})'"
> >
> > It prints CONFIG_IS_ENABLED(x) uses where there is no SPL_x or TPL_x.
> >
> > BR
> > Troy
> >
> > _______
> > And here is the opposite check
> >
> > git grep -w IS_ENABLED|sed -n -e "s/\(IS_ENABLED(CONFIG_[0-9a-zA-Z_]*)\)/\n\1\n/gp"| \
> > sed -n -r "s/IS_ENABLED\(CONFIG_([0-9a-zA-Z_]+)\)/\1/p" |sort -u|xargs -I {} \
> > sh -c "git grep -E 'config [ST]PL_{}' | grep -q -E -w '[ST]PL_{}' && git grep 'IS_ENABLED(CONFIG_{})'"
> >
> >
> > It prints uses of IS_ENABLED(CONFIG_x) where CONFIG_SPL_x exists.
> 
> Thank you for that. We definitely have quite a few of these.
> 
> By a great coincidence I updated moveconfig.py to do something a
> little like that:.
> 
> https://patchwork.ozlabs.org/project/uboot/patch/20230123220031.3540724-2-sjg@chromium.org/

I think this also shows that we might really want to just drop the
checkpatch.pl note about IS_ENABLED / CONFIG_IS_ENABLED, it's getting
used in a lot of wrong places where it's not helpful. It's not the root
cause here (where a compile time check that allows for the rest of the
code to be statically checked still is OK), but it's part of the
problem.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]