All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
@ 2015-07-21  6:48 ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni, lorenzo.pieralisi, James Morse, Liviu.Dudau
  Cc: linux-pci, linux-arm-kernel, devicetree, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liudongdong3, qiujiang, kangfenglong,
	liguozhu, Zhou Wang

This patchset adds PCIe host support for Hisilicon Soc Hip05. The PCIe hosts
use PCIe IP core from Synopsys, So this driver is base on designware PCIe driver.

Hip05 is an ARMv8 architecture Soc. It should be able to use ARM64 PCIe API in
designeware PCIe driver. So this patch also adds ARM64 support for designware
pcie.

This patchset is based on v4.2-rc1 and Gabriele's patch about of_pci_range
fix[1].

Change from v3:
- Change 1/5 to what Gabriele suggested.
- Use win->__res.start to get *_mod_base in 2/5, this fix a bug in v3 series.

Change from v2:
- Move struct pci_dev *dev and struct pci_sys_data *sys in
  pcibios_align_resource in 1/5.
- Add Gabriele's codes in 2/5 which delete unnecessary information parse and
  use of_pci_get_host_bridge_resources for both ARM32 and ARM64.
- Add maintainer patch 5/5.

Change from RFC v1:
- Add 1/4 patch by Arnd which removes align_resource callback in ARM
  pcibios_align_resource.
- Change head file in pcie-designware from asm/hardirq.h to linux/hardirq.h.
- Set pp->root_bus_nr = 0 in dra7xx, exynos, imx6, keystone, layerscape,
  spear13xx.
- Remove unnecessary parentheses of some macros in pcie-hisi.
- Use macro to replace some magic values.
- Merge two loops together and add some comments about it in context_config
  function in pcie-hisi.
- Modify some value of items in pcie node example in binding document. 

Change from RFC:
- delete dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct hw_pci,
  merge related operations into dw_pcie_host_init.

Link of v3:
- http://www.spinics.net/lists/linux-pci/msg42539.html
Link of v2:
- http://www.spinics.net/lists/linux-pci/msg41844.html
Link of RFC v1:
- http://www.spinics.net/lists/linux-pci/msg41305.html
Link of RFC:
- http://www.spinics.net/lists/linux-pci/msg40434.html

[1] https://patchwork.ozlabs.org/patch/495018/

Zhou Wang (5):
  ARM/PCI: remove align_resource in pci_sys_data
  PCI: designware: Add ARM64 support
  PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
  Documentation: DT: Add Hisilicon PCIe host binding
  MAINTAINERS: Add pcie-hisi maintainer

 .../devicetree/bindings/pci/hisilicon-pcie.txt     |  46 ++++
 MAINTAINERS                                        |   7 +
 arch/arm/include/asm/mach/pci.h                    |   5 -
 arch/arm/kernel/bios32.c                           |  12 +-
 drivers/pci/host/Kconfig                           |   5 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-dra7xx.c                      |   1 +
 drivers/pci/host/pci-exynos.c                      |   2 +-
 drivers/pci/host/pci-imx6.c                        |   2 +-
 drivers/pci/host/pci-keystone-dw.c                 |   2 +-
 drivers/pci/host/pci-keystone.c                    |   2 +-
 drivers/pci/host/pci-layerscape.c                  |   2 +-
 drivers/pci/host/pcie-designware.c                 | 217 ++++++-----------
 drivers/pci/host/pcie-designware.h                 |  10 +-
 drivers/pci/host/pcie-hisi.c                       | 257 +++++++++++++++++++++
 drivers/pci/host/pcie-spear13xx.c                  |   2 +-
 16 files changed, 410 insertions(+), 163 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
 create mode 100644 drivers/pci/host/pcie-hisi.c

-- 
1.9.1

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

* [PATCH v4 0/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
@ 2015-07-21  6:48 ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: linux-arm-kernel

This patchset adds PCIe host support for Hisilicon Soc Hip05. The PCIe hosts
use PCIe IP core from Synopsys, So this driver is base on designware PCIe driver.

Hip05 is an ARMv8 architecture Soc. It should be able to use ARM64 PCIe API in
designeware PCIe driver. So this patch also adds ARM64 support for designware
pcie.

This patchset is based on v4.2-rc1 and Gabriele's patch about of_pci_range
fix[1].

Change from v3:
- Change 1/5 to what Gabriele suggested.
- Use win->__res.start to get *_mod_base in 2/5, this fix a bug in v3 series.

Change from v2:
- Move struct pci_dev *dev and struct pci_sys_data *sys in
  pcibios_align_resource in 1/5.
- Add Gabriele's codes in 2/5 which delete unnecessary information parse and
  use of_pci_get_host_bridge_resources for both ARM32 and ARM64.
- Add maintainer patch 5/5.

Change from RFC v1:
- Add 1/4 patch by Arnd which removes align_resource callback in ARM
  pcibios_align_resource.
- Change head file in pcie-designware from asm/hardirq.h to linux/hardirq.h.
- Set pp->root_bus_nr = 0 in dra7xx, exynos, imx6, keystone, layerscape,
  spear13xx.
- Remove unnecessary parentheses of some macros in pcie-hisi.
- Use macro to replace some magic values.
- Merge two loops together and add some comments about it in context_config
  function in pcie-hisi.
- Modify some value of items in pcie node example in binding document. 

Change from RFC:
- delete dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct hw_pci,
  merge related operations into dw_pcie_host_init.

Link of v3:
- http://www.spinics.net/lists/linux-pci/msg42539.html
Link of v2:
- http://www.spinics.net/lists/linux-pci/msg41844.html
Link of RFC v1:
- http://www.spinics.net/lists/linux-pci/msg41305.html
Link of RFC:
- http://www.spinics.net/lists/linux-pci/msg40434.html

[1] https://patchwork.ozlabs.org/patch/495018/

Zhou Wang (5):
  ARM/PCI: remove align_resource in pci_sys_data
  PCI: designware: Add ARM64 support
  PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
  Documentation: DT: Add Hisilicon PCIe host binding
  MAINTAINERS: Add pcie-hisi maintainer

 .../devicetree/bindings/pci/hisilicon-pcie.txt     |  46 ++++
 MAINTAINERS                                        |   7 +
 arch/arm/include/asm/mach/pci.h                    |   5 -
 arch/arm/kernel/bios32.c                           |  12 +-
 drivers/pci/host/Kconfig                           |   5 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-dra7xx.c                      |   1 +
 drivers/pci/host/pci-exynos.c                      |   2 +-
 drivers/pci/host/pci-imx6.c                        |   2 +-
 drivers/pci/host/pci-keystone-dw.c                 |   2 +-
 drivers/pci/host/pci-keystone.c                    |   2 +-
 drivers/pci/host/pci-layerscape.c                  |   2 +-
 drivers/pci/host/pcie-designware.c                 | 217 ++++++-----------
 drivers/pci/host/pcie-designware.h                 |  10 +-
 drivers/pci/host/pcie-hisi.c                       | 257 +++++++++++++++++++++
 drivers/pci/host/pcie-spear13xx.c                  |   2 +-
 16 files changed, 410 insertions(+), 163 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
 create mode 100644 drivers/pci/host/pcie-hisi.c

-- 
1.9.1

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

* [PATCH v4 0/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
@ 2015-07-21  6:48 ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni, lorenzo.pieralisi, James Morse, Liviu.Dudau
  Cc: linux-pci, linux-arm-kernel, devicetree, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liudongdong3, qiujiang, kangfenglong,
	liguozhu, Zhou Wang

This patchset adds PCIe host support for Hisilicon Soc Hip05. The PCIe hosts
use PCIe IP core from Synopsys, So this driver is base on designware PCIe driver.

Hip05 is an ARMv8 architecture Soc. It should be able to use ARM64 PCIe API in
designeware PCIe driver. So this patch also adds ARM64 support for designware
pcie.

This patchset is based on v4.2-rc1 and Gabriele's patch about of_pci_range
fix[1].

Change from v3:
- Change 1/5 to what Gabriele suggested.
- Use win->__res.start to get *_mod_base in 2/5, this fix a bug in v3 series.

Change from v2:
- Move struct pci_dev *dev and struct pci_sys_data *sys in
  pcibios_align_resource in 1/5.
- Add Gabriele's codes in 2/5 which delete unnecessary information parse and
  use of_pci_get_host_bridge_resources for both ARM32 and ARM64.
- Add maintainer patch 5/5.

Change from RFC v1:
- Add 1/4 patch by Arnd which removes align_resource callback in ARM
  pcibios_align_resource.
- Change head file in pcie-designware from asm/hardirq.h to linux/hardirq.h.
- Set pp->root_bus_nr = 0 in dra7xx, exynos, imx6, keystone, layerscape,
  spear13xx.
- Remove unnecessary parentheses of some macros in pcie-hisi.
- Use macro to replace some magic values.
- Merge two loops together and add some comments about it in context_config
  function in pcie-hisi.
- Modify some value of items in pcie node example in binding document. 

Change from RFC:
- delete dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct hw_pci,
  merge related operations into dw_pcie_host_init.

Link of v3:
- http://www.spinics.net/lists/linux-pci/msg42539.html
Link of v2:
- http://www.spinics.net/lists/linux-pci/msg41844.html
Link of RFC v1:
- http://www.spinics.net/lists/linux-pci/msg41305.html
Link of RFC:
- http://www.spinics.net/lists/linux-pci/msg40434.html

[1] https://patchwork.ozlabs.org/patch/495018/

Zhou Wang (5):
  ARM/PCI: remove align_resource in pci_sys_data
  PCI: designware: Add ARM64 support
  PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
  Documentation: DT: Add Hisilicon PCIe host binding
  MAINTAINERS: Add pcie-hisi maintainer

 .../devicetree/bindings/pci/hisilicon-pcie.txt     |  46 ++++
 MAINTAINERS                                        |   7 +
 arch/arm/include/asm/mach/pci.h                    |   5 -
 arch/arm/kernel/bios32.c                           |  12 +-
 drivers/pci/host/Kconfig                           |   5 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-dra7xx.c                      |   1 +
 drivers/pci/host/pci-exynos.c                      |   2 +-
 drivers/pci/host/pci-imx6.c                        |   2 +-
 drivers/pci/host/pci-keystone-dw.c                 |   2 +-
 drivers/pci/host/pci-keystone.c                    |   2 +-
 drivers/pci/host/pci-layerscape.c                  |   2 +-
 drivers/pci/host/pcie-designware.c                 | 217 ++++++-----------
 drivers/pci/host/pcie-designware.h                 |  10 +-
 drivers/pci/host/pcie-hisi.c                       | 257 +++++++++++++++++++++
 drivers/pci/host/pcie-spear13xx.c                  |   2 +-
 16 files changed, 410 insertions(+), 163 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
 create mode 100644 drivers/pci/host/pcie-hisi.c

-- 
1.9.1


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

* [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data
  2015-07-21  6:48 ` Zhou Wang
  (?)
@ 2015-07-21  6:48   ` Zhou Wang
  -1 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni, lorenzo.pieralisi, James Morse, Liviu.Dudau
  Cc: zhudacai, devicetree, kangfenglong, linux-pci, yuanzhichang,
	Zhou Wang, qiuzhenfa, liudongdong3, zhangjukuo, liguozhu,
	qiujiang, linux-arm-kernel

This patch is needed in order to unify the PCIe designware framework for ARM and
ARM64 architectures. In the PCIe designware unification process we are calling
pci_create_root_bus() passing a "sysdata" parameter that is the same for both
ARM and ARM64 and is of type "struct pcie_port*". In the ARM case this will
cause a problem with the function pcibios_align_resource(); in fact this will
cast "dev->sysdata" to "struct pci_sys_data*", whereas designware had passed a
"struct pcie_port*" pointer.

This patch solves the issue by removing "align_resource" from "pci_sys_data"
struct and defining a static global function pointer in "bios32.c"

Signed-off-by: Gabriele Paoloni <gabriele.paoloni at huawei.com>
Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 arch/arm/include/asm/mach/pci.h |  5 -----
 arch/arm/kernel/bios32.c        | 12 ++++++++----
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
index 28b9bb3..8a4e4de 100644
--- a/arch/arm/include/asm/mach/pci.h
+++ b/arch/arm/include/asm/mach/pci.h
@@ -58,11 +58,6 @@ struct pci_sys_data {
 					/* IRQ mapping				*/
 	int		(*map_irq)(const struct pci_dev *, u8, u8);
 					/* Resource alignement requirements	*/
-	resource_size_t (*align_resource)(struct pci_dev *dev,
-					  const struct resource *res,
-					  resource_size_t start,
-					  resource_size_t size,
-					  resource_size_t align);
 	void		*private_data;	/* platform controller private data	*/
 };
 
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index fcbbbb1..4cdc64d 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -17,6 +17,11 @@
 #include <asm/mach/pci.h>
 
 static int debug_pci;
+static resource_size_t (*align_resource)(struct pci_dev *dev,
+		  const struct resource *res,
+		  resource_size_t start,
+		  resource_size_t size,
+		  resource_size_t align) = NULL;
 
 #ifdef CONFIG_PCI_MSI
 struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
@@ -468,7 +473,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		sys->busnr   = busnr;
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;
-		sys->align_resource = hw->align_resource;
+		align_resource = hw->align_resource;
 		INIT_LIST_HEAD(&sys->resources);
 
 		if (hw->private_data)
@@ -589,7 +594,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
-	struct pci_sys_data *sys = dev->sysdata;
 	resource_size_t start = res->start;
 
 	if (res->flags & IORESOURCE_IO && start & 0x300)
@@ -597,8 +601,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 
 	start = (start + align - 1) & ~(align - 1);
 
-	if (sys->align_resource)
-		return sys->align_resource(dev, res, start, size, align);
+	if (align_resource)
+		return align_resource(dev, res, start, size, align);
 
 	return start;
 }
-- 
1.9.1

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

* [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data
@ 2015-07-21  6:48   ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: linux-arm-kernel

This patch is needed in order to unify the PCIe designware framework for ARM and
ARM64 architectures. In the PCIe designware unification process we are calling
pci_create_root_bus() passing a "sysdata" parameter that is the same for both
ARM and ARM64 and is of type "struct pcie_port*". In the ARM case this will
cause a problem with the function pcibios_align_resource(); in fact this will
cast "dev->sysdata" to "struct pci_sys_data*", whereas designware had passed a
"struct pcie_port*" pointer.

This patch solves the issue by removing "align_resource" from "pci_sys_data"
struct and defining a static global function pointer in "bios32.c"

Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 arch/arm/include/asm/mach/pci.h |  5 -----
 arch/arm/kernel/bios32.c        | 12 ++++++++----
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
index 28b9bb3..8a4e4de 100644
--- a/arch/arm/include/asm/mach/pci.h
+++ b/arch/arm/include/asm/mach/pci.h
@@ -58,11 +58,6 @@ struct pci_sys_data {
 					/* IRQ mapping				*/
 	int		(*map_irq)(const struct pci_dev *, u8, u8);
 					/* Resource alignement requirements	*/
-	resource_size_t (*align_resource)(struct pci_dev *dev,
-					  const struct resource *res,
-					  resource_size_t start,
-					  resource_size_t size,
-					  resource_size_t align);
 	void		*private_data;	/* platform controller private data	*/
 };
 
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index fcbbbb1..4cdc64d 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -17,6 +17,11 @@
 #include <asm/mach/pci.h>
 
 static int debug_pci;
+static resource_size_t (*align_resource)(struct pci_dev *dev,
+		  const struct resource *res,
+		  resource_size_t start,
+		  resource_size_t size,
+		  resource_size_t align) = NULL;
 
 #ifdef CONFIG_PCI_MSI
 struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
@@ -468,7 +473,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		sys->busnr   = busnr;
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;
-		sys->align_resource = hw->align_resource;
+		align_resource = hw->align_resource;
 		INIT_LIST_HEAD(&sys->resources);
 
 		if (hw->private_data)
@@ -589,7 +594,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
-	struct pci_sys_data *sys = dev->sysdata;
 	resource_size_t start = res->start;
 
 	if (res->flags & IORESOURCE_IO && start & 0x300)
@@ -597,8 +601,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 
 	start = (start + align - 1) & ~(align - 1);
 
-	if (sys->align_resource)
-		return sys->align_resource(dev, res, start, size, align);
+	if (align_resource)
+		return align_resource(dev, res, start, size, align);
 
 	return start;
 }
-- 
1.9.1

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

* [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data
@ 2015-07-21  6:48   ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni, lorenzo.pieralisi, James Morse, Liviu.Dudau
  Cc: linux-pci, linux-arm-kernel, devicetree, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liudongdong3, qiujiang, kangfenglong,
	liguozhu, Zhou Wang

This patch is needed in order to unify the PCIe designware framework for ARM and
ARM64 architectures. In the PCIe designware unification process we are calling
pci_create_root_bus() passing a "sysdata" parameter that is the same for both
ARM and ARM64 and is of type "struct pcie_port*". In the ARM case this will
cause a problem with the function pcibios_align_resource(); in fact this will
cast "dev->sysdata" to "struct pci_sys_data*", whereas designware had passed a
"struct pcie_port*" pointer.

This patch solves the issue by removing "align_resource" from "pci_sys_data"
struct and defining a static global function pointer in "bios32.c"

Signed-off-by: Gabriele Paoloni <gabriele.paoloni at huawei.com>
Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 arch/arm/include/asm/mach/pci.h |  5 -----
 arch/arm/kernel/bios32.c        | 12 ++++++++----
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
index 28b9bb3..8a4e4de 100644
--- a/arch/arm/include/asm/mach/pci.h
+++ b/arch/arm/include/asm/mach/pci.h
@@ -58,11 +58,6 @@ struct pci_sys_data {
 					/* IRQ mapping				*/
 	int		(*map_irq)(const struct pci_dev *, u8, u8);
 					/* Resource alignement requirements	*/
-	resource_size_t (*align_resource)(struct pci_dev *dev,
-					  const struct resource *res,
-					  resource_size_t start,
-					  resource_size_t size,
-					  resource_size_t align);
 	void		*private_data;	/* platform controller private data	*/
 };
 
diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index fcbbbb1..4cdc64d 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -17,6 +17,11 @@
 #include <asm/mach/pci.h>
 
 static int debug_pci;
+static resource_size_t (*align_resource)(struct pci_dev *dev,
+		  const struct resource *res,
+		  resource_size_t start,
+		  resource_size_t size,
+		  resource_size_t align) = NULL;
 
 #ifdef CONFIG_PCI_MSI
 struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
@@ -468,7 +473,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
 		sys->busnr   = busnr;
 		sys->swizzle = hw->swizzle;
 		sys->map_irq = hw->map_irq;
-		sys->align_resource = hw->align_resource;
+		align_resource = hw->align_resource;
 		INIT_LIST_HEAD(&sys->resources);
 
 		if (hw->private_data)
@@ -589,7 +594,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 				resource_size_t size, resource_size_t align)
 {
 	struct pci_dev *dev = data;
-	struct pci_sys_data *sys = dev->sysdata;
 	resource_size_t start = res->start;
 
 	if (res->flags & IORESOURCE_IO && start & 0x300)
@@ -597,8 +601,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 
 	start = (start + align - 1) & ~(align - 1);
 
-	if (sys->align_resource)
-		return sys->align_resource(dev, res, start, size, align);
+	if (align_resource)
+		return align_resource(dev, res, start, size, align);
 
 	return start;
 }
-- 
1.9.1


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

* [PATCH v4 2/5] PCI: designware: Add ARM64 support
  2015-07-21  6:48 ` Zhou Wang
  (?)
@ 2015-07-21  6:48   ` Zhou Wang
  -1 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni, lorenzo.pieralisi, James Morse, Liviu.Dudau
  Cc: linux-pci, linux-arm-kernel, devicetree, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liudongdong3, qiujiang, kangfenglong,
	liguozhu, Zhou Wang

This patch tries to unify ARM32 and ARM64 PCIe in designware driver. Delete
function dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct hw_pci,
move related operations to dw_pcie_host_init. Also set pp->root_bus_nr = 0 in
each PCIe host driver which is based on pcie-designware. This patch also try
to use of_pci_get_host_bridge_resources for ARM32 and ARM64 according to the
suggestion for Gabriele[1]

This patch is based on Gabriele's patch about of_pci_range fix[2]

I have compiled the driver with multi_v7_defconfig. However, I don't have
ARM32 PCIe related board to do test. It will be appreciated if someone could
help to test it.

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>

[1] http://www.spinics.net/lists/linux-pci/msg42194.html
[2] https://patchwork.ozlabs.org/patch/495018/
---
 drivers/pci/host/pci-dra7xx.c      |   1 +
 drivers/pci/host/pci-exynos.c      |   2 +-
 drivers/pci/host/pci-imx6.c        |   2 +-
 drivers/pci/host/pci-keystone-dw.c |   2 +-
 drivers/pci/host/pci-keystone.c    |   2 +-
 drivers/pci/host/pci-layerscape.c  |   2 +-
 drivers/pci/host/pcie-designware.c | 217 +++++++++++++------------------------
 drivers/pci/host/pcie-designware.h |  10 +-
 drivers/pci/host/pcie-spear13xx.c  |   2 +-
 9 files changed, 86 insertions(+), 154 deletions(-)

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 80db09e..69364e8 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -275,6 +275,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
 
 	pp = &dra7xx->pp;
 	pp->dev = dev;
+	pp->root_bus_nr = 0;
 	pp->ops = &dra7xx_pcie_host_ops;
 
 	pp->irq = platform_get_irq(pdev, 1);
diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
index f9f468d..9771bb0 100644
--- a/drivers/pci/host/pci-exynos.c
+++ b/drivers/pci/host/pci-exynos.c
@@ -530,7 +530,7 @@ static int __init exynos_add_pcie_port(struct pcie_port *pp,
 		}
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &exynos_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 233a196..bec256c 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -551,7 +551,7 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
 		}
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &imx6_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
index f34892e..b1e4135 100644
--- a/drivers/pci/host/pci-keystone-dw.c
+++ b/drivers/pci/host/pci-keystone-dw.c
@@ -327,7 +327,7 @@ static void ks_dw_pcie_clear_dbi_mode(void __iomem *reg_virt)
 void ks_dw_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 {
 	struct pcie_port *pp = &ks_pcie->pp;
-	u32 start = pp->mem.start, end = pp->mem.end;
+	u32 start = pp->mem->start, end = pp->mem->end;
 	int i, tr_size;
 
 	/* Disable BARs for inbound access */
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
index 734da58..8113832 100644
--- a/drivers/pci/host/pci-keystone.c
+++ b/drivers/pci/host/pci-keystone.c
@@ -309,7 +309,7 @@ static int __init ks_add_pcie_port(struct keystone_pcie *ks_pcie,
 			return ret;
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &keystone_pcie_host_ops;
 	ret = ks_dw_pcie_host_init(ks_pcie, ks_pcie->msi_intc_np);
 	if (ret) {
diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index b2328ea1..79ff08c 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -106,7 +106,7 @@ static int ls_add_pcie_port(struct ls_pcie *pcie)
 	pp = &pcie->pp;
 	pp->dev = pcie->dev;
 	pp->dbi_base = pcie->dbi;
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &ls_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 69486be..6092c84 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -11,6 +11,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/hardirq.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
@@ -69,16 +70,7 @@
 #define PCIE_ATU_FUNC(x)		(((x) & 0x7) << 16)
 #define PCIE_ATU_UPPER_TARGET		0x91C
 
-static struct hw_pci dw_pci;
-
-static unsigned long global_io_offset;
-
-static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
-{
-	BUG_ON(!sys->private_data);
-
-	return sys->private_data;
-}
+static struct pci_ops dw_pcie_ops;
 
 int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val)
 {
@@ -255,7 +247,7 @@ static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
 static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
 {
 	int irq, pos0, i;
-	struct pcie_port *pp = sys_to_pcie(desc->dev->bus->sysdata);
+	struct pcie_port *pp = desc->dev->bus->sysdata;
 
 	pos0 = bitmap_find_free_region(pp->msi_irq_in_use, MAX_MSI_IRQS,
 				       order_base_2(no_irqs));
@@ -298,7 +290,7 @@ static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
 {
 	int irq, pos;
 	struct msi_msg msg;
-	struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
+	struct pcie_port *pp = pdev->bus->sysdata;
 
 	if (desc->msi_attrib.is_msix)
 		return -EINVAL;
@@ -327,7 +319,7 @@ static void dw_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
 {
 	struct irq_data *data = irq_get_irq_data(irq);
 	struct msi_desc *msi = irq_data_get_msi(data);
-	struct pcie_port *pp = sys_to_pcie(msi->dev->bus->sysdata);
+	struct pcie_port *pp = msi->dev->bus->sysdata;
 
 	clear_irq_range(pp, irq, 1, data->hwirq);
 }
@@ -359,21 +351,19 @@ static const struct irq_domain_ops msi_domain_ops = {
 	.map = dw_pcie_msi_map,
 };
 
-int dw_pcie_host_init(struct pcie_port *pp)
+int __init dw_pcie_host_init(struct pcie_port *pp)
 {
 	struct device_node *np = pp->dev->of_node;
 	struct platform_device *pdev = to_platform_device(pp->dev);
-	struct of_pci_range range;
-	struct of_pci_range_parser parser;
+	struct pci_bus *bus;
 	struct resource *cfg_res;
-	u32 val, na, ns;
+	LIST_HEAD(res);
+	u32 val, ns;
 	const __be32 *addrp;
 	int i, index, ret;
+	struct resource_entry *win;
 
-	/* Find the address cell size and the number of cells in order to get
-	 * the untranslated address.
-	 */
-	of_property_read_u32(np, "#address-cells", &na);
+	/* Find the number of cells in order to get the untranslated address */
 	ns = of_n_size_cells(np);
 
 	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
@@ -392,78 +382,62 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		dev_err(pp->dev, "missing *config* reg space\n");
 	}
 
-	if (of_pci_range_parser_init(&parser, np)) {
-		dev_err(pp->dev, "missing ranges property\n");
-		return -EINVAL;
-	}
+	ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &res, &pp->io_base);
+	if (ret)
+		return ret;
 
 	/* Get the I/O and memory ranges from DT */
-	for_each_of_pci_range(&parser, &range) {
-		unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
-
-		if (restype == IORESOURCE_IO) {
-			of_pci_range_to_resource(&range, np, &pp->io);
-			pp->io.name = "I/O";
-			pp->io.start = max_t(resource_size_t,
-					     PCIBIOS_MIN_IO,
-					     range.pci_addr + global_io_offset);
-			pp->io.end = min_t(resource_size_t,
-					   IO_SPACE_LIMIT,
-					   range.pci_addr + range.size
-					   + global_io_offset - 1);
-			pp->io_size = resource_size(&pp->io);
-			pp->io_bus_addr = range.pci_addr;
-			pp->io_base = range.cpu_addr;
-
-			/* Find the untranslated IO space address */
-			pp->io_mod_base = of_read_number(parser.range -
-							 parser.np + na, ns);
-		}
-		if (restype == IORESOURCE_MEM) {
-			of_pci_range_to_resource(&range, np, &pp->mem);
-			pp->mem.name = "MEM";
-			pp->mem_size = resource_size(&pp->mem);
-			pp->mem_bus_addr = range.pci_addr;
-
-			/* Find the untranslated MEM space address */
-			pp->mem_mod_base = of_read_number(parser.range -
-							  parser.np + na, ns);
-		}
-		if (restype == 0) {
-			of_pci_range_to_resource(&range, np, &pp->cfg);
-			pp->cfg0_size = resource_size(&pp->cfg)/2;
-			pp->cfg1_size = resource_size(&pp->cfg)/2;
-			pp->cfg0_base = pp->cfg.start;
-			pp->cfg1_base = pp->cfg.start + pp->cfg0_size;
+	resource_list_for_each_entry(win, &res) {
+		switch (resource_type(win->res)) {
+		case IORESOURCE_IO:
+			pp->io = win->res;
+			pp->io->name = "I/O";
+			pp->io_size = resource_size(pp->io);
+			pp->io_bus_addr = pp->io->start - win->offset;
+			pp->io_mod_base = win->__res.start;
+			ret = pci_remap_iospace(pp->io, pp->io_base);
+			if (ret) {
+				dev_warn(pp->dev, "error %d: failed to map resource %pR\n",
+					 ret, pp->io);
+				continue;
+			}
+			break;
+		case IORESOURCE_MEM:
+			pp->mem = win->res;
+			pp->mem->name = "MEM";
+			pp->mem_size = resource_size(pp->mem);
+			pp->mem_bus_addr = pp->mem->start - win->offset;
+			pp->mem_mod_base = win->__res.start;
+			break;
+		case 0:
+			pp->cfg = win->res;
+			pp->cfg0_size = resource_size(pp->cfg)/2;
+			pp->cfg1_size = resource_size(pp->cfg)/2;
+			pp->cfg0_base = pp->cfg->start;
+			pp->cfg1_base = pp->cfg->start + pp->cfg0_size;
 
 			/* Find the untranslated configuration space address */
-			pp->cfg0_mod_base = of_read_number(parser.range -
-							   parser.np + na, ns);
-			pp->cfg1_mod_base = pp->cfg0_mod_base +
-					    pp->cfg0_size;
+			pp->cfg0_mod_base = win->__res.start;
+			pp->cfg1_mod_base = pp->cfg0_mod_base + pp->cfg0_size;
+			break;
+		case IORESOURCE_BUS:
+			pp->busn = win->res;
+			break;
+		default:
+			continue;
 		}
 	}
 
-	ret = of_pci_parse_bus_range(np, &pp->busn);
-	if (ret < 0) {
-		pp->busn.name = np->name;
-		pp->busn.start = 0;
-		pp->busn.end = 0xff;
-		pp->busn.flags = IORESOURCE_BUS;
-		dev_dbg(pp->dev, "failed to parse bus-range property: %d, using default %pR\n",
-			ret, &pp->busn);
-	}
-
 	if (!pp->dbi_base) {
-		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
-					resource_size(&pp->cfg));
+		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg->start,
+					resource_size(pp->cfg));
 		if (!pp->dbi_base) {
 			dev_err(pp->dev, "error with ioremap\n");
 			return -ENOMEM;
 		}
 	}
 
-	pp->mem_base = pp->mem.start;
+	pp->mem_base = pp->mem->start;
 
 	if (!pp->va_cfg0_base) {
 		pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
@@ -524,15 +498,28 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	val |= PORT_LOGIC_SPEED_CHANGE;
 	dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
 
-#ifdef CONFIG_PCI_MSI
-	dw_pcie_msi_chip.dev = pp->dev;
-	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
+	bus = pci_create_root_bus(pp->dev, pp->root_bus_nr, &dw_pcie_ops,
+			      pp, &res);
+	if (!bus)
+		return -ENOMEM;
+
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	bus->msi = container_of(&pp->irq_domain, struct msi_controller, domain);
+#else
+	bus->msi = &dw_pcie_msi_chip;
 #endif
 
-	dw_pci.nr_controllers = 1;
-	dw_pci.private_data = (void **)&pp;
+	pci_scan_child_bus(bus);
+	if (pp->ops->scan_bus)
+		pp->ops->scan_bus(pp);
 
-	pci_common_init_dev(pp->dev, &dw_pci);
+#ifdef CONFIG_ARM
+	/* support old dtbs that incorrectly describe IRQs */
+	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+#endif
+
+	pci_assign_unassigned_bus_resources(bus);
+	pci_bus_add_devices(bus);
 
 	return 0;
 }
@@ -633,7 +620,7 @@ static int dw_pcie_valid_config(struct pcie_port *pp,
 static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 			int size, u32 *val)
 {
-	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
+	struct pcie_port *pp = bus->sysdata;
 	int ret;
 
 	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
@@ -657,7 +644,7 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 			int where, int size, u32 val)
 {
-	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
+	struct pcie_port *pp = bus->sysdata;
 	int ret;
 
 	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
@@ -681,62 +668,6 @@ static struct pci_ops dw_pcie_ops = {
 	.write = dw_pcie_wr_conf,
 };
 
-static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
-{
-	struct pcie_port *pp;
-
-	pp = sys_to_pcie(sys);
-
-	if (global_io_offset < SZ_1M && pp->io_size > 0) {
-		sys->io_offset = global_io_offset - pp->io_bus_addr;
-		pci_ioremap_io(global_io_offset, pp->io_base);
-		global_io_offset += SZ_64K;
-		pci_add_resource_offset(&sys->resources, &pp->io,
-					sys->io_offset);
-	}
-
-	sys->mem_offset = pp->mem.start - pp->mem_bus_addr;
-	pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
-	pci_add_resource(&sys->resources, &pp->busn);
-
-	return 1;
-}
-
-static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
-{
-	struct pci_bus *bus;
-	struct pcie_port *pp = sys_to_pcie(sys);
-
-	pp->root_bus_nr = sys->busnr;
-	bus = pci_scan_root_bus(pp->dev, sys->busnr,
-				  &dw_pcie_ops, sys, &sys->resources);
-	if (!bus)
-		return NULL;
-
-	if (bus && pp->ops->scan_bus)
-		pp->ops->scan_bus(pp);
-
-	return bus;
-}
-
-static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
-{
-	struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
-	int irq;
-
-	irq = of_irq_parse_and_map_pci(dev, slot, pin);
-	if (!irq)
-		irq = pp->irq;
-
-	return irq;
-}
-
-static struct hw_pci dw_pci = {
-	.setup		= dw_pcie_setup,
-	.scan		= dw_pcie_scan_bus,
-	.map_irq	= dw_pcie_map_irq,
-};
-
 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 d0bbd27..efac57d 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -34,7 +34,7 @@ struct pcie_port {
 	u64			cfg1_mod_base;
 	void __iomem		*va_cfg1_base;
 	u32			cfg1_size;
-	u64			io_base;
+	resource_size_t		io_base;
 	u64			io_mod_base;
 	phys_addr_t		io_bus_addr;
 	u32			io_size;
@@ -42,10 +42,10 @@ struct pcie_port {
 	u64			mem_mod_base;
 	phys_addr_t		mem_bus_addr;
 	u32			mem_size;
-	struct resource		cfg;
-	struct resource		io;
-	struct resource		mem;
-	struct resource		busn;
+	struct resource		*cfg;
+	struct resource		*io;
+	struct resource		*mem;
+	struct resource		*busn;
 	int			irq;
 	u32			lanes;
 	struct pcie_host_ops	*ops;
diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index c49fbdc..03eb204 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -286,7 +286,7 @@ static int spear13xx_add_pcie_port(struct pcie_port *pp,
 		return ret;
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &spear13xx_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
-- 
1.9.1

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

* [PATCH v4 2/5] PCI: designware: Add ARM64 support
@ 2015-07-21  6:48   ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: linux-arm-kernel

This patch tries to unify ARM32 and ARM64 PCIe in designware driver. Delete
function dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct hw_pci,
move related operations to dw_pcie_host_init. Also set pp->root_bus_nr = 0 in
each PCIe host driver which is based on pcie-designware. This patch also try
to use of_pci_get_host_bridge_resources for ARM32 and ARM64 according to the
suggestion for Gabriele[1]

This patch is based on Gabriele's patch about of_pci_range fix[2]

I have compiled the driver with multi_v7_defconfig. However, I don't have
ARM32 PCIe related board to do test. It will be appreciated if someone could
help to test it.

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>

[1] http://www.spinics.net/lists/linux-pci/msg42194.html
[2] https://patchwork.ozlabs.org/patch/495018/
---
 drivers/pci/host/pci-dra7xx.c      |   1 +
 drivers/pci/host/pci-exynos.c      |   2 +-
 drivers/pci/host/pci-imx6.c        |   2 +-
 drivers/pci/host/pci-keystone-dw.c |   2 +-
 drivers/pci/host/pci-keystone.c    |   2 +-
 drivers/pci/host/pci-layerscape.c  |   2 +-
 drivers/pci/host/pcie-designware.c | 217 +++++++++++++------------------------
 drivers/pci/host/pcie-designware.h |  10 +-
 drivers/pci/host/pcie-spear13xx.c  |   2 +-
 9 files changed, 86 insertions(+), 154 deletions(-)

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 80db09e..69364e8 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -275,6 +275,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
 
 	pp = &dra7xx->pp;
 	pp->dev = dev;
+	pp->root_bus_nr = 0;
 	pp->ops = &dra7xx_pcie_host_ops;
 
 	pp->irq = platform_get_irq(pdev, 1);
diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
index f9f468d..9771bb0 100644
--- a/drivers/pci/host/pci-exynos.c
+++ b/drivers/pci/host/pci-exynos.c
@@ -530,7 +530,7 @@ static int __init exynos_add_pcie_port(struct pcie_port *pp,
 		}
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &exynos_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 233a196..bec256c 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -551,7 +551,7 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
 		}
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &imx6_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
index f34892e..b1e4135 100644
--- a/drivers/pci/host/pci-keystone-dw.c
+++ b/drivers/pci/host/pci-keystone-dw.c
@@ -327,7 +327,7 @@ static void ks_dw_pcie_clear_dbi_mode(void __iomem *reg_virt)
 void ks_dw_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 {
 	struct pcie_port *pp = &ks_pcie->pp;
-	u32 start = pp->mem.start, end = pp->mem.end;
+	u32 start = pp->mem->start, end = pp->mem->end;
 	int i, tr_size;
 
 	/* Disable BARs for inbound access */
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
index 734da58..8113832 100644
--- a/drivers/pci/host/pci-keystone.c
+++ b/drivers/pci/host/pci-keystone.c
@@ -309,7 +309,7 @@ static int __init ks_add_pcie_port(struct keystone_pcie *ks_pcie,
 			return ret;
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &keystone_pcie_host_ops;
 	ret = ks_dw_pcie_host_init(ks_pcie, ks_pcie->msi_intc_np);
 	if (ret) {
diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index b2328ea1..79ff08c 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -106,7 +106,7 @@ static int ls_add_pcie_port(struct ls_pcie *pcie)
 	pp = &pcie->pp;
 	pp->dev = pcie->dev;
 	pp->dbi_base = pcie->dbi;
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &ls_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 69486be..6092c84 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -11,6 +11,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/hardirq.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
@@ -69,16 +70,7 @@
 #define PCIE_ATU_FUNC(x)		(((x) & 0x7) << 16)
 #define PCIE_ATU_UPPER_TARGET		0x91C
 
-static struct hw_pci dw_pci;
-
-static unsigned long global_io_offset;
-
-static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
-{
-	BUG_ON(!sys->private_data);
-
-	return sys->private_data;
-}
+static struct pci_ops dw_pcie_ops;
 
 int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val)
 {
@@ -255,7 +247,7 @@ static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
 static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
 {
 	int irq, pos0, i;
-	struct pcie_port *pp = sys_to_pcie(desc->dev->bus->sysdata);
+	struct pcie_port *pp = desc->dev->bus->sysdata;
 
 	pos0 = bitmap_find_free_region(pp->msi_irq_in_use, MAX_MSI_IRQS,
 				       order_base_2(no_irqs));
@@ -298,7 +290,7 @@ static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
 {
 	int irq, pos;
 	struct msi_msg msg;
-	struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
+	struct pcie_port *pp = pdev->bus->sysdata;
 
 	if (desc->msi_attrib.is_msix)
 		return -EINVAL;
@@ -327,7 +319,7 @@ static void dw_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
 {
 	struct irq_data *data = irq_get_irq_data(irq);
 	struct msi_desc *msi = irq_data_get_msi(data);
-	struct pcie_port *pp = sys_to_pcie(msi->dev->bus->sysdata);
+	struct pcie_port *pp = msi->dev->bus->sysdata;
 
 	clear_irq_range(pp, irq, 1, data->hwirq);
 }
@@ -359,21 +351,19 @@ static const struct irq_domain_ops msi_domain_ops = {
 	.map = dw_pcie_msi_map,
 };
 
-int dw_pcie_host_init(struct pcie_port *pp)
+int __init dw_pcie_host_init(struct pcie_port *pp)
 {
 	struct device_node *np = pp->dev->of_node;
 	struct platform_device *pdev = to_platform_device(pp->dev);
-	struct of_pci_range range;
-	struct of_pci_range_parser parser;
+	struct pci_bus *bus;
 	struct resource *cfg_res;
-	u32 val, na, ns;
+	LIST_HEAD(res);
+	u32 val, ns;
 	const __be32 *addrp;
 	int i, index, ret;
+	struct resource_entry *win;
 
-	/* Find the address cell size and the number of cells in order to get
-	 * the untranslated address.
-	 */
-	of_property_read_u32(np, "#address-cells", &na);
+	/* Find the number of cells in order to get the untranslated address */
 	ns = of_n_size_cells(np);
 
 	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
@@ -392,78 +382,62 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		dev_err(pp->dev, "missing *config* reg space\n");
 	}
 
-	if (of_pci_range_parser_init(&parser, np)) {
-		dev_err(pp->dev, "missing ranges property\n");
-		return -EINVAL;
-	}
+	ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &res, &pp->io_base);
+	if (ret)
+		return ret;
 
 	/* Get the I/O and memory ranges from DT */
-	for_each_of_pci_range(&parser, &range) {
-		unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
-
-		if (restype == IORESOURCE_IO) {
-			of_pci_range_to_resource(&range, np, &pp->io);
-			pp->io.name = "I/O";
-			pp->io.start = max_t(resource_size_t,
-					     PCIBIOS_MIN_IO,
-					     range.pci_addr + global_io_offset);
-			pp->io.end = min_t(resource_size_t,
-					   IO_SPACE_LIMIT,
-					   range.pci_addr + range.size
-					   + global_io_offset - 1);
-			pp->io_size = resource_size(&pp->io);
-			pp->io_bus_addr = range.pci_addr;
-			pp->io_base = range.cpu_addr;
-
-			/* Find the untranslated IO space address */
-			pp->io_mod_base = of_read_number(parser.range -
-							 parser.np + na, ns);
-		}
-		if (restype == IORESOURCE_MEM) {
-			of_pci_range_to_resource(&range, np, &pp->mem);
-			pp->mem.name = "MEM";
-			pp->mem_size = resource_size(&pp->mem);
-			pp->mem_bus_addr = range.pci_addr;
-
-			/* Find the untranslated MEM space address */
-			pp->mem_mod_base = of_read_number(parser.range -
-							  parser.np + na, ns);
-		}
-		if (restype == 0) {
-			of_pci_range_to_resource(&range, np, &pp->cfg);
-			pp->cfg0_size = resource_size(&pp->cfg)/2;
-			pp->cfg1_size = resource_size(&pp->cfg)/2;
-			pp->cfg0_base = pp->cfg.start;
-			pp->cfg1_base = pp->cfg.start + pp->cfg0_size;
+	resource_list_for_each_entry(win, &res) {
+		switch (resource_type(win->res)) {
+		case IORESOURCE_IO:
+			pp->io = win->res;
+			pp->io->name = "I/O";
+			pp->io_size = resource_size(pp->io);
+			pp->io_bus_addr = pp->io->start - win->offset;
+			pp->io_mod_base = win->__res.start;
+			ret = pci_remap_iospace(pp->io, pp->io_base);
+			if (ret) {
+				dev_warn(pp->dev, "error %d: failed to map resource %pR\n",
+					 ret, pp->io);
+				continue;
+			}
+			break;
+		case IORESOURCE_MEM:
+			pp->mem = win->res;
+			pp->mem->name = "MEM";
+			pp->mem_size = resource_size(pp->mem);
+			pp->mem_bus_addr = pp->mem->start - win->offset;
+			pp->mem_mod_base = win->__res.start;
+			break;
+		case 0:
+			pp->cfg = win->res;
+			pp->cfg0_size = resource_size(pp->cfg)/2;
+			pp->cfg1_size = resource_size(pp->cfg)/2;
+			pp->cfg0_base = pp->cfg->start;
+			pp->cfg1_base = pp->cfg->start + pp->cfg0_size;
 
 			/* Find the untranslated configuration space address */
-			pp->cfg0_mod_base = of_read_number(parser.range -
-							   parser.np + na, ns);
-			pp->cfg1_mod_base = pp->cfg0_mod_base +
-					    pp->cfg0_size;
+			pp->cfg0_mod_base = win->__res.start;
+			pp->cfg1_mod_base = pp->cfg0_mod_base + pp->cfg0_size;
+			break;
+		case IORESOURCE_BUS:
+			pp->busn = win->res;
+			break;
+		default:
+			continue;
 		}
 	}
 
-	ret = of_pci_parse_bus_range(np, &pp->busn);
-	if (ret < 0) {
-		pp->busn.name = np->name;
-		pp->busn.start = 0;
-		pp->busn.end = 0xff;
-		pp->busn.flags = IORESOURCE_BUS;
-		dev_dbg(pp->dev, "failed to parse bus-range property: %d, using default %pR\n",
-			ret, &pp->busn);
-	}
-
 	if (!pp->dbi_base) {
-		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
-					resource_size(&pp->cfg));
+		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg->start,
+					resource_size(pp->cfg));
 		if (!pp->dbi_base) {
 			dev_err(pp->dev, "error with ioremap\n");
 			return -ENOMEM;
 		}
 	}
 
-	pp->mem_base = pp->mem.start;
+	pp->mem_base = pp->mem->start;
 
 	if (!pp->va_cfg0_base) {
 		pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
@@ -524,15 +498,28 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	val |= PORT_LOGIC_SPEED_CHANGE;
 	dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
 
-#ifdef CONFIG_PCI_MSI
-	dw_pcie_msi_chip.dev = pp->dev;
-	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
+	bus = pci_create_root_bus(pp->dev, pp->root_bus_nr, &dw_pcie_ops,
+			      pp, &res);
+	if (!bus)
+		return -ENOMEM;
+
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	bus->msi = container_of(&pp->irq_domain, struct msi_controller, domain);
+#else
+	bus->msi = &dw_pcie_msi_chip;
 #endif
 
-	dw_pci.nr_controllers = 1;
-	dw_pci.private_data = (void **)&pp;
+	pci_scan_child_bus(bus);
+	if (pp->ops->scan_bus)
+		pp->ops->scan_bus(pp);
 
-	pci_common_init_dev(pp->dev, &dw_pci);
+#ifdef CONFIG_ARM
+	/* support old dtbs that incorrectly describe IRQs */
+	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+#endif
+
+	pci_assign_unassigned_bus_resources(bus);
+	pci_bus_add_devices(bus);
 
 	return 0;
 }
@@ -633,7 +620,7 @@ static int dw_pcie_valid_config(struct pcie_port *pp,
 static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 			int size, u32 *val)
 {
-	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
+	struct pcie_port *pp = bus->sysdata;
 	int ret;
 
 	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
@@ -657,7 +644,7 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 			int where, int size, u32 val)
 {
-	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
+	struct pcie_port *pp = bus->sysdata;
 	int ret;
 
 	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
@@ -681,62 +668,6 @@ static struct pci_ops dw_pcie_ops = {
 	.write = dw_pcie_wr_conf,
 };
 
-static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
-{
-	struct pcie_port *pp;
-
-	pp = sys_to_pcie(sys);
-
-	if (global_io_offset < SZ_1M && pp->io_size > 0) {
-		sys->io_offset = global_io_offset - pp->io_bus_addr;
-		pci_ioremap_io(global_io_offset, pp->io_base);
-		global_io_offset += SZ_64K;
-		pci_add_resource_offset(&sys->resources, &pp->io,
-					sys->io_offset);
-	}
-
-	sys->mem_offset = pp->mem.start - pp->mem_bus_addr;
-	pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
-	pci_add_resource(&sys->resources, &pp->busn);
-
-	return 1;
-}
-
-static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
-{
-	struct pci_bus *bus;
-	struct pcie_port *pp = sys_to_pcie(sys);
-
-	pp->root_bus_nr = sys->busnr;
-	bus = pci_scan_root_bus(pp->dev, sys->busnr,
-				  &dw_pcie_ops, sys, &sys->resources);
-	if (!bus)
-		return NULL;
-
-	if (bus && pp->ops->scan_bus)
-		pp->ops->scan_bus(pp);
-
-	return bus;
-}
-
-static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
-{
-	struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
-	int irq;
-
-	irq = of_irq_parse_and_map_pci(dev, slot, pin);
-	if (!irq)
-		irq = pp->irq;
-
-	return irq;
-}
-
-static struct hw_pci dw_pci = {
-	.setup		= dw_pcie_setup,
-	.scan		= dw_pcie_scan_bus,
-	.map_irq	= dw_pcie_map_irq,
-};
-
 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 d0bbd27..efac57d 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -34,7 +34,7 @@ struct pcie_port {
 	u64			cfg1_mod_base;
 	void __iomem		*va_cfg1_base;
 	u32			cfg1_size;
-	u64			io_base;
+	resource_size_t		io_base;
 	u64			io_mod_base;
 	phys_addr_t		io_bus_addr;
 	u32			io_size;
@@ -42,10 +42,10 @@ struct pcie_port {
 	u64			mem_mod_base;
 	phys_addr_t		mem_bus_addr;
 	u32			mem_size;
-	struct resource		cfg;
-	struct resource		io;
-	struct resource		mem;
-	struct resource		busn;
+	struct resource		*cfg;
+	struct resource		*io;
+	struct resource		*mem;
+	struct resource		*busn;
 	int			irq;
 	u32			lanes;
 	struct pcie_host_ops	*ops;
diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index c49fbdc..03eb204 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -286,7 +286,7 @@ static int spear13xx_add_pcie_port(struct pcie_port *pp,
 		return ret;
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &spear13xx_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
-- 
1.9.1

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

* [PATCH v4 2/5] PCI: designware: Add ARM64 support
@ 2015-07-21  6:48   ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni, lorenzo.pieralisi, James Morse, Liviu.Dudau
  Cc: linux-pci, linux-arm-kernel, devicetree, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liudongdong3, qiujiang, kangfenglong,
	liguozhu, Zhou Wang

This patch tries to unify ARM32 and ARM64 PCIe in designware driver. Delete
function dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct hw_pci,
move related operations to dw_pcie_host_init. Also set pp->root_bus_nr = 0 in
each PCIe host driver which is based on pcie-designware. This patch also try
to use of_pci_get_host_bridge_resources for ARM32 and ARM64 according to the
suggestion for Gabriele[1]

This patch is based on Gabriele's patch about of_pci_range fix[2]

I have compiled the driver with multi_v7_defconfig. However, I don't have
ARM32 PCIe related board to do test. It will be appreciated if someone could
help to test it.

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>

[1] http://www.spinics.net/lists/linux-pci/msg42194.html
[2] https://patchwork.ozlabs.org/patch/495018/
---
 drivers/pci/host/pci-dra7xx.c      |   1 +
 drivers/pci/host/pci-exynos.c      |   2 +-
 drivers/pci/host/pci-imx6.c        |   2 +-
 drivers/pci/host/pci-keystone-dw.c |   2 +-
 drivers/pci/host/pci-keystone.c    |   2 +-
 drivers/pci/host/pci-layerscape.c  |   2 +-
 drivers/pci/host/pcie-designware.c | 217 +++++++++++++------------------------
 drivers/pci/host/pcie-designware.h |  10 +-
 drivers/pci/host/pcie-spear13xx.c  |   2 +-
 9 files changed, 86 insertions(+), 154 deletions(-)

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 80db09e..69364e8 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -275,6 +275,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
 
 	pp = &dra7xx->pp;
 	pp->dev = dev;
+	pp->root_bus_nr = 0;
 	pp->ops = &dra7xx_pcie_host_ops;
 
 	pp->irq = platform_get_irq(pdev, 1);
diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
index f9f468d..9771bb0 100644
--- a/drivers/pci/host/pci-exynos.c
+++ b/drivers/pci/host/pci-exynos.c
@@ -530,7 +530,7 @@ static int __init exynos_add_pcie_port(struct pcie_port *pp,
 		}
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &exynos_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 233a196..bec256c 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -551,7 +551,7 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
 		}
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &imx6_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
index f34892e..b1e4135 100644
--- a/drivers/pci/host/pci-keystone-dw.c
+++ b/drivers/pci/host/pci-keystone-dw.c
@@ -327,7 +327,7 @@ static void ks_dw_pcie_clear_dbi_mode(void __iomem *reg_virt)
 void ks_dw_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 {
 	struct pcie_port *pp = &ks_pcie->pp;
-	u32 start = pp->mem.start, end = pp->mem.end;
+	u32 start = pp->mem->start, end = pp->mem->end;
 	int i, tr_size;
 
 	/* Disable BARs for inbound access */
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
index 734da58..8113832 100644
--- a/drivers/pci/host/pci-keystone.c
+++ b/drivers/pci/host/pci-keystone.c
@@ -309,7 +309,7 @@ static int __init ks_add_pcie_port(struct keystone_pcie *ks_pcie,
 			return ret;
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &keystone_pcie_host_ops;
 	ret = ks_dw_pcie_host_init(ks_pcie, ks_pcie->msi_intc_np);
 	if (ret) {
diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index b2328ea1..79ff08c 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -106,7 +106,7 @@ static int ls_add_pcie_port(struct ls_pcie *pcie)
 	pp = &pcie->pp;
 	pp->dev = pcie->dev;
 	pp->dbi_base = pcie->dbi;
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &ls_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 69486be..6092c84 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -11,6 +11,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/hardirq.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
@@ -69,16 +70,7 @@
 #define PCIE_ATU_FUNC(x)		(((x) & 0x7) << 16)
 #define PCIE_ATU_UPPER_TARGET		0x91C
 
-static struct hw_pci dw_pci;
-
-static unsigned long global_io_offset;
-
-static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
-{
-	BUG_ON(!sys->private_data);
-
-	return sys->private_data;
-}
+static struct pci_ops dw_pcie_ops;
 
 int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val)
 {
@@ -255,7 +247,7 @@ static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
 static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
 {
 	int irq, pos0, i;
-	struct pcie_port *pp = sys_to_pcie(desc->dev->bus->sysdata);
+	struct pcie_port *pp = desc->dev->bus->sysdata;
 
 	pos0 = bitmap_find_free_region(pp->msi_irq_in_use, MAX_MSI_IRQS,
 				       order_base_2(no_irqs));
@@ -298,7 +290,7 @@ static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
 {
 	int irq, pos;
 	struct msi_msg msg;
-	struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
+	struct pcie_port *pp = pdev->bus->sysdata;
 
 	if (desc->msi_attrib.is_msix)
 		return -EINVAL;
@@ -327,7 +319,7 @@ static void dw_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
 {
 	struct irq_data *data = irq_get_irq_data(irq);
 	struct msi_desc *msi = irq_data_get_msi(data);
-	struct pcie_port *pp = sys_to_pcie(msi->dev->bus->sysdata);
+	struct pcie_port *pp = msi->dev->bus->sysdata;
 
 	clear_irq_range(pp, irq, 1, data->hwirq);
 }
@@ -359,21 +351,19 @@ static const struct irq_domain_ops msi_domain_ops = {
 	.map = dw_pcie_msi_map,
 };
 
-int dw_pcie_host_init(struct pcie_port *pp)
+int __init dw_pcie_host_init(struct pcie_port *pp)
 {
 	struct device_node *np = pp->dev->of_node;
 	struct platform_device *pdev = to_platform_device(pp->dev);
-	struct of_pci_range range;
-	struct of_pci_range_parser parser;
+	struct pci_bus *bus;
 	struct resource *cfg_res;
-	u32 val, na, ns;
+	LIST_HEAD(res);
+	u32 val, ns;
 	const __be32 *addrp;
 	int i, index, ret;
+	struct resource_entry *win;
 
-	/* Find the address cell size and the number of cells in order to get
-	 * the untranslated address.
-	 */
-	of_property_read_u32(np, "#address-cells", &na);
+	/* Find the number of cells in order to get the untranslated address */
 	ns = of_n_size_cells(np);
 
 	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
@@ -392,78 +382,62 @@ int dw_pcie_host_init(struct pcie_port *pp)
 		dev_err(pp->dev, "missing *config* reg space\n");
 	}
 
-	if (of_pci_range_parser_init(&parser, np)) {
-		dev_err(pp->dev, "missing ranges property\n");
-		return -EINVAL;
-	}
+	ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &res, &pp->io_base);
+	if (ret)
+		return ret;
 
 	/* Get the I/O and memory ranges from DT */
-	for_each_of_pci_range(&parser, &range) {
-		unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
-
-		if (restype == IORESOURCE_IO) {
-			of_pci_range_to_resource(&range, np, &pp->io);
-			pp->io.name = "I/O";
-			pp->io.start = max_t(resource_size_t,
-					     PCIBIOS_MIN_IO,
-					     range.pci_addr + global_io_offset);
-			pp->io.end = min_t(resource_size_t,
-					   IO_SPACE_LIMIT,
-					   range.pci_addr + range.size
-					   + global_io_offset - 1);
-			pp->io_size = resource_size(&pp->io);
-			pp->io_bus_addr = range.pci_addr;
-			pp->io_base = range.cpu_addr;
-
-			/* Find the untranslated IO space address */
-			pp->io_mod_base = of_read_number(parser.range -
-							 parser.np + na, ns);
-		}
-		if (restype == IORESOURCE_MEM) {
-			of_pci_range_to_resource(&range, np, &pp->mem);
-			pp->mem.name = "MEM";
-			pp->mem_size = resource_size(&pp->mem);
-			pp->mem_bus_addr = range.pci_addr;
-
-			/* Find the untranslated MEM space address */
-			pp->mem_mod_base = of_read_number(parser.range -
-							  parser.np + na, ns);
-		}
-		if (restype == 0) {
-			of_pci_range_to_resource(&range, np, &pp->cfg);
-			pp->cfg0_size = resource_size(&pp->cfg)/2;
-			pp->cfg1_size = resource_size(&pp->cfg)/2;
-			pp->cfg0_base = pp->cfg.start;
-			pp->cfg1_base = pp->cfg.start + pp->cfg0_size;
+	resource_list_for_each_entry(win, &res) {
+		switch (resource_type(win->res)) {
+		case IORESOURCE_IO:
+			pp->io = win->res;
+			pp->io->name = "I/O";
+			pp->io_size = resource_size(pp->io);
+			pp->io_bus_addr = pp->io->start - win->offset;
+			pp->io_mod_base = win->__res.start;
+			ret = pci_remap_iospace(pp->io, pp->io_base);
+			if (ret) {
+				dev_warn(pp->dev, "error %d: failed to map resource %pR\n",
+					 ret, pp->io);
+				continue;
+			}
+			break;
+		case IORESOURCE_MEM:
+			pp->mem = win->res;
+			pp->mem->name = "MEM";
+			pp->mem_size = resource_size(pp->mem);
+			pp->mem_bus_addr = pp->mem->start - win->offset;
+			pp->mem_mod_base = win->__res.start;
+			break;
+		case 0:
+			pp->cfg = win->res;
+			pp->cfg0_size = resource_size(pp->cfg)/2;
+			pp->cfg1_size = resource_size(pp->cfg)/2;
+			pp->cfg0_base = pp->cfg->start;
+			pp->cfg1_base = pp->cfg->start + pp->cfg0_size;
 
 			/* Find the untranslated configuration space address */
-			pp->cfg0_mod_base = of_read_number(parser.range -
-							   parser.np + na, ns);
-			pp->cfg1_mod_base = pp->cfg0_mod_base +
-					    pp->cfg0_size;
+			pp->cfg0_mod_base = win->__res.start;
+			pp->cfg1_mod_base = pp->cfg0_mod_base + pp->cfg0_size;
+			break;
+		case IORESOURCE_BUS:
+			pp->busn = win->res;
+			break;
+		default:
+			continue;
 		}
 	}
 
-	ret = of_pci_parse_bus_range(np, &pp->busn);
-	if (ret < 0) {
-		pp->busn.name = np->name;
-		pp->busn.start = 0;
-		pp->busn.end = 0xff;
-		pp->busn.flags = IORESOURCE_BUS;
-		dev_dbg(pp->dev, "failed to parse bus-range property: %d, using default %pR\n",
-			ret, &pp->busn);
-	}
-
 	if (!pp->dbi_base) {
-		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
-					resource_size(&pp->cfg));
+		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg->start,
+					resource_size(pp->cfg));
 		if (!pp->dbi_base) {
 			dev_err(pp->dev, "error with ioremap\n");
 			return -ENOMEM;
 		}
 	}
 
-	pp->mem_base = pp->mem.start;
+	pp->mem_base = pp->mem->start;
 
 	if (!pp->va_cfg0_base) {
 		pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
@@ -524,15 +498,28 @@ int dw_pcie_host_init(struct pcie_port *pp)
 	val |= PORT_LOGIC_SPEED_CHANGE;
 	dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
 
-#ifdef CONFIG_PCI_MSI
-	dw_pcie_msi_chip.dev = pp->dev;
-	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
+	bus = pci_create_root_bus(pp->dev, pp->root_bus_nr, &dw_pcie_ops,
+			      pp, &res);
+	if (!bus)
+		return -ENOMEM;
+
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	bus->msi = container_of(&pp->irq_domain, struct msi_controller, domain);
+#else
+	bus->msi = &dw_pcie_msi_chip;
 #endif
 
-	dw_pci.nr_controllers = 1;
-	dw_pci.private_data = (void **)&pp;
+	pci_scan_child_bus(bus);
+	if (pp->ops->scan_bus)
+		pp->ops->scan_bus(pp);
 
-	pci_common_init_dev(pp->dev, &dw_pci);
+#ifdef CONFIG_ARM
+	/* support old dtbs that incorrectly describe IRQs */
+	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+#endif
+
+	pci_assign_unassigned_bus_resources(bus);
+	pci_bus_add_devices(bus);
 
 	return 0;
 }
@@ -633,7 +620,7 @@ static int dw_pcie_valid_config(struct pcie_port *pp,
 static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 			int size, u32 *val)
 {
-	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
+	struct pcie_port *pp = bus->sysdata;
 	int ret;
 
 	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
@@ -657,7 +644,7 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 			int where, int size, u32 val)
 {
-	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
+	struct pcie_port *pp = bus->sysdata;
 	int ret;
 
 	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
@@ -681,62 +668,6 @@ static struct pci_ops dw_pcie_ops = {
 	.write = dw_pcie_wr_conf,
 };
 
-static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
-{
-	struct pcie_port *pp;
-
-	pp = sys_to_pcie(sys);
-
-	if (global_io_offset < SZ_1M && pp->io_size > 0) {
-		sys->io_offset = global_io_offset - pp->io_bus_addr;
-		pci_ioremap_io(global_io_offset, pp->io_base);
-		global_io_offset += SZ_64K;
-		pci_add_resource_offset(&sys->resources, &pp->io,
-					sys->io_offset);
-	}
-
-	sys->mem_offset = pp->mem.start - pp->mem_bus_addr;
-	pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
-	pci_add_resource(&sys->resources, &pp->busn);
-
-	return 1;
-}
-
-static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
-{
-	struct pci_bus *bus;
-	struct pcie_port *pp = sys_to_pcie(sys);
-
-	pp->root_bus_nr = sys->busnr;
-	bus = pci_scan_root_bus(pp->dev, sys->busnr,
-				  &dw_pcie_ops, sys, &sys->resources);
-	if (!bus)
-		return NULL;
-
-	if (bus && pp->ops->scan_bus)
-		pp->ops->scan_bus(pp);
-
-	return bus;
-}
-
-static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
-{
-	struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
-	int irq;
-
-	irq = of_irq_parse_and_map_pci(dev, slot, pin);
-	if (!irq)
-		irq = pp->irq;
-
-	return irq;
-}
-
-static struct hw_pci dw_pci = {
-	.setup		= dw_pcie_setup,
-	.scan		= dw_pcie_scan_bus,
-	.map_irq	= dw_pcie_map_irq,
-};
-
 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 d0bbd27..efac57d 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -34,7 +34,7 @@ struct pcie_port {
 	u64			cfg1_mod_base;
 	void __iomem		*va_cfg1_base;
 	u32			cfg1_size;
-	u64			io_base;
+	resource_size_t		io_base;
 	u64			io_mod_base;
 	phys_addr_t		io_bus_addr;
 	u32			io_size;
@@ -42,10 +42,10 @@ struct pcie_port {
 	u64			mem_mod_base;
 	phys_addr_t		mem_bus_addr;
 	u32			mem_size;
-	struct resource		cfg;
-	struct resource		io;
-	struct resource		mem;
-	struct resource		busn;
+	struct resource		*cfg;
+	struct resource		*io;
+	struct resource		*mem;
+	struct resource		*busn;
 	int			irq;
 	u32			lanes;
 	struct pcie_host_ops	*ops;
diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index c49fbdc..03eb204 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -286,7 +286,7 @@ static int spear13xx_add_pcie_port(struct pcie_port *pp,
 		return ret;
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &spear13xx_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
-- 
1.9.1


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

* [PATCH v4 3/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
  2015-07-21  6:48 ` Zhou Wang
  (?)
@ 2015-07-21  6:48   ` Zhou Wang
  -1 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni, lorenzo.pieralisi, James Morse, Liviu.Dudau
  Cc: linux-pci, linux-arm-kernel, devicetree, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liudongdong3, qiujiang, kangfenglong,
	liguozhu, Zhou Wang

This patch adds PCIe host support for Hisilicon Soc Hip05.

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 drivers/pci/host/Kconfig     |   5 +
 drivers/pci/host/Makefile    |   1 +
 drivers/pci/host/pcie-hisi.c | 257 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 drivers/pci/host/pcie-hisi.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index c132bdd..37f2075 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -145,4 +145,9 @@ config PCIE_IPROC_BCMA
 	  Say Y here if you want to use the Broadcom iProc PCIe controller
 	  through the BCMA bus interface
 
+config PCI_HISI
+	depends on OF && ARM64
+	bool "Hisilicon Soc HIP05 PCIe controller"
+	select PCIE_DW
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 140d66f..ea1dbf2 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
 obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
 obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
 obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
+obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
new file mode 100644
index 0000000..9f2fac1
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi.c
@@ -0,0 +1,257 @@
+/*
+ * PCIe host controller driver for Hisilicon Hip05 SoCs
+ *
+ * Copyright (C) 2015 Hisilicon Co., Ltd. http://www.hisilicon.com
+ *
+ * Author: Zhou Wang <wangzhou1@hisilicon.com>
+ *         Dacai Zhu <zhudacai@hisilicon.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/interrupt.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_pci.h>
+#include <linux/platform_device.h>
+
+#include "pcie-designware.h"
+
+#define PCIE_SUBCTRL_MODE_REG                           0x2800
+#define PCIE_SUBCTRL_SYS_STATE4_REG                     0x6818
+#define PCIE_SLV_DBI_MODE                               0x0
+#define PCIE_SLV_SYSCTRL_MODE                           0x1
+#define PCIE_SLV_CONTENT_MODE                           0x2
+#define PCIE_SLV_MSI_ASID                               0x10
+#define PCIE_LTSSM_LINKUP_STATE                         0x11
+#define PCIE_LTSSM_STATE_MASK                           0x3F
+#define PCIE_MSI_ASID_ENABLE                            (0x1 << 12)
+#define PCIE_MSI_ASID_VALUE                             (0x1 << 16)
+#define PCIE_MSI_TRANS_ENABLE                           (0x1 << 12)
+#define PCIE_MSI_TRANS_REG                              0x1c8
+#define PCIE_MSI_LOW_ADDRESS                            0x1b4
+#define PCIE_MSI_HIGH_ADDRESS                           0x1c4
+#define PCIE_MSI_ADDRESS_VAL                            0xb7010040
+
+#define to_hisi_pcie(x)	container_of(x, struct hisi_pcie, pp)
+
+struct hisi_pcie {
+	void __iomem *subctrl_base;
+	void __iomem *reg_base;
+	struct msi_controller *msi;
+	u32 port_id;
+	struct pcie_port pp;
+};
+
+static inline void hisi_pcie_subctrl_writel(struct hisi_pcie *pcie,
+					    u32 val, u32 reg)
+{
+	writel(val, pcie->subctrl_base + reg);
+}
+
+static inline u32 hisi_pcie_subctrl_readl(struct hisi_pcie *pcie, u32 reg)
+{
+	return readl(pcie->subctrl_base + reg);
+}
+
+static inline void hisi_pcie_apb_writel(struct hisi_pcie *pcie,
+					u32 val, u32 reg)
+{
+	writel(val, pcie->reg_base + reg);
+}
+
+static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *pcie, u32 reg)
+{
+	return readl(pcie->reg_base + reg);
+}
+
+/*
+ * Change mode to indicate the same reg_base to base of PCIe host configure
+ * registers, base of RC configure space or base of vmid/asid context table
+ */
+static void hisi_pcie_change_apb_mode(struct hisi_pcie *pcie, u32 mode)
+{
+	u32 val;
+	u32 bit_mask;
+	u32 bit_shift;
+	u32 port_id = pcie->port_id;
+	u32 reg = PCIE_SUBCTRL_MODE_REG + 0x100 * port_id;
+
+	if ((port_id == 1) || (port_id == 2)) {
+		bit_mask = 0xc;
+		bit_shift = 0x2;
+	} else {
+		bit_mask = 0x6;
+		bit_shift = 0x1;
+	}
+
+	val = hisi_pcie_subctrl_readl(pcie, reg);
+	val = (val & (~bit_mask)) | (mode << bit_shift);
+	hisi_pcie_subctrl_writel(pcie, val, reg);
+}
+
+/* Configure vmid/asid table in PCIe host */
+static void hisi_pcie_config_context(struct hisi_pcie *pcie)
+{
+	int i;
+
+	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_CONTENT_MODE);
+
+	/*
+	 * init vmid and asid tables for all PCIes device as 0
+	 * vmid table: 0 ~ 0x3ff, asid table: 0x400 ~ 0x7ff
+	 */
+	for (i = 0; i < 0x800; i++)
+		hisi_pcie_apb_writel(pcie, 0x0, i * 4);
+
+	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_SYSCTRL_MODE);
+
+	hisi_pcie_apb_writel(pcie, PCIE_MSI_ADDRESS_VAL, PCIE_MSI_LOW_ADDRESS);
+	hisi_pcie_apb_writel(pcie, 0x0, PCIE_MSI_HIGH_ADDRESS);
+	hisi_pcie_apb_writel(pcie, PCIE_MSI_ASID_ENABLE | PCIE_MSI_ASID_VALUE,
+			     PCIE_SLV_MSI_ASID);
+	hisi_pcie_apb_writel(pcie, PCIE_MSI_TRANS_ENABLE, PCIE_MSI_TRANS_REG);
+
+	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_DBI_MODE);
+}
+
+static int hisi_pcie_link_up(struct pcie_port *pp)
+{
+	u32 val;
+
+	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
+
+	val = hisi_pcie_subctrl_readl(hisi_pcie, PCIE_SUBCTRL_SYS_STATE4_REG +
+				      0x100 * hisi_pcie->port_id);
+
+	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
+}
+
+static
+int hisi_pcie_msi_host_init(struct pcie_port *pp, struct msi_controller *chip)
+{
+	struct device_node *msi_node;
+	struct irq_domain *irq_domain;
+	struct device_node *np = pp->dev->of_node;
+
+	msi_node = of_parse_phandle(np, "msi-parent", 0);
+	if (!msi_node) {
+		pr_err("failed to find msi-parent\n");
+		return -ENODEV;
+	}
+
+	irq_domain = irq_find_host(msi_node);
+	if (!irq_domain) {
+		pr_err("failed to find irq domain\n");
+		return -ENODEV;
+	}
+
+	pp->irq_domain = irq_domain;
+
+	return 0;
+}
+
+static struct pcie_host_ops hisi_pcie_host_ops = {
+	.link_up = hisi_pcie_link_up,
+	.msi_host_init = hisi_pcie_msi_host_init,
+};
+
+static int __init hisi_add_pcie_port(struct pcie_port *pp,
+				     struct platform_device *pdev)
+{
+	int ret;
+	u32 port_id;
+	struct resource busn;
+
+	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
+
+	if (of_property_read_u32(pdev->dev.of_node, "port-id", &port_id)) {
+		dev_err(&pdev->dev, "failed to read port-id\n");
+		return -EINVAL;
+	}
+	if (port_id > 3) {
+		dev_err(&pdev->dev, "Invalid port-id\n");
+		return -EINVAL;
+	}
+
+	hisi_pcie->port_id = port_id;
+
+	if (of_pci_parse_bus_range(pdev->dev.of_node, &busn)) {
+		dev_err(&pdev->dev, "failed to parse bus-ranges\n");
+		return -EINVAL;
+	}
+
+	pp->root_bus_nr = busn.start;
+	pp->ops = &hisi_pcie_host_ops;
+
+	hisi_pcie_config_context(hisi_pcie);
+
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize host\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int __init hisi_pcie_probe(struct platform_device *pdev)
+{
+	struct hisi_pcie *hisi_pcie;
+	struct pcie_port *pp;
+	struct resource *reg;
+	struct resource *subctrl;
+	int ret;
+
+	hisi_pcie = devm_kzalloc(&pdev->dev, sizeof(*hisi_pcie), GFP_KERNEL);
+	if (!hisi_pcie)
+		return -ENOMEM;
+
+	pp = &hisi_pcie->pp;
+	pp->dev = &pdev->dev;
+
+	subctrl = platform_get_resource_byname(pdev, IORESOURCE_MEM, "subctrl");
+	hisi_pcie->subctrl_base = devm_ioremap_nocache(&pdev->dev,
+					subctrl->start, resource_size(subctrl));
+	if (IS_ERR(hisi_pcie->subctrl_base)) {
+		dev_err(pp->dev, "cannot get subctrl base\n");
+		return PTR_ERR(hisi_pcie->subctrl_base);
+	}
+
+	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
+	hisi_pcie->reg_base = devm_ioremap_resource(&pdev->dev, reg);
+	if (IS_ERR(hisi_pcie->reg_base)) {
+		dev_err(pp->dev, "cannot get reg base\n");
+		return PTR_ERR(hisi_pcie->reg_base);
+	}
+
+	hisi_pcie->pp.dbi_base = hisi_pcie->reg_base;
+
+	ret = hisi_add_pcie_port(pp, pdev);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, hisi_pcie);
+
+	return ret;
+}
+
+static const struct of_device_id hisi_pcie_of_match[] = {
+	{.compatible = "hisilicon,hip05-pcie",},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, hisi_pcie_of_match);
+
+static struct platform_driver hisi_pcie_driver = {
+	.probe  = hisi_pcie_probe,
+	.driver = {
+		   .name = "hisi-pcie",
+		   .owner = THIS_MODULE,
+		   .of_match_table = hisi_pcie_of_match,
+	},
+};
+
+module_platform_driver(hisi_pcie_driver);
-- 
1.9.1

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

* [PATCH v4 3/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
@ 2015-07-21  6:48   ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: linux-arm-kernel

This patch adds PCIe host support for Hisilicon Soc Hip05.

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 drivers/pci/host/Kconfig     |   5 +
 drivers/pci/host/Makefile    |   1 +
 drivers/pci/host/pcie-hisi.c | 257 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 drivers/pci/host/pcie-hisi.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index c132bdd..37f2075 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -145,4 +145,9 @@ config PCIE_IPROC_BCMA
 	  Say Y here if you want to use the Broadcom iProc PCIe controller
 	  through the BCMA bus interface
 
+config PCI_HISI
+	depends on OF && ARM64
+	bool "Hisilicon Soc HIP05 PCIe controller"
+	select PCIE_DW
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 140d66f..ea1dbf2 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
 obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
 obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
 obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
+obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
new file mode 100644
index 0000000..9f2fac1
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi.c
@@ -0,0 +1,257 @@
+/*
+ * PCIe host controller driver for Hisilicon Hip05 SoCs
+ *
+ * Copyright (C) 2015 Hisilicon Co., Ltd. http://www.hisilicon.com
+ *
+ * Author: Zhou Wang <wangzhou1@hisilicon.com>
+ *         Dacai Zhu <zhudacai@hisilicon.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/interrupt.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_pci.h>
+#include <linux/platform_device.h>
+
+#include "pcie-designware.h"
+
+#define PCIE_SUBCTRL_MODE_REG                           0x2800
+#define PCIE_SUBCTRL_SYS_STATE4_REG                     0x6818
+#define PCIE_SLV_DBI_MODE                               0x0
+#define PCIE_SLV_SYSCTRL_MODE                           0x1
+#define PCIE_SLV_CONTENT_MODE                           0x2
+#define PCIE_SLV_MSI_ASID                               0x10
+#define PCIE_LTSSM_LINKUP_STATE                         0x11
+#define PCIE_LTSSM_STATE_MASK                           0x3F
+#define PCIE_MSI_ASID_ENABLE                            (0x1 << 12)
+#define PCIE_MSI_ASID_VALUE                             (0x1 << 16)
+#define PCIE_MSI_TRANS_ENABLE                           (0x1 << 12)
+#define PCIE_MSI_TRANS_REG                              0x1c8
+#define PCIE_MSI_LOW_ADDRESS                            0x1b4
+#define PCIE_MSI_HIGH_ADDRESS                           0x1c4
+#define PCIE_MSI_ADDRESS_VAL                            0xb7010040
+
+#define to_hisi_pcie(x)	container_of(x, struct hisi_pcie, pp)
+
+struct hisi_pcie {
+	void __iomem *subctrl_base;
+	void __iomem *reg_base;
+	struct msi_controller *msi;
+	u32 port_id;
+	struct pcie_port pp;
+};
+
+static inline void hisi_pcie_subctrl_writel(struct hisi_pcie *pcie,
+					    u32 val, u32 reg)
+{
+	writel(val, pcie->subctrl_base + reg);
+}
+
+static inline u32 hisi_pcie_subctrl_readl(struct hisi_pcie *pcie, u32 reg)
+{
+	return readl(pcie->subctrl_base + reg);
+}
+
+static inline void hisi_pcie_apb_writel(struct hisi_pcie *pcie,
+					u32 val, u32 reg)
+{
+	writel(val, pcie->reg_base + reg);
+}
+
+static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *pcie, u32 reg)
+{
+	return readl(pcie->reg_base + reg);
+}
+
+/*
+ * Change mode to indicate the same reg_base to base of PCIe host configure
+ * registers, base of RC configure space or base of vmid/asid context table
+ */
+static void hisi_pcie_change_apb_mode(struct hisi_pcie *pcie, u32 mode)
+{
+	u32 val;
+	u32 bit_mask;
+	u32 bit_shift;
+	u32 port_id = pcie->port_id;
+	u32 reg = PCIE_SUBCTRL_MODE_REG + 0x100 * port_id;
+
+	if ((port_id == 1) || (port_id == 2)) {
+		bit_mask = 0xc;
+		bit_shift = 0x2;
+	} else {
+		bit_mask = 0x6;
+		bit_shift = 0x1;
+	}
+
+	val = hisi_pcie_subctrl_readl(pcie, reg);
+	val = (val & (~bit_mask)) | (mode << bit_shift);
+	hisi_pcie_subctrl_writel(pcie, val, reg);
+}
+
+/* Configure vmid/asid table in PCIe host */
+static void hisi_pcie_config_context(struct hisi_pcie *pcie)
+{
+	int i;
+
+	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_CONTENT_MODE);
+
+	/*
+	 * init vmid and asid tables for all PCIes device as 0
+	 * vmid table: 0 ~ 0x3ff, asid table: 0x400 ~ 0x7ff
+	 */
+	for (i = 0; i < 0x800; i++)
+		hisi_pcie_apb_writel(pcie, 0x0, i * 4);
+
+	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_SYSCTRL_MODE);
+
+	hisi_pcie_apb_writel(pcie, PCIE_MSI_ADDRESS_VAL, PCIE_MSI_LOW_ADDRESS);
+	hisi_pcie_apb_writel(pcie, 0x0, PCIE_MSI_HIGH_ADDRESS);
+	hisi_pcie_apb_writel(pcie, PCIE_MSI_ASID_ENABLE | PCIE_MSI_ASID_VALUE,
+			     PCIE_SLV_MSI_ASID);
+	hisi_pcie_apb_writel(pcie, PCIE_MSI_TRANS_ENABLE, PCIE_MSI_TRANS_REG);
+
+	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_DBI_MODE);
+}
+
+static int hisi_pcie_link_up(struct pcie_port *pp)
+{
+	u32 val;
+
+	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
+
+	val = hisi_pcie_subctrl_readl(hisi_pcie, PCIE_SUBCTRL_SYS_STATE4_REG +
+				      0x100 * hisi_pcie->port_id);
+
+	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
+}
+
+static
+int hisi_pcie_msi_host_init(struct pcie_port *pp, struct msi_controller *chip)
+{
+	struct device_node *msi_node;
+	struct irq_domain *irq_domain;
+	struct device_node *np = pp->dev->of_node;
+
+	msi_node = of_parse_phandle(np, "msi-parent", 0);
+	if (!msi_node) {
+		pr_err("failed to find msi-parent\n");
+		return -ENODEV;
+	}
+
+	irq_domain = irq_find_host(msi_node);
+	if (!irq_domain) {
+		pr_err("failed to find irq domain\n");
+		return -ENODEV;
+	}
+
+	pp->irq_domain = irq_domain;
+
+	return 0;
+}
+
+static struct pcie_host_ops hisi_pcie_host_ops = {
+	.link_up = hisi_pcie_link_up,
+	.msi_host_init = hisi_pcie_msi_host_init,
+};
+
+static int __init hisi_add_pcie_port(struct pcie_port *pp,
+				     struct platform_device *pdev)
+{
+	int ret;
+	u32 port_id;
+	struct resource busn;
+
+	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
+
+	if (of_property_read_u32(pdev->dev.of_node, "port-id", &port_id)) {
+		dev_err(&pdev->dev, "failed to read port-id\n");
+		return -EINVAL;
+	}
+	if (port_id > 3) {
+		dev_err(&pdev->dev, "Invalid port-id\n");
+		return -EINVAL;
+	}
+
+	hisi_pcie->port_id = port_id;
+
+	if (of_pci_parse_bus_range(pdev->dev.of_node, &busn)) {
+		dev_err(&pdev->dev, "failed to parse bus-ranges\n");
+		return -EINVAL;
+	}
+
+	pp->root_bus_nr = busn.start;
+	pp->ops = &hisi_pcie_host_ops;
+
+	hisi_pcie_config_context(hisi_pcie);
+
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize host\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int __init hisi_pcie_probe(struct platform_device *pdev)
+{
+	struct hisi_pcie *hisi_pcie;
+	struct pcie_port *pp;
+	struct resource *reg;
+	struct resource *subctrl;
+	int ret;
+
+	hisi_pcie = devm_kzalloc(&pdev->dev, sizeof(*hisi_pcie), GFP_KERNEL);
+	if (!hisi_pcie)
+		return -ENOMEM;
+
+	pp = &hisi_pcie->pp;
+	pp->dev = &pdev->dev;
+
+	subctrl = platform_get_resource_byname(pdev, IORESOURCE_MEM, "subctrl");
+	hisi_pcie->subctrl_base = devm_ioremap_nocache(&pdev->dev,
+					subctrl->start, resource_size(subctrl));
+	if (IS_ERR(hisi_pcie->subctrl_base)) {
+		dev_err(pp->dev, "cannot get subctrl base\n");
+		return PTR_ERR(hisi_pcie->subctrl_base);
+	}
+
+	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
+	hisi_pcie->reg_base = devm_ioremap_resource(&pdev->dev, reg);
+	if (IS_ERR(hisi_pcie->reg_base)) {
+		dev_err(pp->dev, "cannot get reg base\n");
+		return PTR_ERR(hisi_pcie->reg_base);
+	}
+
+	hisi_pcie->pp.dbi_base = hisi_pcie->reg_base;
+
+	ret = hisi_add_pcie_port(pp, pdev);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, hisi_pcie);
+
+	return ret;
+}
+
+static const struct of_device_id hisi_pcie_of_match[] = {
+	{.compatible = "hisilicon,hip05-pcie",},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, hisi_pcie_of_match);
+
+static struct platform_driver hisi_pcie_driver = {
+	.probe  = hisi_pcie_probe,
+	.driver = {
+		   .name = "hisi-pcie",
+		   .owner = THIS_MODULE,
+		   .of_match_table = hisi_pcie_of_match,
+	},
+};
+
+module_platform_driver(hisi_pcie_driver);
-- 
1.9.1

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

* [PATCH v4 3/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
@ 2015-07-21  6:48   ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni, lorenzo.pieralisi, James Morse, Liviu.Dudau
  Cc: linux-pci, linux-arm-kernel, devicetree, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liudongdong3, qiujiang, kangfenglong,
	liguozhu, Zhou Wang

This patch adds PCIe host support for Hisilicon Soc Hip05.

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 drivers/pci/host/Kconfig     |   5 +
 drivers/pci/host/Makefile    |   1 +
 drivers/pci/host/pcie-hisi.c | 257 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 263 insertions(+)
 create mode 100644 drivers/pci/host/pcie-hisi.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index c132bdd..37f2075 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -145,4 +145,9 @@ config PCIE_IPROC_BCMA
 	  Say Y here if you want to use the Broadcom iProc PCIe controller
 	  through the BCMA bus interface
 
+config PCI_HISI
+	depends on OF && ARM64
+	bool "Hisilicon Soc HIP05 PCIe controller"
+	select PCIE_DW
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 140d66f..ea1dbf2 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
 obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
 obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
 obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
+obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
new file mode 100644
index 0000000..9f2fac1
--- /dev/null
+++ b/drivers/pci/host/pcie-hisi.c
@@ -0,0 +1,257 @@
+/*
+ * PCIe host controller driver for Hisilicon Hip05 SoCs
+ *
+ * Copyright (C) 2015 Hisilicon Co., Ltd. http://www.hisilicon.com
+ *
+ * Author: Zhou Wang <wangzhou1@hisilicon.com>
+ *         Dacai Zhu <zhudacai@hisilicon.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/interrupt.h>
+#include <linux/irqdomain.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_pci.h>
+#include <linux/platform_device.h>
+
+#include "pcie-designware.h"
+
+#define PCIE_SUBCTRL_MODE_REG                           0x2800
+#define PCIE_SUBCTRL_SYS_STATE4_REG                     0x6818
+#define PCIE_SLV_DBI_MODE                               0x0
+#define PCIE_SLV_SYSCTRL_MODE                           0x1
+#define PCIE_SLV_CONTENT_MODE                           0x2
+#define PCIE_SLV_MSI_ASID                               0x10
+#define PCIE_LTSSM_LINKUP_STATE                         0x11
+#define PCIE_LTSSM_STATE_MASK                           0x3F
+#define PCIE_MSI_ASID_ENABLE                            (0x1 << 12)
+#define PCIE_MSI_ASID_VALUE                             (0x1 << 16)
+#define PCIE_MSI_TRANS_ENABLE                           (0x1 << 12)
+#define PCIE_MSI_TRANS_REG                              0x1c8
+#define PCIE_MSI_LOW_ADDRESS                            0x1b4
+#define PCIE_MSI_HIGH_ADDRESS                           0x1c4
+#define PCIE_MSI_ADDRESS_VAL                            0xb7010040
+
+#define to_hisi_pcie(x)	container_of(x, struct hisi_pcie, pp)
+
+struct hisi_pcie {
+	void __iomem *subctrl_base;
+	void __iomem *reg_base;
+	struct msi_controller *msi;
+	u32 port_id;
+	struct pcie_port pp;
+};
+
+static inline void hisi_pcie_subctrl_writel(struct hisi_pcie *pcie,
+					    u32 val, u32 reg)
+{
+	writel(val, pcie->subctrl_base + reg);
+}
+
+static inline u32 hisi_pcie_subctrl_readl(struct hisi_pcie *pcie, u32 reg)
+{
+	return readl(pcie->subctrl_base + reg);
+}
+
+static inline void hisi_pcie_apb_writel(struct hisi_pcie *pcie,
+					u32 val, u32 reg)
+{
+	writel(val, pcie->reg_base + reg);
+}
+
+static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *pcie, u32 reg)
+{
+	return readl(pcie->reg_base + reg);
+}
+
+/*
+ * Change mode to indicate the same reg_base to base of PCIe host configure
+ * registers, base of RC configure space or base of vmid/asid context table
+ */
+static void hisi_pcie_change_apb_mode(struct hisi_pcie *pcie, u32 mode)
+{
+	u32 val;
+	u32 bit_mask;
+	u32 bit_shift;
+	u32 port_id = pcie->port_id;
+	u32 reg = PCIE_SUBCTRL_MODE_REG + 0x100 * port_id;
+
+	if ((port_id == 1) || (port_id == 2)) {
+		bit_mask = 0xc;
+		bit_shift = 0x2;
+	} else {
+		bit_mask = 0x6;
+		bit_shift = 0x1;
+	}
+
+	val = hisi_pcie_subctrl_readl(pcie, reg);
+	val = (val & (~bit_mask)) | (mode << bit_shift);
+	hisi_pcie_subctrl_writel(pcie, val, reg);
+}
+
+/* Configure vmid/asid table in PCIe host */
+static void hisi_pcie_config_context(struct hisi_pcie *pcie)
+{
+	int i;
+
+	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_CONTENT_MODE);
+
+	/*
+	 * init vmid and asid tables for all PCIes device as 0
+	 * vmid table: 0 ~ 0x3ff, asid table: 0x400 ~ 0x7ff
+	 */
+	for (i = 0; i < 0x800; i++)
+		hisi_pcie_apb_writel(pcie, 0x0, i * 4);
+
+	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_SYSCTRL_MODE);
+
+	hisi_pcie_apb_writel(pcie, PCIE_MSI_ADDRESS_VAL, PCIE_MSI_LOW_ADDRESS);
+	hisi_pcie_apb_writel(pcie, 0x0, PCIE_MSI_HIGH_ADDRESS);
+	hisi_pcie_apb_writel(pcie, PCIE_MSI_ASID_ENABLE | PCIE_MSI_ASID_VALUE,
+			     PCIE_SLV_MSI_ASID);
+	hisi_pcie_apb_writel(pcie, PCIE_MSI_TRANS_ENABLE, PCIE_MSI_TRANS_REG);
+
+	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_DBI_MODE);
+}
+
+static int hisi_pcie_link_up(struct pcie_port *pp)
+{
+	u32 val;
+
+	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
+
+	val = hisi_pcie_subctrl_readl(hisi_pcie, PCIE_SUBCTRL_SYS_STATE4_REG +
+				      0x100 * hisi_pcie->port_id);
+
+	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
+}
+
+static
+int hisi_pcie_msi_host_init(struct pcie_port *pp, struct msi_controller *chip)
+{
+	struct device_node *msi_node;
+	struct irq_domain *irq_domain;
+	struct device_node *np = pp->dev->of_node;
+
+	msi_node = of_parse_phandle(np, "msi-parent", 0);
+	if (!msi_node) {
+		pr_err("failed to find msi-parent\n");
+		return -ENODEV;
+	}
+
+	irq_domain = irq_find_host(msi_node);
+	if (!irq_domain) {
+		pr_err("failed to find irq domain\n");
+		return -ENODEV;
+	}
+
+	pp->irq_domain = irq_domain;
+
+	return 0;
+}
+
+static struct pcie_host_ops hisi_pcie_host_ops = {
+	.link_up = hisi_pcie_link_up,
+	.msi_host_init = hisi_pcie_msi_host_init,
+};
+
+static int __init hisi_add_pcie_port(struct pcie_port *pp,
+				     struct platform_device *pdev)
+{
+	int ret;
+	u32 port_id;
+	struct resource busn;
+
+	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
+
+	if (of_property_read_u32(pdev->dev.of_node, "port-id", &port_id)) {
+		dev_err(&pdev->dev, "failed to read port-id\n");
+		return -EINVAL;
+	}
+	if (port_id > 3) {
+		dev_err(&pdev->dev, "Invalid port-id\n");
+		return -EINVAL;
+	}
+
+	hisi_pcie->port_id = port_id;
+
+	if (of_pci_parse_bus_range(pdev->dev.of_node, &busn)) {
+		dev_err(&pdev->dev, "failed to parse bus-ranges\n");
+		return -EINVAL;
+	}
+
+	pp->root_bus_nr = busn.start;
+	pp->ops = &hisi_pcie_host_ops;
+
+	hisi_pcie_config_context(hisi_pcie);
+
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize host\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int __init hisi_pcie_probe(struct platform_device *pdev)
+{
+	struct hisi_pcie *hisi_pcie;
+	struct pcie_port *pp;
+	struct resource *reg;
+	struct resource *subctrl;
+	int ret;
+
+	hisi_pcie = devm_kzalloc(&pdev->dev, sizeof(*hisi_pcie), GFP_KERNEL);
+	if (!hisi_pcie)
+		return -ENOMEM;
+
+	pp = &hisi_pcie->pp;
+	pp->dev = &pdev->dev;
+
+	subctrl = platform_get_resource_byname(pdev, IORESOURCE_MEM, "subctrl");
+	hisi_pcie->subctrl_base = devm_ioremap_nocache(&pdev->dev,
+					subctrl->start, resource_size(subctrl));
+	if (IS_ERR(hisi_pcie->subctrl_base)) {
+		dev_err(pp->dev, "cannot get subctrl base\n");
+		return PTR_ERR(hisi_pcie->subctrl_base);
+	}
+
+	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
+	hisi_pcie->reg_base = devm_ioremap_resource(&pdev->dev, reg);
+	if (IS_ERR(hisi_pcie->reg_base)) {
+		dev_err(pp->dev, "cannot get reg base\n");
+		return PTR_ERR(hisi_pcie->reg_base);
+	}
+
+	hisi_pcie->pp.dbi_base = hisi_pcie->reg_base;
+
+	ret = hisi_add_pcie_port(pp, pdev);
+	if (ret < 0)
+		return ret;
+
+	platform_set_drvdata(pdev, hisi_pcie);
+
+	return ret;
+}
+
+static const struct of_device_id hisi_pcie_of_match[] = {
+	{.compatible = "hisilicon,hip05-pcie",},
+	{},
+};
+
+MODULE_DEVICE_TABLE(of, hisi_pcie_of_match);
+
+static struct platform_driver hisi_pcie_driver = {
+	.probe  = hisi_pcie_probe,
+	.driver = {
+		   .name = "hisi-pcie",
+		   .owner = THIS_MODULE,
+		   .of_match_table = hisi_pcie_of_match,
+	},
+};
+
+module_platform_driver(hisi_pcie_driver);
-- 
1.9.1


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

* [PATCH v4 4/5] Documentation: DT: Add Hisilicon PCIe host binding
  2015-07-21  6:48 ` Zhou Wang
  (?)
@ 2015-07-21  6:48     ` Zhou Wang
  -1 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	lorenzo.pieralisi-5wv7dgnIgG8, James Morse,
	Liviu.Dudau-5wv7dgnIgG8
  Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	yuanzhichang-C8/M+/jPZTeaMJb+Lgu22Q,
	zhudacai-C8/M+/jPZTeaMJb+Lgu22Q,
	zhangjukuo-hv44wF8Li93QT0dZR+AlfA,
	qiuzhenfa-C8/M+/jPZTeaMJb+Lgu22Q,
	liudongdong3-hv44wF8Li93QT0dZR+AlfA,
	qiujiang-hv44wF8Li93QT0dZR+AlfA,
	kangfenglong-hv44wF8Li93QT0dZR+AlfA,
	liguozhu-C8/M+/jPZTeaMJb+Lgu22Q, Zhou Wang

This patch adds related DTS binding document for Hisilicon PCIe host driver.

Signed-off-by: Zhou Wang <wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
---
 .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
new file mode 100644
index 0000000..6c9b827
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
@@ -0,0 +1,46 @@
+Hisilicon PCIe host bridge DT description
+
+Hisilicon PCIe host controller is based on Designware PCI core.
+It shares common functions with PCIe Designware core driver and inherits
+common properties defined in
+Documentation/devicetree/bindings/pci/designware-pci.txt.
+
+Additional properties are described here:
+
+Required properties:
+- compatible: Should contain "hisilicon,hip05-pcie".
+- reg: Should contain rc_dbi, subctrl, config registers location and length.
+- reg-names: Must include the following entries:
+  "rc_dbi": controller configuration registers;
+  "subctrl": whole PCIe hosts configuration registers;
+  "config": PCIe configuration space registers.
+- msi-parent: Should be its_pcie which is an its receiving MSI interrupts.
+- port-id: Should be 0, 1, 2 or 3.
+
+Optional properties:
+- status: Either "ok" or "disabled".
+- dma-coherent: Present if dma operations are coherent.
+
+Example:
+	pcie@0xb0080000 {
+		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
+		reg = <0 0xb0080000 0 0x10000>, <0 0xb0000000 0 0x10000>,
+		      <0x220 0x00000000 0 0x2000>;
+		reg-names = "rc_dbi", "subctrl", "config";
+		bus-range = <0  15>;
+		msi-parent = <&its_pcie>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		dma-coherent;
+		ranges = <0x82000000 0 0x00000000 0x220 0x00000000 0 0x10000000>;
+		num-lanes = <8>;
+		port-id = <1>;
+		#interrupts-cells = <1>;
+		interrupts-map-mask = <0xf800 0 0 7>;
+		interrupts-map = <0x0 0 0 1 &mbigen_pcie 1 10
+				  0x0 0 0 2 &mbigen_pcie 2 11
+				  0x0 0 0 3 &mbigen_pcie 3 12
+				  0x0 0 0 4 &mbigen_pcie 4 13>;
+		status = "ok";
+	};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH v4 4/5] Documentation: DT: Add Hisilicon PCIe host binding
@ 2015-07-21  6:48     ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: linux-arm-kernel

This patch adds related DTS binding document for Hisilicon PCIe host driver.

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
new file mode 100644
index 0000000..6c9b827
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
@@ -0,0 +1,46 @@
+Hisilicon PCIe host bridge DT description
+
+Hisilicon PCIe host controller is based on Designware PCI core.
+It shares common functions with PCIe Designware core driver and inherits
+common properties defined in
+Documentation/devicetree/bindings/pci/designware-pci.txt.
+
+Additional properties are described here:
+
+Required properties:
+- compatible: Should contain "hisilicon,hip05-pcie".
+- reg: Should contain rc_dbi, subctrl, config registers location and length.
+- reg-names: Must include the following entries:
+  "rc_dbi": controller configuration registers;
+  "subctrl": whole PCIe hosts configuration registers;
+  "config": PCIe configuration space registers.
+- msi-parent: Should be its_pcie which is an its receiving MSI interrupts.
+- port-id: Should be 0, 1, 2 or 3.
+
+Optional properties:
+- status: Either "ok" or "disabled".
+- dma-coherent: Present if dma operations are coherent.
+
+Example:
+	pcie at 0xb0080000 {
+		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
+		reg = <0 0xb0080000 0 0x10000>, <0 0xb0000000 0 0x10000>,
+		      <0x220 0x00000000 0 0x2000>;
+		reg-names = "rc_dbi", "subctrl", "config";
+		bus-range = <0  15>;
+		msi-parent = <&its_pcie>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		dma-coherent;
+		ranges = <0x82000000 0 0x00000000 0x220 0x00000000 0 0x10000000>;
+		num-lanes = <8>;
+		port-id = <1>;
+		#interrupts-cells = <1>;
+		interrupts-map-mask = <0xf800 0 0 7>;
+		interrupts-map = <0x0 0 0 1 &mbigen_pcie 1 10
+				  0x0 0 0 2 &mbigen_pcie 2 11
+				  0x0 0 0 3 &mbigen_pcie 3 12
+				  0x0 0 0 4 &mbigen_pcie 4 13>;
+		status = "ok";
+	};
-- 
1.9.1

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

* [PATCH v4 4/5] Documentation: DT: Add Hisilicon PCIe host binding
@ 2015-07-21  6:48     ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni, lorenzo.pieralisi, James Morse, Liviu.Dudau
  Cc: linux-pci, linux-arm-kernel, devicetree, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liudongdong3, qiujiang, kangfenglong,
	liguozhu, Zhou Wang

This patch adds related DTS binding document for Hisilicon PCIe host driver.

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
new file mode 100644
index 0000000..6c9b827
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
@@ -0,0 +1,46 @@
+Hisilicon PCIe host bridge DT description
+
+Hisilicon PCIe host controller is based on Designware PCI core.
+It shares common functions with PCIe Designware core driver and inherits
+common properties defined in
+Documentation/devicetree/bindings/pci/designware-pci.txt.
+
+Additional properties are described here:
+
+Required properties:
+- compatible: Should contain "hisilicon,hip05-pcie".
+- reg: Should contain rc_dbi, subctrl, config registers location and length.
+- reg-names: Must include the following entries:
+  "rc_dbi": controller configuration registers;
+  "subctrl": whole PCIe hosts configuration registers;
+  "config": PCIe configuration space registers.
+- msi-parent: Should be its_pcie which is an its receiving MSI interrupts.
+- port-id: Should be 0, 1, 2 or 3.
+
+Optional properties:
+- status: Either "ok" or "disabled".
+- dma-coherent: Present if dma operations are coherent.
+
+Example:
+	pcie@0xb0080000 {
+		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
+		reg = <0 0xb0080000 0 0x10000>, <0 0xb0000000 0 0x10000>,
+		      <0x220 0x00000000 0 0x2000>;
+		reg-names = "rc_dbi", "subctrl", "config";
+		bus-range = <0  15>;
+		msi-parent = <&its_pcie>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		dma-coherent;
+		ranges = <0x82000000 0 0x00000000 0x220 0x00000000 0 0x10000000>;
+		num-lanes = <8>;
+		port-id = <1>;
+		#interrupts-cells = <1>;
+		interrupts-map-mask = <0xf800 0 0 7>;
+		interrupts-map = <0x0 0 0 1 &mbigen_pcie 1 10
+				  0x0 0 0 2 &mbigen_pcie 2 11
+				  0x0 0 0 3 &mbigen_pcie 3 12
+				  0x0 0 0 4 &mbigen_pcie 4 13>;
+		status = "ok";
+	};
-- 
1.9.1


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

* [PATCH v4 5/5] MAINTAINERS: Add pcie-hisi maintainer
  2015-07-21  6:48 ` Zhou Wang
  (?)
@ 2015-07-21  6:48     ` Zhou Wang
  -1 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	lorenzo.pieralisi-5wv7dgnIgG8, James Morse,
	Liviu.Dudau-5wv7dgnIgG8
  Cc: linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	yuanzhichang-C8/M+/jPZTeaMJb+Lgu22Q,
	zhudacai-C8/M+/jPZTeaMJb+Lgu22Q,
	zhangjukuo-hv44wF8Li93QT0dZR+AlfA,
	qiuzhenfa-C8/M+/jPZTeaMJb+Lgu22Q,
	liudongdong3-hv44wF8Li93QT0dZR+AlfA,
	qiujiang-hv44wF8Li93QT0dZR+AlfA,
	kangfenglong-hv44wF8Li93QT0dZR+AlfA,
	liguozhu-C8/M+/jPZTeaMJb+Lgu22Q, Zhou Wang

Signed-off-by: Zhou Wang <wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8133cef..7cd8e47 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7854,6 +7854,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/pci/xgene-pci-msi.txt
 F:	drivers/pci/host/pci-xgene-msi.c
 
+PCIE DRIVER FOR HISILICON
+M:	Zhou Wang <wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
+L:	linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
+F:	drivers/pci/host/pcie-hisi.c
+
 PCMCIA SUBSYSTEM
 P:	Linux PCMCIA Team
 L:	linux-pcmcia-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH v4 5/5] MAINTAINERS: Add pcie-hisi maintainer
@ 2015-07-21  6:48     ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: linux-arm-kernel

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8133cef..7cd8e47 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7854,6 +7854,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/pci/xgene-pci-msi.txt
 F:	drivers/pci/host/pci-xgene-msi.c
 
+PCIE DRIVER FOR HISILICON
+M:	Zhou Wang <wangzhou1@hisilicon.com>
+L:	linux-pci at vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
+F:	drivers/pci/host/pcie-hisi.c
+
 PCMCIA SUBSYSTEM
 P:	Linux PCMCIA Team
 L:	linux-pcmcia at lists.infradead.org
-- 
1.9.1

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

* [PATCH v4 5/5] MAINTAINERS: Add pcie-hisi maintainer
@ 2015-07-21  6:48     ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-21  6:48 UTC (permalink / raw
  To: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni, lorenzo.pieralisi, James Morse, Liviu.Dudau
  Cc: linux-pci, linux-arm-kernel, devicetree, yuanzhichang, zhudacai,
	zhangjukuo, qiuzhenfa, liudongdong3, qiujiang, kangfenglong,
	liguozhu, Zhou Wang

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
---
 MAINTAINERS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8133cef..7cd8e47 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7854,6 +7854,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/pci/xgene-pci-msi.txt
 F:	drivers/pci/host/pci-xgene-msi.c
 
+PCIE DRIVER FOR HISILICON
+M:	Zhou Wang <wangzhou1@hisilicon.com>
+L:	linux-pci@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
+F:	drivers/pci/host/pcie-hisi.c
+
 PCMCIA SUBSYSTEM
 P:	Linux PCMCIA Team
 L:	linux-pcmcia@lists.infradead.org
-- 
1.9.1


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

* Re: [PATCH v4 3/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
  2015-07-21  6:48   ` Zhou Wang
@ 2015-07-21 22:37     ` Bjorn Helgaas
  -1 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2015-07-21 22:37 UTC (permalink / raw
  To: Zhou Wang
  Cc: Jingoo Han, Pratyush Anand, Arnd Bergmann, gabriele.paoloni,
	lorenzo.pieralisi, James Morse, Liviu.Dudau, linux-pci,
	linux-arm-kernel, devicetree, yuanzhichang, zhudacai, zhangjukuo,
	qiuzhenfa, liudongdong3, qiujiang, kangfenglong, liguozhu

Hi Zhou,

On Tue, Jul 21, 2015 at 02:48:41PM +0800, Zhou Wang wrote:
> This patch adds PCIe host support for Hisilicon Soc Hip05.
> 
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> ---
>  drivers/pci/host/Kconfig     |   5 +
>  drivers/pci/host/Makefile    |   1 +
>  drivers/pci/host/pcie-hisi.c | 257 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 263 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-hisi.c
> 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index c132bdd..37f2075 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -145,4 +145,9 @@ config PCIE_IPROC_BCMA
>  	  Say Y here if you want to use the Broadcom iProc PCIe controller
>  	  through the BCMA bus interface
>  
> +config PCI_HISI
> +	depends on OF && ARM64
> +	bool "Hisilicon Soc HIP05 PCIe controller"
> +	select PCIE_DW

Almost all the other DesignWare-based drivers select PCIEPORTBUS in
addition to PCIE_DW.  I'm not sure that's the best Kconfiggery (maybe we
should leave this up to the user), but it'd be nice if all the drivers at
least did this consistently.  So do you have a reason for doing it
differently?

>  endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 140d66f..ea1dbf2 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
>  obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
>  obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
>  obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
> +obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
> diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
> new file mode 100644
> index 0000000..9f2fac1
> --- /dev/null
> +++ b/drivers/pci/host/pcie-hisi.c
> @@ -0,0 +1,257 @@
> +/*
> + * PCIe host controller driver for Hisilicon Hip05 SoCs
> + *
> + * Copyright (C) 2015 Hisilicon Co., Ltd. http://www.hisilicon.com
> + *
> + * Author: Zhou Wang <wangzhou1@hisilicon.com>
> + *         Dacai Zhu <zhudacai@hisilicon.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/interrupt.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_pci.h>
> +#include <linux/platform_device.h>
> +
> +#include "pcie-designware.h"
> +
> +#define PCIE_SUBCTRL_MODE_REG                           0x2800
> +#define PCIE_SUBCTRL_SYS_STATE4_REG                     0x6818
> +#define PCIE_SLV_DBI_MODE                               0x0
> +#define PCIE_SLV_SYSCTRL_MODE                           0x1
> +#define PCIE_SLV_CONTENT_MODE                           0x2
> +#define PCIE_SLV_MSI_ASID                               0x10
> +#define PCIE_LTSSM_LINKUP_STATE                         0x11
> +#define PCIE_LTSSM_STATE_MASK                           0x3F
> +#define PCIE_MSI_ASID_ENABLE                            (0x1 << 12)
> +#define PCIE_MSI_ASID_VALUE                             (0x1 << 16)
> +#define PCIE_MSI_TRANS_ENABLE                           (0x1 << 12)
> +#define PCIE_MSI_TRANS_REG                              0x1c8
> +#define PCIE_MSI_LOW_ADDRESS                            0x1b4
> +#define PCIE_MSI_HIGH_ADDRESS                           0x1c4
> +#define PCIE_MSI_ADDRESS_VAL                            0xb7010040
> +
> +#define to_hisi_pcie(x)	container_of(x, struct hisi_pcie, pp)
> +
> +struct hisi_pcie {
> +	void __iomem *subctrl_base;
> +	void __iomem *reg_base;
> +	struct msi_controller *msi;
> +	u32 port_id;
> +	struct pcie_port pp;
> +};
> +
> +static inline void hisi_pcie_subctrl_writel(struct hisi_pcie *pcie,
> +					    u32 val, u32 reg)
> +{
> +	writel(val, pcie->subctrl_base + reg);
> +}
> +
> +static inline u32 hisi_pcie_subctrl_readl(struct hisi_pcie *pcie, u32 reg)
> +{
> +	return readl(pcie->subctrl_base + reg);
> +}
> +
> +static inline void hisi_pcie_apb_writel(struct hisi_pcie *pcie,
> +					u32 val, u32 reg)
> +{
> +	writel(val, pcie->reg_base + reg);
> +}
> +
> +static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *pcie, u32 reg)
> +{
> +	return readl(pcie->reg_base + reg);
> +}
> +
> +/*
> + * Change mode to indicate the same reg_base to base of PCIe host configure
> + * registers, base of RC configure space or base of vmid/asid context table
> + */
> +static void hisi_pcie_change_apb_mode(struct hisi_pcie *pcie, u32 mode)
> +{
> +	u32 val;
> +	u32 bit_mask;
> +	u32 bit_shift;
> +	u32 port_id = pcie->port_id;
> +	u32 reg = PCIE_SUBCTRL_MODE_REG + 0x100 * port_id;
> +
> +	if ((port_id == 1) || (port_id == 2)) {
> +		bit_mask = 0xc;
> +		bit_shift = 0x2;
> +	} else {
> +		bit_mask = 0x6;
> +		bit_shift = 0x1;
> +	}
> +
> +	val = hisi_pcie_subctrl_readl(pcie, reg);
> +	val = (val & (~bit_mask)) | (mode << bit_shift);
> +	hisi_pcie_subctrl_writel(pcie, val, reg);
> +}
> +
> +/* Configure vmid/asid table in PCIe host */
> +static void hisi_pcie_config_context(struct hisi_pcie *pcie)
> +{
> +	int i;
> +
> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_CONTENT_MODE);
> +
> +	/*
> +	 * init vmid and asid tables for all PCIes device as 0

s/PCIes device as/PCIe devices to/ ?

> +	 * vmid table: 0 ~ 0x3ff, asid table: 0x400 ~ 0x7ff
> +	 */
> +	for (i = 0; i < 0x800; i++)
> +		hisi_pcie_apb_writel(pcie, 0x0, i * 4);
> +
> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_SYSCTRL_MODE);
> +
> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_ADDRESS_VAL, PCIE_MSI_LOW_ADDRESS);
> +	hisi_pcie_apb_writel(pcie, 0x0, PCIE_MSI_HIGH_ADDRESS);
> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_ASID_ENABLE | PCIE_MSI_ASID_VALUE,
> +			     PCIE_SLV_MSI_ASID);
> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_TRANS_ENABLE, PCIE_MSI_TRANS_REG);
> +
> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_DBI_MODE);
> +}
> +
> +static int hisi_pcie_link_up(struct pcie_port *pp)
> +{
> +	u32 val;
> +

Remove this extra blank line.

> +	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
> +
> +	val = hisi_pcie_subctrl_readl(hisi_pcie, PCIE_SUBCTRL_SYS_STATE4_REG +
> +				      0x100 * hisi_pcie->port_id);
> +
> +	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
> +}
> +
> +static
> +int hisi_pcie_msi_host_init(struct pcie_port *pp, struct msi_controller *chip)
> +{
> +	struct device_node *msi_node;
> +	struct irq_domain *irq_domain;
> +	struct device_node *np = pp->dev->of_node;
> +
> +	msi_node = of_parse_phandle(np, "msi-parent", 0);
> +	if (!msi_node) {
> +		pr_err("failed to find msi-parent\n");

Can you use dev_err() here (and below)?  A message like this that doesn't
have a hint about what device it's related to is not as useful as it could
be.

> +		return -ENODEV;
> +	}
> +
> +	irq_domain = irq_find_host(msi_node);
> +	if (!irq_domain) {
> +		pr_err("failed to find irq domain\n");
> +		return -ENODEV;
> +	}
> +
> +	pp->irq_domain = irq_domain;
> +
> +	return 0;
> +}
> +
> +static struct pcie_host_ops hisi_pcie_host_ops = {
> +	.link_up = hisi_pcie_link_up,
> +	.msi_host_init = hisi_pcie_msi_host_init,
> +};
> +
> +static int __init hisi_add_pcie_port(struct pcie_port *pp,
> +				     struct platform_device *pdev)
> +{
> +	int ret;
> +	u32 port_id;
> +	struct resource busn;
> +

Remove this extra blank line.

> +	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
> +
> +	if (of_property_read_u32(pdev->dev.of_node, "port-id", &port_id)) {
> +		dev_err(&pdev->dev, "failed to read port-id\n");
> +		return -EINVAL;
> +	}
> +	if (port_id > 3) {
> +		dev_err(&pdev->dev, "Invalid port-id\n");

Since you have the invalid port_id here, maybe you could include the
invalid value in the output?

> +		return -EINVAL;
> +	}
> +
> +	hisi_pcie->port_id = port_id;
> +
> +	if (of_pci_parse_bus_range(pdev->dev.of_node, &busn)) {
> +		dev_err(&pdev->dev, "failed to parse bus-ranges\n");
> +		return -EINVAL;
> +	}
> +
> +	pp->root_bus_nr = busn.start;
> +	pp->ops = &hisi_pcie_host_ops;
> +
> +	hisi_pcie_config_context(hisi_pcie);
> +
> +	ret = dw_pcie_host_init(pp);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to initialize host\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int __init hisi_pcie_probe(struct platform_device *pdev)
> +{
> +	struct hisi_pcie *hisi_pcie;
> +	struct pcie_port *pp;
> +	struct resource *reg;
> +	struct resource *subctrl;
> +	int ret;
> +
> +	hisi_pcie = devm_kzalloc(&pdev->dev, sizeof(*hisi_pcie), GFP_KERNEL);
> +	if (!hisi_pcie)
> +		return -ENOMEM;
> +
> +	pp = &hisi_pcie->pp;
> +	pp->dev = &pdev->dev;
> +
> +	subctrl = platform_get_resource_byname(pdev, IORESOURCE_MEM, "subctrl");
> +	hisi_pcie->subctrl_base = devm_ioremap_nocache(&pdev->dev,
> +					subctrl->start, resource_size(subctrl));
> +	if (IS_ERR(hisi_pcie->subctrl_base)) {
> +		dev_err(pp->dev, "cannot get subctrl base\n");
> +		return PTR_ERR(hisi_pcie->subctrl_base);
> +	}
> +
> +	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
> +	hisi_pcie->reg_base = devm_ioremap_resource(&pdev->dev, reg);
> +	if (IS_ERR(hisi_pcie->reg_base)) {
> +		dev_err(pp->dev, "cannot get reg base\n");

Maybe this could include the actual resource name ("rc_dbi") as the message
above does?

> +		return PTR_ERR(hisi_pcie->reg_base);
> +	}
> +
> +	hisi_pcie->pp.dbi_base = hisi_pcie->reg_base;
> +
> +	ret = hisi_add_pcie_port(pp, pdev);
> +	if (ret < 0)

Seems like this return value check should be the same as the one above in 
hisi_add_pcie_port().  In both cases, the only possible returns are zero
(success) or a negative error number.  Using "if (ret)" here has the
advantage that below you could "return 0" and it would be crystal clear
that you're always returning success if you get that far.

> +		return ret;
> +
> +	platform_set_drvdata(pdev, hisi_pcie);
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id hisi_pcie_of_match[] = {
> +	{.compatible = "hisilicon,hip05-pcie",},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, hisi_pcie_of_match);
> +
> +static struct platform_driver hisi_pcie_driver = {
> +	.probe  = hisi_pcie_probe,
> +	.driver = {
> +		   .name = "hisi-pcie",
> +		   .owner = THIS_MODULE,

You can drop this ".owner =" initialization.  platform_driver_register()
supplies that automatically.

> +		   .of_match_table = hisi_pcie_of_match,
> +	},
> +};
> +
> +module_platform_driver(hisi_pcie_driver);
> -- 
> 1.9.1
> 
> --
> 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] 36+ messages in thread

* [PATCH v4 3/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
@ 2015-07-21 22:37     ` Bjorn Helgaas
  0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2015-07-21 22:37 UTC (permalink / raw
  To: linux-arm-kernel

Hi Zhou,

On Tue, Jul 21, 2015 at 02:48:41PM +0800, Zhou Wang wrote:
> This patch adds PCIe host support for Hisilicon Soc Hip05.
> 
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> ---
>  drivers/pci/host/Kconfig     |   5 +
>  drivers/pci/host/Makefile    |   1 +
>  drivers/pci/host/pcie-hisi.c | 257 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 263 insertions(+)
>  create mode 100644 drivers/pci/host/pcie-hisi.c
> 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index c132bdd..37f2075 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -145,4 +145,9 @@ config PCIE_IPROC_BCMA
>  	  Say Y here if you want to use the Broadcom iProc PCIe controller
>  	  through the BCMA bus interface
>  
> +config PCI_HISI
> +	depends on OF && ARM64
> +	bool "Hisilicon Soc HIP05 PCIe controller"
> +	select PCIE_DW

Almost all the other DesignWare-based drivers select PCIEPORTBUS in
addition to PCIE_DW.  I'm not sure that's the best Kconfiggery (maybe we
should leave this up to the user), but it'd be nice if all the drivers at
least did this consistently.  So do you have a reason for doing it
differently?

>  endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 140d66f..ea1dbf2 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
>  obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
>  obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
>  obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
> +obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
> diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
> new file mode 100644
> index 0000000..9f2fac1
> --- /dev/null
> +++ b/drivers/pci/host/pcie-hisi.c
> @@ -0,0 +1,257 @@
> +/*
> + * PCIe host controller driver for Hisilicon Hip05 SoCs
> + *
> + * Copyright (C) 2015 Hisilicon Co., Ltd. http://www.hisilicon.com
> + *
> + * Author: Zhou Wang <wangzhou1@hisilicon.com>
> + *         Dacai Zhu <zhudacai@hisilicon.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/interrupt.h>
> +#include <linux/irqdomain.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_pci.h>
> +#include <linux/platform_device.h>
> +
> +#include "pcie-designware.h"
> +
> +#define PCIE_SUBCTRL_MODE_REG                           0x2800
> +#define PCIE_SUBCTRL_SYS_STATE4_REG                     0x6818
> +#define PCIE_SLV_DBI_MODE                               0x0
> +#define PCIE_SLV_SYSCTRL_MODE                           0x1
> +#define PCIE_SLV_CONTENT_MODE                           0x2
> +#define PCIE_SLV_MSI_ASID                               0x10
> +#define PCIE_LTSSM_LINKUP_STATE                         0x11
> +#define PCIE_LTSSM_STATE_MASK                           0x3F
> +#define PCIE_MSI_ASID_ENABLE                            (0x1 << 12)
> +#define PCIE_MSI_ASID_VALUE                             (0x1 << 16)
> +#define PCIE_MSI_TRANS_ENABLE                           (0x1 << 12)
> +#define PCIE_MSI_TRANS_REG                              0x1c8
> +#define PCIE_MSI_LOW_ADDRESS                            0x1b4
> +#define PCIE_MSI_HIGH_ADDRESS                           0x1c4
> +#define PCIE_MSI_ADDRESS_VAL                            0xb7010040
> +
> +#define to_hisi_pcie(x)	container_of(x, struct hisi_pcie, pp)
> +
> +struct hisi_pcie {
> +	void __iomem *subctrl_base;
> +	void __iomem *reg_base;
> +	struct msi_controller *msi;
> +	u32 port_id;
> +	struct pcie_port pp;
> +};
> +
> +static inline void hisi_pcie_subctrl_writel(struct hisi_pcie *pcie,
> +					    u32 val, u32 reg)
> +{
> +	writel(val, pcie->subctrl_base + reg);
> +}
> +
> +static inline u32 hisi_pcie_subctrl_readl(struct hisi_pcie *pcie, u32 reg)
> +{
> +	return readl(pcie->subctrl_base + reg);
> +}
> +
> +static inline void hisi_pcie_apb_writel(struct hisi_pcie *pcie,
> +					u32 val, u32 reg)
> +{
> +	writel(val, pcie->reg_base + reg);
> +}
> +
> +static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *pcie, u32 reg)
> +{
> +	return readl(pcie->reg_base + reg);
> +}
> +
> +/*
> + * Change mode to indicate the same reg_base to base of PCIe host configure
> + * registers, base of RC configure space or base of vmid/asid context table
> + */
> +static void hisi_pcie_change_apb_mode(struct hisi_pcie *pcie, u32 mode)
> +{
> +	u32 val;
> +	u32 bit_mask;
> +	u32 bit_shift;
> +	u32 port_id = pcie->port_id;
> +	u32 reg = PCIE_SUBCTRL_MODE_REG + 0x100 * port_id;
> +
> +	if ((port_id == 1) || (port_id == 2)) {
> +		bit_mask = 0xc;
> +		bit_shift = 0x2;
> +	} else {
> +		bit_mask = 0x6;
> +		bit_shift = 0x1;
> +	}
> +
> +	val = hisi_pcie_subctrl_readl(pcie, reg);
> +	val = (val & (~bit_mask)) | (mode << bit_shift);
> +	hisi_pcie_subctrl_writel(pcie, val, reg);
> +}
> +
> +/* Configure vmid/asid table in PCIe host */
> +static void hisi_pcie_config_context(struct hisi_pcie *pcie)
> +{
> +	int i;
> +
> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_CONTENT_MODE);
> +
> +	/*
> +	 * init vmid and asid tables for all PCIes device as 0

s/PCIes device as/PCIe devices to/ ?

> +	 * vmid table: 0 ~ 0x3ff, asid table: 0x400 ~ 0x7ff
> +	 */
> +	for (i = 0; i < 0x800; i++)
> +		hisi_pcie_apb_writel(pcie, 0x0, i * 4);
> +
> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_SYSCTRL_MODE);
> +
> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_ADDRESS_VAL, PCIE_MSI_LOW_ADDRESS);
> +	hisi_pcie_apb_writel(pcie, 0x0, PCIE_MSI_HIGH_ADDRESS);
> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_ASID_ENABLE | PCIE_MSI_ASID_VALUE,
> +			     PCIE_SLV_MSI_ASID);
> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_TRANS_ENABLE, PCIE_MSI_TRANS_REG);
> +
> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_DBI_MODE);
> +}
> +
> +static int hisi_pcie_link_up(struct pcie_port *pp)
> +{
> +	u32 val;
> +

Remove this extra blank line.

> +	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
> +
> +	val = hisi_pcie_subctrl_readl(hisi_pcie, PCIE_SUBCTRL_SYS_STATE4_REG +
> +				      0x100 * hisi_pcie->port_id);
> +
> +	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
> +}
> +
> +static
> +int hisi_pcie_msi_host_init(struct pcie_port *pp, struct msi_controller *chip)
> +{
> +	struct device_node *msi_node;
> +	struct irq_domain *irq_domain;
> +	struct device_node *np = pp->dev->of_node;
> +
> +	msi_node = of_parse_phandle(np, "msi-parent", 0);
> +	if (!msi_node) {
> +		pr_err("failed to find msi-parent\n");

Can you use dev_err() here (and below)?  A message like this that doesn't
have a hint about what device it's related to is not as useful as it could
be.

> +		return -ENODEV;
> +	}
> +
> +	irq_domain = irq_find_host(msi_node);
> +	if (!irq_domain) {
> +		pr_err("failed to find irq domain\n");
> +		return -ENODEV;
> +	}
> +
> +	pp->irq_domain = irq_domain;
> +
> +	return 0;
> +}
> +
> +static struct pcie_host_ops hisi_pcie_host_ops = {
> +	.link_up = hisi_pcie_link_up,
> +	.msi_host_init = hisi_pcie_msi_host_init,
> +};
> +
> +static int __init hisi_add_pcie_port(struct pcie_port *pp,
> +				     struct platform_device *pdev)
> +{
> +	int ret;
> +	u32 port_id;
> +	struct resource busn;
> +

Remove this extra blank line.

> +	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
> +
> +	if (of_property_read_u32(pdev->dev.of_node, "port-id", &port_id)) {
> +		dev_err(&pdev->dev, "failed to read port-id\n");
> +		return -EINVAL;
> +	}
> +	if (port_id > 3) {
> +		dev_err(&pdev->dev, "Invalid port-id\n");

Since you have the invalid port_id here, maybe you could include the
invalid value in the output?

> +		return -EINVAL;
> +	}
> +
> +	hisi_pcie->port_id = port_id;
> +
> +	if (of_pci_parse_bus_range(pdev->dev.of_node, &busn)) {
> +		dev_err(&pdev->dev, "failed to parse bus-ranges\n");
> +		return -EINVAL;
> +	}
> +
> +	pp->root_bus_nr = busn.start;
> +	pp->ops = &hisi_pcie_host_ops;
> +
> +	hisi_pcie_config_context(hisi_pcie);
> +
> +	ret = dw_pcie_host_init(pp);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to initialize host\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int __init hisi_pcie_probe(struct platform_device *pdev)
> +{
> +	struct hisi_pcie *hisi_pcie;
> +	struct pcie_port *pp;
> +	struct resource *reg;
> +	struct resource *subctrl;
> +	int ret;
> +
> +	hisi_pcie = devm_kzalloc(&pdev->dev, sizeof(*hisi_pcie), GFP_KERNEL);
> +	if (!hisi_pcie)
> +		return -ENOMEM;
> +
> +	pp = &hisi_pcie->pp;
> +	pp->dev = &pdev->dev;
> +
> +	subctrl = platform_get_resource_byname(pdev, IORESOURCE_MEM, "subctrl");
> +	hisi_pcie->subctrl_base = devm_ioremap_nocache(&pdev->dev,
> +					subctrl->start, resource_size(subctrl));
> +	if (IS_ERR(hisi_pcie->subctrl_base)) {
> +		dev_err(pp->dev, "cannot get subctrl base\n");
> +		return PTR_ERR(hisi_pcie->subctrl_base);
> +	}
> +
> +	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
> +	hisi_pcie->reg_base = devm_ioremap_resource(&pdev->dev, reg);
> +	if (IS_ERR(hisi_pcie->reg_base)) {
> +		dev_err(pp->dev, "cannot get reg base\n");

Maybe this could include the actual resource name ("rc_dbi") as the message
above does?

> +		return PTR_ERR(hisi_pcie->reg_base);
> +	}
> +
> +	hisi_pcie->pp.dbi_base = hisi_pcie->reg_base;
> +
> +	ret = hisi_add_pcie_port(pp, pdev);
> +	if (ret < 0)

Seems like this return value check should be the same as the one above in 
hisi_add_pcie_port().  In both cases, the only possible returns are zero
(success) or a negative error number.  Using "if (ret)" here has the
advantage that below you could "return 0" and it would be crystal clear
that you're always returning success if you get that far.

> +		return ret;
> +
> +	platform_set_drvdata(pdev, hisi_pcie);
> +
> +	return ret;
> +}
> +
> +static const struct of_device_id hisi_pcie_of_match[] = {
> +	{.compatible = "hisilicon,hip05-pcie",},
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, hisi_pcie_of_match);
> +
> +static struct platform_driver hisi_pcie_driver = {
> +	.probe  = hisi_pcie_probe,
> +	.driver = {
> +		   .name = "hisi-pcie",
> +		   .owner = THIS_MODULE,

You can drop this ".owner =" initialization.  platform_driver_register()
supplies that automatically.

> +		   .of_match_table = hisi_pcie_of_match,
> +	},
> +};
> +
> +module_platform_driver(hisi_pcie_driver);
> -- 
> 1.9.1
> 
> --
> 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] 36+ messages in thread

* Re: [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data
  2015-07-21  6:48   ` Zhou Wang
@ 2015-07-21 22:44     ` Bjorn Helgaas
  -1 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2015-07-21 22:44 UTC (permalink / raw
  To: Zhou Wang
  Cc: Jingoo Han, Pratyush Anand, Arnd Bergmann, gabriele.paoloni,
	lorenzo.pieralisi, James Morse, Liviu.Dudau, linux-pci,
	linux-arm-kernel, devicetree, yuanzhichang, zhudacai, zhangjukuo,
	qiuzhenfa, liudongdong3, qiujiang, kangfenglong, liguozhu

Hi Zhou & Gabriele,

On Tue, Jul 21, 2015 at 02:48:39PM +0800, Zhou Wang wrote:
> This patch is needed in order to unify the PCIe designware framework for ARM and
> ARM64 architectures. In the PCIe designware unification process we are calling
> pci_create_root_bus() passing a "sysdata" parameter that is the same for both
> ARM and ARM64 and is of type "struct pcie_port*". In the ARM case this will
> cause a problem with the function pcibios_align_resource(); in fact this will
> cast "dev->sysdata" to "struct pci_sys_data*", whereas designware had passed a
> "struct pcie_port*" pointer.
> 
> This patch solves the issue by removing "align_resource" from "pci_sys_data"
> struct and defining a static global function pointer in "bios32.c"
> 
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni at huawei.com>
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>

Can you fix up Gabriele's email address above (currently contains " at ")
and verify that this sign-off chain is accurate per section 11 of
Documentation/SubmittingPatches?  The above suggests to me that Gabriele
originally wrote this patch and Zhou is passing it along.  That might well
be true; I haven't followed the development, so I'm just double-checking.

> ---
>  arch/arm/include/asm/mach/pci.h |  5 -----
>  arch/arm/kernel/bios32.c        | 12 ++++++++----
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
> index 28b9bb3..8a4e4de 100644
> --- a/arch/arm/include/asm/mach/pci.h
> +++ b/arch/arm/include/asm/mach/pci.h
> @@ -58,11 +58,6 @@ struct pci_sys_data {
>  					/* IRQ mapping				*/
>  	int		(*map_irq)(const struct pci_dev *, u8, u8);
>  					/* Resource alignement requirements	*/
> -	resource_size_t (*align_resource)(struct pci_dev *dev,
> -					  const struct resource *res,
> -					  resource_size_t start,
> -					  resource_size_t size,
> -					  resource_size_t align);
>  	void		*private_data;	/* platform controller private data	*/
>  };
>  
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index fcbbbb1..4cdc64d 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -17,6 +17,11 @@
>  #include <asm/mach/pci.h>
>  
>  static int debug_pci;
> +static resource_size_t (*align_resource)(struct pci_dev *dev,
> +		  const struct resource *res,
> +		  resource_size_t start,
> +		  resource_size_t size,
> +		  resource_size_t align) = NULL;
>  
>  #ifdef CONFIG_PCI_MSI
>  struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
> @@ -468,7 +473,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
>  		sys->busnr   = busnr;
>  		sys->swizzle = hw->swizzle;
>  		sys->map_irq = hw->map_irq;
> -		sys->align_resource = hw->align_resource;
> +		align_resource = hw->align_resource;
>  		INIT_LIST_HEAD(&sys->resources);
>  
>  		if (hw->private_data)
> @@ -589,7 +594,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  				resource_size_t size, resource_size_t align)
>  {
>  	struct pci_dev *dev = data;
> -	struct pci_sys_data *sys = dev->sysdata;
>  	resource_size_t start = res->start;
>  
>  	if (res->flags & IORESOURCE_IO && start & 0x300)
> @@ -597,8 +601,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  
>  	start = (start + align - 1) & ~(align - 1);
>  
> -	if (sys->align_resource)
> -		return sys->align_resource(dev, res, start, size, align);
> +	if (align_resource)
> +		return align_resource(dev, res, start, size, align);
>  
>  	return start;
>  }
> -- 
> 1.9.1
> 
> --
> 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] 36+ messages in thread

* [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data
@ 2015-07-21 22:44     ` Bjorn Helgaas
  0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2015-07-21 22:44 UTC (permalink / raw
  To: linux-arm-kernel

Hi Zhou & Gabriele,

On Tue, Jul 21, 2015 at 02:48:39PM +0800, Zhou Wang wrote:
> This patch is needed in order to unify the PCIe designware framework for ARM and
> ARM64 architectures. In the PCIe designware unification process we are calling
> pci_create_root_bus() passing a "sysdata" parameter that is the same for both
> ARM and ARM64 and is of type "struct pcie_port*". In the ARM case this will
> cause a problem with the function pcibios_align_resource(); in fact this will
> cast "dev->sysdata" to "struct pci_sys_data*", whereas designware had passed a
> "struct pcie_port*" pointer.
> 
> This patch solves the issue by removing "align_resource" from "pci_sys_data"
> struct and defining a static global function pointer in "bios32.c"
> 
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>

Can you fix up Gabriele's email address above (currently contains " at ")
and verify that this sign-off chain is accurate per section 11 of
Documentation/SubmittingPatches?  The above suggests to me that Gabriele
originally wrote this patch and Zhou is passing it along.  That might well
be true; I haven't followed the development, so I'm just double-checking.

> ---
>  arch/arm/include/asm/mach/pci.h |  5 -----
>  arch/arm/kernel/bios32.c        | 12 ++++++++----
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
> index 28b9bb3..8a4e4de 100644
> --- a/arch/arm/include/asm/mach/pci.h
> +++ b/arch/arm/include/asm/mach/pci.h
> @@ -58,11 +58,6 @@ struct pci_sys_data {
>  					/* IRQ mapping				*/
>  	int		(*map_irq)(const struct pci_dev *, u8, u8);
>  					/* Resource alignement requirements	*/
> -	resource_size_t (*align_resource)(struct pci_dev *dev,
> -					  const struct resource *res,
> -					  resource_size_t start,
> -					  resource_size_t size,
> -					  resource_size_t align);
>  	void		*private_data;	/* platform controller private data	*/
>  };
>  
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index fcbbbb1..4cdc64d 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -17,6 +17,11 @@
>  #include <asm/mach/pci.h>
>  
>  static int debug_pci;
> +static resource_size_t (*align_resource)(struct pci_dev *dev,
> +		  const struct resource *res,
> +		  resource_size_t start,
> +		  resource_size_t size,
> +		  resource_size_t align) = NULL;
>  
>  #ifdef CONFIG_PCI_MSI
>  struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
> @@ -468,7 +473,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
>  		sys->busnr   = busnr;
>  		sys->swizzle = hw->swizzle;
>  		sys->map_irq = hw->map_irq;
> -		sys->align_resource = hw->align_resource;
> +		align_resource = hw->align_resource;
>  		INIT_LIST_HEAD(&sys->resources);
>  
>  		if (hw->private_data)
> @@ -589,7 +594,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  				resource_size_t size, resource_size_t align)
>  {
>  	struct pci_dev *dev = data;
> -	struct pci_sys_data *sys = dev->sysdata;
>  	resource_size_t start = res->start;
>  
>  	if (res->flags & IORESOURCE_IO && start & 0x300)
> @@ -597,8 +601,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  
>  	start = (start + align - 1) & ~(align - 1);
>  
> -	if (sys->align_resource)
> -		return sys->align_resource(dev, res, start, size, align);
> +	if (align_resource)
> +		return align_resource(dev, res, start, size, align);
>  
>  	return start;
>  }
> -- 
> 1.9.1
> 
> --
> 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] 36+ messages in thread

* Re: [PATCH v4 4/5] Documentation: DT: Add Hisilicon PCIe host binding
  2015-07-21  6:48     ` Zhou Wang
@ 2015-07-21 23:02       ` Bjorn Helgaas
  -1 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2015-07-21 23:02 UTC (permalink / raw
  To: Zhou Wang
  Cc: Jingoo Han, Pratyush Anand, Arnd Bergmann, gabriele.paoloni,
	lorenzo.pieralisi, James Morse, Liviu.Dudau, linux-pci,
	linux-arm-kernel, devicetree, yuanzhichang, zhudacai, zhangjukuo,
	qiuzhenfa, liudongdong3, qiujiang, kangfenglong, liguozhu,
	Rob Herring

[+cc Rob]

On Tue, Jul 21, 2015 at 02:48:42PM +0800, Zhou Wang wrote:
> This patch adds related DTS binding document for Hisilicon PCIe host driver.
> 
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>

If I merge this via my tree, I'm looking for an ack from Arnd and/or Rob
here.

> ---
>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> new file mode 100644
> index 0000000..6c9b827
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> @@ -0,0 +1,46 @@
> +Hisilicon PCIe host bridge DT description

Even the website at http://hisilicon.com isn't consistent, but there is
some indication that the correct capitalization would be "HiSilicon".
Since this is English text, feel free to capitalize it correctly here :)

Similarly, Synopsys seems to use "DesignWare," so I try to use that when
it makes sense.

> +Hisilicon PCIe host controller is based on Designware PCI core.
> +It shares common functions with PCIe Designware core driver and inherits
> +common properties defined in
> +Documentation/devicetree/bindings/pci/designware-pci.txt.
> +
> +Additional properties are described here:
> +
> +Required properties:
> +- compatible: Should contain "hisilicon,hip05-pcie".
> +- reg: Should contain rc_dbi, subctrl, config registers location and length.
> +- reg-names: Must include the following entries:
> +  "rc_dbi": controller configuration registers;
> +  "subctrl": whole PCIe hosts configuration registers;
> +  "config": PCIe configuration space registers.
> +- msi-parent: Should be its_pcie which is an its receiving MSI interrupts.

I guess "its" here is an abbreviation for something; if so, this would read
better as "... which is an ITS receiving ..."

> +- port-id: Should be 0, 1, 2 or 3.
> +
> +Optional properties:
> +- status: Either "ok" or "disabled".
> +- dma-coherent: Present if dma operations are coherent.

"if DMA operations"

> +
> +Example:
> +	pcie@0xb0080000 {
> +		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
> +		reg = <0 0xb0080000 0 0x10000>, <0 0xb0000000 0 0x10000>,
> +		      <0x220 0x00000000 0 0x2000>;
> +		reg-names = "rc_dbi", "subctrl", "config";
> +		bus-range = <0  15>;
> +		msi-parent = <&its_pcie>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		dma-coherent;
> +		ranges = <0x82000000 0 0x00000000 0x220 0x00000000 0 0x10000000>;
> +		num-lanes = <8>;
> +		port-id = <1>;
> +		#interrupts-cells = <1>;
> +		interrupts-map-mask = <0xf800 0 0 7>;
> +		interrupts-map = <0x0 0 0 1 &mbigen_pcie 1 10
> +				  0x0 0 0 2 &mbigen_pcie 2 11
> +				  0x0 0 0 3 &mbigen_pcie 3 12
> +				  0x0 0 0 4 &mbigen_pcie 4 13>;
> +		status = "ok";
> +	};
> -- 
> 1.9.1
> 
> --
> 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] 36+ messages in thread

* [PATCH v4 4/5] Documentation: DT: Add Hisilicon PCIe host binding
@ 2015-07-21 23:02       ` Bjorn Helgaas
  0 siblings, 0 replies; 36+ messages in thread
From: Bjorn Helgaas @ 2015-07-21 23:02 UTC (permalink / raw
  To: linux-arm-kernel

[+cc Rob]

On Tue, Jul 21, 2015 at 02:48:42PM +0800, Zhou Wang wrote:
> This patch adds related DTS binding document for Hisilicon PCIe host driver.
> 
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>

If I merge this via my tree, I'm looking for an ack from Arnd and/or Rob
here.

> ---
>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> new file mode 100644
> index 0000000..6c9b827
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
> @@ -0,0 +1,46 @@
> +Hisilicon PCIe host bridge DT description

Even the website at http://hisilicon.com isn't consistent, but there is
some indication that the correct capitalization would be "HiSilicon".
Since this is English text, feel free to capitalize it correctly here :)

Similarly, Synopsys seems to use "DesignWare," so I try to use that when
it makes sense.

> +Hisilicon PCIe host controller is based on Designware PCI core.
> +It shares common functions with PCIe Designware core driver and inherits
> +common properties defined in
> +Documentation/devicetree/bindings/pci/designware-pci.txt.
> +
> +Additional properties are described here:
> +
> +Required properties:
> +- compatible: Should contain "hisilicon,hip05-pcie".
> +- reg: Should contain rc_dbi, subctrl, config registers location and length.
> +- reg-names: Must include the following entries:
> +  "rc_dbi": controller configuration registers;
> +  "subctrl": whole PCIe hosts configuration registers;
> +  "config": PCIe configuration space registers.
> +- msi-parent: Should be its_pcie which is an its receiving MSI interrupts.

I guess "its" here is an abbreviation for something; if so, this would read
better as "... which is an ITS receiving ..."

> +- port-id: Should be 0, 1, 2 or 3.
> +
> +Optional properties:
> +- status: Either "ok" or "disabled".
> +- dma-coherent: Present if dma operations are coherent.

"if DMA operations"

> +
> +Example:
> +	pcie at 0xb0080000 {
> +		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
> +		reg = <0 0xb0080000 0 0x10000>, <0 0xb0000000 0 0x10000>,
> +		      <0x220 0x00000000 0 0x2000>;
> +		reg-names = "rc_dbi", "subctrl", "config";
> +		bus-range = <0  15>;
> +		msi-parent = <&its_pcie>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		device_type = "pci";
> +		dma-coherent;
> +		ranges = <0x82000000 0 0x00000000 0x220 0x00000000 0 0x10000000>;
> +		num-lanes = <8>;
> +		port-id = <1>;
> +		#interrupts-cells = <1>;
> +		interrupts-map-mask = <0xf800 0 0 7>;
> +		interrupts-map = <0x0 0 0 1 &mbigen_pcie 1 10
> +				  0x0 0 0 2 &mbigen_pcie 2 11
> +				  0x0 0 0 3 &mbigen_pcie 3 12
> +				  0x0 0 0 4 &mbigen_pcie 4 13>;
> +		status = "ok";
> +	};
> -- 
> 1.9.1
> 
> --
> 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] 36+ messages in thread

* Re: [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data
  2015-07-21 22:44     ` Bjorn Helgaas
  (?)
@ 2015-07-22  2:00       ` Zhou Wang
  -1 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-22  2:00 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Jingoo Han, Pratyush Anand, Arnd Bergmann, gabriele.paoloni,
	lorenzo.pieralisi, James Morse, Liviu.Dudau, linux-pci,
	linux-arm-kernel, devicetree, yuanzhichang, zhudacai, zhangjukuo,
	qiuzhenfa, liudongdong3, qiujiang, kangfenglong, liguozhu

On 2015/7/22 6:44, Bjorn Helgaas wrote:
> Hi Zhou & Gabriele,
> 
> On Tue, Jul 21, 2015 at 02:48:39PM +0800, Zhou Wang wrote:
>> This patch is needed in order to unify the PCIe designware framework for ARM and
>> ARM64 architectures. In the PCIe designware unification process we are calling
>> pci_create_root_bus() passing a "sysdata" parameter that is the same for both
>> ARM and ARM64 and is of type "struct pcie_port*". In the ARM case this will
>> cause a problem with the function pcibios_align_resource(); in fact this will
>> cast "dev->sysdata" to "struct pci_sys_data*", whereas designware had passed a
>> "struct pcie_port*" pointer.
>>
>> This patch solves the issue by removing "align_resource" from "pci_sys_data"
>> struct and defining a static global function pointer in "bios32.c"
>>
>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni at huawei.com>
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> 
> Can you fix up Gabriele's email address above (currently contains " at ")
> and verify that this sign-off chain is accurate per section 11 of
> Documentation/SubmittingPatches?  The above suggests to me that Gabriele
> originally wrote this patch and Zhou is passing it along.  That might well
> be true; I haven't followed the development, so I'm just double-checking.
>

Hi Bjorn,

Sorry about above mistake. Will change it in next version.

Thanks for your reminding,
Zhou

>> ---
>>  arch/arm/include/asm/mach/pci.h |  5 -----
>>  arch/arm/kernel/bios32.c        | 12 ++++++++----
>>  2 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
>> index 28b9bb3..8a4e4de 100644
>> --- a/arch/arm/include/asm/mach/pci.h
>> +++ b/arch/arm/include/asm/mach/pci.h
>> @@ -58,11 +58,6 @@ struct pci_sys_data {
>>  					/* IRQ mapping				*/
>>  	int		(*map_irq)(const struct pci_dev *, u8, u8);
>>  					/* Resource alignement requirements	*/
>> -	resource_size_t (*align_resource)(struct pci_dev *dev,
>> -					  const struct resource *res,
>> -					  resource_size_t start,
>> -					  resource_size_t size,
>> -					  resource_size_t align);
>>  	void		*private_data;	/* platform controller private data	*/
>>  };
>>  
>> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
>> index fcbbbb1..4cdc64d 100644
>> --- a/arch/arm/kernel/bios32.c
>> +++ b/arch/arm/kernel/bios32.c
>> @@ -17,6 +17,11 @@
>>  #include <asm/mach/pci.h>
>>  
>>  static int debug_pci;
>> +static resource_size_t (*align_resource)(struct pci_dev *dev,
>> +		  const struct resource *res,
>> +		  resource_size_t start,
>> +		  resource_size_t size,
>> +		  resource_size_t align) = NULL;
>>  
>>  #ifdef CONFIG_PCI_MSI
>>  struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
>> @@ -468,7 +473,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
>>  		sys->busnr   = busnr;
>>  		sys->swizzle = hw->swizzle;
>>  		sys->map_irq = hw->map_irq;
>> -		sys->align_resource = hw->align_resource;
>> +		align_resource = hw->align_resource;
>>  		INIT_LIST_HEAD(&sys->resources);
>>  
>>  		if (hw->private_data)
>> @@ -589,7 +594,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>  				resource_size_t size, resource_size_t align)
>>  {
>>  	struct pci_dev *dev = data;
>> -	struct pci_sys_data *sys = dev->sysdata;
>>  	resource_size_t start = res->start;
>>  
>>  	if (res->flags & IORESOURCE_IO && start & 0x300)
>> @@ -597,8 +601,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>  
>>  	start = (start + align - 1) & ~(align - 1);
>>  
>> -	if (sys->align_resource)
>> -		return sys->align_resource(dev, res, start, size, align);
>> +	if (align_resource)
>> +		return align_resource(dev, res, start, size, align);
>>  
>>  	return start;
>>  }
>> -- 
>> 1.9.1
>>
>> --
>> 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] 36+ messages in thread

* [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data
@ 2015-07-22  2:00       ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-22  2:00 UTC (permalink / raw
  To: linux-arm-kernel

On 2015/7/22 6:44, Bjorn Helgaas wrote:
> Hi Zhou & Gabriele,
> 
> On Tue, Jul 21, 2015 at 02:48:39PM +0800, Zhou Wang wrote:
>> This patch is needed in order to unify the PCIe designware framework for ARM and
>> ARM64 architectures. In the PCIe designware unification process we are calling
>> pci_create_root_bus() passing a "sysdata" parameter that is the same for both
>> ARM and ARM64 and is of type "struct pcie_port*". In the ARM case this will
>> cause a problem with the function pcibios_align_resource(); in fact this will
>> cast "dev->sysdata" to "struct pci_sys_data*", whereas designware had passed a
>> "struct pcie_port*" pointer.
>>
>> This patch solves the issue by removing "align_resource" from "pci_sys_data"
>> struct and defining a static global function pointer in "bios32.c"
>>
>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> 
> Can you fix up Gabriele's email address above (currently contains " at ")
> and verify that this sign-off chain is accurate per section 11 of
> Documentation/SubmittingPatches?  The above suggests to me that Gabriele
> originally wrote this patch and Zhou is passing it along.  That might well
> be true; I haven't followed the development, so I'm just double-checking.
>

Hi Bjorn,

Sorry about above mistake. Will change it in next version.

Thanks for your reminding,
Zhou

>> ---
>>  arch/arm/include/asm/mach/pci.h |  5 -----
>>  arch/arm/kernel/bios32.c        | 12 ++++++++----
>>  2 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
>> index 28b9bb3..8a4e4de 100644
>> --- a/arch/arm/include/asm/mach/pci.h
>> +++ b/arch/arm/include/asm/mach/pci.h
>> @@ -58,11 +58,6 @@ struct pci_sys_data {
>>  					/* IRQ mapping				*/
>>  	int		(*map_irq)(const struct pci_dev *, u8, u8);
>>  					/* Resource alignement requirements	*/
>> -	resource_size_t (*align_resource)(struct pci_dev *dev,
>> -					  const struct resource *res,
>> -					  resource_size_t start,
>> -					  resource_size_t size,
>> -					  resource_size_t align);
>>  	void		*private_data;	/* platform controller private data	*/
>>  };
>>  
>> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
>> index fcbbbb1..4cdc64d 100644
>> --- a/arch/arm/kernel/bios32.c
>> +++ b/arch/arm/kernel/bios32.c
>> @@ -17,6 +17,11 @@
>>  #include <asm/mach/pci.h>
>>  
>>  static int debug_pci;
>> +static resource_size_t (*align_resource)(struct pci_dev *dev,
>> +		  const struct resource *res,
>> +		  resource_size_t start,
>> +		  resource_size_t size,
>> +		  resource_size_t align) = NULL;
>>  
>>  #ifdef CONFIG_PCI_MSI
>>  struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
>> @@ -468,7 +473,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
>>  		sys->busnr   = busnr;
>>  		sys->swizzle = hw->swizzle;
>>  		sys->map_irq = hw->map_irq;
>> -		sys->align_resource = hw->align_resource;
>> +		align_resource = hw->align_resource;
>>  		INIT_LIST_HEAD(&sys->resources);
>>  
>>  		if (hw->private_data)
>> @@ -589,7 +594,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>  				resource_size_t size, resource_size_t align)
>>  {
>>  	struct pci_dev *dev = data;
>> -	struct pci_sys_data *sys = dev->sysdata;
>>  	resource_size_t start = res->start;
>>  
>>  	if (res->flags & IORESOURCE_IO && start & 0x300)
>> @@ -597,8 +601,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>  
>>  	start = (start + align - 1) & ~(align - 1);
>>  
>> -	if (sys->align_resource)
>> -		return sys->align_resource(dev, res, start, size, align);
>> +	if (align_resource)
>> +		return align_resource(dev, res, start, size, align);
>>  
>>  	return start;
>>  }
>> -- 
>> 1.9.1
>>
>> --
>> 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] 36+ messages in thread

* Re: [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data
@ 2015-07-22  2:00       ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-22  2:00 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Jingoo Han, Pratyush Anand, Arnd Bergmann, gabriele.paoloni,
	lorenzo.pieralisi, James Morse, Liviu.Dudau, linux-pci,
	linux-arm-kernel, devicetree, yuanzhichang, zhudacai, zhangjukuo,
	qiuzhenfa, liudongdong3, qiujiang, kangfenglong, liguozhu

On 2015/7/22 6:44, Bjorn Helgaas wrote:
> Hi Zhou & Gabriele,
> 
> On Tue, Jul 21, 2015 at 02:48:39PM +0800, Zhou Wang wrote:
>> This patch is needed in order to unify the PCIe designware framework for ARM and
>> ARM64 architectures. In the PCIe designware unification process we are calling
>> pci_create_root_bus() passing a "sysdata" parameter that is the same for both
>> ARM and ARM64 and is of type "struct pcie_port*". In the ARM case this will
>> cause a problem with the function pcibios_align_resource(); in fact this will
>> cast "dev->sysdata" to "struct pci_sys_data*", whereas designware had passed a
>> "struct pcie_port*" pointer.
>>
>> This patch solves the issue by removing "align_resource" from "pci_sys_data"
>> struct and defining a static global function pointer in "bios32.c"
>>
>> Signed-off-by: Gabriele Paoloni <gabriele.paoloni at huawei.com>
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> 
> Can you fix up Gabriele's email address above (currently contains " at ")
> and verify that this sign-off chain is accurate per section 11 of
> Documentation/SubmittingPatches?  The above suggests to me that Gabriele
> originally wrote this patch and Zhou is passing it along.  That might well
> be true; I haven't followed the development, so I'm just double-checking.
>

Hi Bjorn,

Sorry about above mistake. Will change it in next version.

Thanks for your reminding,
Zhou

>> ---
>>  arch/arm/include/asm/mach/pci.h |  5 -----
>>  arch/arm/kernel/bios32.c        | 12 ++++++++----
>>  2 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
>> index 28b9bb3..8a4e4de 100644
>> --- a/arch/arm/include/asm/mach/pci.h
>> +++ b/arch/arm/include/asm/mach/pci.h
>> @@ -58,11 +58,6 @@ struct pci_sys_data {
>>  					/* IRQ mapping				*/
>>  	int		(*map_irq)(const struct pci_dev *, u8, u8);
>>  					/* Resource alignement requirements	*/
>> -	resource_size_t (*align_resource)(struct pci_dev *dev,
>> -					  const struct resource *res,
>> -					  resource_size_t start,
>> -					  resource_size_t size,
>> -					  resource_size_t align);
>>  	void		*private_data;	/* platform controller private data	*/
>>  };
>>  
>> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
>> index fcbbbb1..4cdc64d 100644
>> --- a/arch/arm/kernel/bios32.c
>> +++ b/arch/arm/kernel/bios32.c
>> @@ -17,6 +17,11 @@
>>  #include <asm/mach/pci.h>
>>  
>>  static int debug_pci;
>> +static resource_size_t (*align_resource)(struct pci_dev *dev,
>> +		  const struct resource *res,
>> +		  resource_size_t start,
>> +		  resource_size_t size,
>> +		  resource_size_t align) = NULL;
>>  
>>  #ifdef CONFIG_PCI_MSI
>>  struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
>> @@ -468,7 +473,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
>>  		sys->busnr   = busnr;
>>  		sys->swizzle = hw->swizzle;
>>  		sys->map_irq = hw->map_irq;
>> -		sys->align_resource = hw->align_resource;
>> +		align_resource = hw->align_resource;
>>  		INIT_LIST_HEAD(&sys->resources);
>>  
>>  		if (hw->private_data)
>> @@ -589,7 +594,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>  				resource_size_t size, resource_size_t align)
>>  {
>>  	struct pci_dev *dev = data;
>> -	struct pci_sys_data *sys = dev->sysdata;
>>  	resource_size_t start = res->start;
>>  
>>  	if (res->flags & IORESOURCE_IO && start & 0x300)
>> @@ -597,8 +601,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>>  
>>  	start = (start + align - 1) & ~(align - 1);
>>  
>> -	if (sys->align_resource)
>> -		return sys->align_resource(dev, res, start, size, align);
>> +	if (align_resource)
>> +		return align_resource(dev, res, start, size, align);
>>  
>>  	return start;
>>  }
>> -- 
>> 1.9.1
>>
>> --
>> 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] 36+ messages in thread

* Re: [PATCH v4 3/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
  2015-07-21 22:37     ` Bjorn Helgaas
  (?)
@ 2015-07-22  2:33       ` Zhou Wang
  -1 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-22  2:33 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Jingoo Han, Pratyush Anand, Arnd Bergmann, gabriele.paoloni,
	lorenzo.pieralisi, James Morse, Liviu.Dudau, linux-pci,
	linux-arm-kernel, devicetree, yuanzhichang, zhudacai, zhangjukuo,
	qiuzhenfa, liudongdong3, qiujiang, kangfenglong, liguozhu

On 2015/7/22 6:37, Bjorn Helgaas wrote:
> Hi Zhou,
> 
> On Tue, Jul 21, 2015 at 02:48:41PM +0800, Zhou Wang wrote:
>> This patch adds PCIe host support for Hisilicon Soc Hip05.
>>
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>> ---
>>  drivers/pci/host/Kconfig     |   5 +
>>  drivers/pci/host/Makefile    |   1 +
>>  drivers/pci/host/pcie-hisi.c | 257 +++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 263 insertions(+)
>>  create mode 100644 drivers/pci/host/pcie-hisi.c
>>
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index c132bdd..37f2075 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -145,4 +145,9 @@ config PCIE_IPROC_BCMA
>>  	  Say Y here if you want to use the Broadcom iProc PCIe controller
>>  	  through the BCMA bus interface
>>  
>> +config PCI_HISI
>> +	depends on OF && ARM64
>> +	bool "Hisilicon Soc HIP05 PCIe controller"
>> +	select PCIE_DW
> 
> Almost all the other DesignWare-based drivers select PCIEPORTBUS in
> addition to PCIE_DW.  I'm not sure that's the best Kconfiggery (maybe we
> should leave this up to the user), but it'd be nice if all the drivers at
> least did this consistently.  So do you have a reason for doing it
> differently?
>

Hi Bjorn,

I made a mistake there. PCIEPORTBUS should be selected as other DesignWare-based
drivers did.

>>  endmenu
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 140d66f..ea1dbf2 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
>>  obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
>>  obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
>>  obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
>> +obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
>> diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
>> new file mode 100644
>> index 0000000..9f2fac1
>> --- /dev/null
>> +++ b/drivers/pci/host/pcie-hisi.c
>> @@ -0,0 +1,257 @@
>> +/*
>> + * PCIe host controller driver for Hisilicon Hip05 SoCs
>> + *
>> + * Copyright (C) 2015 Hisilicon Co., Ltd. http://www.hisilicon.com
>> + *
>> + * Author: Zhou Wang <wangzhou1@hisilicon.com>
>> + *         Dacai Zhu <zhudacai@hisilicon.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/interrupt.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "pcie-designware.h"
>> +
>> +#define PCIE_SUBCTRL_MODE_REG                           0x2800
>> +#define PCIE_SUBCTRL_SYS_STATE4_REG                     0x6818
>> +#define PCIE_SLV_DBI_MODE                               0x0
>> +#define PCIE_SLV_SYSCTRL_MODE                           0x1
>> +#define PCIE_SLV_CONTENT_MODE                           0x2
>> +#define PCIE_SLV_MSI_ASID                               0x10
>> +#define PCIE_LTSSM_LINKUP_STATE                         0x11
>> +#define PCIE_LTSSM_STATE_MASK                           0x3F
>> +#define PCIE_MSI_ASID_ENABLE                            (0x1 << 12)
>> +#define PCIE_MSI_ASID_VALUE                             (0x1 << 16)
>> +#define PCIE_MSI_TRANS_ENABLE                           (0x1 << 12)
>> +#define PCIE_MSI_TRANS_REG                              0x1c8
>> +#define PCIE_MSI_LOW_ADDRESS                            0x1b4
>> +#define PCIE_MSI_HIGH_ADDRESS                           0x1c4
>> +#define PCIE_MSI_ADDRESS_VAL                            0xb7010040
>> +
>> +#define to_hisi_pcie(x)	container_of(x, struct hisi_pcie, pp)
>> +
>> +struct hisi_pcie {
>> +	void __iomem *subctrl_base;
>> +	void __iomem *reg_base;
>> +	struct msi_controller *msi;
>> +	u32 port_id;
>> +	struct pcie_port pp;
>> +};
>> +
>> +static inline void hisi_pcie_subctrl_writel(struct hisi_pcie *pcie,
>> +					    u32 val, u32 reg)
>> +{
>> +	writel(val, pcie->subctrl_base + reg);
>> +}
>> +
>> +static inline u32 hisi_pcie_subctrl_readl(struct hisi_pcie *pcie, u32 reg)
>> +{
>> +	return readl(pcie->subctrl_base + reg);
>> +}
>> +
>> +static inline void hisi_pcie_apb_writel(struct hisi_pcie *pcie,
>> +					u32 val, u32 reg)
>> +{
>> +	writel(val, pcie->reg_base + reg);
>> +}
>> +
>> +static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *pcie, u32 reg)
>> +{
>> +	return readl(pcie->reg_base + reg);
>> +}
>> +
>> +/*
>> + * Change mode to indicate the same reg_base to base of PCIe host configure
>> + * registers, base of RC configure space or base of vmid/asid context table
>> + */
>> +static void hisi_pcie_change_apb_mode(struct hisi_pcie *pcie, u32 mode)
>> +{
>> +	u32 val;
>> +	u32 bit_mask;
>> +	u32 bit_shift;
>> +	u32 port_id = pcie->port_id;
>> +	u32 reg = PCIE_SUBCTRL_MODE_REG + 0x100 * port_id;
>> +
>> +	if ((port_id == 1) || (port_id == 2)) {
>> +		bit_mask = 0xc;
>> +		bit_shift = 0x2;
>> +	} else {
>> +		bit_mask = 0x6;
>> +		bit_shift = 0x1;
>> +	}
>> +
>> +	val = hisi_pcie_subctrl_readl(pcie, reg);
>> +	val = (val & (~bit_mask)) | (mode << bit_shift);
>> +	hisi_pcie_subctrl_writel(pcie, val, reg);
>> +}
>> +
>> +/* Configure vmid/asid table in PCIe host */
>> +static void hisi_pcie_config_context(struct hisi_pcie *pcie)
>> +{
>> +	int i;
>> +
>> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_CONTENT_MODE);
>> +
>> +	/*
>> +	 * init vmid and asid tables for all PCIes device as 0
> 
> s/PCIes device as/PCIe devices to/ ?
>

Thanks, will modify this.

>> +	 * vmid table: 0 ~ 0x3ff, asid table: 0x400 ~ 0x7ff
>> +	 */
>> +	for (i = 0; i < 0x800; i++)
>> +		hisi_pcie_apb_writel(pcie, 0x0, i * 4);
>> +
>> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_SYSCTRL_MODE);
>> +
>> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_ADDRESS_VAL, PCIE_MSI_LOW_ADDRESS);
>> +	hisi_pcie_apb_writel(pcie, 0x0, PCIE_MSI_HIGH_ADDRESS);
>> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_ASID_ENABLE | PCIE_MSI_ASID_VALUE,
>> +			     PCIE_SLV_MSI_ASID);
>> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_TRANS_ENABLE, PCIE_MSI_TRANS_REG);
>> +
>> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_DBI_MODE);
>> +}
>> +
>> +static int hisi_pcie_link_up(struct pcie_port *pp)
>> +{
>> +	u32 val;
>> +
> 
> Remove this extra blank line.
> 

Yes, will remove it. Thanks.

>> +	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
>> +
>> +	val = hisi_pcie_subctrl_readl(hisi_pcie, PCIE_SUBCTRL_SYS_STATE4_REG +
>> +				      0x100 * hisi_pcie->port_id);
>> +
>> +	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
>> +}
>> +
>> +static
>> +int hisi_pcie_msi_host_init(struct pcie_port *pp, struct msi_controller *chip)
>> +{
>> +	struct device_node *msi_node;
>> +	struct irq_domain *irq_domain;
>> +	struct device_node *np = pp->dev->of_node;
>> +
>> +	msi_node = of_parse_phandle(np, "msi-parent", 0);
>> +	if (!msi_node) {
>> +		pr_err("failed to find msi-parent\n");
> 
> Can you use dev_err() here (and below)?  A message like this that doesn't
> have a hint about what device it's related to is not as useful as it could
> be.
>

Right, will use dev_err(). Thanks.

>> +		return -ENODEV;
>> +	}
>> +
>> +	irq_domain = irq_find_host(msi_node);
>> +	if (!irq_domain) {
>> +		pr_err("failed to find irq domain\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	pp->irq_domain = irq_domain;
>> +
>> +	return 0;
>> +}
>> +
>> +static struct pcie_host_ops hisi_pcie_host_ops = {
>> +	.link_up = hisi_pcie_link_up,
>> +	.msi_host_init = hisi_pcie_msi_host_init,
>> +};
>> +
>> +static int __init hisi_add_pcie_port(struct pcie_port *pp,
>> +				     struct platform_device *pdev)
>> +{
>> +	int ret;
>> +	u32 port_id;
>> +	struct resource busn;
>> +
> 
> Remove this extra blank line.

Will remove it. Thanks.

> 
>> +	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
>> +
>> +	if (of_property_read_u32(pdev->dev.of_node, "port-id", &port_id)) {
>> +		dev_err(&pdev->dev, "failed to read port-id\n");
>> +		return -EINVAL;
>> +	}
>> +	if (port_id > 3) {
>> +		dev_err(&pdev->dev, "Invalid port-id\n");
> 
> Since you have the invalid port_id here, maybe you could include the
> invalid value in the output?
>

Right, will do like this. Thanks.

>> +		return -EINVAL;
>> +	}
>> +
>> +	hisi_pcie->port_id = port_id;
>> +
>> +	if (of_pci_parse_bus_range(pdev->dev.of_node, &busn)) {
>> +		dev_err(&pdev->dev, "failed to parse bus-ranges\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	pp->root_bus_nr = busn.start;
>> +	pp->ops = &hisi_pcie_host_ops;
>> +
>> +	hisi_pcie_config_context(hisi_pcie);
>> +
>> +	ret = dw_pcie_host_init(pp);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "failed to initialize host\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int __init hisi_pcie_probe(struct platform_device *pdev)
>> +{
>> +	struct hisi_pcie *hisi_pcie;
>> +	struct pcie_port *pp;
>> +	struct resource *reg;
>> +	struct resource *subctrl;
>> +	int ret;
>> +
>> +	hisi_pcie = devm_kzalloc(&pdev->dev, sizeof(*hisi_pcie), GFP_KERNEL);
>> +	if (!hisi_pcie)
>> +		return -ENOMEM;
>> +
>> +	pp = &hisi_pcie->pp;
>> +	pp->dev = &pdev->dev;
>> +
>> +	subctrl = platform_get_resource_byname(pdev, IORESOURCE_MEM, "subctrl");
>> +	hisi_pcie->subctrl_base = devm_ioremap_nocache(&pdev->dev,
>> +					subctrl->start, resource_size(subctrl));
>> +	if (IS_ERR(hisi_pcie->subctrl_base)) {
>> +		dev_err(pp->dev, "cannot get subctrl base\n");
>> +		return PTR_ERR(hisi_pcie->subctrl_base);
>> +	}
>> +
>> +	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
>> +	hisi_pcie->reg_base = devm_ioremap_resource(&pdev->dev, reg);
>> +	if (IS_ERR(hisi_pcie->reg_base)) {
>> +		dev_err(pp->dev, "cannot get reg base\n");
> 
> Maybe this could include the actual resource name ("rc_dbi") as the message
> above does?
>

Will modify this message. Thanks.

>> +		return PTR_ERR(hisi_pcie->reg_base);
>> +	}
>> +
>> +	hisi_pcie->pp.dbi_base = hisi_pcie->reg_base;
>> +
>> +	ret = hisi_add_pcie_port(pp, pdev);
>> +	if (ret < 0)
> 
> Seems like this return value check should be the same as the one above in 
> hisi_add_pcie_port().  In both cases, the only possible returns are zero
> (success) or a negative error number.  Using "if (ret)" here has the
> advantage that below you could "return 0" and it would be crystal clear
> that you're always returning success if you get that far.
> 

Got it, will do like this. Thanks for your suggestion.

>> +		return ret;
>> +
>> +	platform_set_drvdata(pdev, hisi_pcie);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct of_device_id hisi_pcie_of_match[] = {
>> +	{.compatible = "hisilicon,hip05-pcie",},
>> +	{},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, hisi_pcie_of_match);
>> +
>> +static struct platform_driver hisi_pcie_driver = {
>> +	.probe  = hisi_pcie_probe,
>> +	.driver = {
>> +		   .name = "hisi-pcie",
>> +		   .owner = THIS_MODULE,
> 
> You can drop this ".owner =" initialization.  platform_driver_register()
> supplies that automatically.
> 

Will drop it, Thanks.

Best regards,
Zhou

>> +		   .of_match_table = hisi_pcie_of_match,
>> +	},
>> +};
>> +
>> +module_platform_driver(hisi_pcie_driver);
>> -- 
>> 1.9.1
>>
>> --
>> 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] 36+ messages in thread

* [PATCH v4 3/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
@ 2015-07-22  2:33       ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-22  2:33 UTC (permalink / raw
  To: linux-arm-kernel

On 2015/7/22 6:37, Bjorn Helgaas wrote:
> Hi Zhou,
> 
> On Tue, Jul 21, 2015 at 02:48:41PM +0800, Zhou Wang wrote:
>> This patch adds PCIe host support for Hisilicon Soc Hip05.
>>
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>> ---
>>  drivers/pci/host/Kconfig     |   5 +
>>  drivers/pci/host/Makefile    |   1 +
>>  drivers/pci/host/pcie-hisi.c | 257 +++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 263 insertions(+)
>>  create mode 100644 drivers/pci/host/pcie-hisi.c
>>
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index c132bdd..37f2075 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -145,4 +145,9 @@ config PCIE_IPROC_BCMA
>>  	  Say Y here if you want to use the Broadcom iProc PCIe controller
>>  	  through the BCMA bus interface
>>  
>> +config PCI_HISI
>> +	depends on OF && ARM64
>> +	bool "Hisilicon Soc HIP05 PCIe controller"
>> +	select PCIE_DW
> 
> Almost all the other DesignWare-based drivers select PCIEPORTBUS in
> addition to PCIE_DW.  I'm not sure that's the best Kconfiggery (maybe we
> should leave this up to the user), but it'd be nice if all the drivers at
> least did this consistently.  So do you have a reason for doing it
> differently?
>

Hi Bjorn,

I made a mistake there. PCIEPORTBUS should be selected as other DesignWare-based
drivers did.

>>  endmenu
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 140d66f..ea1dbf2 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
>>  obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
>>  obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
>>  obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
>> +obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
>> diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
>> new file mode 100644
>> index 0000000..9f2fac1
>> --- /dev/null
>> +++ b/drivers/pci/host/pcie-hisi.c
>> @@ -0,0 +1,257 @@
>> +/*
>> + * PCIe host controller driver for Hisilicon Hip05 SoCs
>> + *
>> + * Copyright (C) 2015 Hisilicon Co., Ltd. http://www.hisilicon.com
>> + *
>> + * Author: Zhou Wang <wangzhou1@hisilicon.com>
>> + *         Dacai Zhu <zhudacai@hisilicon.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/interrupt.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "pcie-designware.h"
>> +
>> +#define PCIE_SUBCTRL_MODE_REG                           0x2800
>> +#define PCIE_SUBCTRL_SYS_STATE4_REG                     0x6818
>> +#define PCIE_SLV_DBI_MODE                               0x0
>> +#define PCIE_SLV_SYSCTRL_MODE                           0x1
>> +#define PCIE_SLV_CONTENT_MODE                           0x2
>> +#define PCIE_SLV_MSI_ASID                               0x10
>> +#define PCIE_LTSSM_LINKUP_STATE                         0x11
>> +#define PCIE_LTSSM_STATE_MASK                           0x3F
>> +#define PCIE_MSI_ASID_ENABLE                            (0x1 << 12)
>> +#define PCIE_MSI_ASID_VALUE                             (0x1 << 16)
>> +#define PCIE_MSI_TRANS_ENABLE                           (0x1 << 12)
>> +#define PCIE_MSI_TRANS_REG                              0x1c8
>> +#define PCIE_MSI_LOW_ADDRESS                            0x1b4
>> +#define PCIE_MSI_HIGH_ADDRESS                           0x1c4
>> +#define PCIE_MSI_ADDRESS_VAL                            0xb7010040
>> +
>> +#define to_hisi_pcie(x)	container_of(x, struct hisi_pcie, pp)
>> +
>> +struct hisi_pcie {
>> +	void __iomem *subctrl_base;
>> +	void __iomem *reg_base;
>> +	struct msi_controller *msi;
>> +	u32 port_id;
>> +	struct pcie_port pp;
>> +};
>> +
>> +static inline void hisi_pcie_subctrl_writel(struct hisi_pcie *pcie,
>> +					    u32 val, u32 reg)
>> +{
>> +	writel(val, pcie->subctrl_base + reg);
>> +}
>> +
>> +static inline u32 hisi_pcie_subctrl_readl(struct hisi_pcie *pcie, u32 reg)
>> +{
>> +	return readl(pcie->subctrl_base + reg);
>> +}
>> +
>> +static inline void hisi_pcie_apb_writel(struct hisi_pcie *pcie,
>> +					u32 val, u32 reg)
>> +{
>> +	writel(val, pcie->reg_base + reg);
>> +}
>> +
>> +static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *pcie, u32 reg)
>> +{
>> +	return readl(pcie->reg_base + reg);
>> +}
>> +
>> +/*
>> + * Change mode to indicate the same reg_base to base of PCIe host configure
>> + * registers, base of RC configure space or base of vmid/asid context table
>> + */
>> +static void hisi_pcie_change_apb_mode(struct hisi_pcie *pcie, u32 mode)
>> +{
>> +	u32 val;
>> +	u32 bit_mask;
>> +	u32 bit_shift;
>> +	u32 port_id = pcie->port_id;
>> +	u32 reg = PCIE_SUBCTRL_MODE_REG + 0x100 * port_id;
>> +
>> +	if ((port_id == 1) || (port_id == 2)) {
>> +		bit_mask = 0xc;
>> +		bit_shift = 0x2;
>> +	} else {
>> +		bit_mask = 0x6;
>> +		bit_shift = 0x1;
>> +	}
>> +
>> +	val = hisi_pcie_subctrl_readl(pcie, reg);
>> +	val = (val & (~bit_mask)) | (mode << bit_shift);
>> +	hisi_pcie_subctrl_writel(pcie, val, reg);
>> +}
>> +
>> +/* Configure vmid/asid table in PCIe host */
>> +static void hisi_pcie_config_context(struct hisi_pcie *pcie)
>> +{
>> +	int i;
>> +
>> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_CONTENT_MODE);
>> +
>> +	/*
>> +	 * init vmid and asid tables for all PCIes device as 0
> 
> s/PCIes device as/PCIe devices to/ ?
>

Thanks, will modify this.

>> +	 * vmid table: 0 ~ 0x3ff, asid table: 0x400 ~ 0x7ff
>> +	 */
>> +	for (i = 0; i < 0x800; i++)
>> +		hisi_pcie_apb_writel(pcie, 0x0, i * 4);
>> +
>> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_SYSCTRL_MODE);
>> +
>> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_ADDRESS_VAL, PCIE_MSI_LOW_ADDRESS);
>> +	hisi_pcie_apb_writel(pcie, 0x0, PCIE_MSI_HIGH_ADDRESS);
>> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_ASID_ENABLE | PCIE_MSI_ASID_VALUE,
>> +			     PCIE_SLV_MSI_ASID);
>> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_TRANS_ENABLE, PCIE_MSI_TRANS_REG);
>> +
>> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_DBI_MODE);
>> +}
>> +
>> +static int hisi_pcie_link_up(struct pcie_port *pp)
>> +{
>> +	u32 val;
>> +
> 
> Remove this extra blank line.
> 

Yes, will remove it. Thanks.

>> +	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
>> +
>> +	val = hisi_pcie_subctrl_readl(hisi_pcie, PCIE_SUBCTRL_SYS_STATE4_REG +
>> +				      0x100 * hisi_pcie->port_id);
>> +
>> +	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
>> +}
>> +
>> +static
>> +int hisi_pcie_msi_host_init(struct pcie_port *pp, struct msi_controller *chip)
>> +{
>> +	struct device_node *msi_node;
>> +	struct irq_domain *irq_domain;
>> +	struct device_node *np = pp->dev->of_node;
>> +
>> +	msi_node = of_parse_phandle(np, "msi-parent", 0);
>> +	if (!msi_node) {
>> +		pr_err("failed to find msi-parent\n");
> 
> Can you use dev_err() here (and below)?  A message like this that doesn't
> have a hint about what device it's related to is not as useful as it could
> be.
>

Right, will use dev_err(). Thanks.

>> +		return -ENODEV;
>> +	}
>> +
>> +	irq_domain = irq_find_host(msi_node);
>> +	if (!irq_domain) {
>> +		pr_err("failed to find irq domain\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	pp->irq_domain = irq_domain;
>> +
>> +	return 0;
>> +}
>> +
>> +static struct pcie_host_ops hisi_pcie_host_ops = {
>> +	.link_up = hisi_pcie_link_up,
>> +	.msi_host_init = hisi_pcie_msi_host_init,
>> +};
>> +
>> +static int __init hisi_add_pcie_port(struct pcie_port *pp,
>> +				     struct platform_device *pdev)
>> +{
>> +	int ret;
>> +	u32 port_id;
>> +	struct resource busn;
>> +
> 
> Remove this extra blank line.

Will remove it. Thanks.

> 
>> +	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
>> +
>> +	if (of_property_read_u32(pdev->dev.of_node, "port-id", &port_id)) {
>> +		dev_err(&pdev->dev, "failed to read port-id\n");
>> +		return -EINVAL;
>> +	}
>> +	if (port_id > 3) {
>> +		dev_err(&pdev->dev, "Invalid port-id\n");
> 
> Since you have the invalid port_id here, maybe you could include the
> invalid value in the output?
>

Right, will do like this. Thanks.

>> +		return -EINVAL;
>> +	}
>> +
>> +	hisi_pcie->port_id = port_id;
>> +
>> +	if (of_pci_parse_bus_range(pdev->dev.of_node, &busn)) {
>> +		dev_err(&pdev->dev, "failed to parse bus-ranges\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	pp->root_bus_nr = busn.start;
>> +	pp->ops = &hisi_pcie_host_ops;
>> +
>> +	hisi_pcie_config_context(hisi_pcie);
>> +
>> +	ret = dw_pcie_host_init(pp);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "failed to initialize host\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int __init hisi_pcie_probe(struct platform_device *pdev)
>> +{
>> +	struct hisi_pcie *hisi_pcie;
>> +	struct pcie_port *pp;
>> +	struct resource *reg;
>> +	struct resource *subctrl;
>> +	int ret;
>> +
>> +	hisi_pcie = devm_kzalloc(&pdev->dev, sizeof(*hisi_pcie), GFP_KERNEL);
>> +	if (!hisi_pcie)
>> +		return -ENOMEM;
>> +
>> +	pp = &hisi_pcie->pp;
>> +	pp->dev = &pdev->dev;
>> +
>> +	subctrl = platform_get_resource_byname(pdev, IORESOURCE_MEM, "subctrl");
>> +	hisi_pcie->subctrl_base = devm_ioremap_nocache(&pdev->dev,
>> +					subctrl->start, resource_size(subctrl));
>> +	if (IS_ERR(hisi_pcie->subctrl_base)) {
>> +		dev_err(pp->dev, "cannot get subctrl base\n");
>> +		return PTR_ERR(hisi_pcie->subctrl_base);
>> +	}
>> +
>> +	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
>> +	hisi_pcie->reg_base = devm_ioremap_resource(&pdev->dev, reg);
>> +	if (IS_ERR(hisi_pcie->reg_base)) {
>> +		dev_err(pp->dev, "cannot get reg base\n");
> 
> Maybe this could include the actual resource name ("rc_dbi") as the message
> above does?
>

Will modify this message. Thanks.

>> +		return PTR_ERR(hisi_pcie->reg_base);
>> +	}
>> +
>> +	hisi_pcie->pp.dbi_base = hisi_pcie->reg_base;
>> +
>> +	ret = hisi_add_pcie_port(pp, pdev);
>> +	if (ret < 0)
> 
> Seems like this return value check should be the same as the one above in 
> hisi_add_pcie_port().  In both cases, the only possible returns are zero
> (success) or a negative error number.  Using "if (ret)" here has the
> advantage that below you could "return 0" and it would be crystal clear
> that you're always returning success if you get that far.
> 

Got it, will do like this. Thanks for your suggestion.

>> +		return ret;
>> +
>> +	platform_set_drvdata(pdev, hisi_pcie);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct of_device_id hisi_pcie_of_match[] = {
>> +	{.compatible = "hisilicon,hip05-pcie",},
>> +	{},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, hisi_pcie_of_match);
>> +
>> +static struct platform_driver hisi_pcie_driver = {
>> +	.probe  = hisi_pcie_probe,
>> +	.driver = {
>> +		   .name = "hisi-pcie",
>> +		   .owner = THIS_MODULE,
> 
> You can drop this ".owner =" initialization.  platform_driver_register()
> supplies that automatically.
> 

Will drop it, Thanks.

Best regards,
Zhou

>> +		   .of_match_table = hisi_pcie_of_match,
>> +	},
>> +};
>> +
>> +module_platform_driver(hisi_pcie_driver);
>> -- 
>> 1.9.1
>>
>> --
>> 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] 36+ messages in thread

* Re: [PATCH v4 3/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05
@ 2015-07-22  2:33       ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-22  2:33 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Jingoo Han, Pratyush Anand, Arnd Bergmann, gabriele.paoloni,
	lorenzo.pieralisi, James Morse, Liviu.Dudau, linux-pci,
	linux-arm-kernel, devicetree, yuanzhichang, zhudacai, zhangjukuo,
	qiuzhenfa, liudongdong3, qiujiang, kangfenglong, liguozhu

On 2015/7/22 6:37, Bjorn Helgaas wrote:
> Hi Zhou,
> 
> On Tue, Jul 21, 2015 at 02:48:41PM +0800, Zhou Wang wrote:
>> This patch adds PCIe host support for Hisilicon Soc Hip05.
>>
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
>> ---
>>  drivers/pci/host/Kconfig     |   5 +
>>  drivers/pci/host/Makefile    |   1 +
>>  drivers/pci/host/pcie-hisi.c | 257 +++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 263 insertions(+)
>>  create mode 100644 drivers/pci/host/pcie-hisi.c
>>
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index c132bdd..37f2075 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -145,4 +145,9 @@ config PCIE_IPROC_BCMA
>>  	  Say Y here if you want to use the Broadcom iProc PCIe controller
>>  	  through the BCMA bus interface
>>  
>> +config PCI_HISI
>> +	depends on OF && ARM64
>> +	bool "Hisilicon Soc HIP05 PCIe controller"
>> +	select PCIE_DW
> 
> Almost all the other DesignWare-based drivers select PCIEPORTBUS in
> addition to PCIE_DW.  I'm not sure that's the best Kconfiggery (maybe we
> should leave this up to the user), but it'd be nice if all the drivers at
> least did this consistently.  So do you have a reason for doing it
> differently?
>

Hi Bjorn,

I made a mistake there. PCIEPORTBUS should be selected as other DesignWare-based
drivers did.

>>  endmenu
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index 140d66f..ea1dbf2 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -17,3 +17,4 @@ obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
>>  obj-$(CONFIG_PCIE_IPROC) += pcie-iproc.o
>>  obj-$(CONFIG_PCIE_IPROC_PLATFORM) += pcie-iproc-platform.o
>>  obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
>> +obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
>> diff --git a/drivers/pci/host/pcie-hisi.c b/drivers/pci/host/pcie-hisi.c
>> new file mode 100644
>> index 0000000..9f2fac1
>> --- /dev/null
>> +++ b/drivers/pci/host/pcie-hisi.c
>> @@ -0,0 +1,257 @@
>> +/*
>> + * PCIe host controller driver for Hisilicon Hip05 SoCs
>> + *
>> + * Copyright (C) 2015 Hisilicon Co., Ltd. http://www.hisilicon.com
>> + *
>> + * Author: Zhou Wang <wangzhou1@hisilicon.com>
>> + *         Dacai Zhu <zhudacai@hisilicon.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/interrupt.h>
>> +#include <linux/irqdomain.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/platform_device.h>
>> +
>> +#include "pcie-designware.h"
>> +
>> +#define PCIE_SUBCTRL_MODE_REG                           0x2800
>> +#define PCIE_SUBCTRL_SYS_STATE4_REG                     0x6818
>> +#define PCIE_SLV_DBI_MODE                               0x0
>> +#define PCIE_SLV_SYSCTRL_MODE                           0x1
>> +#define PCIE_SLV_CONTENT_MODE                           0x2
>> +#define PCIE_SLV_MSI_ASID                               0x10
>> +#define PCIE_LTSSM_LINKUP_STATE                         0x11
>> +#define PCIE_LTSSM_STATE_MASK                           0x3F
>> +#define PCIE_MSI_ASID_ENABLE                            (0x1 << 12)
>> +#define PCIE_MSI_ASID_VALUE                             (0x1 << 16)
>> +#define PCIE_MSI_TRANS_ENABLE                           (0x1 << 12)
>> +#define PCIE_MSI_TRANS_REG                              0x1c8
>> +#define PCIE_MSI_LOW_ADDRESS                            0x1b4
>> +#define PCIE_MSI_HIGH_ADDRESS                           0x1c4
>> +#define PCIE_MSI_ADDRESS_VAL                            0xb7010040
>> +
>> +#define to_hisi_pcie(x)	container_of(x, struct hisi_pcie, pp)
>> +
>> +struct hisi_pcie {
>> +	void __iomem *subctrl_base;
>> +	void __iomem *reg_base;
>> +	struct msi_controller *msi;
>> +	u32 port_id;
>> +	struct pcie_port pp;
>> +};
>> +
>> +static inline void hisi_pcie_subctrl_writel(struct hisi_pcie *pcie,
>> +					    u32 val, u32 reg)
>> +{
>> +	writel(val, pcie->subctrl_base + reg);
>> +}
>> +
>> +static inline u32 hisi_pcie_subctrl_readl(struct hisi_pcie *pcie, u32 reg)
>> +{
>> +	return readl(pcie->subctrl_base + reg);
>> +}
>> +
>> +static inline void hisi_pcie_apb_writel(struct hisi_pcie *pcie,
>> +					u32 val, u32 reg)
>> +{
>> +	writel(val, pcie->reg_base + reg);
>> +}
>> +
>> +static inline u32 hisi_pcie_apb_readl(struct hisi_pcie *pcie, u32 reg)
>> +{
>> +	return readl(pcie->reg_base + reg);
>> +}
>> +
>> +/*
>> + * Change mode to indicate the same reg_base to base of PCIe host configure
>> + * registers, base of RC configure space or base of vmid/asid context table
>> + */
>> +static void hisi_pcie_change_apb_mode(struct hisi_pcie *pcie, u32 mode)
>> +{
>> +	u32 val;
>> +	u32 bit_mask;
>> +	u32 bit_shift;
>> +	u32 port_id = pcie->port_id;
>> +	u32 reg = PCIE_SUBCTRL_MODE_REG + 0x100 * port_id;
>> +
>> +	if ((port_id == 1) || (port_id == 2)) {
>> +		bit_mask = 0xc;
>> +		bit_shift = 0x2;
>> +	} else {
>> +		bit_mask = 0x6;
>> +		bit_shift = 0x1;
>> +	}
>> +
>> +	val = hisi_pcie_subctrl_readl(pcie, reg);
>> +	val = (val & (~bit_mask)) | (mode << bit_shift);
>> +	hisi_pcie_subctrl_writel(pcie, val, reg);
>> +}
>> +
>> +/* Configure vmid/asid table in PCIe host */
>> +static void hisi_pcie_config_context(struct hisi_pcie *pcie)
>> +{
>> +	int i;
>> +
>> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_CONTENT_MODE);
>> +
>> +	/*
>> +	 * init vmid and asid tables for all PCIes device as 0
> 
> s/PCIes device as/PCIe devices to/ ?
>

Thanks, will modify this.

>> +	 * vmid table: 0 ~ 0x3ff, asid table: 0x400 ~ 0x7ff
>> +	 */
>> +	for (i = 0; i < 0x800; i++)
>> +		hisi_pcie_apb_writel(pcie, 0x0, i * 4);
>> +
>> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_SYSCTRL_MODE);
>> +
>> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_ADDRESS_VAL, PCIE_MSI_LOW_ADDRESS);
>> +	hisi_pcie_apb_writel(pcie, 0x0, PCIE_MSI_HIGH_ADDRESS);
>> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_ASID_ENABLE | PCIE_MSI_ASID_VALUE,
>> +			     PCIE_SLV_MSI_ASID);
>> +	hisi_pcie_apb_writel(pcie, PCIE_MSI_TRANS_ENABLE, PCIE_MSI_TRANS_REG);
>> +
>> +	hisi_pcie_change_apb_mode(pcie, PCIE_SLV_DBI_MODE);
>> +}
>> +
>> +static int hisi_pcie_link_up(struct pcie_port *pp)
>> +{
>> +	u32 val;
>> +
> 
> Remove this extra blank line.
> 

Yes, will remove it. Thanks.

>> +	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
>> +
>> +	val = hisi_pcie_subctrl_readl(hisi_pcie, PCIE_SUBCTRL_SYS_STATE4_REG +
>> +				      0x100 * hisi_pcie->port_id);
>> +
>> +	return ((val & PCIE_LTSSM_STATE_MASK) == PCIE_LTSSM_LINKUP_STATE);
>> +}
>> +
>> +static
>> +int hisi_pcie_msi_host_init(struct pcie_port *pp, struct msi_controller *chip)
>> +{
>> +	struct device_node *msi_node;
>> +	struct irq_domain *irq_domain;
>> +	struct device_node *np = pp->dev->of_node;
>> +
>> +	msi_node = of_parse_phandle(np, "msi-parent", 0);
>> +	if (!msi_node) {
>> +		pr_err("failed to find msi-parent\n");
> 
> Can you use dev_err() here (and below)?  A message like this that doesn't
> have a hint about what device it's related to is not as useful as it could
> be.
>

Right, will use dev_err(). Thanks.

>> +		return -ENODEV;
>> +	}
>> +
>> +	irq_domain = irq_find_host(msi_node);
>> +	if (!irq_domain) {
>> +		pr_err("failed to find irq domain\n");
>> +		return -ENODEV;
>> +	}
>> +
>> +	pp->irq_domain = irq_domain;
>> +
>> +	return 0;
>> +}
>> +
>> +static struct pcie_host_ops hisi_pcie_host_ops = {
>> +	.link_up = hisi_pcie_link_up,
>> +	.msi_host_init = hisi_pcie_msi_host_init,
>> +};
>> +
>> +static int __init hisi_add_pcie_port(struct pcie_port *pp,
>> +				     struct platform_device *pdev)
>> +{
>> +	int ret;
>> +	u32 port_id;
>> +	struct resource busn;
>> +
> 
> Remove this extra blank line.

Will remove it. Thanks.

> 
>> +	struct hisi_pcie *hisi_pcie = to_hisi_pcie(pp);
>> +
>> +	if (of_property_read_u32(pdev->dev.of_node, "port-id", &port_id)) {
>> +		dev_err(&pdev->dev, "failed to read port-id\n");
>> +		return -EINVAL;
>> +	}
>> +	if (port_id > 3) {
>> +		dev_err(&pdev->dev, "Invalid port-id\n");
> 
> Since you have the invalid port_id here, maybe you could include the
> invalid value in the output?
>

Right, will do like this. Thanks.

>> +		return -EINVAL;
>> +	}
>> +
>> +	hisi_pcie->port_id = port_id;
>> +
>> +	if (of_pci_parse_bus_range(pdev->dev.of_node, &busn)) {
>> +		dev_err(&pdev->dev, "failed to parse bus-ranges\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	pp->root_bus_nr = busn.start;
>> +	pp->ops = &hisi_pcie_host_ops;
>> +
>> +	hisi_pcie_config_context(hisi_pcie);
>> +
>> +	ret = dw_pcie_host_init(pp);
>> +	if (ret) {
>> +		dev_err(&pdev->dev, "failed to initialize host\n");
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int __init hisi_pcie_probe(struct platform_device *pdev)
>> +{
>> +	struct hisi_pcie *hisi_pcie;
>> +	struct pcie_port *pp;
>> +	struct resource *reg;
>> +	struct resource *subctrl;
>> +	int ret;
>> +
>> +	hisi_pcie = devm_kzalloc(&pdev->dev, sizeof(*hisi_pcie), GFP_KERNEL);
>> +	if (!hisi_pcie)
>> +		return -ENOMEM;
>> +
>> +	pp = &hisi_pcie->pp;
>> +	pp->dev = &pdev->dev;
>> +
>> +	subctrl = platform_get_resource_byname(pdev, IORESOURCE_MEM, "subctrl");
>> +	hisi_pcie->subctrl_base = devm_ioremap_nocache(&pdev->dev,
>> +					subctrl->start, resource_size(subctrl));
>> +	if (IS_ERR(hisi_pcie->subctrl_base)) {
>> +		dev_err(pp->dev, "cannot get subctrl base\n");
>> +		return PTR_ERR(hisi_pcie->subctrl_base);
>> +	}
>> +
>> +	reg = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
>> +	hisi_pcie->reg_base = devm_ioremap_resource(&pdev->dev, reg);
>> +	if (IS_ERR(hisi_pcie->reg_base)) {
>> +		dev_err(pp->dev, "cannot get reg base\n");
> 
> Maybe this could include the actual resource name ("rc_dbi") as the message
> above does?
>

Will modify this message. Thanks.

>> +		return PTR_ERR(hisi_pcie->reg_base);
>> +	}
>> +
>> +	hisi_pcie->pp.dbi_base = hisi_pcie->reg_base;
>> +
>> +	ret = hisi_add_pcie_port(pp, pdev);
>> +	if (ret < 0)
> 
> Seems like this return value check should be the same as the one above in 
> hisi_add_pcie_port().  In both cases, the only possible returns are zero
> (success) or a negative error number.  Using "if (ret)" here has the
> advantage that below you could "return 0" and it would be crystal clear
> that you're always returning success if you get that far.
> 

Got it, will do like this. Thanks for your suggestion.

>> +		return ret;
>> +
>> +	platform_set_drvdata(pdev, hisi_pcie);
>> +
>> +	return ret;
>> +}
>> +
>> +static const struct of_device_id hisi_pcie_of_match[] = {
>> +	{.compatible = "hisilicon,hip05-pcie",},
>> +	{},
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, hisi_pcie_of_match);
>> +
>> +static struct platform_driver hisi_pcie_driver = {
>> +	.probe  = hisi_pcie_probe,
>> +	.driver = {
>> +		   .name = "hisi-pcie",
>> +		   .owner = THIS_MODULE,
> 
> You can drop this ".owner =" initialization.  platform_driver_register()
> supplies that automatically.
> 

Will drop it, Thanks.

Best regards,
Zhou

>> +		   .of_match_table = hisi_pcie_of_match,
>> +	},
>> +};
>> +
>> +module_platform_driver(hisi_pcie_driver);
>> -- 
>> 1.9.1
>>
>> --
>> 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] 36+ messages in thread

* Re: [PATCH v4 4/5] Documentation: DT: Add Hisilicon PCIe host binding
  2015-07-21 23:02       ` Bjorn Helgaas
  (?)
@ 2015-07-22  2:50           ` Zhou Wang
  -1 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-22  2:50 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni-hv44wF8Li93QT0dZR+AlfA,
	lorenzo.pieralisi-5wv7dgnIgG8, James Morse,
	Liviu.Dudau-5wv7dgnIgG8, linux-pci-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	yuanzhichang-C8/M+/jPZTeaMJb+Lgu22Q,
	zhudacai-C8/M+/jPZTeaMJb+Lgu22Q,
	zhangjukuo-hv44wF8Li93QT0dZR+AlfA,
	qiuzhenfa-C8/M+/jPZTeaMJb+Lgu22Q,
	liudongdong3-hv44wF8Li93QT0dZR+AlfA,
	qiujiang-hv44wF8Li93QT0dZR+AlfA,
	kangfenglong-hv44wF8Li93QT0dZR+AlfA,
	liguozhu-C8/M+/jPZTeaMJb+Lgu22Q, Rob Herring

On 2015/7/22 7:02, Bjorn Helgaas wrote:
> [+cc Rob]
> 
> On Tue, Jul 21, 2015 at 02:48:42PM +0800, Zhou Wang wrote:
>> This patch adds related DTS binding document for Hisilicon PCIe host driver.
>>
>> Signed-off-by: Zhou Wang <wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> 
> If I merge this via my tree, I'm looking for an ack from Arnd and/or Rob
> here.
> 
>> ---
>>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>> new file mode 100644
>> index 0000000..6c9b827
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>> @@ -0,0 +1,46 @@
>> +Hisilicon PCIe host bridge DT description
> 
> Even the website at http://hisilicon.com isn't consistent, but there is
> some indication that the correct capitalization would be "HiSilicon".
> Since this is English text, feel free to capitalize it correctly here :)
>

Hi Bjorn,

I checked with related colleagues about this. It should be "HiSilicon" as
formal name. Thanks for pointing this :)

> Similarly, Synopsys seems to use "DesignWare," so I try to use that when
> it makes sense.
> 
>> +Hisilicon PCIe host controller is based on Designware PCI core.
>> +It shares common functions with PCIe Designware core driver and inherits
>> +common properties defined in
>> +Documentation/devicetree/bindings/pci/designware-pci.txt.
>> +
>> +Additional properties are described here:
>> +
>> +Required properties:
>> +- compatible: Should contain "hisilicon,hip05-pcie".
>> +- reg: Should contain rc_dbi, subctrl, config registers location and length.
>> +- reg-names: Must include the following entries:
>> +  "rc_dbi": controller configuration registers;
>> +  "subctrl": whole PCIe hosts configuration registers;
>> +  "config": PCIe configuration space registers.
>> +- msi-parent: Should be its_pcie which is an its receiving MSI interrupts.
> 
> I guess "its" here is an abbreviation for something; if so, this would read
> better as "... which is an ITS receiving ..."
> 

It is ITS(Interrupt Translation Service) in GIC v3. Will modify it.

>> +- port-id: Should be 0, 1, 2 or 3.
>> +
>> +Optional properties:
>> +- status: Either "ok" or "disabled".
>> +- dma-coherent: Present if dma operations are coherent.
> 
> "if DMA operations"
>

Thanks, will modify this.

Best regards,
Zhou

>> +
>> +Example:
>> +	pcie@0xb0080000 {
>> +		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
>> +		reg = <0 0xb0080000 0 0x10000>, <0 0xb0000000 0 0x10000>,
>> +		      <0x220 0x00000000 0 0x2000>;
>> +		reg-names = "rc_dbi", "subctrl", "config";
>> +		bus-range = <0  15>;
>> +		msi-parent = <&its_pcie>;
>> +		#address-cells = <3>;
>> +		#size-cells = <2>;
>> +		device_type = "pci";
>> +		dma-coherent;
>> +		ranges = <0x82000000 0 0x00000000 0x220 0x00000000 0 0x10000000>;
>> +		num-lanes = <8>;
>> +		port-id = <1>;
>> +		#interrupts-cells = <1>;
>> +		interrupts-map-mask = <0xf800 0 0 7>;
>> +		interrupts-map = <0x0 0 0 1 &mbigen_pcie 1 10
>> +				  0x0 0 0 2 &mbigen_pcie 2 11
>> +				  0x0 0 0 3 &mbigen_pcie 3 12
>> +				  0x0 0 0 4 &mbigen_pcie 4 13>;
>> +		status = "ok";
>> +	};
>> -- 
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> .
> 


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH v4 4/5] Documentation: DT: Add Hisilicon PCIe host binding
@ 2015-07-22  2:50           ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-22  2:50 UTC (permalink / raw
  To: linux-arm-kernel

On 2015/7/22 7:02, Bjorn Helgaas wrote:
> [+cc Rob]
> 
> On Tue, Jul 21, 2015 at 02:48:42PM +0800, Zhou Wang wrote:
>> This patch adds related DTS binding document for Hisilicon PCIe host driver.
>>
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> 
> If I merge this via my tree, I'm looking for an ack from Arnd and/or Rob
> here.
> 
>> ---
>>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>> new file mode 100644
>> index 0000000..6c9b827
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>> @@ -0,0 +1,46 @@
>> +Hisilicon PCIe host bridge DT description
> 
> Even the website at http://hisilicon.com isn't consistent, but there is
> some indication that the correct capitalization would be "HiSilicon".
> Since this is English text, feel free to capitalize it correctly here :)
>

Hi Bjorn,

I checked with related colleagues about this. It should be "HiSilicon" as
formal name. Thanks for pointing this :)

> Similarly, Synopsys seems to use "DesignWare," so I try to use that when
> it makes sense.
> 
>> +Hisilicon PCIe host controller is based on Designware PCI core.
>> +It shares common functions with PCIe Designware core driver and inherits
>> +common properties defined in
>> +Documentation/devicetree/bindings/pci/designware-pci.txt.
>> +
>> +Additional properties are described here:
>> +
>> +Required properties:
>> +- compatible: Should contain "hisilicon,hip05-pcie".
>> +- reg: Should contain rc_dbi, subctrl, config registers location and length.
>> +- reg-names: Must include the following entries:
>> +  "rc_dbi": controller configuration registers;
>> +  "subctrl": whole PCIe hosts configuration registers;
>> +  "config": PCIe configuration space registers.
>> +- msi-parent: Should be its_pcie which is an its receiving MSI interrupts.
> 
> I guess "its" here is an abbreviation for something; if so, this would read
> better as "... which is an ITS receiving ..."
> 

It is ITS(Interrupt Translation Service) in GIC v3. Will modify it.

>> +- port-id: Should be 0, 1, 2 or 3.
>> +
>> +Optional properties:
>> +- status: Either "ok" or "disabled".
>> +- dma-coherent: Present if dma operations are coherent.
> 
> "if DMA operations"
>

Thanks, will modify this.

Best regards,
Zhou

>> +
>> +Example:
>> +	pcie at 0xb0080000 {
>> +		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
>> +		reg = <0 0xb0080000 0 0x10000>, <0 0xb0000000 0 0x10000>,
>> +		      <0x220 0x00000000 0 0x2000>;
>> +		reg-names = "rc_dbi", "subctrl", "config";
>> +		bus-range = <0  15>;
>> +		msi-parent = <&its_pcie>;
>> +		#address-cells = <3>;
>> +		#size-cells = <2>;
>> +		device_type = "pci";
>> +		dma-coherent;
>> +		ranges = <0x82000000 0 0x00000000 0x220 0x00000000 0 0x10000000>;
>> +		num-lanes = <8>;
>> +		port-id = <1>;
>> +		#interrupts-cells = <1>;
>> +		interrupts-map-mask = <0xf800 0 0 7>;
>> +		interrupts-map = <0x0 0 0 1 &mbigen_pcie 1 10
>> +				  0x0 0 0 2 &mbigen_pcie 2 11
>> +				  0x0 0 0 3 &mbigen_pcie 3 12
>> +				  0x0 0 0 4 &mbigen_pcie 4 13>;
>> +		status = "ok";
>> +	};
>> -- 
>> 1.9.1
>>
>> --
>> 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] 36+ messages in thread

* Re: [PATCH v4 4/5] Documentation: DT: Add Hisilicon PCIe host binding
@ 2015-07-22  2:50           ` Zhou Wang
  0 siblings, 0 replies; 36+ messages in thread
From: Zhou Wang @ 2015-07-22  2:50 UTC (permalink / raw
  To: Bjorn Helgaas
  Cc: Jingoo Han, Pratyush Anand, Arnd Bergmann, gabriele.paoloni,
	lorenzo.pieralisi, James Morse, Liviu.Dudau, linux-pci,
	linux-arm-kernel, devicetree, yuanzhichang, zhudacai, zhangjukuo,
	qiuzhenfa, liudongdong3, qiujiang, kangfenglong, liguozhu,
	Rob Herring

On 2015/7/22 7:02, Bjorn Helgaas wrote:
> [+cc Rob]
> 
> On Tue, Jul 21, 2015 at 02:48:42PM +0800, Zhou Wang wrote:
>> This patch adds related DTS binding document for Hisilicon PCIe host driver.
>>
>> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> 
> If I merge this via my tree, I'm looking for an ack from Arnd and/or Rob
> here.
> 
>> ---
>>  .../devicetree/bindings/pci/hisilicon-pcie.txt     | 46 ++++++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>> new file mode 100644
>> index 0000000..6c9b827
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/hisilicon-pcie.txt
>> @@ -0,0 +1,46 @@
>> +Hisilicon PCIe host bridge DT description
> 
> Even the website at http://hisilicon.com isn't consistent, but there is
> some indication that the correct capitalization would be "HiSilicon".
> Since this is English text, feel free to capitalize it correctly here :)
>

Hi Bjorn,

I checked with related colleagues about this. It should be "HiSilicon" as
formal name. Thanks for pointing this :)

> Similarly, Synopsys seems to use "DesignWare," so I try to use that when
> it makes sense.
> 
>> +Hisilicon PCIe host controller is based on Designware PCI core.
>> +It shares common functions with PCIe Designware core driver and inherits
>> +common properties defined in
>> +Documentation/devicetree/bindings/pci/designware-pci.txt.
>> +
>> +Additional properties are described here:
>> +
>> +Required properties:
>> +- compatible: Should contain "hisilicon,hip05-pcie".
>> +- reg: Should contain rc_dbi, subctrl, config registers location and length.
>> +- reg-names: Must include the following entries:
>> +  "rc_dbi": controller configuration registers;
>> +  "subctrl": whole PCIe hosts configuration registers;
>> +  "config": PCIe configuration space registers.
>> +- msi-parent: Should be its_pcie which is an its receiving MSI interrupts.
> 
> I guess "its" here is an abbreviation for something; if so, this would read
> better as "... which is an ITS receiving ..."
> 

It is ITS(Interrupt Translation Service) in GIC v3. Will modify it.

>> +- port-id: Should be 0, 1, 2 or 3.
>> +
>> +Optional properties:
>> +- status: Either "ok" or "disabled".
>> +- dma-coherent: Present if dma operations are coherent.
> 
> "if DMA operations"
>

Thanks, will modify this.

Best regards,
Zhou

>> +
>> +Example:
>> +	pcie@0xb0080000 {
>> +		compatible = "hisilicon,hip05-pcie", "snps,dw-pcie";
>> +		reg = <0 0xb0080000 0 0x10000>, <0 0xb0000000 0 0x10000>,
>> +		      <0x220 0x00000000 0 0x2000>;
>> +		reg-names = "rc_dbi", "subctrl", "config";
>> +		bus-range = <0  15>;
>> +		msi-parent = <&its_pcie>;
>> +		#address-cells = <3>;
>> +		#size-cells = <2>;
>> +		device_type = "pci";
>> +		dma-coherent;
>> +		ranges = <0x82000000 0 0x00000000 0x220 0x00000000 0 0x10000000>;
>> +		num-lanes = <8>;
>> +		port-id = <1>;
>> +		#interrupts-cells = <1>;
>> +		interrupts-map-mask = <0xf800 0 0 7>;
>> +		interrupts-map = <0x0 0 0 1 &mbigen_pcie 1 10
>> +				  0x0 0 0 2 &mbigen_pcie 2 11
>> +				  0x0 0 0 3 &mbigen_pcie 3 12
>> +				  0x0 0 0 4 &mbigen_pcie 4 13>;
>> +		status = "ok";
>> +	};
>> -- 
>> 1.9.1
>>
>> --
>> 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] 36+ messages in thread

* Re: [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data
  2015-07-21  6:48   ` Zhou Wang
  (?)
@ 2015-07-23 18:06     ` Lorenzo Pieralisi
  -1 siblings, 0 replies; 36+ messages in thread
From: Lorenzo Pieralisi @ 2015-07-23 18:06 UTC (permalink / raw
  To: Zhou Wang
  Cc: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni@huawei.com, James Morse, Liviu Dudau,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, yuanzhichang@hisilicon.com,
	zhudacai@hisilicon.com, zhangjukuo@huawei.com,
	qiuzhenfa@hisilicon.com, liudongdong3@huawei.com,
	qiujiang@huawei.com, kangfenglong@huawei.com,
	liguozhu@hisilicon.com

[CC'ing Thomas and Jason for pci-mvebu]

On Tue, Jul 21, 2015 at 07:48:39AM +0100, Zhou Wang wrote:
> This patch is needed in order to unify the PCIe designware framework for ARM and
> ARM64 architectures. In the PCIe designware unification process we are calling
> pci_create_root_bus() passing a "sysdata" parameter that is the same for both
> ARM and ARM64 and is of type "struct pcie_port*". In the ARM case this will
> cause a problem with the function pcibios_align_resource(); in fact this will
> cast "dev->sysdata" to "struct pci_sys_data*", whereas designware had passed a
> "struct pcie_port*" pointer.
> 
> This patch solves the issue by removing "align_resource" from "pci_sys_data"
> struct and defining a static global function pointer in "bios32.c"
> 
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni at huawei.com>
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>

Arnd, Rob any opinion on this ? It is really the last blocking bit
to having common ARM/ARM64 drivers (and get rid of pci_sys_data) so I
would like to get this sorted asap (having a global function pointer
might be a temporary solution before moving it to the host bridge
structure).

Comments welcome.

Thanks !
Lorenzo

> ---
>  arch/arm/include/asm/mach/pci.h |  5 -----
>  arch/arm/kernel/bios32.c        | 12 ++++++++----
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
> index 28b9bb3..8a4e4de 100644
> --- a/arch/arm/include/asm/mach/pci.h
> +++ b/arch/arm/include/asm/mach/pci.h
> @@ -58,11 +58,6 @@ struct pci_sys_data {
>  					/* IRQ mapping				*/
>  	int		(*map_irq)(const struct pci_dev *, u8, u8);
>  					/* Resource alignement requirements	*/
> -	resource_size_t (*align_resource)(struct pci_dev *dev,
> -					  const struct resource *res,
> -					  resource_size_t start,
> -					  resource_size_t size,
> -					  resource_size_t align);
>  	void		*private_data;	/* platform controller private data	*/
>  };
>  
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index fcbbbb1..4cdc64d 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -17,6 +17,11 @@
>  #include <asm/mach/pci.h>
>  
>  static int debug_pci;
> +static resource_size_t (*align_resource)(struct pci_dev *dev,
> +		  const struct resource *res,
> +		  resource_size_t start,
> +		  resource_size_t size,
> +		  resource_size_t align) = NULL;
>  
>  #ifdef CONFIG_PCI_MSI
>  struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
> @@ -468,7 +473,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
>  		sys->busnr   = busnr;
>  		sys->swizzle = hw->swizzle;
>  		sys->map_irq = hw->map_irq;
> -		sys->align_resource = hw->align_resource;
> +		align_resource = hw->align_resource;
>  		INIT_LIST_HEAD(&sys->resources);
>  
>  		if (hw->private_data)
> @@ -589,7 +594,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  				resource_size_t size, resource_size_t align)
>  {
>  	struct pci_dev *dev = data;
> -	struct pci_sys_data *sys = dev->sysdata;
>  	resource_size_t start = res->start;
>  
>  	if (res->flags & IORESOURCE_IO && start & 0x300)
> @@ -597,8 +601,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  
>  	start = (start + align - 1) & ~(align - 1);
>  
> -	if (sys->align_resource)
> -		return sys->align_resource(dev, res, start, size, align);
> +	if (align_resource)
> +		return align_resource(dev, res, start, size, align);
>  
>  	return start;
>  }
> -- 
> 1.9.1
> 
> --
> 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] 36+ messages in thread

* [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data
@ 2015-07-23 18:06     ` Lorenzo Pieralisi
  0 siblings, 0 replies; 36+ messages in thread
From: Lorenzo Pieralisi @ 2015-07-23 18:06 UTC (permalink / raw
  To: linux-arm-kernel

[CC'ing Thomas and Jason for pci-mvebu]

On Tue, Jul 21, 2015 at 07:48:39AM +0100, Zhou Wang wrote:
> This patch is needed in order to unify the PCIe designware framework for ARM and
> ARM64 architectures. In the PCIe designware unification process we are calling
> pci_create_root_bus() passing a "sysdata" parameter that is the same for both
> ARM and ARM64 and is of type "struct pcie_port*". In the ARM case this will
> cause a problem with the function pcibios_align_resource(); in fact this will
> cast "dev->sysdata" to "struct pci_sys_data*", whereas designware had passed a
> "struct pcie_port*" pointer.
> 
> This patch solves the issue by removing "align_resource" from "pci_sys_data"
> struct and defining a static global function pointer in "bios32.c"
> 
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>

Arnd, Rob any opinion on this ? It is really the last blocking bit
to having common ARM/ARM64 drivers (and get rid of pci_sys_data) so I
would like to get this sorted asap (having a global function pointer
might be a temporary solution before moving it to the host bridge
structure).

Comments welcome.

Thanks !
Lorenzo

> ---
>  arch/arm/include/asm/mach/pci.h |  5 -----
>  arch/arm/kernel/bios32.c        | 12 ++++++++----
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
> index 28b9bb3..8a4e4de 100644
> --- a/arch/arm/include/asm/mach/pci.h
> +++ b/arch/arm/include/asm/mach/pci.h
> @@ -58,11 +58,6 @@ struct pci_sys_data {
>  					/* IRQ mapping				*/
>  	int		(*map_irq)(const struct pci_dev *, u8, u8);
>  					/* Resource alignement requirements	*/
> -	resource_size_t (*align_resource)(struct pci_dev *dev,
> -					  const struct resource *res,
> -					  resource_size_t start,
> -					  resource_size_t size,
> -					  resource_size_t align);
>  	void		*private_data;	/* platform controller private data	*/
>  };
>  
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index fcbbbb1..4cdc64d 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -17,6 +17,11 @@
>  #include <asm/mach/pci.h>
>  
>  static int debug_pci;
> +static resource_size_t (*align_resource)(struct pci_dev *dev,
> +		  const struct resource *res,
> +		  resource_size_t start,
> +		  resource_size_t size,
> +		  resource_size_t align) = NULL;
>  
>  #ifdef CONFIG_PCI_MSI
>  struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
> @@ -468,7 +473,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
>  		sys->busnr   = busnr;
>  		sys->swizzle = hw->swizzle;
>  		sys->map_irq = hw->map_irq;
> -		sys->align_resource = hw->align_resource;
> +		align_resource = hw->align_resource;
>  		INIT_LIST_HEAD(&sys->resources);
>  
>  		if (hw->private_data)
> @@ -589,7 +594,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  				resource_size_t size, resource_size_t align)
>  {
>  	struct pci_dev *dev = data;
> -	struct pci_sys_data *sys = dev->sysdata;
>  	resource_size_t start = res->start;
>  
>  	if (res->flags & IORESOURCE_IO && start & 0x300)
> @@ -597,8 +601,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  
>  	start = (start + align - 1) & ~(align - 1);
>  
> -	if (sys->align_resource)
> -		return sys->align_resource(dev, res, start, size, align);
> +	if (align_resource)
> +		return align_resource(dev, res, start, size, align);
>  
>  	return start;
>  }
> -- 
> 1.9.1
> 
> --
> 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] 36+ messages in thread

* Re: [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data
@ 2015-07-23 18:06     ` Lorenzo Pieralisi
  0 siblings, 0 replies; 36+ messages in thread
From: Lorenzo Pieralisi @ 2015-07-23 18:06 UTC (permalink / raw
  To: Zhou Wang
  Cc: Bjorn Helgaas, Jingoo Han, Pratyush Anand, Arnd Bergmann,
	gabriele.paoloni@huawei.com, James Morse, Liviu Dudau,
	linux-pci@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	devicetree@vger.kernel.org, yuanzhichang@hisilicon.com,
	zhudacai@hisilicon.com, zhangjukuo@huawei.com,
	qiuzhenfa@hisilicon.com, liudongdong3@huawei.com,
	qiujiang@huawei.com, kangfenglong@huawei.com,
	liguozhu@hisilicon.com, thomas.petazzoni, Jason Cooper, robh

[CC'ing Thomas and Jason for pci-mvebu]

On Tue, Jul 21, 2015 at 07:48:39AM +0100, Zhou Wang wrote:
> This patch is needed in order to unify the PCIe designware framework for ARM and
> ARM64 architectures. In the PCIe designware unification process we are calling
> pci_create_root_bus() passing a "sysdata" parameter that is the same for both
> ARM and ARM64 and is of type "struct pcie_port*". In the ARM case this will
> cause a problem with the function pcibios_align_resource(); in fact this will
> cast "dev->sysdata" to "struct pci_sys_data*", whereas designware had passed a
> "struct pcie_port*" pointer.
> 
> This patch solves the issue by removing "align_resource" from "pci_sys_data"
> struct and defining a static global function pointer in "bios32.c"
> 
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni at huawei.com>
> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>

Arnd, Rob any opinion on this ? It is really the last blocking bit
to having common ARM/ARM64 drivers (and get rid of pci_sys_data) so I
would like to get this sorted asap (having a global function pointer
might be a temporary solution before moving it to the host bridge
structure).

Comments welcome.

Thanks !
Lorenzo

> ---
>  arch/arm/include/asm/mach/pci.h |  5 -----
>  arch/arm/kernel/bios32.c        | 12 ++++++++----
>  2 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h
> index 28b9bb3..8a4e4de 100644
> --- a/arch/arm/include/asm/mach/pci.h
> +++ b/arch/arm/include/asm/mach/pci.h
> @@ -58,11 +58,6 @@ struct pci_sys_data {
>  					/* IRQ mapping				*/
>  	int		(*map_irq)(const struct pci_dev *, u8, u8);
>  					/* Resource alignement requirements	*/
> -	resource_size_t (*align_resource)(struct pci_dev *dev,
> -					  const struct resource *res,
> -					  resource_size_t start,
> -					  resource_size_t size,
> -					  resource_size_t align);
>  	void		*private_data;	/* platform controller private data	*/
>  };
>  
> diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
> index fcbbbb1..4cdc64d 100644
> --- a/arch/arm/kernel/bios32.c
> +++ b/arch/arm/kernel/bios32.c
> @@ -17,6 +17,11 @@
>  #include <asm/mach/pci.h>
>  
>  static int debug_pci;
> +static resource_size_t (*align_resource)(struct pci_dev *dev,
> +		  const struct resource *res,
> +		  resource_size_t start,
> +		  resource_size_t size,
> +		  resource_size_t align) = NULL;
>  
>  #ifdef CONFIG_PCI_MSI
>  struct msi_controller *pcibios_msi_controller(struct pci_dev *dev)
> @@ -468,7 +473,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw,
>  		sys->busnr   = busnr;
>  		sys->swizzle = hw->swizzle;
>  		sys->map_irq = hw->map_irq;
> -		sys->align_resource = hw->align_resource;
> +		align_resource = hw->align_resource;
>  		INIT_LIST_HEAD(&sys->resources);
>  
>  		if (hw->private_data)
> @@ -589,7 +594,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  				resource_size_t size, resource_size_t align)
>  {
>  	struct pci_dev *dev = data;
> -	struct pci_sys_data *sys = dev->sysdata;
>  	resource_size_t start = res->start;
>  
>  	if (res->flags & IORESOURCE_IO && start & 0x300)
> @@ -597,8 +601,8 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
>  
>  	start = (start + align - 1) & ~(align - 1);
>  
> -	if (sys->align_resource)
> -		return sys->align_resource(dev, res, start, size, align);
> +	if (align_resource)
> +		return align_resource(dev, res, start, size, align);
>  
>  	return start;
>  }
> -- 
> 1.9.1
> 
> --
> 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] 36+ messages in thread

end of thread, other threads:[~2015-07-23 18:06 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-21  6:48 [PATCH v4 0/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05 Zhou Wang
2015-07-21  6:48 ` Zhou Wang
2015-07-21  6:48 ` Zhou Wang
2015-07-21  6:48 ` [PATCH v4 1/5] ARM/PCI: remove align_resource in pci_sys_data Zhou Wang
2015-07-21  6:48   ` Zhou Wang
2015-07-21  6:48   ` Zhou Wang
2015-07-21 22:44   ` Bjorn Helgaas
2015-07-21 22:44     ` Bjorn Helgaas
2015-07-22  2:00     ` Zhou Wang
2015-07-22  2:00       ` Zhou Wang
2015-07-22  2:00       ` Zhou Wang
2015-07-23 18:06   ` Lorenzo Pieralisi
2015-07-23 18:06     ` Lorenzo Pieralisi
2015-07-23 18:06     ` Lorenzo Pieralisi
2015-07-21  6:48 ` [PATCH v4 2/5] PCI: designware: Add ARM64 support Zhou Wang
2015-07-21  6:48   ` Zhou Wang
2015-07-21  6:48   ` Zhou Wang
2015-07-21  6:48 ` [PATCH v4 3/5] PCI: hisi: Add PCIe host support for Hisilicon Soc Hip05 Zhou Wang
2015-07-21  6:48   ` Zhou Wang
2015-07-21  6:48   ` Zhou Wang
2015-07-21 22:37   ` Bjorn Helgaas
2015-07-21 22:37     ` Bjorn Helgaas
2015-07-22  2:33     ` Zhou Wang
2015-07-22  2:33       ` Zhou Wang
2015-07-22  2:33       ` Zhou Wang
     [not found] ` <1437461323-3531-1-git-send-email-wangzhou1-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
2015-07-21  6:48   ` [PATCH v4 4/5] Documentation: DT: Add Hisilicon PCIe host binding Zhou Wang
2015-07-21  6:48     ` Zhou Wang
2015-07-21  6:48     ` Zhou Wang
2015-07-21 23:02     ` Bjorn Helgaas
2015-07-21 23:02       ` Bjorn Helgaas
     [not found]       ` <20150721230205.GN21967-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2015-07-22  2:50         ` Zhou Wang
2015-07-22  2:50           ` Zhou Wang
2015-07-22  2:50           ` Zhou Wang
2015-07-21  6:48   ` [PATCH v4 5/5] MAINTAINERS: Add pcie-hisi maintainer Zhou Wang
2015-07-21  6:48     ` Zhou Wang
2015-07-21  6:48     ` Zhou Wang

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.