Patchwork powerpc: remove default=y from PMAC and CHRP Kconfigs

login
register
mail settings
Submitter Timur Tabi
Date Oct. 8, 2008, 7:57 p.m.
Message ID <1223495845-14149-1-git-send-email-timur@freescale.com>
Download mbox | patch
Permalink /patch/3362/
State Rejected
Headers show

Comments

Timur Tabi - Oct. 8, 2008, 7:57 p.m.
Remove the "default y" from the Kconfig options for CHRP, PMAC, and PMAC64
platforms.  This patch is a follow-up to "remove CHRP and PMAC support from
defconfigs, fix Kconfigs", which was applied incompletely.

Signed-off-by: Timur Tabi <timur@freescale.com>
---

Ben, in the future, please apply either all of my patch or none of it.
Applying half of my patch and "thinking about" the other half doesn't do me
any good.

 arch/powerpc/platforms/chrp/Kconfig     |    1 -
 arch/powerpc/platforms/powermac/Kconfig |    4 ----
 2 files changed, 0 insertions(+), 5 deletions(-)
Benjamin Herrenschmidt - Oct. 8, 2008, 8:03 p.m.
On Wed, 2008-10-08 at 14:57 -0500, Timur Tabi wrote:
> Remove the "default y" from the Kconfig options for CHRP, PMAC, and PMAC64
> platforms.  This patch is a follow-up to "remove CHRP and PMAC support from
> defconfigs, fix Kconfigs", which was applied incompletely.

No. Instead, send a patch that fixes the defconfig's to explicitely set
those to "n". As to whether those should be defaults or not, this is a
different discussion (I'm almost tempted to have everything default to
y).

Ben.

> Signed-off-by: Timur Tabi <timur@freescale.com>
> ---
> 
> Ben, in the future, please apply either all of my patch or none of it.
> Applying half of my patch and "thinking about" the other half doesn't do me
> any good.
> 
>  arch/powerpc/platforms/chrp/Kconfig     |    1 -
>  arch/powerpc/platforms/powermac/Kconfig |    4 ----
>  2 files changed, 0 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/chrp/Kconfig b/arch/powerpc/platforms/chrp/Kconfig
> index 22b4b4e..682afbc 100644
> --- a/arch/powerpc/platforms/chrp/Kconfig
> +++ b/arch/powerpc/platforms/chrp/Kconfig
> @@ -9,4 +9,3 @@ config PPC_CHRP
>  	select PPC_UDBG_16550
>  	select PPC_NATIVE
>  	select PCI
> -	default y
> diff --git a/arch/powerpc/platforms/powermac/Kconfig b/arch/powerpc/platforms/powermac/Kconfig
> index 055990c..85619d3 100644
> --- a/arch/powerpc/platforms/powermac/Kconfig
> +++ b/arch/powerpc/platforms/powermac/Kconfig
> @@ -6,7 +6,6 @@ config PPC_PMAC
>  	select PPC_INDIRECT_PCI if PPC32
>  	select PPC_MPC106 if PPC32
>  	select PPC_NATIVE
> -	default y
>  
>  config PPC_PMAC64
>  	bool
> @@ -16,6 +15,3 @@ config PPC_PMAC64
>  	select MPIC_U3_HT_IRQS
>  	select GENERIC_TBSYNC
>  	select PPC_970_NAP
> -	default y
> -
> -
Timur Tabi - Oct. 8, 2008, 8:14 p.m.
On Wed, Oct 8, 2008 at 3:03 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:

> No. Instead, send a patch that fixes the defconfig's to explicitely set
> those to "n". As to whether those should be defaults or not, this is a
> different discussion (I'm almost tempted to have everything default to
> y).

But this is the reason I sent you the patch in the first place.  I
think "default y" is wrong and doesn't make any sense, so I'm asking
you for a technical reason why PMAC and CHRP should default to yes.
That is the focus of my patch.  The defconfig changes are just to
clean up the mess that "default y" left behind.
Benjamin Herrenschmidt - Oct. 8, 2008, 8:39 p.m.
On Wed, 2008-10-08 at 15:14 -0500, Timur Tabi wrote:
> 
> But this is the reason I sent you the patch in the first place.  I
> think "default y" is wrong and doesn't make any sense, so I'm asking
> you for a technical reason why PMAC and CHRP should default to yes.
> That is the focus of my patch.  The defconfig changes are just to
> clean up the mess that "default y" left behind.

I'm happy with -all- platforms for a given CPU type defaulting to y. 

Ben.
Timur Tabi - Oct. 8, 2008, 8:44 p.m.
Benjamin Herrenschmidt wrote:

> I'm happy with -all- platforms for a given CPU type defaulting to y. 

I'm not sure I understand.  The current Kconfigs defaults to Y for CHRP and PMAC
on *all* PowerPC systems, including those that are neither CHRP nor PMAC.  Are
you saying that's a good thing?
Scott Wood - Oct. 8, 2008, 8:46 p.m.
On Thu, Oct 09, 2008 at 07:03:06AM +1100, Benjamin Herrenschmidt wrote:
> No. Instead, send a patch that fixes the defconfig's to explicitely set
> those to "n". 

What's the difference, other than wasting some bytes in the Kconfig file?

> As to whether those should be defaults or not, this is a different
> discussion (I'm almost tempted to have everything default to y).

Please, no.  It's much easier to turn on what you want, than to go
through menuconfig turning a million things off.

-Scott
Matt Sealey - Oct. 9, 2008, 3:59 p.m.
Timur Tabi wrote:
> Benjamin Herrenschmidt wrote:
> 
>> I'm happy with -all- platforms for a given CPU type defaulting to y. 
> 
> I'm not sure I understand.  The current Kconfigs defaults to Y for CHRP and PMAC
> on *all* PowerPC systems, including those that are neither CHRP nor PMAC.  Are
> you saying that's a good thing?

Timur,

You do realise it's entirely possible to build a kernel which supports 50
different Freescale CPUs all in one?

Enabling CHRP, PMAC support *AND* non-CHRP platforms is possible in the same
binary. The relevant code that differentiates them is in the boot wrapper
and this is selected by *how you compile the kernel* and not which objects
are compiled in. make zImage and make cuImage on the same kernel objects
will give you an 'identical' image just packaged in a different way.

If you really want to build a single-cpu single-board kernel, disable CHRP
and PMAC for those board configs, but the default definitely should be to
enable them all within reason (obviously coherent and non-coherent designs
cannot coexist in the same kernel, 32-bit and 64-bit do not play well in
the same kernel..)
Scott Wood - Oct. 9, 2008, 4:21 p.m.
Matt Sealey wrote:
> You do realise it's entirely possible to build a kernel which supports 50
> different Freescale CPUs all in one?
> 
> Enabling CHRP, PMAC support *AND* non-CHRP platforms is possible in the 
> same
> binary. 

It's possible to build a kernel with 50 network drivers, too.  That 
doesn't mean they should all be "default y".  The kernel build takes 
long enough as is.

> If you really want to build a single-cpu single-board kernel, disable CHRP
> and PMAC for those board configs,

If you really want to build a targets-everything kernel, then turn on 
everything.  Go ahead and create a 6xx_defconfig that has support for 
everything enabled.  But don't cause huge heaps of stuff to be enabled 
for everyone that does a "make oldconfig" with a config file that 
predates change d8267c1a36864fc30a2ce01f4349a8f2931ae741.

-Scott
Matt Sealey - Oct. 9, 2008, 4:49 p.m.
Timur Tabi wrote:
> On Thu, Oct 9, 2008 at 10:59 AM, Matt Sealey <matt@genesi-usa.com> wrote:
> 
>> If you really want to build a single-cpu single-board kernel, disable CHRP
>> and PMAC for those board configs, but the default definitely should be to
>> enable them all within reason
> 
> The problem is that this is inconsistent with most Kconfig options.
> Last I heard, the kernel community generally frowns on "default y" in
> Kconfig options.
> 
> I'm waiting for someone to explain to me what's so special about CHRP
> and PMAC that these two platforms should be enabled by default, when
> all other PowerPC platforms are disabled by default.
> 
> This is what I see in menuconfig when I create a fresh .config for PowerPC:
> 
>   │ │    [*] Common Hardware Reference Platform (CHRP) based machines (NEW)
>   │ │    [ ] Freescale MPC5121E ADS (NEW)
>   │ │    [ ] Generic support for simple MPC5121 based boards (NEW)
>   │ │    [ ] 52xx-based boards (NEW)
>   │ │    [*] Apple PowerMac based machines (NEW)
>   │ │    [ ] 82xx-based boards (PQ II) (NEW)  --->
>   │ │    [ ] 83xx-based boards (NEW)  --->
>   │ │    [ ] 86xx-based boards (NEW)  --->
>   │ │    [ ] Embedded 6xx/7xx/7xxx-based boards (NEW)
> 
> CHRP and PMAC aren't following the rules that everyone else is following.  Why?

Because they are by far the historically most common configuration, and
still in production as the defacto standard PowerPC system configuration.
IBM blades etc. with SLOF will boot up as a CHRP-ish system, as well as the
Efika and Pegasos and anything else Genesi produces. Since Linux distributions
generally do not support tiny embedded boards, you can imagine why it's
disabled by default, but there's no reason it can't be ENABLED by default
and turned off by a distribution, the same way it can't be enabled by
default and turned off by YOU (compare and contrast having to manually
select which board you want to build for every time).

But, turning them all on would not matter. You would build a kernel for
every one and a device tree for every one increasing your build time a
bit for a default kernel, but you would be guaranteed to get a kernel
binary somewhere in the tree that would work on all of them :)
Scott Wood - Oct. 9, 2008, 5:08 p.m.
Matt Sealey wrote:
>> CHRP and PMAC aren't following the rules that everyone else is 
>> following.  Why?
> 
> Because they are by far the historically most common configuration, and
> still in production as the defacto standard PowerPC system configuration.

So you could make an argument for it being turned on in 
arch/powerpc/defconfig, if we had one.  That doesn't mean it should be 
"default y".

> IBM blades etc. with SLOF will boot up as a CHRP-ish system, as well as the
> Efika and Pegasos and anything else Genesi produces. Since Linux 
> distributions
> generally do not support tiny embedded boards,

What do distributions have to do with it?  It's harder for distributions 
to set the options they want than for embedded developers?

> you can imagine why it's
> disabled by default, but there's no reason it can't be ENABLED by default
> and turned off by a distribution, the same way it can't be enabled by
> default and turned off by YOU (compare and contrast having to manually
> select which board you want to build for every time).

Likewise there's no reason that PMAC/CHRP can't be DISABLED by default, 
and turned on by YOU.

> But, turning them all on would not matter. You would build a kernel for
> every one and a device tree for every one increasing your build time a
> bit for a default kernel,

s/a bit/a lot/

> but you would be guaranteed to get a kernel
> binary somewhere in the tree that would work on all of them :)

Really?  Who do I go to for this guarantee, when there's no support for 
the hardware I'm trying to get to work? :-)

-Scott
Timur Tabi - Oct. 9, 2008, 5:23 p.m.
On Thu, Oct 9, 2008 at 11:49 AM, Matt Sealey <matt@genesi-usa.com> wrote:
> Because they are by far the historically most common configuration, and
> still in production as the defacto standard PowerPC system configuration.

Not really.  PMAC systems are not being built any more.  So that leaves CHRP.

> IBM blades etc. with SLOF will boot up as a CHRP-ish system, as well as the
> Efika and Pegasos and anything else Genesi produces. Since Linux
> distributions generally do not support tiny embedded boards,

So what?  Distributions don't need our help to turn on the options
that are important to them.  This is a ridiculous argument.

> you can imagine why it's
> disabled by default, but there's no reason it can't be ENABLED by default
> and turned off by a distribution, the same way it can't be enabled by
> default and turned off by YOU (compare and contrast having to manually
> select which board you want to build for every time).

This problem is solved with defconfigs.  Kconfig options are supposed
to make sense.  Not all PowerPC systems are CHRP, therefore CHRP
should not be enabled by default.

> But, turning them all on would not matter. You would build a kernel for
> every one and a device tree for every one increasing your build time a
> bit for a default kernel,

Not really true.  Having the default be disabled for specific
platforms can make a big difference in compile time.

> but you would be guaranteed to get a kernel
> binary somewhere in the tree that would work on all of them :)

Again, not relevant.  One giant kernel that works on a Freescale
embedded system *and* an IBM mainframe is useless.  No one would
actually ever do that.

> Genesi, Manager, Developer Relations

Developer Relations, eh?
Sven Luther - Oct. 9, 2008, 8:18 p.m.
On Thu, Oct 09, 2008 at 12:23:06PM -0500, Timur Tabi wrote:
> On Thu, Oct 9, 2008 at 11:49 AM, Matt Sealey <matt@genesi-usa.com> wrote:
> > Because they are by far the historically most common configuration, and
> > still in production as the defacto standard PowerPC system configuration.
> 
> Not really.  PMAC systems are not being built any more.  So that leaves CHRP.
> 
> > IBM blades etc. with SLOF will boot up as a CHRP-ish system, as well as the
> > Efika and Pegasos and anything else Genesi produces. Since Linux
> > distributions generally do not support tiny embedded boards,
> 
> So what?  Distributions don't need our help to turn on the options
> that are important to them.  This is a ridiculous argument.
> 
> > you can imagine why it's
> > disabled by default, but there's no reason it can't be ENABLED by default
> > and turned off by a distribution, the same way it can't be enabled by
> > default and turned off by YOU (compare and contrast having to manually
> > select which board you want to build for every time).
> 
> This problem is solved with defconfigs.  Kconfig options are supposed

...

> Not really true.  Having the default be disabled for specific
> platforms can make a big difference in compile time.

Notice that the defconfigs answer also applies here :)

Friendly,

Sven Luther
Timur Tabi - Oct. 9, 2008, 9:14 p.m.
On Thu, Oct 9, 2008 at 3:18 PM, Sven Luther <sven@genesi-usa.com> wrote:
>> Not really true.  Having the default be disabled for specific
>> platforms can make a big difference in compile time.
>
> Notice that the defconfigs answer also applies here :)

Not really.  In order for me to create a new defconfig for an embedded
platform, I have to turn *off* CHRP *and* PMAC support, and then turn
on whatever platform I want.

That's the point behind my patch.  The default is on, so anyone
creating a non-CHRP or non-PMAC platform has to do extra work.

And the argument that embedded platforms aren't as important as the
exalted CHRP is bollocks.  All platforms should be treated equally.
And like I said earlier, PMAC platforms aren't even being built any
more, so it's ridiculous to consider that the default.
Sven Luther - Oct. 9, 2008, 9:28 p.m.
On Thu, Oct 09, 2008 at 04:14:36PM -0500, Timur Tabi wrote:
> On Thu, Oct 9, 2008 at 3:18 PM, Sven Luther <sven@genesi-usa.com> wrote:
> >> Not really true.  Having the default be disabled for specific
> >> platforms can make a big difference in compile time.
> >
> > Notice that the defconfigs answer also applies here :)
> 
> Not really.  In order for me to create a new defconfig for an embedded
> platform, I have to turn *off* CHRP *and* PMAC support, and then turn
> on whatever platform I want.
> 
> That's the point behind my patch.  The default is on, so anyone
> creating a non-CHRP or non-PMAC platform has to do extra work.
> 
> And the argument that embedded platforms aren't as important as the
> exalted CHRP is bollocks.  All platforms should be treated equally.
> And like I said earlier, PMAC platforms aren't even being built any
> more, so it's ridiculous to consider that the default.

You just turn it on by default for 64bit and idsable it for 32bit :)

Friendly,

Sven Luther
Timur Tabi - Oct. 9, 2008, 9:32 p.m.
Sven Luther wrote:

> You just turn it on by default for 64bit and idsable it for 32bit :)

That hack might work for now, but it will break once we have 64-bit embedded
systems.  Regardless, it doesn't address the core issue - there should be no
default platform for PowerPC.
Paul Mackerras - Oct. 10, 2008, 12:43 a.m.
Timur Tabi writes:

> I'm waiting for someone to explain to me what's so special about CHRP
> and PMAC that these two platforms should be enabled by default, when
> all other PowerPC platforms are disabled by default.

Historically they have been the most important 32-bit platforms, along
with PReP.

The problem with making all platforms default to N is that if you take
the default values for all config options you end up with a config
that won't build.  So we want at least one platform to default to Y,
and pmac has been as good a choice as any for that (and probably still
is).

Paul.
Kumar Gala - Oct. 10, 2008, 1:22 a.m.
On Oct 9, 2008, at 7:43 PM, Paul Mackerras wrote:

> Timur Tabi writes:
>
>> I'm waiting for someone to explain to me what's so special about CHRP
>> and PMAC that these two platforms should be enabled by default, when
>> all other PowerPC platforms are disabled by default.
>
> Historically they have been the most important 32-bit platforms, along
> with PReP.
>
> The problem with making all platforms default to N is that if you take
> the default values for all config options you end up with a config
> that won't build.  So we want at least one platform to default to Y,
> and pmac has been as good a choice as any for that (and probably still
> is).

I've got no issue w/grandfathering CHRP or PMAC as default Y.  However  
I DO NOT think we should default other platforms to 'Y' as that means  
updating configs will pick up platforms they dont want.

- k
Timur Tabi - Oct. 10, 2008, 1:51 a.m.
On Thu, Oct 9, 2008 at 8:22 PM, Kumar Gala <galak@kernel.crashing.org> wrote:

>> The problem with making all platforms default to N is that if you take
>> the default values for all config options you end up with a config
>> that won't build.  So we want at least one platform to default to Y,
>> and pmac has been as good a choice as any for that (and probably still
>> is).

Ok, I can accept that.

> I've got no issue w/grandfathering CHRP or PMAC as default Y.  However I DO
> NOT think we should default other platforms to 'Y' as that means updating
> configs will pick up platforms they dont want.

As far as I know, CHRP and PMAC are the only PowerPC platforms that
default to Y.
Benjamin Herrenschmidt - Oct. 10, 2008, 3:01 a.m.
On Thu, 2008-10-09 at 20:22 -0500, Kumar Gala wrote:
> > Historically they have been the most important 32-bit platforms, along
> > with PReP.
> >
> > The problem with making all platforms default to N is that if you take
> > the default values for all config options you end up with a config
> > that won't build.  So we want at least one platform to default to Y,
> > and pmac has been as good a choice as any for that (and probably still
> > is).
> 
> I've got no issue w/grandfathering CHRP or PMAC as default Y.  However  
> I DO NOT think we should default other platforms to 'Y' as that means  
> updating configs will pick up platforms they dont want.

Ok. So Timur, please send a new patch on top of your previous one since
I already merged it, that explicitely sets PMAC and CHRP to "n" in your
defconfigs.

Ben.
Benjamin Herrenschmidt - Oct. 10, 2008, 3:01 a.m.
On Thu, 2008-10-09 at 20:51 -0500, Timur Tabi wrote:
> On Thu, Oct 9, 2008 at 8:22 PM, Kumar Gala <galak@kernel.crashing.org> wrote:
> 
> >> The problem with making all platforms default to N is that if you take
> >> the default values for all config options you end up with a config
> >> that won't build.  So we want at least one platform to default to Y,
> >> and pmac has been as good a choice as any for that (and probably still
> >> is).
> 
> Ok, I can accept that.
> 
> > I've got no issue w/grandfathering CHRP or PMAC as default Y.  However I DO
> > NOT think we should default other platforms to 'Y' as that means updating
> > configs will pick up platforms they dont want.
> 
> As far as I know, CHRP and PMAC are the only PowerPC platforms that
> default to Y.

There use to be PREP too I think :-)

Ben.

Patch

diff --git a/arch/powerpc/platforms/chrp/Kconfig b/arch/powerpc/platforms/chrp/Kconfig
index 22b4b4e..682afbc 100644
--- a/arch/powerpc/platforms/chrp/Kconfig
+++ b/arch/powerpc/platforms/chrp/Kconfig
@@ -9,4 +9,3 @@  config PPC_CHRP
 	select PPC_UDBG_16550
 	select PPC_NATIVE
 	select PCI
-	default y
diff --git a/arch/powerpc/platforms/powermac/Kconfig b/arch/powerpc/platforms/powermac/Kconfig
index 055990c..85619d3 100644
--- a/arch/powerpc/platforms/powermac/Kconfig
+++ b/arch/powerpc/platforms/powermac/Kconfig
@@ -6,7 +6,6 @@  config PPC_PMAC
 	select PPC_INDIRECT_PCI if PPC32
 	select PPC_MPC106 if PPC32
 	select PPC_NATIVE
-	default y
 
 config PPC_PMAC64
 	bool
@@ -16,6 +15,3 @@  config PPC_PMAC64
 	select MPIC_U3_HT_IRQS
 	select GENERIC_TBSYNC
 	select PPC_970_NAP
-	default y
-
-