| Message ID | 20180204113712.8983-1-rosca.eugeniu@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Feb 4, 2018 at 12:37 PM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: > From: Eugeniu Rosca <erosca@de.adit-jv.com> > > Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" > readable") made an incredible improvement in how reverse dependencies > are perceived by the user, by breaking down the single (often > interminable) expression string into small readable chunks. > > Even so, what happens in practice when reading the reverse > dependencies is that 80-90% of the OR sub-expressions simply don't > matter, since they evaluate to [=n]. > > Assuming commit 617aebe6a97e ("Merge tag 'usercopy-v4.16-rc1' of > git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux"), ARCH=arm64 > and vanilla arm64 defconfig, here is the top 30 of CONFIG options with > the highest amount of OR sub-expressions that make up the final > "{Selected,Implied} by" reverse dependency expression. > > | Config | Revdep all | Revdep ![=n] | > |-------------------------------|------------|--------------| > | REGMAP_I2C | 212 | 9 | > | CRC32 | 167 | 25 | > | FW_LOADER | 128 | 5 | > | MFD_CORE | 124 | 9 | > | FB_CFB_IMAGEBLIT | 114 | 2 | > | FB_CFB_COPYAREA | 111 | 2 | > | FB_CFB_FILLRECT | 110 | 2 | > | SND_PCM | 103 | 2 | > | CRYPTO_HASH | 87 | 19 | > | WATCHDOG_CORE | 86 | 6 | > | IRQ_DOMAIN | 75 | 19 | > | SERIAL_CORE | 75 | 9 | > | PHYLIB | 74 | 16 | > | REGMAP_MMIO | 72 | 15 | > | GENERIC_PHY | 67 | 20 | > | DMA_ENGINE | 66 | 11 | > | SERIAL_CORE_CONSOLE | 64 | 9 | > | CRYPTO_BLKCIPHER | 64 | 13 | > | PINMUX | 60 | 17 | > | CRYPTO | 59 | 10 | > | MII | 58 | 8 | > | GPIOLIB_IRQCHIP | 58 | 9 | > | MFD_SYSCON | 58 | 15 | > | VIDEOBUF2_DMA_CONTIG | 46 | 4 | > | REGMAP_IRQ | 45 | 6 | > | REGMAP_SPI | 44 | 2 | > | CLKSRC_MMIO | 42 | 5 | > | SND_SOC_GENERIC_DMAENGINE_PCM | 41 | 3 | > | CRYPTO_SHA1 | 37 | 2 | > | REGMAP | 36 | 4 | > > The story behind the above is that we still need to visually > review/evaluate 212 expressions which *potentially* select REGMAP_I2C > in order to identify the expressions which *actually* select > REGMAP_I2C, for a particular ARCH and for a particular defconfig used. > > This patch attempts to bring at user's fingertips those reverse > dependencies that actually participate in selection of given symbol > filtering out the rest of them. > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > --- > scripts/kconfig/expr.c | 24 +++++++++++++++++------- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c > index 2ba332b3fed7..147b2d8a8f3e 100644 > --- a/scripts/kconfig/expr.c > +++ b/scripts/kconfig/expr.c > @@ -1234,14 +1234,24 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con > fn(data, e->right.sym, e->right.sym->name); > break; > case E_OR: > - if (revdep && e->left.expr->type != E_OR) > - fn(data, NULL, "\n - "); > - __expr_print(e->left.expr, fn, data, E_OR, revdep); > - if (revdep) > - fn(data, NULL, "\n - "); > - else > + if (revdep) { > + struct expr *left = e->left.expr; > + struct expr *right = e->right.expr; > + > + if (expr_calc_value(left) != no) { > + if (left->type != E_OR) > + fn(data, NULL, "\n - "); > + __expr_print(left, fn, data, E_OR, revdep); > + } > + if (expr_calc_value(right) != no) { > + fn(data, NULL, "\n - "); > + __expr_print(right, fn, data, E_OR, revdep); > + } > + } else { > + __expr_print(e->left.expr, fn, data, E_OR, revdep); > fn(data, NULL, " || "); > - __expr_print(e->right.expr, fn, data, E_OR, revdep); > + __expr_print(e->right.expr, fn, data, E_OR, revdep); > + } > break; > case E_AND: > expr_print(e->left.expr, fn, data, E_AND); > -- > 2.16.1 > Hello, One downside to this is that people might expect e.g. the '?' menuconfig screen to list all the selecting symbols and use it as a reference. The best solution IMO would be to have a separate "Currently selected by:" section on that screen, listing just the non-n selects. The simpler next best thing would be to just replace the "Selected by:" heading with "Currently selected by:", to make it clear that it includes just the active selects. For the most-selected symbols you listed, most of them end up as "m" on my system by the way, because they come from drivers compiled in as modules. "n" is the minority. Might want to check that most of the ones with a million selects aren't like that, because it might not be that hard to see what's going on for those anyway. I used a similar approach in https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L3022 by the way. I was always a bit worried that all the expression simplification shenanigans going on in the C implementation might mess with an approach like that, but it seems fine in practice. :) Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 4, 2018 at 3:46 PM, Ulf Magnusson <ulfalizer@gmail.com> wrote: > On Sun, Feb 4, 2018 at 12:37 PM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: >> From: Eugeniu Rosca <erosca@de.adit-jv.com> >> >> Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" >> readable") made an incredible improvement in how reverse dependencies >> are perceived by the user, by breaking down the single (often >> interminable) expression string into small readable chunks. >> >> Even so, what happens in practice when reading the reverse >> dependencies is that 80-90% of the OR sub-expressions simply don't >> matter, since they evaluate to [=n]. >> >> Assuming commit 617aebe6a97e ("Merge tag 'usercopy-v4.16-rc1' of >> git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux"), ARCH=arm64 >> and vanilla arm64 defconfig, here is the top 30 of CONFIG options with >> the highest amount of OR sub-expressions that make up the final >> "{Selected,Implied} by" reverse dependency expression. >> >> | Config | Revdep all | Revdep ![=n] | >> |-------------------------------|------------|--------------| >> | REGMAP_I2C | 212 | 9 | >> | CRC32 | 167 | 25 | >> | FW_LOADER | 128 | 5 | >> | MFD_CORE | 124 | 9 | >> | FB_CFB_IMAGEBLIT | 114 | 2 | >> | FB_CFB_COPYAREA | 111 | 2 | >> | FB_CFB_FILLRECT | 110 | 2 | >> | SND_PCM | 103 | 2 | >> | CRYPTO_HASH | 87 | 19 | >> | WATCHDOG_CORE | 86 | 6 | >> | IRQ_DOMAIN | 75 | 19 | >> | SERIAL_CORE | 75 | 9 | >> | PHYLIB | 74 | 16 | >> | REGMAP_MMIO | 72 | 15 | >> | GENERIC_PHY | 67 | 20 | >> | DMA_ENGINE | 66 | 11 | >> | SERIAL_CORE_CONSOLE | 64 | 9 | >> | CRYPTO_BLKCIPHER | 64 | 13 | >> | PINMUX | 60 | 17 | >> | CRYPTO | 59 | 10 | >> | MII | 58 | 8 | >> | GPIOLIB_IRQCHIP | 58 | 9 | >> | MFD_SYSCON | 58 | 15 | >> | VIDEOBUF2_DMA_CONTIG | 46 | 4 | >> | REGMAP_IRQ | 45 | 6 | >> | REGMAP_SPI | 44 | 2 | >> | CLKSRC_MMIO | 42 | 5 | >> | SND_SOC_GENERIC_DMAENGINE_PCM | 41 | 3 | >> | CRYPTO_SHA1 | 37 | 2 | >> | REGMAP | 36 | 4 | >> >> The story behind the above is that we still need to visually >> review/evaluate 212 expressions which *potentially* select REGMAP_I2C >> in order to identify the expressions which *actually* select >> REGMAP_I2C, for a particular ARCH and for a particular defconfig used. >> >> This patch attempts to bring at user's fingertips those reverse >> dependencies that actually participate in selection of given symbol >> filtering out the rest of them. >> >> Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >> --- >> scripts/kconfig/expr.c | 24 +++++++++++++++++------- >> 1 file changed, 17 insertions(+), 7 deletions(-) >> >> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c >> index 2ba332b3fed7..147b2d8a8f3e 100644 >> --- a/scripts/kconfig/expr.c >> +++ b/scripts/kconfig/expr.c >> @@ -1234,14 +1234,24 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con >> fn(data, e->right.sym, e->right.sym->name); >> break; >> case E_OR: >> - if (revdep && e->left.expr->type != E_OR) >> - fn(data, NULL, "\n - "); >> - __expr_print(e->left.expr, fn, data, E_OR, revdep); >> - if (revdep) >> - fn(data, NULL, "\n - "); >> - else >> + if (revdep) { >> + struct expr *left = e->left.expr; >> + struct expr *right = e->right.expr; >> + >> + if (expr_calc_value(left) != no) { >> + if (left->type != E_OR) >> + fn(data, NULL, "\n - "); >> + __expr_print(left, fn, data, E_OR, revdep); >> + } >> + if (expr_calc_value(right) != no) { >> + fn(data, NULL, "\n - "); >> + __expr_print(right, fn, data, E_OR, revdep); >> + } >> + } else { >> + __expr_print(e->left.expr, fn, data, E_OR, revdep); >> fn(data, NULL, " || "); >> - __expr_print(e->right.expr, fn, data, E_OR, revdep); >> + __expr_print(e->right.expr, fn, data, E_OR, revdep); >> + } >> break; >> case E_AND: >> expr_print(e->left.expr, fn, data, E_AND); >> -- >> 2.16.1 >> > > Hello, > > One downside to this is that people might expect e.g. the '?' > menuconfig screen to list all the selecting symbols and use it as a > reference. > > The best solution IMO would be to have a separate "Currently selected > by:" section on that screen, listing just the non-n selects. The > simpler next best thing would be to just replace the "Selected by:" > heading with "Currently selected by:", to make it clear that it > includes just the active selects. > > For the most-selected symbols you listed, most of them end up as "m" > on my system by the way, because they come from drivers compiled in as > modules. Most of the selecting symbols that is. > "n" is the minority. Might want to check that most of the > ones with a million selects aren't like that, because it might not be > that hard to see what's going on for those anyway. > > I used a similar approach in > https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L3022 > by the way. I was always a bit worried that all the expression > simplification shenanigans going on in the C implementation might mess > with an approach like that, but it seems fine in practice. :) > > Cheers, > Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Eugeniu, > From: Eugeniu Rosca <erosca@de.adit-jv.com> > Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" > readable") made an incredible improvement in how reverse dependencies > are perceived by the user, by breaking down the single (often > interminable) expression string into small readable chunks. > Even so, what happens in practice when reading the reverse > dependencies is that 80-90% of the OR sub-expressions simply don't > matter, since they evaluate to [=n]. > Assuming commit 617aebe6a97e ("Merge tag 'usercopy-v4.16-rc1' of > git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux"), ARCH=arm64 > and vanilla arm64 defconfig, here is the top 30 of CONFIG options with > the highest amount of OR sub-expressions that make up the final > "{Selected,Implied} by" reverse dependency expression. > | Config | Revdep all | Revdep ![=n] | > |-------------------------------|------------|--------------| > | REGMAP_I2C | 212 | 9 | > | CRC32 | 167 | 25 | > | FW_LOADER | 128 | 5 | > | MFD_CORE | 124 | 9 | > | FB_CFB_IMAGEBLIT | 114 | 2 | > | FB_CFB_COPYAREA | 111 | 2 | > | FB_CFB_FILLRECT | 110 | 2 | > | SND_PCM | 103 | 2 | > | CRYPTO_HASH | 87 | 19 | > | WATCHDOG_CORE | 86 | 6 | > | IRQ_DOMAIN | 75 | 19 | > | SERIAL_CORE | 75 | 9 | > | PHYLIB | 74 | 16 | > | REGMAP_MMIO | 72 | 15 | > | GENERIC_PHY | 67 | 20 | > | DMA_ENGINE | 66 | 11 | > | SERIAL_CORE_CONSOLE | 64 | 9 | > | CRYPTO_BLKCIPHER | 64 | 13 | > | PINMUX | 60 | 17 | > | CRYPTO | 59 | 10 | > | MII | 58 | 8 | > | GPIOLIB_IRQCHIP | 58 | 9 | > | MFD_SYSCON | 58 | 15 | > | VIDEOBUF2_DMA_CONTIG | 46 | 4 | > | REGMAP_IRQ | 45 | 6 | > | REGMAP_SPI | 44 | 2 | > | CLKSRC_MMIO | 42 | 5 | > | SND_SOC_GENERIC_DMAENGINE_PCM | 41 | 3 | > | CRYPTO_SHA1 | 37 | 2 | > | REGMAP | 36 | 4 | > The story behind the above is that we still need to visually > review/evaluate 212 expressions which *potentially* select REGMAP_I2C > in order to identify the expressions which *actually* select > REGMAP_I2C, for a particular ARCH and for a particular defconfig used. > This patch attempts to bring at user's fingertips those reverse > dependencies that actually participate in selection of given symbol > filtering out the rest of them. > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > --- > scripts/kconfig/expr.c | 24 +++++++++++++++++------- > 1 file changed, 17 insertions(+), 7 deletions(-) > diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c > index 2ba332b3fed7..147b2d8a8f3e 100644 > --- a/scripts/kconfig/expr.c > +++ b/scripts/kconfig/expr.c > @@ -1234,14 +1234,24 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con > fn(data, e->right.sym, e->right.sym->name); > break; > case E_OR: > - if (revdep && e->left.expr->type != E_OR) > - fn(data, NULL, "\n - "); > - __expr_print(e->left.expr, fn, data, E_OR, revdep); > - if (revdep) > - fn(data, NULL, "\n - "); > - else > + if (revdep) { > + struct expr *left = e->left.expr; > + struct expr *right = e->right.expr; > + > + if (expr_calc_value(left) != no) { > + if (left->type != E_OR) > + fn(data, NULL, "\n - "); > + __expr_print(left, fn, data, E_OR, revdep); > + } > + if (expr_calc_value(right) != no) { > + fn(data, NULL, "\n - "); > + __expr_print(right, fn, data, E_OR, revdep); > + } > + } else { > + __expr_print(e->left.expr, fn, data, E_OR, revdep); > fn(data, NULL, " || "); > - __expr_print(e->right.expr, fn, data, E_OR, revdep); > + __expr_print(e->right.expr, fn, data, E_OR, revdep); > + } > break; > case E_AND: > expr_print(e->left.expr, fn, data, E_AND); Thanks for trying to improve my change. Applying your patch, running $ ARCH=arm64 make defconfig && ARCH=arm64 make menuconfig and searching for USB prints "Selected by:" with nothing actually selected. Kind regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Ulf, On Sun, Feb 04, 2018 at 03:46:00PM +0100, Ulf Magnusson wrote: > On Sun, Feb 4, 2018 at 12:37 PM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: > > From: Eugeniu Rosca <erosca@de.adit-jv.com> > > > > Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" > > readable") made an incredible improvement in how reverse dependencies > > are perceived by the user, by breaking down the single (often > > interminable) expression string into small readable chunks. > > > > Even so, what happens in practice when reading the reverse > > dependencies is that 80-90% of the OR sub-expressions simply don't > > matter, since they evaluate to [=n]. > > > > Assuming commit 617aebe6a97e ("Merge tag 'usercopy-v4.16-rc1' of > > git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux"), ARCH=arm64 > > and vanilla arm64 defconfig, here is the top 30 of CONFIG options with > > the highest amount of OR sub-expressions that make up the final > > "{Selected,Implied} by" reverse dependency expression. > > > > | Config | Revdep all | Revdep ![=n] | > > |-------------------------------|------------|--------------| > > | REGMAP_I2C | 212 | 9 | > > | CRC32 | 167 | 25 | > > | FW_LOADER | 128 | 5 | > > | MFD_CORE | 124 | 9 | > > | FB_CFB_IMAGEBLIT | 114 | 2 | > > | FB_CFB_COPYAREA | 111 | 2 | > > | FB_CFB_FILLRECT | 110 | 2 | > > | SND_PCM | 103 | 2 | > > | CRYPTO_HASH | 87 | 19 | > > | WATCHDOG_CORE | 86 | 6 | > > | IRQ_DOMAIN | 75 | 19 | > > | SERIAL_CORE | 75 | 9 | > > | PHYLIB | 74 | 16 | > > | REGMAP_MMIO | 72 | 15 | > > | GENERIC_PHY | 67 | 20 | > > | DMA_ENGINE | 66 | 11 | > > | SERIAL_CORE_CONSOLE | 64 | 9 | > > | CRYPTO_BLKCIPHER | 64 | 13 | > > | PINMUX | 60 | 17 | > > | CRYPTO | 59 | 10 | > > | MII | 58 | 8 | > > | GPIOLIB_IRQCHIP | 58 | 9 | > > | MFD_SYSCON | 58 | 15 | > > | VIDEOBUF2_DMA_CONTIG | 46 | 4 | > > | REGMAP_IRQ | 45 | 6 | > > | REGMAP_SPI | 44 | 2 | > > | CLKSRC_MMIO | 42 | 5 | > > | SND_SOC_GENERIC_DMAENGINE_PCM | 41 | 3 | > > | CRYPTO_SHA1 | 37 | 2 | > > | REGMAP | 36 | 4 | > > > > The story behind the above is that we still need to visually > > review/evaluate 212 expressions which *potentially* select REGMAP_I2C > > in order to identify the expressions which *actually* select > > REGMAP_I2C, for a particular ARCH and for a particular defconfig used. > > > > This patch attempts to bring at user's fingertips those reverse > > dependencies that actually participate in selection of given symbol > > filtering out the rest of them. > > > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > --- > > scripts/kconfig/expr.c | 24 +++++++++++++++++------- > > 1 file changed, 17 insertions(+), 7 deletions(-) > > > > diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c > > index 2ba332b3fed7..147b2d8a8f3e 100644 > > --- a/scripts/kconfig/expr.c > > +++ b/scripts/kconfig/expr.c > > @@ -1234,14 +1234,24 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con > > fn(data, e->right.sym, e->right.sym->name); > > break; > > case E_OR: > > - if (revdep && e->left.expr->type != E_OR) > > - fn(data, NULL, "\n - "); > > - __expr_print(e->left.expr, fn, data, E_OR, revdep); > > - if (revdep) > > - fn(data, NULL, "\n - "); > > - else > > + if (revdep) { > > + struct expr *left = e->left.expr; > > + struct expr *right = e->right.expr; > > + > > + if (expr_calc_value(left) != no) { > > + if (left->type != E_OR) > > + fn(data, NULL, "\n - "); > > + __expr_print(left, fn, data, E_OR, revdep); > > + } > > + if (expr_calc_value(right) != no) { > > + fn(data, NULL, "\n - "); > > + __expr_print(right, fn, data, E_OR, revdep); > > + } > > + } else { > > + __expr_print(e->left.expr, fn, data, E_OR, revdep); > > fn(data, NULL, " || "); > > - __expr_print(e->right.expr, fn, data, E_OR, revdep); > > + __expr_print(e->right.expr, fn, data, E_OR, revdep); > > + } > > break; > > case E_AND: > > expr_print(e->left.expr, fn, data, E_AND); > > -- > > 2.16.1 > > > > Hello, > > One downside to this is that people might expect e.g. the '?' > menuconfig screen to list all the selecting symbols and use it as a > reference. Agreed. See my proposals below. > The best solution IMO would be to have a separate "Currently selected > by:" section on that screen, listing just the non-n selects. The > simpler next best thing would be to just replace the "Selected by:" > heading with "Currently selected by:", to make it clear that it > includes just the active selects. One certain thing is that with below two categories, some reverse dependencies would be printed twice: - "Currently selected by" - showing non-n expressions. - "Selected by" - showing both non-n and n expressions. To avoid the duplicates, I would think about (naming could be improved): - "Actively selected by" or "Currently selected by" - "Inactively selected by" or "Passively selected by" - "Actively implied by" or "Currently implied by" - "Inactively implied by" or "Passively implied by" I do believe that before proceeding with any further alternative implementations, we better first agree that the above way to print the reverse dependencies is fine for everybody. > For the most-selected symbols you listed, most of them end up as "m" > on my system by the way, because they come from drivers compiled in as > modules. "n" is the minority. Might want to check that most of the > ones with a million selects aren't like that, because it might not be > that hard to see what's going on for those anyway. Replying specifically to your `it might not be that hard to see what's going on for those anyway`, I do believe that for certain configs it is a pain to visually identify the meaningful (i.e. non-n) reverse dependencies when there are tens or hundreds of them. Getting this information directly from Kbuild (instead of computing it externally, either by hand or scripted) was my main motivation and driving factor behind sharing the patch. > I used a similar approach in > https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L3022 > by the way. I was always a bit worried that all the expression > simplification shenanigans going on in the C implementation might mess > with an approach like that, but it seems fine in practice. :) Can't wait to put my hands on Kconfiglib. Thanks for sharing! > Cheers, > Ulf Best regards, Eugeniu. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello Petr, On Sun, Feb 04, 2018 at 04:50:54PM +0100, Petr Vorel wrote: > Hi Eugeniu, > > > From: Eugeniu Rosca <erosca@de.adit-jv.com> > > > Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" > > readable") made an incredible improvement in how reverse dependencies > > are perceived by the user, by breaking down the single (often > > interminable) expression string into small readable chunks. > > > Even so, what happens in practice when reading the reverse > > dependencies is that 80-90% of the OR sub-expressions simply don't > > matter, since they evaluate to [=n]. > > > Assuming commit 617aebe6a97e ("Merge tag 'usercopy-v4.16-rc1' of > > git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux"), ARCH=arm64 > > and vanilla arm64 defconfig, here is the top 30 of CONFIG options with > > the highest amount of OR sub-expressions that make up the final > > "{Selected,Implied} by" reverse dependency expression. > > > | Config | Revdep all | Revdep ![=n] | > > |-------------------------------|------------|--------------| > > | REGMAP_I2C | 212 | 9 | > > | CRC32 | 167 | 25 | > > | FW_LOADER | 128 | 5 | > > | MFD_CORE | 124 | 9 | > > | FB_CFB_IMAGEBLIT | 114 | 2 | > > | FB_CFB_COPYAREA | 111 | 2 | > > | FB_CFB_FILLRECT | 110 | 2 | > > | SND_PCM | 103 | 2 | > > | CRYPTO_HASH | 87 | 19 | > > | WATCHDOG_CORE | 86 | 6 | > > | IRQ_DOMAIN | 75 | 19 | > > | SERIAL_CORE | 75 | 9 | > > | PHYLIB | 74 | 16 | > > | REGMAP_MMIO | 72 | 15 | > > | GENERIC_PHY | 67 | 20 | > > | DMA_ENGINE | 66 | 11 | > > | SERIAL_CORE_CONSOLE | 64 | 9 | > > | CRYPTO_BLKCIPHER | 64 | 13 | > > | PINMUX | 60 | 17 | > > | CRYPTO | 59 | 10 | > > | MII | 58 | 8 | > > | GPIOLIB_IRQCHIP | 58 | 9 | > > | MFD_SYSCON | 58 | 15 | > > | VIDEOBUF2_DMA_CONTIG | 46 | 4 | > > | REGMAP_IRQ | 45 | 6 | > > | REGMAP_SPI | 44 | 2 | > > | CLKSRC_MMIO | 42 | 5 | > > | SND_SOC_GENERIC_DMAENGINE_PCM | 41 | 3 | > > | CRYPTO_SHA1 | 37 | 2 | > > | REGMAP | 36 | 4 | > > > The story behind the above is that we still need to visually > > review/evaluate 212 expressions which *potentially* select REGMAP_I2C > > in order to identify the expressions which *actually* select > > REGMAP_I2C, for a particular ARCH and for a particular defconfig used. > > > This patch attempts to bring at user's fingertips those reverse > > dependencies that actually participate in selection of given symbol > > filtering out the rest of them. > > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > > --- > > scripts/kconfig/expr.c | 24 +++++++++++++++++------- > > 1 file changed, 17 insertions(+), 7 deletions(-) > > > diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c > > index 2ba332b3fed7..147b2d8a8f3e 100644 > > --- a/scripts/kconfig/expr.c > > +++ b/scripts/kconfig/expr.c > > @@ -1234,14 +1234,24 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con > > fn(data, e->right.sym, e->right.sym->name); > > break; > > case E_OR: > > - if (revdep && e->left.expr->type != E_OR) > > - fn(data, NULL, "\n - "); > > - __expr_print(e->left.expr, fn, data, E_OR, revdep); > > - if (revdep) > > - fn(data, NULL, "\n - "); > > - else > > + if (revdep) { > > + struct expr *left = e->left.expr; > > + struct expr *right = e->right.expr; > > + > > + if (expr_calc_value(left) != no) { > > + if (left->type != E_OR) > > + fn(data, NULL, "\n - "); > > + __expr_print(left, fn, data, E_OR, revdep); > > + } > > + if (expr_calc_value(right) != no) { > > + fn(data, NULL, "\n - "); > > + __expr_print(right, fn, data, E_OR, revdep); > > + } > > + } else { > > + __expr_print(e->left.expr, fn, data, E_OR, revdep); > > fn(data, NULL, " || "); > > - __expr_print(e->right.expr, fn, data, E_OR, revdep); > > + __expr_print(e->right.expr, fn, data, E_OR, revdep); > > + } > > break; > > case E_AND: > > expr_print(e->left.expr, fn, data, E_AND); > > Thanks for trying to improve my change. > > Applying your patch, running > $ ARCH=arm64 make defconfig && ARCH=arm64 make menuconfig > and searching for USB prints "Selected by:" with nothing actually selected. I think the behavior is correct. All expressions that select USB translate/evaluate to =n. CONFIG_USB is enabled in the arm64 defconfig. > Kind regards, > Petr Thanks, Eugeniu. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Feb 4, 2018 at 10:49 PM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: > Hi Ulf, > > On Sun, Feb 04, 2018 at 03:46:00PM +0100, Ulf Magnusson wrote: >> On Sun, Feb 4, 2018 at 12:37 PM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: >> > From: Eugeniu Rosca <erosca@de.adit-jv.com> >> > >> > Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" >> > readable") made an incredible improvement in how reverse dependencies >> > are perceived by the user, by breaking down the single (often >> > interminable) expression string into small readable chunks. >> > >> > Even so, what happens in practice when reading the reverse >> > dependencies is that 80-90% of the OR sub-expressions simply don't >> > matter, since they evaluate to [=n]. >> > >> > Assuming commit 617aebe6a97e ("Merge tag 'usercopy-v4.16-rc1' of >> > git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux"), ARCH=arm64 >> > and vanilla arm64 defconfig, here is the top 30 of CONFIG options with >> > the highest amount of OR sub-expressions that make up the final >> > "{Selected,Implied} by" reverse dependency expression. >> > >> > | Config | Revdep all | Revdep ![=n] | >> > |-------------------------------|------------|--------------| >> > | REGMAP_I2C | 212 | 9 | >> > | CRC32 | 167 | 25 | >> > | FW_LOADER | 128 | 5 | >> > | MFD_CORE | 124 | 9 | >> > | FB_CFB_IMAGEBLIT | 114 | 2 | >> > | FB_CFB_COPYAREA | 111 | 2 | >> > | FB_CFB_FILLRECT | 110 | 2 | >> > | SND_PCM | 103 | 2 | >> > | CRYPTO_HASH | 87 | 19 | >> > | WATCHDOG_CORE | 86 | 6 | >> > | IRQ_DOMAIN | 75 | 19 | >> > | SERIAL_CORE | 75 | 9 | >> > | PHYLIB | 74 | 16 | >> > | REGMAP_MMIO | 72 | 15 | >> > | GENERIC_PHY | 67 | 20 | >> > | DMA_ENGINE | 66 | 11 | >> > | SERIAL_CORE_CONSOLE | 64 | 9 | >> > | CRYPTO_BLKCIPHER | 64 | 13 | >> > | PINMUX | 60 | 17 | >> > | CRYPTO | 59 | 10 | >> > | MII | 58 | 8 | >> > | GPIOLIB_IRQCHIP | 58 | 9 | >> > | MFD_SYSCON | 58 | 15 | >> > | VIDEOBUF2_DMA_CONTIG | 46 | 4 | >> > | REGMAP_IRQ | 45 | 6 | >> > | REGMAP_SPI | 44 | 2 | >> > | CLKSRC_MMIO | 42 | 5 | >> > | SND_SOC_GENERIC_DMAENGINE_PCM | 41 | 3 | >> > | CRYPTO_SHA1 | 37 | 2 | >> > | REGMAP | 36 | 4 | >> > >> > The story behind the above is that we still need to visually >> > review/evaluate 212 expressions which *potentially* select REGMAP_I2C >> > in order to identify the expressions which *actually* select >> > REGMAP_I2C, for a particular ARCH and for a particular defconfig used. >> > >> > This patch attempts to bring at user's fingertips those reverse >> > dependencies that actually participate in selection of given symbol >> > filtering out the rest of them. >> > >> > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >> > --- >> > scripts/kconfig/expr.c | 24 +++++++++++++++++------- >> > 1 file changed, 17 insertions(+), 7 deletions(-) >> > >> > diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c >> > index 2ba332b3fed7..147b2d8a8f3e 100644 >> > --- a/scripts/kconfig/expr.c >> > +++ b/scripts/kconfig/expr.c >> > @@ -1234,14 +1234,24 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con >> > fn(data, e->right.sym, e->right.sym->name); >> > break; >> > case E_OR: >> > - if (revdep && e->left.expr->type != E_OR) >> > - fn(data, NULL, "\n - "); >> > - __expr_print(e->left.expr, fn, data, E_OR, revdep); >> > - if (revdep) >> > - fn(data, NULL, "\n - "); >> > - else >> > + if (revdep) { >> > + struct expr *left = e->left.expr; >> > + struct expr *right = e->right.expr; >> > + >> > + if (expr_calc_value(left) != no) { >> > + if (left->type != E_OR) >> > + fn(data, NULL, "\n - "); >> > + __expr_print(left, fn, data, E_OR, revdep); >> > + } >> > + if (expr_calc_value(right) != no) { >> > + fn(data, NULL, "\n - "); >> > + __expr_print(right, fn, data, E_OR, revdep); >> > + } >> > + } else { >> > + __expr_print(e->left.expr, fn, data, E_OR, revdep); >> > fn(data, NULL, " || "); >> > - __expr_print(e->right.expr, fn, data, E_OR, revdep); >> > + __expr_print(e->right.expr, fn, data, E_OR, revdep); >> > + } >> > break; >> > case E_AND: >> > expr_print(e->left.expr, fn, data, E_AND); >> > -- >> > 2.16.1 >> > >> >> Hello, >> >> One downside to this is that people might expect e.g. the '?' >> menuconfig screen to list all the selecting symbols and use it as a >> reference. > > Agreed. See my proposals below. > >> The best solution IMO would be to have a separate "Currently selected >> by:" section on that screen, listing just the non-n selects. The >> simpler next best thing would be to just replace the "Selected by:" >> heading with "Currently selected by:", to make it clear that it >> includes just the active selects. > > One certain thing is that with below two categories, some reverse > dependencies would be printed twice: > - "Currently selected by" - showing non-n expressions. > - "Selected by" - showing both non-n and n expressions. > > To avoid the duplicates, I would think about (naming could be improved): > - "Actively selected by" or "Currently selected by" > - "Inactively selected by" or "Passively selected by" > - "Actively implied by" or "Currently implied by" > - "Inactively implied by" or "Passively implied by" > > I do believe that before proceeding with any further alternative > implementations, we better first agree that the above way to print the > reverse dependencies is fine for everybody. > Looks good to me. Could go with something like "Current active (m/y) selects" and "Current inactive (n) selects" maybe, to make it super clear. >> For the most-selected symbols you listed, most of them end up as "m" >> on my system by the way, because they come from drivers compiled in as >> modules. "n" is the minority. Might want to check that most of the >> ones with a million selects aren't like that, because it might not be >> that hard to see what's going on for those anyway. > > Replying specifically to your `it might not be that hard to see what's > going on for those anyway`, I do believe that for certain configs > it is a pain to visually identify the meaningful (i.e. > non-n) reverse dependencies when there are tens or hundreds of them. > Getting this information directly from Kbuild (instead of computing it > externally, either by hand or scripted) was my main motivation and > driving factor behind sharing the patch. Consider that a before-coffee comment. It's clearly pretty helpful even for those "obvious" cases. I had missed those ARM stats. :) > >> I used a similar approach in >> https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L3022 >> by the way. I was always a bit worried that all the expression >> simplification shenanigans going on in the C implementation might mess >> with an approach like that, but it seems fine in practice. :) > > Can't wait to put my hands on Kconfiglib. Thanks for sharing! > Might gain some features that make it viable as a full C implementation replacement soon, if some stuff pans out. It's always been more of an auxiliary library. Apparently the C implementation is a bit of a PITA on Windows. >> Cheers, >> Ulf > > Best regards, > Eugeniu. Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2018-02-05 7:12 GMT+09:00 Ulf Magnusson <ulfalizer@gmail.com>: > On Sun, Feb 4, 2018 at 10:49 PM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: >> Hi Ulf, >> >> On Sun, Feb 04, 2018 at 03:46:00PM +0100, Ulf Magnusson wrote: >>> On Sun, Feb 4, 2018 at 12:37 PM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: >>> > From: Eugeniu Rosca <erosca@de.adit-jv.com> >>> > >>> > Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" >>> > readable") made an incredible improvement in how reverse dependencies >>> > are perceived by the user, by breaking down the single (often >>> > interminable) expression string into small readable chunks. >>> > >>> > Even so, what happens in practice when reading the reverse >>> > dependencies is that 80-90% of the OR sub-expressions simply don't >>> > matter, since they evaluate to [=n]. >>> > >>> > Assuming commit 617aebe6a97e ("Merge tag 'usercopy-v4.16-rc1' of >>> > git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux"), ARCH=arm64 >>> > and vanilla arm64 defconfig, here is the top 30 of CONFIG options with >>> > the highest amount of OR sub-expressions that make up the final >>> > "{Selected,Implied} by" reverse dependency expression. >>> > >>> > | Config | Revdep all | Revdep ![=n] | >>> > |-------------------------------|------------|--------------| >>> > | REGMAP_I2C | 212 | 9 | >>> > | CRC32 | 167 | 25 | >>> > | FW_LOADER | 128 | 5 | >>> > | MFD_CORE | 124 | 9 | >>> > | FB_CFB_IMAGEBLIT | 114 | 2 | >>> > | FB_CFB_COPYAREA | 111 | 2 | >>> > | FB_CFB_FILLRECT | 110 | 2 | >>> > | SND_PCM | 103 | 2 | >>> > | CRYPTO_HASH | 87 | 19 | >>> > | WATCHDOG_CORE | 86 | 6 | >>> > | IRQ_DOMAIN | 75 | 19 | >>> > | SERIAL_CORE | 75 | 9 | >>> > | PHYLIB | 74 | 16 | >>> > | REGMAP_MMIO | 72 | 15 | >>> > | GENERIC_PHY | 67 | 20 | >>> > | DMA_ENGINE | 66 | 11 | >>> > | SERIAL_CORE_CONSOLE | 64 | 9 | >>> > | CRYPTO_BLKCIPHER | 64 | 13 | >>> > | PINMUX | 60 | 17 | >>> > | CRYPTO | 59 | 10 | >>> > | MII | 58 | 8 | >>> > | GPIOLIB_IRQCHIP | 58 | 9 | >>> > | MFD_SYSCON | 58 | 15 | >>> > | VIDEOBUF2_DMA_CONTIG | 46 | 4 | >>> > | REGMAP_IRQ | 45 | 6 | >>> > | REGMAP_SPI | 44 | 2 | >>> > | CLKSRC_MMIO | 42 | 5 | >>> > | SND_SOC_GENERIC_DMAENGINE_PCM | 41 | 3 | >>> > | CRYPTO_SHA1 | 37 | 2 | >>> > | REGMAP | 36 | 4 | >>> > >>> > The story behind the above is that we still need to visually >>> > review/evaluate 212 expressions which *potentially* select REGMAP_I2C >>> > in order to identify the expressions which *actually* select >>> > REGMAP_I2C, for a particular ARCH and for a particular defconfig used. >>> > >>> > This patch attempts to bring at user's fingertips those reverse >>> > dependencies that actually participate in selection of given symbol >>> > filtering out the rest of them. >>> > >>> > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> >>> > --- >>> > scripts/kconfig/expr.c | 24 +++++++++++++++++------- >>> > 1 file changed, 17 insertions(+), 7 deletions(-) >>> > >>> > diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c >>> > index 2ba332b3fed7..147b2d8a8f3e 100644 >>> > --- a/scripts/kconfig/expr.c >>> > +++ b/scripts/kconfig/expr.c >>> > @@ -1234,14 +1234,24 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con >>> > fn(data, e->right.sym, e->right.sym->name); >>> > break; >>> > case E_OR: >>> > - if (revdep && e->left.expr->type != E_OR) >>> > - fn(data, NULL, "\n - "); >>> > - __expr_print(e->left.expr, fn, data, E_OR, revdep); >>> > - if (revdep) >>> > - fn(data, NULL, "\n - "); >>> > - else >>> > + if (revdep) { >>> > + struct expr *left = e->left.expr; >>> > + struct expr *right = e->right.expr; >>> > + >>> > + if (expr_calc_value(left) != no) { >>> > + if (left->type != E_OR) >>> > + fn(data, NULL, "\n - "); >>> > + __expr_print(left, fn, data, E_OR, revdep); >>> > + } >>> > + if (expr_calc_value(right) != no) { >>> > + fn(data, NULL, "\n - "); >>> > + __expr_print(right, fn, data, E_OR, revdep); >>> > + } >>> > + } else { >>> > + __expr_print(e->left.expr, fn, data, E_OR, revdep); >>> > fn(data, NULL, " || "); >>> > - __expr_print(e->right.expr, fn, data, E_OR, revdep); >>> > + __expr_print(e->right.expr, fn, data, E_OR, revdep); >>> > + } >>> > break; >>> > case E_AND: >>> > expr_print(e->left.expr, fn, data, E_AND); >>> > -- >>> > 2.16.1 >>> > >>> >>> Hello, >>> >>> One downside to this is that people might expect e.g. the '?' >>> menuconfig screen to list all the selecting symbols and use it as a >>> reference. >> >> Agreed. See my proposals below. >> >>> The best solution IMO would be to have a separate "Currently selected >>> by:" section on that screen, listing just the non-n selects. The >>> simpler next best thing would be to just replace the "Selected by:" >>> heading with "Currently selected by:", to make it clear that it >>> includes just the active selects. >> >> One certain thing is that with below two categories, some reverse >> dependencies would be printed twice: >> - "Currently selected by" - showing non-n expressions. >> - "Selected by" - showing both non-n and n expressions. >> >> To avoid the duplicates, I would think about (naming could be improved): >> - "Actively selected by" or "Currently selected by" >> - "Inactively selected by" or "Passively selected by" >> - "Actively implied by" or "Currently implied by" >> - "Inactively implied by" or "Passively implied by" >> >> I do believe that before proceeding with any further alternative >> implementations, we better first agree that the above way to print the >> reverse dependencies is fine for everybody. >> > > Looks good to me. Could go with something like "Current active (m/y) > selects" and "Current inactive (n) selects" maybe, to make it super > clear. > >>> For the most-selected symbols you listed, most of them end up as "m" >>> on my system by the way, because they come from drivers compiled in as >>> modules. "n" is the minority. Might want to check that most of the >>> ones with a million selects aren't like that, because it might not be >>> that hard to see what's going on for those anyway. >> >> Replying specifically to your `it might not be that hard to see what's >> going on for those anyway`, I do believe that for certain configs >> it is a pain to visually identify the meaningful (i.e. >> non-n) reverse dependencies when there are tens or hundreds of them. >> Getting this information directly from Kbuild (instead of computing it >> externally, either by hand or scripted) was my main motivation and >> driving factor behind sharing the patch. > > Consider that a before-coffee comment. It's clearly pretty helpful > even for those "obvious" cases. I had missed those ARM stats. :) > >> >>> I used a similar approach in >>> https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L3022 >>> by the way. I was always a bit worried that all the expression >>> simplification shenanigans going on in the C implementation might mess >>> with an approach like that, but it seems fine in practice. :) >> >> Can't wait to put my hands on Kconfiglib. Thanks for sharing! >> > > Might gain some features that make it viable as a full C > implementation replacement soon, if some stuff pans out. It's always > been more of an auxiliary library. Apparently the C implementation is > a bit of a PITA on Windows. > Another possibility? Add [ ]: for each line to show the expression value. (but blank for 'n' for readability) Symbol: REGMAP_I2C [=y] Type : tristate Defined at drivers/base/regmap/Kconfig:19 Depends on: I2C [=y] Selected by: [y]: EEPROM_AT24 [=y] && I2C [=y] && SYSFS [=y] [ ]: NET_DSA_SMSC_LAN9303_I2C [=n] && NETDEVICES [=y] && HAVE_NET_DSA [=y] && NET_DSA [=n] && I2C [=y] [ ]: KEYBOARD_CAP11XX [=n] && !UML && INPUT [=y] && INPUT_KEYBOARD [=y] && OF [=y] && I2C [=y] [ ]: TOUCHSCREEN_AD7879_I2C [=n] && !UML && INPUT [=y] && INPUT_TOUCHSCREEN [=n] && TOUCHSCREEN_AD7879 [=n] && [ ]: TOUCHSCREEN_TSC2004 [=n] && !UML && INPUT [=y] && INPUT_TOUCHSCREEN [=n] && I2C [=y] [ ]: INPUT_DRV260X_HAPTICS [=n] && !UML && INPUT_MISC [=y] && INPUT [=y] && I2C [=y] && (GPIOLIB [=y] || COMPI [ ]: INPUT_DRV2665_HAPTICS [=n] && !UML && INPUT_MISC [=y] && INPUT [=y] && I2C [=y] [ ]: INPUT_DRV2667_HAPTICS [=n] && !UML && INPUT_MISC [=y] && INPUT [=y] && I2C [=y] [ ]: SERIAL_SC16IS7XX_I2C [=n] && TTY [=y] && HAS_IOMEM [=y] && SERIAL_SC16IS7XX [=n] && I2C [=y] [ ]: I2C_MUX_LTC4306 [=n] && I2C [=y] && I2C_MUX [=y] [ ]: PINCTRL_MCP23S08 [=n] && PINCTRL [=y] && (SPI_MASTER [=y] || I2C [=y]) && (I2C [=y] || I2C [=y]=n) && I2C [ ]: GPIO_TS4900 [=n] && GPIOLIB [=y] && I2C [=y] && (SOC_IMX6 || COMPILE_TEST [=n]) [ ]: BATTERY_MAX17042 [=n] && POWER_SUPPLY [=y] && I2C [=y] [ ]: CHARGER_BQ25890 [=n] && POWER_SUPPLY [=y] && I2C [=y] && (GPIOLIB [=y] || COMPILE_TEST [=n]) [ ]: CHARGER_SMB347 [=n] && POWER_SUPPLY [=y] && I2C [=y] [ ]: CHARGER_RT9455 [=n] && POWER_SUPPLY [=y] && I2C [=y] && (GPIOLIB [=y] || COMPILE_TEST [=n]) [y]: SENSORS_LTC2945 [=y] && HWMON [=y] && I2C [=y] ... Best Regards Masahiro Yamada -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Eugeniu, > > Applying your patch, running > > $ ARCH=arm64 make defconfig && ARCH=arm64 make menuconfig > > and searching for USB prints "Selected by:" with nothing actually selected. > I think the behavior is correct. All expressions that select USB > translate/evaluate to =n. CONFIG_USB is enabled in the arm64 defconfig. Oh, sorry, I got that, that was the version with filtering. > Thanks, > Eugeniu. Kind regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > Another possibility? > Add [ ]: for each line to show the expression value. > (but blank for 'n' for readability) > Symbol: REGMAP_I2C [=y] > Type : tristate > Defined at drivers/base/regmap/Kconfig:19 > Depends on: I2C [=y] > Selected by: > [y]: EEPROM_AT24 [=y] && I2C [=y] && SYSFS [=y] > [ ]: NET_DSA_SMSC_LAN9303_I2C [=n] && NETDEVICES [=y] && > HAVE_NET_DSA [=y] && NET_DSA [=n] && I2C [=y] > [ ]: KEYBOARD_CAP11XX [=n] && !UML && INPUT [=y] && INPUT_KEYBOARD > [=y] && OF [=y] && I2C [=y] > [ ]: TOUCHSCREEN_AD7879_I2C [=n] && !UML && INPUT [=y] && > INPUT_TOUCHSCREEN [=n] && TOUCHSCREEN_AD7879 [=n] && > [ ]: TOUCHSCREEN_TSC2004 [=n] && !UML && INPUT [=y] && > INPUT_TOUCHSCREEN [=n] && I2C [=y] > [ ]: INPUT_DRV260X_HAPTICS [=n] && !UML && INPUT_MISC [=y] && > INPUT [=y] && I2C [=y] && (GPIOLIB [=y] || COMPI > [ ]: INPUT_DRV2665_HAPTICS [=n] && !UML && INPUT_MISC [=y] && > INPUT [=y] && I2C [=y] > [ ]: INPUT_DRV2667_HAPTICS [=n] && !UML && INPUT_MISC [=y] && > INPUT [=y] && I2C [=y] > [ ]: SERIAL_SC16IS7XX_I2C [=n] && TTY [=y] && HAS_IOMEM [=y] && > SERIAL_SC16IS7XX [=n] && I2C [=y] > [ ]: I2C_MUX_LTC4306 [=n] && I2C [=y] && I2C_MUX [=y] > [ ]: PINCTRL_MCP23S08 [=n] && PINCTRL [=y] && (SPI_MASTER [=y] || > I2C [=y]) && (I2C [=y] || I2C [=y]=n) && I2C > [ ]: GPIO_TS4900 [=n] && GPIOLIB [=y] && I2C [=y] && (SOC_IMX6 || > COMPILE_TEST [=n]) > [ ]: BATTERY_MAX17042 [=n] && POWER_SUPPLY [=y] && I2C [=y] > [ ]: CHARGER_BQ25890 [=n] && POWER_SUPPLY [=y] && I2C [=y] && > (GPIOLIB [=y] || COMPILE_TEST [=n]) > [ ]: CHARGER_SMB347 [=n] && POWER_SUPPLY [=y] && I2C [=y] > [ ]: CHARGER_RT9455 [=n] && POWER_SUPPLY [=y] && I2C [=y] && > (GPIOLIB [=y] || COMPILE_TEST [=n]) > [y]: SENSORS_LTC2945 [=y] && HWMON [=y] && I2C [=y] > ... This looks best variant to me. > Best Regards > Masahiro Yamada Kind regards, Petr -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi all, On Tue, Feb 6, 2018 at 1:00 PM, Petr Vorel <petr.vorel@gmail.com> wrote: > Hi, > >> Another possibility? > > >> Add [ ]: for each line to show the expression value. >> (but blank for 'n' for readability) > > >> Symbol: REGMAP_I2C [=y] >> Type : tristate >> Defined at drivers/base/regmap/Kconfig:19 >> Depends on: I2C [=y] >> Selected by: >> [y]: EEPROM_AT24 [=y] && I2C [=y] && SYSFS [=y] >> [ ]: NET_DSA_SMSC_LAN9303_I2C [=n] && NETDEVICES [=y] && >> HAVE_NET_DSA [=y] && NET_DSA [=n] && I2C [=y] >> [ ]: KEYBOARD_CAP11XX [=n] && !UML && INPUT [=y] && INPUT_KEYBOARD >> [=y] && OF [=y] && I2C [=y] >> [ ]: TOUCHSCREEN_AD7879_I2C [=n] && !UML && INPUT [=y] && >> INPUT_TOUCHSCREEN [=n] && TOUCHSCREEN_AD7879 [=n] && >> [ ]: TOUCHSCREEN_TSC2004 [=n] && !UML && INPUT [=y] && >> INPUT_TOUCHSCREEN [=n] && I2C [=y] >> [ ]: INPUT_DRV260X_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >> INPUT [=y] && I2C [=y] && (GPIOLIB [=y] || COMPI >> [ ]: INPUT_DRV2665_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >> INPUT [=y] && I2C [=y] >> [ ]: INPUT_DRV2667_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >> INPUT [=y] && I2C [=y] >> [ ]: SERIAL_SC16IS7XX_I2C [=n] && TTY [=y] && HAS_IOMEM [=y] && >> SERIAL_SC16IS7XX [=n] && I2C [=y] >> [ ]: I2C_MUX_LTC4306 [=n] && I2C [=y] && I2C_MUX [=y] >> [ ]: PINCTRL_MCP23S08 [=n] && PINCTRL [=y] && (SPI_MASTER [=y] || >> I2C [=y]) && (I2C [=y] || I2C [=y]=n) && I2C >> [ ]: GPIO_TS4900 [=n] && GPIOLIB [=y] && I2C [=y] && (SOC_IMX6 || >> COMPILE_TEST [=n]) >> [ ]: BATTERY_MAX17042 [=n] && POWER_SUPPLY [=y] && I2C [=y] >> [ ]: CHARGER_BQ25890 [=n] && POWER_SUPPLY [=y] && I2C [=y] && >> (GPIOLIB [=y] || COMPILE_TEST [=n]) >> [ ]: CHARGER_SMB347 [=n] && POWER_SUPPLY [=y] && I2C [=y] >> [ ]: CHARGER_RT9455 [=n] && POWER_SUPPLY [=y] && I2C [=y] && >> (GPIOLIB [=y] || COMPILE_TEST [=n]) >> [y]: SENSORS_LTC2945 [=y] && HWMON [=y] && I2C [=y] >> ... > > > This looks best variant to me. I like this approach too. The only downside I see for not doing explicit classification/break-down is that you will still have to scroll through 212 Selected-by entries of REGMAP_I2C to identify the 9 ones which evaluate to =y or =m. Fortunately, there are not so many symbols like REGMAP_I2C, so it will most lilkely not bother anybody. I'll make a sketch/proposal in the next days. But if anybody is impatient about implementing this himself, I don't mind. Thanks for your replies. Best regards, Eugeniu. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Feb 6, 2018 at 3:43 PM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: > Hi all, > > On Tue, Feb 6, 2018 at 1:00 PM, Petr Vorel <petr.vorel@gmail.com> wrote: >> Hi, >> >>> Another possibility? >> >> >>> Add [ ]: for each line to show the expression value. >>> (but blank for 'n' for readability) >> >> >>> Symbol: REGMAP_I2C [=y] >>> Type : tristate >>> Defined at drivers/base/regmap/Kconfig:19 >>> Depends on: I2C [=y] >>> Selected by: >>> [y]: EEPROM_AT24 [=y] && I2C [=y] && SYSFS [=y] >>> [ ]: NET_DSA_SMSC_LAN9303_I2C [=n] && NETDEVICES [=y] && >>> HAVE_NET_DSA [=y] && NET_DSA [=n] && I2C [=y] >>> [ ]: KEYBOARD_CAP11XX [=n] && !UML && INPUT [=y] && INPUT_KEYBOARD >>> [=y] && OF [=y] && I2C [=y] >>> [ ]: TOUCHSCREEN_AD7879_I2C [=n] && !UML && INPUT [=y] && >>> INPUT_TOUCHSCREEN [=n] && TOUCHSCREEN_AD7879 [=n] && >>> [ ]: TOUCHSCREEN_TSC2004 [=n] && !UML && INPUT [=y] && >>> INPUT_TOUCHSCREEN [=n] && I2C [=y] >>> [ ]: INPUT_DRV260X_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >>> INPUT [=y] && I2C [=y] && (GPIOLIB [=y] || COMPI >>> [ ]: INPUT_DRV2665_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >>> INPUT [=y] && I2C [=y] >>> [ ]: INPUT_DRV2667_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >>> INPUT [=y] && I2C [=y] >>> [ ]: SERIAL_SC16IS7XX_I2C [=n] && TTY [=y] && HAS_IOMEM [=y] && >>> SERIAL_SC16IS7XX [=n] && I2C [=y] >>> [ ]: I2C_MUX_LTC4306 [=n] && I2C [=y] && I2C_MUX [=y] >>> [ ]: PINCTRL_MCP23S08 [=n] && PINCTRL [=y] && (SPI_MASTER [=y] || >>> I2C [=y]) && (I2C [=y] || I2C [=y]=n) && I2C >>> [ ]: GPIO_TS4900 [=n] && GPIOLIB [=y] && I2C [=y] && (SOC_IMX6 || >>> COMPILE_TEST [=n]) >>> [ ]: BATTERY_MAX17042 [=n] && POWER_SUPPLY [=y] && I2C [=y] >>> [ ]: CHARGER_BQ25890 [=n] && POWER_SUPPLY [=y] && I2C [=y] && >>> (GPIOLIB [=y] || COMPILE_TEST [=n]) >>> [ ]: CHARGER_SMB347 [=n] && POWER_SUPPLY [=y] && I2C [=y] >>> [ ]: CHARGER_RT9455 [=n] && POWER_SUPPLY [=y] && I2C [=y] && >>> (GPIOLIB [=y] || COMPILE_TEST [=n]) >>> [y]: SENSORS_LTC2945 [=y] && HWMON [=y] && I2C [=y] >>> ... >> >> >> This looks best variant to me. > > I like this approach too. The only downside I see for not doing > explicit classification/break-down is that you will still have to > scroll through 212 Selected-by entries of REGMAP_I2C to identify the 9 > ones which evaluate to =y or =m. > Fortunately, there are not so many symbols like REGMAP_I2C, so it will > most lilkely not bother anybody. > I'll make a sketch/proposal in the next days. But if anybody is > impatient about implementing this himself, I don't mind. > > Thanks for your replies. > > Best regards, > Eugeniu. Yeah, I like the grouping feature too, with the m/y selects listed separately from the n selects. That'd be helpful both when trying to figure why a particular symbol is being selected, and when trying to figure why it isn't being selected, both of which are probably pretty common. Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 7, 2018 at 1:28 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote: > On Tue, Feb 6, 2018 at 3:43 PM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: >> Hi all, >> >> On Tue, Feb 6, 2018 at 1:00 PM, Petr Vorel <petr.vorel@gmail.com> wrote: >>> Hi, >>> >>>> Another possibility? >>> >>> >>>> Add [ ]: for each line to show the expression value. >>>> (but blank for 'n' for readability) >>> >>> >>>> Symbol: REGMAP_I2C [=y] >>>> Type : tristate >>>> Defined at drivers/base/regmap/Kconfig:19 >>>> Depends on: I2C [=y] >>>> Selected by: >>>> [y]: EEPROM_AT24 [=y] && I2C [=y] && SYSFS [=y] >>>> [ ]: NET_DSA_SMSC_LAN9303_I2C [=n] && NETDEVICES [=y] && >>>> HAVE_NET_DSA [=y] && NET_DSA [=n] && I2C [=y] >>>> [ ]: KEYBOARD_CAP11XX [=n] && !UML && INPUT [=y] && INPUT_KEYBOARD >>>> [=y] && OF [=y] && I2C [=y] >>>> [ ]: TOUCHSCREEN_AD7879_I2C [=n] && !UML && INPUT [=y] && >>>> INPUT_TOUCHSCREEN [=n] && TOUCHSCREEN_AD7879 [=n] && >>>> [ ]: TOUCHSCREEN_TSC2004 [=n] && !UML && INPUT [=y] && >>>> INPUT_TOUCHSCREEN [=n] && I2C [=y] >>>> [ ]: INPUT_DRV260X_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >>>> INPUT [=y] && I2C [=y] && (GPIOLIB [=y] || COMPI >>>> [ ]: INPUT_DRV2665_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >>>> INPUT [=y] && I2C [=y] >>>> [ ]: INPUT_DRV2667_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >>>> INPUT [=y] && I2C [=y] >>>> [ ]: SERIAL_SC16IS7XX_I2C [=n] && TTY [=y] && HAS_IOMEM [=y] && >>>> SERIAL_SC16IS7XX [=n] && I2C [=y] >>>> [ ]: I2C_MUX_LTC4306 [=n] && I2C [=y] && I2C_MUX [=y] >>>> [ ]: PINCTRL_MCP23S08 [=n] && PINCTRL [=y] && (SPI_MASTER [=y] || >>>> I2C [=y]) && (I2C [=y] || I2C [=y]=n) && I2C >>>> [ ]: GPIO_TS4900 [=n] && GPIOLIB [=y] && I2C [=y] && (SOC_IMX6 || >>>> COMPILE_TEST [=n]) >>>> [ ]: BATTERY_MAX17042 [=n] && POWER_SUPPLY [=y] && I2C [=y] >>>> [ ]: CHARGER_BQ25890 [=n] && POWER_SUPPLY [=y] && I2C [=y] && >>>> (GPIOLIB [=y] || COMPILE_TEST [=n]) >>>> [ ]: CHARGER_SMB347 [=n] && POWER_SUPPLY [=y] && I2C [=y] >>>> [ ]: CHARGER_RT9455 [=n] && POWER_SUPPLY [=y] && I2C [=y] && >>>> (GPIOLIB [=y] || COMPILE_TEST [=n]) >>>> [y]: SENSORS_LTC2945 [=y] && HWMON [=y] && I2C [=y] >>>> ... >>> >>> >>> This looks best variant to me. >> >> I like this approach too. The only downside I see for not doing >> explicit classification/break-down is that you will still have to >> scroll through 212 Selected-by entries of REGMAP_I2C to identify the 9 >> ones which evaluate to =y or =m. >> Fortunately, there are not so many symbols like REGMAP_I2C, so it will >> most lilkely not bother anybody. >> I'll make a sketch/proposal in the next days. But if anybody is >> impatient about implementing this himself, I don't mind. >> >> Thanks for your replies. >> >> Best regards, >> Eugeniu. > > Yeah, I like the grouping feature too, with the m/y selects listed > separately from the n selects. That'd be helpful both when trying to > figure why a particular symbol is being selected, and when trying to > figure why it isn't being selected, both of which are probably pretty > common. > > Cheers, > Ulf Though I guess it wouldn't matter for the not-selected case. Many cases where it'd be helpful still I think. Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Feb 7, 2018 at 1:31 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote: > On Wed, Feb 7, 2018 at 1:28 AM, Ulf Magnusson <ulfalizer@gmail.com> wrote: >> On Tue, Feb 6, 2018 at 3:43 PM, Eugeniu Rosca <roscaeugeniu@gmail.com> wrote: >>> Hi all, >>> >>> On Tue, Feb 6, 2018 at 1:00 PM, Petr Vorel <petr.vorel@gmail.com> wrote: >>>> Hi, >>>> >>>>> Another possibility? >>>> >>>> >>>>> Add [ ]: for each line to show the expression value. >>>>> (but blank for 'n' for readability) >>>> >>>> >>>>> Symbol: REGMAP_I2C [=y] >>>>> Type : tristate >>>>> Defined at drivers/base/regmap/Kconfig:19 >>>>> Depends on: I2C [=y] >>>>> Selected by: >>>>> [y]: EEPROM_AT24 [=y] && I2C [=y] && SYSFS [=y] >>>>> [ ]: NET_DSA_SMSC_LAN9303_I2C [=n] && NETDEVICES [=y] && >>>>> HAVE_NET_DSA [=y] && NET_DSA [=n] && I2C [=y] >>>>> [ ]: KEYBOARD_CAP11XX [=n] && !UML && INPUT [=y] && INPUT_KEYBOARD >>>>> [=y] && OF [=y] && I2C [=y] >>>>> [ ]: TOUCHSCREEN_AD7879_I2C [=n] && !UML && INPUT [=y] && >>>>> INPUT_TOUCHSCREEN [=n] && TOUCHSCREEN_AD7879 [=n] && >>>>> [ ]: TOUCHSCREEN_TSC2004 [=n] && !UML && INPUT [=y] && >>>>> INPUT_TOUCHSCREEN [=n] && I2C [=y] >>>>> [ ]: INPUT_DRV260X_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >>>>> INPUT [=y] && I2C [=y] && (GPIOLIB [=y] || COMPI >>>>> [ ]: INPUT_DRV2665_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >>>>> INPUT [=y] && I2C [=y] >>>>> [ ]: INPUT_DRV2667_HAPTICS [=n] && !UML && INPUT_MISC [=y] && >>>>> INPUT [=y] && I2C [=y] >>>>> [ ]: SERIAL_SC16IS7XX_I2C [=n] && TTY [=y] && HAS_IOMEM [=y] && >>>>> SERIAL_SC16IS7XX [=n] && I2C [=y] >>>>> [ ]: I2C_MUX_LTC4306 [=n] && I2C [=y] && I2C_MUX [=y] >>>>> [ ]: PINCTRL_MCP23S08 [=n] && PINCTRL [=y] && (SPI_MASTER [=y] || >>>>> I2C [=y]) && (I2C [=y] || I2C [=y]=n) && I2C >>>>> [ ]: GPIO_TS4900 [=n] && GPIOLIB [=y] && I2C [=y] && (SOC_IMX6 || >>>>> COMPILE_TEST [=n]) >>>>> [ ]: BATTERY_MAX17042 [=n] && POWER_SUPPLY [=y] && I2C [=y] >>>>> [ ]: CHARGER_BQ25890 [=n] && POWER_SUPPLY [=y] && I2C [=y] && >>>>> (GPIOLIB [=y] || COMPILE_TEST [=n]) >>>>> [ ]: CHARGER_SMB347 [=n] && POWER_SUPPLY [=y] && I2C [=y] >>>>> [ ]: CHARGER_RT9455 [=n] && POWER_SUPPLY [=y] && I2C [=y] && >>>>> (GPIOLIB [=y] || COMPILE_TEST [=n]) >>>>> [y]: SENSORS_LTC2945 [=y] && HWMON [=y] && I2C [=y] >>>>> ... >>>> >>>> >>>> This looks best variant to me. >>> >>> I like this approach too. The only downside I see for not doing >>> explicit classification/break-down is that you will still have to >>> scroll through 212 Selected-by entries of REGMAP_I2C to identify the 9 >>> ones which evaluate to =y or =m. >>> Fortunately, there are not so many symbols like REGMAP_I2C, so it will >>> most lilkely not bother anybody. >>> I'll make a sketch/proposal in the next days. But if anybody is >>> impatient about implementing this himself, I don't mind. >>> >>> Thanks for your replies. >>> >>> Best regards, >>> Eugeniu. >> >> Yeah, I like the grouping feature too, with the m/y selects listed >> separately from the n selects. That'd be helpful both when trying to >> figure why a particular symbol is being selected, and when trying to >> figure why it isn't being selected, both of which are probably pretty >> common. >> >> Cheers, >> Ulf > > Though I guess it wouldn't matter for the not-selected case. Many > cases where it'd be helpful still I think. > > Cheers, > Ulf ...even there, it'd make it immediately obvious that the symbol isn't being selected. Cheers, Ulf "trigger-happy sender" -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index 2ba332b3fed7..147b2d8a8f3e 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -1234,14 +1234,24 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con fn(data, e->right.sym, e->right.sym->name); break; case E_OR: - if (revdep && e->left.expr->type != E_OR) - fn(data, NULL, "\n - "); - __expr_print(e->left.expr, fn, data, E_OR, revdep); - if (revdep) - fn(data, NULL, "\n - "); - else + if (revdep) { + struct expr *left = e->left.expr; + struct expr *right = e->right.expr; + + if (expr_calc_value(left) != no) { + if (left->type != E_OR) + fn(data, NULL, "\n - "); + __expr_print(left, fn, data, E_OR, revdep); + } + if (expr_calc_value(right) != no) { + fn(data, NULL, "\n - "); + __expr_print(right, fn, data, E_OR, revdep); + } + } else { + __expr_print(e->left.expr, fn, data, E_OR, revdep); fn(data, NULL, " || "); - __expr_print(e->right.expr, fn, data, E_OR, revdep); + __expr_print(e->right.expr, fn, data, E_OR, revdep); + } break; case E_AND: expr_print(e->left.expr, fn, data, E_AND);