Re: [PATCH] ARM: Gemini: Add support for PCI Bus

Previous thread: [PATCH 0/3] ASoC: add support for audio on iPaq hx4700 by Dmitry Artamonow on Saturday, November 20, 2010 - 5:59 am. (3 messages)

Next thread: general protection fault: 0000 [#1] SMP by Justin Mattock on Saturday, November 20, 2010 - 9:35 am. (7 messages)
From: Hans Ulli Kroll
Date: Saturday, November 20, 2010 - 7:27 am

Add support for PCI Bus on Gemini Devices SL3516

Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
---
 arch/arm/Kconfig                             |    1 +
 arch/arm/mach-gemini/Makefile                |    2 +
 arch/arm/mach-gemini/include/mach/hardware.h |    8 +
 arch/arm/mach-gemini/include/mach/irqs.h     |    7 +-
 arch/arm/mach-gemini/mm.c                    |    5 +
 arch/arm/mach-gemini/pci.c                   |  319 ++++++++++++++++++++++++++
 6 files changed, 340 insertions(+), 2 deletions(-)
 create mode 100644 arch/arm/mach-gemini/pci.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a19a526..5d4b398 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -307,6 +307,7 @@ config ARCH_GEMINI
 	select CPU_FA526
 	select ARCH_REQUIRE_GPIOLIB
 	select ARCH_USES_GETTIMEOFFSET
+	select PCI
 	help
 	  Support for the Cortina Systems Gemini family SoCs
 
diff --git a/arch/arm/mach-gemini/Makefile b/arch/arm/mach-gemini/Makefile
index c5b24b9..c263f48 100644
--- a/arch/arm/mach-gemini/Makefile
+++ b/arch/arm/mach-gemini/Makefile
@@ -6,6 +6,8 @@
 
 obj-y			:= irq.o mm.o time.o devices.o gpio.o
 
+obj-$(CONFIG_PCI)	+= pci.o
+
 # Board-specific support
 obj-$(CONFIG_MACH_NAS4220B)	+= board-nas4220b.o
 obj-$(CONFIG_MACH_RUT100)	+= board-rut1xx.o
diff --git a/arch/arm/mach-gemini/include/mach/hardware.h b/arch/arm/mach-gemini/include/mach/hardware.h
index 213a4fc..38c530f 100644
--- a/arch/arm/mach-gemini/include/mach/hardware.h
+++ b/arch/arm/mach-gemini/include/mach/hardware.h
@@ -71,4 +71,12 @@
  */
 #define IO_ADDRESS(x)	((((x) & 0xFFF00000) >> 4) | ((x) & 0x000FFFFF) | 0xF0000000)
 
+/*
+ * PCI subsystem macros
+ */
+#define PCIBIOS_MIN_IO		0x00000100
+#define PCIBIOS_MIN_MEM		0x00000000
+
+#define pcibios_assign_all_busses()	1
+
 #endif
diff --git a/arch/arm/mach-gemini/include/mach/irqs.h b/arch/arm/mach-gemini/include/mach/irqs.h
index 06bc47e..c737673 100644
--- a/arch/arm/mach-gemini/include/mach/irqs.h
+++ ...
From: Paulius Zaleckas
Date: Saturday, November 20, 2010 - 12:30 pm

From: Russell King - ARM Linux
Date: Friday, November 26, 2010 - 4:18 am

linux/gpio.h, not asm/gpio.h please.
--

From: Michał Mirosław
Date: Friday, November 26, 2010 - 4:57 am

[...]

I think it's better to add ARCH_GEMINI to CONFIG_PCI dependencies.
AFAIK IB-4220B NAS does not use PCI and doesn't need all that code.
Board support entries could select PCI if the board is useless without
it.

Best Regards,
Michał Mirosław
--

From: Hans Ulli Kroll
Date: Saturday, November 27, 2010 - 5:16 am

I know this, I have some.

There are many boards with PCI i.e WBD 222.
Other ARM devices uses the same select inside ARCH_*
I don't want to pollute drivers/pci/Kconfig.

Ulli
From: Michał Mirosław
Date: Saturday, November 27, 2010 - 6:01 am

W dniu 27 listopada 2010 13:16 użytkownik Hans Ulli Kroll

I meant 'config PCI' entry in arch/arm/Kconfig, see following diff.

Best Regards,
Michał Mirosław

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index db524e7..74ea522 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1164,7 +1164,7 @@ config ISA_DMA_API
 	bool

 config PCI
-	bool "PCI support" if ARCH_INTEGRATOR_AP || ARCH_VERSATILE_PB ||
ARCH_IXP4XX || ARCH_KS8695 || MACH_ARMCORE || ARCH_CNS3XXX
+	bool "PCI support" if ARCH_INTEGRATOR_AP || ARCH_VERSATILE_PB ||
ARCH_IXP4XX || ARCH_KS8695 || MACH_ARMCORE || ARCH_CNS3XXX ||
ARCH_GEMINI
 	help
 	  Find out whether you have a PCI motherboard. PCI is the name of a
 	  bus system, i.e. the way the CPU talks to the other stuff inside
--

From: Arnd Bergmann
Date: Saturday, November 27, 2010 - 8:39 am

This approach really does not scale as we add more boards to the list.

Better make a new CONFIG_HAVE_PCI option that you can select from the
individual boards, and make that the only dependency that CONFIG_PCI has.

	Arnd
--

From: Hans Ulli Kroll
Date: Monday, November 29, 2010 - 1:12 am

Good idea.
I write a patch for this.

Ulli
From: Russell King - ARM Linux
Date: Monday, November 29, 2010 - 7:22 am

Be careful.  There are two things going on here:

1. those which PCI support is configurable
2. those which always have PCI support

Making PCI "depend on HAVE_PCI" is wrong, and will throw up lots of
Kconfig warnings, as those platforms which always have PCI support
won't select HAVE_PCI - and making them do so such that "PCI support"
gets offered to them - with the only possible value being 'Y' is
silly.

So, rather than HAVE_PCI, it should be MIGHT_HAVE_PCI, and that
symbol needs to control whether the "PCI support" prompt is offered
to the user, not whether PCI is available or not.
--

From: Hans Ulli Kroll
Date: Monday, November 29, 2010 - 7:50 am

Here's my idea

bool "PIC support" if MIGHT_HAVE_PCI

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index a19a526..614cf2f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -34,6 +34,9 @@ config ARM
 config HAVE_PWM
 	bool
 
+config MIGHT_HAVE_PCI
+	bool
+
 config SYS_SUPPORTS_APM_EMULATION
 	bool
 
@@ -298,6 +301,7 @@ config ARCH_CNS3XXX
 	select CPU_V6
 	select GENERIC_CLOCKEVENTS
 	select ARM_GIC
+	select MIGHT_HAVE_PCI
 	select PCI_DOMAINS if PCI
 	help
 	  Support for Cavium Networks CNS3XXX platform.
@@ -433,6 +437,7 @@ config ARCH_IXP4XX
 	select CPU_XSCALE
 	select GENERIC_GPIO
 	select GENERIC_CLOCKEVENTS
+	select MIGHT_HAVE_PCI
 	select DMABOUNCE if PCI
 	help
 	  Support for Intel's IXP4XX (XScale) family of processors.
@@ -1164,7 +1169,7 @@ config ISA_DMA_API
 	bool
 
 config PCI
-	bool "PCI support" if ARCH_INTEGRATOR_AP || ARCH_VERSATILE_PB || 
ARCH_IXP4XX || ARCH_KS8695 || MACH_ARMCORE || ARCH_CNS3XXX
+	bool "PCI support" if MIGHT_HAVE_PCI
 	help
 	  Find out whether you have a PCI motherboard. PCI is the name of 
a
 	  bus system, i.e. the way the CPU talks to the other stuff inside
diff --git a/arch/arm/mach-cns3xxx/Kconfig b/arch/arm/mach-cns3xxx/Kconfig
index 9ebfcc4..29b13f2 100644
--- a/arch/arm/mach-cns3xxx/Kconfig
+++ b/arch/arm/mach-cns3xxx/Kconfig
@@ -3,6 +3,7 @@ menu "CNS3XXX platform type"
 
 config MACH_CNS3420VB
 	bool "Support for CNS3420 Validation Board"
+	select MIGHT_HAVE_PCI
 	help
 	  Include support for the Cavium Networks CNS3420 MPCore Platform
 	  Baseboard.
diff --git a/arch/arm/mach-integrator/Kconfig 
b/arch/arm/mach-integrator/Kconfig
index 27db275..769b0f1 100644
--- a/arch/arm/mach-integrator/Kconfig
+++ b/arch/arm/mach-integrator/Kconfig
@@ -4,6 +4,7 @@ menu "Integrator Options"
 
 config ARCH_INTEGRATOR_AP
 	bool "Support Integrator/AP and Integrator/PP2 platforms"
+	select MIGHT_HAVE_PCI
 	help
 	  Include support for the ARM(R) Integrator/AP and
 	  Integrator/PP2 ...
From: Arnd Bergmann
Date: Monday, November 29, 2010 - 8:57 am

This does not solve the problem that Russell mentioned: existing platforms
select PCI unconditionally, e.g. Iop13XX, some IXP, Orion, Shark
and more. At the very least, these would need to also select MIGHT_HAVE_PCI

Typo: you certainly meant MIGHT_HAVE_PCI here.

We still need to agree on what it should be doing, but otherwise this
is what I had in mind.

	Arnd
--

From: Hans Ulli Kroll
Date: Tuesday, November 30, 2010 - 8:38 am

so 
select MIGHT_HAVE_PCI 
in conjunction of
select PCI

Damn, I missed this.


--

From: Russell King - ARM Linux
Date: Tuesday, November 30, 2010 - 9:05 am

No, the above is correct.  What I was talking about was the difference
between these:

config PCI
	bool "PCI support" if MIGHT_HAVE_PCI

and

config PCI
	bool "PCI support"
	depends on MIGHT_HAVE_PCI

In the first instance, PCI itself does not depend on MIGHT_HAVE_PCI.
MIGHT_HAVE_PCI controls whether the user is offered the "PCI support"
option.

In the second instance, PCI depends on MIGHT_HAVE_PCI, which must be
set to 'y' to offer the option _and_ also if PCI is selected.

We want the first behaviour.  Platforms which must have PCI support can
continue to select PCI as they currently do, and leave the MIGHT_HAVE_PCI
option alone.

Platforms which may optionally have PCI support should select
MIGHT_HAVE_PCI instead.
--

From: Arnd Bergmann
Date: Tuesday, November 30, 2010 - 9:19 am

Right, I misread the patch as doing the wrong thing, thanks for

Yes, that sounds good.

	Arnd
--

From: Hans Ulli Kroll
Date: Wednesday, December 1, 2010 - 8:05 am

OK.
I prepare some patch.

Ulli
--

From: Arnd Bergmann
Date: Monday, November 29, 2010 - 8:50 am

We definitely need to be careful with combinations of select and
depends. The obvious danger is enabling two boards, one of which
requires PCI while the other one cannot deal with CONFIG_PCI=y.
As we get to more multiplatform builds, we have to eventually
solve this, along with the problem that certain PCI implementations
are currently mutually exclusive.

What behaviour do you want to see in a multiplatform build where
one board is known to have PCI, one may have it and a third
one cannot have PCI? Should it be enabled, disabled or

Most architectures make PCI optional even though it is a bit silly
to disable it like on x86. There are a lot of other useful configuration
options that end up always enabled in practice.

The easiest way would be to always select HAVE_PCI (or MIGHT_HAVE_PCI
if you prefer the term) and make the default CONFIG_PCI default to
yes.

Or you could go fancy and have both HAVE_PCI and MIGHT_HAVE_PCI, with
each platform selecting at most one of the two and this

config PCI
	bool "PCI support"
	depends on MIGHT_HAVE_PCI && !HAVE_PCI
	default HAVE_PCI

This way it will be silently turned on if one of the machines requires
PCI, but stay visible if the machines might want to disable PCI.

	Arnd
--

Previous thread: [PATCH 0/3] ASoC: add support for audio on iPaq hx4700 by Dmitry Artamonow on Saturday, November 20, 2010 - 5:59 am. (3 messages)

Next thread: general protection fault: 0000 [#1] SMP by Justin Mattock on Saturday, November 20, 2010 - 9:35 am. (7 messages)