* CONFIG_IS_ENABLED vs IS_ENABLED
@ 2023-01-24 22:52 Troy Kisky
2023-01-24 23:31 ` Troy Kisky
0 siblings, 1 reply; 17+ messages in thread
From: Troy Kisky @ 2023-01-24 22:52 UTC (permalink / raw)
To: u-boot@lists.denx.de, sbabic@denx.de, trini@konsulko.com,
festevam@gmail.com
Cc: sjg@chromium.org, marex@denx.de
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
THIS MESSAGE, ANY ATTACHMENT(S), AND THE INFORMATION CONTAINED HEREIN MAY BE PROPRIETARY TO LAIRD CONNECTIVITY, LLC., BOUNDARY DEVICES, LLC. AND/OR ANOTHER PARTY, AND MAY FURTHER BE INTENDED TO BE KEPT CONFIDENTIAL. IF YOU ARE NOT THE INTENDED RECIPIENT, PLEASE DELETE THE EMAIL AND ANY ATTACHMENTS, AND IMMEDIATELY NOTIFY THE SENDER BY RETURN EMAIL. THIS MESSAGE AND ITS CONTENTS ARE THE PROPERTY OF LAIRD CONNECTIVITY, LLC. AND BOUNDARY DEVICES, LLC. AND MAY NOT BE REPRODUCED OR USED WITHOUT THE EXPRESS WRITTEN CONSENT OF LAIRD CONNECTIVITY, LLC. AND BOUNDARY DEVICES, LLC.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-24 22:52 CONFIG_IS_ENABLED vs IS_ENABLED Troy Kisky @ 2023-01-24 23:31 ` Troy Kisky 2023-01-25 1:36 ` Simon Glass 0 siblings, 1 reply; 17+ messages in thread From: Troy Kisky @ 2023-01-24 23:31 UTC (permalink / raw) To: Troy Kisky, u-boot@lists.denx.de, sbabic@denx.de, trini@konsulko.com, festevam@gmail.com Cc: sjg@chromium.org, marex@denx.de ________________________________ 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. BR Troy THIS MESSAGE, ANY ATTACHMENT(S), AND THE INFORMATION CONTAINED HEREIN MAY BE PROPRIETARY TO LAIRD CONNECTIVITY, LLC., BOUNDARY DEVICES, LLC. AND/OR ANOTHER PARTY, AND MAY FURTHER BE INTENDED TO BE KEPT CONFIDENTIAL. IF YOU ARE NOT THE INTENDED RECIPIENT, PLEASE DELETE THE EMAIL AND ANY ATTACHMENTS, AND IMMEDIATELY NOTIFY THE SENDER BY RETURN EMAIL. THIS MESSAGE AND ITS CONTENTS ARE THE PROPERTY OF LAIRD CONNECTIVITY, LLC. AND BOUNDARY DEVICES, LLC. AND MAY NOT BE REPRODUCED OR USED WITHOUT THE EXPRESS WRITTEN CONSENT OF LAIRD CONNECTIVITY, LLC. AND BOUNDARY DEVICES, LLC. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-24 23:31 ` Troy Kisky @ 2023-01-25 1:36 ` Simon Glass 2023-01-26 17:21 ` Tom Rini 0 siblings, 1 reply; 17+ messages in thread From: Simon Glass @ 2023-01-25 1:36 UTC (permalink / raw) To: Troy Kisky Cc: u-boot@lists.denx.de, sbabic@denx.de, trini@konsulko.com, festevam@gmail.com, marex@denx.de 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/ Regards, Simon > > > THIS MESSAGE, ANY ATTACHMENT(S), AND THE INFORMATION CONTAINED HEREIN MAY BE PROPRIETARY TO LAIRD CONNECTIVITY, LLC., BOUNDARY DEVICES, LLC. AND/OR ANOTHER PARTY, AND MAY FURTHER BE INTENDED TO BE KEPT CONFIDENTIAL. IF YOU ARE NOT THE INTENDED RECIPIENT, PLEASE DELETE THE EMAIL AND ANY ATTACHMENTS, AND IMMEDIATELY NOTIFY THE SENDER BY RETURN EMAIL. THIS MESSAGE AND ITS CONTENTS ARE THE PROPERTY OF LAIRD CONNECTIVITY, LLC. AND BOUNDARY DEVICES, LLC. AND MAY NOT BE REPRODUCED OR USED WITHOUT THE EXPRESS WRITTEN CONSENT OF LAIRD CONNECTIVITY, LLC. AND BOUNDARY DEVICES, LLC. Are you able to drop that from mailing list posts, please? ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-25 1:36 ` Simon Glass @ 2023-01-26 17:21 ` Tom Rini 2023-01-26 18:04 ` Simon Glass 0 siblings, 1 reply; 17+ messages in thread From: Tom Rini @ 2023-01-26 17:21 UTC (permalink / raw) To: Simon Glass Cc: Troy Kisky, u-boot@lists.denx.de, sbabic@denx.de, festevam@gmail.com, marex@denx.de [-- 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 --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-26 17:21 ` Tom Rini @ 2023-01-26 18:04 ` Simon Glass 2023-01-26 18:16 ` Tom Rini 0 siblings, 1 reply; 17+ messages in thread From: Simon Glass @ 2023-01-26 18:04 UTC (permalink / raw) To: Tom Rini Cc: Troy Kisky, u-boot@lists.denx.de, sbabic@denx.de, festevam@gmail.com, marex@denx.de Hi Tom, On Thu, 26 Jan 2023 at 10:21, Tom Rini <trini@konsulko.com> wrote: > > 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. Firstly, we want to drop the use of #ifdef so what should we say instead? Secondly, I think we should fix all this by splitting the config, along the lines of my old series [1]. I hit similar problems to Troy and have modified moveconfig.py to detect these (hence my recent series [2]). I will see if I can get some sort of series out by Monday. I had something pretty close(TM) but it failed on a few qemu tests [3] Troy, are you able to create patches for the problems you find, and perhaps adding your checks to a Makefile rule or script so we can run it in CI? Regards, Simon [1] https://patchwork.ozlabs.org/project/uboot/list/?series=262853&state=* [2] https://patchwork.ozlabs.org/project/uboot/list/?series=338142 [3] https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/14830 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-26 18:04 ` Simon Glass @ 2023-01-26 18:16 ` Tom Rini 2023-01-26 21:29 ` Simon Glass 0 siblings, 1 reply; 17+ messages in thread From: Tom Rini @ 2023-01-26 18:16 UTC (permalink / raw) To: Simon Glass Cc: Troy Kisky, u-boot@lists.denx.de, sbabic@denx.de, festevam@gmail.com, marex@denx.de [-- Attachment #1: Type: text/plain, Size: 4795 bytes --] On Thu, Jan 26, 2023 at 11:04:21AM -0700, Simon Glass wrote: > Hi Tom, > > On Thu, 26 Jan 2023 at 10:21, Tom Rini <trini@konsulko.com> wrote: > > > > 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. > > Firstly, we want to drop the use of #ifdef so what should we say instead? I'm not sure that dropping #ifdef in and of itself is a good goal. #if IS_ENABLED(CONFIG_FOO) does not read better #ifdef CONFIG_FOO And we have a lot of cases of the former that I'm not sure can logically or helpfully be replaced with if (IS_ENABLED(CONFIG_FOO)) either for functional / legibility reasons (ie it doesn't read better and just getting static analysis isn't a great reasons, more indent makes the code harder to follow) or isn't possible because things like: #ifdef CONFIG_FOO int i = CONFIG_BAZ; ... #endif Can't be replaced with if (IS_ENABLED(CONFIG_FOO)) { ... } when CONFIG_BAZ is only ever asked when CONFIG_FOO is enabled. And we can't define CONFIG_BAZ to 0 if not set (that leads back to introducing CONFIG things being defined). > Secondly, I think we should fix all this by splitting the config, > along the lines of my old series [1]. I hit similar problems to Troy > and have modified moveconfig.py to detect these (hence my recent > series [2]). I will see if I can get some sort of series out by > Monday. I had something pretty close(TM) but it failed on a few qemu > tests [3] I don't disagree with seeing what things look like with a "split" config again, but I think that fundamentally Yamada-san was right way back, and we really need to move towards separate config files for each stage and then combine them when applicable. That's also not easy, but I'm also not sure how to deal with the cases where we intentionally have CONFIG_FOO=n CONFIG_SPL_FOO=y without doing what we're doing today, at some level. > Troy, are you able to create patches for the problems you find, and > perhaps adding your checks to a Makefile rule or script so we can run > it in CI? What we're doing today does need to be analyzed and some good number of cases corrected I am sure. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-26 18:16 ` Tom Rini @ 2023-01-26 21:29 ` Simon Glass 2023-01-26 21:33 ` Tom Rini 0 siblings, 1 reply; 17+ messages in thread From: Simon Glass @ 2023-01-26 21:29 UTC (permalink / raw) To: Tom Rini Cc: Troy Kisky, u-boot@lists.denx.de, sbabic@denx.de, festevam@gmail.com, marex@denx.de Hi Tom, On Thu, 26 Jan 2023 at 11:16, Tom Rini <trini@konsulko.com> wrote: > > On Thu, Jan 26, 2023 at 11:04:21AM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Thu, 26 Jan 2023 at 10:21, Tom Rini <trini@konsulko.com> wrote: > > > > > > 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. > > > > Firstly, we want to drop the use of #ifdef so what should we say instead? > > I'm not sure that dropping #ifdef in and of itself is a good goal. > #if IS_ENABLED(CONFIG_FOO) > does not read better > #ifdef CONFIG_FOO So far in my prototype I have implemented #if CONFIG(FOO) and if (CONFIG(FOO)) which replaces direct use of CONFIG_FOO and also CONFIG_IS_ENABLED(FOO). We could ban use of CONFIG_FOO easily enough. > > And we have a lot of cases of the former that I'm not sure can > logically or helpfully be replaced with > if (IS_ENABLED(CONFIG_FOO)) > either for functional / legibility reasons (ie it doesn't read better > and just getting static analysis isn't a great reasons, more indent > makes the code harder to follow) or isn't possible because things like: > #ifdef CONFIG_FOO > int i = CONFIG_BAZ; > ... > #endif > > Can't be replaced with if (IS_ENABLED(CONFIG_FOO)) { ... } when > CONFIG_BAZ is only ever asked when CONFIG_FOO is enabled. And we can't > define CONFIG_BAZ to 0 if not set (that leads back to introducing > CONFIG things being defined). We have IF_ENABLED_INT() so can do things like that > > > Secondly, I think we should fix all this by splitting the config, > > along the lines of my old series [1]. I hit similar problems to Troy > > and have modified moveconfig.py to detect these (hence my recent > > series [2]). I will see if I can get some sort of series out by > > Monday. I had something pretty close(TM) but it failed on a few qemu > > tests [3] > > I don't disagree with seeing what things look like with a "split" config > again, but I think that fundamentally Yamada-san was right way back, and > we really need to move towards separate config files for each stage and > then combine them when applicable. Well let's see what you think once I get this next version of the series out. > That's also not easy, but I'm also > not sure how to deal with the cases where we intentionally have > CONFIG_FOO=n CONFIG_SPL_FOO=y without doing what we're doing today, at > some level. Well I believe these are bugs, but don't know how many. Will probably have some idea by early next week. > > > Troy, are you able to create patches for the problems you find, and > > perhaps adding your checks to a Makefile rule or script so we can run > > it in CI? > > What we're doing today does need to be analyzed and some good number of > cases corrected I am sure. Yes. Regards, Simon ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-26 21:29 ` Simon Glass @ 2023-01-26 21:33 ` Tom Rini 2023-01-27 14:30 ` Simon Glass 0 siblings, 1 reply; 17+ messages in thread From: Tom Rini @ 2023-01-26 21:33 UTC (permalink / raw) To: Simon Glass Cc: Troy Kisky, u-boot@lists.denx.de, sbabic@denx.de, festevam@gmail.com, marex@denx.de [-- Attachment #1: Type: text/plain, Size: 5890 bytes --] On Thu, Jan 26, 2023 at 02:29:53PM -0700, Simon Glass wrote: > Hi Tom, > > On Thu, 26 Jan 2023 at 11:16, Tom Rini <trini@konsulko.com> wrote: > > > > On Thu, Jan 26, 2023 at 11:04:21AM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Thu, 26 Jan 2023 at 10:21, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > 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. > > > > > > Firstly, we want to drop the use of #ifdef so what should we say instead? > > > > I'm not sure that dropping #ifdef in and of itself is a good goal. > > #if IS_ENABLED(CONFIG_FOO) > > does not read better > > #ifdef CONFIG_FOO > > So far in my prototype I have implemented > > #if CONFIG(FOO) > > and > > if (CONFIG(FOO)) > > which replaces direct use of CONFIG_FOO and also > CONFIG_IS_ENABLED(FOO). We could ban use of CONFIG_FOO easily enough. I'm not convinced #if CONFIG(FOO) is better, but OK. > > And we have a lot of cases of the former that I'm not sure can > > logically or helpfully be replaced with > > if (IS_ENABLED(CONFIG_FOO)) > > either for functional / legibility reasons (ie it doesn't read better > > and just getting static analysis isn't a great reasons, more indent > > makes the code harder to follow) or isn't possible because things like: > > #ifdef CONFIG_FOO > > int i = CONFIG_BAZ; > > ... > > #endif > > > > Can't be replaced with if (IS_ENABLED(CONFIG_FOO)) { ... } when > > CONFIG_BAZ is only ever asked when CONFIG_FOO is enabled. And we can't > > define CONFIG_BAZ to 0 if not set (that leads back to introducing > > CONFIG things being defined). > > We have IF_ENABLED_INT() so can do things like that And could come up wit IF_ENABLED_STRING() I suppose. But I'm still not sure it'll read better / more clearly. > > > Secondly, I think we should fix all this by splitting the config, > > > along the lines of my old series [1]. I hit similar problems to Troy > > > and have modified moveconfig.py to detect these (hence my recent > > > series [2]). I will see if I can get some sort of series out by > > > Monday. I had something pretty close(TM) but it failed on a few qemu > > > tests [3] > > > > I don't disagree with seeing what things look like with a "split" config > > again, but I think that fundamentally Yamada-san was right way back, and > > we really need to move towards separate config files for each stage and > > then combine them when applicable. > > Well let's see what you think once I get this next version of the series out. OK. > > That's also not easy, but I'm also > > not sure how to deal with the cases where we intentionally have > > CONFIG_FOO=n CONFIG_SPL_FOO=y without doing what we're doing today, at > > some level. > > Well I believe these are bugs, but don't know how many. Will probably > have some idea by early next week. There's certainly some bugs, but there's also some intentionally done ones, such as around SKIP_LOWLEVEL_INIT -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-26 21:33 ` Tom Rini @ 2023-01-27 14:30 ` Simon Glass 0 siblings, 0 replies; 17+ messages in thread From: Simon Glass @ 2023-01-27 14:30 UTC (permalink / raw) To: Tom Rini Cc: Troy Kisky, u-boot@lists.denx.de, sbabic@denx.de, festevam@gmail.com, marex@denx.de Hi Tom, On Thu, 26 Jan 2023 at 14:33, Tom Rini <trini@konsulko.com> wrote: > > On Thu, Jan 26, 2023 at 02:29:53PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Thu, 26 Jan 2023 at 11:16, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Thu, Jan 26, 2023 at 11:04:21AM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Thu, 26 Jan 2023 at 10:21, Tom Rini <trini@konsulko.com> wrote: > > > > > > > > > > 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. > > > > > > > > Firstly, we want to drop the use of #ifdef so what should we say instead? > > > > > > I'm not sure that dropping #ifdef in and of itself is a good goal. > > > #if IS_ENABLED(CONFIG_FOO) > > > does not read better > > > #ifdef CONFIG_FOO > > > > So far in my prototype I have implemented > > > > #if CONFIG(FOO) > > > > and > > > > if (CONFIG(FOO)) > > > > which replaces direct use of CONFIG_FOO and also > > CONFIG_IS_ENABLED(FOO). We could ban use of CONFIG_FOO easily enough. > > I'm not convinced #if CONFIG(FOO) is better, but OK. OK. > > > > And we have a lot of cases of the former that I'm not sure can > > > logically or helpfully be replaced with > > > if (IS_ENABLED(CONFIG_FOO)) > > > either for functional / legibility reasons (ie it doesn't read better > > > and just getting static analysis isn't a great reasons, more indent > > > makes the code harder to follow) or isn't possible because things like: > > > #ifdef CONFIG_FOO > > > int i = CONFIG_BAZ; > > > ... > > > #endif > > > > > > Can't be replaced with if (IS_ENABLED(CONFIG_FOO)) { ... } when > > > CONFIG_BAZ is only ever asked when CONFIG_FOO is enabled. And we can't > > > define CONFIG_BAZ to 0 if not set (that leads back to introducing > > > CONFIG things being defined). > > > > We have IF_ENABLED_INT() so can do things like that > > And could come up wit IF_ENABLED_STRING() I suppose. But I'm still not > sure it'll read better / more clearly. No it isn't that great, but we might get used to it. It is rare but does resolve one of the two main remaining needs for #ifdef > > > > > Secondly, I think we should fix all this by splitting the config, > > > > along the lines of my old series [1]. I hit similar problems to Troy > > > > and have modified moveconfig.py to detect these (hence my recent > > > > series [2]). I will see if I can get some sort of series out by > > > > Monday. I had something pretty close(TM) but it failed on a few qemu > > > > tests [3] > > > > > > I don't disagree with seeing what things look like with a "split" config > > > again, but I think that fundamentally Yamada-san was right way back, and > > > we really need to move towards separate config files for each stage and > > > then combine them when applicable. > > > > Well let's see what you think once I get this next version of the series out. > > OK. > > > > That's also not easy, but I'm also > > > not sure how to deal with the cases where we intentionally have > > > CONFIG_FOO=n CONFIG_SPL_FOO=y without doing what we're doing today, at > > > some level. > > > > Well I believe these are bugs, but don't know how many. Will probably > > have some idea by early next week. > > There's certainly some bugs, but there's also some intentionally done > ones, such as around SKIP_LOWLEVEL_INIT Ah OK. Well if there is SPL_ symbol as well, as in this case, then perhaps it is clear what the intention is? Regards, Simon ^ permalink raw reply [flat|nested] 17+ messages in thread
* re: CONFIG_IS_ENABLED vs IS_ENABLED
@ 2023-01-26 17:26 Troy Kisky
2023-01-26 17:34 ` Tom Rini
0 siblings, 1 reply; 17+ messages in thread
From: Troy Kisky @ 2023-01-26 17:26 UTC (permalink / raw)
To: u-boot@lists.denx.de, sjg, trini, festevam, marex; +Cc: gary.bisson
Thanks Simon
I switched emails to get rid of the legalese.
Below are scripts to commit the changes sorted by
CONFIG_x variable. The only one I know causes a problem
is CONFIG_OF_LIVE because of
drivers/core/Makefile
obj-$(CONFIG_$(SPL_)OF_LIVE)
So, that config needs to keep using CONFIG_IS_ENABLED even
though SPL_OF_LIVE isn't in any Kconfig file.
Maybe something like
config SPL_OF_LIVE
bool
can be added to a Kconfig to prevent the bad change.
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 -l 'CONFIG_IS_ENABLED({})' | \
xargs -IFile sh -c \"sed -i
\\\"s/CONFIG_IS_ENABLED({})/IS_ENABLED(CONFIG_{})/g\\\" File\" ; \
git commit -a -m\"CONFIG_{}: change CONFIG_IS_ENABLED to IS_ENABLED\"; }"
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 -l 'IS_ENABLED(CONFIG_{})' | \
xargs -IFile sh -c \"sed -i
\\\"s/IS_ENABLED(CONFIG_{})/CONFIG_IS_ENABLED({})/g\\\" File\" ; \
git commit -a -m\"CONFIG_{}: change IS_ENABLED to CONFIG_IS_ENABLED\"; }"
I compile tested a few boards, but a thorough compile test would be good. I
hope the above helps someone get a few of the changes in mainline.
BR
Troy
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-26 17:26 Troy Kisky @ 2023-01-26 17:34 ` Tom Rini 2023-01-28 17:25 ` Troy Kisky 0 siblings, 1 reply; 17+ messages in thread From: Tom Rini @ 2023-01-26 17:34 UTC (permalink / raw) To: Troy Kisky; +Cc: u-boot@lists.denx.de, sjg, festevam, marex, gary.bisson [-- Attachment #1: Type: text/plain, Size: 1920 bytes --] On Thu, Jan 26, 2023 at 09:26:18AM -0800, Troy Kisky wrote: > Thanks Simon > > I switched emails to get rid of the legalese. > Below are scripts to commit the changes sorted by > CONFIG_x variable. The only one I know causes a problem > is CONFIG_OF_LIVE because of > > drivers/core/Makefile > obj-$(CONFIG_$(SPL_)OF_LIVE) > > So, that config needs to keep using CONFIG_IS_ENABLED even > though SPL_OF_LIVE isn't in any Kconfig file. > > Maybe something like > config SPL_OF_LIVE > bool > > can be added to a Kconfig to prevent the bad change. > > 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 -l 'CONFIG_IS_ENABLED({})' | \ > xargs -IFile sh -c \"sed -i > \\\"s/CONFIG_IS_ENABLED({})/IS_ENABLED(CONFIG_{})/g\\\" File\" ; \ > git commit -a -m\"CONFIG_{}: change CONFIG_IS_ENABLED to IS_ENABLED\"; }" > > 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 -l 'IS_ENABLED(CONFIG_{})' | \ > xargs -IFile sh -c \"sed -i > \\\"s/IS_ENABLED(CONFIG_{})/CONFIG_IS_ENABLED({})/g\\\" File\" ; \ > git commit -a -m\"CONFIG_{}: change IS_ENABLED to CONFIG_IS_ENABLED\"; }" > > > I compile tested a few boards, but a thorough compile test would be good. I > hope the above helps someone get a few of the changes in mainline. Submitting a PR against https://github.com/u-boot/u-boot/ will trigger an Azure CI run, and so a global build. And adding a test to CI to fail on new introductions of this would be how to prevent further issues. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-26 17:34 ` Tom Rini @ 2023-01-28 17:25 ` Troy Kisky 2023-01-30 17:18 ` Tom Rini 0 siblings, 1 reply; 17+ messages in thread From: Troy Kisky @ 2023-01-28 17:25 UTC (permalink / raw) To: Tom Rini; +Cc: u-boot@lists.denx.de, sjg, festevam, marex, gary.bisson Thanks Tom, I cleaned up the PR based on the CI results. Here's my current changes. Author: Troy Kisky <troy.kisky@boundarydevices.com> Date: Fri Jan 27 11:03:11 2023 -0800 dm: device-internal: use EVENT instead of DM_EVENT, because event_notify is built for EVENT Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h index f31c4702086..2e725aa9416 100644 --- a/include/dm/device-internal.h +++ b/include/dm/device-internal.h @@ -431,7 +431,7 @@ static inline void devres_release_all(struct udevice *dev) static inline int device_notify(const struct udevice *dev, enum event_t type) { -#if CONFIG_IS_ENABLED(DM_EVENT) +#if CONFIG_IS_ENABLED(EVENT) return event_notify(type, &dev, sizeof(dev)); #else return 0; ______________________________ Here's the CI test scripts to prevent more occurrences. { { git grep 'obj-$(CONFIG_$(SPL_'|grep Makefile|sed -e "s/SPL_TPL_/SPL_/"| \ sed -n -r 's/obj\-\$\(CONFIG_\$\(SPL_\)([0-9a-zA-Z_]+)\)/\n\{\1\}\n/gp'| \ sed -n -r 's/\{([0-9a-zA-Z_]+)\}/\1/p'; } ;\ { git grep -E 'config [ST]PL_'|grep Kconfig| \ sed -n -r "s/config [ST]PL_([0-9a-zA-Z_]+)/\n\{\1\}\n/p" | sed -n -r 's/\{([0-9a-zA-Z_]+)\}/\1/p'; } ; \ git grep -E 'CONFIG_IS_ENABLED\(CMD_'|sed -n -e "s/\(CONFIG_IS_ENABLED(CMD_[0-9a-zA-Z_]*)\)/\n\1\n/gp"| \ sed -n -r "s/CONFIG_IS_ENABLED\((CMD_[0-9a-zA-Z_]+)\)/\1/p"; \ echo -e "\ BZIP2\n\ CONFIG_CLK\n\ EFI_DEVICE_PATH_TO_TEXT\n\ EFI_LOADER\n\ ERRNO_STR\n\ FOO\n\ GENERATE_SMBIOS_TABLE\n\ ";\ } | sort -u >splvar.tmp 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| comm -23 - splvar.tmp 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|grep -v FOO| join - splvar.tmp ______________________ And the commit scripts to change current complaints. 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| comm -23 - splvar.tmp|xargs -I {} \ sh -c "git grep -l 'CONFIG_IS_ENABLED({})' | \ xargs -IFile sh -c \"sed -i -e \\\"s/CONFIG_IS_ENABLED({})/IS_ENABLED(CONFIG_{})/g\\\" File\" ; \ git commit -a -m\"CONFIG_{}: change CONFIG_IS_ENABLED to IS_ENABLED\";" 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|grep -v FOO| \ join - splvar.tmp |xargs -I {} \ sh -c "git grep -l 'IS_ENABLED(CONFIG_{})' | \ xargs -IFile sh -c \"sed -i -e \\\"s/\([^_]\)IS_ENABLED(CONFIG_{})/\1CONFIG_IS_ENABLED({})/g\\\" File\" ; \ git commit -a -m\"CONFIG_{}: change IS_ENABLED to CONFIG_IS_ENABLED\";" __________ BR Troy On Thu, Jan 26, 2023 at 9:34 AM Tom Rini <trini@konsulko.com> wrote: > On Thu, Jan 26, 2023 at 09:26:18AM -0800, Troy Kisky wrote: > > Thanks Simon > > > > I switched emails to get rid of the legalese. > > Below are scripts to commit the changes sorted by > > CONFIG_x variable. The only one I know causes a problem > > is CONFIG_OF_LIVE because of > > > > drivers/core/Makefile > > obj-$(CONFIG_$(SPL_)OF_LIVE) > > > > So, that config needs to keep using CONFIG_IS_ENABLED even > > though SPL_OF_LIVE isn't in any Kconfig file. > > > > Maybe something like > > config SPL_OF_LIVE > > bool > > > > can be added to a Kconfig to prevent the bad change. > > > > 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 -l 'CONFIG_IS_ENABLED({})' | \ > > xargs -IFile sh -c \"sed -i > > \\\"s/CONFIG_IS_ENABLED({})/IS_ENABLED(CONFIG_{})/g\\\" File\" ; \ > > git commit -a -m\"CONFIG_{}: change CONFIG_IS_ENABLED to IS_ENABLED\"; }" > > > > 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 -l 'IS_ENABLED(CONFIG_{})' | \ > > xargs -IFile sh -c \"sed -i > > \\\"s/IS_ENABLED(CONFIG_{})/CONFIG_IS_ENABLED({})/g\\\" File\" ; \ > > git commit -a -m\"CONFIG_{}: change IS_ENABLED to CONFIG_IS_ENABLED\"; }" > > > > > > I compile tested a few boards, but a thorough compile test would be > good. I > > hope the above helps someone get a few of the changes in mainline. > > Submitting a PR against https://github.com/u-boot/u-boot/ will trigger > an Azure CI run, and so a global build. And adding a test to CI to fail > on new introductions of this would be how to prevent further issues. > > -- > Tom > ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-28 17:25 ` Troy Kisky @ 2023-01-30 17:18 ` Tom Rini 2023-01-30 18:51 ` Troy Kisky 0 siblings, 1 reply; 17+ messages in thread From: Tom Rini @ 2023-01-30 17:18 UTC (permalink / raw) To: Troy Kisky; +Cc: u-boot@lists.denx.de, sjg, festevam, marex, gary.bisson [-- Attachment #1: Type: text/plain, Size: 1305 bytes --] On Sat, Jan 28, 2023 at 09:25:54AM -0800, Troy Kisky wrote: > Thanks Tom, > > I cleaned up the PR based on the CI results. Here's my current changes. > > Author: Troy Kisky <troy.kisky@boundarydevices.com> > Date: Fri Jan 27 11:03:11 2023 -0800 > > dm: device-internal: use EVENT instead of DM_EVENT, because > event_notify is built for EVENT > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> > > diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h > index f31c4702086..2e725aa9416 100644 > --- a/include/dm/device-internal.h > +++ b/include/dm/device-internal.h > @@ -431,7 +431,7 @@ static inline void devres_release_all(struct udevice > *dev) > > static inline int device_notify(const struct udevice *dev, enum event_t > type) > { > -#if CONFIG_IS_ENABLED(DM_EVENT) > +#if CONFIG_IS_ENABLED(EVENT) > return event_notify(type, &dev, sizeof(dev)); > #else > return 0; Given 448e2b6327d0 ("event: Correct dependencies on the EVENT framework") I'm a little worried about this change here and want to be extra sure it doesn't break something inadvertently. Aside from that, are you able to post your series? And incorporate your checking script in to .gitlab-ci.yml / .azure-pipline.yml ? -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-30 17:18 ` Tom Rini @ 2023-01-30 18:51 ` Troy Kisky 2023-01-30 19:44 ` Tom Rini 0 siblings, 1 reply; 17+ messages in thread From: Troy Kisky @ 2023-01-30 18:51 UTC (permalink / raw) To: Tom Rini; +Cc: u-boot@lists.denx.de, sjg, festevam, marex, gary.bisson Hi Tom On Mon, Jan 30, 2023 at 9:18 AM Tom Rini <trini@konsulko.com> wrote: > On Sat, Jan 28, 2023 at 09:25:54AM -0800, Troy Kisky wrote: > > Thanks Tom, > > > > I cleaned up the PR based on the CI results. Here's my current changes. > > > > Author: Troy Kisky <troy.kisky@boundarydevices.com> > > Date: Fri Jan 27 11:03:11 2023 -0800 > > > > dm: device-internal: use EVENT instead of DM_EVENT, because > > event_notify is built for EVENT > > > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> > > > > diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h > > index f31c4702086..2e725aa9416 100644 > > --- a/include/dm/device-internal.h > > +++ b/include/dm/device-internal.h > > @@ -431,7 +431,7 @@ static inline void devres_release_all(struct udevice > > *dev) > > > > static inline int device_notify(const struct udevice *dev, enum event_t > > type) > > { > > -#if CONFIG_IS_ENABLED(DM_EVENT) > > +#if CONFIG_IS_ENABLED(EVENT) > > return event_notify(type, &dev, sizeof(dev)); > > #else > > return 0; > > Given 448e2b6327d0 ("event: Correct dependencies on the EVENT > framework") I'm a little worried about this change here and want to be > extra sure it doesn't break something inadvertently. event_notify is in common/event, and the Makefile has obj-$(CONFIG_$(SPL_TPL_)EVENT) += event.o So, the other option is to change the Makefile line to obj-$(CONFIG_$(SPL_TPL_)DM_EVENT) += event.o I don't know which is best. > Aside from that, > are you able to post your series? I was hoping each of the maintainers could run the script and see if the patches for their area make sense. I don't need my sign-off on any of the patches. And incorporate your checking script > in to .gitlab-ci.yml / .azure-pipline.yml ? > I'll post a patch for that as an RFC > > -- > Tom > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-30 18:51 ` Troy Kisky @ 2023-01-30 19:44 ` Tom Rini 2023-01-30 22:16 ` Troy Kisky 0 siblings, 1 reply; 17+ messages in thread From: Tom Rini @ 2023-01-30 19:44 UTC (permalink / raw) To: Troy Kisky; +Cc: u-boot@lists.denx.de, sjg, festevam, marex, gary.bisson [-- Attachment #1: Type: text/plain, Size: 2327 bytes --] On Mon, Jan 30, 2023 at 10:51:03AM -0800, Troy Kisky wrote: > Hi Tom > > > On Mon, Jan 30, 2023 at 9:18 AM Tom Rini <trini@konsulko.com> wrote: > > > On Sat, Jan 28, 2023 at 09:25:54AM -0800, Troy Kisky wrote: > > > Thanks Tom, > > > > > > I cleaned up the PR based on the CI results. Here's my current changes. > > > > > > Author: Troy Kisky <troy.kisky@boundarydevices.com> > > > Date: Fri Jan 27 11:03:11 2023 -0800 > > > > > > dm: device-internal: use EVENT instead of DM_EVENT, because > > > event_notify is built for EVENT > > > > > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> > > > > > > diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h > > > index f31c4702086..2e725aa9416 100644 > > > --- a/include/dm/device-internal.h > > > +++ b/include/dm/device-internal.h > > > @@ -431,7 +431,7 @@ static inline void devres_release_all(struct udevice > > > *dev) > > > > > > static inline int device_notify(const struct udevice *dev, enum event_t > > > type) > > > { > > > -#if CONFIG_IS_ENABLED(DM_EVENT) > > > +#if CONFIG_IS_ENABLED(EVENT) > > > return event_notify(type, &dev, sizeof(dev)); > > > #else > > > return 0; > > > > Given 448e2b6327d0 ("event: Correct dependencies on the EVENT > > framework") I'm a little worried about this change here and want to be > > extra sure it doesn't break something inadvertently. > > > event_notify is in common/event, and the Makefile has > obj-$(CONFIG_$(SPL_TPL_)EVENT) += event.o > > So, the other option is to change the Makefile line to > obj-$(CONFIG_$(SPL_TPL_)DM_EVENT) += event.o > > I don't know which is best. Right, event_notify is part of the general event framework. The function above, device_notify, is part of DM_EVENT. This should probably be IS_ENABLED and not CONFIG_IS_ENABLED. > > Aside from that, > > are you able to post your series? > > > > I was hoping each of the maintainers could run the script and see if the > patches for their area make sense. > I don't need my sign-off on any of the patches. Ah. I'm not sure how likely that is to happen. > And incorporate your checking script > > in to .gitlab-ci.yml / .azure-pipline.yml ? > > > > I'll post a patch for that as an RFC OK. -- Tom [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 659 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-30 19:44 ` Tom Rini @ 2023-01-30 22:16 ` Troy Kisky 2023-01-31 14:16 ` Simon Glass 0 siblings, 1 reply; 17+ messages in thread From: Troy Kisky @ 2023-01-30 22:16 UTC (permalink / raw) To: Tom Rini; +Cc: u-boot@lists.denx.de, sjg, festevam, marex, gary.bisson On Mon, Jan 30, 2023 at 11:44 AM Tom Rini <trini@konsulko.com> wrote: > On Mon, Jan 30, 2023 at 10:51:03AM -0800, Troy Kisky wrote: > > Hi Tom > > > > > > On Mon, Jan 30, 2023 at 9:18 AM Tom Rini <trini@konsulko.com> wrote: > > > > > On Sat, Jan 28, 2023 at 09:25:54AM -0800, Troy Kisky wrote: > > > > Thanks Tom, > > > > > > > > I cleaned up the PR based on the CI results. Here's my current > changes. > > > > > > > > Author: Troy Kisky <troy.kisky@boundarydevices.com> > > > > Date: Fri Jan 27 11:03:11 2023 -0800 > > > > > > > > dm: device-internal: use EVENT instead of DM_EVENT, because > > > > event_notify is built for EVENT > > > > > > > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> > > > > > > > > diff --git a/include/dm/device-internal.h > b/include/dm/device-internal.h > > > > index f31c4702086..2e725aa9416 100644 > > > > --- a/include/dm/device-internal.h > > > > +++ b/include/dm/device-internal.h > > > > @@ -431,7 +431,7 @@ static inline void devres_release_all(struct > udevice > > > > *dev) > > > > > > > > static inline int device_notify(const struct udevice *dev, enum > event_t > > > > type) > > > > { > > > > -#if CONFIG_IS_ENABLED(DM_EVENT) > > > > +#if CONFIG_IS_ENABLED(EVENT) > > > > return event_notify(type, &dev, sizeof(dev)); > > > > #else > > > > return 0; > > > > > > Given 448e2b6327d0 ("event: Correct dependencies on the EVENT > > > framework") I'm a little worried about this change here and want to be > > > extra sure it doesn't break something inadvertently. > > > > > > event_notify is in common/event, and the Makefile has > > obj-$(CONFIG_$(SPL_TPL_)EVENT) += event.o > > > > So, the other option is to change the Makefile line to > > obj-$(CONFIG_$(SPL_TPL_)DM_EVENT) += event.o > > > > I don't know which is best. > > Right, event_notify is part of the general event framework. The function > above, device_notify, is part of DM_EVENT. This should probably be > IS_ENABLED and not CONFIG_IS_ENABLED. > > I think DM_EVENT should be globally replaced by EVENT. It doesn't seem to control much. Alternatively, I could add DM_EVENT to my SPL config list. changing to IS_ENABLED is what my script did before this patch, and caused CI to fail. Simon ? > > Aside from that, > > > are you able to post your series? > > > > > > > > I was hoping each of the maintainers could run the script and see if the > > patches for their area make sense. > > I don't need my sign-off on any of the patches. > > Ah. I'm not sure how likely that is to happen. > > > And incorporate your checking script > > > in to .gitlab-ci.yml / .azure-pipline.yml ? > > > > > > > I'll post a patch for that as an RFC > > OK. > > -- > Tom > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: CONFIG_IS_ENABLED vs IS_ENABLED 2023-01-30 22:16 ` Troy Kisky @ 2023-01-31 14:16 ` Simon Glass 0 siblings, 0 replies; 17+ messages in thread From: Simon Glass @ 2023-01-31 14:16 UTC (permalink / raw) To: Troy Kisky; +Cc: Tom Rini, u-boot@lists.denx.de, festevam, marex, gary.bisson Hi Troy, On Mon, 30 Jan 2023 at 15:17, Troy Kisky <troykiskyboundary@gmail.com> wrote: > > > > On Mon, Jan 30, 2023 at 11:44 AM Tom Rini <trini@konsulko.com> wrote: >> >> On Mon, Jan 30, 2023 at 10:51:03AM -0800, Troy Kisky wrote: >> > Hi Tom >> > >> > >> > On Mon, Jan 30, 2023 at 9:18 AM Tom Rini <trini@konsulko.com> wrote: >> > >> > > On Sat, Jan 28, 2023 at 09:25:54AM -0800, Troy Kisky wrote: >> > > > Thanks Tom, >> > > > >> > > > I cleaned up the PR based on the CI results. Here's my current changes. >> > > > >> > > > Author: Troy Kisky <troy.kisky@boundarydevices.com> >> > > > Date: Fri Jan 27 11:03:11 2023 -0800 >> > > > >> > > > dm: device-internal: use EVENT instead of DM_EVENT, because >> > > > event_notify is built for EVENT >> > > > >> > > > Signed-off-by: Troy Kisky <troy.kisky@boundarydevices.com> >> > > > >> > > > diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h >> > > > index f31c4702086..2e725aa9416 100644 >> > > > --- a/include/dm/device-internal.h >> > > > +++ b/include/dm/device-internal.h >> > > > @@ -431,7 +431,7 @@ static inline void devres_release_all(struct udevice >> > > > *dev) >> > > > >> > > > static inline int device_notify(const struct udevice *dev, enum event_t >> > > > type) >> > > > { >> > > > -#if CONFIG_IS_ENABLED(DM_EVENT) >> > > > +#if CONFIG_IS_ENABLED(EVENT) >> > > > return event_notify(type, &dev, sizeof(dev)); >> > > > #else >> > > > return 0; >> > > >> > > Given 448e2b6327d0 ("event: Correct dependencies on the EVENT >> > > framework") I'm a little worried about this change here and want to be >> > > extra sure it doesn't break something inadvertently. >> > >> > >> > event_notify is in common/event, and the Makefile has >> > obj-$(CONFIG_$(SPL_TPL_)EVENT) += event.o >> > >> > So, the other option is to change the Makefile line to >> > obj-$(CONFIG_$(SPL_TPL_)DM_EVENT) += event.o >> > >> > I don't know which is best. >> >> Right, event_notify is part of the general event framework. The function >> above, device_notify, is part of DM_EVENT. This should probably be >> IS_ENABLED and not CONFIG_IS_ENABLED. >> > > I think DM_EVENT should be globally replaced by EVENT. It doesn't seem to control much. > Alternatively, I could add DM_EVENT to my SPL config list. > changing to IS_ENABLED is what my script did before this patch, and caused CI to fail. > > > Simon ? DM_EVENT controls whether driver model sends out events via device_notify(). I don't see that this feature is widely enabled yet, so I don't think we should require it. However perhaps the usage will increase with time. > > >> > > Aside from that, >> > > are you able to post your series? >> > >> > >> > >> > I was hoping each of the maintainers could run the script and see if the >> > patches for their area make sense. >> > I don't need my sign-off on any of the patches. >> >> Ah. I'm not sure how likely that is to happen. >> >> > And incorporate your checking script >> > > in to .gitlab-ci.yml / .azure-pipline.yml ? >> > > >> > >> > I'll post a patch for that as an RFC >> >> OK. Regards, Simon ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-01-31 14:22 UTC | newest] Thread overview: 17+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-01-24 22:52 CONFIG_IS_ENABLED vs IS_ENABLED Troy Kisky 2023-01-24 23:31 ` Troy Kisky 2023-01-25 1:36 ` Simon Glass 2023-01-26 17:21 ` Tom Rini 2023-01-26 18:04 ` Simon Glass 2023-01-26 18:16 ` Tom Rini 2023-01-26 21:29 ` Simon Glass 2023-01-26 21:33 ` Tom Rini 2023-01-27 14:30 ` Simon Glass -- strict thread matches above, loose matches on Subject: below -- 2023-01-26 17:26 Troy Kisky 2023-01-26 17:34 ` Tom Rini 2023-01-28 17:25 ` Troy Kisky 2023-01-30 17:18 ` Tom Rini 2023-01-30 18:51 ` Troy Kisky 2023-01-30 19:44 ` Tom Rini 2023-01-30 22:16 ` Troy Kisky 2023-01-31 14:16 ` Simon Glass
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.