diff mbox

[v2,1/4] arm64, thunder: Add Kconfig option for Cavium Thunder SoC Family

Message ID 1409903205-2762-2-git-send-email-rric@kernel.org (mailing list archive)
State New, archived
Headers show

Commit Message

Robert Richter Sept. 5, 2014, 7:46 a.m. UTC
From: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>

Increase maximum numbers of cpus to 32. This relates to current
maximal possible cpu number. Increasing this to 64 cpus will be a
separate patch not part of this enablement patches.

Signed-off-by: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
Signed-off-by: Robert Richter <rrichter@cavium.com>
---
 arch/arm64/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Will Deacon Sept. 5, 2014, 8:39 a.m. UTC | #1
On Fri, Sep 05, 2014 at 08:46:42AM +0100, Robert Richter wrote:
> From: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> 
> Increase maximum numbers of cpus to 32. This relates to current
> maximal possible cpu number. Increasing this to 64 cpus will be a
> separate patch not part of this enablement patches.

Just out of interest, does raising the current maximum limit actually break
any existing code? If not, then doing this as two patches doesn't seem worth
it.

Will
Robert Richter Sept. 5, 2014, 9:21 a.m. UTC | #2
On 05.09.14 09:39:32, Will Deacon wrote:
> On Fri, Sep 05, 2014 at 08:46:42AM +0100, Robert Richter wrote:
> > From: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> > 
> > Increase maximum numbers of cpus to 32. This relates to current
> > maximal possible cpu number. Increasing this to 64 cpus will be a
> > separate patch not part of this enablement patches.
> 
> Just out of interest, does raising the current maximum limit actually break
> any existing code? If not, then doing this as two patches doesn't seem worth
> it.

Increasing to 64 should be fine from the perspective of cpu mask
implementation. Memory foot print should be the same already as this
uses long which is 64 bit. So this wouldn't hurt.

However, I felt a bit uncomfortable having a dependency here to
enabling 64 cpus and getting this patch set upstream. Support for more
than 32 cpus is not well tested yet and there still might be problems
with e.g. interrupt delivery or topology.

If increasing to 32 cpus as an intermediate step isn't the best
approach here, we can live with the current defaults too. I would then
just remove that change from the patch.

For follow on driver patch sets it would be good to have ARCH_THUNDER
upstream as this option will be shared between all drivers.

-Robert
Will Deacon Sept. 5, 2014, 9:25 a.m. UTC | #3
On Fri, Sep 05, 2014 at 10:21:35AM +0100, Robert Richter wrote:
> On 05.09.14 09:39:32, Will Deacon wrote:
> > On Fri, Sep 05, 2014 at 08:46:42AM +0100, Robert Richter wrote:
> > > From: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> > > 
> > > Increase maximum numbers of cpus to 32. This relates to current
> > > maximal possible cpu number. Increasing this to 64 cpus will be a
> > > separate patch not part of this enablement patches.
> > 
> > Just out of interest, does raising the current maximum limit actually break
> > any existing code? If not, then doing this as two patches doesn't seem worth
> > it.
> 
> Increasing to 64 should be fine from the perspective of cpu mask
> implementation. Memory foot print should be the same already as this
> uses long which is 64 bit. So this wouldn't hurt.
> 
> However, I felt a bit uncomfortable having a dependency here to
> enabling 64 cpus and getting this patch set upstream. Support for more
> than 32 cpus is not well tested yet and there still might be problems
> with e.g. interrupt delivery or topology.

All I mean is, if the kernel doesn't explode on existing systems by changing
the upper limit to 64, then we should do that. If you're not comfortable
that the Thunder code can handle that, then leave the thunder default as 32,
like you do in the current patch. It just seems odd not to change the
maximum number, since it's an arbitrary limit from my perspective.

Will
Russell King - ARM Linux Sept. 5, 2014, 9:32 a.m. UTC | #4
On Fri, Sep 05, 2014 at 09:46:42AM +0200, Robert Richter wrote:
> From: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> 
> Increase maximum numbers of cpus to 32. This relates to current
> maximal possible cpu number. Increasing this to 64 cpus will be a
> separate patch not part of this enablement patches.
> 
> Signed-off-by: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> Signed-off-by: Robert Richter <rrichter@cavium.com>
> ---
>  arch/arm64/Kconfig | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index fd4e81a4e1ce..77fb82b0f684 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -134,6 +134,11 @@ source "kernel/Kconfig.freezer"
>  
>  menu "Platform selection"
>  
> +config ARCH_THUNDER
> +	bool "Cavium Inc. Thunder SoC Family"
> +	help
> +	  This enables support for Cavium's Thunder Family of SoCs.
> +
>  config ARCH_VEXPRESS
>  	bool "ARMv8 software model (Versatile Express)"
>  	select ARCH_REQUIRE_GPIOLIB
> @@ -256,6 +261,7 @@ config NR_CPUS
>  	range 2 32
>  	depends on SMP
>  	# These have to remain sorted largest to smallest
> +	default "32" if ARCH_THUNDER
>  	default "8"

Why do you need ARCH_THUNDER?  If it's just to change this default,
please don't bother - we don't do this kind of thing on x86, so why
should it be done here?  Just ensure that NR_CPUS is appropriately
adjusted.  Alternatively, look at how x86 deals with this (with
X86_BIGSMP / MAXSMP).
Mark Rutland Sept. 5, 2014, 9:36 a.m. UTC | #5
On Fri, Sep 05, 2014 at 10:25:08AM +0100, Will Deacon wrote:
> On Fri, Sep 05, 2014 at 10:21:35AM +0100, Robert Richter wrote:
> > On 05.09.14 09:39:32, Will Deacon wrote:
> > > On Fri, Sep 05, 2014 at 08:46:42AM +0100, Robert Richter wrote:
> > > > From: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> > > > 
> > > > Increase maximum numbers of cpus to 32. This relates to current
> > > > maximal possible cpu number. Increasing this to 64 cpus will be a
> > > > separate patch not part of this enablement patches.
> > > 
> > > Just out of interest, does raising the current maximum limit actually break
> > > any existing code? If not, then doing this as two patches doesn't seem worth
> > > it.
> > 
> > Increasing to 64 should be fine from the perspective of cpu mask
> > implementation. Memory foot print should be the same already as this
> > uses long which is 64 bit. So this wouldn't hurt.
> > 
> > However, I felt a bit uncomfortable having a dependency here to
> > enabling 64 cpus and getting this patch set upstream. Support for more
> > than 32 cpus is not well tested yet and there still might be problems
> > with e.g. interrupt delivery or topology.
> 
> All I mean is, if the kernel doesn't explode on existing systems by changing
> the upper limit to 64, then we should do that. If you're not comfortable
> that the Thunder code can handle that, then leave the thunder default as 32,
> like you do in the current patch. It just seems odd not to change the
> maximum number, since it's an arbitrary limit from my perspective.

FWIW my Juno happily boots with a NR_CPUS=64 kernel.

Mark.
Robert Richter Sept. 5, 2014, 10:45 a.m. UTC | #6
On 05.09.14 10:32:40, Russell King - ARM Linux wrote:
> On Fri, Sep 05, 2014 at 09:46:42AM +0200, Robert Richter wrote:
> > From: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> > 
> > Increase maximum numbers of cpus to 32. This relates to current
> > maximal possible cpu number. Increasing this to 64 cpus will be a
> > separate patch not part of this enablement patches.
> > 
> > Signed-off-by: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> > Signed-off-by: Robert Richter <rrichter@cavium.com>
> > ---
> >  arch/arm64/Kconfig | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > index fd4e81a4e1ce..77fb82b0f684 100644
> > --- a/arch/arm64/Kconfig
> > +++ b/arch/arm64/Kconfig
> > @@ -134,6 +134,11 @@ source "kernel/Kconfig.freezer"
> >  
> >  menu "Platform selection"
> >  
> > +config ARCH_THUNDER
> > +	bool "Cavium Inc. Thunder SoC Family"
> > +	help
> > +	  This enables support for Cavium's Thunder Family of SoCs.
> > +
> >  config ARCH_VEXPRESS
> >  	bool "ARMv8 software model (Versatile Express)"
> >  	select ARCH_REQUIRE_GPIOLIB
> > @@ -256,6 +261,7 @@ config NR_CPUS
> >  	range 2 32
> >  	depends on SMP
> >  	# These have to remain sorted largest to smallest
> > +	default "32" if ARCH_THUNDER
> >  	default "8"
> 
> Why do you need ARCH_THUNDER?  If it's just to change this default,

No, we need it just to enable all our drivers on the SoC. We want to
enable the SoC by using defconfig + ARCH_THUNDER. As said in my other
mail, I put it here to be able to base all other thunder driver patch
sets on this initial base patch set. Otherwise this particular patch
and also the dtb patch need to be shipped with all other driver patch
sets. This might lead to duplicate submissions of the same patch.

With doing defconfig + ARCH_THUNDER we also want to enable the max
number of cpus that is currently supported. I only enable 32 cpus
since booting more cpus is untested. There might be problems at the 32
cpu boundary. Just setting it to 64 does not mean a kernel will
actually boot more than 32 cpus. But if it will be ack'ed, I would be
fine to set NR_CPUS to 32 or 64 in general and independent from
ARCH_THUNDER.

For simplicity I better drop setting NR_CPUS in this patch.

-Robert

> please don't bother - we don't do this kind of thing on x86, so why
> should it be done here?  Just ensure that NR_CPUS is appropriately
> adjusted.  Alternatively, look at how x86 deals with this (with
> X86_BIGSMP / MAXSMP).
Robert Richter Sept. 5, 2014, 10:51 a.m. UTC | #7
On 05.09.14 10:36:47, Mark Rutland wrote:
> On Fri, Sep 05, 2014 at 10:25:08AM +0100, Will Deacon wrote:
> > All I mean is, if the kernel doesn't explode on existing systems by changing
> > the upper limit to 64, then we should do that. If you're not comfortable
> > that the Thunder code can handle that, then leave the thunder default as 32,
> > like you do in the current patch. It just seems odd not to change the
> > maximum number, since it's an arbitrary limit from my perspective.
> 
> FWIW my Juno happily boots with a NR_CPUS=64 kernel.

It looks like if it's fine to set the general default to NR_CPUS=64
even if the kernel might not yet boot more than 32 cpus. Will make
this change separately and drop it in this patch.

See also my other reply to Russell in this thread.

-Robert
Russell King - ARM Linux Sept. 5, 2014, 11:05 a.m. UTC | #8
On Fri, Sep 05, 2014 at 12:45:47PM +0200, Robert Richter wrote:
> On 05.09.14 10:32:40, Russell King - ARM Linux wrote:
> > On Fri, Sep 05, 2014 at 09:46:42AM +0200, Robert Richter wrote:
> > > From: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> > > 
> > > Increase maximum numbers of cpus to 32. This relates to current
> > > maximal possible cpu number. Increasing this to 64 cpus will be a
> > > separate patch not part of this enablement patches.
> > > 
> > > Signed-off-by: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> > > Signed-off-by: Robert Richter <rrichter@cavium.com>
> > > ---
> > >  arch/arm64/Kconfig | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > index fd4e81a4e1ce..77fb82b0f684 100644
> > > --- a/arch/arm64/Kconfig
> > > +++ b/arch/arm64/Kconfig
> > > @@ -134,6 +134,11 @@ source "kernel/Kconfig.freezer"
> > >  
> > >  menu "Platform selection"
> > >  
> > > +config ARCH_THUNDER
> > > +	bool "Cavium Inc. Thunder SoC Family"
> > > +	help
> > > +	  This enables support for Cavium's Thunder Family of SoCs.
> > > +
> > >  config ARCH_VEXPRESS
> > >  	bool "ARMv8 software model (Versatile Express)"
> > >  	select ARCH_REQUIRE_GPIOLIB
> > > @@ -256,6 +261,7 @@ config NR_CPUS
> > >  	range 2 32
> > >  	depends on SMP
> > >  	# These have to remain sorted largest to smallest
> > > +	default "32" if ARCH_THUNDER
> > >  	default "8"
> > 
> > Why do you need ARCH_THUNDER?  If it's just to change this default,
> 
> No, we need it just to enable all our drivers on the SoC. We want to
> enable the SoC by using defconfig + ARCH_THUNDER. As said in my other
> mail, I put it here to be able to base all other thunder driver patch
> sets on this initial base patch set. Otherwise this particular patch
> and also the dtb patch need to be shipped with all other driver patch
> sets. This might lead to duplicate submissions of the same patch.
> 
> With doing defconfig + ARCH_THUNDER we also want to enable the max
> number of cpus that is currently supported. I only enable 32 cpus
> since booting more cpus is untested. There might be problems at the 32
> cpu boundary. Just setting it to 64 does not mean a kernel will
> actually boot more than 32 cpus. But if it will be ack'ed, I would be
> fine to set NR_CPUS to 32 or 64 in general and independent from
> ARCH_THUNDER.
> 
> For simplicity I better drop setting NR_CPUS in this patch.

So, ARM64 will get a big long list of "config ARCH_foo" options just
to stuff lots of broken select statements into the configuration.  Yes,
this may have been the norm with ARM, but it's turned out to be more
of a problem than a solution, especially as it keeps causing Kconfig
warnings when things change in the rest of the kernel tree.

The same is true with defconfigs - Linus threatened to delete all ARM
defconfigs except one at one point.

As I said below, this isn't how stuff is dealt with on x86.

What I'm questioning here is the entire ethos of "have an ARCH_foo
configuration which sets stuff up for platform foo".

> > please don't bother - we don't do this kind of thing on x86, so why
> > should it be done here?  Just ensure that NR_CPUS is appropriately
> > adjusted.  Alternatively, look at how x86 deals with this (with
> > X86_BIGSMP / MAXSMP).
Mark Rutland Sept. 5, 2014, 12:51 p.m. UTC | #9
On Fri, Sep 05, 2014 at 12:05:52PM +0100, Russell King - ARM Linux wrote:
> On Fri, Sep 05, 2014 at 12:45:47PM +0200, Robert Richter wrote:
> > On 05.09.14 10:32:40, Russell King - ARM Linux wrote:
> > > On Fri, Sep 05, 2014 at 09:46:42AM +0200, Robert Richter wrote:
> > > > From: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> > > > 
> > > > Increase maximum numbers of cpus to 32. This relates to current
> > > > maximal possible cpu number. Increasing this to 64 cpus will be a
> > > > separate patch not part of this enablement patches.
> > > > 
> > > > Signed-off-by: Radha Mohan Chintakuntla <rchintakuntla@cavium.com>
> > > > Signed-off-by: Robert Richter <rrichter@cavium.com>
> > > > ---
> > > >  arch/arm64/Kconfig | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> > > > index fd4e81a4e1ce..77fb82b0f684 100644
> > > > --- a/arch/arm64/Kconfig
> > > > +++ b/arch/arm64/Kconfig
> > > > @@ -134,6 +134,11 @@ source "kernel/Kconfig.freezer"
> > > >  
> > > >  menu "Platform selection"
> > > >  
> > > > +config ARCH_THUNDER
> > > > +	bool "Cavium Inc. Thunder SoC Family"
> > > > +	help
> > > > +	  This enables support for Cavium's Thunder Family of SoCs.
> > > > +
> > > >  config ARCH_VEXPRESS
> > > >  	bool "ARMv8 software model (Versatile Express)"
> > > >  	select ARCH_REQUIRE_GPIOLIB
> > > > @@ -256,6 +261,7 @@ config NR_CPUS
> > > >  	range 2 32
> > > >  	depends on SMP
> > > >  	# These have to remain sorted largest to smallest
> > > > +	default "32" if ARCH_THUNDER
> > > >  	default "8"
> > > 
> > > Why do you need ARCH_THUNDER?  If it's just to change this default,
> > 
> > No, we need it just to enable all our drivers on the SoC. We want to
> > enable the SoC by using defconfig + ARCH_THUNDER. As said in my other
> > mail, I put it here to be able to base all other thunder driver patch
> > sets on this initial base patch set. Otherwise this particular patch
> > and also the dtb patch need to be shipped with all other driver patch
> > sets. This might lead to duplicate submissions of the same patch.
> > 
> > With doing defconfig + ARCH_THUNDER we also want to enable the max
> > number of cpus that is currently supported. I only enable 32 cpus
> > since booting more cpus is untested. There might be problems at the 32
> > cpu boundary. Just setting it to 64 does not mean a kernel will
> > actually boot more than 32 cpus. But if it will be ack'ed, I would be
> > fine to set NR_CPUS to 32 or 64 in general and independent from
> > ARCH_THUNDER.
> > 
> > For simplicity I better drop setting NR_CPUS in this patch.
> 
> So, ARM64 will get a big long list of "config ARCH_foo" options just
> to stuff lots of broken select statements into the configuration.  Yes,
> this may have been the norm with ARM, but it's turned out to be more
> of a problem than a solution, especially as it keeps causing Kconfig
> warnings when things change in the rest of the kernel tree.

Agreed; this seems more pain than it is worth.

> The same is true with defconfigs - Linus threatened to delete all ARM
> defconfigs except one at one point.

IMO we should continue doing what we've done so far and make the ARM64
defconfig work on everything it can by default, no ARCH_* necessary.
That's what most people will build and test and we shouldn't get
platform-specific code (well, drivers) breaking the single image.

For the extreme configurations (really tiny or really big) custom
configuration being necessary is fine. That doesn't have to involve
ARCH_* config options.

If you want to build a custom config then you should have an idea of
what you need. ARCH_* options are only necessary if someone wants a
kernel tuned for a specific SoC but doesn't know anything about that
SoC.

Mark.
Arnd Bergmann Sept. 5, 2014, 2:04 p.m. UTC | #10
On Friday 05 September 2014 13:51:47 Mark Rutland wrote:
> On Fri, Sep 05, 2014 at 12:05:52PM +0100, Russell King - ARM Linux wrote:
> > On Fri, Sep 05, 2014 at 12:45:47PM +0200, Robert Richter wrote:
> > > On 05.09.14 10:32:40, Russell King - ARM Linux wrote:
> > > No, we need it just to enable all our drivers on the SoC. We want to
> > > enable the SoC by using defconfig + ARCH_THUNDER. As said in my other
> > > mail, I put it here to be able to base all other thunder driver patch
> > > sets on this initial base patch set. Otherwise this particular patch
> > > and also the dtb patch need to be shipped with all other driver patch
> > > sets. This might lead to duplicate submissions of the same patch.
> > > 
> > > With doing defconfig + ARCH_THUNDER we also want to enable the max
> > > number of cpus that is currently supported. I only enable 32 cpus
> > > since booting more cpus is untested. There might be problems at the 32
> > > cpu boundary. Just setting it to 64 does not mean a kernel will
> > > actually boot more than 32 cpus. But if it will be ack'ed, I would be
> > > fine to set NR_CPUS to 32 or 64 in general and independent from
> > > ARCH_THUNDER.
> > > 
> > > For simplicity I better drop setting NR_CPUS in this patch.
> > 
> > So, ARM64 will get a big long list of "config ARCH_foo" options just
> > to stuff lots of broken select statements into the configuration.  Yes,
> > this may have been the norm with ARM, but it's turned out to be more
> > of a problem than a solution, especially as it keeps causing Kconfig
> > warnings when things change in the rest of the kernel tree.
> 
> Agreed; this seems more pain than it is worth.

Lots of select statements indeed would be a problem, but I don't think
that is what Robert was suggesting.

> > The same is true with defconfigs - Linus threatened to delete all ARM
> > defconfigs except one at one point.
> 
> IMO we should continue doing what we've done so far and make the ARM64
> defconfig work on everything it can by default, no ARCH_* necessary.
> That's what most people will build and test and we shouldn't get
> platform-specific code (well, drivers) breaking the single image.

Right.

> For the extreme configurations (really tiny or really big) custom
> configuration being necessary is fine. That doesn't have to involve
> ARCH_* config options.
> 
> If you want to build a custom config then you should have an idea of
> what you need. ARCH_* options are only necessary if someone wants a
> kernel tuned for a specific SoC but doesn't know anything about that
> SoC.

A common pattern these days is to do dependencies like

arch/*/Kconfig:
	config ARCH_FOO
	bool "Enable support for Foo platform"
	help
	  ...


drivers/*/Kconfig
	config SUBSYS_FOO
	bool "SUBSYS driver for Foo"
	depends on ARCH_FOO || COMPILE_TEST
	depends on OF && REGULATOR && GENERIC_PHY # or whatever

That way we can enable everything in the defconfig, but someone
who likes to build a more specialized kernel can disable the
other platforms and won't get the drivers that are specific to
those.

I personally think this is a bit more verbose than what we need, but
I don't strongly object doing it that way.

The code size really should not matter much on ARM64 though: it's
unlikely we will see a lot of systems with less than a few gigabytes
of memory, and I expect that a generic kernel would be e.g. 6 MB
instead of 4 MB for a platform specific kernel.

	Arnd
Mark Rutland Sept. 5, 2014, 2:22 p.m. UTC | #11
Hi Arnd,

[...]

> A common pattern these days is to do dependencies like
> 
> arch/*/Kconfig:
> 	config ARCH_FOO
> 	bool "Enable support for Foo platform"
> 	help
> 	  ...
> 
> 
> drivers/*/Kconfig
> 	config SUBSYS_FOO
> 	bool "SUBSYS driver for Foo"
> 	depends on ARCH_FOO || COMPILE_TEST
> 	depends on OF && REGULATOR && GENERIC_PHY # or whatever

Russell's comments w.r.t. Kconfig warnings when config names change
still holds regardless of select vs depends on.

> That way we can enable everything in the defconfig, but someone
> who likes to build a more specialized kernel can disable the
> other platforms and won't get the drivers that are specific to
> those.
> 
> I personally think this is a bit more verbose than what we need, but
> I don't strongly object doing it that way.

You'd still be able to do this without ARCH_FOO, though you would need
to know which drivers are necessary for a particular SoC. That seems to
be the way things are handled on x86; I don't recall having to select
support for specific machines there, just the individual drivers.

> The code size really should not matter much on ARM64 though: it's
> unlikely we will see a lot of systems with less than a few gigabytes
> of memory, and I expect that a generic kernel would be e.g. 6 MB
> instead of 4 MB for a platform specific kernel.

Agreed. If there kernel size begins looking unwieldy we either need to
be compiling more things as modules or a more drastic kernel weight loss
program.

Mark.
Arnd Bergmann Sept. 5, 2014, 4:25 p.m. UTC | #12
On Friday 05 September 2014 15:22:46 Mark Rutland wrote:
> 
> > A common pattern these days is to do dependencies like
> > 
> > arch/*/Kconfig:
> >       config ARCH_FOO
> >       bool "Enable support for Foo platform"
> >       help
> >         ...
> > 
> > 
> > drivers/*/Kconfig
> >       config SUBSYS_FOO
> >       bool "SUBSYS driver for Foo"
> >       depends on ARCH_FOO || COMPILE_TEST
> >       depends on OF && REGULATOR && GENERIC_PHY # or whatever
> 
> Russell's comments w.r.t. Kconfig warnings when config names change
> still holds regardless of select vs depends on.

Yes, that's what I wrote in my reply as well.

> > That way we can enable everything in the defconfig, but someone
> > who likes to build a more specialized kernel can disable the
> > other platforms and won't get the drivers that are specific to
> > those.
> > 
> > I personally think this is a bit more verbose than what we need, but
> > I don't strongly object doing it that way.
> 
> You'd still be able to do this without ARCH_FOO, though you would need
> to know which drivers are necessary for a particular SoC. That seems to
> be the way things are handled on x86; I don't recall having to select
> support for specific machines there, just the individual drivers.

The main difference is that there are very few drivers on x86 that are
specific to one of the two chip makers. Almost everything is a PCI
device that can actually be plugged in anywhere.

On ARM64 there is going to be a lot of stuff that really makes sense
only for one of the 50 licensees.

	Arnd
Robert Richter Sept. 8, 2014, 11:01 a.m. UTC | #13
On 05.09.14 18:25:35, Arnd Bergmann wrote:
> On Friday 05 September 2014 15:22:46 Mark Rutland wrote:
> > 
> > > A common pattern these days is to do dependencies like
> > > 
> > > arch/*/Kconfig:
> > >       config ARCH_FOO
> > >       bool "Enable support for Foo platform"
> > >       help
> > >         ...
> > > 
> > > 
> > > drivers/*/Kconfig
> > >       config SUBSYS_FOO
> > >       bool "SUBSYS driver for Foo"
> > >       depends on ARCH_FOO || COMPILE_TEST
> > >       depends on OF && REGULATOR && GENERIC_PHY # or whatever
> >
> > Russell's comments w.r.t. Kconfig warnings when config names change
> > still holds regardless of select vs depends on.
> 
> Yes, that's what I wrote in my reply as well.
> 
> > > That way we can enable everything in the defconfig, but someone
> > > who likes to build a more specialized kernel can disable the
> > > other platforms and won't get the drivers that are specific to
> > > those.
> > > 
> > > I personally think this is a bit more verbose than what we need, but
> > > I don't strongly object doing it that way.
> > 
> > You'd still be able to do this without ARCH_FOO, though you would need
> > to know which drivers are necessary for a particular SoC. That seems to
> > be the way things are handled on x86; I don't recall having to select
> > support for specific machines there, just the individual drivers.

This is well hidden on x86, but each vendor has a config option. For
AMD systems we have had our own option to disable vendor specific
code, see CPU_SUP_AMD for example. Disabling this will remove all AMD
specific code in the kernel. Of course this is enabled per default.

With ARCH_THUNDER I intended to do the same (you could name this also
SOC_SUP_CAVIUM or so but I kept the current naming scheme). In patch
4/4 I have added the option to defconfig. This enables this per
default and nobody has to deal with any option manually, just running
make defconfig is fine.

Also, at least to disable building the dtb file for foo, you will need
ARCH_FOO too. How else would you deal with dtb files then?

Having ARCH_FOO might not be necessary for drivers. One could enable
drivers manually, but this option is still a good reference for the
drivers needed by foo. At some point you will carry tons of enabled
drivers in your defconfig and you don't know which platform actually
is using it. For generic drivers this might be fine.  But in my point
of few, each soc specific driver should have an soc specific option
too. Then you easily can remove an soc from the defconfig.

-Robert

> The main difference is that there are very few drivers on x86 that are
> specific to one of the two chip makers. Almost everything is a PCI
> device that can actually be plugged in anywhere.
> 
> On ARM64 there is going to be a lot of stuff that really makes sense
> only for one of the 50 licensees.
Arnd Bergmann Sept. 8, 2014, 12:29 p.m. UTC | #14
On Monday 08 September 2014 13:01:31 Robert Richter wrote:
> This is well hidden on x86, but each vendor has a config option. For
> AMD systems we have had our own option to disable vendor specific
> code, see CPU_SUP_AMD for example. Disabling this will remove all AMD
> specific code in the kernel. Of course this is enabled per default.
> 
> With ARCH_THUNDER I intended to do the same (you could name this also
> SOC_SUP_CAVIUM or so but I kept the current naming scheme). In patch
> 4/4 I have added the option to defconfig. This enables this per
> default and nobody has to deal with any option manually, just running
> make defconfig is fine.
> 
> Also, at least to disable building the dtb file for foo, you will need
> ARCH_FOO too. How else would you deal with dtb files then?

The only alternative I see is to build them all.

> Having ARCH_FOO might not be necessary for drivers. One could enable
> drivers manually, but this option is still a good reference for the
> drivers needed by foo. At some point you will carry tons of enabled
> drivers in your defconfig and you don't know which platform actually
> is using it. For generic drivers this might be fine.  But in my point
> of few, each soc specific driver should have an soc specific option
> too. Then you easily can remove an soc from the defconfig.

Yes, that's what I had in my example.

	Arnd
Rob Herring Sept. 8, 2014, 6:25 p.m. UTC | #15
On Mon, Sep 8, 2014 at 6:01 AM, Robert Richter <rric@kernel.org> wrote:
> On 05.09.14 18:25:35, Arnd Bergmann wrote:
>> On Friday 05 September 2014 15:22:46 Mark Rutland wrote:
>> >
>> > > A common pattern these days is to do dependencies like
>> > >
>> > > arch/*/Kconfig:
>> > >       config ARCH_FOO
>> > >       bool "Enable support for Foo platform"
>> > >       help
>> > >         ...
>> > >
>> > >
>> > > drivers/*/Kconfig
>> > >       config SUBSYS_FOO
>> > >       bool "SUBSYS driver for Foo"
>> > >       depends on ARCH_FOO || COMPILE_TEST
>> > >       depends on OF && REGULATOR && GENERIC_PHY # or whatever
>> >
>> > Russell's comments w.r.t. Kconfig warnings when config names change
>> > still holds regardless of select vs depends on.
>>
>> Yes, that's what I wrote in my reply as well.
>>
>> > > That way we can enable everything in the defconfig, but someone
>> > > who likes to build a more specialized kernel can disable the
>> > > other platforms and won't get the drivers that are specific to
>> > > those.
>> > >
>> > > I personally think this is a bit more verbose than what we need, but
>> > > I don't strongly object doing it that way.
>> >
>> > You'd still be able to do this without ARCH_FOO, though you would need
>> > to know which drivers are necessary for a particular SoC. That seems to
>> > be the way things are handled on x86; I don't recall having to select
>> > support for specific machines there, just the individual drivers.
>
> This is well hidden on x86, but each vendor has a config option. For
> AMD systems we have had our own option to disable vendor specific
> code, see CPU_SUP_AMD for example. Disabling this will remove all AMD
> specific code in the kernel. Of course this is enabled per default.
>
> With ARCH_THUNDER I intended to do the same (you could name this also
> SOC_SUP_CAVIUM or so but I kept the current naming scheme). In patch
> 4/4 I have added the option to defconfig. This enables this per
> default and nobody has to deal with any option manually, just running
> make defconfig is fine.
>
> Also, at least to disable building the dtb file for foo, you will need
> ARCH_FOO too. How else would you deal with dtb files then?

dtb builds are quick and don't affect the image size at all.
Personally, I would prefer to see all dtb's get built for the dtbs
rule. You can still specify the specific dtb you want in the case of
only caring about 1 platform.

Rob
diff mbox

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index fd4e81a4e1ce..77fb82b0f684 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -134,6 +134,11 @@  source "kernel/Kconfig.freezer"
 
 menu "Platform selection"
 
+config ARCH_THUNDER
+	bool "Cavium Inc. Thunder SoC Family"
+	help
+	  This enables support for Cavium's Thunder Family of SoCs.
+
 config ARCH_VEXPRESS
 	bool "ARMv8 software model (Versatile Express)"
 	select ARCH_REQUIRE_GPIOLIB
@@ -256,6 +261,7 @@  config NR_CPUS
 	range 2 32
 	depends on SMP
 	# These have to remain sorted largest to smallest
+	default "32" if ARCH_THUNDER
 	default "8"
 
 config HOTPLUG_CPU