All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/2] adding PCI support to AXS10x
@ 2016-01-12 15:45 ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-12 15:45 UTC (permalink / raw)
  To: Vineet.Gupta1, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, Joao Pinto

This patch set has the goal to add suppport for DesignWare PCIe RC in ARC
AXS10x. It includes the necessary tweaks to the ARC architecture, 
necessary tweaks to the PCI subsystem and a new driver (pcie-snpsdev).
This new driver will be used extensively in the PCIe RC Prototyping Kit.

The patches were produced against Bjorn Helgaas' repository. It was properly
tested in an IP Prototyping Kit.

Joao Pinto (2):
  PCI support added to ARC
  add new platform driver for PCI RC

 .../devicetree/bindings/pci/pcie-snpsdev.txt       |  33 +++
 MAINTAINERS                                        |   7 +
 arch/arc/Kconfig                                   |  23 ++
 arch/arc/include/asm/dma.h                         |   5 +
 arch/arc/include/asm/io.h                          |   9 +
 arch/arc/include/asm/pci.h                         |  31 +++
 arch/arc/kernel/Makefile                           |   1 +
 arch/arc/kernel/pcibios.c                          |  24 ++
 arch/arc/plat-axs10x/Kconfig                       |   1 +
 drivers/pci/Makefile                               |   1 +
 drivers/pci/host/Kconfig                           |   8 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-designware.c                 |   9 +
 drivers/pci/host/pcie-designware.h                 |   1 +
 drivers/pci/host/pcie-snpsdev.c                    | 286 +++++++++++++++++++++
 15 files changed, 440 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
 create mode 100644 arch/arc/include/asm/pci.h
 create mode 100644 arch/arc/kernel/pcibios.c
 create mode 100644 drivers/pci/host/pcie-snpsdev.c

-- 
1.8.1.5

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v5 0/2] adding PCI support to AXS10x
@ 2016-01-12 15:45 ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-12 15:45 UTC (permalink / raw)
  To: linux-snps-arc

This patch set has the goal to add suppport for DesignWare PCIe RC in ARC
AXS10x. It includes the necessary tweaks to the ARC architecture, 
necessary tweaks to the PCI subsystem and a new driver (pcie-snpsdev).
This new driver will be used extensively in the PCIe RC Prototyping Kit.

The patches were produced against Bjorn Helgaas' repository. It was properly
tested in an IP Prototyping Kit.

Joao Pinto (2):
  PCI support added to ARC
  add new platform driver for PCI RC

 .../devicetree/bindings/pci/pcie-snpsdev.txt       |  33 +++
 MAINTAINERS                                        |   7 +
 arch/arc/Kconfig                                   |  23 ++
 arch/arc/include/asm/dma.h                         |   5 +
 arch/arc/include/asm/io.h                          |   9 +
 arch/arc/include/asm/pci.h                         |  31 +++
 arch/arc/kernel/Makefile                           |   1 +
 arch/arc/kernel/pcibios.c                          |  24 ++
 arch/arc/plat-axs10x/Kconfig                       |   1 +
 drivers/pci/Makefile                               |   1 +
 drivers/pci/host/Kconfig                           |   8 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-designware.c                 |   9 +
 drivers/pci/host/pcie-designware.h                 |   1 +
 drivers/pci/host/pcie-snpsdev.c                    | 286 +++++++++++++++++++++
 15 files changed, 440 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
 create mode 100644 arch/arc/include/asm/pci.h
 create mode 100644 arch/arc/kernel/pcibios.c
 create mode 100644 drivers/pci/host/pcie-snpsdev.c

-- 
1.8.1.5

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v5 1/2] PCI support added to ARC
  2016-01-12 15:45 ` [PATCH v5 " Joao Pinto
  (?)
@ 2016-01-12 15:45   ` Joao Pinto
  -1 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-12 15:45 UTC (permalink / raw)
  To: Vineet.Gupta1, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, Joao Pinto

This patch adds PCI support to ARC and updates drivers/pci Makefile
enabling the ARC arch to use the generic PCI setup functions.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Change v4 -> v5 (Vineet Gupta):
- pcibios.c unused functions, variables and includes were removed
- ioremap.c has no need for changes (no patch is necessary)
- pci.h unused functions and variables were removed
Change v3 -> v4:
- Nothing changed (just to keep up with patch set version).
Change v2 -> v3 (Bjorn Helgaas):
- arch/arc/kernel/pcibios.c unused functions were removed and also the
arch/arc/include/asm/mach/pci.h was removed because was no longer necessary
Change v1 -> v2:
- In arch/arc/Kconfig, the new menu entry (Bus Configuration) was moved to the
slot between sourcing of drivers/Kconfig and fs/Kconfig (Vineet Gupta)
- In arch/arc/plat-axs10x/Kconfig the "select MIGHT_HAVE_PCI" option was placed
in order as suggested (Vineet Gupta)
- ioport_map() and ioport_unmap() were static inlined and included in
the io.h (Vineet Gupta)
- pcibios_min_io and pcibios_min_mem declaration moved to
pcibios.c (Vineet Gupta)
- pr_err() replaced by dev_err() in pcibios_enable_device() (Bjorn Helgaas)
- string simplified in pcibios_enable_device() (Vineet Gupta)
- pci_host_bridge_window structure was replaced by resource_entry structure, and
list_for_each_entry() for resource_list_for_each_entry() in pcibios.c

 arch/arc/Kconfig             | 23 +++++++++++++++++++++++
 arch/arc/include/asm/dma.h   |  5 +++++
 arch/arc/include/asm/io.h    |  9 +++++++++
 arch/arc/include/asm/pci.h   | 31 +++++++++++++++++++++++++++++++
 arch/arc/kernel/Makefile     |  1 +
 arch/arc/kernel/pcibios.c    | 24 ++++++++++++++++++++++++
 arch/arc/plat-axs10x/Kconfig |  1 +
 drivers/pci/Makefile         |  1 +
 8 files changed, 95 insertions(+)
 create mode 100644 arch/arc/include/asm/pci.h
 create mode 100644 arch/arc/kernel/pcibios.c

diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 2c2ac3f..98b32c1 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -19,6 +19,7 @@ config ARC
 	select GENERIC_FIND_FIRST_BIT
 	# for now, we don't need GENERIC_IRQ_PROBE, CONFIG_GENERIC_IRQ_CHIP
 	select GENERIC_IRQ_SHOW
+	select GENERIC_PCI_IOMAP
 	select GENERIC_PENDING_IRQ if SMP
 	select GENERIC_SMP_IDLE_THREAD
 	select HAVE_ARCH_KGDB
@@ -39,6 +40,9 @@ config ARC
 	select PERF_USE_VMALLOC
 	select HAVE_DEBUG_STACKOVERFLOW
 
+config MIGHT_HAVE_PCI
+	bool
+
 config TRACE_IRQFLAGS_SUPPORT
 	def_bool y
 
@@ -570,6 +574,25 @@ endmenu	 # "ARC Architecture Configuration"
 source "mm/Kconfig"
 source "net/Kconfig"
 source "drivers/Kconfig"
+
+menu "Bus Support"
+
+config PCI
+	bool "PCI support" if MIGHT_HAVE_PCI
+	help
+	  PCI is the name of a bus system, i.e. the way the CPU talks to the other stuff inside
+	  your box.Find out if your board/platform have PCI.
+	  Note: PCIE support for Synopsys Device will be available only when
+	  HAPS DX is configured with PCIE RC bitmap. If you have PCI, say Y, otherwise N.
+
+config PCI_SYSCALL
+	def_bool PCI
+
+source "drivers/pci/Kconfig"
+source "drivers/pci/pcie/Kconfig"
+
+endmenu
+
 source "fs/Kconfig"
 source "arch/arc/Kconfig.debug"
 source "security/Kconfig"
diff --git a/arch/arc/include/asm/dma.h b/arch/arc/include/asm/dma.h
index ca7c451..37942fa 100644
--- a/arch/arc/include/asm/dma.h
+++ b/arch/arc/include/asm/dma.h
@@ -10,5 +10,10 @@
 #define ASM_ARC_DMA_H
 
 #define MAX_DMA_ADDRESS 0xC0000000
+#ifdef CONFIG_PCI
+extern int isa_dma_bridge_buggy;
+#else
+#define isa_dma_bridge_buggy    (0)
+#endif
 
 #endif
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index 694ece8..947bf0c 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -16,6 +16,15 @@
 extern void __iomem *ioremap(unsigned long physaddr, unsigned long size);
 extern void __iomem *ioremap_prot(phys_addr_t offset, unsigned long size,
 				  unsigned long flags);
+static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
+{
+	return (void __iomem *)port;
+}
+
+static inline void ioport_unmap(void __iomem *addr)
+{
+}
+
 extern void iounmap(const void __iomem *addr);
 
 #define ioremap_nocache(phy, sz)	ioremap(phy, sz)
diff --git a/arch/arc/include/asm/pci.h b/arch/arc/include/asm/pci.h
new file mode 100644
index 0000000..2f091e1
--- /dev/null
+++ b/arch/arc/include/asm/pci.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _ASM_ARC_PCI_H
+#define _ASM_ARC_PCI_H
+
+#ifdef __KERNEL__
+#include <asm-generic/pci-dma-compat.h>
+#include <asm-generic/pci-bridge.h>
+
+#include <linux/ioport.h>
+
+#define PCIBIOS_MIN_IO 0x100
+#define PCIBIOS_MIN_MEM 0x100000
+
+#define pcibios_assign_all_busses()	1
+/*
+ * The PCI address space does equal the physical memory address space.
+ * The networking and block device layers use this boolean for bounce
+ * buffer decisions.
+ */
+#define PCI_DMA_BUS_IS_PHYS     (1)
+
+#endif /* __KERNEL__ */
+
+#endif /* _ASM_ARC_PCI_H */
diff --git a/arch/arc/kernel/Makefile b/arch/arc/kernel/Makefile
index e7f3625..1bc2036 100644
--- a/arch/arc/kernel/Makefile
+++ b/arch/arc/kernel/Makefile
@@ -12,6 +12,7 @@ obj-y	:= arcksyms.o setup.o irq.o time.o reset.o ptrace.o process.o devtree.o
 obj-y	+= signal.o traps.o sys.o troubleshoot.o stacktrace.o disasm.o clk.o
 obj-$(CONFIG_ISA_ARCOMPACT)		+= entry-compact.o intc-compact.o
 obj-$(CONFIG_ISA_ARCV2)			+= entry-arcv2.o intc-arcv2.o
+obj-$(CONFIG_PCI)  			+= pcibios.o
 
 obj-$(CONFIG_MODULES)			+= arcksyms.o module.o
 obj-$(CONFIG_SMP) 			+= smp.o
diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
new file mode 100644
index 0000000..12ea45a
--- /dev/null
+++ b/arch/arc/kernel/pcibios.c
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2014-2015 Synopsys, Inc. (www.synopsys.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/pci.h>
+
+/*
+ * We don't have to worry about legacy ISA devices, so nothing to do here
+ */
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
+				resource_size_t size, resource_size_t align)
+{
+	return res->start;
+}
+
+void pcibios_fixup_bus(struct pci_bus *bus)
+{
+}
+EXPORT_SYMBOL(pcibios_fixup_bus);
diff --git a/arch/arc/plat-axs10x/Kconfig b/arch/arc/plat-axs10x/Kconfig
index d475f9d..426ac4b 100644
--- a/arch/arc/plat-axs10x/Kconfig
+++ b/arch/arc/plat-axs10x/Kconfig
@@ -11,6 +11,7 @@ menuconfig ARC_PLAT_AXS10X
 	select DW_APB_ICTL
 	select GPIO_DWAPB
 	select OF_GPIO
+	select MIGHT_HAVE_PCI
 	select GENERIC_IRQ_CHIP
 	select ARCH_REQUIRE_GPIOLIB
 	help
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index be3f631..2154092 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
 # Some architectures use the generic PCI setup functions
 #
 obj-$(CONFIG_ALPHA) += setup-irq.o
+obj-$(CONFIG_ARC) += setup-irq.o
 obj-$(CONFIG_ARM) += setup-irq.o
 obj-$(CONFIG_ARM64) += setup-irq.o
 obj-$(CONFIG_UNICORE32) += setup-irq.o
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-12 15:45   ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-12 15:45 UTC (permalink / raw)
  To: linux-snps-arc

This patch adds PCI support to ARC and updates drivers/pci Makefile
enabling the ARC arch to use the generic PCI setup functions.

Signed-off-by: Joao Pinto <jpinto at synopsys.com>
---
Change v4 -> v5 (Vineet Gupta):
- pcibios.c unused functions, variables and includes were removed
- ioremap.c has no need for changes (no patch is necessary)
- pci.h unused functions and variables were removed
Change v3 -> v4:
- Nothing changed (just to keep up with patch set version).
Change v2 -> v3 (Bjorn Helgaas):
- arch/arc/kernel/pcibios.c unused functions were removed and also the
arch/arc/include/asm/mach/pci.h was removed because was no longer necessary
Change v1 -> v2:
- In arch/arc/Kconfig, the new menu entry (Bus Configuration) was moved to the
slot between sourcing of drivers/Kconfig and fs/Kconfig (Vineet Gupta)
- In arch/arc/plat-axs10x/Kconfig the "select MIGHT_HAVE_PCI" option was placed
in order as suggested (Vineet Gupta)
- ioport_map() and ioport_unmap() were static inlined and included in
the io.h (Vineet Gupta)
- pcibios_min_io and pcibios_min_mem declaration moved to
pcibios.c (Vineet Gupta)
- pr_err() replaced by dev_err() in pcibios_enable_device() (Bjorn Helgaas)
- string simplified in pcibios_enable_device() (Vineet Gupta)
- pci_host_bridge_window structure was replaced by resource_entry structure, and
list_for_each_entry() for resource_list_for_each_entry() in pcibios.c

 arch/arc/Kconfig             | 23 +++++++++++++++++++++++
 arch/arc/include/asm/dma.h   |  5 +++++
 arch/arc/include/asm/io.h    |  9 +++++++++
 arch/arc/include/asm/pci.h   | 31 +++++++++++++++++++++++++++++++
 arch/arc/kernel/Makefile     |  1 +
 arch/arc/kernel/pcibios.c    | 24 ++++++++++++++++++++++++
 arch/arc/plat-axs10x/Kconfig |  1 +
 drivers/pci/Makefile         |  1 +
 8 files changed, 95 insertions(+)
 create mode 100644 arch/arc/include/asm/pci.h
 create mode 100644 arch/arc/kernel/pcibios.c

diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 2c2ac3f..98b32c1 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -19,6 +19,7 @@ config ARC
 	select GENERIC_FIND_FIRST_BIT
 	# for now, we don't need GENERIC_IRQ_PROBE, CONFIG_GENERIC_IRQ_CHIP
 	select GENERIC_IRQ_SHOW
+	select GENERIC_PCI_IOMAP
 	select GENERIC_PENDING_IRQ if SMP
 	select GENERIC_SMP_IDLE_THREAD
 	select HAVE_ARCH_KGDB
@@ -39,6 +40,9 @@ config ARC
 	select PERF_USE_VMALLOC
 	select HAVE_DEBUG_STACKOVERFLOW
 
+config MIGHT_HAVE_PCI
+	bool
+
 config TRACE_IRQFLAGS_SUPPORT
 	def_bool y
 
@@ -570,6 +574,25 @@ endmenu	 # "ARC Architecture Configuration"
 source "mm/Kconfig"
 source "net/Kconfig"
 source "drivers/Kconfig"
+
+menu "Bus Support"
+
+config PCI
+	bool "PCI support" if MIGHT_HAVE_PCI
+	help
+	  PCI is the name of a bus system, i.e. the way the CPU talks to the other stuff inside
+	  your box.Find out if your board/platform have PCI.
+	  Note: PCIE support for Synopsys Device will be available only when
+	  HAPS DX is configured with PCIE RC bitmap. If you have PCI, say Y, otherwise N.
+
+config PCI_SYSCALL
+	def_bool PCI
+
+source "drivers/pci/Kconfig"
+source "drivers/pci/pcie/Kconfig"
+
+endmenu
+
 source "fs/Kconfig"
 source "arch/arc/Kconfig.debug"
 source "security/Kconfig"
diff --git a/arch/arc/include/asm/dma.h b/arch/arc/include/asm/dma.h
index ca7c451..37942fa 100644
--- a/arch/arc/include/asm/dma.h
+++ b/arch/arc/include/asm/dma.h
@@ -10,5 +10,10 @@
 #define ASM_ARC_DMA_H
 
 #define MAX_DMA_ADDRESS 0xC0000000
+#ifdef CONFIG_PCI
+extern int isa_dma_bridge_buggy;
+#else
+#define isa_dma_bridge_buggy    (0)
+#endif
 
 #endif
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index 694ece8..947bf0c 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -16,6 +16,15 @@
 extern void __iomem *ioremap(unsigned long physaddr, unsigned long size);
 extern void __iomem *ioremap_prot(phys_addr_t offset, unsigned long size,
 				  unsigned long flags);
+static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
+{
+	return (void __iomem *)port;
+}
+
+static inline void ioport_unmap(void __iomem *addr)
+{
+}
+
 extern void iounmap(const void __iomem *addr);
 
 #define ioremap_nocache(phy, sz)	ioremap(phy, sz)
diff --git a/arch/arc/include/asm/pci.h b/arch/arc/include/asm/pci.h
new file mode 100644
index 0000000..2f091e1
--- /dev/null
+++ b/arch/arc/include/asm/pci.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _ASM_ARC_PCI_H
+#define _ASM_ARC_PCI_H
+
+#ifdef __KERNEL__
+#include <asm-generic/pci-dma-compat.h>
+#include <asm-generic/pci-bridge.h>
+
+#include <linux/ioport.h>
+
+#define PCIBIOS_MIN_IO 0x100
+#define PCIBIOS_MIN_MEM 0x100000
+
+#define pcibios_assign_all_busses()	1
+/*
+ * The PCI address space does equal the physical memory address space.
+ * The networking and block device layers use this boolean for bounce
+ * buffer decisions.
+ */
+#define PCI_DMA_BUS_IS_PHYS     (1)
+
+#endif /* __KERNEL__ */
+
+#endif /* _ASM_ARC_PCI_H */
diff --git a/arch/arc/kernel/Makefile b/arch/arc/kernel/Makefile
index e7f3625..1bc2036 100644
--- a/arch/arc/kernel/Makefile
+++ b/arch/arc/kernel/Makefile
@@ -12,6 +12,7 @@ obj-y	:= arcksyms.o setup.o irq.o time.o reset.o ptrace.o process.o devtree.o
 obj-y	+= signal.o traps.o sys.o troubleshoot.o stacktrace.o disasm.o clk.o
 obj-$(CONFIG_ISA_ARCOMPACT)		+= entry-compact.o intc-compact.o
 obj-$(CONFIG_ISA_ARCV2)			+= entry-arcv2.o intc-arcv2.o
+obj-$(CONFIG_PCI)  			+= pcibios.o
 
 obj-$(CONFIG_MODULES)			+= arcksyms.o module.o
 obj-$(CONFIG_SMP) 			+= smp.o
diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
new file mode 100644
index 0000000..12ea45a
--- /dev/null
+++ b/arch/arc/kernel/pcibios.c
@@ -0,0 +1,24 @@
+/*
+ * Copyright (C) 2014-2015 Synopsys, Inc. (www.synopsys.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/pci.h>
+
+/*
+ * We don't have to worry about legacy ISA devices, so nothing to do here
+ */
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
+				resource_size_t size, resource_size_t align)
+{
+	return res->start;
+}
+
+void pcibios_fixup_bus(struct pci_bus *bus)
+{
+}
+EXPORT_SYMBOL(pcibios_fixup_bus);
diff --git a/arch/arc/plat-axs10x/Kconfig b/arch/arc/plat-axs10x/Kconfig
index d475f9d..426ac4b 100644
--- a/arch/arc/plat-axs10x/Kconfig
+++ b/arch/arc/plat-axs10x/Kconfig
@@ -11,6 +11,7 @@ menuconfig ARC_PLAT_AXS10X
 	select DW_APB_ICTL
 	select GPIO_DWAPB
 	select OF_GPIO
+	select MIGHT_HAVE_PCI
 	select GENERIC_IRQ_CHIP
 	select ARCH_REQUIRE_GPIOLIB
 	help
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index be3f631..2154092 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
 # Some architectures use the generic PCI setup functions
 #
 obj-$(CONFIG_ALPHA) += setup-irq.o
+obj-$(CONFIG_ARC) += setup-irq.o
 obj-$(CONFIG_ARM) += setup-irq.o
 obj-$(CONFIG_ARM64) += setup-irq.o
 obj-$(CONFIG_UNICORE32) += setup-irq.o
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v5 2/2] add new platform driver for PCI RC
  2016-01-12 15:45 ` [PATCH v5 " Joao Pinto
  (?)
@ 2016-01-12 15:45   ` Joao Pinto
  -1 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-12 15:45 UTC (permalink / raw)
  To: Vineet.Gupta1, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, Joao Pinto

This patch adds a new driver that will be the reference platform driver
for all PCI RC IP Protoyping Kits based on ARC SDP.
This patch is composed by:

-Changes to pcie-designware driver add a function that enables the
feature of starting the LTSSM (Link Train Status State) used by the
new driver
-MAINTAINERS file was updated to include the new driver
-Documentation/devicetree/bindings/pci was updated to include the new
driver documentation
-New driver called pcie-snpsdev

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Change v4 -> v5:
- Nothing changed (just to keep up with patch set version).
Changes v3 -> v4 (Bjorn Helgaas):
- ARCH dependencies were added to the drivers/pci/host/kconfig for the
PCIE_SNPSDEV.
Changes v2 -> v3 (Bjorn Helgaas):
- link init stuff was moved to a snpsdev_pcie_establish_link() function in
pcie-snpsdev
- pcie-snpsdev driver declaration was changed to be more 
standard (Bjorn Helgaas)
- pcie-designware' dw_pcie_link_retrain() now use standard registers from
pci-regs.h (Bjorn Helgaas)
- pcie-snpsdev.txt was complemented with more info (Mark Rutland)
Changes v1 -> v2 (Bjorn Helgaas):
- Fixups snpsdev_pcie_fixup_bridge() and snpsdev_pcie_fixup_res() were removed
from the driver (these functions were for specific tests only and not usefull
in mainline)
- Driver' comments were reviewed (fix Typos and excessive comments removal)
- Removed unnecessary definitions in the driver source (PCIE_PHY_CTRL and
PCIE_PHY_STAT)

 .../devicetree/bindings/pci/pcie-snpsdev.txt       |  33 +++
 MAINTAINERS                                        |   7 +
 drivers/pci/host/Kconfig                           |   8 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-designware.c                 |   9 +
 drivers/pci/host/pcie-designware.h                 |   1 +
 drivers/pci/host/pcie-snpsdev.c                    | 286 +++++++++++++++++++++
 7 files changed, 345 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
 create mode 100644 drivers/pci/host/pcie-snpsdev.c

diff --git a/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
new file mode 100644
index 0000000..cae548b
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
@@ -0,0 +1,33 @@
+Synopsys PCI RC IP Prototyping Kit
+----------------------------------
+
+This is the reference platform driver to be used in the Synopsys PCI Root
+Complex IP Prototyping Kit.
+
+Required properties:
+- compatible: set to "snps,pcie-snpsdev";
+- reg: base address and length of the pcie controller registers.
+- #address-cells: set to <3>
+- #size-cells: set to <2>
+- device_type: set to "pci"
+- ranges: ranges for the PCI memory and I/O regions.
+- interrupts: one interrupt source for MSI interrupts, followed by interrupt
+	source for hardware related interrupts.
+- #interrupt-cells: set to <1>
+- num-lanes: set to <1>;
+
+Example configuration:
+
+	pcie: pcie@0xdffff000 {
+		compatible = "snps,pcie-snpsdev";
+		reg = <0xdffff000 0x1000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x00000800 0 0xd0000000 0xd0000000 0 0x00002000>,
+			 <0x81000000 0 0x00000000 0xde000000 0 0x00010000>,
+			 <0x82000000 0 0xd0400000 0xd0400000 0 0x0d000000>;
+		interrupts = <25>, <24>;
+		#interrupt-cells = <1>;
+		num-lanes = <1>;
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index e9caa4b..d2e4506 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8230,6 +8230,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
 F:	drivers/pci/host/pcie-hisi.c
 
+PCI DRIVER FOR SYNOPSYS PROTOTYPING DEVICE
+M:	Joao Pinto <jpinto@synopsys.com>
+L:	linux-pci@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
+F:	drivers/pci/host/pcie-snpsdev.c
+
 PCMCIA SUBSYSTEM
 P:	Linux PCMCIA Team
 L:	linux-pcmcia@lists.infradead.org
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index f131ba9..589bc15 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -172,4 +172,12 @@ config PCI_HISI
 	help
 	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
 
+config PCIE_SNPSDEV
+	bool "Platform Driver for Synopsys Device"
+	depends on ARC && OF
+	select PCIEPORTBUS
+	select PCIE_DW
+	help
+	  Say Y here if you want to enable PCIe controller support on the
+	  Synopsys IP Prototyping Kits.
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 9d4d3c6..e422f65 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
 obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
 obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
 obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
+obj-$(CONFIG_PCIE_SNPSDEV) += pcie-snpsdev.o
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 540f077..f73a3cf 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -706,6 +706,15 @@ static struct pci_ops dw_pcie_ops = {
 	.write = dw_pcie_wr_conf,
 };
 
+void dw_pcie_link_retrain(struct pcie_port *pp)
+{
+	u32 val = 0;
+
+	dw_pcie_readl_rc(pp, PCI_EXP_LNKCTL+0x70, &val);
+	val = val | PCI_EXP_LNKCTL_RL;
+	dw_pcie_writel_rc(pp, val, PCI_EXP_LNKCTL+0x70);
+}
+
 void dw_pcie_setup_rc(struct pcie_port *pp)
 {
 	u32 val;
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 2356d29..249b631 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -79,5 +79,6 @@ void dw_pcie_msi_init(struct pcie_port *pp);
 int dw_pcie_link_up(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);
+void dw_pcie_link_retrain(struct pcie_port *pp);
 
 #endif /* _PCIE_DESIGNWARE_H */
diff --git a/drivers/pci/host/pcie-snpsdev.c b/drivers/pci/host/pcie-snpsdev.c
new file mode 100644
index 0000000..4ca7ec5
--- /dev/null
+++ b/drivers/pci/host/pcie-snpsdev.c
@@ -0,0 +1,286 @@
+/*
+ * PCIe RC driver for Synopsys Designware Core
+ *
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * Authors: Manjunath Bettegowda <manjumb@synopsys.com>,
+ *	    Jie Deng <jiedeng@synopsys.com>
+ *	    Joao Pinto <jpinto@synopsys.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/resource.h>
+#include <linux/signal.h>
+#include <linux/types.h>
+
+#include "pcie-designware.h"
+
+#define to_snpsdev_pcie(x)	container_of(x, struct snpsdev_pcie, pp)
+
+struct snpsdev_pcie {
+	void __iomem		*mem_base; /* Memory Base to access Core's [RC]
+					    * Config Space Layout
+					    */
+	struct pcie_port	pp;        /* RC Root Port specific structure -
+					    * DWC_PCIE_RC stuff
+					    */
+};
+
+#define PCI_EQUAL_CONTROL_PHY 0x00000707
+
+/* PCIe Port Logic registers (memory-mapped) */
+#define PLR_OFFSET 0x700
+#define PCIE_PHY_DEBUG_R0 (PLR_OFFSET + 0x28) /* 0x728 */
+#define PCIE_PHY_DEBUG_R1 (PLR_OFFSET + 0x2c) /* 0x72c */
+
+/* This handler was created for future work */
+static irqreturn_t snpsdev_pcie_irq_handler(int irq, void *arg)
+{
+	return IRQ_NONE;
+}
+
+static irqreturn_t snpsdev_pcie_msi_irq_handler(int irq, void *arg)
+{
+	struct pcie_port *pp = arg;
+
+	dw_handle_msi_irq(pp);
+
+	return IRQ_HANDLED;
+}
+
+static void snpsdev_pcie_init_phy(struct pcie_port *pp)
+{
+	/* write Lane 0 Equalization Control fields register */
+	writel(PCI_EQUAL_CONTROL_PHY, pp->dbi_base + 0x154);
+}
+
+static int snpsdev_pcie_deassert_core_reset(struct pcie_port *pp)
+{
+	return 0;
+}
+
+static void snpsdev_pcie_establish_link(struct pcie_port *pp)
+{
+	int count = 0;
+
+	/* Initialize Phy (Reset/poweron/control-inputs ) */
+	snpsdev_pcie_init_phy(pp);
+
+	/* de-assert core reset */
+	snpsdev_pcie_deassert_core_reset(pp);
+
+	/* We expect the PCIe Link to be up by this time */
+	dw_pcie_setup_rc(pp);
+
+	/* Start LTSSM here */
+	dw_pcie_link_retrain(pp);
+
+	while (!dw_pcie_link_up(pp)) {
+		usleep_range(1000, 1100);
+		count++;
+		if (count == 20) {
+			dev_err(pp->dev, "phy link never came up\n");
+			dev_dbg(pp->dev,
+				"PL_DEBUG0: 0x%08x, DEBUG_R1: 0x%08x\n",
+				readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
+				readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
+			break;
+		}
+	}
+}
+
+/*
+ * snpsdev_pcie_host_init()
+ * Platform specific host/RC initialization
+ *	a. Assert the core reset
+ *	b. Assert and deassert phy reset and initialize the phy
+ *	c. De-Assert the core reset
+ *	d. Initializet the Root Port (BARs/Memory Or IO/ Interrupt/ Commnad Reg)
+ *	e. Initiate Link startup procedure
+ *
+ */
+static void snpsdev_pcie_host_init(struct pcie_port *pp)
+{
+	/* Establish link */
+	snpsdev_pcie_establish_link(pp);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		dw_pcie_msi_init(pp);
+}
+
+static int snpsdev_pcie_link_up(struct pcie_port *pp)
+{
+	u32 status;
+
+	/* Bit number 36: reports LTSSM PHY Link UP; Available in bit 3 of
+	 * PCIE_PHY_DEBUG_R1
+	 */
+	status = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1) & (0x1 << 4);
+	if (status != 0)
+		return 1;
+
+	/* TODO:Now Link is in L0;Initiate GEN2/GEN3 migration if RC Supports*/
+	return 0;
+}
+
+/**
+ * This is RC operation structure
+ * snpsdev_pcie_link_up: the function which initiates the phy link up procedure
+ * snpsdev_pcie_host_init: the function which does the host/RC Root port
+ * initialization.
+ */
+static struct pcie_host_ops snpsdev_pcie_host_ops = {
+	.link_up = snpsdev_pcie_link_up,
+	.host_init = snpsdev_pcie_host_init,
+};
+
+/**
+ * snpsdev_add_pcie_port
+ * This function
+ * a. installs the interrupt handler
+ * b. registers host operations in the pcie_port structure
+ */
+static int snpsdev_add_pcie_port(struct pcie_port *pp,
+				 struct platform_device *pdev)
+{
+	int ret;
+
+	pp->irq = platform_get_irq(pdev, 1);
+
+	if (pp->irq < 0) {
+		if (pp->irq != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "cannot get irq\n");
+		return pp->irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, pp->irq, snpsdev_pcie_irq_handler,
+				IRQF_SHARED, "snpsdev-pcie", pp);
+
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request irq\n");
+		return ret;
+	}
+
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		pp->msi_irq = platform_get_irq(pdev, 0);
+
+		if (pp->msi_irq < 0) {
+			if (pp->msi_irq != -EPROBE_DEFER)
+				dev_err(&pdev->dev, "cannot get msi irq\n");
+			return pp->msi_irq;
+		}
+
+		ret = devm_request_irq(&pdev->dev, pp->msi_irq,
+					snpsdev_pcie_msi_irq_handler,
+					IRQF_SHARED, "snpsdev-pcie-msi", pp);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to request msi irq\n");
+			return ret;
+		}
+	}
+
+	pp->root_bus_nr = -1;
+	pp->ops = &snpsdev_pcie_host_ops;
+
+	/* Below function:
+	 * Checks for range property from DT
+	 * Gets the IO and MEMORY and CONFIG-Space ranges from DT
+	 * Does IOREMAPS on the physical addresses
+	 * Gets the num-lanes from DT
+	 * Gets MSI capability from DT
+	 * Calls the platform specific host initialization
+	 * Program the correct class, BAR0, Link width, in Config space
+	 * Then it calls pci common init routine
+	 * Then it calls function to assign "unassigned resources"
+	 */
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize host\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * snpsdev_pcie_rc_probe()
+ * This function gets called as part of pcie registration. if the id matches
+ * the platform driver framework will call this function.
+ *
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Returns zero on success; Negative errorno on failure
+ */
+static int snpsdev_pcie_rc_probe(struct platform_device *pdev)
+{
+	struct snpsdev_pcie *snpsdev_pcie;
+	struct pcie_port *pp;
+	struct resource *dwc_pcie_rc_res;  /* Resource from DT */
+	int ret;
+
+	snpsdev_pcie = devm_kzalloc(&pdev->dev, sizeof(*snpsdev_pcie),
+					GFP_KERNEL);
+	if (!snpsdev_pcie)
+		return -ENOMEM;
+
+	pp = &snpsdev_pcie->pp;
+	pp->dev = &pdev->dev;
+
+	dwc_pcie_rc_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	if (!dwc_pcie_rc_res)
+		return -ENODEV;
+
+	snpsdev_pcie->mem_base = devm_ioremap_resource(&pdev->dev,
+							dwc_pcie_rc_res);
+	if (IS_ERR(snpsdev_pcie->mem_base)) {
+		ret = PTR_ERR(snpsdev_pcie->mem_base);
+		return ret;
+	}
+	pp->dbi_base = snpsdev_pcie->mem_base;
+
+	ret = snpsdev_add_pcie_port(pp, pdev);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, snpsdev_pcie);
+
+	return 0;
+}
+
+static int snpsdev_pcie_rc_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static const struct of_device_id snpsdev_pcie_rc_of_match[] = {
+	{ .compatible = "snps,pcie-snpsdev", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, snpsdev_pcie_rc_of_match);
+
+static struct platform_driver snpsdev_pcie_rc_driver = {
+	.driver = {
+		.name	= "pcie-snpsdev",
+		.of_match_table = snpsdev_pcie_rc_of_match,
+	},
+	.probe = snpsdev_pcie_rc_probe,
+};
+
+module_platform_driver(snpsdev_pcie_rc_driver);
+
+MODULE_AUTHOR("Manjunath Bettegowda <manjumb@synopsys.com>");
+MODULE_DESCRIPTION("Platform Driver for Synopsys Device");
+MODULE_LICENSE("GPL v2");
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v5 2/2] add new platform driver for PCI RC
@ 2016-01-12 15:45   ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-12 15:45 UTC (permalink / raw)
  To: linux-snps-arc

This patch adds a new driver that will be the reference platform driver
for all PCI RC IP Protoyping Kits based on ARC SDP.
This patch is composed by:

-Changes to pcie-designware driver add a function that enables the
feature of starting the LTSSM (Link Train Status State) used by the
new driver
-MAINTAINERS file was updated to include the new driver
-Documentation/devicetree/bindings/pci was updated to include the new
driver documentation
-New driver called pcie-snpsdev

Signed-off-by: Joao Pinto <jpinto at synopsys.com>
---
Change v4 -> v5:
- Nothing changed (just to keep up with patch set version).
Changes v3 -> v4 (Bjorn Helgaas):
- ARCH dependencies were added to the drivers/pci/host/kconfig for the
PCIE_SNPSDEV.
Changes v2 -> v3 (Bjorn Helgaas):
- link init stuff was moved to a snpsdev_pcie_establish_link() function in
pcie-snpsdev
- pcie-snpsdev driver declaration was changed to be more 
standard (Bjorn Helgaas)
- pcie-designware' dw_pcie_link_retrain() now use standard registers from
pci-regs.h (Bjorn Helgaas)
- pcie-snpsdev.txt was complemented with more info (Mark Rutland)
Changes v1 -> v2 (Bjorn Helgaas):
- Fixups snpsdev_pcie_fixup_bridge() and snpsdev_pcie_fixup_res() were removed
from the driver (these functions were for specific tests only and not usefull
in mainline)
- Driver' comments were reviewed (fix Typos and excessive comments removal)
- Removed unnecessary definitions in the driver source (PCIE_PHY_CTRL and
PCIE_PHY_STAT)

 .../devicetree/bindings/pci/pcie-snpsdev.txt       |  33 +++
 MAINTAINERS                                        |   7 +
 drivers/pci/host/Kconfig                           |   8 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-designware.c                 |   9 +
 drivers/pci/host/pcie-designware.h                 |   1 +
 drivers/pci/host/pcie-snpsdev.c                    | 286 +++++++++++++++++++++
 7 files changed, 345 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
 create mode 100644 drivers/pci/host/pcie-snpsdev.c

diff --git a/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
new file mode 100644
index 0000000..cae548b
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
@@ -0,0 +1,33 @@
+Synopsys PCI RC IP Prototyping Kit
+----------------------------------
+
+This is the reference platform driver to be used in the Synopsys PCI Root
+Complex IP Prototyping Kit.
+
+Required properties:
+- compatible: set to "snps,pcie-snpsdev";
+- reg: base address and length of the pcie controller registers.
+- #address-cells: set to <3>
+- #size-cells: set to <2>
+- device_type: set to "pci"
+- ranges: ranges for the PCI memory and I/O regions.
+- interrupts: one interrupt source for MSI interrupts, followed by interrupt
+	source for hardware related interrupts.
+- #interrupt-cells: set to <1>
+- num-lanes: set to <1>;
+
+Example configuration:
+
+	pcie: pcie at 0xdffff000 {
+		compatible = "snps,pcie-snpsdev";
+		reg = <0xdffff000 0x1000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x00000800 0 0xd0000000 0xd0000000 0 0x00002000>,
+			 <0x81000000 0 0x00000000 0xde000000 0 0x00010000>,
+			 <0x82000000 0 0xd0400000 0xd0400000 0 0x0d000000>;
+		interrupts = <25>, <24>;
+		#interrupt-cells = <1>;
+		num-lanes = <1>;
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index e9caa4b..d2e4506 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8230,6 +8230,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
 F:	drivers/pci/host/pcie-hisi.c
 
+PCI DRIVER FOR SYNOPSYS PROTOTYPING DEVICE
+M:	Joao Pinto <jpinto at synopsys.com>
+L:	linux-pci at vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
+F:	drivers/pci/host/pcie-snpsdev.c
+
 PCMCIA SUBSYSTEM
 P:	Linux PCMCIA Team
 L:	linux-pcmcia at lists.infradead.org
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index f131ba9..589bc15 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -172,4 +172,12 @@ config PCI_HISI
 	help
 	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
 
+config PCIE_SNPSDEV
+	bool "Platform Driver for Synopsys Device"
+	depends on ARC && OF
+	select PCIEPORTBUS
+	select PCIE_DW
+	help
+	  Say Y here if you want to enable PCIe controller support on the
+	  Synopsys IP Prototyping Kits.
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 9d4d3c6..e422f65 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
 obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
 obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
 obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
+obj-$(CONFIG_PCIE_SNPSDEV) += pcie-snpsdev.o
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 540f077..f73a3cf 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -706,6 +706,15 @@ static struct pci_ops dw_pcie_ops = {
 	.write = dw_pcie_wr_conf,
 };
 
+void dw_pcie_link_retrain(struct pcie_port *pp)
+{
+	u32 val = 0;
+
+	dw_pcie_readl_rc(pp, PCI_EXP_LNKCTL+0x70, &val);
+	val = val | PCI_EXP_LNKCTL_RL;
+	dw_pcie_writel_rc(pp, val, PCI_EXP_LNKCTL+0x70);
+}
+
 void dw_pcie_setup_rc(struct pcie_port *pp)
 {
 	u32 val;
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 2356d29..249b631 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -79,5 +79,6 @@ void dw_pcie_msi_init(struct pcie_port *pp);
 int dw_pcie_link_up(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);
+void dw_pcie_link_retrain(struct pcie_port *pp);
 
 #endif /* _PCIE_DESIGNWARE_H */
diff --git a/drivers/pci/host/pcie-snpsdev.c b/drivers/pci/host/pcie-snpsdev.c
new file mode 100644
index 0000000..4ca7ec5
--- /dev/null
+++ b/drivers/pci/host/pcie-snpsdev.c
@@ -0,0 +1,286 @@
+/*
+ * PCIe RC driver for Synopsys Designware Core
+ *
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * Authors: Manjunath Bettegowda <manjumb at synopsys.com>,
+ *	    Jie Deng <jiedeng at synopsys.com>
+ *	    Joao Pinto <jpinto at synopsys.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/resource.h>
+#include <linux/signal.h>
+#include <linux/types.h>
+
+#include "pcie-designware.h"
+
+#define to_snpsdev_pcie(x)	container_of(x, struct snpsdev_pcie, pp)
+
+struct snpsdev_pcie {
+	void __iomem		*mem_base; /* Memory Base to access Core's [RC]
+					    * Config Space Layout
+					    */
+	struct pcie_port	pp;        /* RC Root Port specific structure -
+					    * DWC_PCIE_RC stuff
+					    */
+};
+
+#define PCI_EQUAL_CONTROL_PHY 0x00000707
+
+/* PCIe Port Logic registers (memory-mapped) */
+#define PLR_OFFSET 0x700
+#define PCIE_PHY_DEBUG_R0 (PLR_OFFSET + 0x28) /* 0x728 */
+#define PCIE_PHY_DEBUG_R1 (PLR_OFFSET + 0x2c) /* 0x72c */
+
+/* This handler was created for future work */
+static irqreturn_t snpsdev_pcie_irq_handler(int irq, void *arg)
+{
+	return IRQ_NONE;
+}
+
+static irqreturn_t snpsdev_pcie_msi_irq_handler(int irq, void *arg)
+{
+	struct pcie_port *pp = arg;
+
+	dw_handle_msi_irq(pp);
+
+	return IRQ_HANDLED;
+}
+
+static void snpsdev_pcie_init_phy(struct pcie_port *pp)
+{
+	/* write Lane 0 Equalization Control fields register */
+	writel(PCI_EQUAL_CONTROL_PHY, pp->dbi_base + 0x154);
+}
+
+static int snpsdev_pcie_deassert_core_reset(struct pcie_port *pp)
+{
+	return 0;
+}
+
+static void snpsdev_pcie_establish_link(struct pcie_port *pp)
+{
+	int count = 0;
+
+	/* Initialize Phy (Reset/poweron/control-inputs ) */
+	snpsdev_pcie_init_phy(pp);
+
+	/* de-assert core reset */
+	snpsdev_pcie_deassert_core_reset(pp);
+
+	/* We expect the PCIe Link to be up by this time */
+	dw_pcie_setup_rc(pp);
+
+	/* Start LTSSM here */
+	dw_pcie_link_retrain(pp);
+
+	while (!dw_pcie_link_up(pp)) {
+		usleep_range(1000, 1100);
+		count++;
+		if (count == 20) {
+			dev_err(pp->dev, "phy link never came up\n");
+			dev_dbg(pp->dev,
+				"PL_DEBUG0: 0x%08x, DEBUG_R1: 0x%08x\n",
+				readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
+				readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
+			break;
+		}
+	}
+}
+
+/*
+ * snpsdev_pcie_host_init()
+ * Platform specific host/RC initialization
+ *	a. Assert the core reset
+ *	b. Assert and deassert phy reset and initialize the phy
+ *	c. De-Assert the core reset
+ *	d. Initializet the Root Port (BARs/Memory Or IO/ Interrupt/ Commnad Reg)
+ *	e. Initiate Link startup procedure
+ *
+ */
+static void snpsdev_pcie_host_init(struct pcie_port *pp)
+{
+	/* Establish link */
+	snpsdev_pcie_establish_link(pp);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		dw_pcie_msi_init(pp);
+}
+
+static int snpsdev_pcie_link_up(struct pcie_port *pp)
+{
+	u32 status;
+
+	/* Bit number 36: reports LTSSM PHY Link UP; Available in bit 3 of
+	 * PCIE_PHY_DEBUG_R1
+	 */
+	status = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1) & (0x1 << 4);
+	if (status != 0)
+		return 1;
+
+	/* TODO:Now Link is in L0;Initiate GEN2/GEN3 migration if RC Supports*/
+	return 0;
+}
+
+/**
+ * This is RC operation structure
+ * snpsdev_pcie_link_up: the function which initiates the phy link up procedure
+ * snpsdev_pcie_host_init: the function which does the host/RC Root port
+ * initialization.
+ */
+static struct pcie_host_ops snpsdev_pcie_host_ops = {
+	.link_up = snpsdev_pcie_link_up,
+	.host_init = snpsdev_pcie_host_init,
+};
+
+/**
+ * snpsdev_add_pcie_port
+ * This function
+ * a. installs the interrupt handler
+ * b. registers host operations in the pcie_port structure
+ */
+static int snpsdev_add_pcie_port(struct pcie_port *pp,
+				 struct platform_device *pdev)
+{
+	int ret;
+
+	pp->irq = platform_get_irq(pdev, 1);
+
+	if (pp->irq < 0) {
+		if (pp->irq != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "cannot get irq\n");
+		return pp->irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, pp->irq, snpsdev_pcie_irq_handler,
+				IRQF_SHARED, "snpsdev-pcie", pp);
+
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request irq\n");
+		return ret;
+	}
+
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		pp->msi_irq = platform_get_irq(pdev, 0);
+
+		if (pp->msi_irq < 0) {
+			if (pp->msi_irq != -EPROBE_DEFER)
+				dev_err(&pdev->dev, "cannot get msi irq\n");
+			return pp->msi_irq;
+		}
+
+		ret = devm_request_irq(&pdev->dev, pp->msi_irq,
+					snpsdev_pcie_msi_irq_handler,
+					IRQF_SHARED, "snpsdev-pcie-msi", pp);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to request msi irq\n");
+			return ret;
+		}
+	}
+
+	pp->root_bus_nr = -1;
+	pp->ops = &snpsdev_pcie_host_ops;
+
+	/* Below function:
+	 * Checks for range property from DT
+	 * Gets the IO and MEMORY and CONFIG-Space ranges from DT
+	 * Does IOREMAPS on the physical addresses
+	 * Gets the num-lanes from DT
+	 * Gets MSI capability from DT
+	 * Calls the platform specific host initialization
+	 * Program the correct class, BAR0, Link width, in Config space
+	 * Then it calls pci common init routine
+	 * Then it calls function to assign "unassigned resources"
+	 */
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize host\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * snpsdev_pcie_rc_probe()
+ * This function gets called as part of pcie registration. if the id matches
+ * the platform driver framework will call this function.
+ *
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Returns zero on success; Negative errorno on failure
+ */
+static int snpsdev_pcie_rc_probe(struct platform_device *pdev)
+{
+	struct snpsdev_pcie *snpsdev_pcie;
+	struct pcie_port *pp;
+	struct resource *dwc_pcie_rc_res;  /* Resource from DT */
+	int ret;
+
+	snpsdev_pcie = devm_kzalloc(&pdev->dev, sizeof(*snpsdev_pcie),
+					GFP_KERNEL);
+	if (!snpsdev_pcie)
+		return -ENOMEM;
+
+	pp = &snpsdev_pcie->pp;
+	pp->dev = &pdev->dev;
+
+	dwc_pcie_rc_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	if (!dwc_pcie_rc_res)
+		return -ENODEV;
+
+	snpsdev_pcie->mem_base = devm_ioremap_resource(&pdev->dev,
+							dwc_pcie_rc_res);
+	if (IS_ERR(snpsdev_pcie->mem_base)) {
+		ret = PTR_ERR(snpsdev_pcie->mem_base);
+		return ret;
+	}
+	pp->dbi_base = snpsdev_pcie->mem_base;
+
+	ret = snpsdev_add_pcie_port(pp, pdev);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, snpsdev_pcie);
+
+	return 0;
+}
+
+static int snpsdev_pcie_rc_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static const struct of_device_id snpsdev_pcie_rc_of_match[] = {
+	{ .compatible = "snps,pcie-snpsdev", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, snpsdev_pcie_rc_of_match);
+
+static struct platform_driver snpsdev_pcie_rc_driver = {
+	.driver = {
+		.name	= "pcie-snpsdev",
+		.of_match_table = snpsdev_pcie_rc_of_match,
+	},
+	.probe = snpsdev_pcie_rc_probe,
+};
+
+module_platform_driver(snpsdev_pcie_rc_driver);
+
+MODULE_AUTHOR("Manjunath Bettegowda <manjumb at synopsys.com>");
+MODULE_DESCRIPTION("Platform Driver for Synopsys Device");
+MODULE_LICENSE("GPL v2");
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
  2016-01-12 15:45   ` [PATCH v5 " Joao Pinto
  (?)
@ 2016-01-14  5:26     ` Vineet Gupta
  -1 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14  5:26 UTC (permalink / raw)
  To: Joao Pinto, Vineet.Gupta1, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, Arnd Bergmann

+CC Arnd

On Tuesday 12 January 2016 09:15 PM, Joao Pinto wrote:
> This patch adds PCI support to ARC and updates drivers/pci Makefile
> enabling the ARC arch to use the generic PCI setup functions.
>
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>

Much better now. I have one small question below and if that is not contended fine
by me.

> diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
> new file mode 100644
> index 0000000..12ea45a
> --- /dev/null
> +++ b/arch/arc/kernel/pcibios.c
> @@ -0,0 +1,24 @@
> +/*
> + * Copyright (C) 2014-2015 Synopsys, Inc. (www.synopsys.com)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/pci.h>
> +
> +/*
> + * We don't have to worry about legacy ISA devices, so nothing to do here
> + */
> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> +				resource_size_t size, resource_size_t align)
> +{
> +	return res->start;
> +}

Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
(setup-res.c) can build as module ?

> +
> +void pcibios_fixup_bus(struct pci_bus *bus)
> +{
> +}
> +EXPORT_SYMBOL(pcibios_fixup_bus);

EXPORT_SYMBOL_GPL ?

As a seperate enhancement, it would be nicer if these 2 functions are defined weak
in common code. That would make basic PCI support almost arch independent !

> diff --git a/arch/arc/plat-axs10x/Kconfig b/arch/arc/plat-axs10x/Kconfig
> index d475f9d..426ac4b 100644
> --- a/arch/arc/plat-axs10x/Kconfig
> +++ b/arch/arc/plat-axs10x/Kconfig
> @@ -11,6 +11,7 @@ menuconfig ARC_PLAT_AXS10X
>  	select DW_APB_ICTL
>  	select GPIO_DWAPB
>  	select OF_GPIO
> +	select MIGHT_HAVE_PCI
>  	select GENERIC_IRQ_CHIP
>  	select ARCH_REQUIRE_GPIOLIB
>  	help
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index be3f631..2154092 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
>  # Some architectures use the generic PCI setup functions
>  #
>  obj-$(CONFIG_ALPHA) += setup-irq.o
> +obj-$(CONFIG_ARC) += setup-irq.o
>  obj-$(CONFIG_ARM) += setup-irq.o
>  obj-$(CONFIG_ARM64) += setup-irq.o
>  obj-$(CONFIG_UNICORE32) += setup-irq.o

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14  5:26     ` Vineet Gupta
  0 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14  5:26 UTC (permalink / raw)
  To: Joao Pinto, Vineet.Gupta1, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, Arnd Bergmann

+CC Arnd

On Tuesday 12 January 2016 09:15 PM, Joao Pinto wrote:
> This patch adds PCI support to ARC and updates drivers/pci Makefile
> enabling the ARC arch to use the generic PCI setup functions.
>
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>

Much better now. I have one small question below and if that is not contended fine
by me.

> diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
> new file mode 100644
> index 0000000..12ea45a
> --- /dev/null
> +++ b/arch/arc/kernel/pcibios.c
> @@ -0,0 +1,24 @@
> +/*
> + * Copyright (C) 2014-2015 Synopsys, Inc. (www.synopsys.com)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/pci.h>
> +
> +/*
> + * We don't have to worry about legacy ISA devices, so nothing to do here
> + */
> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> +				resource_size_t size, resource_size_t align)
> +{
> +	return res->start;
> +}

Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
(setup-res.c) can build as module ?

> +
> +void pcibios_fixup_bus(struct pci_bus *bus)
> +{
> +}
> +EXPORT_SYMBOL(pcibios_fixup_bus);

EXPORT_SYMBOL_GPL ?

As a seperate enhancement, it would be nicer if these 2 functions are defined weak
in common code. That would make basic PCI support almost arch independent !

> diff --git a/arch/arc/plat-axs10x/Kconfig b/arch/arc/plat-axs10x/Kconfig
> index d475f9d..426ac4b 100644
> --- a/arch/arc/plat-axs10x/Kconfig
> +++ b/arch/arc/plat-axs10x/Kconfig
> @@ -11,6 +11,7 @@ menuconfig ARC_PLAT_AXS10X
>  	select DW_APB_ICTL
>  	select GPIO_DWAPB
>  	select OF_GPIO
> +	select MIGHT_HAVE_PCI
>  	select GENERIC_IRQ_CHIP
>  	select ARCH_REQUIRE_GPIOLIB
>  	help
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index be3f631..2154092 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
>  # Some architectures use the generic PCI setup functions
>  #
>  obj-$(CONFIG_ALPHA) += setup-irq.o
> +obj-$(CONFIG_ARC) += setup-irq.o
>  obj-$(CONFIG_ARM) += setup-irq.o
>  obj-$(CONFIG_ARM64) += setup-irq.o
>  obj-$(CONFIG_UNICORE32) += setup-irq.o

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14  5:26     ` Vineet Gupta
  0 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14  5:26 UTC (permalink / raw)
  To: linux-snps-arc

+CC Arnd

On Tuesday 12 January 2016 09:15 PM, Joao Pinto wrote:
> This patch adds PCI support to ARC and updates drivers/pci Makefile
> enabling the ARC arch to use the generic PCI setup functions.
>
> Signed-off-by: Joao Pinto <jpinto at synopsys.com>

Much better now. I have one small question below and if that is not contended fine
by me.

> diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
> new file mode 100644
> index 0000000..12ea45a
> --- /dev/null
> +++ b/arch/arc/kernel/pcibios.c
> @@ -0,0 +1,24 @@
> +/*
> + * Copyright (C) 2014-2015 Synopsys, Inc. (www.synopsys.com)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/pci.h>
> +
> +/*
> + * We don't have to worry about legacy ISA devices, so nothing to do here
> + */
> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> +				resource_size_t size, resource_size_t align)
> +{
> +	return res->start;
> +}

Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
(setup-res.c) can build as module ?

> +
> +void pcibios_fixup_bus(struct pci_bus *bus)
> +{
> +}
> +EXPORT_SYMBOL(pcibios_fixup_bus);

EXPORT_SYMBOL_GPL ?

As a seperate enhancement, it would be nicer if these 2 functions are defined weak
in common code. That would make basic PCI support almost arch independent !

> diff --git a/arch/arc/plat-axs10x/Kconfig b/arch/arc/plat-axs10x/Kconfig
> index d475f9d..426ac4b 100644
> --- a/arch/arc/plat-axs10x/Kconfig
> +++ b/arch/arc/plat-axs10x/Kconfig
> @@ -11,6 +11,7 @@ menuconfig ARC_PLAT_AXS10X
>  	select DW_APB_ICTL
>  	select GPIO_DWAPB
>  	select OF_GPIO
> +	select MIGHT_HAVE_PCI
>  	select GENERIC_IRQ_CHIP
>  	select ARCH_REQUIRE_GPIOLIB
>  	help
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index be3f631..2154092 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
>  # Some architectures use the generic PCI setup functions
>  #
>  obj-$(CONFIG_ALPHA) += setup-irq.o
> +obj-$(CONFIG_ARC) += setup-irq.o
>  obj-$(CONFIG_ARM) += setup-irq.o
>  obj-$(CONFIG_ARM64) += setup-irq.o
>  obj-$(CONFIG_UNICORE32) += setup-irq.o

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
  2016-01-14  5:26     ` Vineet Gupta
  (?)
@ 2016-01-14 10:22       ` Arnd Bergmann
  -1 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2016-01-14 10:22 UTC (permalink / raw)
  To: Vineet Gupta
  Cc: Joao Pinto, helgaas, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak

On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
> > +/*
> > + * We don't have to worry about legacy ISA devices, so nothing to do here
> > + */
> > +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> > +                             resource_size_t size, resource_size_t align)
> > +{
> > +     return res->start;
> > +}
> 
> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
> (setup-res.c) can build as module ?

I only see a caller in drivers/pci/setup-res.c, and that is never part of a
loadable module.

> > +
> > +void pcibios_fixup_bus(struct pci_bus *bus)
> > +{
> > +}
> > +EXPORT_SYMBOL(pcibios_fixup_bus);
> 
> EXPORT_SYMBOL_GPL ?
> 
> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
> in common code. That would make basic PCI support almost arch independent !

I agree, that would be ideal. An easy way to do this would be to add
them as __weak functions in drivers/pci/, similar to how we handle
a lot of the other pcibios_* functions.

A somewhat nicer method would be to have callback pointers in struct
pci_host_bridge, and call those when they are non-NULL so we can
remove the global pcibios_* functions from the API. That would also
bring us a big step closer to having PCI support itself as a loadable
module, and it would better reflect that those functions are really
host bridge specific rather than architecture specific. When you use
the same host bridge on multiple architectures, you typically have
the same requirements for hacks in there, but each architectures may
need to support multiple host bridges with different requirements.

	Arnd

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 10:22       ` Arnd Bergmann
  0 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2016-01-14 10:22 UTC (permalink / raw)
  To: Vineet Gupta
  Cc: Joao Pinto, helgaas, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak

On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
> > +/*
> > + * We don't have to worry about legacy ISA devices, so nothing to do here
> > + */
> > +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> > +                             resource_size_t size, resource_size_t align)
> > +{
> > +     return res->start;
> > +}
> 
> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
> (setup-res.c) can build as module ?

I only see a caller in drivers/pci/setup-res.c, and that is never part of a
loadable module.

> > +
> > +void pcibios_fixup_bus(struct pci_bus *bus)
> > +{
> > +}
> > +EXPORT_SYMBOL(pcibios_fixup_bus);
> 
> EXPORT_SYMBOL_GPL ?
> 
> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
> in common code. That would make basic PCI support almost arch independent !

I agree, that would be ideal. An easy way to do this would be to add
them as __weak functions in drivers/pci/, similar to how we handle
a lot of the other pcibios_* functions.

A somewhat nicer method would be to have callback pointers in struct
pci_host_bridge, and call those when they are non-NULL so we can
remove the global pcibios_* functions from the API. That would also
bring us a big step closer to having PCI support itself as a loadable
module, and it would better reflect that those functions are really
host bridge specific rather than architecture specific. When you use
the same host bridge on multiple architectures, you typically have
the same requirements for hacks in there, but each architectures may
need to support multiple host bridges with different requirements.

	Arnd

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 10:22       ` Arnd Bergmann
  0 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2016-01-14 10:22 UTC (permalink / raw)
  To: linux-snps-arc

On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
> > +/*
> > + * We don't have to worry about legacy ISA devices, so nothing to do here
> > + */
> > +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> > +                             resource_size_t size, resource_size_t align)
> > +{
> > +     return res->start;
> > +}
> 
> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
> (setup-res.c) can build as module ?

I only see a caller in drivers/pci/setup-res.c, and that is never part of a
loadable module.

> > +
> > +void pcibios_fixup_bus(struct pci_bus *bus)
> > +{
> > +}
> > +EXPORT_SYMBOL(pcibios_fixup_bus);
> 
> EXPORT_SYMBOL_GPL ?
> 
> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
> in common code. That would make basic PCI support almost arch independent !

I agree, that would be ideal. An easy way to do this would be to add
them as __weak functions in drivers/pci/, similar to how we handle
a lot of the other pcibios_* functions.

A somewhat nicer method would be to have callback pointers in struct
pci_host_bridge, and call those when they are non-NULL so we can
remove the global pcibios_* functions from the API. That would also
bring us a big step closer to having PCI support itself as a loadable
module, and it would better reflect that those functions are really
host bridge specific rather than architecture specific. When you use
the same host bridge on multiple architectures, you typically have
the same requirements for hacks in there, but each architectures may
need to support multiple host bridges with different requirements.

	Arnd

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
  2016-01-14 10:22       ` Arnd Bergmann
  (?)
@ 2016-01-14 10:42         ` Joao Pinto
  -1 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-14 10:42 UTC (permalink / raw)
  To: Arnd Bergmann, Vineet Gupta
  Cc: Joao Pinto, helgaas, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak

Hi,

On 1/14/2016 10:22 AM, Arnd Bergmann wrote:
> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
>>> +/*
>>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>>> + */
>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>> +                             resource_size_t size, resource_size_t align)
>>> +{
>>> +     return res->start;
>>> +}
>>
>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
>> (setup-res.c) can build as module ?
> 
> I only see a caller in drivers/pci/setup-res.c, and that is never part of a
> loadable module.
> 
>>> +
>>> +void pcibios_fixup_bus(struct pci_bus *bus)
>>> +{
>>> +}
>>> +EXPORT_SYMBOL(pcibios_fixup_bus);
>>
>> EXPORT_SYMBOL_GPL ?
>>
>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
>> in common code. That would make basic PCI support almost arch independent !
> 
> I agree, that would be ideal. An easy way to do this would be to add
> them as __weak functions in drivers/pci/, similar to how we handle
> a lot of the other pcibios_* functions.
> 
> A somewhat nicer method would be to have callback pointers in struct
> pci_host_bridge, and call those when they are non-NULL so we can
> remove the global pcibios_* functions from the API. That would also
> bring us a big step closer to having PCI support itself as a loadable
> module, and it would better reflect that those functions are really
> host bridge specific rather than architecture specific. When you use
> the same host bridge on multiple architectures, you typically have
> the same requirements for hacks in there, but each architectures may
> need to support multiple host bridges with different requirements.

Since we will be constantly improving the driver and the core itself, I suggest
that this functions be made __weak and in an update we can turn it struct
pointers just like Arnd suggested. Is this good for you?

> 
> 	Arnd
> 

Thanks
Joao

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 10:42         ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-14 10:42 UTC (permalink / raw)
  To: Arnd Bergmann, Vineet Gupta
  Cc: Joao Pinto, helgaas, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak

Hi,

On 1/14/2016 10:22 AM, Arnd Bergmann wrote:
> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
>>> +/*
>>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>>> + */
>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>> +                             resource_size_t size, resource_size_t align)
>>> +{
>>> +     return res->start;
>>> +}
>>
>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
>> (setup-res.c) can build as module ?
> 
> I only see a caller in drivers/pci/setup-res.c, and that is never part of a
> loadable module.
> 
>>> +
>>> +void pcibios_fixup_bus(struct pci_bus *bus)
>>> +{
>>> +}
>>> +EXPORT_SYMBOL(pcibios_fixup_bus);
>>
>> EXPORT_SYMBOL_GPL ?
>>
>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
>> in common code. That would make basic PCI support almost arch independent !
> 
> I agree, that would be ideal. An easy way to do this would be to add
> them as __weak functions in drivers/pci/, similar to how we handle
> a lot of the other pcibios_* functions.
> 
> A somewhat nicer method would be to have callback pointers in struct
> pci_host_bridge, and call those when they are non-NULL so we can
> remove the global pcibios_* functions from the API. That would also
> bring us a big step closer to having PCI support itself as a loadable
> module, and it would better reflect that those functions are really
> host bridge specific rather than architecture specific. When you use
> the same host bridge on multiple architectures, you typically have
> the same requirements for hacks in there, but each architectures may
> need to support multiple host bridges with different requirements.

Since we will be constantly improving the driver and the core itself, I suggest
that this functions be made __weak and in an update we can turn it struct
pointers just like Arnd suggested. Is this good for you?

> 
> 	Arnd
> 

Thanks
Joao



^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 10:42         ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-14 10:42 UTC (permalink / raw)
  To: linux-snps-arc

Hi,

On 1/14/2016 10:22 AM, Arnd Bergmann wrote:
> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
>>> +/*
>>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>>> + */
>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>> +                             resource_size_t size, resource_size_t align)
>>> +{
>>> +     return res->start;
>>> +}
>>
>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
>> (setup-res.c) can build as module ?
> 
> I only see a caller in drivers/pci/setup-res.c, and that is never part of a
> loadable module.
> 
>>> +
>>> +void pcibios_fixup_bus(struct pci_bus *bus)
>>> +{
>>> +}
>>> +EXPORT_SYMBOL(pcibios_fixup_bus);
>>
>> EXPORT_SYMBOL_GPL ?
>>
>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
>> in common code. That would make basic PCI support almost arch independent !
> 
> I agree, that would be ideal. An easy way to do this would be to add
> them as __weak functions in drivers/pci/, similar to how we handle
> a lot of the other pcibios_* functions.
> 
> A somewhat nicer method would be to have callback pointers in struct
> pci_host_bridge, and call those when they are non-NULL so we can
> remove the global pcibios_* functions from the API. That would also
> bring us a big step closer to having PCI support itself as a loadable
> module, and it would better reflect that those functions are really
> host bridge specific rather than architecture specific. When you use
> the same host bridge on multiple architectures, you typically have
> the same requirements for hacks in there, but each architectures may
> need to support multiple host bridges with different requirements.

Since we will be constantly improving the driver and the core itself, I suggest
that this functions be made __weak and in an update we can turn it struct
pointers just like Arnd suggested. Is this good for you?

> 
> 	Arnd
> 

Thanks
Joao

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
  2016-01-14 10:42         ` Joao Pinto
  (?)
@ 2016-01-14 10:51           ` Vineet Gupta
  -1 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14 10:51 UTC (permalink / raw)
  To: Joao Pinto, Arnd Bergmann
  Cc: Joao Pinto, helgaas, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak

On Thursday 14 January 2016 04:12 PM, Joao Pinto wrote:
> Hi,
>
> On 1/14/2016 10:22 AM, Arnd Bergmann wrote:
>> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
>>>> +/*
>>>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>>>> + */
>>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>>> +                             resource_size_t size, resource_size_t align)
>>>> +{
>>>> +     return res->start;
>>>> +}
>>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
>>> (setup-res.c) can build as module ?
>> I only see a caller in drivers/pci/setup-res.c, and that is never part of a
>> loadable module.
>>
>>>> +
>>>> +void pcibios_fixup_bus(struct pci_bus *bus)
>>>> +{
>>>> +}
>>>> +EXPORT_SYMBOL(pcibios_fixup_bus);
>>> EXPORT_SYMBOL_GPL ?
>>>
>>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
>>> in common code. That would make basic PCI support almost arch independent !
>> I agree, that would be ideal. An easy way to do this would be to add
>> them as __weak functions in drivers/pci/, similar to how we handle
>> a lot of the other pcibios_* functions.
>>
>> A somewhat nicer method would be to have callback pointers in struct
>> pci_host_bridge, and call those when they are non-NULL so we can
>> remove the global pcibios_* functions from the API. That would also
>> bring us a big step closer to having PCI support itself as a loadable
>> module, and it would better reflect that those functions are really
>> host bridge specific rather than architecture specific. When you use
>> the same host bridge on multiple architectures, you typically have
>> the same requirements for hacks in there, but each architectures may
>> need to support multiple host bridges with different requirements.
> Since we will be constantly improving the driver and the core itself, I suggest
> that this functions be made __weak and in an update we can turn it struct
> pointers just like Arnd suggested. Is this good for you?

There is no point in making it weak, w/o a fallback version in generic code. For
this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.

And then as a follow up to make them weak (and hence eliminate the scattered
definitions all over). And then add as callbacks as suggested by Arnd.

Thx,
-Vineet

>
>> 	Arnd
>>
> Thanks
> Joao
>
>
>

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 10:51           ` Vineet Gupta
  0 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14 10:51 UTC (permalink / raw)
  To: Joao Pinto, Arnd Bergmann
  Cc: Joao Pinto, helgaas, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak

On Thursday 14 January 2016 04:12 PM, Joao Pinto wrote:
> Hi,
>
> On 1/14/2016 10:22 AM, Arnd Bergmann wrote:
>> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
>>>> +/*
>>>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>>>> + */
>>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>>> +                             resource_size_t size, resource_size_t align)
>>>> +{
>>>> +     return res->start;
>>>> +}
>>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
>>> (setup-res.c) can build as module ?
>> I only see a caller in drivers/pci/setup-res.c, and that is never part of a
>> loadable module.
>>
>>>> +
>>>> +void pcibios_fixup_bus(struct pci_bus *bus)
>>>> +{
>>>> +}
>>>> +EXPORT_SYMBOL(pcibios_fixup_bus);
>>> EXPORT_SYMBOL_GPL ?
>>>
>>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
>>> in common code. That would make basic PCI support almost arch independent !
>> I agree, that would be ideal. An easy way to do this would be to add
>> them as __weak functions in drivers/pci/, similar to how we handle
>> a lot of the other pcibios_* functions.
>>
>> A somewhat nicer method would be to have callback pointers in struct
>> pci_host_bridge, and call those when they are non-NULL so we can
>> remove the global pcibios_* functions from the API. That would also
>> bring us a big step closer to having PCI support itself as a loadable
>> module, and it would better reflect that those functions are really
>> host bridge specific rather than architecture specific. When you use
>> the same host bridge on multiple architectures, you typically have
>> the same requirements for hacks in there, but each architectures may
>> need to support multiple host bridges with different requirements.
> Since we will be constantly improving the driver and the core itself, I suggest
> that this functions be made __weak and in an update we can turn it struct
> pointers just like Arnd suggested. Is this good for you?

There is no point in making it weak, w/o a fallback version in generic code. For
this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.

And then as a follow up to make them weak (and hence eliminate the scattered
definitions all over). And then add as callbacks as suggested by Arnd.

Thx,
-Vineet

>
>> 	Arnd
>>
> Thanks
> Joao
>
>
>


^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 10:51           ` Vineet Gupta
  0 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14 10:51 UTC (permalink / raw)
  To: linux-snps-arc

On Thursday 14 January 2016 04:12 PM, Joao Pinto wrote:
> Hi,
>
> On 1/14/2016 10:22 AM, Arnd Bergmann wrote:
>> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
>>>> +/*
>>>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>>>> + */
>>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>>> +                             resource_size_t size, resource_size_t align)
>>>> +{
>>>> +     return res->start;
>>>> +}
>>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
>>> (setup-res.c) can build as module ?
>> I only see a caller in drivers/pci/setup-res.c, and that is never part of a
>> loadable module.
>>
>>>> +
>>>> +void pcibios_fixup_bus(struct pci_bus *bus)
>>>> +{
>>>> +}
>>>> +EXPORT_SYMBOL(pcibios_fixup_bus);
>>> EXPORT_SYMBOL_GPL ?
>>>
>>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
>>> in common code. That would make basic PCI support almost arch independent !
>> I agree, that would be ideal. An easy way to do this would be to add
>> them as __weak functions in drivers/pci/, similar to how we handle
>> a lot of the other pcibios_* functions.
>>
>> A somewhat nicer method would be to have callback pointers in struct
>> pci_host_bridge, and call those when they are non-NULL so we can
>> remove the global pcibios_* functions from the API. That would also
>> bring us a big step closer to having PCI support itself as a loadable
>> module, and it would better reflect that those functions are really
>> host bridge specific rather than architecture specific. When you use
>> the same host bridge on multiple architectures, you typically have
>> the same requirements for hacks in there, but each architectures may
>> need to support multiple host bridges with different requirements.
> Since we will be constantly improving the driver and the core itself, I suggest
> that this functions be made __weak and in an update we can turn it struct
> pointers just like Arnd suggested. Is this good for you?

There is no point in making it weak, w/o a fallback version in generic code. For
this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.

And then as a follow up to make them weak (and hence eliminate the scattered
definitions all over). And then add as callbacks as suggested by Arnd.

Thx,
-Vineet

>
>> 	Arnd
>>
> Thanks
> Joao
>
>
>

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
  2016-01-14 10:51           ` Vineet Gupta
  (?)
@ 2016-01-14 10:54             ` Joao Pinto
  -1 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-14 10:54 UTC (permalink / raw)
  To: Vineet Gupta, Arnd Bergmann
  Cc: Joao Pinto, helgaas, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak


Hi,

On 1/14/2016 10:51 AM, Vineet Gupta wrote:
> On Thursday 14 January 2016 04:12 PM, Joao Pinto wrote:
>> Hi,
>>
>> On 1/14/2016 10:22 AM, Arnd Bergmann wrote:
>>> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
>>>>> +/*
>>>>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>>>>> + */
>>>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>>>> +                             resource_size_t size, resource_size_t align)
>>>>> +{
>>>>> +     return res->start;
>>>>> +}
>>>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
>>>> (setup-res.c) can build as module ?
>>> I only see a caller in drivers/pci/setup-res.c, and that is never part of a
>>> loadable module.
>>>
>>>>> +
>>>>> +void pcibios_fixup_bus(struct pci_bus *bus)
>>>>> +{
>>>>> +}
>>>>> +EXPORT_SYMBOL(pcibios_fixup_bus);
>>>> EXPORT_SYMBOL_GPL ?
>>>>
>>>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
>>>> in common code. That would make basic PCI support almost arch independent !
>>> I agree, that would be ideal. An easy way to do this would be to add
>>> them as __weak functions in drivers/pci/, similar to how we handle
>>> a lot of the other pcibios_* functions.
>>>
>>> A somewhat nicer method would be to have callback pointers in struct
>>> pci_host_bridge, and call those when they are non-NULL so we can
>>> remove the global pcibios_* functions from the API. That would also
>>> bring us a big step closer to having PCI support itself as a loadable
>>> module, and it would better reflect that those functions are really
>>> host bridge specific rather than architecture specific. When you use
>>> the same host bridge on multiple architectures, you typically have
>>> the same requirements for hacks in there, but each architectures may
>>> need to support multiple host bridges with different requirements.
>> Since we will be constantly improving the driver and the core itself, I suggest
>> that this functions be made __weak and in an update we can turn it struct
>> pointers just like Arnd suggested. Is this good for you?
> 
> There is no point in making it weak, w/o a fallback version in generic code. For
> this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.
> 
> And then as a follow up to make them weak (and hence eliminate the scattered
> definitions all over). And then add as callbacks as suggested by Arnd.
> 

Ok, I'll removed the EXPORT_SYMBOL and submit a new patch version.
Thanks for your comments!

> Thx,
> -Vineet
> 
>>
>>> 	Arnd
>>>
>> Thanks
>> Joao
>>
>>
>>
> 
> 

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 10:54             ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-14 10:54 UTC (permalink / raw)
  To: Vineet Gupta, Arnd Bergmann
  Cc: Joao Pinto, helgaas, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak


Hi,

On 1/14/2016 10:51 AM, Vineet Gupta wrote:
> On Thursday 14 January 2016 04:12 PM, Joao Pinto wrote:
>> Hi,
>>
>> On 1/14/2016 10:22 AM, Arnd Bergmann wrote:
>>> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
>>>>> +/*
>>>>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>>>>> + */
>>>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>>>> +                             resource_size_t size, resource_size_t align)
>>>>> +{
>>>>> +     return res->start;
>>>>> +}
>>>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
>>>> (setup-res.c) can build as module ?
>>> I only see a caller in drivers/pci/setup-res.c, and that is never part of a
>>> loadable module.
>>>
>>>>> +
>>>>> +void pcibios_fixup_bus(struct pci_bus *bus)
>>>>> +{
>>>>> +}
>>>>> +EXPORT_SYMBOL(pcibios_fixup_bus);
>>>> EXPORT_SYMBOL_GPL ?
>>>>
>>>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
>>>> in common code. That would make basic PCI support almost arch independent !
>>> I agree, that would be ideal. An easy way to do this would be to add
>>> them as __weak functions in drivers/pci/, similar to how we handle
>>> a lot of the other pcibios_* functions.
>>>
>>> A somewhat nicer method would be to have callback pointers in struct
>>> pci_host_bridge, and call those when they are non-NULL so we can
>>> remove the global pcibios_* functions from the API. That would also
>>> bring us a big step closer to having PCI support itself as a loadable
>>> module, and it would better reflect that those functions are really
>>> host bridge specific rather than architecture specific. When you use
>>> the same host bridge on multiple architectures, you typically have
>>> the same requirements for hacks in there, but each architectures may
>>> need to support multiple host bridges with different requirements.
>> Since we will be constantly improving the driver and the core itself, I suggest
>> that this functions be made __weak and in an update we can turn it struct
>> pointers just like Arnd suggested. Is this good for you?
> 
> There is no point in making it weak, w/o a fallback version in generic code. For
> this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.
> 
> And then as a follow up to make them weak (and hence eliminate the scattered
> definitions all over). And then add as callbacks as suggested by Arnd.
> 

Ok, I'll removed the EXPORT_SYMBOL and submit a new patch version.
Thanks for your comments!

> Thx,
> -Vineet
> 
>>
>>> 	Arnd
>>>
>> Thanks
>> Joao
>>
>>
>>
> 
> 

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 10:54             ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-14 10:54 UTC (permalink / raw)
  To: linux-snps-arc


Hi,

On 1/14/2016 10:51 AM, Vineet Gupta wrote:
> On Thursday 14 January 2016 04:12 PM, Joao Pinto wrote:
>> Hi,
>>
>> On 1/14/2016 10:22 AM, Arnd Bergmann wrote:
>>> On Thursday 14 January 2016 05:26:58 Vineet Gupta wrote:
>>>>> +/*
>>>>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>>>>> + */
>>>>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>>>> +                             resource_size_t size, resource_size_t align)
>>>>> +{
>>>>> +     return res->start;
>>>>> +}
>>>> Doesn't this have to be EXPORT_SYMBOL_xxx as well given that the call
>>>> (setup-res.c) can build as module ?
>>> I only see a caller in drivers/pci/setup-res.c, and that is never part of a
>>> loadable module.
>>>
>>>>> +
>>>>> +void pcibios_fixup_bus(struct pci_bus *bus)
>>>>> +{
>>>>> +}
>>>>> +EXPORT_SYMBOL(pcibios_fixup_bus);
>>>> EXPORT_SYMBOL_GPL ?
>>>>
>>>> As a seperate enhancement, it would be nicer if these 2 functions are defined weak
>>>> in common code. That would make basic PCI support almost arch independent !
>>> I agree, that would be ideal. An easy way to do this would be to add
>>> them as __weak functions in drivers/pci/, similar to how we handle
>>> a lot of the other pcibios_* functions.
>>>
>>> A somewhat nicer method would be to have callback pointers in struct
>>> pci_host_bridge, and call those when they are non-NULL so we can
>>> remove the global pcibios_* functions from the API. That would also
>>> bring us a big step closer to having PCI support itself as a loadable
>>> module, and it would better reflect that those functions are really
>>> host bridge specific rather than architecture specific. When you use
>>> the same host bridge on multiple architectures, you typically have
>>> the same requirements for hacks in there, but each architectures may
>>> need to support multiple host bridges with different requirements.
>> Since we will be constantly improving the driver and the core itself, I suggest
>> that this functions be made __weak and in an update we can turn it struct
>> pointers just like Arnd suggested. Is this good for you?
> 
> There is no point in making it weak, w/o a fallback version in generic code. For
> this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.
> 
> And then as a follow up to make them weak (and hence eliminate the scattered
> definitions all over). And then add as callbacks as suggested by Arnd.
> 

Ok, I'll removed the EXPORT_SYMBOL and submit a new patch version.
Thanks for your comments!

> Thx,
> -Vineet
> 
>>
>>> 	Arnd
>>>
>> Thanks
>> Joao
>>
>>
>>
> 
> 

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v6 0/2] adding PCI support to AXS10x
@ 2016-01-12 15:45 ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-14 11:04 UTC (permalink / raw)
  To: Vineet.Gupta1, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, arnd, Joao Pinto

This patch set has the goal to add suppport for DesignWare PCIe RC in ARC
AXS10x. It includes the necessary tweaks to the ARC architecture, 
necessary tweaks to the PCI subsystem and a new driver (pcie-snpsdev).
This new driver will be used extensively in the PCIe RC Prototyping Kit.

The patches were produced against Bjorn Helgaas' repository. It was properly
tested in an IP Prototyping Kit.

Joao Pinto (2):
  PCI support added to ARC
  add new platform driver for PCI RC

 .../devicetree/bindings/pci/pcie-snpsdev.txt       |  33 +++
 MAINTAINERS                                        |   7 +
 arch/arc/Kconfig                                   |  23 ++
 arch/arc/include/asm/dma.h                         |   5 +
 arch/arc/include/asm/io.h                          |   9 +
 arch/arc/include/asm/pci.h                         |  31 +++
 arch/arc/kernel/Makefile                           |   1 +
 arch/arc/kernel/pcibios.c                          |  23 ++
 arch/arc/plat-axs10x/Kconfig                       |   1 +
 drivers/pci/Makefile                               |   1 +
 drivers/pci/host/Kconfig                           |   8 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-designware.c                 |   9 +
 drivers/pci/host/pcie-designware.h                 |   1 +
 drivers/pci/host/pcie-snpsdev.c                    | 286 +++++++++++++++++++++
 15 files changed, 440 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
 create mode 100644 arch/arc/include/asm/pci.h
 create mode 100644 arch/arc/kernel/pcibios.c
 create mode 100644 drivers/pci/host/pcie-snpsdev.c

-- 
1.8.1.5

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-12 15:45   ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-14 11:04 UTC (permalink / raw)
  To: Vineet.Gupta1, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, arnd, Joao Pinto

This patch adds PCI support to ARC and updates drivers/pci Makefile
enabling the ARC arch to use the generic PCI setup functions.

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Change v5 -> v6 (Vineet Gupta):
- Removed EXPORT_SYMBOL(pcibios_fixup_bus) from pcibios.c
Change v4 -> v5 (Vineet Gupta):
- pcibios.c unused functions, variables and includes were removed
- ioremap.c has no need for changes (no patch is necessary)
- pci.h unused functions and variables were removed
Change v3 -> v4:
- Nothing changed (just to keep up with patch set version).
Change v2 -> v3 (Bjorn Helgaas):
- arch/arc/kernel/pcibios.c unused functions were removed and also the
arch/arc/include/asm/mach/pci.h was removed because was no longer necessary
Change v1 -> v2:
- In arch/arc/Kconfig, the new menu entry (Bus Configuration) was moved to the
slot between sourcing of drivers/Kconfig and fs/Kconfig (Vineet Gupta)
- In arch/arc/plat-axs10x/Kconfig the "select MIGHT_HAVE_PCI" option was placed
in order as suggested (Vineet Gupta)
- ioport_map() and ioport_unmap() were static inlined and included in
the io.h (Vineet Gupta)
- pcibios_min_io and pcibios_min_mem declaration moved to
pcibios.c (Vineet Gupta)
- pr_err() replaced by dev_err() in pcibios_enable_device() (Bjorn Helgaas)
- string simplified in pcibios_enable_device() (Vineet Gupta)
- pci_host_bridge_window structure was replaced by resource_entry structure, and
list_for_each_entry() for resource_list_for_each_entry() in pcibios.c

 arch/arc/Kconfig             | 23 +++++++++++++++++++++++
 arch/arc/include/asm/dma.h   |  5 +++++
 arch/arc/include/asm/io.h    |  9 +++++++++
 arch/arc/include/asm/pci.h   | 31 +++++++++++++++++++++++++++++++
 arch/arc/kernel/Makefile     |  1 +
 arch/arc/kernel/pcibios.c    | 23 ++++++++++++++++++++++++
 arch/arc/plat-axs10x/Kconfig |  1 +
 drivers/pci/Makefile         |  1 +
 8 files changed, 95 insertions(+)
 create mode 100644 arch/arc/include/asm/pci.h
 create mode 100644 arch/arc/kernel/pcibios.c

diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
index 2c2ac3f..98b32c1 100644
--- a/arch/arc/Kconfig
+++ b/arch/arc/Kconfig
@@ -19,6 +19,7 @@ config ARC
 	select GENERIC_FIND_FIRST_BIT
 	# for now, we don't need GENERIC_IRQ_PROBE, CONFIG_GENERIC_IRQ_CHIP
 	select GENERIC_IRQ_SHOW
+	select GENERIC_PCI_IOMAP
 	select GENERIC_PENDING_IRQ if SMP
 	select GENERIC_SMP_IDLE_THREAD
 	select HAVE_ARCH_KGDB
@@ -39,6 +40,9 @@ config ARC
 	select PERF_USE_VMALLOC
 	select HAVE_DEBUG_STACKOVERFLOW
 
+config MIGHT_HAVE_PCI
+	bool
+
 config TRACE_IRQFLAGS_SUPPORT
 	def_bool y
 
@@ -570,6 +574,25 @@ endmenu	 # "ARC Architecture Configuration"
 source "mm/Kconfig"
 source "net/Kconfig"
 source "drivers/Kconfig"
+
+menu "Bus Support"
+
+config PCI
+	bool "PCI support" if MIGHT_HAVE_PCI
+	help
+	  PCI is the name of a bus system, i.e. the way the CPU talks to the other stuff inside
+	  your box.Find out if your board/platform have PCI.
+	  Note: PCIE support for Synopsys Device will be available only when
+	  HAPS DX is configured with PCIE RC bitmap. If you have PCI, say Y, otherwise N.
+
+config PCI_SYSCALL
+	def_bool PCI
+
+source "drivers/pci/Kconfig"
+source "drivers/pci/pcie/Kconfig"
+
+endmenu
+
 source "fs/Kconfig"
 source "arch/arc/Kconfig.debug"
 source "security/Kconfig"
diff --git a/arch/arc/include/asm/dma.h b/arch/arc/include/asm/dma.h
index ca7c451..37942fa 100644
--- a/arch/arc/include/asm/dma.h
+++ b/arch/arc/include/asm/dma.h
@@ -10,5 +10,10 @@
 #define ASM_ARC_DMA_H
 
 #define MAX_DMA_ADDRESS 0xC0000000
+#ifdef CONFIG_PCI
+extern int isa_dma_bridge_buggy;
+#else
+#define isa_dma_bridge_buggy    (0)
+#endif
 
 #endif
diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
index 694ece8..947bf0c 100644
--- a/arch/arc/include/asm/io.h
+++ b/arch/arc/include/asm/io.h
@@ -16,6 +16,15 @@
 extern void __iomem *ioremap(unsigned long physaddr, unsigned long size);
 extern void __iomem *ioremap_prot(phys_addr_t offset, unsigned long size,
 				  unsigned long flags);
+static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
+{
+	return (void __iomem *)port;
+}
+
+static inline void ioport_unmap(void __iomem *addr)
+{
+}
+
 extern void iounmap(const void __iomem *addr);
 
 #define ioremap_nocache(phy, sz)	ioremap(phy, sz)
diff --git a/arch/arc/include/asm/pci.h b/arch/arc/include/asm/pci.h
new file mode 100644
index 0000000..2f091e1
--- /dev/null
+++ b/arch/arc/include/asm/pci.h
@@ -0,0 +1,31 @@
+/*
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _ASM_ARC_PCI_H
+#define _ASM_ARC_PCI_H
+
+#ifdef __KERNEL__
+#include <asm-generic/pci-dma-compat.h>
+#include <asm-generic/pci-bridge.h>
+
+#include <linux/ioport.h>
+
+#define PCIBIOS_MIN_IO 0x100
+#define PCIBIOS_MIN_MEM 0x100000
+
+#define pcibios_assign_all_busses()	1
+/*
+ * The PCI address space does equal the physical memory address space.
+ * The networking and block device layers use this boolean for bounce
+ * buffer decisions.
+ */
+#define PCI_DMA_BUS_IS_PHYS     (1)
+
+#endif /* __KERNEL__ */
+
+#endif /* _ASM_ARC_PCI_H */
diff --git a/arch/arc/kernel/Makefile b/arch/arc/kernel/Makefile
index e7f3625..1bc2036 100644
--- a/arch/arc/kernel/Makefile
+++ b/arch/arc/kernel/Makefile
@@ -12,6 +12,7 @@ obj-y	:= arcksyms.o setup.o irq.o time.o reset.o ptrace.o process.o devtree.o
 obj-y	+= signal.o traps.o sys.o troubleshoot.o stacktrace.o disasm.o clk.o
 obj-$(CONFIG_ISA_ARCOMPACT)		+= entry-compact.o intc-compact.o
 obj-$(CONFIG_ISA_ARCV2)			+= entry-arcv2.o intc-arcv2.o
+obj-$(CONFIG_PCI)  			+= pcibios.o
 
 obj-$(CONFIG_MODULES)			+= arcksyms.o module.o
 obj-$(CONFIG_SMP) 			+= smp.o
diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
new file mode 100644
index 0000000..12ea45a
--- /dev/null
+++ b/arch/arc/kernel/pcibios.c
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2014-2015 Synopsys, Inc. (www.synopsys.com)
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#include <linux/pci.h>
+
+/*
+ * We don't have to worry about legacy ISA devices, so nothing to do here
+ */
+resource_size_t pcibios_align_resource(void *data, const struct resource *res,
+				resource_size_t size, resource_size_t align)
+{
+	return res->start;
+}
+
+void pcibios_fixup_bus(struct pci_bus *bus)
+{
+}
diff --git a/arch/arc/plat-axs10x/Kconfig b/arch/arc/plat-axs10x/Kconfig
index d475f9d..426ac4b 100644
--- a/arch/arc/plat-axs10x/Kconfig
+++ b/arch/arc/plat-axs10x/Kconfig
@@ -11,6 +11,7 @@ menuconfig ARC_PLAT_AXS10X
 	select DW_APB_ICTL
 	select GPIO_DWAPB
 	select OF_GPIO
+	select MIGHT_HAVE_PCI
 	select GENERIC_IRQ_CHIP
 	select ARCH_REQUIRE_GPIOLIB
 	help
diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
index be3f631..2154092 100644
--- a/drivers/pci/Makefile
+++ b/drivers/pci/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
 # Some architectures use the generic PCI setup functions
 #
 obj-$(CONFIG_ALPHA) += setup-irq.o
+obj-$(CONFIG_ARC) += setup-irq.o
 obj-$(CONFIG_ARM) += setup-irq.o
 obj-$(CONFIG_ARM64) += setup-irq.o
 obj-$(CONFIG_UNICORE32) += setup-irq.o
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* [PATCH v6 2/2] add new platform driver for PCI RC
@ 2016-01-12 15:45   ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-14 11:04 UTC (permalink / raw)
  To: Vineet.Gupta1, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, arnd, Joao Pinto

This patch adds a new driver that will be the reference platform driver
for all PCI RC IP Protoyping Kits based on ARC SDP.
This patch is composed by:

-Changes to pcie-designware driver add a function that enables the
feature of starting the LTSSM (Link Train Status State) used by the
new driver
-MAINTAINERS file was updated to include the new driver
-Documentation/devicetree/bindings/pci was updated to include the new
driver documentation
-New driver called pcie-snpsdev

Signed-off-by: Joao Pinto <jpinto@synopsys.com>
---
Change v5 -> v6:
- Nothing changed (just to keep up with patch set version).
Change v4 -> v5:
- Nothing changed (just to keep up with patch set version).
Changes v3 -> v4 (Bjorn Helgaas):
- ARCH dependencies were added to the drivers/pci/host/kconfig for the
PCIE_SNPSDEV.
Changes v2 -> v3 (Bjorn Helgaas):
- link init stuff was moved to a snpsdev_pcie_establish_link() function in
pcie-snpsdev
- pcie-snpsdev driver declaration was changed to be more 
standard (Bjorn Helgaas)
- pcie-designware' dw_pcie_link_retrain() now use standard registers from
pci-regs.h (Bjorn Helgaas)
- pcie-snpsdev.txt was complemented with more info (Mark Rutland)
Changes v1 -> v2 (Bjorn Helgaas):
- Fixups snpsdev_pcie_fixup_bridge() and snpsdev_pcie_fixup_res() were removed
from the driver (these functions were for specific tests only and not usefull
in mainline)
- Driver' comments were reviewed (fix Typos and excessive comments removal)
- Removed unnecessary definitions in the driver source (PCIE_PHY_CTRL and
PCIE_PHY_STAT)

 .../devicetree/bindings/pci/pcie-snpsdev.txt       |  33 +++
 MAINTAINERS                                        |   7 +
 drivers/pci/host/Kconfig                           |   8 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pcie-designware.c                 |   9 +
 drivers/pci/host/pcie-designware.h                 |   1 +
 drivers/pci/host/pcie-snpsdev.c                    | 286 +++++++++++++++++++++
 7 files changed, 345 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
 create mode 100644 drivers/pci/host/pcie-snpsdev.c

diff --git a/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
new file mode 100644
index 0000000..cae548b
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
@@ -0,0 +1,33 @@
+Synopsys PCI RC IP Prototyping Kit
+----------------------------------
+
+This is the reference platform driver to be used in the Synopsys PCI Root
+Complex IP Prototyping Kit.
+
+Required properties:
+- compatible: set to "snps,pcie-snpsdev";
+- reg: base address and length of the pcie controller registers.
+- #address-cells: set to <3>
+- #size-cells: set to <2>
+- device_type: set to "pci"
+- ranges: ranges for the PCI memory and I/O regions.
+- interrupts: one interrupt source for MSI interrupts, followed by interrupt
+	source for hardware related interrupts.
+- #interrupt-cells: set to <1>
+- num-lanes: set to <1>;
+
+Example configuration:
+
+	pcie: pcie@0xdffff000 {
+		compatible = "snps,pcie-snpsdev";
+		reg = <0xdffff000 0x1000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x00000800 0 0xd0000000 0xd0000000 0 0x00002000>,
+			 <0x81000000 0 0x00000000 0xde000000 0 0x00010000>,
+			 <0x82000000 0 0xd0400000 0xd0400000 0 0x0d000000>;
+		interrupts = <25>, <24>;
+		#interrupt-cells = <1>;
+		num-lanes = <1>;
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index e9caa4b..d2e4506 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8230,6 +8230,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
 F:	drivers/pci/host/pcie-hisi.c
 
+PCI DRIVER FOR SYNOPSYS PROTOTYPING DEVICE
+M:	Joao Pinto <jpinto@synopsys.com>
+L:	linux-pci@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
+F:	drivers/pci/host/pcie-snpsdev.c
+
 PCMCIA SUBSYSTEM
 P:	Linux PCMCIA Team
 L:	linux-pcmcia@lists.infradead.org
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index f131ba9..589bc15 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -172,4 +172,12 @@ config PCI_HISI
 	help
 	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
 
+config PCIE_SNPSDEV
+	bool "Platform Driver for Synopsys Device"
+	depends on ARC && OF
+	select PCIEPORTBUS
+	select PCIE_DW
+	help
+	  Say Y here if you want to enable PCIe controller support on the
+	  Synopsys IP Prototyping Kits.
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 9d4d3c6..e422f65 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -20,3 +20,4 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
 obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
 obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
 obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
+obj-$(CONFIG_PCIE_SNPSDEV) += pcie-snpsdev.o
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 540f077..f73a3cf 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -706,6 +706,15 @@ static struct pci_ops dw_pcie_ops = {
 	.write = dw_pcie_wr_conf,
 };
 
+void dw_pcie_link_retrain(struct pcie_port *pp)
+{
+	u32 val = 0;
+
+	dw_pcie_readl_rc(pp, PCI_EXP_LNKCTL+0x70, &val);
+	val = val | PCI_EXP_LNKCTL_RL;
+	dw_pcie_writel_rc(pp, val, PCI_EXP_LNKCTL+0x70);
+}
+
 void dw_pcie_setup_rc(struct pcie_port *pp)
 {
 	u32 val;
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 2356d29..249b631 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -79,5 +79,6 @@ void dw_pcie_msi_init(struct pcie_port *pp);
 int dw_pcie_link_up(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
 int dw_pcie_host_init(struct pcie_port *pp);
+void dw_pcie_link_retrain(struct pcie_port *pp);
 
 #endif /* _PCIE_DESIGNWARE_H */
diff --git a/drivers/pci/host/pcie-snpsdev.c b/drivers/pci/host/pcie-snpsdev.c
new file mode 100644
index 0000000..4ca7ec5
--- /dev/null
+++ b/drivers/pci/host/pcie-snpsdev.c
@@ -0,0 +1,286 @@
+/*
+ * PCIe RC driver for Synopsys Designware Core
+ *
+ * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
+ *
+ * Authors: Manjunath Bettegowda <manjumb@synopsys.com>,
+ *	    Jie Deng <jiedeng@synopsys.com>
+ *	    Joao Pinto <jpinto@synopsys.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/gpio.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/resource.h>
+#include <linux/signal.h>
+#include <linux/types.h>
+
+#include "pcie-designware.h"
+
+#define to_snpsdev_pcie(x)	container_of(x, struct snpsdev_pcie, pp)
+
+struct snpsdev_pcie {
+	void __iomem		*mem_base; /* Memory Base to access Core's [RC]
+					    * Config Space Layout
+					    */
+	struct pcie_port	pp;        /* RC Root Port specific structure -
+					    * DWC_PCIE_RC stuff
+					    */
+};
+
+#define PCI_EQUAL_CONTROL_PHY 0x00000707
+
+/* PCIe Port Logic registers (memory-mapped) */
+#define PLR_OFFSET 0x700
+#define PCIE_PHY_DEBUG_R0 (PLR_OFFSET + 0x28) /* 0x728 */
+#define PCIE_PHY_DEBUG_R1 (PLR_OFFSET + 0x2c) /* 0x72c */
+
+/* This handler was created for future work */
+static irqreturn_t snpsdev_pcie_irq_handler(int irq, void *arg)
+{
+	return IRQ_NONE;
+}
+
+static irqreturn_t snpsdev_pcie_msi_irq_handler(int irq, void *arg)
+{
+	struct pcie_port *pp = arg;
+
+	dw_handle_msi_irq(pp);
+
+	return IRQ_HANDLED;
+}
+
+static void snpsdev_pcie_init_phy(struct pcie_port *pp)
+{
+	/* write Lane 0 Equalization Control fields register */
+	writel(PCI_EQUAL_CONTROL_PHY, pp->dbi_base + 0x154);
+}
+
+static int snpsdev_pcie_deassert_core_reset(struct pcie_port *pp)
+{
+	return 0;
+}
+
+static void snpsdev_pcie_establish_link(struct pcie_port *pp)
+{
+	int count = 0;
+
+	/* Initialize Phy (Reset/poweron/control-inputs ) */
+	snpsdev_pcie_init_phy(pp);
+
+	/* de-assert core reset */
+	snpsdev_pcie_deassert_core_reset(pp);
+
+	/* We expect the PCIe Link to be up by this time */
+	dw_pcie_setup_rc(pp);
+
+	/* Start LTSSM here */
+	dw_pcie_link_retrain(pp);
+
+	while (!dw_pcie_link_up(pp)) {
+		usleep_range(1000, 1100);
+		count++;
+		if (count == 20) {
+			dev_err(pp->dev, "phy link never came up\n");
+			dev_dbg(pp->dev,
+				"PL_DEBUG0: 0x%08x, DEBUG_R1: 0x%08x\n",
+				readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
+				readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
+			break;
+		}
+	}
+}
+
+/*
+ * snpsdev_pcie_host_init()
+ * Platform specific host/RC initialization
+ *	a. Assert the core reset
+ *	b. Assert and deassert phy reset and initialize the phy
+ *	c. De-Assert the core reset
+ *	d. Initializet the Root Port (BARs/Memory Or IO/ Interrupt/ Commnad Reg)
+ *	e. Initiate Link startup procedure
+ *
+ */
+static void snpsdev_pcie_host_init(struct pcie_port *pp)
+{
+	/* Establish link */
+	snpsdev_pcie_establish_link(pp);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		dw_pcie_msi_init(pp);
+}
+
+static int snpsdev_pcie_link_up(struct pcie_port *pp)
+{
+	u32 status;
+
+	/* Bit number 36: reports LTSSM PHY Link UP; Available in bit 3 of
+	 * PCIE_PHY_DEBUG_R1
+	 */
+	status = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1) & (0x1 << 4);
+	if (status != 0)
+		return 1;
+
+	/* TODO:Now Link is in L0;Initiate GEN2/GEN3 migration if RC Supports*/
+	return 0;
+}
+
+/**
+ * This is RC operation structure
+ * snpsdev_pcie_link_up: the function which initiates the phy link up procedure
+ * snpsdev_pcie_host_init: the function which does the host/RC Root port
+ * initialization.
+ */
+static struct pcie_host_ops snpsdev_pcie_host_ops = {
+	.link_up = snpsdev_pcie_link_up,
+	.host_init = snpsdev_pcie_host_init,
+};
+
+/**
+ * snpsdev_add_pcie_port
+ * This function
+ * a. installs the interrupt handler
+ * b. registers host operations in the pcie_port structure
+ */
+static int snpsdev_add_pcie_port(struct pcie_port *pp,
+				 struct platform_device *pdev)
+{
+	int ret;
+
+	pp->irq = platform_get_irq(pdev, 1);
+
+	if (pp->irq < 0) {
+		if (pp->irq != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "cannot get irq\n");
+		return pp->irq;
+	}
+
+	ret = devm_request_irq(&pdev->dev, pp->irq, snpsdev_pcie_irq_handler,
+				IRQF_SHARED, "snpsdev-pcie", pp);
+
+	if (ret) {
+		dev_err(&pdev->dev, "failed to request irq\n");
+		return ret;
+	}
+
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		pp->msi_irq = platform_get_irq(pdev, 0);
+
+		if (pp->msi_irq < 0) {
+			if (pp->msi_irq != -EPROBE_DEFER)
+				dev_err(&pdev->dev, "cannot get msi irq\n");
+			return pp->msi_irq;
+		}
+
+		ret = devm_request_irq(&pdev->dev, pp->msi_irq,
+					snpsdev_pcie_msi_irq_handler,
+					IRQF_SHARED, "snpsdev-pcie-msi", pp);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to request msi irq\n");
+			return ret;
+		}
+	}
+
+	pp->root_bus_nr = -1;
+	pp->ops = &snpsdev_pcie_host_ops;
+
+	/* Below function:
+	 * Checks for range property from DT
+	 * Gets the IO and MEMORY and CONFIG-Space ranges from DT
+	 * Does IOREMAPS on the physical addresses
+	 * Gets the num-lanes from DT
+	 * Gets MSI capability from DT
+	 * Calls the platform specific host initialization
+	 * Program the correct class, BAR0, Link width, in Config space
+	 * Then it calls pci common init routine
+	 * Then it calls function to assign "unassigned resources"
+	 */
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize host\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+/**
+ * snpsdev_pcie_rc_probe()
+ * This function gets called as part of pcie registration. if the id matches
+ * the platform driver framework will call this function.
+ *
+ * @pdev: Pointer to the platform_device structure
+ *
+ * Returns zero on success; Negative errorno on failure
+ */
+static int snpsdev_pcie_rc_probe(struct platform_device *pdev)
+{
+	struct snpsdev_pcie *snpsdev_pcie;
+	struct pcie_port *pp;
+	struct resource *dwc_pcie_rc_res;  /* Resource from DT */
+	int ret;
+
+	snpsdev_pcie = devm_kzalloc(&pdev->dev, sizeof(*snpsdev_pcie),
+					GFP_KERNEL);
+	if (!snpsdev_pcie)
+		return -ENOMEM;
+
+	pp = &snpsdev_pcie->pp;
+	pp->dev = &pdev->dev;
+
+	dwc_pcie_rc_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+
+	if (!dwc_pcie_rc_res)
+		return -ENODEV;
+
+	snpsdev_pcie->mem_base = devm_ioremap_resource(&pdev->dev,
+							dwc_pcie_rc_res);
+	if (IS_ERR(snpsdev_pcie->mem_base)) {
+		ret = PTR_ERR(snpsdev_pcie->mem_base);
+		return ret;
+	}
+	pp->dbi_base = snpsdev_pcie->mem_base;
+
+	ret = snpsdev_add_pcie_port(pp, pdev);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, snpsdev_pcie);
+
+	return 0;
+}
+
+static int snpsdev_pcie_rc_remove(struct platform_device *pdev)
+{
+	return 0;
+}
+
+static const struct of_device_id snpsdev_pcie_rc_of_match[] = {
+	{ .compatible = "snps,pcie-snpsdev", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, snpsdev_pcie_rc_of_match);
+
+static struct platform_driver snpsdev_pcie_rc_driver = {
+	.driver = {
+		.name	= "pcie-snpsdev",
+		.of_match_table = snpsdev_pcie_rc_of_match,
+	},
+	.probe = snpsdev_pcie_rc_probe,
+};
+
+module_platform_driver(snpsdev_pcie_rc_driver);
+
+MODULE_AUTHOR("Manjunath Bettegowda <manjumb@synopsys.com>");
+MODULE_DESCRIPTION("Platform Driver for Synopsys Device");
+MODULE_LICENSE("GPL v2");
-- 
1.8.1.5

^ permalink raw reply related	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
  2016-01-12 15:45   ` [PATCH v5 " Joao Pinto
  (?)
@ 2016-01-14 11:11     ` Vineet Gupta
  -1 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14 11:11 UTC (permalink / raw)
  To: Joao Pinto, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, arnd

On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:

This patch adds PCI support to ARC and updates drivers/pci Makefile
enabling the ARC arch to use the generic PCI setup functions.

Signed-off-by: Joao Pinto <jpinto@synopsys.com><mailto:jpinto@synopsys.com>

Acked-by: Vineet Gupta <vgupta@synopsys.com><mailto:vgupta@synopsys.com>

@Bjorn are you happy with the driver bits.
Although ARC support is independent of driver, and I can pick it up separately, it would be better if you took it along with the driver. I don't see potential for any conflicts.

Thx,
-Vineet

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-14 11:11     ` Vineet Gupta
  0 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14 11:11 UTC (permalink / raw)
  To: Joao Pinto, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, arnd

On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:

This patch adds PCI support to ARC and updates drivers/pci Makefile
enabling the ARC arch to use the generic PCI setup functions.

Signed-off-by: Joao Pinto <jpinto@synopsys.com><mailto:jpinto@synopsys.com>

Acked-by: Vineet Gupta <vgupta@synopsys.com><mailto:vgupta@synopsys.com>

@Bjorn are you happy with the driver bits.
Although ARC support is independent of driver, and I can pick it up separately, it would be better if you took it along with the driver. I don't see potential for any conflicts.

Thx,
-Vineet

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-14 11:11     ` Vineet Gupta
  0 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14 11:11 UTC (permalink / raw)
  To: linux-snps-arc

On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:

This patch adds PCI support to ARC and updates drivers/pci Makefile
enabling the ARC arch to use the generic PCI setup functions.

Signed-off-by: Joao Pinto <jpinto at synopsys.com><mailto:jpinto at synopsys.com>

Acked-by: Vineet Gupta <vgupta at synopsys.com><mailto:vgupta at synopsys.com>

@Bjorn are you happy with the driver bits.
Although ARC support is independent of driver, and I can pick it up separately, it would be better if you took it along with the driver. I don't see potential for any conflicts.

Thx,
-Vineet

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
  2016-01-14 10:51           ` Vineet Gupta
  (?)
@ 2016-01-14 11:59             ` Arnd Bergmann
  -1 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2016-01-14 11:59 UTC (permalink / raw)
  To: Vineet Gupta
  Cc: Joao Pinto, helgaas, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak

On Thursday 14 January 2016 10:51:32 Vineet Gupta wrote:
> >>
> >> A somewhat nicer method would be to have callback pointers in struct
> >> pci_host_bridge, and call those when they are non-NULL so we can
> >> remove the global pcibios_* functions from the API. That would also
> >> bring us a big step closer to having PCI support itself as a loadable
> >> module, and it would better reflect that those functions are really
> >> host bridge specific rather than architecture specific. When you use
> >> the same host bridge on multiple architectures, you typically have
> >> the same requirements for hacks in there, but each architectures may
> >> need to support multiple host bridges with different requirements.
> > Since we will be constantly improving the driver and the core itself, I suggest
> > that this functions be made __weak and in an update we can turn it struct
> > pointers just like Arnd suggested. Is this good for you?
> 
> There is no point in making it weak, w/o a fallback version in generic code. For
> this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.
> 

To clarify: I don't particularly like __weak functions anywhere, but they
are already common in drivers/pci, so we can add a couple more to reach
the goal of removing all architecture specific code.

However, there should never be a reason to add a __weak function in
arch code that gets overridden by common code, that would be very confusing
and not helpful.

	Arnd

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 11:59             ` Arnd Bergmann
  0 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2016-01-14 11:59 UTC (permalink / raw)
  To: Vineet Gupta
  Cc: Joao Pinto, helgaas, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak

On Thursday 14 January 2016 10:51:32 Vineet Gupta wrote:
> >>
> >> A somewhat nicer method would be to have callback pointers in struct
> >> pci_host_bridge, and call those when they are non-NULL so we can
> >> remove the global pcibios_* functions from the API. That would also
> >> bring us a big step closer to having PCI support itself as a loadable
> >> module, and it would better reflect that those functions are really
> >> host bridge specific rather than architecture specific. When you use
> >> the same host bridge on multiple architectures, you typically have
> >> the same requirements for hacks in there, but each architectures may
> >> need to support multiple host bridges with different requirements.
> > Since we will be constantly improving the driver and the core itself, I suggest
> > that this functions be made __weak and in an update we can turn it struct
> > pointers just like Arnd suggested. Is this good for you?
> 
> There is no point in making it weak, w/o a fallback version in generic code. For
> this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.
> 

To clarify: I don't particularly like __weak functions anywhere, but they
are already common in drivers/pci, so we can add a couple more to reach
the goal of removing all architecture specific code.

However, there should never be a reason to add a __weak function in
arch code that gets overridden by common code, that would be very confusing
and not helpful.

	Arnd

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 11:59             ` Arnd Bergmann
  0 siblings, 0 replies; 51+ messages in thread
From: Arnd Bergmann @ 2016-01-14 11:59 UTC (permalink / raw)
  To: linux-snps-arc

On Thursday 14 January 2016 10:51:32 Vineet Gupta wrote:
> >>
> >> A somewhat nicer method would be to have callback pointers in struct
> >> pci_host_bridge, and call those when they are non-NULL so we can
> >> remove the global pcibios_* functions from the API. That would also
> >> bring us a big step closer to having PCI support itself as a loadable
> >> module, and it would better reflect that those functions are really
> >> host bridge specific rather than architecture specific. When you use
> >> the same host bridge on multiple architectures, you typically have
> >> the same requirements for hacks in there, but each architectures may
> >> need to support multiple host bridges with different requirements.
> > Since we will be constantly improving the driver and the core itself, I suggest
> > that this functions be made __weak and in an update we can turn it struct
> > pointers just like Arnd suggested. Is this good for you?
> 
> There is no point in making it weak, w/o a fallback version in generic code. For
> this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.
> 

To clarify: I don't particularly like __weak functions anywhere, but they
are already common in drivers/pci, so we can add a couple more to reach
the goal of removing all architecture specific code.

However, there should never be a reason to add a __weak function in
arch code that gets overridden by common code, that would be very confusing
and not helpful.

	Arnd

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
  2016-01-14 11:59             ` Arnd Bergmann
  (?)
@ 2016-01-14 12:11               ` Vineet Gupta
  -1 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14 12:11 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: mark.rutland, Joao Pinto, pawel.moll, ijc+devicetree, linux-pci,
	Alexey.Brodkin, linux-kernel, CARLOS.PALMINHA, robh+dt, helgaas,
	galak, linux-snps-arc

On Thursday 14 January 2016 05:29 PM, Arnd Bergmann wrote:
> On Thursday 14 January 2016 10:51:32 Vineet Gupta wrote:
>>>>
>>>> A somewhat nicer method would be to have callback pointers in struct
>>>> pci_host_bridge, and call those when they are non-NULL so we can
>>>> remove the global pcibios_* functions from the API. That would also
>>>> bring us a big step closer to having PCI support itself as a loadable
>>>> module, and it would better reflect that those functions are really
>>>> host bridge specific rather than architecture specific. When you use
>>>> the same host bridge on multiple architectures, you typically have
>>>> the same requirements for hacks in there, but each architectures may
>>>> need to support multiple host bridges with different requirements.
>>> Since we will be constantly improving the driver and the core itself, I suggest
>>> that this functions be made __weak and in an update we can turn it struct
>>> pointers just like Arnd suggested. Is this good for you?
>>
>> There is no point in making it weak, w/o a fallback version in generic code. For
>> this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.
>>
> 
> To clarify: I don't particularly like __weak functions anywhere, but they
> are already common in drivers/pci, so we can add a couple more to reach
> the goal of removing all architecture specific code.

But I see quite a bit of "weak design pattern" in kernel is common when an API
needs to be implemented across arches and it same for most of them. I agree that
this is not as ideal from code flow analysis but saves a lot of duplication.


> However, there should never be a reason to add a __weak function in
> arch code that gets overridden by common code, that would be very confusing
> and not helpful.

Agree. That was never my suggestion anyways. I'd asked __weak version be defined
in common code so we could reuse it and elide the ARC version and at the same time
not breaking others, and in longer run removing the dups elsewhere.

Thx,
-Vineet

> 
> 	Arnd
> 

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 12:11               ` Vineet Gupta
  0 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14 12:11 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: mark.rutland, Joao Pinto, pawel.moll, ijc+devicetree, linux-pci,
	Alexey.Brodkin, linux-kernel, CARLOS.PALMINHA, robh+dt, helgaas,
	galak, linux-snps-arc

On Thursday 14 January 2016 05:29 PM, Arnd Bergmann wrote:
> On Thursday 14 January 2016 10:51:32 Vineet Gupta wrote:
>>>>
>>>> A somewhat nicer method would be to have callback pointers in struct
>>>> pci_host_bridge, and call those when they are non-NULL so we can
>>>> remove the global pcibios_* functions from the API. That would also
>>>> bring us a big step closer to having PCI support itself as a loadable
>>>> module, and it would better reflect that those functions are really
>>>> host bridge specific rather than architecture specific. When you use
>>>> the same host bridge on multiple architectures, you typically have
>>>> the same requirements for hacks in there, but each architectures may
>>>> need to support multiple host bridges with different requirements.
>>> Since we will be constantly improving the driver and the core itself, I suggest
>>> that this functions be made __weak and in an update we can turn it struct
>>> pointers just like Arnd suggested. Is this good for you?
>>
>> There is no point in making it weak, w/o a fallback version in generic code. For
>> this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.
>>
> 
> To clarify: I don't particularly like __weak functions anywhere, but they
> are already common in drivers/pci, so we can add a couple more to reach
> the goal of removing all architecture specific code.

But I see quite a bit of "weak design pattern" in kernel is common when an API
needs to be implemented across arches and it same for most of them. I agree that
this is not as ideal from code flow analysis but saves a lot of duplication.


> However, there should never be a reason to add a __weak function in
> arch code that gets overridden by common code, that would be very confusing
> and not helpful.

Agree. That was never my suggestion anyways. I'd asked __weak version be defined
in common code so we could reuse it and elide the ARC version and at the same time
not breaking others, and in longer run removing the dups elsewhere.

Thx,
-Vineet

> 
> 	Arnd
> 

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v5 1/2] PCI support added to ARC
@ 2016-01-14 12:11               ` Vineet Gupta
  0 siblings, 0 replies; 51+ messages in thread
From: Vineet Gupta @ 2016-01-14 12:11 UTC (permalink / raw)
  To: linux-snps-arc

On Thursday 14 January 2016 05:29 PM, Arnd Bergmann wrote:
> On Thursday 14 January 2016 10:51:32 Vineet Gupta wrote:
>>>>
>>>> A somewhat nicer method would be to have callback pointers in struct
>>>> pci_host_bridge, and call those when they are non-NULL so we can
>>>> remove the global pcibios_* functions from the API. That would also
>>>> bring us a big step closer to having PCI support itself as a loadable
>>>> module, and it would better reflect that those functions are really
>>>> host bridge specific rather than architecture specific. When you use
>>>> the same host bridge on multiple architectures, you typically have
>>>> the same requirements for hacks in there, but each architectures may
>>>> need to support multiple host bridges with different requirements.
>>> Since we will be constantly improving the driver and the core itself, I suggest
>>> that this functions be made __weak and in an update we can turn it struct
>>> pointers just like Arnd suggested. Is this good for you?
>>
>> There is no point in making it weak, w/o a fallback version in generic code. For
>> this series, I suggest you just remove the straggler EXPORT_SYMBOL and respin.
>>
> 
> To clarify: I don't particularly like __weak functions anywhere, but they
> are already common in drivers/pci, so we can add a couple more to reach
> the goal of removing all architecture specific code.

But I see quite a bit of "weak design pattern" in kernel is common when an API
needs to be implemented across arches and it same for most of them. I agree that
this is not as ideal from code flow analysis but saves a lot of duplication.


> However, there should never be a reason to add a __weak function in
> arch code that gets overridden by common code, that would be very confusing
> and not helpful.

Agree. That was never my suggestion anyways. I'd asked __weak version be defined
in common code so we could reuse it and elide the ARC version and at the same time
not breaking others, and in longer run removing the dups elsewhere.

Thx,
-Vineet

> 
> 	Arnd
> 

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
  2016-01-14 11:11     ` Vineet Gupta
  (?)
@ 2016-01-18 11:30       ` Joao Pinto
  -1 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-18 11:30 UTC (permalink / raw)
  To: helgaas
  Cc: Vineet Gupta, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, arnd

Hi Bjorn,

On 1/14/2016 11:11 AM, Vineet Gupta wrote:
> On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
>> This patch adds PCI support to ARC and updates drivers/pci Makefile
>> enabling the ARC arch to use the generic PCI setup functions.
>>
>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> 
> Acked-by: Vineet Gupta <vgupta@synopsys.com>
> 
> @Bjorn are you happy with the driver bits.
> Although ARC support is independent of driver, and I can pick it up separately,
> it would be better if you took it along with the driver. I don't see potential
> for any conflicts.
> 
> Thx,
> -Vineet

In your opinion the patch set is ready for merge or do you need me to make any
more tweaks?

Thanks a lot.

Joao

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-18 11:30       ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-18 11:30 UTC (permalink / raw)
  To: helgaas
  Cc: Vineet Gupta, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, arnd

Hi Bjorn,

On 1/14/2016 11:11 AM, Vineet Gupta wrote:
> On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
>> This patch adds PCI support to ARC and updates drivers/pci Makefile
>> enabling the ARC arch to use the generic PCI setup functions.
>>
>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> 
> Acked-by: Vineet Gupta <vgupta@synopsys.com>
> 
> @Bjorn are you happy with the driver bits.
> Although ARC support is independent of driver, and I can pick it up separately,
> it would be better if you took it along with the driver. I don't see potential
> for any conflicts.
> 
> Thx,
> -Vineet

In your opinion the patch set is ready for merge or do you need me to make any
more tweaks?

Thanks a lot.

Joao

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-18 11:30       ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-18 11:30 UTC (permalink / raw)
  To: linux-snps-arc

Hi Bjorn,

On 1/14/2016 11:11 AM, Vineet Gupta wrote:
> On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
>> This patch adds PCI support to ARC and updates drivers/pci Makefile
>> enabling the ARC arch to use the generic PCI setup functions.
>>
>> Signed-off-by: Joao Pinto <jpinto at synopsys.com>
> 
> Acked-by: Vineet Gupta <vgupta at synopsys.com>
> 
> @Bjorn are you happy with the driver bits.
> Although ARC support is independent of driver, and I can pick it up separately,
> it would be better if you took it along with the driver. I don't see potential
> for any conflicts.
> 
> Thx,
> -Vineet

In your opinion the patch set is ready for merge or do you need me to make any
more tweaks?

Thanks a lot.

Joao

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
  2016-01-14 11:11     ` Vineet Gupta
  (?)
@ 2016-01-27 14:29       ` Joao Pinto
  -1 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-27 14:29 UTC (permalink / raw)
  To: Vineet Gupta, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, arnd

Hi Bjorn,
Could you please tell me what are the perspectives of pulling this patch set to
mainline?

On 1/14/2016 11:11 AM, Vineet Gupta wrote:
> On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
>> This patch adds PCI support to ARC and updates drivers/pci Makefile
>> enabling the ARC arch to use the generic PCI setup functions.
>>
>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> 
> Acked-by: Vineet Gupta <vgupta@synopsys.com>
> 
> @Bjorn are you happy with the driver bits.
> Although ARC support is independent of driver, and I can pick it up separately,
> it would be better if you took it along with the driver. I don't see potential
> for any conflicts.
> 
> Thx,
> -Vineet

Thanks,
Joao

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-27 14:29       ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-27 14:29 UTC (permalink / raw)
  To: Vineet Gupta, helgaas
  Cc: linux-pci, linux-kernel, linux-snps-arc, CARLOS.PALMINHA,
	Alexey.Brodkin, robh+dt, pawel.moll, mark.rutland,
	ijc+devicetree, galak, arnd

Hi Bjorn,
Could you please tell me what are the perspectives of pulling this patch set to
mainline?

On 1/14/2016 11:11 AM, Vineet Gupta wrote:
> On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
>> This patch adds PCI support to ARC and updates drivers/pci Makefile
>> enabling the ARC arch to use the generic PCI setup functions.
>>
>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> 
> Acked-by: Vineet Gupta <vgupta@synopsys.com>
> 
> @Bjorn are you happy with the driver bits.
> Although ARC support is independent of driver, and I can pick it up separately,
> it would be better if you took it along with the driver. I don't see potential
> for any conflicts.
> 
> Thx,
> -Vineet

Thanks,
Joao

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-27 14:29       ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-27 14:29 UTC (permalink / raw)
  To: linux-snps-arc

Hi Bjorn,
Could you please tell me what are the perspectives of pulling this patch set to
mainline?

On 1/14/2016 11:11 AM, Vineet Gupta wrote:
> On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
>> This patch adds PCI support to ARC and updates drivers/pci Makefile
>> enabling the ARC arch to use the generic PCI setup functions.
>>
>> Signed-off-by: Joao Pinto <jpinto at synopsys.com>
> 
> Acked-by: Vineet Gupta <vgupta at synopsys.com>
> 
> @Bjorn are you happy with the driver bits.
> Although ARC support is independent of driver, and I can pick it up separately,
> it would be better if you took it along with the driver. I don't see potential
> for any conflicts.
> 
> Thx,
> -Vineet

Thanks,
Joao

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
  2016-01-27 14:29       ` Joao Pinto
  (?)
@ 2016-01-27 21:59         ` Bjorn Helgaas
  -1 siblings, 0 replies; 51+ messages in thread
From: Bjorn Helgaas @ 2016-01-27 21:59 UTC (permalink / raw)
  To: Joao Pinto
  Cc: Vineet Gupta, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, arnd

Hi Joao,

On Wed, Jan 27, 2016 at 02:29:21PM +0000, Joao Pinto wrote:
> Hi Bjorn,
> Could you please tell me what are the perspectives of pulling this patch set to
> mainline?

Sorry for the delay.  I try to handle bug fixes before I look at
functionality, so sometimes things get backed up a little bit.  But I
should be able to take a look later this week or early next week.

Bjorn

> On 1/14/2016 11:11 AM, Vineet Gupta wrote:
> > On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
> >> This patch adds PCI support to ARC and updates drivers/pci Makefile
> >> enabling the ARC arch to use the generic PCI setup functions.
> >>
> >> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> > 
> > Acked-by: Vineet Gupta <vgupta@synopsys.com>
> > 
> > @Bjorn are you happy with the driver bits.
> > Although ARC support is independent of driver, and I can pick it up separately,
> > it would be better if you took it along with the driver. I don't see potential
> > for any conflicts.
> > 
> > Thx,
> > -Vineet
> 
> Thanks,
> Joao
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-27 21:59         ` Bjorn Helgaas
  0 siblings, 0 replies; 51+ messages in thread
From: Bjorn Helgaas @ 2016-01-27 21:59 UTC (permalink / raw)
  To: Joao Pinto
  Cc: Vineet Gupta, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, arnd

Hi Joao,

On Wed, Jan 27, 2016 at 02:29:21PM +0000, Joao Pinto wrote:
> Hi Bjorn,
> Could you please tell me what are the perspectives of pulling this patch set to
> mainline?

Sorry for the delay.  I try to handle bug fixes before I look at
functionality, so sometimes things get backed up a little bit.  But I
should be able to take a look later this week or early next week.

Bjorn

> On 1/14/2016 11:11 AM, Vineet Gupta wrote:
> > On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
> >> This patch adds PCI support to ARC and updates drivers/pci Makefile
> >> enabling the ARC arch to use the generic PCI setup functions.
> >>
> >> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> > 
> > Acked-by: Vineet Gupta <vgupta@synopsys.com>
> > 
> > @Bjorn are you happy with the driver bits.
> > Although ARC support is independent of driver, and I can pick it up separately,
> > it would be better if you took it along with the driver. I don't see potential
> > for any conflicts.
> > 
> > Thx,
> > -Vineet
> 
> Thanks,
> Joao
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-27 21:59         ` Bjorn Helgaas
  0 siblings, 0 replies; 51+ messages in thread
From: Bjorn Helgaas @ 2016-01-27 21:59 UTC (permalink / raw)
  To: linux-snps-arc

Hi Joao,

On Wed, Jan 27, 2016@02:29:21PM +0000, Joao Pinto wrote:
> Hi Bjorn,
> Could you please tell me what are the perspectives of pulling this patch set to
> mainline?

Sorry for the delay.  I try to handle bug fixes before I look at
functionality, so sometimes things get backed up a little bit.  But I
should be able to take a look later this week or early next week.

Bjorn

> On 1/14/2016 11:11 AM, Vineet Gupta wrote:
> > On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
> >> This patch adds PCI support to ARC and updates drivers/pci Makefile
> >> enabling the ARC arch to use the generic PCI setup functions.
> >>
> >> Signed-off-by: Joao Pinto <jpinto at synopsys.com>
> > 
> > Acked-by: Vineet Gupta <vgupta at synopsys.com>
> > 
> > @Bjorn are you happy with the driver bits.
> > Although ARC support is independent of driver, and I can pick it up separately,
> > it would be better if you took it along with the driver. I don't see potential
> > for any conflicts.
> > 
> > Thx,
> > -Vineet
> 
> Thanks,
> Joao
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
  2016-01-27 21:59         ` Bjorn Helgaas
  (?)
@ 2016-01-28 17:00           ` Joao Pinto
  -1 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-28 17:00 UTC (permalink / raw)
  To: Bjorn Helgaas, Joao Pinto
  Cc: Vineet Gupta, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, arnd

Hi Bjorn,

Thanks for the info.

Joao

On 1/27/2016 9:59 PM, Bjorn Helgaas wrote:
> Hi Joao,
> 
> On Wed, Jan 27, 2016 at 02:29:21PM +0000, Joao Pinto wrote:
>> Hi Bjorn,
>> Could you please tell me what are the perspectives of pulling this patch set to
>> mainline?
> 
> Sorry for the delay.  I try to handle bug fixes before I look at
> functionality, so sometimes things get backed up a little bit.  But I
> should be able to take a look later this week or early next week.
> 
> Bjorn
> 
>> On 1/14/2016 11:11 AM, Vineet Gupta wrote:
>>> On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
>>>> This patch adds PCI support to ARC and updates drivers/pci Makefile
>>>> enabling the ARC arch to use the generic PCI setup functions.
>>>>
>>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>>>
>>> Acked-by: Vineet Gupta <vgupta@synopsys.com>
>>>
>>> @Bjorn are you happy with the driver bits.
>>> Although ARC support is independent of driver, and I can pick it up separately,
>>> it would be better if you took it along with the driver. I don't see potential
>>> for any conflicts.
>>>
>>> Thx,
>>> -Vineet
>>
>> Thanks,
>> Joao
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-28 17:00           ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-28 17:00 UTC (permalink / raw)
  To: Bjorn Helgaas, Joao Pinto
  Cc: Vineet Gupta, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, arnd

Hi Bjorn,

Thanks for the info.

Joao

On 1/27/2016 9:59 PM, Bjorn Helgaas wrote:
> Hi Joao,
> 
> On Wed, Jan 27, 2016 at 02:29:21PM +0000, Joao Pinto wrote:
>> Hi Bjorn,
>> Could you please tell me what are the perspectives of pulling this patch set to
>> mainline?
> 
> Sorry for the delay.  I try to handle bug fixes before I look at
> functionality, so sometimes things get backed up a little bit.  But I
> should be able to take a look later this week or early next week.
> 
> Bjorn
> 
>> On 1/14/2016 11:11 AM, Vineet Gupta wrote:
>>> On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
>>>> This patch adds PCI support to ARC and updates drivers/pci Makefile
>>>> enabling the ARC arch to use the generic PCI setup functions.
>>>>
>>>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>>>
>>> Acked-by: Vineet Gupta <vgupta@synopsys.com>
>>>
>>> @Bjorn are you happy with the driver bits.
>>> Although ARC support is independent of driver, and I can pick it up separately,
>>> it would be better if you took it along with the driver. I don't see potential
>>> for any conflicts.
>>>
>>> Thx,
>>> -Vineet
>>
>> Thanks,
>> Joao
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-28 17:00           ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-01-28 17:00 UTC (permalink / raw)
  To: linux-snps-arc

Hi Bjorn,

Thanks for the info.

Joao

On 1/27/2016 9:59 PM, Bjorn Helgaas wrote:
> Hi Joao,
> 
> On Wed, Jan 27, 2016@02:29:21PM +0000, Joao Pinto wrote:
>> Hi Bjorn,
>> Could you please tell me what are the perspectives of pulling this patch set to
>> mainline?
> 
> Sorry for the delay.  I try to handle bug fixes before I look at
> functionality, so sometimes things get backed up a little bit.  But I
> should be able to take a look later this week or early next week.
> 
> Bjorn
> 
>> On 1/14/2016 11:11 AM, Vineet Gupta wrote:
>>> On Thursday 14 January 2016 04:34 PM, Joao Pinto wrote:
>>>> This patch adds PCI support to ARC and updates drivers/pci Makefile
>>>> enabling the ARC arch to use the generic PCI setup functions.
>>>>
>>>> Signed-off-by: Joao Pinto <jpinto at synopsys.com>
>>>
>>> Acked-by: Vineet Gupta <vgupta at synopsys.com>
>>>
>>> @Bjorn are you happy with the driver bits.
>>> Although ARC support is independent of driver, and I can pick it up separately,
>>> it would be better if you took it along with the driver. I don't see potential
>>> for any conflicts.
>>>
>>> Thx,
>>> -Vineet
>>
>> Thanks,
>> Joao
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo at vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 2/2] add new platform driver for PCI RC
  2016-01-12 15:45   ` [PATCH v5 " Joao Pinto
@ 2016-01-29 20:36     ` Bjorn Helgaas
  -1 siblings, 0 replies; 51+ messages in thread
From: Bjorn Helgaas @ 2016-01-29 20:36 UTC (permalink / raw)
  To: Joao Pinto
  Cc: Vineet.Gupta1, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, arnd

Hi Joao,

This is getting pretty close.  I have a few comments below.  This is a
new driver, with no chance of breaking anything else, so I think we
can still get it in for v4.5.

Bjorn

On Thu, Jan 14, 2016 at 11:04:33AM +0000, Joao Pinto wrote:
> This patch adds a new driver that will be the reference platform driver
> for all PCI RC IP Protoyping Kits based on ARC SDP.
> This patch is composed by:
> 
> -Changes to pcie-designware driver add a function that enables the
> feature of starting the LTSSM (Link Train Status State) used by the
> new driver
> -MAINTAINERS file was updated to include the new driver
> -Documentation/devicetree/bindings/pci was updated to include the new
> driver documentation
> -New driver called pcie-snpsdev
> 
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> ---
> Change v5 -> v6:
> - Nothing changed (just to keep up with patch set version).
> Change v4 -> v5:
> - Nothing changed (just to keep up with patch set version).
> Changes v3 -> v4 (Bjorn Helgaas):
> - ARCH dependencies were added to the drivers/pci/host/kconfig for the
> PCIE_SNPSDEV.
> Changes v2 -> v3 (Bjorn Helgaas):
> - link init stuff was moved to a snpsdev_pcie_establish_link() function in
> pcie-snpsdev
> - pcie-snpsdev driver declaration was changed to be more 
> standard (Bjorn Helgaas)
> - pcie-designware' dw_pcie_link_retrain() now use standard registers from
> pci-regs.h (Bjorn Helgaas)
> - pcie-snpsdev.txt was complemented with more info (Mark Rutland)
> Changes v1 -> v2 (Bjorn Helgaas):
> - Fixups snpsdev_pcie_fixup_bridge() and snpsdev_pcie_fixup_res() were removed
> from the driver (these functions were for specific tests only and not usefull
> in mainline)
> - Driver' comments were reviewed (fix Typos and excessive comments removal)
> - Removed unnecessary definitions in the driver source (PCIE_PHY_CTRL and
> PCIE_PHY_STAT)
> 
>  .../devicetree/bindings/pci/pcie-snpsdev.txt       |  33 +++
>  MAINTAINERS                                        |   7 +
>  drivers/pci/host/Kconfig                           |   8 +
>  drivers/pci/host/Makefile                          |   1 +
>  drivers/pci/host/pcie-designware.c                 |   9 +
>  drivers/pci/host/pcie-designware.h                 |   1 +
>  drivers/pci/host/pcie-snpsdev.c                    | 286 +++++++++++++++++++++
>  7 files changed, 345 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
>  create mode 100644 drivers/pci/host/pcie-snpsdev.c
> 
> diff --git a/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
> new file mode 100644
> index 0000000..cae548b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
> @@ -0,0 +1,33 @@
> +Synopsys PCI RC IP Prototyping Kit
> +----------------------------------
> +
> +This is the reference platform driver to be used in the Synopsys PCI Root
> +Complex IP Prototyping Kit.
> +
> +Required properties:
> +- compatible: set to "snps,pcie-snpsdev";
> +- reg: base address and length of the pcie controller registers.
> +- #address-cells: set to <3>
> +- #size-cells: set to <2>
> +- device_type: set to "pci"
> +- ranges: ranges for the PCI memory and I/O regions.
> +- interrupts: one interrupt source for MSI interrupts, followed by interrupt
> +	source for hardware related interrupts.
> +- #interrupt-cells: set to <1>
> +- num-lanes: set to <1>;
> +
> +Example configuration:
> +
> +	pcie: pcie@0xdffff000 {
> +		compatible = "snps,pcie-snpsdev";
> +		reg = <0xdffff000 0x1000>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		ranges = <0x00000800 0 0xd0000000 0xd0000000 0 0x00002000>,
> +			 <0x81000000 0 0x00000000 0xde000000 0 0x00010000>,
> +			 <0x82000000 0 0xd0400000 0xd0400000 0 0x0d000000>;
> +		interrupts = <25>, <24>;
> +		#interrupt-cells = <1>;
> +		num-lanes = <1>;
> +	};

Can we get an ack from the DT guys for this?

Is "snpsdev" an already-established abbreviation?  The "dev" part
seems obvious and maybe could go without saying.  This would look
nicer if we could just use "synopsis" everywhere you have "snpsdev" --
DT compatible string, filename, function names, etc.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index e9caa4b..d2e4506 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8230,6 +8230,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>  F:	drivers/pci/host/pcie-hisi.c
>  
> +PCI DRIVER FOR SYNOPSYS PROTOTYPING DEVICE
> +M:	Joao Pinto <jpinto@synopsys.com>
> +L:	linux-pci@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
> +F:	drivers/pci/host/pcie-snpsdev.c
> +
>  PCMCIA SUBSYSTEM
>  P:	Linux PCMCIA Team
>  L:	linux-pcmcia@lists.infradead.org
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index f131ba9..589bc15 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -172,4 +172,12 @@ config PCI_HISI
>  	help
>  	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
>  
> +config PCIE_SNPSDEV
> +	bool "Platform Driver for Synopsys Device"
> +	depends on ARC && OF
> +	select PCIEPORTBUS
> +	select PCIE_DW
> +	help
> +	  Say Y here if you want to enable PCIe controller support on the
> +	  Synopsys IP Prototyping Kits.
>  endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 9d4d3c6..e422f65 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -20,3 +20,4 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
>  obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
>  obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
>  obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
> +obj-$(CONFIG_PCIE_SNPSDEV) += pcie-snpsdev.o
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 540f077..f73a3cf 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -706,6 +706,15 @@ static struct pci_ops dw_pcie_ops = {
>  	.write = dw_pcie_wr_conf,
>  };
>  
> +void dw_pcie_link_retrain(struct pcie_port *pp)
> +{
> +	u32 val = 0;
> +
> +	dw_pcie_readl_rc(pp, PCI_EXP_LNKCTL+0x70, &val);
> +	val = val | PCI_EXP_LNKCTL_RL;
> +	dw_pcie_writel_rc(pp, val, PCI_EXP_LNKCTL+0x70);
> +}

It seems odd that you need to add this.  Please split it into a
separate patch and we can get the DW guys to ack it.

> +
>  void dw_pcie_setup_rc(struct pcie_port *pp)
>  {
>  	u32 val;
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index 2356d29..249b631 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -79,5 +79,6 @@ void dw_pcie_msi_init(struct pcie_port *pp);
>  int dw_pcie_link_up(struct pcie_port *pp);
>  void dw_pcie_setup_rc(struct pcie_port *pp);
>  int dw_pcie_host_init(struct pcie_port *pp);
> +void dw_pcie_link_retrain(struct pcie_port *pp);
>  
>  #endif /* _PCIE_DESIGNWARE_H */
> diff --git a/drivers/pci/host/pcie-snpsdev.c b/drivers/pci/host/pcie-snpsdev.c
> new file mode 100644
> index 0000000..4ca7ec5
> --- /dev/null
> +++ b/drivers/pci/host/pcie-snpsdev.c
> @@ -0,0 +1,286 @@
> +/*
> + * PCIe RC driver for Synopsys Designware Core
> + *
> + * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
> + *
> + * Authors: Manjunath Bettegowda <manjumb@synopsys.com>,
> + *	    Jie Deng <jiedeng@synopsys.com>
> + *	    Joao Pinto <jpinto@synopsys.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/resource.h>
> +#include <linux/signal.h>
> +#include <linux/types.h>
> +
> +#include "pcie-designware.h"
> +
> +#define to_snpsdev_pcie(x)	container_of(x, struct snpsdev_pcie, pp)
> +
> +struct snpsdev_pcie {
> +	void __iomem		*mem_base; /* Memory Base to access Core's [RC]
> +					    * Config Space Layout
> +					    */
> +	struct pcie_port	pp;        /* RC Root Port specific structure -
> +					    * DWC_PCIE_RC stuff
> +					    */
> +};
> +
> +#define PCI_EQUAL_CONTROL_PHY 0x00000707
> +
> +/* PCIe Port Logic registers (memory-mapped) */
> +#define PLR_OFFSET 0x700
> +#define PCIE_PHY_DEBUG_R0 (PLR_OFFSET + 0x28) /* 0x728 */
> +#define PCIE_PHY_DEBUG_R1 (PLR_OFFSET + 0x2c) /* 0x72c */
> +
> +/* This handler was created for future work */
> +static irqreturn_t snpsdev_pcie_irq_handler(int irq, void *arg)
> +{
> +	return IRQ_NONE;
> +}
> +
> +static irqreturn_t snpsdev_pcie_msi_irq_handler(int irq, void *arg)
> +{
> +	struct pcie_port *pp = arg;
> +
> +	dw_handle_msi_irq(pp);
> +
> +	return IRQ_HANDLED;

I think this should be

  return dw_handle_msi_irq(pp);

as it is in other DW-based drivers (or there should be a comment about
why this needs to be different).

> +}
> +
> +static void snpsdev_pcie_init_phy(struct pcie_port *pp)
> +{
> +	/* write Lane 0 Equalization Control fields register */
> +	writel(PCI_EQUAL_CONTROL_PHY, pp->dbi_base + 0x154);
> +}
> +
> +static int snpsdev_pcie_deassert_core_reset(struct pcie_port *pp)
> +{
> +	return 0;
> +}
> +
> +static void snpsdev_pcie_establish_link(struct pcie_port *pp)
> +{
> +	int count = 0;
> +
> +	/* Initialize Phy (Reset/poweron/control-inputs ) */
> +	snpsdev_pcie_init_phy(pp);
> +
> +	/* de-assert core reset */

Superfluous comment, since the function name repeats exactly the same
thing.  The one above is probably superfluous, too.

> +	snpsdev_pcie_deassert_core_reset(pp);
> +
> +	/* We expect the PCIe Link to be up by this time */

Is the comment really true?  It seems strange to retrain the link
below if you expect it to already be up before you even call
dw_pcie_setup_rc().

> +	dw_pcie_setup_rc(pp);

I count 7 existing callers of dw_pcie_setup_rc().  4 are from
*_pcie_host_init(), and 3 are from *_pcie_establish_link().  Let's
follow the trend by doing the things above in
snpsdev_pcie_host_init().  Unless there's a *reason* to be different,
I want all these DW-based drivers to look as much alike as possible.

> +
> +	/* Start LTSSM here */
> +	dw_pcie_link_retrain(pp);

What's different about this system that means you require this
link_retrain() interface, when all the other drivers don't?

> +
> +	while (!dw_pcie_link_up(pp)) {
> +		usleep_range(1000, 1100);
> +		count++;
> +		if (count == 20) {
> +			dev_err(pp->dev, "phy link never came up\n");
> +			dev_dbg(pp->dev,
> +				"PL_DEBUG0: 0x%08x, DEBUG_R1: 0x%08x\n",
> +				readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
> +				readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
> +			break;
> +		}
> +	}

Can you look at the other DW-based drivers and copy their "wait for
link up" code?  This is basically similar, but again, doing it
"basically similar but slightly different" just makes work and errors.

> +}
> +
> +/*
> + * snpsdev_pcie_host_init()
> + * Platform specific host/RC initialization
> + *	a. Assert the core reset
> + *	b. Assert and deassert phy reset and initialize the phy
> + *	c. De-Assert the core reset
> + *	d. Initializet the Root Port (BARs/Memory Or IO/ Interrupt/ Commnad Reg)
> + *	e. Initiate Link startup procedure
> + *
> + */
> +static void snpsdev_pcie_host_init(struct pcie_port *pp)
> +{
> +	/* Establish link */

Superfluous comment.

> +	snpsdev_pcie_establish_link(pp);
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI))
> +		dw_pcie_msi_init(pp);
> +}
> +
> +static int snpsdev_pcie_link_up(struct pcie_port *pp)
> +{
> +	u32 status;
> +
> +	/* Bit number 36: reports LTSSM PHY Link UP; Available in bit 3 of
> +	 * PCIE_PHY_DEBUG_R1
> +	 */
> +	status = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1) & (0x1 << 4);
> +	if (status != 0)
> +		return 1;

This would be easier to read as something like:

  #define PCIE_PHY_DEBUG_R1_LINK_UP 0x00000010

  val = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1);
  return val & PCIE_PHY_DEBUG_R1_LINK_UP;

> +
> +	/* TODO:Now Link is in L0;Initiate GEN2/GEN3 migration if RC Supports*/
> +	return 0;
> +}
> +
> +/**
> + * This is RC operation structure
> + * snpsdev_pcie_link_up: the function which initiates the phy link up procedure
> + * snpsdev_pcie_host_init: the function which does the host/RC Root port
> + * initialization.
> + */
> +static struct pcie_host_ops snpsdev_pcie_host_ops = {
> +	.link_up = snpsdev_pcie_link_up,
> +	.host_init = snpsdev_pcie_host_init,
> +};
> +
> +/**
> + * snpsdev_add_pcie_port
> + * This function
> + * a. installs the interrupt handler
> + * b. registers host operations in the pcie_port structure
> + */
> +static int snpsdev_add_pcie_port(struct pcie_port *pp,
> +				 struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	pp->irq = platform_get_irq(pdev, 1);
> +
> +	if (pp->irq < 0) {
> +		if (pp->irq != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "cannot get irq\n");

Hmmm...  I don't really understand this EPROBE_DEFER stuff.  Most
callers of platform_get_irq() don't check for it explicitly (and
*none* of the DW-based drivers does), so I hope it's not something
drivers are supposed to do something special with.  In this case, the
only difference is makes is whether you print a message, so my advice
is to just print the message unconditionally if platform_get_irq()
fails for any reason.

> +		return pp->irq;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, pp->irq, snpsdev_pcie_irq_handler,
> +				IRQF_SHARED, "snpsdev-pcie", pp);
> +
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request irq\n");
> +		return ret;
> +	}
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +		pp->msi_irq = platform_get_irq(pdev, 0);
> +
> +		if (pp->msi_irq < 0) {
> +			if (pp->msi_irq != -EPROBE_DEFER)
> +				dev_err(&pdev->dev, "cannot get msi irq\n");

Same here.

> +			return pp->msi_irq;
> +		}
> +
> +		ret = devm_request_irq(&pdev->dev, pp->msi_irq,
> +					snpsdev_pcie_msi_irq_handler,
> +					IRQF_SHARED, "snpsdev-pcie-msi", pp);
> +		if (ret) {
> +			dev_err(&pdev->dev, "failed to request msi irq\n");
> +			return ret;
> +		}
> +	}
> +
> +	pp->root_bus_nr = -1;
> +	pp->ops = &snpsdev_pcie_host_ops;
> +
> +	/* Below function:
> +	 * Checks for range property from DT
> +	 * Gets the IO and MEMORY and CONFIG-Space ranges from DT
> +	 * Does IOREMAPS on the physical addresses
> +	 * Gets the num-lanes from DT
> +	 * Gets MSI capability from DT
> +	 * Calls the platform specific host initialization
> +	 * Program the correct class, BAR0, Link width, in Config space
> +	 * Then it calls pci common init routine
> +	 * Then it calls function to assign "unassigned resources"
> +	 */
> +	ret = dw_pcie_host_init(pp);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to initialize host\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * snpsdev_pcie_rc_probe()
> + * This function gets called as part of pcie registration. if the id matches
> + * the platform driver framework will call this function.
> + *
> + * @pdev: Pointer to the platform_device structure
> + *
> + * Returns zero on success; Negative errorno on failure
> + */
> +static int snpsdev_pcie_rc_probe(struct platform_device *pdev)

snpsdev_pcie_probe() (so it follows the pattern of other drivers).

> +{
> +	struct snpsdev_pcie *snpsdev_pcie;
> +	struct pcie_port *pp;
> +	struct resource *dwc_pcie_rc_res;  /* Resource from DT */

"res" would be a good enough name and would avoid line wrapping below.

> +	int ret;
> +
> +	snpsdev_pcie = devm_kzalloc(&pdev->dev, sizeof(*snpsdev_pcie),
> +					GFP_KERNEL);
> +	if (!snpsdev_pcie)
> +		return -ENOMEM;
> +
> +	pp = &snpsdev_pcie->pp;
> +	pp->dev = &pdev->dev;
> +
> +	dwc_pcie_rc_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	if (!dwc_pcie_rc_res)
> +		return -ENODEV;
> +
> +	snpsdev_pcie->mem_base = devm_ioremap_resource(&pdev->dev,
> +							dwc_pcie_rc_res);
> +	if (IS_ERR(snpsdev_pcie->mem_base)) {
> +		ret = PTR_ERR(snpsdev_pcie->mem_base);
> +		return ret;

  return PTR_ERR(snpsdev_pcie->mem_base);

> +	}
> +	pp->dbi_base = snpsdev_pcie->mem_base;
> +
> +	ret = snpsdev_add_pcie_port(pp, pdev);
> +	if (ret < 0)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, snpsdev_pcie);
> +
> +	return 0;
> +}
> +
> +static int snpsdev_pcie_rc_remove(struct platform_device *pdev)
> +{
> +	return 0;
> +}

This isn't referenced anywhere, so I think you could remove it.

> +static const struct of_device_id snpsdev_pcie_rc_of_match[] = {
> +	{ .compatible = "snps,pcie-snpsdev", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, snpsdev_pcie_rc_of_match);
> +
> +static struct platform_driver snpsdev_pcie_rc_driver = {
> +	.driver = {
> +		.name	= "pcie-snpsdev",
> +		.of_match_table = snpsdev_pcie_rc_of_match,
> +	},
> +	.probe = snpsdev_pcie_rc_probe,
> +};
> +
> +module_platform_driver(snpsdev_pcie_rc_driver);
> +
> +MODULE_AUTHOR("Manjunath Bettegowda <manjumb@synopsys.com>");
> +MODULE_DESCRIPTION("Platform Driver for Synopsys Device");

Should probably mention the PCI connection, e.g., "Synopsys PCIe host
controller driver".

> +MODULE_LICENSE("GPL v2");
> -- 
> 1.8.1.5

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v6 2/2] add new platform driver for PCI RC
@ 2016-01-29 20:36     ` Bjorn Helgaas
  0 siblings, 0 replies; 51+ messages in thread
From: Bjorn Helgaas @ 2016-01-29 20:36 UTC (permalink / raw)
  To: linux-snps-arc

Hi Joao,

This is getting pretty close.  I have a few comments below.  This is a
new driver, with no chance of breaking anything else, so I think we
can still get it in for v4.5.

Bjorn

On Thu, Jan 14, 2016@11:04:33AM +0000, Joao Pinto wrote:
> This patch adds a new driver that will be the reference platform driver
> for all PCI RC IP Protoyping Kits based on ARC SDP.
> This patch is composed by:
> 
> -Changes to pcie-designware driver add a function that enables the
> feature of starting the LTSSM (Link Train Status State) used by the
> new driver
> -MAINTAINERS file was updated to include the new driver
> -Documentation/devicetree/bindings/pci was updated to include the new
> driver documentation
> -New driver called pcie-snpsdev
> 
> Signed-off-by: Joao Pinto <jpinto at synopsys.com>
> ---
> Change v5 -> v6:
> - Nothing changed (just to keep up with patch set version).
> Change v4 -> v5:
> - Nothing changed (just to keep up with patch set version).
> Changes v3 -> v4 (Bjorn Helgaas):
> - ARCH dependencies were added to the drivers/pci/host/kconfig for the
> PCIE_SNPSDEV.
> Changes v2 -> v3 (Bjorn Helgaas):
> - link init stuff was moved to a snpsdev_pcie_establish_link() function in
> pcie-snpsdev
> - pcie-snpsdev driver declaration was changed to be more 
> standard (Bjorn Helgaas)
> - pcie-designware' dw_pcie_link_retrain() now use standard registers from
> pci-regs.h (Bjorn Helgaas)
> - pcie-snpsdev.txt was complemented with more info (Mark Rutland)
> Changes v1 -> v2 (Bjorn Helgaas):
> - Fixups snpsdev_pcie_fixup_bridge() and snpsdev_pcie_fixup_res() were removed
> from the driver (these functions were for specific tests only and not usefull
> in mainline)
> - Driver' comments were reviewed (fix Typos and excessive comments removal)
> - Removed unnecessary definitions in the driver source (PCIE_PHY_CTRL and
> PCIE_PHY_STAT)
> 
>  .../devicetree/bindings/pci/pcie-snpsdev.txt       |  33 +++
>  MAINTAINERS                                        |   7 +
>  drivers/pci/host/Kconfig                           |   8 +
>  drivers/pci/host/Makefile                          |   1 +
>  drivers/pci/host/pcie-designware.c                 |   9 +
>  drivers/pci/host/pcie-designware.h                 |   1 +
>  drivers/pci/host/pcie-snpsdev.c                    | 286 +++++++++++++++++++++
>  7 files changed, 345 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
>  create mode 100644 drivers/pci/host/pcie-snpsdev.c
> 
> diff --git a/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
> new file mode 100644
> index 0000000..cae548b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
> @@ -0,0 +1,33 @@
> +Synopsys PCI RC IP Prototyping Kit
> +----------------------------------
> +
> +This is the reference platform driver to be used in the Synopsys PCI Root
> +Complex IP Prototyping Kit.
> +
> +Required properties:
> +- compatible: set to "snps,pcie-snpsdev";
> +- reg: base address and length of the pcie controller registers.
> +- #address-cells: set to <3>
> +- #size-cells: set to <2>
> +- device_type: set to "pci"
> +- ranges: ranges for the PCI memory and I/O regions.
> +- interrupts: one interrupt source for MSI interrupts, followed by interrupt
> +	source for hardware related interrupts.
> +- #interrupt-cells: set to <1>
> +- num-lanes: set to <1>;
> +
> +Example configuration:
> +
> +	pcie: pcie at 0xdffff000 {
> +		compatible = "snps,pcie-snpsdev";
> +		reg = <0xdffff000 0x1000>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		ranges = <0x00000800 0 0xd0000000 0xd0000000 0 0x00002000>,
> +			 <0x81000000 0 0x00000000 0xde000000 0 0x00010000>,
> +			 <0x82000000 0 0xd0400000 0xd0400000 0 0x0d000000>;
> +		interrupts = <25>, <24>;
> +		#interrupt-cells = <1>;
> +		num-lanes = <1>;
> +	};

Can we get an ack from the DT guys for this?

Is "snpsdev" an already-established abbreviation?  The "dev" part
seems obvious and maybe could go without saying.  This would look
nicer if we could just use "synopsis" everywhere you have "snpsdev" --
DT compatible string, filename, function names, etc.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index e9caa4b..d2e4506 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8230,6 +8230,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>  F:	drivers/pci/host/pcie-hisi.c
>  
> +PCI DRIVER FOR SYNOPSYS PROTOTYPING DEVICE
> +M:	Joao Pinto <jpinto at synopsys.com>
> +L:	linux-pci at vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/pci/pcie-snpsdev.txt
> +F:	drivers/pci/host/pcie-snpsdev.c
> +
>  PCMCIA SUBSYSTEM
>  P:	Linux PCMCIA Team
>  L:	linux-pcmcia at lists.infradead.org
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index f131ba9..589bc15 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -172,4 +172,12 @@ config PCI_HISI
>  	help
>  	  Say Y here if you want PCIe controller support on HiSilicon HIP05 SoC
>  
> +config PCIE_SNPSDEV
> +	bool "Platform Driver for Synopsys Device"
> +	depends on ARC && OF
> +	select PCIEPORTBUS
> +	select PCIE_DW
> +	help
> +	  Say Y here if you want to enable PCIe controller support on the
> +	  Synopsys IP Prototyping Kits.
>  endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 9d4d3c6..e422f65 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -20,3 +20,4 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
>  obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
>  obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
>  obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
> +obj-$(CONFIG_PCIE_SNPSDEV) += pcie-snpsdev.o
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 540f077..f73a3cf 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -706,6 +706,15 @@ static struct pci_ops dw_pcie_ops = {
>  	.write = dw_pcie_wr_conf,
>  };
>  
> +void dw_pcie_link_retrain(struct pcie_port *pp)
> +{
> +	u32 val = 0;
> +
> +	dw_pcie_readl_rc(pp, PCI_EXP_LNKCTL+0x70, &val);
> +	val = val | PCI_EXP_LNKCTL_RL;
> +	dw_pcie_writel_rc(pp, val, PCI_EXP_LNKCTL+0x70);
> +}

It seems odd that you need to add this.  Please split it into a
separate patch and we can get the DW guys to ack it.

> +
>  void dw_pcie_setup_rc(struct pcie_port *pp)
>  {
>  	u32 val;
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index 2356d29..249b631 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -79,5 +79,6 @@ void dw_pcie_msi_init(struct pcie_port *pp);
>  int dw_pcie_link_up(struct pcie_port *pp);
>  void dw_pcie_setup_rc(struct pcie_port *pp);
>  int dw_pcie_host_init(struct pcie_port *pp);
> +void dw_pcie_link_retrain(struct pcie_port *pp);
>  
>  #endif /* _PCIE_DESIGNWARE_H */
> diff --git a/drivers/pci/host/pcie-snpsdev.c b/drivers/pci/host/pcie-snpsdev.c
> new file mode 100644
> index 0000000..4ca7ec5
> --- /dev/null
> +++ b/drivers/pci/host/pcie-snpsdev.c
> @@ -0,0 +1,286 @@
> +/*
> + * PCIe RC driver for Synopsys Designware Core
> + *
> + * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
> + *
> + * Authors: Manjunath Bettegowda <manjumb at synopsys.com>,
> + *	    Jie Deng <jiedeng at synopsys.com>
> + *	    Joao Pinto <jpinto at synopsys.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/delay.h>
> +#include <linux/gpio.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_gpio.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/resource.h>
> +#include <linux/signal.h>
> +#include <linux/types.h>
> +
> +#include "pcie-designware.h"
> +
> +#define to_snpsdev_pcie(x)	container_of(x, struct snpsdev_pcie, pp)
> +
> +struct snpsdev_pcie {
> +	void __iomem		*mem_base; /* Memory Base to access Core's [RC]
> +					    * Config Space Layout
> +					    */
> +	struct pcie_port	pp;        /* RC Root Port specific structure -
> +					    * DWC_PCIE_RC stuff
> +					    */
> +};
> +
> +#define PCI_EQUAL_CONTROL_PHY 0x00000707
> +
> +/* PCIe Port Logic registers (memory-mapped) */
> +#define PLR_OFFSET 0x700
> +#define PCIE_PHY_DEBUG_R0 (PLR_OFFSET + 0x28) /* 0x728 */
> +#define PCIE_PHY_DEBUG_R1 (PLR_OFFSET + 0x2c) /* 0x72c */
> +
> +/* This handler was created for future work */
> +static irqreturn_t snpsdev_pcie_irq_handler(int irq, void *arg)
> +{
> +	return IRQ_NONE;
> +}
> +
> +static irqreturn_t snpsdev_pcie_msi_irq_handler(int irq, void *arg)
> +{
> +	struct pcie_port *pp = arg;
> +
> +	dw_handle_msi_irq(pp);
> +
> +	return IRQ_HANDLED;

I think this should be

  return dw_handle_msi_irq(pp);

as it is in other DW-based drivers (or there should be a comment about
why this needs to be different).

> +}
> +
> +static void snpsdev_pcie_init_phy(struct pcie_port *pp)
> +{
> +	/* write Lane 0 Equalization Control fields register */
> +	writel(PCI_EQUAL_CONTROL_PHY, pp->dbi_base + 0x154);
> +}
> +
> +static int snpsdev_pcie_deassert_core_reset(struct pcie_port *pp)
> +{
> +	return 0;
> +}
> +
> +static void snpsdev_pcie_establish_link(struct pcie_port *pp)
> +{
> +	int count = 0;
> +
> +	/* Initialize Phy (Reset/poweron/control-inputs ) */
> +	snpsdev_pcie_init_phy(pp);
> +
> +	/* de-assert core reset */

Superfluous comment, since the function name repeats exactly the same
thing.  The one above is probably superfluous, too.

> +	snpsdev_pcie_deassert_core_reset(pp);
> +
> +	/* We expect the PCIe Link to be up by this time */

Is the comment really true?  It seems strange to retrain the link
below if you expect it to already be up before you even call
dw_pcie_setup_rc().

> +	dw_pcie_setup_rc(pp);

I count 7 existing callers of dw_pcie_setup_rc().  4 are from
*_pcie_host_init(), and 3 are from *_pcie_establish_link().  Let's
follow the trend by doing the things above in
snpsdev_pcie_host_init().  Unless there's a *reason* to be different,
I want all these DW-based drivers to look as much alike as possible.

> +
> +	/* Start LTSSM here */
> +	dw_pcie_link_retrain(pp);

What's different about this system that means you require this
link_retrain() interface, when all the other drivers don't?

> +
> +	while (!dw_pcie_link_up(pp)) {
> +		usleep_range(1000, 1100);
> +		count++;
> +		if (count == 20) {
> +			dev_err(pp->dev, "phy link never came up\n");
> +			dev_dbg(pp->dev,
> +				"PL_DEBUG0: 0x%08x, DEBUG_R1: 0x%08x\n",
> +				readl(pp->dbi_base + PCIE_PHY_DEBUG_R0),
> +				readl(pp->dbi_base + PCIE_PHY_DEBUG_R1));
> +			break;
> +		}
> +	}

Can you look at the other DW-based drivers and copy their "wait for
link up" code?  This is basically similar, but again, doing it
"basically similar but slightly different" just makes work and errors.

> +}
> +
> +/*
> + * snpsdev_pcie_host_init()
> + * Platform specific host/RC initialization
> + *	a. Assert the core reset
> + *	b. Assert and deassert phy reset and initialize the phy
> + *	c. De-Assert the core reset
> + *	d. Initializet the Root Port (BARs/Memory Or IO/ Interrupt/ Commnad Reg)
> + *	e. Initiate Link startup procedure
> + *
> + */
> +static void snpsdev_pcie_host_init(struct pcie_port *pp)
> +{
> +	/* Establish link */

Superfluous comment.

> +	snpsdev_pcie_establish_link(pp);
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI))
> +		dw_pcie_msi_init(pp);
> +}
> +
> +static int snpsdev_pcie_link_up(struct pcie_port *pp)
> +{
> +	u32 status;
> +
> +	/* Bit number 36: reports LTSSM PHY Link UP; Available in bit 3 of
> +	 * PCIE_PHY_DEBUG_R1
> +	 */
> +	status = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1) & (0x1 << 4);
> +	if (status != 0)
> +		return 1;

This would be easier to read as something like:

  #define PCIE_PHY_DEBUG_R1_LINK_UP 0x00000010

  val = readl(pp->dbi_base + PCIE_PHY_DEBUG_R1);
  return val & PCIE_PHY_DEBUG_R1_LINK_UP;

> +
> +	/* TODO:Now Link is in L0;Initiate GEN2/GEN3 migration if RC Supports*/
> +	return 0;
> +}
> +
> +/**
> + * This is RC operation structure
> + * snpsdev_pcie_link_up: the function which initiates the phy link up procedure
> + * snpsdev_pcie_host_init: the function which does the host/RC Root port
> + * initialization.
> + */
> +static struct pcie_host_ops snpsdev_pcie_host_ops = {
> +	.link_up = snpsdev_pcie_link_up,
> +	.host_init = snpsdev_pcie_host_init,
> +};
> +
> +/**
> + * snpsdev_add_pcie_port
> + * This function
> + * a. installs the interrupt handler
> + * b. registers host operations in the pcie_port structure
> + */
> +static int snpsdev_add_pcie_port(struct pcie_port *pp,
> +				 struct platform_device *pdev)
> +{
> +	int ret;
> +
> +	pp->irq = platform_get_irq(pdev, 1);
> +
> +	if (pp->irq < 0) {
> +		if (pp->irq != -EPROBE_DEFER)
> +			dev_err(&pdev->dev, "cannot get irq\n");

Hmmm...  I don't really understand this EPROBE_DEFER stuff.  Most
callers of platform_get_irq() don't check for it explicitly (and
*none* of the DW-based drivers does), so I hope it's not something
drivers are supposed to do something special with.  In this case, the
only difference is makes is whether you print a message, so my advice
is to just print the message unconditionally if platform_get_irq()
fails for any reason.

> +		return pp->irq;
> +	}
> +
> +	ret = devm_request_irq(&pdev->dev, pp->irq, snpsdev_pcie_irq_handler,
> +				IRQF_SHARED, "snpsdev-pcie", pp);
> +
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to request irq\n");
> +		return ret;
> +	}
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> +		pp->msi_irq = platform_get_irq(pdev, 0);
> +
> +		if (pp->msi_irq < 0) {
> +			if (pp->msi_irq != -EPROBE_DEFER)
> +				dev_err(&pdev->dev, "cannot get msi irq\n");

Same here.

> +			return pp->msi_irq;
> +		}
> +
> +		ret = devm_request_irq(&pdev->dev, pp->msi_irq,
> +					snpsdev_pcie_msi_irq_handler,
> +					IRQF_SHARED, "snpsdev-pcie-msi", pp);
> +		if (ret) {
> +			dev_err(&pdev->dev, "failed to request msi irq\n");
> +			return ret;
> +		}
> +	}
> +
> +	pp->root_bus_nr = -1;
> +	pp->ops = &snpsdev_pcie_host_ops;
> +
> +	/* Below function:
> +	 * Checks for range property from DT
> +	 * Gets the IO and MEMORY and CONFIG-Space ranges from DT
> +	 * Does IOREMAPS on the physical addresses
> +	 * Gets the num-lanes from DT
> +	 * Gets MSI capability from DT
> +	 * Calls the platform specific host initialization
> +	 * Program the correct class, BAR0, Link width, in Config space
> +	 * Then it calls pci common init routine
> +	 * Then it calls function to assign "unassigned resources"
> +	 */
> +	ret = dw_pcie_host_init(pp);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to initialize host\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * snpsdev_pcie_rc_probe()
> + * This function gets called as part of pcie registration. if the id matches
> + * the platform driver framework will call this function.
> + *
> + * @pdev: Pointer to the platform_device structure
> + *
> + * Returns zero on success; Negative errorno on failure
> + */
> +static int snpsdev_pcie_rc_probe(struct platform_device *pdev)

snpsdev_pcie_probe() (so it follows the pattern of other drivers).

> +{
> +	struct snpsdev_pcie *snpsdev_pcie;
> +	struct pcie_port *pp;
> +	struct resource *dwc_pcie_rc_res;  /* Resource from DT */

"res" would be a good enough name and would avoid line wrapping below.

> +	int ret;
> +
> +	snpsdev_pcie = devm_kzalloc(&pdev->dev, sizeof(*snpsdev_pcie),
> +					GFP_KERNEL);
> +	if (!snpsdev_pcie)
> +		return -ENOMEM;
> +
> +	pp = &snpsdev_pcie->pp;
> +	pp->dev = &pdev->dev;
> +
> +	dwc_pcie_rc_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +
> +	if (!dwc_pcie_rc_res)
> +		return -ENODEV;
> +
> +	snpsdev_pcie->mem_base = devm_ioremap_resource(&pdev->dev,
> +							dwc_pcie_rc_res);
> +	if (IS_ERR(snpsdev_pcie->mem_base)) {
> +		ret = PTR_ERR(snpsdev_pcie->mem_base);
> +		return ret;

  return PTR_ERR(snpsdev_pcie->mem_base);

> +	}
> +	pp->dbi_base = snpsdev_pcie->mem_base;
> +
> +	ret = snpsdev_add_pcie_port(pp, pdev);
> +	if (ret < 0)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, snpsdev_pcie);
> +
> +	return 0;
> +}
> +
> +static int snpsdev_pcie_rc_remove(struct platform_device *pdev)
> +{
> +	return 0;
> +}

This isn't referenced anywhere, so I think you could remove it.

> +static const struct of_device_id snpsdev_pcie_rc_of_match[] = {
> +	{ .compatible = "snps,pcie-snpsdev", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, snpsdev_pcie_rc_of_match);
> +
> +static struct platform_driver snpsdev_pcie_rc_driver = {
> +	.driver = {
> +		.name	= "pcie-snpsdev",
> +		.of_match_table = snpsdev_pcie_rc_of_match,
> +	},
> +	.probe = snpsdev_pcie_rc_probe,
> +};
> +
> +module_platform_driver(snpsdev_pcie_rc_driver);
> +
> +MODULE_AUTHOR("Manjunath Bettegowda <manjumb at synopsys.com>");
> +MODULE_DESCRIPTION("Platform Driver for Synopsys Device");

Should probably mention the PCI connection, e.g., "Synopsys PCIe host
controller driver".

> +MODULE_LICENSE("GPL v2");
> -- 
> 1.8.1.5

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
  2016-01-12 15:45   ` [PATCH v5 " Joao Pinto
@ 2016-01-29 20:40     ` Bjorn Helgaas
  -1 siblings, 0 replies; 51+ messages in thread
From: Bjorn Helgaas @ 2016-01-29 20:40 UTC (permalink / raw)
  To: Joao Pinto
  Cc: Vineet.Gupta1, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, arnd

Hi Joao,

Trivial nits below; I wouldn't even mention them if you weren't making
a few updates to the other patch.

On Thu, Jan 14, 2016 at 11:04:32AM +0000, Joao Pinto wrote:
> This patch adds PCI support to ARC and updates drivers/pci Makefile
> enabling the ARC arch to use the generic PCI setup functions.
> 
> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
> ---
> Change v5 -> v6 (Vineet Gupta):
> - Removed EXPORT_SYMBOL(pcibios_fixup_bus) from pcibios.c
> Change v4 -> v5 (Vineet Gupta):
> - pcibios.c unused functions, variables and includes were removed
> - ioremap.c has no need for changes (no patch is necessary)
> - pci.h unused functions and variables were removed
> Change v3 -> v4:
> - Nothing changed (just to keep up with patch set version).
> Change v2 -> v3 (Bjorn Helgaas):
> - arch/arc/kernel/pcibios.c unused functions were removed and also the
> arch/arc/include/asm/mach/pci.h was removed because was no longer necessary
> Change v1 -> v2:
> - In arch/arc/Kconfig, the new menu entry (Bus Configuration) was moved to the
> slot between sourcing of drivers/Kconfig and fs/Kconfig (Vineet Gupta)
> - In arch/arc/plat-axs10x/Kconfig the "select MIGHT_HAVE_PCI" option was placed
> in order as suggested (Vineet Gupta)
> - ioport_map() and ioport_unmap() were static inlined and included in
> the io.h (Vineet Gupta)
> - pcibios_min_io and pcibios_min_mem declaration moved to
> pcibios.c (Vineet Gupta)
> - pr_err() replaced by dev_err() in pcibios_enable_device() (Bjorn Helgaas)
> - string simplified in pcibios_enable_device() (Vineet Gupta)
> - pci_host_bridge_window structure was replaced by resource_entry structure, and
> list_for_each_entry() for resource_list_for_each_entry() in pcibios.c
> 
>  arch/arc/Kconfig             | 23 +++++++++++++++++++++++
>  arch/arc/include/asm/dma.h   |  5 +++++
>  arch/arc/include/asm/io.h    |  9 +++++++++
>  arch/arc/include/asm/pci.h   | 31 +++++++++++++++++++++++++++++++
>  arch/arc/kernel/Makefile     |  1 +
>  arch/arc/kernel/pcibios.c    | 23 ++++++++++++++++++++++++
>  arch/arc/plat-axs10x/Kconfig |  1 +
>  drivers/pci/Makefile         |  1 +
>  8 files changed, 95 insertions(+)
>  create mode 100644 arch/arc/include/asm/pci.h
>  create mode 100644 arch/arc/kernel/pcibios.c
> 
> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
> index 2c2ac3f..98b32c1 100644
> --- a/arch/arc/Kconfig
> +++ b/arch/arc/Kconfig
> @@ -19,6 +19,7 @@ config ARC
>  	select GENERIC_FIND_FIRST_BIT
>  	# for now, we don't need GENERIC_IRQ_PROBE, CONFIG_GENERIC_IRQ_CHIP
>  	select GENERIC_IRQ_SHOW
> +	select GENERIC_PCI_IOMAP
>  	select GENERIC_PENDING_IRQ if SMP
>  	select GENERIC_SMP_IDLE_THREAD
>  	select HAVE_ARCH_KGDB
> @@ -39,6 +40,9 @@ config ARC
>  	select PERF_USE_VMALLOC
>  	select HAVE_DEBUG_STACKOVERFLOW
>  
> +config MIGHT_HAVE_PCI
> +	bool
> +
>  config TRACE_IRQFLAGS_SUPPORT
>  	def_bool y
>  
> @@ -570,6 +574,25 @@ endmenu	 # "ARC Architecture Configuration"
>  source "mm/Kconfig"
>  source "net/Kconfig"
>  source "drivers/Kconfig"
> +
> +menu "Bus Support"
> +
> +config PCI
> +	bool "PCI support" if MIGHT_HAVE_PCI
> +	help
> +	  PCI is the name of a bus system, i.e. the way the CPU talks to the other stuff inside
> +	  your box.Find out if your board/platform have PCI.
> +	  Note: PCIE support for Synopsys Device will be available only when
> +	  HAPS DX is configured with PCIE RC bitmap. If you have PCI, say Y, otherwise N.
> +
> +config PCI_SYSCALL
> +	def_bool PCI
> +
> +source "drivers/pci/Kconfig"
> +source "drivers/pci/pcie/Kconfig"
> +
> +endmenu
> +
>  source "fs/Kconfig"
>  source "arch/arc/Kconfig.debug"
>  source "security/Kconfig"
> diff --git a/arch/arc/include/asm/dma.h b/arch/arc/include/asm/dma.h
> index ca7c451..37942fa 100644
> --- a/arch/arc/include/asm/dma.h
> +++ b/arch/arc/include/asm/dma.h
> @@ -10,5 +10,10 @@
>  #define ASM_ARC_DMA_H
>  
>  #define MAX_DMA_ADDRESS 0xC0000000
> +#ifdef CONFIG_PCI
> +extern int isa_dma_bridge_buggy;
> +#else
> +#define isa_dma_bridge_buggy    (0)

No parens needed around "0".

> +#endif
>  
>  #endif
> diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
> index 694ece8..947bf0c 100644
> --- a/arch/arc/include/asm/io.h
> +++ b/arch/arc/include/asm/io.h
> @@ -16,6 +16,15 @@
>  extern void __iomem *ioremap(unsigned long physaddr, unsigned long size);
>  extern void __iomem *ioremap_prot(phys_addr_t offset, unsigned long size,
>  				  unsigned long flags);
> +static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
> +{
> +	return (void __iomem *)port;
> +}
> +
> +static inline void ioport_unmap(void __iomem *addr)
> +{
> +}
> +
>  extern void iounmap(const void __iomem *addr);
>  
>  #define ioremap_nocache(phy, sz)	ioremap(phy, sz)
> diff --git a/arch/arc/include/asm/pci.h b/arch/arc/include/asm/pci.h
> new file mode 100644
> index 0000000..2f091e1
> --- /dev/null
> +++ b/arch/arc/include/asm/pci.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _ASM_ARC_PCI_H
> +#define _ASM_ARC_PCI_H
> +
> +#ifdef __KERNEL__
> +#include <asm-generic/pci-dma-compat.h>
> +#include <asm-generic/pci-bridge.h>
> +
> +#include <linux/ioport.h>
> +
> +#define PCIBIOS_MIN_IO 0x100
> +#define PCIBIOS_MIN_MEM 0x100000
> +
> +#define pcibios_assign_all_busses()	1
> +/*
> + * The PCI address space does equal the physical memory address space.
> + * The networking and block device layers use this boolean for bounce
> + * buffer decisions.
> + */
> +#define PCI_DMA_BUS_IS_PHYS     (1)

Or around "1".

> +
> +#endif /* __KERNEL__ */
> +
> +#endif /* _ASM_ARC_PCI_H */
> diff --git a/arch/arc/kernel/Makefile b/arch/arc/kernel/Makefile
> index e7f3625..1bc2036 100644
> --- a/arch/arc/kernel/Makefile
> +++ b/arch/arc/kernel/Makefile
> @@ -12,6 +12,7 @@ obj-y	:= arcksyms.o setup.o irq.o time.o reset.o ptrace.o process.o devtree.o
>  obj-y	+= signal.o traps.o sys.o troubleshoot.o stacktrace.o disasm.o clk.o
>  obj-$(CONFIG_ISA_ARCOMPACT)		+= entry-compact.o intc-compact.o
>  obj-$(CONFIG_ISA_ARCV2)			+= entry-arcv2.o intc-arcv2.o
> +obj-$(CONFIG_PCI)  			+= pcibios.o
>  
>  obj-$(CONFIG_MODULES)			+= arcksyms.o module.o
>  obj-$(CONFIG_SMP) 			+= smp.o
> diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
> new file mode 100644
> index 0000000..12ea45a
> --- /dev/null
> +++ b/arch/arc/kernel/pcibios.c
> @@ -0,0 +1,23 @@
> +/*
> + * Copyright (C) 2014-2015 Synopsys, Inc. (www.synopsys.com)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/pci.h>
> +
> +/*
> + * We don't have to worry about legacy ISA devices, so nothing to do here
> + */
> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> +				resource_size_t size, resource_size_t align)
> +{
> +	return res->start;
> +}
> +
> +void pcibios_fixup_bus(struct pci_bus *bus)
> +{
> +}
> diff --git a/arch/arc/plat-axs10x/Kconfig b/arch/arc/plat-axs10x/Kconfig
> index d475f9d..426ac4b 100644
> --- a/arch/arc/plat-axs10x/Kconfig
> +++ b/arch/arc/plat-axs10x/Kconfig
> @@ -11,6 +11,7 @@ menuconfig ARC_PLAT_AXS10X
>  	select DW_APB_ICTL
>  	select GPIO_DWAPB
>  	select OF_GPIO
> +	select MIGHT_HAVE_PCI
>  	select GENERIC_IRQ_CHIP
>  	select ARCH_REQUIRE_GPIOLIB
>  	help
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index be3f631..2154092 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
>  # Some architectures use the generic PCI setup functions
>  #
>  obj-$(CONFIG_ALPHA) += setup-irq.o
> +obj-$(CONFIG_ARC) += setup-irq.o
>  obj-$(CONFIG_ARM) += setup-irq.o
>  obj-$(CONFIG_ARM64) += setup-irq.o
>  obj-$(CONFIG_UNICORE32) += setup-irq.o
> -- 
> 1.8.1.5

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v6 1/2] PCI support added to ARC
@ 2016-01-29 20:40     ` Bjorn Helgaas
  0 siblings, 0 replies; 51+ messages in thread
From: Bjorn Helgaas @ 2016-01-29 20:40 UTC (permalink / raw)
  To: linux-snps-arc

Hi Joao,

Trivial nits below; I wouldn't even mention them if you weren't making
a few updates to the other patch.

On Thu, Jan 14, 2016@11:04:32AM +0000, Joao Pinto wrote:
> This patch adds PCI support to ARC and updates drivers/pci Makefile
> enabling the ARC arch to use the generic PCI setup functions.
> 
> Signed-off-by: Joao Pinto <jpinto at synopsys.com>
> ---
> Change v5 -> v6 (Vineet Gupta):
> - Removed EXPORT_SYMBOL(pcibios_fixup_bus) from pcibios.c
> Change v4 -> v5 (Vineet Gupta):
> - pcibios.c unused functions, variables and includes were removed
> - ioremap.c has no need for changes (no patch is necessary)
> - pci.h unused functions and variables were removed
> Change v3 -> v4:
> - Nothing changed (just to keep up with patch set version).
> Change v2 -> v3 (Bjorn Helgaas):
> - arch/arc/kernel/pcibios.c unused functions were removed and also the
> arch/arc/include/asm/mach/pci.h was removed because was no longer necessary
> Change v1 -> v2:
> - In arch/arc/Kconfig, the new menu entry (Bus Configuration) was moved to the
> slot between sourcing of drivers/Kconfig and fs/Kconfig (Vineet Gupta)
> - In arch/arc/plat-axs10x/Kconfig the "select MIGHT_HAVE_PCI" option was placed
> in order as suggested (Vineet Gupta)
> - ioport_map() and ioport_unmap() were static inlined and included in
> the io.h (Vineet Gupta)
> - pcibios_min_io and pcibios_min_mem declaration moved to
> pcibios.c (Vineet Gupta)
> - pr_err() replaced by dev_err() in pcibios_enable_device() (Bjorn Helgaas)
> - string simplified in pcibios_enable_device() (Vineet Gupta)
> - pci_host_bridge_window structure was replaced by resource_entry structure, and
> list_for_each_entry() for resource_list_for_each_entry() in pcibios.c
> 
>  arch/arc/Kconfig             | 23 +++++++++++++++++++++++
>  arch/arc/include/asm/dma.h   |  5 +++++
>  arch/arc/include/asm/io.h    |  9 +++++++++
>  arch/arc/include/asm/pci.h   | 31 +++++++++++++++++++++++++++++++
>  arch/arc/kernel/Makefile     |  1 +
>  arch/arc/kernel/pcibios.c    | 23 ++++++++++++++++++++++++
>  arch/arc/plat-axs10x/Kconfig |  1 +
>  drivers/pci/Makefile         |  1 +
>  8 files changed, 95 insertions(+)
>  create mode 100644 arch/arc/include/asm/pci.h
>  create mode 100644 arch/arc/kernel/pcibios.c
> 
> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
> index 2c2ac3f..98b32c1 100644
> --- a/arch/arc/Kconfig
> +++ b/arch/arc/Kconfig
> @@ -19,6 +19,7 @@ config ARC
>  	select GENERIC_FIND_FIRST_BIT
>  	# for now, we don't need GENERIC_IRQ_PROBE, CONFIG_GENERIC_IRQ_CHIP
>  	select GENERIC_IRQ_SHOW
> +	select GENERIC_PCI_IOMAP
>  	select GENERIC_PENDING_IRQ if SMP
>  	select GENERIC_SMP_IDLE_THREAD
>  	select HAVE_ARCH_KGDB
> @@ -39,6 +40,9 @@ config ARC
>  	select PERF_USE_VMALLOC
>  	select HAVE_DEBUG_STACKOVERFLOW
>  
> +config MIGHT_HAVE_PCI
> +	bool
> +
>  config TRACE_IRQFLAGS_SUPPORT
>  	def_bool y
>  
> @@ -570,6 +574,25 @@ endmenu	 # "ARC Architecture Configuration"
>  source "mm/Kconfig"
>  source "net/Kconfig"
>  source "drivers/Kconfig"
> +
> +menu "Bus Support"
> +
> +config PCI
> +	bool "PCI support" if MIGHT_HAVE_PCI
> +	help
> +	  PCI is the name of a bus system, i.e. the way the CPU talks to the other stuff inside
> +	  your box.Find out if your board/platform have PCI.
> +	  Note: PCIE support for Synopsys Device will be available only when
> +	  HAPS DX is configured with PCIE RC bitmap. If you have PCI, say Y, otherwise N.
> +
> +config PCI_SYSCALL
> +	def_bool PCI
> +
> +source "drivers/pci/Kconfig"
> +source "drivers/pci/pcie/Kconfig"
> +
> +endmenu
> +
>  source "fs/Kconfig"
>  source "arch/arc/Kconfig.debug"
>  source "security/Kconfig"
> diff --git a/arch/arc/include/asm/dma.h b/arch/arc/include/asm/dma.h
> index ca7c451..37942fa 100644
> --- a/arch/arc/include/asm/dma.h
> +++ b/arch/arc/include/asm/dma.h
> @@ -10,5 +10,10 @@
>  #define ASM_ARC_DMA_H
>  
>  #define MAX_DMA_ADDRESS 0xC0000000
> +#ifdef CONFIG_PCI
> +extern int isa_dma_bridge_buggy;
> +#else
> +#define isa_dma_bridge_buggy    (0)

No parens needed around "0".

> +#endif
>  
>  #endif
> diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
> index 694ece8..947bf0c 100644
> --- a/arch/arc/include/asm/io.h
> +++ b/arch/arc/include/asm/io.h
> @@ -16,6 +16,15 @@
>  extern void __iomem *ioremap(unsigned long physaddr, unsigned long size);
>  extern void __iomem *ioremap_prot(phys_addr_t offset, unsigned long size,
>  				  unsigned long flags);
> +static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
> +{
> +	return (void __iomem *)port;
> +}
> +
> +static inline void ioport_unmap(void __iomem *addr)
> +{
> +}
> +
>  extern void iounmap(const void __iomem *addr);
>  
>  #define ioremap_nocache(phy, sz)	ioremap(phy, sz)
> diff --git a/arch/arc/include/asm/pci.h b/arch/arc/include/asm/pci.h
> new file mode 100644
> index 0000000..2f091e1
> --- /dev/null
> +++ b/arch/arc/include/asm/pci.h
> @@ -0,0 +1,31 @@
> +/*
> + * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _ASM_ARC_PCI_H
> +#define _ASM_ARC_PCI_H
> +
> +#ifdef __KERNEL__
> +#include <asm-generic/pci-dma-compat.h>
> +#include <asm-generic/pci-bridge.h>
> +
> +#include <linux/ioport.h>
> +
> +#define PCIBIOS_MIN_IO 0x100
> +#define PCIBIOS_MIN_MEM 0x100000
> +
> +#define pcibios_assign_all_busses()	1
> +/*
> + * The PCI address space does equal the physical memory address space.
> + * The networking and block device layers use this boolean for bounce
> + * buffer decisions.
> + */
> +#define PCI_DMA_BUS_IS_PHYS     (1)

Or around "1".

> +
> +#endif /* __KERNEL__ */
> +
> +#endif /* _ASM_ARC_PCI_H */
> diff --git a/arch/arc/kernel/Makefile b/arch/arc/kernel/Makefile
> index e7f3625..1bc2036 100644
> --- a/arch/arc/kernel/Makefile
> +++ b/arch/arc/kernel/Makefile
> @@ -12,6 +12,7 @@ obj-y	:= arcksyms.o setup.o irq.o time.o reset.o ptrace.o process.o devtree.o
>  obj-y	+= signal.o traps.o sys.o troubleshoot.o stacktrace.o disasm.o clk.o
>  obj-$(CONFIG_ISA_ARCOMPACT)		+= entry-compact.o intc-compact.o
>  obj-$(CONFIG_ISA_ARCV2)			+= entry-arcv2.o intc-arcv2.o
> +obj-$(CONFIG_PCI)  			+= pcibios.o
>  
>  obj-$(CONFIG_MODULES)			+= arcksyms.o module.o
>  obj-$(CONFIG_SMP) 			+= smp.o
> diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
> new file mode 100644
> index 0000000..12ea45a
> --- /dev/null
> +++ b/arch/arc/kernel/pcibios.c
> @@ -0,0 +1,23 @@
> +/*
> + * Copyright (C) 2014-2015 Synopsys, Inc. (www.synopsys.com)
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + */
> +
> +#include <linux/pci.h>
> +
> +/*
> + * We don't have to worry about legacy ISA devices, so nothing to do here
> + */
> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> +				resource_size_t size, resource_size_t align)
> +{
> +	return res->start;
> +}
> +
> +void pcibios_fixup_bus(struct pci_bus *bus)
> +{
> +}
> diff --git a/arch/arc/plat-axs10x/Kconfig b/arch/arc/plat-axs10x/Kconfig
> index d475f9d..426ac4b 100644
> --- a/arch/arc/plat-axs10x/Kconfig
> +++ b/arch/arc/plat-axs10x/Kconfig
> @@ -11,6 +11,7 @@ menuconfig ARC_PLAT_AXS10X
>  	select DW_APB_ICTL
>  	select GPIO_DWAPB
>  	select OF_GPIO
> +	select MIGHT_HAVE_PCI
>  	select GENERIC_IRQ_CHIP
>  	select ARCH_REQUIRE_GPIOLIB
>  	help
> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
> index be3f631..2154092 100644
> --- a/drivers/pci/Makefile
> +++ b/drivers/pci/Makefile
> @@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
>  # Some architectures use the generic PCI setup functions
>  #
>  obj-$(CONFIG_ALPHA) += setup-irq.o
> +obj-$(CONFIG_ARC) += setup-irq.o
>  obj-$(CONFIG_ARM) += setup-irq.o
>  obj-$(CONFIG_ARM64) += setup-irq.o
>  obj-$(CONFIG_UNICORE32) += setup-irq.o
> -- 
> 1.8.1.5

^ permalink raw reply	[flat|nested] 51+ messages in thread

* Re: [PATCH v6 1/2] PCI support added to ARC
  2016-01-29 20:40     ` Bjorn Helgaas
@ 2016-02-01  9:33       ` Joao Pinto
  -1 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-02-01  9:33 UTC (permalink / raw)
  To: Bjorn Helgaas, Joao Pinto
  Cc: Vineet.Gupta1, linux-pci, linux-kernel, linux-snps-arc,
	CARLOS.PALMINHA, Alexey.Brodkin, robh+dt, pawel.moll,
	mark.rutland, ijc+devicetree, galak, arnd


Hi Bjorn,

Thank you for the review. I am going to update the needed points and send a new
patch version until tomorrow.

Joao

On 1/29/2016 8:40 PM, Bjorn Helgaas wrote:
> Hi Joao,
> 
> Trivial nits below; I wouldn't even mention them if you weren't making
> a few updates to the other patch.
> 
> On Thu, Jan 14, 2016 at 11:04:32AM +0000, Joao Pinto wrote:
>> This patch adds PCI support to ARC and updates drivers/pci Makefile
>> enabling the ARC arch to use the generic PCI setup functions.
>>
>> Signed-off-by: Joao Pinto <jpinto@synopsys.com>
>> ---
>> Change v5 -> v6 (Vineet Gupta):
>> - Removed EXPORT_SYMBOL(pcibios_fixup_bus) from pcibios.c
>> Change v4 -> v5 (Vineet Gupta):
>> - pcibios.c unused functions, variables and includes were removed
>> - ioremap.c has no need for changes (no patch is necessary)
>> - pci.h unused functions and variables were removed
>> Change v3 -> v4:
>> - Nothing changed (just to keep up with patch set version).
>> Change v2 -> v3 (Bjorn Helgaas):
>> - arch/arc/kernel/pcibios.c unused functions were removed and also the
>> arch/arc/include/asm/mach/pci.h was removed because was no longer necessary
>> Change v1 -> v2:
>> - In arch/arc/Kconfig, the new menu entry (Bus Configuration) was moved to the
>> slot between sourcing of drivers/Kconfig and fs/Kconfig (Vineet Gupta)
>> - In arch/arc/plat-axs10x/Kconfig the "select MIGHT_HAVE_PCI" option was placed
>> in order as suggested (Vineet Gupta)
>> - ioport_map() and ioport_unmap() were static inlined and included in
>> the io.h (Vineet Gupta)
>> - pcibios_min_io and pcibios_min_mem declaration moved to
>> pcibios.c (Vineet Gupta)
>> - pr_err() replaced by dev_err() in pcibios_enable_device() (Bjorn Helgaas)
>> - string simplified in pcibios_enable_device() (Vineet Gupta)
>> - pci_host_bridge_window structure was replaced by resource_entry structure, and
>> list_for_each_entry() for resource_list_for_each_entry() in pcibios.c
>>
>>  arch/arc/Kconfig             | 23 +++++++++++++++++++++++
>>  arch/arc/include/asm/dma.h   |  5 +++++
>>  arch/arc/include/asm/io.h    |  9 +++++++++
>>  arch/arc/include/asm/pci.h   | 31 +++++++++++++++++++++++++++++++
>>  arch/arc/kernel/Makefile     |  1 +
>>  arch/arc/kernel/pcibios.c    | 23 ++++++++++++++++++++++++
>>  arch/arc/plat-axs10x/Kconfig |  1 +
>>  drivers/pci/Makefile         |  1 +
>>  8 files changed, 95 insertions(+)
>>  create mode 100644 arch/arc/include/asm/pci.h
>>  create mode 100644 arch/arc/kernel/pcibios.c
>>
>> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
>> index 2c2ac3f..98b32c1 100644
>> --- a/arch/arc/Kconfig
>> +++ b/arch/arc/Kconfig
>> @@ -19,6 +19,7 @@ config ARC
>>  	select GENERIC_FIND_FIRST_BIT
>>  	# for now, we don't need GENERIC_IRQ_PROBE, CONFIG_GENERIC_IRQ_CHIP
>>  	select GENERIC_IRQ_SHOW
>> +	select GENERIC_PCI_IOMAP
>>  	select GENERIC_PENDING_IRQ if SMP
>>  	select GENERIC_SMP_IDLE_THREAD
>>  	select HAVE_ARCH_KGDB
>> @@ -39,6 +40,9 @@ config ARC
>>  	select PERF_USE_VMALLOC
>>  	select HAVE_DEBUG_STACKOVERFLOW
>>  
>> +config MIGHT_HAVE_PCI
>> +	bool
>> +
>>  config TRACE_IRQFLAGS_SUPPORT
>>  	def_bool y
>>  
>> @@ -570,6 +574,25 @@ endmenu	 # "ARC Architecture Configuration"
>>  source "mm/Kconfig"
>>  source "net/Kconfig"
>>  source "drivers/Kconfig"
>> +
>> +menu "Bus Support"
>> +
>> +config PCI
>> +	bool "PCI support" if MIGHT_HAVE_PCI
>> +	help
>> +	  PCI is the name of a bus system, i.e. the way the CPU talks to the other stuff inside
>> +	  your box.Find out if your board/platform have PCI.
>> +	  Note: PCIE support for Synopsys Device will be available only when
>> +	  HAPS DX is configured with PCIE RC bitmap. If you have PCI, say Y, otherwise N.
>> +
>> +config PCI_SYSCALL
>> +	def_bool PCI
>> +
>> +source "drivers/pci/Kconfig"
>> +source "drivers/pci/pcie/Kconfig"
>> +
>> +endmenu
>> +
>>  source "fs/Kconfig"
>>  source "arch/arc/Kconfig.debug"
>>  source "security/Kconfig"
>> diff --git a/arch/arc/include/asm/dma.h b/arch/arc/include/asm/dma.h
>> index ca7c451..37942fa 100644
>> --- a/arch/arc/include/asm/dma.h
>> +++ b/arch/arc/include/asm/dma.h
>> @@ -10,5 +10,10 @@
>>  #define ASM_ARC_DMA_H
>>  
>>  #define MAX_DMA_ADDRESS 0xC0000000
>> +#ifdef CONFIG_PCI
>> +extern int isa_dma_bridge_buggy;
>> +#else
>> +#define isa_dma_bridge_buggy    (0)
> 
> No parens needed around "0".
> 
>> +#endif
>>  
>>  #endif
>> diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
>> index 694ece8..947bf0c 100644
>> --- a/arch/arc/include/asm/io.h
>> +++ b/arch/arc/include/asm/io.h
>> @@ -16,6 +16,15 @@
>>  extern void __iomem *ioremap(unsigned long physaddr, unsigned long size);
>>  extern void __iomem *ioremap_prot(phys_addr_t offset, unsigned long size,
>>  				  unsigned long flags);
>> +static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
>> +{
>> +	return (void __iomem *)port;
>> +}
>> +
>> +static inline void ioport_unmap(void __iomem *addr)
>> +{
>> +}
>> +
>>  extern void iounmap(const void __iomem *addr);
>>  
>>  #define ioremap_nocache(phy, sz)	ioremap(phy, sz)
>> diff --git a/arch/arc/include/asm/pci.h b/arch/arc/include/asm/pci.h
>> new file mode 100644
>> index 0000000..2f091e1
>> --- /dev/null
>> +++ b/arch/arc/include/asm/pci.h
>> @@ -0,0 +1,31 @@
>> +/*
>> + * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef _ASM_ARC_PCI_H
>> +#define _ASM_ARC_PCI_H
>> +
>> +#ifdef __KERNEL__
>> +#include <asm-generic/pci-dma-compat.h>
>> +#include <asm-generic/pci-bridge.h>
>> +
>> +#include <linux/ioport.h>
>> +
>> +#define PCIBIOS_MIN_IO 0x100
>> +#define PCIBIOS_MIN_MEM 0x100000
>> +
>> +#define pcibios_assign_all_busses()	1
>> +/*
>> + * The PCI address space does equal the physical memory address space.
>> + * The networking and block device layers use this boolean for bounce
>> + * buffer decisions.
>> + */
>> +#define PCI_DMA_BUS_IS_PHYS     (1)
> 
> Or around "1".
> 
>> +
>> +#endif /* __KERNEL__ */
>> +
>> +#endif /* _ASM_ARC_PCI_H */
>> diff --git a/arch/arc/kernel/Makefile b/arch/arc/kernel/Makefile
>> index e7f3625..1bc2036 100644
>> --- a/arch/arc/kernel/Makefile
>> +++ b/arch/arc/kernel/Makefile
>> @@ -12,6 +12,7 @@ obj-y	:= arcksyms.o setup.o irq.o time.o reset.o ptrace.o process.o devtree.o
>>  obj-y	+= signal.o traps.o sys.o troubleshoot.o stacktrace.o disasm.o clk.o
>>  obj-$(CONFIG_ISA_ARCOMPACT)		+= entry-compact.o intc-compact.o
>>  obj-$(CONFIG_ISA_ARCV2)			+= entry-arcv2.o intc-arcv2.o
>> +obj-$(CONFIG_PCI)  			+= pcibios.o
>>  
>>  obj-$(CONFIG_MODULES)			+= arcksyms.o module.o
>>  obj-$(CONFIG_SMP) 			+= smp.o
>> diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
>> new file mode 100644
>> index 0000000..12ea45a
>> --- /dev/null
>> +++ b/arch/arc/kernel/pcibios.c
>> @@ -0,0 +1,23 @@
>> +/*
>> + * Copyright (C) 2014-2015 Synopsys, Inc. (www.synopsys.com)
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <linux/pci.h>
>> +
>> +/*
>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>> + */
>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>> +				resource_size_t size, resource_size_t align)
>> +{
>> +	return res->start;
>> +}
>> +
>> +void pcibios_fixup_bus(struct pci_bus *bus)
>> +{
>> +}
>> diff --git a/arch/arc/plat-axs10x/Kconfig b/arch/arc/plat-axs10x/Kconfig
>> index d475f9d..426ac4b 100644
>> --- a/arch/arc/plat-axs10x/Kconfig
>> +++ b/arch/arc/plat-axs10x/Kconfig
>> @@ -11,6 +11,7 @@ menuconfig ARC_PLAT_AXS10X
>>  	select DW_APB_ICTL
>>  	select GPIO_DWAPB
>>  	select OF_GPIO
>> +	select MIGHT_HAVE_PCI
>>  	select GENERIC_IRQ_CHIP
>>  	select ARCH_REQUIRE_GPIOLIB
>>  	help
>> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
>> index be3f631..2154092 100644
>> --- a/drivers/pci/Makefile
>> +++ b/drivers/pci/Makefile
>> @@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
>>  # Some architectures use the generic PCI setup functions
>>  #
>>  obj-$(CONFIG_ALPHA) += setup-irq.o
>> +obj-$(CONFIG_ARC) += setup-irq.o
>>  obj-$(CONFIG_ARM) += setup-irq.o
>>  obj-$(CONFIG_ARM64) += setup-irq.o
>>  obj-$(CONFIG_UNICORE32) += setup-irq.o
>> -- 
>> 1.8.1.5

^ permalink raw reply	[flat|nested] 51+ messages in thread

* [PATCH v6 1/2] PCI support added to ARC
@ 2016-02-01  9:33       ` Joao Pinto
  0 siblings, 0 replies; 51+ messages in thread
From: Joao Pinto @ 2016-02-01  9:33 UTC (permalink / raw)
  To: linux-snps-arc


Hi Bjorn,

Thank you for the review. I am going to update the needed points and send a new
patch version until tomorrow.

Joao

On 1/29/2016 8:40 PM, Bjorn Helgaas wrote:
> Hi Joao,
> 
> Trivial nits below; I wouldn't even mention them if you weren't making
> a few updates to the other patch.
> 
> On Thu, Jan 14, 2016@11:04:32AM +0000, Joao Pinto wrote:
>> This patch adds PCI support to ARC and updates drivers/pci Makefile
>> enabling the ARC arch to use the generic PCI setup functions.
>>
>> Signed-off-by: Joao Pinto <jpinto at synopsys.com>
>> ---
>> Change v5 -> v6 (Vineet Gupta):
>> - Removed EXPORT_SYMBOL(pcibios_fixup_bus) from pcibios.c
>> Change v4 -> v5 (Vineet Gupta):
>> - pcibios.c unused functions, variables and includes were removed
>> - ioremap.c has no need for changes (no patch is necessary)
>> - pci.h unused functions and variables were removed
>> Change v3 -> v4:
>> - Nothing changed (just to keep up with patch set version).
>> Change v2 -> v3 (Bjorn Helgaas):
>> - arch/arc/kernel/pcibios.c unused functions were removed and also the
>> arch/arc/include/asm/mach/pci.h was removed because was no longer necessary
>> Change v1 -> v2:
>> - In arch/arc/Kconfig, the new menu entry (Bus Configuration) was moved to the
>> slot between sourcing of drivers/Kconfig and fs/Kconfig (Vineet Gupta)
>> - In arch/arc/plat-axs10x/Kconfig the "select MIGHT_HAVE_PCI" option was placed
>> in order as suggested (Vineet Gupta)
>> - ioport_map() and ioport_unmap() were static inlined and included in
>> the io.h (Vineet Gupta)
>> - pcibios_min_io and pcibios_min_mem declaration moved to
>> pcibios.c (Vineet Gupta)
>> - pr_err() replaced by dev_err() in pcibios_enable_device() (Bjorn Helgaas)
>> - string simplified in pcibios_enable_device() (Vineet Gupta)
>> - pci_host_bridge_window structure was replaced by resource_entry structure, and
>> list_for_each_entry() for resource_list_for_each_entry() in pcibios.c
>>
>>  arch/arc/Kconfig             | 23 +++++++++++++++++++++++
>>  arch/arc/include/asm/dma.h   |  5 +++++
>>  arch/arc/include/asm/io.h    |  9 +++++++++
>>  arch/arc/include/asm/pci.h   | 31 +++++++++++++++++++++++++++++++
>>  arch/arc/kernel/Makefile     |  1 +
>>  arch/arc/kernel/pcibios.c    | 23 ++++++++++++++++++++++++
>>  arch/arc/plat-axs10x/Kconfig |  1 +
>>  drivers/pci/Makefile         |  1 +
>>  8 files changed, 95 insertions(+)
>>  create mode 100644 arch/arc/include/asm/pci.h
>>  create mode 100644 arch/arc/kernel/pcibios.c
>>
>> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
>> index 2c2ac3f..98b32c1 100644
>> --- a/arch/arc/Kconfig
>> +++ b/arch/arc/Kconfig
>> @@ -19,6 +19,7 @@ config ARC
>>  	select GENERIC_FIND_FIRST_BIT
>>  	# for now, we don't need GENERIC_IRQ_PROBE, CONFIG_GENERIC_IRQ_CHIP
>>  	select GENERIC_IRQ_SHOW
>> +	select GENERIC_PCI_IOMAP
>>  	select GENERIC_PENDING_IRQ if SMP
>>  	select GENERIC_SMP_IDLE_THREAD
>>  	select HAVE_ARCH_KGDB
>> @@ -39,6 +40,9 @@ config ARC
>>  	select PERF_USE_VMALLOC
>>  	select HAVE_DEBUG_STACKOVERFLOW
>>  
>> +config MIGHT_HAVE_PCI
>> +	bool
>> +
>>  config TRACE_IRQFLAGS_SUPPORT
>>  	def_bool y
>>  
>> @@ -570,6 +574,25 @@ endmenu	 # "ARC Architecture Configuration"
>>  source "mm/Kconfig"
>>  source "net/Kconfig"
>>  source "drivers/Kconfig"
>> +
>> +menu "Bus Support"
>> +
>> +config PCI
>> +	bool "PCI support" if MIGHT_HAVE_PCI
>> +	help
>> +	  PCI is the name of a bus system, i.e. the way the CPU talks to the other stuff inside
>> +	  your box.Find out if your board/platform have PCI.
>> +	  Note: PCIE support for Synopsys Device will be available only when
>> +	  HAPS DX is configured with PCIE RC bitmap. If you have PCI, say Y, otherwise N.
>> +
>> +config PCI_SYSCALL
>> +	def_bool PCI
>> +
>> +source "drivers/pci/Kconfig"
>> +source "drivers/pci/pcie/Kconfig"
>> +
>> +endmenu
>> +
>>  source "fs/Kconfig"
>>  source "arch/arc/Kconfig.debug"
>>  source "security/Kconfig"
>> diff --git a/arch/arc/include/asm/dma.h b/arch/arc/include/asm/dma.h
>> index ca7c451..37942fa 100644
>> --- a/arch/arc/include/asm/dma.h
>> +++ b/arch/arc/include/asm/dma.h
>> @@ -10,5 +10,10 @@
>>  #define ASM_ARC_DMA_H
>>  
>>  #define MAX_DMA_ADDRESS 0xC0000000
>> +#ifdef CONFIG_PCI
>> +extern int isa_dma_bridge_buggy;
>> +#else
>> +#define isa_dma_bridge_buggy    (0)
> 
> No parens needed around "0".
> 
>> +#endif
>>  
>>  #endif
>> diff --git a/arch/arc/include/asm/io.h b/arch/arc/include/asm/io.h
>> index 694ece8..947bf0c 100644
>> --- a/arch/arc/include/asm/io.h
>> +++ b/arch/arc/include/asm/io.h
>> @@ -16,6 +16,15 @@
>>  extern void __iomem *ioremap(unsigned long physaddr, unsigned long size);
>>  extern void __iomem *ioremap_prot(phys_addr_t offset, unsigned long size,
>>  				  unsigned long flags);
>> +static inline void __iomem *ioport_map(unsigned long port, unsigned int nr)
>> +{
>> +	return (void __iomem *)port;
>> +}
>> +
>> +static inline void ioport_unmap(void __iomem *addr)
>> +{
>> +}
>> +
>>  extern void iounmap(const void __iomem *addr);
>>  
>>  #define ioremap_nocache(phy, sz)	ioremap(phy, sz)
>> diff --git a/arch/arc/include/asm/pci.h b/arch/arc/include/asm/pci.h
>> new file mode 100644
>> index 0000000..2f091e1
>> --- /dev/null
>> +++ b/arch/arc/include/asm/pci.h
>> @@ -0,0 +1,31 @@
>> +/*
>> + * Copyright (C) 2015-2016 Synopsys, Inc. (www.synopsys.com)
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#ifndef _ASM_ARC_PCI_H
>> +#define _ASM_ARC_PCI_H
>> +
>> +#ifdef __KERNEL__
>> +#include <asm-generic/pci-dma-compat.h>
>> +#include <asm-generic/pci-bridge.h>
>> +
>> +#include <linux/ioport.h>
>> +
>> +#define PCIBIOS_MIN_IO 0x100
>> +#define PCIBIOS_MIN_MEM 0x100000
>> +
>> +#define pcibios_assign_all_busses()	1
>> +/*
>> + * The PCI address space does equal the physical memory address space.
>> + * The networking and block device layers use this boolean for bounce
>> + * buffer decisions.
>> + */
>> +#define PCI_DMA_BUS_IS_PHYS     (1)
> 
> Or around "1".
> 
>> +
>> +#endif /* __KERNEL__ */
>> +
>> +#endif /* _ASM_ARC_PCI_H */
>> diff --git a/arch/arc/kernel/Makefile b/arch/arc/kernel/Makefile
>> index e7f3625..1bc2036 100644
>> --- a/arch/arc/kernel/Makefile
>> +++ b/arch/arc/kernel/Makefile
>> @@ -12,6 +12,7 @@ obj-y	:= arcksyms.o setup.o irq.o time.o reset.o ptrace.o process.o devtree.o
>>  obj-y	+= signal.o traps.o sys.o troubleshoot.o stacktrace.o disasm.o clk.o
>>  obj-$(CONFIG_ISA_ARCOMPACT)		+= entry-compact.o intc-compact.o
>>  obj-$(CONFIG_ISA_ARCV2)			+= entry-arcv2.o intc-arcv2.o
>> +obj-$(CONFIG_PCI)  			+= pcibios.o
>>  
>>  obj-$(CONFIG_MODULES)			+= arcksyms.o module.o
>>  obj-$(CONFIG_SMP) 			+= smp.o
>> diff --git a/arch/arc/kernel/pcibios.c b/arch/arc/kernel/pcibios.c
>> new file mode 100644
>> index 0000000..12ea45a
>> --- /dev/null
>> +++ b/arch/arc/kernel/pcibios.c
>> @@ -0,0 +1,23 @@
>> +/*
>> + * Copyright (C) 2014-2015 Synopsys, Inc. (www.synopsys.com)
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + */
>> +
>> +#include <linux/pci.h>
>> +
>> +/*
>> + * We don't have to worry about legacy ISA devices, so nothing to do here
>> + */
>> +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>> +				resource_size_t size, resource_size_t align)
>> +{
>> +	return res->start;
>> +}
>> +
>> +void pcibios_fixup_bus(struct pci_bus *bus)
>> +{
>> +}
>> diff --git a/arch/arc/plat-axs10x/Kconfig b/arch/arc/plat-axs10x/Kconfig
>> index d475f9d..426ac4b 100644
>> --- a/arch/arc/plat-axs10x/Kconfig
>> +++ b/arch/arc/plat-axs10x/Kconfig
>> @@ -11,6 +11,7 @@ menuconfig ARC_PLAT_AXS10X
>>  	select DW_APB_ICTL
>>  	select GPIO_DWAPB
>>  	select OF_GPIO
>> +	select MIGHT_HAVE_PCI
>>  	select GENERIC_IRQ_CHIP
>>  	select ARCH_REQUIRE_GPIOLIB
>>  	help
>> diff --git a/drivers/pci/Makefile b/drivers/pci/Makefile
>> index be3f631..2154092 100644
>> --- a/drivers/pci/Makefile
>> +++ b/drivers/pci/Makefile
>> @@ -32,6 +32,7 @@ obj-$(CONFIG_PCI_IOV) += iov.o
>>  # Some architectures use the generic PCI setup functions
>>  #
>>  obj-$(CONFIG_ALPHA) += setup-irq.o
>> +obj-$(CONFIG_ARC) += setup-irq.o
>>  obj-$(CONFIG_ARM) += setup-irq.o
>>  obj-$(CONFIG_ARM64) += setup-irq.o
>>  obj-$(CONFIG_UNICORE32) += setup-irq.o
>> -- 
>> 1.8.1.5

^ permalink raw reply	[flat|nested] 51+ messages in thread

end of thread, other threads:[~2016-02-01  9:33 UTC | newest]

Thread overview: 51+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-12 15:45 [PATCH v5 0/2] adding PCI support to AXS10x Joao Pinto
2016-01-14 11:04 ` [PATCH v6 " Joao Pinto
2016-01-12 15:45 ` [PATCH v5 " Joao Pinto
2016-01-12 15:45 ` [PATCH v5 1/2] PCI support added to ARC Joao Pinto
2016-01-14 11:04   ` [PATCH v6 " Joao Pinto
2016-01-12 15:45   ` [PATCH v5 " Joao Pinto
2016-01-14  5:26   ` Vineet Gupta
2016-01-14  5:26     ` Vineet Gupta
2016-01-14  5:26     ` Vineet Gupta
2016-01-14 10:22     ` Arnd Bergmann
2016-01-14 10:22       ` Arnd Bergmann
2016-01-14 10:22       ` Arnd Bergmann
2016-01-14 10:42       ` Joao Pinto
2016-01-14 10:42         ` Joao Pinto
2016-01-14 10:42         ` Joao Pinto
2016-01-14 10:51         ` Vineet Gupta
2016-01-14 10:51           ` Vineet Gupta
2016-01-14 10:51           ` Vineet Gupta
2016-01-14 10:54           ` Joao Pinto
2016-01-14 10:54             ` Joao Pinto
2016-01-14 10:54             ` Joao Pinto
2016-01-14 11:59           ` Arnd Bergmann
2016-01-14 11:59             ` Arnd Bergmann
2016-01-14 11:59             ` Arnd Bergmann
2016-01-14 12:11             ` Vineet Gupta
2016-01-14 12:11               ` Vineet Gupta
2016-01-14 12:11               ` Vineet Gupta
2016-01-14 11:11   ` [PATCH v6 " Vineet Gupta
2016-01-14 11:11     ` Vineet Gupta
2016-01-14 11:11     ` Vineet Gupta
2016-01-18 11:30     ` Joao Pinto
2016-01-18 11:30       ` Joao Pinto
2016-01-18 11:30       ` Joao Pinto
2016-01-27 14:29     ` Joao Pinto
2016-01-27 14:29       ` Joao Pinto
2016-01-27 14:29       ` Joao Pinto
2016-01-27 21:59       ` Bjorn Helgaas
2016-01-27 21:59         ` Bjorn Helgaas
2016-01-27 21:59         ` Bjorn Helgaas
2016-01-28 17:00         ` Joao Pinto
2016-01-28 17:00           ` Joao Pinto
2016-01-28 17:00           ` Joao Pinto
2016-01-29 20:40   ` Bjorn Helgaas
2016-01-29 20:40     ` Bjorn Helgaas
2016-02-01  9:33     ` Joao Pinto
2016-02-01  9:33       ` Joao Pinto
2016-01-12 15:45 ` [PATCH v5 2/2] add new platform driver for PCI RC Joao Pinto
2016-01-14 11:04   ` [PATCH v6 " Joao Pinto
2016-01-12 15:45   ` [PATCH v5 " Joao Pinto
2016-01-29 20:36   ` [PATCH v6 " Bjorn Helgaas
2016-01-29 20:36     ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.