[PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Eugeniu Rosca]

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

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Ulf Magnusson]

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

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Ulf Magnusson]

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

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Eugeniu Rosca]

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.

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Ulf Magnusson]

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

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Masahiro Yamada]

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

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Petr Vorel]

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

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Eugeniu Rosca]

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.

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Ulf Magnusson]

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

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Ulf Magnusson]

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

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Ulf Magnusson]

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"

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Petr Vorel]

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

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Ulf Magnusson]

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

On Sun, Feb 4, 2018 at 4:50 PM, Petr Vorel <petr.vorel@gmail.com> 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.
>
> Kind regards,
> Petr

Hello,

I implemented the behavior I had in mind. With this patch, REGMAP_I2C
shows up as follows:

Symbol: REGMAP_I2C [=y]│
Type  : tristate│
  Defined at drivers/base/regmap/Kconfig:19
  Depends on: I2C [=y]
  Currently selected by:
  - RTC_I2C_AND_SPI [=y] && RTC_CLASS [=y] && I2C [=y]
  - USB_HSIC_USB3503 [=y] && USB_SUPPORT [=y] && USB [=y] && I2C [=y]
  - SND_SOC [=y] && SOUND [=y] && !UML && SND [=y] && I2C [=y]
  - DRM_I2C_ADV7511 [=m] && HAS_IOMEM [=y] && DRM [=m] && DRM_BRIDGE
[=y] && OF [=y]
  - REGULATOR_FAN53555 [=y] && REGULATOR [=y] && I2C [=y]
  - MFD_SEC_CORE [=y] && HAS_IOMEM [=y] && I2C [=y]=y
  - MFD_RK808 [=y] && HAS_IOMEM [=y] && I2C [=y] && OF [=y]
  - MFD_MAX77620 [=y] && HAS_IOMEM [=y] && I2C [=y]=y && (OF [=y] ||
COMPILE_TEST [=n])
  - SENSORS_INA2XX [=m] && HWMON [=y] && I2C [=y]
  All selecting symbols:
  - PWM_PCA9685 [=n] && PWM [=y] && I2C [=y]
  - SX9500 [=n] && IIO [=y] && I2C [=y]
  - ZPA2326_I2C [=n] && IIO [=y]
  - HP03 [=n] && IIO [=y] && I2C [=y]
  - BMP280_I2C [=n] && IIO [=y] && BMP280 [=n] && I2C [=y]
  ...

USB shows up as:

Symbol: USB [=y]
Type  : tristate
Prompt: Support for Host-side USB
  Location:
    -> Device Drivers
(1)   -> USB support (USB_SUPPORT [=y])
  Defined at drivers/usb/Kconfig:38
  Depends on: USB_SUPPORT [=y] && USB_ARCH_HAS_HCD [=y]
  Selects: USB_COMMON [=y] && NLS [=y]
  Currently selected by:
    No symbols
  All selecting symbols:
  - DRM_UDL [=n] && HAS_IOMEM [=y] && DRM [=m] && USB_SUPPORT [=y] &&
USB_ARCH_HAS_HCD [=y]
  - IR_TTUSBIR [=n] && RC_DEVICES [=y] && USB_ARCH_HAS_HCD [=y] && RC_CORE [=m]
  - IR_IGUANA [=n] && RC_DEVICES [=y] && USB_ARCH_HAS_HCD [=y] && RC_CORE [=m]
  - IR_IGORPLUGUSB [=n] && RC_DEVICES [=y] && USB_ARCH_HAS_HCD [=y] &&
RC_CORE [=m]
  - IR_STREAMZAP [=n] && RC_DEVICES [=y] && USB_ARCH_HAS_HCD [=y] &&
RC_CORE [=m]
  ...

What do you think about this behavior? Tell me if you spot any bugs.

While working on the patch, I thought I had a regression where a
reverse dependency of just a single symbol showed up without a leading
"  - ", hence the slight overengineering. On a second look, I think
the original patch also did that though. It's arguable whether that's
actually a bug, but now it always prints the "  - ".

I don't have access to Mutt at the moment, and GMail's web interface
mangles patches, so I'll attach the patch.

If you go with something similar, you could add a Suggested-by:.

Cheers,
Ulf

[-- Attachment #2: 0001-Readable-rev.-dep-WIP.patch --]
[-- Type: text/x-patch, Size: 6083 bytes --]

From 8d3e34dbf4eb11f492519464031cd610ad866b53 Mon Sep 17 00:00:00 2001
From: Ulf Magnusson <ulfalizer@gmail.com>
Date: Sun, 4 Feb 2018 18:52:40 +0100
Subject: [PATCH] Readable rev. dep WIP

---
 scripts/kconfig/expr.c | 97 +++++++++++++++++++++++++++++++++++++++++---------
 scripts/kconfig/expr.h |  1 +
 scripts/kconfig/menu.c | 11 +++++-
 3 files changed, 92 insertions(+), 17 deletions(-)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index 2ba332b3fed7..fd4814cb1a31 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -1179,7 +1179,33 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2)
 	return expr_get_leftmost_symbol(ret);
 }
 
-static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken, bool revdep)
+enum expr_print_mode {
+	PRINT_NORMAL,
+
+	/* Print reverse dependencies with selects on separate lines */
+	PRINT_ALL_SELECTS,
+
+	/* Like PRINT_ALL_SELECTS, but skip 'n'-valued selects */
+	PRINT_ACTIVE_SELECTS
+};
+
+/* Helper for pretty-printing reverse dependencies with one select per line */
+static void print_select(struct expr *e,
+			 void (*fn)(void *, struct symbol *, const char *),
+			 void *data,
+			 enum expr_print_mode mode)
+{
+	if (mode == PRINT_ALL_SELECTS || expr_calc_value(e) != no) {
+		fn(data, NULL, "\n  - ");
+		expr_print(e, fn, data, E_OR);
+	}
+}
+
+static void __expr_print(struct expr *e,
+			 void (*fn)(void *, struct symbol *, const char *),
+			 void *data,
+			 int prevtoken,
+			 enum expr_print_mode mode)
 {
 	if (!e) {
 		fn(data, NULL, "y");
@@ -1190,10 +1216,18 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con
 		fn(data, NULL, "(");
 	switch (e->type) {
 	case E_SYMBOL:
-		if (e->left.sym->name)
-			fn(data, e->left.sym, e->left.sym->name);
-		else
+		if (e->left.sym->name) {
+			if (mode == PRINT_NORMAL)
+				fn(data, e->left.sym, e->left.sym->name);
+			else
+				/*
+				 * PRINT_*_SELECTS, with a final select that
+				 * has no condition
+				 */
+				print_select(e, fn, data, mode);
+		} else {
 			fn(data, NULL, "<choice>");
+		}
 		break;
 	case E_NOT:
 		fn(data, NULL, "!");
@@ -1234,19 +1268,42 @@ 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 (mode == PRINT_NORMAL) {
+			expr_print(e->left.expr, fn, data, E_OR);
 			fn(data, NULL, " || ");
-		__expr_print(e->right.expr, fn, data, E_OR, revdep);
+			expr_print(e->right.expr, fn, data, E_OR);
+		} else {
+			/*
+			 * PRINT_*_SELECTS
+			 *
+			 * Support both (OR, <select 1>, (OR, <select 2>)) and
+			 * (OR, (OR, <select 1>), <select 2>) format for
+			 * selects. I think you always get the second one in
+			 * practice as of writing.
+			 */
+			if (e->right.expr->type != E_OR) {
+				print_select(e->right.expr, fn, data, mode);
+				/* Move on to next select */
+				__expr_print(e->left.expr, fn, data, E_OR, mode);
+			} else {
+				print_select(e->left.expr, fn, data, mode);
+				/* Move on to next select */
+				__expr_print(e->right.expr, fn, data, E_OR, mode);
+			}
+		}
 		break;
 	case E_AND:
-		expr_print(e->left.expr, fn, data, E_AND);
-		fn(data, NULL, " && ");
-		expr_print(e->right.expr, fn, data, E_AND);
+		if (mode == PRINT_NORMAL) {
+			expr_print(e->left.expr, fn, data, E_AND);
+			fn(data, NULL, " && ");
+			expr_print(e->right.expr, fn, data, E_AND);
+		} else {
+			/*
+			 * PRINT_*_SELECTS, with a final select that has a
+			 * condition
+			 */
+			print_select(e, fn, data, mode);
+		}
 		break;
 	case E_LIST:
 		fn(data, e->right.sym, e->right.sym->name);
@@ -1276,7 +1333,7 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con
 
 void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken)
 {
-	__expr_print(e, fn, data, prevtoken, false);
+	__expr_print(e, fn, data, prevtoken, PRINT_NORMAL);
 }
 
 static void expr_print_file_helper(void *data, struct symbol *sym, const char *str)
@@ -1331,5 +1388,13 @@ void expr_gstr_print(struct expr *e, struct gstr *gs)
  */
 void expr_gstr_print_revdep(struct expr *e, struct gstr *gs)
 {
-	__expr_print(e, expr_print_gstr_helper, gs, E_NONE, true);
+	__expr_print(e, expr_print_gstr_helper, gs, E_NONE, PRINT_ALL_SELECTS);
+}
+
+/*
+ * Like expr_gstr_print_revdep(), but do not print 'n'-valued selects
+ */
+void expr_gstr_print_active_revdep(struct expr *e, struct gstr *gs)
+{
+	__expr_print(e, expr_print_gstr_helper, gs, E_NONE, PRINT_ACTIVE_SELECTS);
 }
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index c16e82e302a2..c0274dc23a47 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -311,6 +311,7 @@ void expr_fprint(struct expr *e, FILE *out);
 struct gstr; /* forward */
 void expr_gstr_print(struct expr *e, struct gstr *gs);
 void expr_gstr_print_revdep(struct expr *e, struct gstr *gs);
+void expr_gstr_print_active_revdep(struct expr *e, struct gstr *gs);
 
 static inline int expr_is_yes(struct expr *e)
 {
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 99222855544c..170a8b421e97 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -827,7 +827,16 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym,
 
 	get_symbol_props_str(r, sym, P_SELECT, _("  Selects: "));
 	if (sym->rev_dep.expr) {
-		str_append(r, _("  Selected by: "));
+		str_append(r, _("  Currently selected by:"));
+		if (expr_calc_value(sym->rev_dep.expr) == no) {
+		        str_append(r, "\n");
+			str_append(r, _("    No symbols"));
+		} else {
+			expr_gstr_print_active_revdep(sym->rev_dep.expr, r);
+		}
+		str_append(r, "\n");
+
+		str_append(r, _("  All selecting symbols:"));
 		expr_gstr_print_revdep(sym->rev_dep.expr, r);
 		str_append(r, "\n");
 	}
-- 
2.14.1

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Eugeniu Rosca]

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.

Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

[Petr Vorel]

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