[PATCH] ARM: at91: force CPU selection

[PATCH] ARM: at91: force CPU selection

[Brian Norris]

The mach-at91 build does not force the user to make a CPU selection.
This can yield a build failure when SOC_SAM_V7 is used, but neither
SOC_SAMA5D4 nor SOC_SAMA5D3 are defined:

    arch/arm/include/asm/glue-pf.h:54:2: error: #error Unknown prefetch abort handler type
     #error Unknown prefetch abort handler type
      ^
    make[2]: *** [arch/arm/kernel/asm-offsets.s] Error 1

The forced default selection is completely arbitrary.

Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 arch/arm/mach-at91/Kconfig | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
index c74a44324e5b..afbf03e96746 100644
--- a/arch/arm/mach-at91/Kconfig
+++ b/arch/arm/mach-at91/Kconfig
@@ -114,6 +114,12 @@ config SOC_AT91SAM9
 	    AT91SAM9XE
 endif # SOC_SAM_V4_V5
 
+config MACH_AT91_AUTO
+	def_bool y
+	depends on !SOC_AT91SAM9 && !SOC_SAMA5D4
+	select SOC_SAMA5D3 if SOC_SAM_V7
+	select SOC_AT91RM9200 if SOC_SAM_V4_V5
+
 comment "AT91 Feature Selections"
 
 config AT91_SLOW_CLOCK
-- 
2.3.2

Re: [PATCH] ARM: at91: force CPU selection

[Arnd Bergmann]

On Monday 09 March 2015 23:59:58 Brian Norris wrote:
> @@ -114,6 +114,12 @@ config SOC_AT91SAM9
>             AT91SAM9XE
>  endif # SOC_SAM_V4_V5
>  
> +config MACH_AT91_AUTO
> +       def_bool y
> +       depends on !SOC_AT91SAM9 && !SOC_SAMA5D4
> +       select SOC_SAMA5D3 if SOC_SAM_V7
> +       select SOC_AT91RM9200 if SOC_SAM_V4_V5
> +
>  comment "AT91 Feature Selections"
>  
>  config AT91_SLOW_CLOCK

Hi Brian,

I actually added a bunch of these in other places, but have stopped
doing so because Russell didn't like them, and I tend to follow
his argument now that it's actually pretty confusing.

In case of at91, we can do better now, once we have multiplatform
enabled, and we can simply remove the SOC_SAM_V7 and SOC_SAM_V4_V5
symbols entirely (if Alexandre's patch doesn't already do that).

With multiplatform enabled, you no longer have to select a particular
SoC, because we always get the default selection and the kernel
will build without any one of them being enabled.

	Arnd

Re: [PATCH] ARM: at91: force CPU selection

[Alexandre Belloni]

On 10/03/2015 at 10:02:15 +0100, Arnd Bergmann wrote :
> On Monday 09 March 2015 23:59:58 Brian Norris wrote:
> > @@ -114,6 +114,12 @@ config SOC_AT91SAM9
> >             AT91SAM9XE
> >  endif # SOC_SAM_V4_V5
> >  
> > +config MACH_AT91_AUTO
> > +       def_bool y
> > +       depends on !SOC_AT91SAM9 && !SOC_SAMA5D4
> > +       select SOC_SAMA5D3 if SOC_SAM_V7
> > +       select SOC_AT91RM9200 if SOC_SAM_V4_V5
> > +
> >  comment "AT91 Feature Selections"
> >  
> >  config AT91_SLOW_CLOCK
> 
> Hi Brian,
> 
> I actually added a bunch of these in other places, but have stopped
> doing so because Russell didn't like them, and I tend to follow
> his argument now that it's actually pretty confusing.
> 
> In case of at91, we can do better now, once we have multiplatform
> enabled, and we can simply remove the SOC_SAM_V7 and SOC_SAM_V4_V5
> symbols entirely (if Alexandre's patch doesn't already do that).
> 

I didn't remove it to keep the sub menus SOC_SAM_V7 -> SOC_SAMA5D4 and
SOC_SAMA5D3, SOC_SAM_V4_V5 -> SOC_AT91SAM9 and SOC_AT91RM9200. But I can
also expose SOC_AT91RM9200, SOC_AT91SAM9, SOC_SAMA5D3 and SOC_SAMA5D4
instead of SOC_SAM_V7 and SOC_SAM_V4_V5 and remove them.

> With multiplatform enabled, you no longer have to select a particular
> SoC, because we always get the default selection and the kernel
> will build without any one of them being enabled.
> 

Indeed, with multiplatform, the kernel will compile without any of those
being selected.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH] ARM: at91: force CPU selection

[Arnd Bergmann]

On Tuesday 10 March 2015 10:42:09 Alexandre Belloni wrote:
> > 
> > I actually added a bunch of these in other places, but have stopped
> > doing so because Russell didn't like them, and I tend to follow
> > his argument now that it's actually pretty confusing.
> > 
> > In case of at91, we can do better now, once we have multiplatform
> > enabled, and we can simply remove the SOC_SAM_V7 and SOC_SAM_V4_V5
> > symbols entirely (if Alexandre's patch doesn't already do that).
> > 
> 
> I didn't remove it to keep the sub menus SOC_SAM_V7 -> SOC_SAMA5D4 and
> SOC_SAMA5D3, SOC_SAM_V4_V5 -> SOC_AT91SAM9 and SOC_AT91RM9200. But I can
> also expose SOC_AT91RM9200, SOC_AT91SAM9, SOC_SAMA5D3 and SOC_SAMA5D4
> instead of SOC_SAM_V7 and SOC_SAM_V4_V5 and remove them.

Exposing multiple levels won't help here since you can never enable
both v4/v5 and v7 at the same time in multiplatform.

Just use one level for a 'menuconfig ARCH_AT91' entry and put the
four options below that.

	Arnd

Re: [PATCH] ARM: at91: force CPU selection

[Brian Norris]

On Tue, Mar 10, 2015 at 10:02:15AM +0100, Arnd Bergmann wrote:
> I actually added a bunch of these in other places, but have stopped
> doing so because Russell didn't like them, and I tend to follow
> his argument now that it's actually pretty confusing.

I'm not a big fan of my patch, but it did at least solve the problem.
Thanks for the info, and I'm happy to drop my patch. At least I've fixed
my compile-test setups to avoid bad combinations, now.

To be clear, multiplatform support is in progress, but not merged yet,
right?

Brian

Re: [PATCH] ARM: at91: force CPU selection

[Alexandre Belloni]

On 10/03/2015 at 13:02:09 -0700, Brian Norris wrote :
> On Tue, Mar 10, 2015 at 10:02:15AM +0100, Arnd Bergmann wrote:
> > I actually added a bunch of these in other places, but have stopped
> > doing so because Russell didn't like them, and I tend to follow
> > his argument now that it's actually pretty confusing.
> 
> I'm not a big fan of my patch, but it did at least solve the problem.
> Thanks for the info, and I'm happy to drop my patch. At least I've fixed
> my compile-test setups to avoid bad combinations, now.
> 
> To be clear, multiplatform support is in progress, but not merged yet,
> right?
> 

I can point you to the patch series if you want. Hopefully, this will be
merged for 4.1


-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH] ARM: at91: force CPU selection

[Alexandre Belloni]

On 10/03/2015 at 14:42:24 +0100, Arnd Bergmann wrote :
> On Tuesday 10 March 2015 10:42:09 Alexandre Belloni wrote:
> > > 
> > > I actually added a bunch of these in other places, but have stopped
> > > doing so because Russell didn't like them, and I tend to follow
> > > his argument now that it's actually pretty confusing.
> > > 
> > > In case of at91, we can do better now, once we have multiplatform
> > > enabled, and we can simply remove the SOC_SAM_V7 and SOC_SAM_V4_V5
> > > symbols entirely (if Alexandre's patch doesn't already do that).
> > > 
> > 
> > I didn't remove it to keep the sub menus SOC_SAM_V7 -> SOC_SAMA5D4 and
> > SOC_SAMA5D3, SOC_SAM_V4_V5 -> SOC_AT91SAM9 and SOC_AT91RM9200. But I can
> > also expose SOC_AT91RM9200, SOC_AT91SAM9, SOC_SAMA5D3 and SOC_SAMA5D4
> > instead of SOC_SAM_V7 and SOC_SAM_V4_V5 and remove them.
> 
> Exposing multiple levels won't help here since you can never enable
> both v4/v5 and v7 at the same time in multiplatform.
> 
> Just use one level for a 'menuconfig ARCH_AT91' entry and put the
> four options below that.
> 

Actually, I had a closer look today and there is already just one level
in my current version of patches.

I can put everything under menuconfig ARCH_AT91 but I can't remove
SOC_SAM_V7 and SOC_SAM_V4_V5 because that is what is used to select
which dtb should be compiled.

This would also break sama5_defconfig. This is a simple fix but that
means that it makes it difficult to migrate an existing .config.

I'd say let's keep it that way as this is not broken, unless you really
want to see SOC_SAM_V7 and SOC_SAM_V4_V5 being removed.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH] ARM: at91: force CPU selection

[Arnd Bergmann]

On Friday 13 March 2015 19:32:00 Alexandre Belloni wrote:
> On 10/03/2015 at 14:42:24 +0100, Arnd Bergmann wrote :
> > On Tuesday 10 March 2015 10:42:09 Alexandre Belloni wrote:
> > > > 
> > > > I actually added a bunch of these in other places, but have stopped
> > > > doing so because Russell didn't like them, and I tend to follow
> > > > his argument now that it's actually pretty confusing.
> > > > 
> > > > In case of at91, we can do better now, once we have multiplatform
> > > > enabled, and we can simply remove the SOC_SAM_V7 and SOC_SAM_V4_V5
> > > > symbols entirely (if Alexandre's patch doesn't already do that).
> > > > 
> > > 
> > > I didn't remove it to keep the sub menus SOC_SAM_V7 -> SOC_SAMA5D4 and
> > > SOC_SAMA5D3, SOC_SAM_V4_V5 -> SOC_AT91SAM9 and SOC_AT91RM9200. But I can
> > > also expose SOC_AT91RM9200, SOC_AT91SAM9, SOC_SAMA5D3 and SOC_SAMA5D4
> > > instead of SOC_SAM_V7 and SOC_SAM_V4_V5 and remove them.
> > 
> > Exposing multiple levels won't help here since you can never enable
> > both v4/v5 and v7 at the same time in multiplatform.
> > 
> > Just use one level for a 'menuconfig ARCH_AT91' entry and put the
> > four options below that.
> > 
> 
> Actually, I had a closer look today and there is already just one level
> in my current version of patches.
> 
> I can put everything under menuconfig ARCH_AT91 but I can't remove
> SOC_SAM_V7 and SOC_SAM_V4_V5 because that is what is used to select
> which dtb should be compiled.
> 
> This would also break sama5_defconfig. This is a simple fix but that
> means that it makes it difficult to migrate an existing .config.
> 
> I'd say let's keep it that way as this is not broken, unless you really
> want to see SOC_SAM_V7 and SOC_SAM_V4_V5 being removed.
> 

I'm not sure what the latest version looks like, but I think it's fine
to have SOC_SAM_V4_V5 automatically selected when CONFIG_AT91 and
CONFIG_MULTI_V4 || CONFIG_MULTI_V5 are turned on, or selected by
AT91SAM9 and AT91RM9200.

	Arnd

Re: [PATCH] ARM: at91: force CPU selection

[Alexandre Belloni]

On 13/03/2015 at 20:50:59 +0100, Arnd Bergmann wrote :
> > 
> > Actually, I had a closer look today and there is already just one level
> > in my current version of patches.
> > 
> > I can put everything under menuconfig ARCH_AT91 but I can't remove
> > SOC_SAM_V7 and SOC_SAM_V4_V5 because that is what is used to select
> > which dtb should be compiled.
> > 
> > This would also break sama5_defconfig. This is a simple fix but that
> > means that it makes it difficult to migrate an existing .config.
> > 
> > I'd say let's keep it that way as this is not broken, unless you really
> > want to see SOC_SAM_V7 and SOC_SAM_V4_V5 being removed.
> > 
> 
> I'm not sure what the latest version looks like, but I think it's fine
> to have SOC_SAM_V4_V5 automatically selected when CONFIG_AT91 and
> CONFIG_MULTI_V4 || CONFIG_MULTI_V5 are turned on, or selected by
> AT91SAM9 and AT91RM9200.
> 

OK, I'm doing that. I'll send v3 in a few minutes.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com