diff mbox

kconfig: fix allmodconfig

Message ID 1379805443-10945-1-git-send-email-yann.morin.1998@free.fr (mailing list archive)
State Changes Requested
Headers show

Commit Message

Yann E. MORIN Sept. 21, 2013, 11:17 p.m. UTC
From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Michal Marek <mmarek@suse.cz>
---
 scripts/kconfig/confdata.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Yann E. MORIN Sept. 21, 2013, 11:21 p.m. UTC | #1
Stephen, All,

Please, test this patch to fix the allmodconfig issue reported by
Stephen.

From my little testing (some randconfig followed by silentoldconfig, as
well as allmodconfig), this patch seems to fix the issue without any
regression I could ientify.

But since this is sensitive code, I'd like some feedback before this
gets applied.

Thank you!

Regards,
Yann E. MORIN.

On 2013-09-22 01:17 +0200, Yann E. MORIN spake thusly:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> Cc: Sam Ravnborg <sam@ravnborg.org>
> Cc: Michal Marek <mmarek@suse.cz>
> ---
>  scripts/kconfig/confdata.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 87f7238..0cbfc67 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -399,8 +399,6 @@ setsym:
>  	free(line);
>  	fclose(in);
>  
> -	if (modules_sym)
> -		sym_calc_value(modules_sym);
>  	return 0;
>  }
>  
> @@ -1175,6 +1173,10 @@ bool conf_set_all_new_symbols(enum conf_def_mode mode)
>  				sym->def[S_DEF_USER].tri = yes;
>  				break;
>  			case def_mod:
> +				/* Note: although modules_sym must be 'yes' to
> +				 * enable tristates, 'mod' is promoted to 'yes'
> +				 * when applied to a boolean, which modules_sym
> +				 * is. So we need not special-case it here. */
>  				sym->def[S_DEF_USER].tri = mod;
>  				break;
>  			case def_no:
> -- 
> 1.8.1.2
>
Yann E. MORIN Sept. 26, 2013, 9:36 p.m. UTC | #2
Hello All!

On 2013-09-22 01:21 +0200, Yann E. MORIN spake thusly:
> Stephen, All,
> 
> Please, test this patch to fix the allmodconfig issue reported by
> Stephen.
> 
> From my little testing (some randconfig followed by silentoldconfig, as
> well as allmodconfig), this patch seems to fix the issue without any
> regression I could ientify.
> 
> But since this is sensitive code, I'd like some feedback before this
> gets applied.

Ping? :-)

I wanted to push this right after -rc3 is out. If any one has an issue
with that patch, you've got a few days to chime in! ;-)

Regards,
Yann E. MORIN.

> On 2013-09-22 01:17 +0200, Yann E. MORIN spake thusly:
> > From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > 
> > Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > Cc: Stephen Rothwell <sfr@canb.auug.org.au>
> > Cc: Sam Ravnborg <sam@ravnborg.org>
> > Cc: Michal Marek <mmarek@suse.cz>
> > ---
> >  scripts/kconfig/confdata.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> > index 87f7238..0cbfc67 100644
> > --- a/scripts/kconfig/confdata.c
> > +++ b/scripts/kconfig/confdata.c
> > @@ -399,8 +399,6 @@ setsym:
> >  	free(line);
> >  	fclose(in);
> >  
> > -	if (modules_sym)
> > -		sym_calc_value(modules_sym);
> >  	return 0;
> >  }
> >  
> > @@ -1175,6 +1173,10 @@ bool conf_set_all_new_symbols(enum conf_def_mode mode)
> >  				sym->def[S_DEF_USER].tri = yes;
> >  				break;
> >  			case def_mod:
> > +				/* Note: although modules_sym must be 'yes' to
> > +				 * enable tristates, 'mod' is promoted to 'yes'
> > +				 * when applied to a boolean, which modules_sym
> > +				 * is. So we need not special-case it here. */
> >  				sym->def[S_DEF_USER].tri = mod;
> >  				break;
> >  			case def_no:
Michal Marek Sept. 27, 2013, 10:19 a.m. UTC | #3
On 26.9.2013 23:36, Yann E. MORIN wrote:
> Hello All!
> 
> On 2013-09-22 01:21 +0200, Yann E. MORIN spake thusly:
>> Stephen, All,
>>
>> Please, test this patch to fix the allmodconfig issue reported by
>> Stephen.
>>
>> From my little testing (some randconfig followed by silentoldconfig, as
>> well as allmodconfig), this patch seems to fix the issue without any
>> regression I could ientify.
>>
>> But since this is sensitive code, I'd like some feedback before this
>> gets applied.
> 
> Ping? :-)
> 
> I wanted to push this right after -rc3 is out. If any one has an issue
> with that patch, you've got a few days to chime in! ;-)

It works for me, you can add my Tested-by: if you like. The only issue I
have is that a more verbose changelog is missing.

Michal
--
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
Yann E. MORIN Sept. 27, 2013, 8:02 p.m. UTC | #4
Michal, All,

On 2013-09-27 12:19 +0200, Michal Marek spake thusly:
> On 26.9.2013 23:36, Yann E. MORIN wrote:
> > Hello All!
> > 
> > On 2013-09-22 01:21 +0200, Yann E. MORIN spake thusly:
> >> Stephen, All,
> >>
> >> Please, test this patch to fix the allmodconfig issue reported by
> >> Stephen.
> >>
> >> From my little testing (some randconfig followed by silentoldconfig, as
> >> well as allmodconfig), this patch seems to fix the issue without any
> >> regression I could ientify.
> >>
> >> But since this is sensitive code, I'd like some feedback before this
> >> gets applied.
> > 
> > Ping? :-)
> > 
> > I wanted to push this right after -rc3 is out. If any one has an issue
> > with that patch, you've got a few days to chime in! ;-)
> 
> It works for me, you can add my Tested-by: if you like. The only issue I
> have is that a more verbose changelog is missing.

OK. I'll rewrite the commit log with more details before I send the
final pull-request. Thank you! :-)

Regards,
Yann E. MORIN.
Michal Marek Oct. 2, 2013, 8:29 p.m. UTC | #5
Dne 27.9.2013 12:19, Michal Marek napsal(a):
> On 26.9.2013 23:36, Yann E. MORIN wrote:
>> Hello All!
>>
>> On 2013-09-22 01:21 +0200, Yann E. MORIN spake thusly:
>>> Stephen, All,
>>>
>>> Please, test this patch to fix the allmodconfig issue reported by
>>> Stephen.
>>>
>>> From my little testing (some randconfig followed by silentoldconfig, as
>>> well as allmodconfig), this patch seems to fix the issue without any
>>> regression I could ientify.
>>>
>>> But since this is sensitive code, I'd like some feedback before this
>>> gets applied.
>>
>> Ping? :-)
>>
>> I wanted to push this right after -rc3 is out. If any one has an issue
>> with that patch, you've got a few days to chime in! ;-)
> 
> It works for me, you can add my Tested-by: if you like. The only issue I
> have is that a more verbose changelog is missing.

Sorry, I have to retract my Tested-by. When I merge your rc-fixes branch
into 3.12-rc1, I get this behavior:

$ make mrproper
$ make allmodconfig
...
scripts/kconfig/conf --allmodconfig Kconfig
#
# configuration written to .config
#$ make silentoldconfig
scripts/kconfig/conf --silentoldconfig Kconfig
*
* Restart config...
*
(this should not happen)
*
* PHY Device support and infrastructure
*
PHY Device support and infrastructure (PHYLIB) [Y/?] y
  *
  * MII PHY device drivers
  *
  Drivers for Atheros AT803X PHYs (AT803X_PHY) [M/n/y/?] m
  Drivers for the AMD PHYs (AMD_PHY) [M/n/y/?] m
  Drivers for Marvell PHYs (MARVELL_PHY) [M/n/y/?] m
  Drivers for Davicom PHYs (DAVICOM_PHY) [M/n/y/?] m
  Drivers for Quality Semiconductor PHYs (QSEMI_PHY) [M/n/y/?] m
  Drivers for the Intel LXT PHYs (LXT_PHY) [M/n/y/?] m
  Drivers for the Cicada PHYs (CICADA_PHY) [M/n/y/?] m
  Drivers for the Vitesse PHYs (VITESSE_PHY) [M/n/y/?] m
  Drivers for SMSC PHYs (SMSC_PHY) [M/y/?] m
  Drivers for Broadcom PHYs (BROADCOM_PHY) [M/n/y/?] m
  Driver for Broadcom BCM8706 and BCM8727 PHYs (BCM87XX_PHY) [M/n/y/?] m
  Drivers for ICPlus PHYs (ICPLUS_PHY) [M/n/y/?] m
  Drivers for Realtek PHYs (REALTEK_PHY) [M/n/y/?] m
  Drivers for National Semiconductor PHYs (NATIONAL_PHY) [M/n/y/?] m
  Driver for STMicroelectronics STe10Xp PHYs (STE10XP) [M/n/y/?] m
  Driver for LSI ET1011C PHY (LSI_ET1011C_PHY) [M/n/y/?] m
  Driver for Micrel PHYs (MICREL_PHY) [M/n/y/?] m
  Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs
(FIXED_PHY) [N/y/?] (NEW)

The .config generated by make allmodconfig is identical to a .config
generated with plain v3.12-rc1 and thus correct. It's (silent)oldconfig
that has a problem.

Michal
--
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
Yann E. MORIN Oct. 3, 2013, 10:02 p.m. UTC | #6
Michal, All,

On 2013-10-02 22:29 +0200, Michal Marek spake thusly:
> Dne 27.9.2013 12:19, Michal Marek napsal(a):
> > On 26.9.2013 23:36, Yann E. MORIN wrote:
> >> Hello All!
> >>
> >> On 2013-09-22 01:21 +0200, Yann E. MORIN spake thusly:
> >>> Stephen, All,
> >>>
> >>> Please, test this patch to fix the allmodconfig issue reported by
> >>> Stephen.
> >>>
> >>> From my little testing (some randconfig followed by silentoldconfig, as
> >>> well as allmodconfig), this patch seems to fix the issue without any
> >>> regression I could ientify.
> >>>
> >>> But since this is sensitive code, I'd like some feedback before this
> >>> gets applied.
> >>
> >> Ping? :-)
> >>
> >> I wanted to push this right after -rc3 is out. If any one has an issue
> >> with that patch, you've got a few days to chime in! ;-)
> > 
> > It works for me, you can add my Tested-by: if you like. The only issue I
> > have is that a more verbose changelog is missing.
> 
> Sorry, I have to retract my Tested-by. When I merge your rc-fixes branch
> into 3.12-rc1, I get this behavior:
> 
> $ make mrproper
> $ make allmodconfig
> ...
> scripts/kconfig/conf --allmodconfig Kconfig
> #
> # configuration written to .config
> #$ make silentoldconfig
> scripts/kconfig/conf --silentoldconfig Kconfig
> *
> * Restart config...
> *
> (this should not happen)
> *
> * PHY Device support and infrastructure
> *
> PHY Device support and infrastructure (PHYLIB) [Y/?] y
>   *
>   * MII PHY device drivers
>   *
>   Drivers for Atheros AT803X PHYs (AT803X_PHY) [M/n/y/?] m
>   Drivers for the AMD PHYs (AMD_PHY) [M/n/y/?] m
>   Drivers for Marvell PHYs (MARVELL_PHY) [M/n/y/?] m
>   Drivers for Davicom PHYs (DAVICOM_PHY) [M/n/y/?] m
>   Drivers for Quality Semiconductor PHYs (QSEMI_PHY) [M/n/y/?] m
>   Drivers for the Intel LXT PHYs (LXT_PHY) [M/n/y/?] m
>   Drivers for the Cicada PHYs (CICADA_PHY) [M/n/y/?] m
>   Drivers for the Vitesse PHYs (VITESSE_PHY) [M/n/y/?] m
>   Drivers for SMSC PHYs (SMSC_PHY) [M/y/?] m
>   Drivers for Broadcom PHYs (BROADCOM_PHY) [M/n/y/?] m
>   Driver for Broadcom BCM8706 and BCM8727 PHYs (BCM87XX_PHY) [M/n/y/?] m
>   Drivers for ICPlus PHYs (ICPLUS_PHY) [M/n/y/?] m
>   Drivers for Realtek PHYs (REALTEK_PHY) [M/n/y/?] m
>   Drivers for National Semiconductor PHYs (NATIONAL_PHY) [M/n/y/?] m
>   Driver for STMicroelectronics STe10Xp PHYs (STE10XP) [M/n/y/?] m
>   Driver for LSI ET1011C PHY (LSI_ET1011C_PHY) [M/n/y/?] m
>   Driver for Micrel PHYs (MICREL_PHY) [M/n/y/?] m
>   Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs
> (FIXED_PHY) [N/y/?] (NEW)
> 
> The .config generated by make allmodconfig is identical to a .config
> generated with plain v3.12-rc1 and thus correct. It's (silent)oldconfig
> that has a problem.

Doh... :-(

I've played with this a bit, and I noticed that if you do:
    $ git clean -dX; git clean -d    # Make sure tree is clean
    $ make allmodconfig
    $ grep PHYLIB= .config
    CONFIG_PHYLIB=m
    $ make menuconfig
        Device Drivers  -->
          Network device support  -->
            -*-  PHY Device support and infrastructure  -->

Notice how "PHY Device support and infrastructure" is forced to 'y', not
'm'. This is expected, because it is selected by (there are other terms
in the equation that also force it to 'y'):
    ETHOC [=y] && NETDEVICES [=y] && ETHERNET [=y]
    && HAS_IOMEM [=y] && HAS_DMA [=y]

Hence, what we see is that "make allmodconfig" does not properly resolve
the symbols values.

I'll tackle this, but it may take some time...

Regards,
Yann E. MORIN.
Yann E. MORIN Oct. 8, 2013, 9:54 p.m. UTC | #7
Michal, Al,

On 2013-10-04 00:02 +0200, Yann E. MORIN spake thusly:
> On 2013-10-02 22:29 +0200, Michal Marek spake thusly:
> > Dne 27.9.2013 12:19, Michal Marek napsal(a):
> > > On 26.9.2013 23:36, Yann E. MORIN wrote:
> > >> Hello All!
> > >>
> > >> On 2013-09-22 01:21 +0200, Yann E. MORIN spake thusly:
> > >>> Stephen, All,
> > >>>
> > >>> Please, test this patch to fix the allmodconfig issue reported by
> > >>> Stephen.
> > >>>
> > >>> From my little testing (some randconfig followed by silentoldconfig, as
> > >>> well as allmodconfig), this patch seems to fix the issue without any
> > >>> regression I could ientify.
> > >>>
> > >>> But since this is sensitive code, I'd like some feedback before this
> > >>> gets applied.
> > >>
> > >> Ping? :-)
> > >>
> > >> I wanted to push this right after -rc3 is out. If any one has an issue
> > >> with that patch, you've got a few days to chime in! ;-)
> > > 
> > > It works for me, you can add my Tested-by: if you like. The only issue I
> > > have is that a more verbose changelog is missing.
> > 
> > Sorry, I have to retract my Tested-by. When I merge your rc-fixes branch
> > into 3.12-rc1, I get this behavior:
> > 
> > $ make mrproper
> > $ make allmodconfig
> > ...
> > scripts/kconfig/conf --allmodconfig Kconfig
> > #
> > # configuration written to .config
> > #$ make silentoldconfig
> > scripts/kconfig/conf --silentoldconfig Kconfig
> > *
> > * Restart config...
> > *
> > (this should not happen)
> > *
> > * PHY Device support and infrastructure
> > *
> > PHY Device support and infrastructure (PHYLIB) [Y/?] y
> >   *
> >   * MII PHY device drivers
> >   *
> >   Drivers for Atheros AT803X PHYs (AT803X_PHY) [M/n/y/?] m
> >   Drivers for the AMD PHYs (AMD_PHY) [M/n/y/?] m
> >   Drivers for Marvell PHYs (MARVELL_PHY) [M/n/y/?] m
> >   Drivers for Davicom PHYs (DAVICOM_PHY) [M/n/y/?] m
> >   Drivers for Quality Semiconductor PHYs (QSEMI_PHY) [M/n/y/?] m
> >   Drivers for the Intel LXT PHYs (LXT_PHY) [M/n/y/?] m
> >   Drivers for the Cicada PHYs (CICADA_PHY) [M/n/y/?] m
> >   Drivers for the Vitesse PHYs (VITESSE_PHY) [M/n/y/?] m
> >   Drivers for SMSC PHYs (SMSC_PHY) [M/y/?] m
> >   Drivers for Broadcom PHYs (BROADCOM_PHY) [M/n/y/?] m
> >   Driver for Broadcom BCM8706 and BCM8727 PHYs (BCM87XX_PHY) [M/n/y/?] m
> >   Drivers for ICPlus PHYs (ICPLUS_PHY) [M/n/y/?] m
> >   Drivers for Realtek PHYs (REALTEK_PHY) [M/n/y/?] m
> >   Drivers for National Semiconductor PHYs (NATIONAL_PHY) [M/n/y/?] m
> >   Driver for STMicroelectronics STe10Xp PHYs (STE10XP) [M/n/y/?] m
> >   Driver for LSI ET1011C PHY (LSI_ET1011C_PHY) [M/n/y/?] m
> >   Driver for Micrel PHYs (MICREL_PHY) [M/n/y/?] m
> >   Driver for MDIO Bus/PHY emulation with fixed speed/link PHYs
> > (FIXED_PHY) [N/y/?] (NEW)
> > 
> > The .config generated by make allmodconfig is identical to a .config
> > generated with plain v3.12-rc1 and thus correct. It's (silent)oldconfig
> > that has a problem.
> 
> Doh... :-(
> 
> I've played with this a bit, and I noticed that if you do:
>     $ git clean -dX; git clean -d    # Make sure tree is clean
>     $ make allmodconfig
>     $ grep PHYLIB= .config
>     CONFIG_PHYLIB=m
>     $ make menuconfig
>         Device Drivers  -->
>           Network device support  -->
>             -*-  PHY Device support and infrastructure  -->
> 
> Notice how "PHY Device support and infrastructure" is forced to 'y', not
> 'm'. This is expected, because it is selected by (there are other terms
> in the equation that also force it to 'y'):
>     ETHOC [=y] && NETDEVICES [=y] && ETHERNET [=y]
>     && HAS_IOMEM [=y] && HAS_DMA [=y]
> 
> Hence, what we see is that "make allmodconfig" does not properly resolve
> the symbols values.
> 
> I'll tackle this, but it may take some time...

OK, I have no clue... :-(
Any suggestion?

Regards,
Yann E. MORIN.
diff mbox

Patch

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index 87f7238..0cbfc67 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -399,8 +399,6 @@  setsym:
 	free(line);
 	fclose(in);
 
-	if (modules_sym)
-		sym_calc_value(modules_sym);
 	return 0;
 }
 
@@ -1175,6 +1173,10 @@  bool conf_set_all_new_symbols(enum conf_def_mode mode)
 				sym->def[S_DEF_USER].tri = yes;
 				break;
 			case def_mod:
+				/* Note: although modules_sym must be 'yes' to
+				 * enable tristates, 'mod' is promoted to 'yes'
+				 * when applied to a boolean, which modules_sym
+				 * is. So we need not special-case it here. */
 				sym->def[S_DEF_USER].tri = mod;
 				break;
 			case def_no: