diff mbox

[1/2] ARM: add ARM_SINGLE_ARMV7 as config option

Message ID 20170209033044.19513-2-chris.brandt@renesas.com (mailing list archive)
State Changes Requested
Delegated to: Simon Horman
Headers show

Commit Message

Chris Brandt Feb. 9, 2017, 3:30 a.m. UTC
Creates a new ARM_SINGLE_ARMV7 option as an alternative ARCH_MULTIPLATFORM.

ARM_SINGLE_ARMV7 is very similar to ARCH_MULTIPLATFORM, except the options
from ARCH_MULTI_V6_V7 were copied directly into the new ARM_SINGLE_ARMV7.

Additionally, everywhere ARCH_MULTIPLATFORM existed in build scripts,
ARM_SINGLE_ARMV7 was added along side it in order to produce similar
results.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 arch/arm/Kconfig          | 18 ++++++++++++++++++
 arch/arm/Kconfig.debug    |  6 ++++--
 arch/arm/Makefile         |  5 +++++
 arch/arm/kernel/devtree.c |  3 ++-
 4 files changed, 29 insertions(+), 3 deletions(-)

Comments

Chris Brandt Feb. 9, 2017, 3:50 a.m. UTC | #1
Hello Florian,

On Wednesday, February 08, 2017, Florian Fainelli wrote:
> On 02/08/2017 07:30 PM, Chris Brandt wrote:

> > Creates a new ARM_SINGLE_ARMV7 option as an alternative

> ARCH_MULTIPLATFORM.

> >

> > ARM_SINGLE_ARMV7 is very similar to ARCH_MULTIPLATFORM, except the

> > options from ARCH_MULTI_V6_V7 were copied directly into the new

> ARM_SINGLE_ARMV7.

> >

> > Additionally, everywhere ARCH_MULTIPLATFORM existed in build scripts,

> > ARM_SINGLE_ARMV7 was added along side it in order to produce similar

> > results.

> 

> Could we try to be a little smarter than this and if ARCH_MULTIPLATFORM is

> selected but only one platform beneath is we automatically select

> ARM_SINGLE_ARMV7 (this may require Kconfig hackery)?


Months ago I spent hours on trying to get the current kbuild system to do
that and nothing I came up with would work. It's simply not designed that
way.

I think the closest I might have come was to purposely break the build
if more then 1 was select, but that still didn't stop you from making
the selection.

If someone smarter than me has a way to do (not just an idea...I received
lots of ideas but none of them worked), I'd be happy to hear it.



> I see value in what you are doing, but I don't think pulling the platforms

> out of the menu like what you do in the subsequent patch is looking nice

> as an user/configurator.


What I was trying to do (and what was already removed from Kconfigs) was
to avoid having a duplicate set of configs for each SOC: one for single and
one for multi.


#I'm trying to get away from manually hacking these Kconfig files every time
a new kernel is released.


Chris
Florian Fainelli Feb. 9, 2017, 7:53 p.m. UTC | #2
On 02/08/2017 07:50 PM, Chris Brandt wrote:
> Hello Florian,
> 
> On Wednesday, February 08, 2017, Florian Fainelli wrote:
>> On 02/08/2017 07:30 PM, Chris Brandt wrote:
>>> Creates a new ARM_SINGLE_ARMV7 option as an alternative
>> ARCH_MULTIPLATFORM.
>>>
>>> ARM_SINGLE_ARMV7 is very similar to ARCH_MULTIPLATFORM, except the
>>> options from ARCH_MULTI_V6_V7 were copied directly into the new
>> ARM_SINGLE_ARMV7.
>>>
>>> Additionally, everywhere ARCH_MULTIPLATFORM existed in build scripts,
>>> ARM_SINGLE_ARMV7 was added along side it in order to produce similar
>>> results.
>>
>> Could we try to be a little smarter than this and if ARCH_MULTIPLATFORM is
>> selected but only one platform beneath is we automatically select
>> ARM_SINGLE_ARMV7 (this may require Kconfig hackery)?
> 
> Months ago I spent hours on trying to get the current kbuild system to do
> that and nothing I came up with would work. It's simply not designed that
> way.
> 
> I think the closest I might have come was to purposely break the build
> if more then 1 was select, but that still didn't stop you from making
> the selection.
> 
> If someone smarter than me has a way to do (not just an idea...I received
> lots of ideas but none of them worked), I'd be happy to hear it.

I am definitively not a Kbuild expert, but it would almost necessarily
require introduce some kind of new type in the Kconfig/Kbuild syntax
that does something like that:

- have a way to count the number of symbols that are selected and do a
"if ARCH_MULTI_V6_V7" (or an arbitrary expression) this most likely
should exist internally within Kconfig

- introduce a new type of Kconfig type which is a "count", and gets
assigned this value that we just counted, something that could look like
this:

count ARCH_MULTI_V6_V7_COUNT
	tracks ARCH_MULTI_V6_V7

All other typical Kconfig syntax thould then still apply and we could be
doing something like:

depends on ARCH_MULTI_V6_V7_COUNT=1 etc..

So yeah, I am no better than whoever gave you indications better ;)

> 
> 
> 
>> I see value in what you are doing, but I don't think pulling the platforms
>> out of the menu like what you do in the subsequent patch is looking nice
>> as an user/configurator.
> 
> What I was trying to do (and what was already removed from Kconfigs) was
> to avoid having a duplicate set of configs for each SOC: one for single and
> one for multi.
> 
> 
> #I'm trying to get away from manually hacking these Kconfig files every time
> a new kernel is released.

Sure, that makes complete sense to me. FWIW, there is one particular
case that I am interested that you solved which is allowing
DEBUG_UNCOMPRESS to be set in our downstream kernel because we typically
only enable BRCMSTB and nothing else.
Chris Brandt Feb. 9, 2017, 8:21 p.m. UTC | #3
On Thursday, February 09, 2017, Florian Fainelli worte:
> > I think the closest I might have come was to purposely break the build

> > if more then 1 was select, but that still didn't stop you from making

> > the selection.

> >

> > If someone smarter than me has a way to do (not just an idea...I

> received

> > lots of ideas but none of them worked), I'd be happy to hear it.

> 

> I am definitively not a Kbuild expert, but it would almost necessarily

> require introduce some kind of new type in the Kconfig/Kbuild syntax

> that does something like that:

> 

> - have a way to count the number of symbols that are selected and do a

> "if ARCH_MULTI_V6_V7" (or an arbitrary expression) this most likely

> should exist internally within Kconfig

> 

> - introduce a new type of Kconfig type which is a "count", and gets

> assigned this value that we just counted, something that could look like

> this:

> 

> count ARCH_MULTI_V6_V7_COUNT

> 	tracks ARCH_MULTI_V6_V7


I did try the counting thing, but couldn't get it to work.
I admit though, I did stop when the next step was to create a new type
kind of thing that I could use for counting. That seemed to start going
down a deeper path than I was hoping for.

However,
I am hesitant to go and try anything else because everything I've submitted
so far has been NACKed. The only thing Russell said he'd agree to is a top
level single-platform option. But, since that all got taken out, I assume
there's some resistance to putting it back in.


Chris
Arnd Bergmann Feb. 10, 2017, 1:05 p.m. UTC | #4
On Thursday, February 9, 2017 8:21:43 PM CET Chris Brandt wrote:
> On Thursday, February 09, 2017, Florian Fainelli worte:
> > > I think the closest I might have come was to purposely break the build
> > > if more then 1 was select, but that still didn't stop you from making
> > > the selection.
> > >
> > > If someone smarter than me has a way to do (not just an idea...I
> > received
> > > lots of ideas but none of them worked), I'd be happy to hear it.
> > 
> > I am definitively not a Kbuild expert, but it would almost necessarily
> > require introduce some kind of new type in the Kconfig/Kbuild syntax
> > that does something like that:
> > 
> > - have a way to count the number of symbols that are selected and do a
> > "if ARCH_MULTI_V6_V7" (or an arbitrary expression) this most likely
> > should exist internally within Kconfig
> > 
> > - introduce a new type of Kconfig type which is a "count", and gets
> > assigned this value that we just counted, something that could look like
> > this:
> > 
> > count ARCH_MULTI_V6_V7_COUNT
> >       tracks ARCH_MULTI_V6_V7
> 
> I did try the counting thing, but couldn't get it to work.
> I admit though, I did stop when the next step was to create a new type
> kind of thing that I could use for counting. That seemed to start going
> down a deeper path than I was hoping for.

I also couldn't come up with something working when I looked at this,
and it wouldn't solve the related problem of platforms that we want to
be able to use with or without MMU: You can't make the decision of
whether allow an MMU based on the number of platforms since most
platform options can only be offered depending on the setting of
CONFIG_MMU.

> However,
> I am hesitant to go and try anything else because everything I've submitted
> so far has been NACKed. The only thing Russell said he'd agree to is a top
> level single-platform option. But, since that all got taken out, I assume
> there's some resistance to putting it back in.

And I really don't like adding new top-level for a platform here, it
brings us back to the same problems we had before we moved most platforms
to ARCH_MULTIPLATFORM, and it doesn't solve the remaining problems we still
have:

- In some platforms, the decision would have to be done on a per-board
  level, as each board can have its memory at a different location
  base on which chipselect line got connected to the RAM and NOR flash
  respectively

- Some (few) platforms actually have separate top-level Kconfig options
  but are actually very closely related and you could have a kernel
  for all of them even with !MMU and XIP_KERNEL. The most important
  one here is ARM Versatile/Realview/Integrator/Vexpress that have
  more in common than things we put behind a common Kconfig option in
  other platforms.

- CONFIG_DEBUG_UNCOMPRESS has a very similar requirements to
  XIP_KERNEL and !MMU, and we currently allow it for any machine,
  with a lot of flexibility in configuring that always breaks
  running on any machine other than the one you are targetting.

	Arnd
Chris Brandt Feb. 10, 2017, 2:17 p.m. UTC | #5
On Friday, February 10, 2017, Arnd Bergmann wrote:
> I also couldn't come up with something working when I looked at this, and
> it wouldn't solve the related problem of platforms that we want to be able
> to use with or without MMU: You can't make the decision of whether allow
> an MMU based on the number of platforms since most platform options can
> only be offered depending on the setting of CONFIG_MMU.


How about this idea (from a high level only):

1. At a top level, you would have 2 options:
   * "Allow multiple platforms to be selected"
   * "Single platform Only"

2. If SINGLE is selected, you still get all the choices that you
   get with MULTI, except as soon as you click a single ARCH_xx, all
   other ARCH_yy options are automatically 'unselected' and disappear
   from the menus. To change your mind, you simply uncheck that box
   and magically everything re-appears.

3. The current single platform ARCHs that are currently still sitting
   at the top level get moved to go sit with everyone else, except
   they will have "depends on SINGLE" so they will not show up
   when MULTI is selected.

4. XIP_KERNEL can only be selected if you have SINGLE selected.

The new restriction is that if you want XIP_KERNEL, you are
only going to get 1 (and only 1) ARCH_xx.

Of course the only way this is going to happen is by modifying kbuild
system to create a new 'mode' or something (but let's not get concerned
with the details until a high level agreement can be made).


> - In some platforms, the decision would have to be done on a per-board
>   level, as each board can have its memory at a different location
>   base on which chipselect line got connected to the RAM and NOR flash
>   respectively

This idea only effects ARCH_xx. You can still select multiple MACH_xx if
you want after you've selected your ARCH_xx.


> - Some (few) platforms actually have separate top-level Kconfig options
>   but are actually very closely related and you could have a kernel
>   for all of them even with !MMU and XIP_KERNEL. The most important
>   one here is ARM Versatile/Realview/Integrator/Vexpress that have
>   more in common than things we put behind a common Kconfig option in
>   other platforms.

So, this one gets squashed because for XIP_KERNEL, you have to pick
just 1 ARCH_xx.


> - CONFIG_DEBUG_UNCOMPRESS has a very similar requirements to
>   XIP_KERNEL and !MMU, and we currently allow it for any machine,
>   with a lot of flexibility in configuring that always breaks
>   running on any machine other than the one you are targetting.

Since the purpose of this is "debugging" a single platform, could you
put this under the "depends on SINGLE" category?


What I'm trying to understand is if:
A. There is something that can be agreed upon (whether it's technically
   feasible still needs to be worked out)

B. It doesn't work today, and there's no way to make everyone happy,
   so go hack your own Kconfig files and just live with it.


Chris
Arnd Bergmann Feb. 10, 2017, 3:53 p.m. UTC | #6
On Fri, Feb 10, 2017 at 3:17 PM, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Friday, February 10, 2017, Arnd Bergmann wrote:
>> I also couldn't come up with something working when I looked at this, and
>> it wouldn't solve the related problem of platforms that we want to be able
>> to use with or without MMU: You can't make the decision of whether allow
>> an MMU based on the number of platforms since most platform options can
>> only be offered depending on the setting of CONFIG_MMU.
>
>
> How about this idea (from a high level only):
>
> 1. At a top level, you would have 2 options:
>    * "Allow multiple platforms to be selected"
>    * "Single platform Only"
>
> 2. If SINGLE is selected, you still get all the choices that you
>    get with MULTI, except as soon as you click a single ARCH_xx, all
>    other ARCH_yy options are automatically 'unselected' and disappear
>    from the menus. To change your mind, you simply uncheck that box
>    and magically everything re-appears.
>
> 3. The current single platform ARCHs that are currently still sitting
>    at the top level get moved to go sit with everyone else, except
>    they will have "depends on SINGLE" so they will not show up
>    when MULTI is selected.
>
> 4. XIP_KERNEL can only be selected if you have SINGLE selected.
>
> The new restriction is that if you want XIP_KERNEL, you are
> only going to get 1 (and only 1) ARCH_xx.
>
> Of course the only way this is going to happen is by modifying kbuild
> system to create a new 'mode' or something (but let's not get concerned
> with the details until a high level agreement can be made).

You can actually have a 'tristate' choice statement with semantics
close to that, but actually using that here would get rather messy,
as we'd have to move all the per-directory 'menuconfig' statements
back into the top-level choice, and also add logic to detect whether
we use =y or =m.

>> - In some platforms, the decision would have to be done on a per-board
>>   level, as each board can have its memory at a different location
>>   base on which chipselect line got connected to the RAM and NOR flash
>>   respectively
>
> This idea only effects ARCH_xx. You can still select multiple MACH_xx if
> you want after you've selected your ARCH_xx.

Yes, that's my point: we'd have to move some or all of the MACH_xx
options into the top-level list as well, or repeat the same logic within
each platform that may have incompatible boards.

>> - CONFIG_DEBUG_UNCOMPRESS has a very similar requirements to
>>   XIP_KERNEL and !MMU, and we currently allow it for any machine,
>>   with a lot of flexibility in configuring that always breaks
>>   running on any machine other than the one you are targetting.
>
> Since the purpose of this is "debugging" a single platform, could you
> put this under the "depends on SINGLE" category?

It would solve the problem of unusable configurations, but break one
very important use case of DEBUG_LL: you have a problem with a
running kernel and just want to turn on DEBUG_LL without changing
anything else.

       Arnd
Russell King (Oracle) Feb. 10, 2017, 4:39 p.m. UTC | #7
On Fri, Feb 10, 2017 at 02:05:43PM +0100, Arnd Bergmann wrote:
> And I really don't like adding new top-level for a platform here, it
> brings us back to the same problems we had before we moved most platforms
> to ARCH_MULTIPLATFORM, and it doesn't solve the remaining problems we still
> have:

It's the least evil solution.

> - In some platforms, the decision would have to be done on a per-board
>   level, as each board can have its memory at a different location
>   base on which chipselect line got connected to the RAM and NOR flash
>   respectively

Right, but that's something we _had_ solved before multiplatform came
along.

> - Some (few) platforms actually have separate top-level Kconfig options
>   but are actually very closely related and you could have a kernel
>   for all of them even with !MMU and XIP_KERNEL. The most important
>   one here is ARM Versatile/Realview/Integrator/Vexpress that have
>   more in common than things we put behind a common Kconfig option in
>   other platforms.

If you think that Versatile + Realview + Integrator + Vexpress should
be lumped into one kernel image for !MMU and XIP_KERNEL then you're
wrong - from what I remember, their RAM and flash locations are quite
different which rules it out.

The biggest thing that matters for !MMU and XIP_KERNEL is the location
of flash and RAM.  If those are not compatible, !MMU and XIP_KERNEL
has no chance of working.

> - CONFIG_DEBUG_UNCOMPRESS has a very similar requirements to
>   XIP_KERNEL and !MMU, and we currently allow it for any machine,
>   with a lot of flexibility in configuring that always breaks
>   running on any machine other than the one you are targetting.

Right, but that's debug, not core kernel configuration.
Arnd Bergmann Feb. 13, 2017, 8:45 p.m. UTC | #8
On Fri, Feb 10, 2017 at 5:39 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:

>
>> - Some (few) platforms actually have separate top-level Kconfig options
>>   but are actually very closely related and you could have a kernel
>>   for all of them even with !MMU and XIP_KERNEL. The most important
>>   one here is ARM Versatile/Realview/Integrator/Vexpress that have
>>   more in common than things we put behind a common Kconfig option in
>>   other platforms.
>
> If you think that Versatile + Realview + Integrator + Vexpress should
> be lumped into one kernel image for !MMU and XIP_KERNEL then you're
> wrong - from what I remember, their RAM and flash locations are quite
> different which rules it out.

They are not all the same, but the locations are not split according to
the platform names either. The early realview platforms are like versatile
here, while later realview platforms have the alias plus more, and vexpress
has it in two other places depending on the generation.

I misremembered vexpress and thought that it was like the later
realview, but at least versatile and realview are basically the same
thing (the story with NOR flash is similar).

>> - CONFIG_DEBUG_UNCOMPRESS has a very similar requirements to
>>   XIP_KERNEL and !MMU, and we currently allow it for any machine,
>>   with a lot of flexibility in configuring that always breaks
>>   running on any machine other than the one you are targetting.
>
> Right, but that's debug, not core kernel configuration.

I have not yet seen any argument why that is an important distinction, i.e.
what is the downside of letting users just turn the MMU and XIP options
on and off without changing everything else, other than this having been
the default in the past?

Obviously you won't be able to boot on machines when configured for
a different physical address (for RAM, flash or uart), but even that is
no change from what we have on the NOMMU platforms: on any of them
you have to manually type the address when configuring your kernel
for a given target and change it again when you want to run on another
board that doesn't share the exact address.

     Arnd
diff mbox

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index bf8d86d..36107e7 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -317,6 +317,24 @@  config ARCH_MMAP_RND_BITS_MAX
 	default 15 if PAGE_OFFSET=0x80000000
 	default 16
 
+config ARM_SINGLE_ARMV7
+	bool "ARMv7 based single platforms"
+	depends on MMU
+	select ARM_HAS_SG_CHAIN
+	select ARM_PATCH_PHYS_VIRT if !XIP_KERNEL
+	select AUTO_ZRELADDR
+	select CLKSRC_OF
+	select COMMON_CLK
+	select CPU_V7
+	select GENERIC_CLOCKEVENTS
+	select MIGHT_HAVE_PCI
+	select MULTI_IRQ_HANDLER
+	select PCI_DOMAINS if PCI
+	select SPARSE_IRQ
+	select USE_OF
+	select HAVE_SMP
+	select MIGHT_HAVE_CACHE_L2X0
+
 #
 # The "ARM system type" choice list is ordered alphabetically by option
 # text.  Please add new entries in the option alphabetic order.
diff --git a/arch/arm/Kconfig.debug b/arch/arm/Kconfig.debug
index d83f7c3..31db9e3 100644
--- a/arch/arm/Kconfig.debug
+++ b/arch/arm/Kconfig.debug
@@ -1694,7 +1694,8 @@  config DEBUG_UART_8250_FLOW_CONTROL
 
 config DEBUG_UNCOMPRESS
 	bool
-	depends on ARCH_MULTIPLATFORM || PLAT_SAMSUNG || ARM_SINGLE_ARMV7M
+	depends on ARCH_MULTIPLATFORM || PLAT_SAMSUNG || ARM_SINGLE_ARMV7M || \
+		   ARM_SINGLE_ARMV7
 	default y if DEBUG_LL && !DEBUG_OMAP2PLUS_UART && \
 		     (!DEBUG_TEGRA_UART || !ZBOOT_ROM) && \
 		     !DEBUG_BRCMSTB_UART
@@ -1712,7 +1713,8 @@  config DEBUG_UNCOMPRESS
 config UNCOMPRESS_INCLUDE
 	string
 	default "debug/uncompress.h" if ARCH_MULTIPLATFORM || ARCH_MSM || \
-					PLAT_SAMSUNG || ARM_SINGLE_ARMV7M
+					PLAT_SAMSUNG || ARM_SINGLE_ARMV7M || \
+					ARM_SINGLE_ARMV7
 	default "mach/uncompress.h"
 
 config EARLY_PRINTK
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index ab30cc6..c9809b6 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -255,12 +255,16 @@  endif
 ifeq ($(CONFIG_ARCH_MULTIPLATFORM),y)
 MACHINE  :=
 endif
+ifeq ($(CONFIG_ARM_SINGLE_ARMV7),y)
+MACHINE  :=
+endif
 
 machdirs := $(patsubst %,arch/arm/mach-%/,$(machine-y))
 platdirs := $(patsubst %,arch/arm/plat-%/,$(sort $(plat-y)))
 
 ifneq ($(CONFIG_ARCH_MULTIPLATFORM),y)
 ifneq ($(CONFIG_ARM_SINGLE_ARMV7M),y)
+ifneq ($(CONFIG_ARM_SINGLE_ARMV7),y)
 ifeq ($(KBUILD_SRC),)
 KBUILD_CPPFLAGS += $(patsubst %,-I%include,$(machdirs) $(platdirs))
 else
@@ -268,6 +272,7 @@  KBUILD_CPPFLAGS += $(patsubst %,-I$(srctree)/%include,$(machdirs) $(platdirs))
 endif
 endif
 endif
+endif
 
 export	TEXT_OFFSET GZFLAGS MMUEXT
 
diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index f676feb..26cbc29 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -220,7 +220,8 @@  const struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
 {
 	const struct machine_desc *mdesc, *mdesc_best = NULL;
 
-#if defined(CONFIG_ARCH_MULTIPLATFORM) || defined(CONFIG_ARM_SINGLE_ARMV7M)
+#if defined(CONFIG_ARCH_MULTIPLATFORM) || defined(CONFIG_ARM_SINGLE_ARMV7M) || \
+    defined(CONFIG_ARM_SINGLE_ARMV7)
 	DT_MACHINE_START(GENERIC_DT, "Generic DT based system")
 		.l2c_aux_val = 0x0,
 		.l2c_aux_mask = ~0x0,