[PATCH v2 0/5] PCI: st: provide support for dw pcie

[PATCH v2 0/5] PCI: st: provide support for dw pcie

[Gabriel FERNANDEZ]

Changes in v2:
 - comestic corrections in device tree binding
 - add pci-st.c into MAINTAINERS
 - remove st_pcie_ops structure to avoid another level of indirection
 - remove nasty busy-loop
 - remove useless test using virt_to_phys()
 - move disable io support into dw-pcie driver

I don't change the st_pcie_abort_handler() function because abort handling
is masked during boot.


This patch-set introduces a STMicroelectronics PCIe controller.
It's based on designware PCIe driver.

Gabriel Fernandez (5):
  ARM: STi: Kconfig update for PCIe support
  PCI: st: Add Device Tree bindings for sti pcie
  PCI: st: Provide support for the sti PCIe controller
  PCI: designware: Add disable IO support
  MAINTAINERS: Add pci-st.c to ARCH/STI architecture

 .../devicetree/bindings/pci/designware-pcie.txt    |   2 +
 Documentation/devicetree/bindings/pci/st-pcie.txt  |  54 ++
 MAINTAINERS                                        |   1 +
 arch/arm/mach-sti/Kconfig                          |   2 +
 drivers/pci/host/Kconfig                           |   9 +
 drivers/pci/host/Makefile                          |   1 +
 drivers/pci/host/pci-st.c                          | 617 +++++++++++++++++++++
 drivers/pci/host/pcie-designware.c                 |  24 +-
 drivers/pci/host/pcie-designware.h                 |   1 +
 9 files changed, 709 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/st-pcie.txt
 create mode 100644 drivers/pci/host/pci-st.c

-- 
1.9.1

[PATCH v2 1/5] ARM: STi: Kconfig update for PCIe support

[Gabriel FERNANDEZ]

Update Kconfig:
- MIGHT_HAVE_PCI
- PCI_DOMAINS

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
---
 arch/arm/mach-sti/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-sti/Kconfig b/arch/arm/mach-sti/Kconfig
index 3b1ac46..7f9b432 100644
--- a/arch/arm/mach-sti/Kconfig
+++ b/arch/arm/mach-sti/Kconfig
@@ -8,6 +8,8 @@ menuconfig ARCH_STI
 	select ARCH_HAS_RESET_CONTROLLER
 	select HAVE_ARM_SCU if SMP
 	select ARCH_REQUIRE_GPIOLIB
+	select PCI_DOMAINS if PCI
+	select MIGHT_HAVE_PCI
 	select ARM_ERRATA_754322
 	select ARM_ERRATA_764369 if SMP
 	select ARM_ERRATA_775420
-- 
1.9.1

[PATCH v2 2/5] PCI: st: Add Device Tree bindings for sti pcie

[Gabriel FERNANDEZ]

sti pcie is built around a Synopsis Designware PCIe IP.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
---
 Documentation/devicetree/bindings/pci/st-pcie.txt | 54 +++++++++++++++++++++++
 1 file changed, 54 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pci/st-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/st-pcie.txt b/Documentation/devicetree/bindings/pci/st-pcie.txt
new file mode 100644
index 0000000..94aae2d
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/st-pcie.txt
@@ -0,0 +1,54 @@
+STMicroelectronics STi PCIe controller
+
+This PCIe host controller is based on the Synopsis Designware PCIe IP
+and thus inherits all the common properties defined in designware-pcie.txt.
+
+Required properties:
+ - compatible: "st,stih407-pcie"
+ - reg: base address and length of the pcie controller, mem-window address
+   and length available to the controller.
+ - interrupts: A list of interrupt outputs of the controller. Must contain an
+   entry for each entry in the interrupt-names property.
+ - interrupt-names: Should be "msi". STi interrupt that is asserted when an
+   MSI is received.
+ - st,syscfg : should be a phandle of the syscfg node. Also contains syscfg
+   offset for IP configuration.
+ - resets, reset-names: the power-down and soft-reset lines of PCIe IP.
+   Associated names must be "powerdown" and "softreset".
+ - phys, phy-names: the phandle for the PHY device.
+   Associated name must be "pcie"
+
+Optional properties:
+ - reset-gpio: a GPIO spec to define which pin is connected to the bus reset.
+
+Example:
+
+pcie0: pcie at 9b00000 {
+	compatible = "st,stih407-pcie", "snps,dw-pcie";
+	device_type = "pci";
+	reg = <0x09b00000 0x4000>,	/* dbi cntrl registers */
+	      <0x2fff0000 0x00010000>,	/* configuration space */
+	      <0x40000000 0x80000000>;	/* lmi mem window */
+	reg-names = "dbi", "config", "mem-window";
+	st,syscfg = <&syscfg_core 0xd8 0xe0>;
+	#address-cells = <3>;
+	#size-cells = <2>;
+	ranges = <0x00000800 0 0x2fff0000 0x2fff0000 0 0x00010000   /* configuration space */
+		  0x82000000 0 0x20000000 0x20000000 0 0x0FFF0000>; /* non-prefetchable memory */
+	num-lanes = <1>;
+	interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
+	interrupt-names = "msi";
+	#interrupt-cells = <1>;
+	interrupt-map-mask = <0 0 0 7>;
+	interrupt-map = <0 0 0 1 &intc GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>, /* INT A */
+			<0 0 0 2 &intc GIC_SPI 161 IRQ_TYPE_LEVEL_HIGH>, /* INT B */
+			<0 0 0 3 &intc GIC_SPI 162 IRQ_TYPE_LEVEL_HIGH>, /* INT C */
+			<0 0 0 4 &intc GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>; /* INT D */
+
+	resets = <&powerdown STIH407_PCIE0_POWERDOWN>,
+		 <&softreset STIH407_PCIE0_SOFTRESET>;
+	reset-names = "powerdown",
+		      "softreset";
+	phys = <&phy_port0 PHY_TYPE_PCIE>;
+	phy-names = "pcie";
+};
-- 
1.9.1

[PATCH v2 2/5] PCI: st: Add Device Tree bindings for sti pcie

[Liviu Dudau]

Hi Gabriel,

On Mon, Mar 16, 2015 at 02:20:32PM +0000, Gabriel FERNANDEZ wrote:
> sti pcie is built around a Synopsis Designware PCIe IP.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
> ---
>  Documentation/devicetree/bindings/pci/st-pcie.txt | 54 +++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pci/st-pcie.txt
> 
> diff --git a/Documentation/devicetree/bindings/pci/st-pcie.txt b/Documentation/devicetree/bindings/pci/st-pcie.txt
> new file mode 100644
> index 0000000..94aae2d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/st-pcie.txt
> @@ -0,0 +1,54 @@
> +STMicroelectronics STi PCIe controller
> +
> +This PCIe host controller is based on the Synopsis Designware PCIe IP
> +and thus inherits all the common properties defined in designware-pcie.txt.
> +
> +Required properties:
> + - compatible: "st,stih407-pcie"
> + - reg: base address and length of the pcie controller, mem-window address
> +   and length available to the controller.
> + - interrupts: A list of interrupt outputs of the controller. Must contain an
> +   entry for each entry in the interrupt-names property.
> + - interrupt-names: Should be "msi". STi interrupt that is asserted when an
> +   MSI is received.
> + - st,syscfg : should be a phandle of the syscfg node. Also contains syscfg
> +   offset for IP configuration.
> + - resets, reset-names: the power-down and soft-reset lines of PCIe IP.
> +   Associated names must be "powerdown" and "softreset".
> + - phys, phy-names: the phandle for the PHY device.
> +   Associated name must be "pcie"
> +
> +Optional properties:
> + - reset-gpio: a GPIO spec to define which pin is connected to the bus reset.
> +
> +Example:
> +
> +pcie0: pcie at 9b00000 {
> +	compatible = "st,stih407-pcie", "snps,dw-pcie";
> +	device_type = "pci";
> +	reg = <0x09b00000 0x4000>,	/* dbi cntrl registers */
> +	      <0x2fff0000 0x00010000>,	/* configuration space */
> +	      <0x40000000 0x80000000>;	/* lmi mem window */
> +	reg-names = "dbi", "config", "mem-window";
> +	st,syscfg = <&syscfg_core 0xd8 0xe0>;
> +	#address-cells = <3>;
> +	#size-cells = <2>;
> +	ranges = <0x00000800 0 0x2fff0000 0x2fff0000 0 0x00010000   /* configuration space */

Unless you are trying to support some legacy code please remove the configuration space from the ranges.
There is no resource type associated with config space and the generic parser will give you back an
invalid resource type. The other reason for that is that if you really claim to be ECAM compliant by
adding the config space here you need way more than 64K of space.

Best regards,
Liviu

> +		  0x82000000 0 0x20000000 0x20000000 0 0x0FFF0000>; /* non-prefetchable memory */
> +	num-lanes = <1>;
> +	interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
> +	interrupt-names = "msi";
> +	#interrupt-cells = <1>;
> +	interrupt-map-mask = <0 0 0 7>;
> +	interrupt-map = <0 0 0 1 &intc GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>, /* INT A */
> +			<0 0 0 2 &intc GIC_SPI 161 IRQ_TYPE_LEVEL_HIGH>, /* INT B */
> +			<0 0 0 3 &intc GIC_SPI 162 IRQ_TYPE_LEVEL_HIGH>, /* INT C */
> +			<0 0 0 4 &intc GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>; /* INT D */
> +
> +	resets = <&powerdown STIH407_PCIE0_POWERDOWN>,
> +		 <&softreset STIH407_PCIE0_SOFTRESET>;
> +	reset-names = "powerdown",
> +		      "softreset";
> +	phys = <&phy_port0 PHY_TYPE_PCIE>;
> +	phy-names = "pcie";
> +};
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ?\_(?)_/?

[PATCH v2 2/5] PCI: st: Add Device Tree bindings for sti pcie

[Gabriel Fernandez]

Hi Liviu,

You're right, i removed configuration space from the ranges.

Thanks for reviewing.

Gabriel

On 17 March 2015 at 12:42, Liviu Dudau <Liviu.Dudau@arm.com> wrote:
> Hi Gabriel,
>
> On Mon, Mar 16, 2015 at 02:20:32PM +0000, Gabriel FERNANDEZ wrote:
>> sti pcie is built around a Synopsis Designware PCIe IP.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
>> ---
>>  Documentation/devicetree/bindings/pci/st-pcie.txt | 54 +++++++++++++++++++++++
>>  1 file changed, 54 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/pci/st-pcie.txt
>>
>> diff --git a/Documentation/devicetree/bindings/pci/st-pcie.txt b/Documentation/devicetree/bindings/pci/st-pcie.txt
>> new file mode 100644
>> index 0000000..94aae2d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/pci/st-pcie.txt
>> @@ -0,0 +1,54 @@
>> +STMicroelectronics STi PCIe controller
>> +
>> +This PCIe host controller is based on the Synopsis Designware PCIe IP
>> +and thus inherits all the common properties defined in designware-pcie.txt.
>> +
>> +Required properties:
>> + - compatible: "st,stih407-pcie"
>> + - reg: base address and length of the pcie controller, mem-window address
>> +   and length available to the controller.
>> + - interrupts: A list of interrupt outputs of the controller. Must contain an
>> +   entry for each entry in the interrupt-names property.
>> + - interrupt-names: Should be "msi". STi interrupt that is asserted when an
>> +   MSI is received.
>> + - st,syscfg : should be a phandle of the syscfg node. Also contains syscfg
>> +   offset for IP configuration.
>> + - resets, reset-names: the power-down and soft-reset lines of PCIe IP.
>> +   Associated names must be "powerdown" and "softreset".
>> + - phys, phy-names: the phandle for the PHY device.
>> +   Associated name must be "pcie"
>> +
>> +Optional properties:
>> + - reset-gpio: a GPIO spec to define which pin is connected to the bus reset.
>> +
>> +Example:
>> +
>> +pcie0: pcie at 9b00000 {
>> +     compatible = "st,stih407-pcie", "snps,dw-pcie";
>> +     device_type = "pci";
>> +     reg = <0x09b00000 0x4000>,      /* dbi cntrl registers */
>> +           <0x2fff0000 0x00010000>,  /* configuration space */
>> +           <0x40000000 0x80000000>;  /* lmi mem window */
>> +     reg-names = "dbi", "config", "mem-window";
>> +     st,syscfg = <&syscfg_core 0xd8 0xe0>;
>> +     #address-cells = <3>;
>> +     #size-cells = <2>;
>> +     ranges = <0x00000800 0 0x2fff0000 0x2fff0000 0 0x00010000   /* configuration space */
>
> Unless you are trying to support some legacy code please remove the configuration space from the ranges.
> There is no resource type associated with config space and the generic parser will give you back an
> invalid resource type. The other reason for that is that if you really claim to be ECAM compliant by
> adding the config space here you need way more than 64K of space.
>
> Best regards,
> Liviu
>
>> +               0x82000000 0 0x20000000 0x20000000 0 0x0FFF0000>; /* non-prefetchable memory */
>> +     num-lanes = <1>;
>> +     interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
>> +     interrupt-names = "msi";
>> +     #interrupt-cells = <1>;
>> +     interrupt-map-mask = <0 0 0 7>;
>> +     interrupt-map = <0 0 0 1 &intc GIC_SPI 160 IRQ_TYPE_LEVEL_HIGH>, /* INT A */
>> +                     <0 0 0 2 &intc GIC_SPI 161 IRQ_TYPE_LEVEL_HIGH>, /* INT B */
>> +                     <0 0 0 3 &intc GIC_SPI 162 IRQ_TYPE_LEVEL_HIGH>, /* INT C */
>> +                     <0 0 0 4 &intc GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>; /* INT D */
>> +
>> +     resets = <&powerdown STIH407_PCIE0_POWERDOWN>,
>> +              <&softreset STIH407_PCIE0_SOFTRESET>;
>> +     reset-names = "powerdown",
>> +                   "softreset";
>> +     phys = <&phy_port0 PHY_TYPE_PCIE>;
>> +     phy-names = "pcie";
>> +};
>> --
>> 1.9.1
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
> --
> ====================
> | I would like to |
> | fix the world,  |
> | but they're not |
> | giving me the   |
>  \ source code!  /
>   ---------------
>     ?\_(?)_/?
>

[PATCH v2 3/5] PCI: st: Provide support for the sti PCIe controller

[Gabriel FERNANDEZ]

sti pcie is built around a Synopsis Designware PCIe IP.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
---
 drivers/pci/host/Kconfig  |   9 +
 drivers/pci/host/Makefile |   1 +
 drivers/pci/host/pci-st.c | 617 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 627 insertions(+)
 create mode 100644 drivers/pci/host/pci-st.c

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 7b892a9..af9f9212 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -106,4 +106,13 @@ config PCI_VERSATILE
 	bool "ARM Versatile PB PCI controller"
 	depends on ARCH_VERSATILE
 
+config PCI_ST
+	bool "ST PCIe controller"
+	depends on ARCH_STI || (ARM && COMPILE_TEST)
+	select PCIE_DW
+	help
+	  Enable PCIe controller support on ST Socs. This controller is based
+	  on Designware hardware and therefore the driver re-uses the
+	  Designware core functions to implement the driver.
+
 endmenu
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index e61d91c..97c6622 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -13,3 +13,4 @@ obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
 obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
 obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
 obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
+obj-$(CONFIG_PCI_ST) += pci-st.o
diff --git a/drivers/pci/host/pci-st.c b/drivers/pci/host/pci-st.c
new file mode 100644
index 0000000..470000d
--- /dev/null
+++ b/drivers/pci/host/pci-st.c
@@ -0,0 +1,617 @@
+/*
+ * Copyright (C) 2014 STMicroelectronics
+ *
+ * STMicroelectronics PCI express Driver for sti SoCs.
+ * ST PCIe IPs are built around a Synopsys IP Core.
+ *
+ * Author: Fabrice Gasnier <fabrice.gasnier@st.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/delay.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_gpio.h>
+#include <linux/of_pci.h>
+#include <linux/of_platform.h>
+#include <linux/phy/phy.h>
+#include <linux/regmap.h>
+#include <linux/reset.h>
+
+#include "pcie-designware.h"
+
+#define TRANSLATION_CONTROL		0x900
+/* Controls if area is inclusive or exclusive */
+#define RC_PASS_ADDR_RANGE		BIT(1)
+
+/* Base of area reserved for config accesses. Fixed size of 64K. */
+#define CFG_BASE_ADDRESS		0x92c
+#define CFG_REGION_SIZE			65536
+#define CFG_SPACE1_OFFSET		0x1000
+
+/* First 4K of config space has this BDF (bus,device,function) */
+#define FUNC0_BDF_NUM			0x930
+
+/* Mem regions */
+#define IN0_MEM_ADDR_START		0x964
+#define IN0_MEM_ADDR_LIMIT		0x968
+#define IN1_MEM_ADDR_START		0x974
+#define IN1_MEM_ADDR_LIMIT		0x978
+
+/* This actually contains the LTSSM state machine state */
+#define PORT_LOGIC_DEBUG_REG_0		0x728
+
+/* LTSSM state machine values	*/
+#define DEBUG_REG_0_LTSSM_MASK		0x1f
+#define S_DETECT_QUIET			0x00
+#define S_DETECT_ACT			0x01
+#define S_POLL_ACTIVE			0x02
+#define S_POLL_COMPLIANCE		0x03
+#define S_POLL_CONFIG			0x04
+#define S_PRE_DETECT_QUIET		0x05
+#define S_DETECT_WAIT			0x06
+#define S_CFG_LINKWD_START		0x07
+#define S_CFG_LINKWD_ACEPT		0x08
+#define S_CFG_LANENUM_WAIT		0x09
+#define S_CFG_LANENUM_ACEPT		0x0A
+#define S_CFG_COMPLETE			0x0B
+#define S_CFG_IDLE			0x0C
+#define S_RCVRY_LOCK			0x0D
+#define S_RCVRY_SPEED			0x0E
+#define S_RCVRY_RCVRCFG			0x0F
+#define S_RCVRY_IDLE			0x10
+#define S_L0				0x11
+#define S_L0S				0x12
+#define S_L123_SEND_EIDLE		0x13
+#define S_L1_IDLE			0x14
+#define S_L2_IDLE			0x15
+#define S_L2_WAKE			0x16
+#define S_DISABLED_ENTRY		0x17
+#define S_DISABLED_IDLE			0x18
+#define S_DISABLED			0x19
+#define S_LPBK_ENTRY			0x1A
+#define S_LPBK_ACTIVE			0x1B
+#define S_LPBK_EXIT			0x1C
+#define S_LPBK_EXIT_TIMEOUT		0x1D
+#define S_HOT_RESET_ENTRY		0x1E
+#define S_HOT_RESET			0x1F
+
+/* syscfg bits */
+#define PCIE_SYS_INT			BIT(5)
+#define PCIE_APP_REQ_RETRY_EN		BIT(3)
+#define PCIE_APP_LTSSM_ENABLE		BIT(2)
+#define PCIE_APP_INIT_RST		BIT(1)
+#define PCIE_DEVICE_TYPE		BIT(0)
+#define PCIE_DEFAULT_VAL		PCIE_DEVICE_TYPE
+
+/* Time to wait between testing the link in msecs (hardware poll interval) */
+#define LINK_LOOP_DELAY_MS 1
+/* Total amount of time to wait for the link to come up in msecs */
+#define LINK_WAIT_MS 120
+#define LINK_LOOP_COUNT (LINK_WAIT_MS / LINK_LOOP_DELAY_MS)
+
+/* st,syscfg offsets */
+#define SYSCFG0_REG	1
+#define SYSCFG1_REG	2
+
+#define to_st_pcie(x)	container_of(x, struct st_pcie, pp)
+
+/**
+ * struct st_pcie - private data of the controller
+ * @pp: designware pcie port
+ * @syscfg0: PCIe configuration 0 register, regmap offset
+ * @syscfg1: PCIe configuration 1 register, regmap offset
+ * @phy: associated pcie phy
+ * @lmi: memory made available to the controller
+ * @regmap: Syscfg registers bank in which PCIe port is configured
+ * @pwr: power control
+ * @rst: reset control
+ * @reset_gpio: optional reset gpio
+ * @config_window_start: start address of 64K config space area
+ */
+struct st_pcie {
+	struct pcie_port pp;
+	int syscfg0;
+	int syscfg1;
+	struct phy *phy;
+	struct resource	*lmi;
+	struct regmap *regmap;
+	struct reset_control *pwr;
+	struct reset_control *rst;
+	int reset_gpio;
+	phys_addr_t config_window_start;
+};
+
+/*
+ * Function to test if the link is in an operational state or not. We must
+ * ensure the link is operational before we try to do a configuration access.
+ */
+static int st_pcie_link_up(struct pcie_port *pp)
+{
+	u32 status;
+	int link_up;
+	int count = 0;
+
+	/*
+	 * We have to be careful here. This is used in config read/write,
+	 * The higher levels switch off interrupts, so you cannot use
+	 * jiffies to do a timeout, or reschedule
+	 */
+	do {
+		/*
+		 * What about L2? I think software intervention is
+		 * required to get it out of L2, so in effect the link
+		 * is down. Requires more work when/if we implement power
+		 * management
+		 */
+		status = readl_relaxed(pp->dbi_base + PORT_LOGIC_DEBUG_REG_0);
+		status &= DEBUG_REG_0_LTSSM_MASK;
+
+		link_up = (status == S_L0) || (status == S_L0S) ||
+			  (status == S_L1_IDLE);
+
+		/*
+		 * It can take some considerable time for the link to actually
+		 * come up, caused by the PLLs. Experiments indicate it takes
+		 * about 8ms to actually bring the link up, but this can vary
+		 * considerably according to the specification. This code should
+		 * allow sufficient time
+		 */
+		if (!link_up)
+			mdelay(LINK_LOOP_DELAY_MS);
+
+	} while (!link_up && ++count < LINK_LOOP_COUNT);
+
+	return link_up;
+}
+
+/*
+ * On ARM platforms, we actually get a bus error returned when the PCIe IP
+ * returns a UR or CRS instead of an OK.
+ */
+static int st_pcie_abort_handler(unsigned long addr, unsigned int fsr,
+				 struct pt_regs *regs)
+{
+	return 0;
+}
+
+/*
+ * The PCI express core IP expects the following arrangement on it's address
+ * bus (slv_haddr) when driving config cycles.
+ * bus_number		[31:24]
+ * dev_number		[23:19]
+ * func_number		[18:16]
+ * unused		[15:12]
+ * ext_reg_number	[11:8]
+ * reg_number		[7:2]
+ *
+ * Bits [15:12] are unused.
+ *
+ * In the glue logic there is a 64K region of address space that can be
+ * written/read to generate config cycles. The base address of this is
+ * controlled by CFG_BASE_ADDRESS. There are 8 16 bit registers called
+ * FUNC0_BDF_NUM to FUNC8_BDF_NUM. These split the bottom half of the 64K
+ * window into 8 regions@4K boundaries. These control the bus,device and
+ * function number you are trying to talk to.
+ *
+ * The decision on whether to generate a type 0 or type 1 access is controlled
+ * by bits 15:12 of the address you write to.  If they are zero, then a type 0
+ * is generated, if anything else it will be a type 1. Thus the bottom 4K
+ * region controlled by FUNC0_BDF_NUM can only generate type 0, all the others
+ * can only generate type 1.
+ *
+ * We only use FUNC0_BDF_NUM and FUNC1_BDF_NUM. Which one you use is selected
+ * by bit 12 of the address you write to. The selected register is then used
+ * for the top 16 bits of the slv_haddr to form the bus/dev/func, bit 15:12 are
+ * wired to zero, and bits 11:2 form the address of the register you want to
+ * read in config space.
+ *
+ * We always write FUNC0_BDF_NUM as a 32 bit write. So if we want type 1
+ * accesses we have to shift by 16 so in effect we are writing to FUNC1_BDF_NUM
+ */
+static inline u32 bdf_num(int bus, int devfn, int is_root_bus)
+{
+	return ((bus << 8) | devfn) << (is_root_bus ? 0 : 16);
+}
+
+static int st_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
+				 unsigned int devfn, int where, int size,
+				 u32 *val)
+{
+	int ret;
+	u32 bdf, addr;
+
+	addr = where & ~0x3;
+	bdf = bdf_num(bus->number, devfn, pci_is_root_bus(bus));
+
+	/* Set the config packet devfn */
+	writel_relaxed(bdf, pp->dbi_base + FUNC0_BDF_NUM);
+	readl_relaxed(pp->dbi_base + FUNC0_BDF_NUM);
+
+	if (bus->parent->number == pp->root_bus_nr)
+		ret = dw_pcie_cfg_read(pp->va_cfg0_base + addr, where, size,
+				       val);
+	else
+		ret = dw_pcie_cfg_read(pp->va_cfg1_base + addr, where, size,
+				       val);
+	return ret;
+}
+
+static int st_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
+				 unsigned int devfn, int where, int size,
+				 u32 val)
+{
+	int ret;
+	u32 bdf, addr;
+
+	addr = where & ~0x3;
+	bdf = bdf_num(bus->number, devfn, pci_is_root_bus(bus));
+
+	/* Set the config packet devfn */
+	writel_relaxed(bdf, pp->dbi_base + FUNC0_BDF_NUM);
+	readl_relaxed(pp->dbi_base + FUNC0_BDF_NUM);
+
+	if (bus->parent->number == pp->root_bus_nr)
+		ret = dw_pcie_cfg_write(pp->va_cfg0_base + addr, where, size,
+					val);
+	else
+		ret = dw_pcie_cfg_write(pp->va_cfg1_base + addr, where, size,
+					val);
+	return ret;
+}
+
+static void st_pcie_board_reset(struct pcie_port *pp)
+{
+	struct st_pcie *pcie = to_st_pcie(pp);
+
+	if (!gpio_is_valid(pcie->reset_gpio))
+		return;
+
+	if (gpio_direction_output(pcie->reset_gpio, 0)) {
+		dev_err(pp->dev, "Cannot set PERST# (gpio %u) to output\n",
+			pcie->reset_gpio);
+		return;
+	}
+
+	/* From PCIe spec */
+	msleep(2);
+	gpio_direction_output(pcie->reset_gpio, 1);
+
+	/*
+	 * PCIe specification states that you should not issue any config
+	 * requests until 100ms after asserting reset, so we enforce that here
+	 */
+	msleep(100);
+}
+
+static void st_pcie_hw_setup(struct pcie_port *pp)
+{
+	struct st_pcie *pcie = to_st_pcie(pp);
+
+	dw_pcie_setup_rc(pp);
+
+	/* Set up the config window to the top of the PCI address space */
+	writel_relaxed(pcie->config_window_start,
+		       pp->dbi_base + CFG_BASE_ADDRESS);
+
+	/*
+	 * Open up memory to the PCI controller. We could do slightly
+	 * better than this and exclude the kernel text segment and bss etc.
+	 * They are base/limit registers so can be of arbitrary alignment
+	 * presumably
+	 */
+	writel_relaxed(pcie->lmi->start, pp->dbi_base + IN0_MEM_ADDR_START);
+	writel_relaxed(pcie->lmi->end, pp->dbi_base + IN0_MEM_ADDR_LIMIT);
+
+	/* Disable the 2nd region */
+	writel_relaxed(~0, pp->dbi_base + IN1_MEM_ADDR_START);
+	writel_relaxed(0, pp->dbi_base + IN1_MEM_ADDR_LIMIT);
+
+	writel_relaxed(RC_PASS_ADDR_RANGE, pp->dbi_base + TRANSLATION_CONTROL);
+
+	/* Now assert the board level reset to the other PCIe device */
+	st_pcie_board_reset(pp);
+}
+
+static irqreturn_t st_pcie_msi_irq_handler(int irq, void *arg)
+{
+	struct pcie_port *pp = arg;
+
+	return dw_handle_msi_irq(pp);
+}
+
+static int st_pcie_init(struct pcie_port *pp)
+{
+	struct st_pcie *pcie = to_st_pcie(pp);
+	int ret;
+
+	ret = reset_control_deassert(pcie->pwr);
+	if (ret) {
+		dev_err(pp->dev, "unable to bring out of powerdown\n");
+		return ret;
+	}
+
+	ret = reset_control_deassert(pcie->rst);
+	if (ret) {
+		dev_err(pp->dev, "unable to bring out of softreset\n");
+		return ret;
+	}
+
+	/* Set device type : Root Complex */
+	ret = regmap_write(pcie->regmap, pcie->syscfg0, PCIE_DEVICE_TYPE);
+	if (ret < 0) {
+		dev_err(pp->dev, "unable to set device type\n");
+		return ret;
+	}
+
+	usleep_range(1000, 2000);
+	return ret;
+}
+
+static int st_pcie_enable_ltssm(struct pcie_port *pp)
+{
+	struct st_pcie *pcie = to_st_pcie(pp);
+
+	return regmap_update_bits(pcie->regmap, pcie->syscfg1,
+				  PCIE_APP_LTSSM_ENABLE, PCIE_APP_LTSSM_ENABLE);
+}
+
+static int st_pcie_disable_ltssm(struct pcie_port *pp)
+{
+	struct st_pcie *pcie = to_st_pcie(pp);
+
+	return regmap_update_bits(pcie->regmap, pcie->syscfg1,
+				  PCIE_APP_LTSSM_ENABLE, 0);
+}
+
+static void st_pcie_host_init(struct pcie_port *pp)
+{
+	struct st_pcie *pcie = to_st_pcie(pp);
+	int err;
+
+	/*
+	 * We have to initialise the PCIe cell on some hardware before we can
+	 * talk to the phy
+	 */
+	err = st_pcie_init(pp);
+	if (err)
+		return;
+
+	err = st_pcie_disable_ltssm(pp);
+	if (err) {
+		dev_err(pp->dev, "disable ltssm failed, %d\n", err);
+		return;
+	}
+
+	/* Now init the associated miphy */
+	err = phy_init(pcie->phy);
+	if (err < 0) {
+		dev_err(pp->dev, "Cannot init PHY: %d\n", err);
+		return;
+	}
+
+	/* Now do all the register poking */
+	st_pcie_hw_setup(pp);
+
+	/* Re-enable the link */
+	err = st_pcie_enable_ltssm(pp);
+	if (err)
+		dev_err(pp->dev, "enable ltssm failed, %d\n", err);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		dw_pcie_msi_init(pp);
+}
+
+static struct pcie_host_ops st_pcie_host_ops = {
+	.rd_other_conf = st_pcie_rd_other_conf,
+	.wr_other_conf = st_pcie_wr_other_conf,
+	.link_up = st_pcie_link_up,
+	.host_init = st_pcie_host_init,
+};
+
+static const struct of_device_id st_pcie_of_match[] = {
+	{ .compatible = "st,pcie", },
+	{ },
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int st_pcie_suspend(struct device *pcie_dev)
+{
+	struct st_pcie *pcie = dev_get_drvdata(pcie_dev);
+
+	/* To guarantee a real phy initialization on resume */
+	phy_exit(pcie->phy);
+
+	return 0;
+}
+
+static int st_pcie_resume(struct device *pcie_dev)
+{
+	struct st_pcie *pcie = dev_get_drvdata(pcie_dev);
+
+	st_pcie_host_init(&pcie->pp);
+
+	return 0;
+}
+
+static const struct dev_pm_ops st_pcie_pm_ops = {
+	.suspend_noirq = st_pcie_suspend,
+	.resume_noirq = st_pcie_resume,
+};
+
+#define ST_PCIE_PM_OPS (&st_pcie_pm_ops)
+
+#else
+
+#define ST_PCIE_PM_OPS NULL
+
+#endif
+
+static int st_pcie_probe(struct platform_device *pdev)
+{
+	struct resource *res;
+	struct st_pcie *pcie;
+	struct device_node *np = pdev->dev.of_node;
+	struct pcie_port *pp;
+	int ret;
+
+	pcie = devm_kzalloc(&pdev->dev, sizeof(*pcie), GFP_KERNEL);
+	if (!pcie)
+		return -ENOMEM;
+
+	memset(pcie, 0, sizeof(*pcie));
+
+	pp = &pcie->pp;
+	pp->dev = &pdev->dev;
+
+	/* mem regions */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dbi");
+	pp->dbi_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pp->dbi_base))
+		return PTR_ERR(pp->dbi_base);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
+	if (!res)
+		return -ENXIO;
+
+	/* Check that this has sensible values */
+	if ((resource_size(res) != CFG_REGION_SIZE) ||
+	    (res->start & (CFG_REGION_SIZE - 1))) {
+		dev_err(&pdev->dev, "Invalid config space properties\n");
+		return -EINVAL;
+	}
+	pcie->config_window_start = res->start;
+
+	pp->va_cfg0_base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(pp->va_cfg0_base))
+		return PTR_ERR(pp->va_cfg0_base);
+	pp->va_cfg1_base = pp->va_cfg0_base + CFG_SPACE1_OFFSET;
+
+	pcie->lmi = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						 "mem-window");
+	if (!pcie->lmi)
+		return -ENXIO;
+
+	/* regmap registers for PCIe IP configuration */
+	pcie->regmap = syscon_regmap_lookup_by_phandle(np, "st,syscfg");
+	if (IS_ERR(pcie->regmap)) {
+		dev_err(&pdev->dev, "No syscfg phandle specified\n");
+		return PTR_ERR(pcie->regmap);
+	}
+
+	ret = of_property_read_u32_index(np, "st,syscfg", SYSCFG0_REG,
+					 &pcie->syscfg0);
+	if (ret) {
+		dev_err(&pdev->dev, "can't get syscfg0 offset (%d)\n", ret);
+		return ret;
+	}
+
+	ret = of_property_read_u32_index(np, "st,syscfg", SYSCFG1_REG,
+					 &pcie->syscfg1);
+	if (ret) {
+		dev_err(&pdev->dev, "can't get syscfg1 offset (%d)\n", ret);
+		return ret;
+	}
+
+	/* powerdown / resets */
+	pcie->pwr = devm_reset_control_get(&pdev->dev, "powerdown");
+	if (IS_ERR(pcie->pwr)) {
+		dev_err(&pdev->dev, "powerdown reset control not defined\n");
+		return PTR_ERR(pcie->pwr);
+	}
+
+	pcie->rst = devm_reset_control_get(&pdev->dev, "softreset");
+	if (IS_ERR(pcie->rst)) {
+		dev_err(&pdev->dev, "Soft reset control not defined\n");
+		return PTR_ERR(pcie->pwr);
+	}
+
+	/* phy */
+	pcie->phy = devm_phy_get(&pdev->dev, "pcie");
+	if (IS_ERR(pcie->phy)) {
+		dev_err(&pdev->dev, "no PHY configured\n");
+		return PTR_ERR(pcie->phy);
+	}
+
+	/* Claim the GPIO for PRST# if available */
+	pcie->reset_gpio = of_get_named_gpio(np, "reset-gpio", 0);
+	if (!gpio_is_valid(pcie->reset_gpio))
+		dev_dbg(&pdev->dev, "No reset-gpio configured\n");
+	else {
+		ret = devm_gpio_request(&pdev->dev,
+					pcie->reset_gpio,
+					"PCIe reset");
+		if (ret) {
+			dev_err(&pdev->dev, "Cannot request reset-gpio %d\n",
+				pcie->reset_gpio);
+			return ret;
+		}
+	}
+
+	if (IS_ENABLED(CONFIG_PCI_MSI)) {
+		pp->msi_irq = platform_get_irq_byname(pdev, "msi");
+		if (pp->msi_irq <= 0) {
+			dev_err(&pdev->dev, "failed to get MSI irq\n");
+			return -ENODEV;
+		}
+
+		ret = devm_request_irq(&pdev->dev, pp->msi_irq,
+				       st_pcie_msi_irq_handler,
+				       IRQF_SHARED, "st-pcie-msi", pp);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to request MSI irq\n");
+			return -ENODEV;
+		}
+	}
+
+	if (IS_ENABLED(CONFIG_ARM)) {
+		/*
+		 * We have to hook the abort handler so that we can intercept
+		 * bus errors when doing config read/write that return UR,
+		 * which is flagged up as a bus error
+		 */
+		hook_fault_code(16+6, st_pcie_abort_handler, SIGBUS, 0,
+				"imprecise external abort");
+	}
+
+	pp->root_bus_nr = -1;
+	pp->ops = &st_pcie_host_ops;
+
+	ret = dw_pcie_host_init(pp);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize host\n");
+		return ret;
+	}
+
+	platform_set_drvdata(pdev, pcie);
+
+	dev_info(&pdev->dev, "Initialized\n");
+	return 0;
+}
+
+MODULE_DEVICE_TABLE(of, st_pcie_of_match);
+
+static struct platform_driver st_pcie_driver = {
+	.driver = {
+		.name = "st-pcie",
+		.of_match_table = st_pcie_of_match,
+		.pm = ST_PCIE_PM_OPS,
+	},
+};
+
+/* ST PCIe driver does not allow module unload */
+static int __init pcie_init(void)
+{
+	return platform_driver_probe(&st_pcie_driver, st_pcie_probe);
+}
+device_initcall(pcie_init);
+
+MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
+MODULE_DESCRIPTION("PCI express Driver for ST SoCs");
+MODULE_LICENSE("GPL v2");
-- 
1.9.1

[PATCH v2 3/5] PCI: st: Provide support for the sti PCIe controller

[Paul Bolle]

On Mon, 2015-03-16 at 15:20 +0100, Gabriel FERNANDEZ wrote:
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
 
> +config PCI_ST
> +	bool "ST PCIe controller"

You add a bool Kconfig symbol. A week or two ago I saw some patches fly
by that - I think - allowed PCIe controllers to be built modular.

> +	depends on ARCH_STI || (ARM && COMPILE_TEST)
> +	select PCIE_DW
> +	help
> +	  Enable PCIe controller support on ST Socs. This controller is based
> +	  on Designware hardware and therefore the driver re-uses the
> +	  Designware core functions to implement the driver.

> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile

> +obj-$(CONFIG_PCI_ST) += pci-st.o

If you keep that symbol bool this objectfile will never be part of a
module.

> diff --git a/drivers/pci/host/pci-st.c b/drivers/pci/host/pci-st.c
> new file mode 100644
> index 0000000..470000d
> --- /dev/null
> +++ b/drivers/pci/host/pci-st.c

> +#include <linux/module.h>

For built-in code this include is, probably, not needed.

> +MODULE_DEVICE_TABLE(of, st_pcie_of_match);

For built-in code that macro will always be preprocessed away.

> +/* ST PCIe driver does not allow module unload */
> +static int __init pcie_init(void)
> +{
> +	return platform_driver_probe(&st_pcie_driver, st_pcie_probe);
> +}
> +device_initcall(pcie_init);

I think the module unload comment is a bit odd for built-in only code.

> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
> +MODULE_DESCRIPTION("PCI express Driver for ST SoCs");
> +MODULE_LICENSE("GPL v2");

These three macros will be, basically, always preprocessed away as long
as this code can't be built to be modular.


Paul Bolle

[PATCH v2 3/5] PCI: st: Provide support for the sti PCIe controller

[Gabriel Fernandez]

Thanks Paul for reviewing.

I'll check to be modular.

BR

Gabriel

On 16 March 2015 at 16:11, Paul Bolle <pebolle@tiscali.nl> wrote:
> On Mon, 2015-03-16 at 15:20 +0100, Gabriel FERNANDEZ wrote:
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>
>> +config PCI_ST
>> +     bool "ST PCIe controller"
>
> You add a bool Kconfig symbol. A week or two ago I saw some patches fly
> by that - I think - allowed PCIe controllers to be built modular.
>
>> +     depends on ARCH_STI || (ARM && COMPILE_TEST)
>> +     select PCIE_DW
>> +     help
>> +       Enable PCIe controller support on ST Socs. This controller is based
>> +       on Designware hardware and therefore the driver re-uses the
>> +       Designware core functions to implement the driver.
>
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>
>> +obj-$(CONFIG_PCI_ST) += pci-st.o
>
> If you keep that symbol bool this objectfile will never be part of a
> module.
>
>> diff --git a/drivers/pci/host/pci-st.c b/drivers/pci/host/pci-st.c
>> new file mode 100644
>> index 0000000..470000d
>> --- /dev/null
>> +++ b/drivers/pci/host/pci-st.c
>
>> +#include <linux/module.h>
>
> For built-in code this include is, probably, not needed.
>
>> +MODULE_DEVICE_TABLE(of, st_pcie_of_match);
>
> For built-in code that macro will always be preprocessed away.
>
>> +/* ST PCIe driver does not allow module unload */
>> +static int __init pcie_init(void)
>> +{
>> +     return platform_driver_probe(&st_pcie_driver, st_pcie_probe);
>> +}
>> +device_initcall(pcie_init);
>
> I think the module unload comment is a bit odd for built-in only code.
>
>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
>> +MODULE_DESCRIPTION("PCI express Driver for ST SoCs");
>> +MODULE_LICENSE("GPL v2");
>
> These three macros will be, basically, always preprocessed away as long
> as this code can't be built to be modular.
>
>
> Paul Bolle
>

[PATCH v2 3/5] PCI: st: Provide support for the sti PCIe controller

[Fabrice Gasnier]

Hi Paul,

On 03/16/2015 04:11 PM, Paul Bolle wrote:
>> +config PCI_ST
>> +	bool "ST PCIe controller"
> You add a bool Kconfig symbol. A week or two ago I saw some patches fly
> by that - I think - allowed PCIe controllers to be built modular.

Thanks for your review.

Are you talking about "PCI: Export symbols of PCI functions" patch, that 
is part of a series
named "pci: iproc: Add Broadcom iProc PCIe support" ?

This controller doesn't look like to be based on pcie-designware core 
driver.
Other vendors that are using "pcie-designware" core driver are also make 
it bool.
The current core driver doesn't support module loading/unloading as I 
see it.
If this is required, I also think this should be part of another patchset.

What do you think ?

Please advise,
BR,
Fabrice
>
>> +	depends on ARCH_STI || (ARM && COMPILE_TEST)
>> +	select PCIE_DW
>> +	help
>> +	  Enable PCIe controller support on ST Socs. This controller is based
>> +	  on Designware hardware and therefore the driver re-uses the
>> +	  Designware core functions to implement the driver.
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> +obj-$(CONFIG_PCI_ST) += pci-st.o
> If you keep that symbol bool this objectfile will never be part of a
> module.
>
>> diff --git a/drivers/pci/host/pci-st.c b/drivers/pci/host/pci-st.c
>> new file mode 100644
>> index 0000000..470000d
>> --- /dev/null
>> +++ b/drivers/pci/host/pci-st.c
>> +#include <linux/module.h>
> For built-in code this include is, probably, not needed.
>
>> +MODULE_DEVICE_TABLE(of, st_pcie_of_match);
> For built-in code that macro will always be preprocessed away.
>
>> +/* ST PCIe driver does not allow module unload */
>> +static int __init pcie_init(void)
>> +{
>> +	return platform_driver_probe(&st_pcie_driver, st_pcie_probe);
>> +}
>> +device_initcall(pcie_init);
> I think the module unload comment is a bit odd for built-in only code.
>
>> +MODULE_AUTHOR("Fabrice Gasnier <fabrice.gasnier@st.com>");
>> +MODULE_DESCRIPTION("PCI express Driver for ST SoCs");
>> +MODULE_LICENSE("GPL v2");
> These three macros will be, basically, always preprocessed away as long
> as this code can't be built to be modular.
>
>
> Paul Bolle
>

[PATCH v2 3/5] PCI: st: Provide support for the sti PCIe controller

[Paul Bolle]

Hi Fabrice,

Fabrice Gasnier schreef op wo 18-03-2015 om 09:49 [+0100]:
> On 03/16/2015 04:11 PM, Paul Bolle wrote:
> >> +config PCI_ST
> >> +	bool "ST PCIe controller"
> > You add a bool Kconfig symbol. A week or two ago I saw some patches fly
> > by that - I think - allowed PCIe controllers to be built modular.
> 
> Thanks for your review.
> 
> Are you talking about "PCI: Export symbols of PCI functions" patch, that 
> is part of a series
> named "pci: iproc: Add Broadcom iProc PCIe support" ?

Yes, that is the series I was thinking about. (I made you search lkml,
and that was a bit rude. But you found the patch anyhow.)

> This controller doesn't look like to be based on pcie-designware core 
> driver.
> Other vendors that are using "pcie-designware" core driver are also make 
> it bool.
> The current core driver doesn't support module loading/unloading as I 
> see it.
> If this is required, I also think this should be part of another patchset.
> 
> What do you think ?

I wouldn't know whether your driver might work as a loadable module, but
other people reading this surely will. But if it can't work as a module
you should drop all the module related macros etc. I spotted. Because
then they serve no purpose.


Paul Bolle

[PATCH v2 3/5] PCI: st: Provide support for the sti PCIe controller

[Gabriel Fernandez]

Hi Bjorn,

pci-st.c driver could be modular with modification of pcie-designware
core driver. But as Fabrice said  it should be another patchset.

What do you prefer ?

drop all the module related macros as mentioned by Paul ?
or
keep macros like other vendors do ?

Thanks

Gabriel

On 18 March 2015 at 11:35, Paul Bolle <pebolle@tiscali.nl> wrote:
> Hi Fabrice,
>
> Fabrice Gasnier schreef op wo 18-03-2015 om 09:49 [+0100]:
>> On 03/16/2015 04:11 PM, Paul Bolle wrote:
>> >> +config PCI_ST
>> >> +  bool "ST PCIe controller"
>> > You add a bool Kconfig symbol. A week or two ago I saw some patches fly
>> > by that - I think - allowed PCIe controllers to be built modular.
>>
>> Thanks for your review.
>>
>> Are you talking about "PCI: Export symbols of PCI functions" patch, that
>> is part of a series
>> named "pci: iproc: Add Broadcom iProc PCIe support" ?
>
> Yes, that is the series I was thinking about. (I made you search lkml,
> and that was a bit rude. But you found the patch anyhow.)
>
>> This controller doesn't look like to be based on pcie-designware core
>> driver.
>> Other vendors that are using "pcie-designware" core driver are also make
>> it bool.
>> The current core driver doesn't support module loading/unloading as I
>> see it.
>> If this is required, I also think this should be part of another patchset.
>>
>> What do you think ?
>
> I wouldn't know whether your driver might work as a loadable module, but
> other people reading this surely will. But if it can't work as a module
> you should drop all the module related macros etc. I spotted. Because
> then they serve no purpose.
>
>
> Paul Bolle
>

[PATCH v2 3/5] PCI: st: Provide support for the sti PCIe controller

[Bjorn Helgaas]

On Tue, Mar 31, 2015 at 11:11:44AM +0200, Gabriel Fernandez wrote:
> Hi Bjorn,
> 
> pci-st.c driver could be modular with modification of pcie-designware
> core driver. But as Fabrice said  it should be another patchset.
> 
> What do you prefer ?
> 
> drop all the module related macros as mentioned by Paul ?
> or
> keep macros like other vendors do ?

I don't think this is a big deal either way.  You made it consistent with
the style of the existing PCI host drivers, which is exactly what you
should do.

If somebody wants to do the work of making them all modular, or leaving
them non-modular and removing all the MODULE_*() annotations, that can be
done later.

> On 18 March 2015 at 11:35, Paul Bolle <pebolle@tiscali.nl> wrote:
> > Hi Fabrice,
> >
> > Fabrice Gasnier schreef op wo 18-03-2015 om 09:49 [+0100]:
> >> On 03/16/2015 04:11 PM, Paul Bolle wrote:
> >> >> +config PCI_ST
> >> >> +  bool "ST PCIe controller"
> >> > You add a bool Kconfig symbol. A week or two ago I saw some patches fly
> >> > by that - I think - allowed PCIe controllers to be built modular.
> >>
> >> Thanks for your review.
> >>
> >> Are you talking about "PCI: Export symbols of PCI functions" patch, that
> >> is part of a series
> >> named "pci: iproc: Add Broadcom iProc PCIe support" ?
> >
> > Yes, that is the series I was thinking about. (I made you search lkml,
> > and that was a bit rude. But you found the patch anyhow.)
> >
> >> This controller doesn't look like to be based on pcie-designware core
> >> driver.
> >> Other vendors that are using "pcie-designware" core driver are also make
> >> it bool.
> >> The current core driver doesn't support module loading/unloading as I
> >> see it.
> >> If this is required, I also think this should be part of another patchset.
> >>
> >> What do you think ?
> >
> > I wouldn't know whether your driver might work as a loadable module, but
> > other people reading this surely will. But if it can't work as a module
> > you should drop all the module related macros etc. I spotted. Because
> > then they serve no purpose.
> >
> >
> > Paul Bolle
> >

[PATCH v2 3/5] PCI: st: Provide support for the sti PCIe controller

[Kishon Vijay Abraham I]

Hi,

On Monday 16 March 2015 07:50 PM, Gabriel FERNANDEZ wrote:
> sti pcie is built around a Synopsis Designware PCIe IP.
>
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
> ---
>   drivers/pci/host/Kconfig  |   9 +
>   drivers/pci/host/Makefile |   1 +
>   drivers/pci/host/pci-st.c | 617 ++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 627 insertions(+)
>   create mode 100644 drivers/pci/host/pci-st.c
>
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 7b892a9..af9f9212 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -106,4 +106,13 @@ config PCI_VERSATILE
>   	bool "ARM Versatile PB PCI controller"
>   	depends on ARCH_VERSATILE
>
> +config PCI_ST
> +	bool "ST PCIe controller"
> +	depends on ARCH_STI || (ARM && COMPILE_TEST)
> +	select PCIE_DW
> +	help
> +	  Enable PCIe controller support on ST Socs. This controller is based
> +	  on Designware hardware and therefore the driver re-uses the
> +	  Designware core functions to implement the driver.
> +
>   endmenu
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index e61d91c..97c6622 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -13,3 +13,4 @@ obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
>   obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
>   obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>   obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
> +obj-$(CONFIG_PCI_ST) += pci-st.o
> diff --git a/drivers/pci/host/pci-st.c b/drivers/pci/host/pci-st.c
> new file mode 100644
> index 0000000..470000d
> --- /dev/null
> +++ b/drivers/pci/host/pci-st.c
> @@ -0,0 +1,617 @@
> +/*
> + * Copyright (C) 2014 STMicroelectronics
> + *
> + * STMicroelectronics PCI express Driver for sti SoCs.
> + * ST PCIe IPs are built around a Synopsys IP Core.
> + *
> + * Author: Fabrice Gasnier <fabrice.gasnier@st.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/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/syscon.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_gpio.h>
> +#include <linux/of_pci.h>
> +#include <linux/of_platform.h>
> +#include <linux/phy/phy.h>
> +#include <linux/regmap.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +
> +#define TRANSLATION_CONTROL		0x900
> +/* Controls if area is inclusive or exclusive */
> +#define RC_PASS_ADDR_RANGE		BIT(1)
> +
> +/* Base of area reserved for config accesses. Fixed size of 64K. */
> +#define CFG_BASE_ADDRESS		0x92c
> +#define CFG_REGION_SIZE			65536
> +#define CFG_SPACE1_OFFSET		0x1000
> +
> +/* First 4K of config space has this BDF (bus,device,function) */
> +#define FUNC0_BDF_NUM			0x930
> +
> +/* Mem regions */
> +#define IN0_MEM_ADDR_START		0x964
> +#define IN0_MEM_ADDR_LIMIT		0x968
> +#define IN1_MEM_ADDR_START		0x974
> +#define IN1_MEM_ADDR_LIMIT		0x978
> +
> +/* This actually contains the LTSSM state machine state */
> +#define PORT_LOGIC_DEBUG_REG_0		0x728
> +
> +/* LTSSM state machine values	*/
> +#define DEBUG_REG_0_LTSSM_MASK		0x1f
> +#define S_DETECT_QUIET			0x00
> +#define S_DETECT_ACT			0x01
> +#define S_POLL_ACTIVE			0x02
> +#define S_POLL_COMPLIANCE		0x03
> +#define S_POLL_CONFIG			0x04
> +#define S_PRE_DETECT_QUIET		0x05
> +#define S_DETECT_WAIT			0x06
> +#define S_CFG_LINKWD_START		0x07
> +#define S_CFG_LINKWD_ACEPT		0x08
> +#define S_CFG_LANENUM_WAIT		0x09
> +#define S_CFG_LANENUM_ACEPT		0x0A
> +#define S_CFG_COMPLETE			0x0B
> +#define S_CFG_IDLE			0x0C
> +#define S_RCVRY_LOCK			0x0D
> +#define S_RCVRY_SPEED			0x0E
> +#define S_RCVRY_RCVRCFG			0x0F
> +#define S_RCVRY_IDLE			0x10
> +#define S_L0				0x11
> +#define S_L0S				0x12
> +#define S_L123_SEND_EIDLE		0x13
> +#define S_L1_IDLE			0x14
> +#define S_L2_IDLE			0x15
> +#define S_L2_WAKE			0x16
> +#define S_DISABLED_ENTRY		0x17
> +#define S_DISABLED_IDLE			0x18
> +#define S_DISABLED			0x19
> +#define S_LPBK_ENTRY			0x1A
> +#define S_LPBK_ACTIVE			0x1B
> +#define S_LPBK_EXIT			0x1C
> +#define S_LPBK_EXIT_TIMEOUT		0x1D
> +#define S_HOT_RESET_ENTRY		0x1E
> +#define S_HOT_RESET			0x1F
> +
> +/* syscfg bits */
> +#define PCIE_SYS_INT			BIT(5)
> +#define PCIE_APP_REQ_RETRY_EN		BIT(3)
> +#define PCIE_APP_LTSSM_ENABLE		BIT(2)
> +#define PCIE_APP_INIT_RST		BIT(1)
> +#define PCIE_DEVICE_TYPE		BIT(0)
> +#define PCIE_DEFAULT_VAL		PCIE_DEVICE_TYPE
> +
> +/* Time to wait between testing the link in msecs (hardware poll interval) */
> +#define LINK_LOOP_DELAY_MS 1
> +/* Total amount of time to wait for the link to come up in msecs */
> +#define LINK_WAIT_MS 120
> +#define LINK_LOOP_COUNT (LINK_WAIT_MS / LINK_LOOP_DELAY_MS)
> +
> +/* st,syscfg offsets */
> +#define SYSCFG0_REG	1
> +#define SYSCFG1_REG	2
> +
> +#define to_st_pcie(x)	container_of(x, struct st_pcie, pp)
> +
> +/**
> + * struct st_pcie - private data of the controller
> + * @pp: designware pcie port
> + * @syscfg0: PCIe configuration 0 register, regmap offset
> + * @syscfg1: PCIe configuration 1 register, regmap offset
> + * @phy: associated pcie phy
> + * @lmi: memory made available to the controller
> + * @regmap: Syscfg registers bank in which PCIe port is configured
> + * @pwr: power control
> + * @rst: reset control
> + * @reset_gpio: optional reset gpio
> + * @config_window_start: start address of 64K config space area
> + */
> +struct st_pcie {
> +	struct pcie_port pp;
> +	int syscfg0;
> +	int syscfg1;
> +	struct phy *phy;
> +	struct resource	*lmi;
> +	struct regmap *regmap;
> +	struct reset_control *pwr;
> +	struct reset_control *rst;
> +	int reset_gpio;
> +	phys_addr_t config_window_start;
> +};
> +
> +/*
> + * Function to test if the link is in an operational state or not. We must
> + * ensure the link is operational before we try to do a configuration access.
> + */
> +static int st_pcie_link_up(struct pcie_port *pp)
> +{
> +	u32 status;
> +	int link_up;
> +	int count = 0;
> +
> +	/*
> +	 * We have to be careful here. This is used in config read/write,
> +	 * The higher levels switch off interrupts, so you cannot use
> +	 * jiffies to do a timeout, or reschedule
> +	 */
> +	do {
> +		/*
> +		 * What about L2? I think software intervention is
> +		 * required to get it out of L2, so in effect the link
> +		 * is down. Requires more work when/if we implement power
> +		 * management
> +		 */
> +		status = readl_relaxed(pp->dbi_base + PORT_LOGIC_DEBUG_REG_0);
> +		status &= DEBUG_REG_0_LTSSM_MASK;
> +
> +		link_up = (status == S_L0) || (status == S_L0S) ||
> +			  (status == S_L1_IDLE);
> +
> +		/*
> +		 * It can take some considerable time for the link to actually
> +		 * come up, caused by the PLLs. Experiments indicate it takes
> +		 * about 8ms to actually bring the link up, but this can vary
> +		 * considerably according to the specification. This code should
> +		 * allow sufficient time
> +		 */
> +		if (!link_up)
> +			mdelay(LINK_LOOP_DELAY_MS);
> +
> +	} while (!link_up && ++count < LINK_LOOP_COUNT);
> +
> +	return link_up;
> +}
> +
> +/*
> + * On ARM platforms, we actually get a bus error returned when the PCIe IP
> + * returns a UR or CRS instead of an OK.
> + */
> +static int st_pcie_abort_handler(unsigned long addr, unsigned int fsr,
> +				 struct pt_regs *regs)
> +{
> +	return 0;
> +}
> +
> +/*
> + * The PCI express core IP expects the following arrangement on it's address
> + * bus (slv_haddr) when driving config cycles.
> + * bus_number		[31:24]
> + * dev_number		[23:19]
> + * func_number		[18:16]
> + * unused		[15:12]
> + * ext_reg_number	[11:8]
> + * reg_number		[7:2]
> + *
> + * Bits [15:12] are unused.
> + *
> + * In the glue logic there is a 64K region of address space that can be
> + * written/read to generate config cycles. The base address of this is
> + * controlled by CFG_BASE_ADDRESS. There are 8 16 bit registers called
> + * FUNC0_BDF_NUM to FUNC8_BDF_NUM. These split the bottom half of the 64K
> + * window into 8 regions at 4K boundaries. These control the bus,device and
> + * function number you are trying to talk to.
> + *
> + * The decision on whether to generate a type 0 or type 1 access is controlled
> + * by bits 15:12 of the address you write to.  If they are zero, then a type 0
> + * is generated, if anything else it will be a type 1. Thus the bottom 4K
> + * region controlled by FUNC0_BDF_NUM can only generate type 0, all the others
> + * can only generate type 1.
> + *
> + * We only use FUNC0_BDF_NUM and FUNC1_BDF_NUM. Which one you use is selected
> + * by bit 12 of the address you write to. The selected register is then used
> + * for the top 16 bits of the slv_haddr to form the bus/dev/func, bit 15:12 are
> + * wired to zero, and bits 11:2 form the address of the register you want to
> + * read in config space.
> + *
> + * We always write FUNC0_BDF_NUM as a 32 bit write. So if we want type 1
> + * accesses we have to shift by 16 so in effect we are writing to FUNC1_BDF_NUM
> + */
> +static inline u32 bdf_num(int bus, int devfn, int is_root_bus)
> +{
> +	return ((bus << 8) | devfn) << (is_root_bus ? 0 : 16);
> +}
> +
> +static int st_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> +				 unsigned int devfn, int where, int size,
> +				 u32 *val)
> +{
> +	int ret;
> +	u32 bdf, addr;
> +
> +	addr = where & ~0x3;
> +	bdf = bdf_num(bus->number, devfn, pci_is_root_bus(bus));
> +
> +	/* Set the config packet devfn */
> +	writel_relaxed(bdf, pp->dbi_base + FUNC0_BDF_NUM);
> +	readl_relaxed(pp->dbi_base + FUNC0_BDF_NUM);
> +
> +	if (bus->parent->number == pp->root_bus_nr)
> +		ret = dw_pcie_cfg_read(pp->va_cfg0_base + addr, where, size,
> +				       val);
> +	else
> +		ret = dw_pcie_cfg_read(pp->va_cfg1_base + addr, where, size,
> +				       val);
> +	return ret;
> +}
> +
> +static int st_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus *bus,
> +				 unsigned int devfn, int where, int size,
> +				 u32 val)
> +{
> +	int ret;
> +	u32 bdf, addr;
> +
> +	addr = where & ~0x3;
> +	bdf = bdf_num(bus->number, devfn, pci_is_root_bus(bus));
> +
> +	/* Set the config packet devfn */
> +	writel_relaxed(bdf, pp->dbi_base + FUNC0_BDF_NUM);
> +	readl_relaxed(pp->dbi_base + FUNC0_BDF_NUM);
> +
> +	if (bus->parent->number == pp->root_bus_nr)
> +		ret = dw_pcie_cfg_write(pp->va_cfg0_base + addr, where, size,
> +					val);
> +	else
> +		ret = dw_pcie_cfg_write(pp->va_cfg1_base + addr, where, size,
> +					val);
> +	return ret;
> +}
> +
> +static void st_pcie_board_reset(struct pcie_port *pp)
> +{
> +	struct st_pcie *pcie = to_st_pcie(pp);
> +
> +	if (!gpio_is_valid(pcie->reset_gpio))
> +		return;
> +
> +	if (gpio_direction_output(pcie->reset_gpio, 0)) {
> +		dev_err(pp->dev, "Cannot set PERST# (gpio %u) to output\n",
> +			pcie->reset_gpio);
> +		return;
> +	}
> +
> +	/* From PCIe spec */
> +	msleep(2);
> +	gpio_direction_output(pcie->reset_gpio, 1);
> +
> +	/*
> +	 * PCIe specification states that you should not issue any config
> +	 * requests until 100ms after asserting reset, so we enforce that here
> +	 */
> +	msleep(100);
> +}
> +
> +static void st_pcie_hw_setup(struct pcie_port *pp)
> +{
> +	struct st_pcie *pcie = to_st_pcie(pp);
> +
> +	dw_pcie_setup_rc(pp);
> +
> +	/* Set up the config window to the top of the PCI address space */
> +	writel_relaxed(pcie->config_window_start,
> +		       pp->dbi_base + CFG_BASE_ADDRESS);
> +
> +	/*
> +	 * Open up memory to the PCI controller. We could do slightly
> +	 * better than this and exclude the kernel text segment and bss etc.
> +	 * They are base/limit registers so can be of arbitrary alignment
> +	 * presumably
> +	 */
> +	writel_relaxed(pcie->lmi->start, pp->dbi_base + IN0_MEM_ADDR_START);
> +	writel_relaxed(pcie->lmi->end, pp->dbi_base + IN0_MEM_ADDR_LIMIT);
> +
> +	/* Disable the 2nd region */
> +	writel_relaxed(~0, pp->dbi_base + IN1_MEM_ADDR_START);
> +	writel_relaxed(0, pp->dbi_base + IN1_MEM_ADDR_LIMIT);
> +
> +	writel_relaxed(RC_PASS_ADDR_RANGE, pp->dbi_base + TRANSLATION_CONTROL);
> +
> +	/* Now assert the board level reset to the other PCIe device */
> +	st_pcie_board_reset(pp);
> +}
> +
> +static irqreturn_t st_pcie_msi_irq_handler(int irq, void *arg)
> +{
> +	struct pcie_port *pp = arg;
> +
> +	return dw_handle_msi_irq(pp);
> +}
> +
> +static int st_pcie_init(struct pcie_port *pp)
> +{
> +	struct st_pcie *pcie = to_st_pcie(pp);
> +	int ret;
> +
> +	ret = reset_control_deassert(pcie->pwr);
> +	if (ret) {
> +		dev_err(pp->dev, "unable to bring out of powerdown\n");
> +		return ret;
> +	}
> +
> +	ret = reset_control_deassert(pcie->rst);
> +	if (ret) {
> +		dev_err(pp->dev, "unable to bring out of softreset\n");
> +		return ret;
> +	}
> +
> +	/* Set device type : Root Complex */
> +	ret = regmap_write(pcie->regmap, pcie->syscfg0, PCIE_DEVICE_TYPE);
> +	if (ret < 0) {
> +		dev_err(pp->dev, "unable to set device type\n");
> +		return ret;
> +	}
> +
> +	usleep_range(1000, 2000);
> +	return ret;
> +}
> +
> +static int st_pcie_enable_ltssm(struct pcie_port *pp)
> +{
> +	struct st_pcie *pcie = to_st_pcie(pp);
> +
> +	return regmap_update_bits(pcie->regmap, pcie->syscfg1,
> +				  PCIE_APP_LTSSM_ENABLE, PCIE_APP_LTSSM_ENABLE);
> +}
> +
> +static int st_pcie_disable_ltssm(struct pcie_port *pp)
> +{
> +	struct st_pcie *pcie = to_st_pcie(pp);
> +
> +	return regmap_update_bits(pcie->regmap, pcie->syscfg1,
> +				  PCIE_APP_LTSSM_ENABLE, 0);
> +}
> +
> +static void st_pcie_host_init(struct pcie_port *pp)
> +{
> +	struct st_pcie *pcie = to_st_pcie(pp);
> +	int err;
> +
> +	/*
> +	 * We have to initialise the PCIe cell on some hardware before we can
> +	 * talk to the phy
> +	 */
> +	err = st_pcie_init(pp);
> +	if (err)
> +		return;
> +
> +	err = st_pcie_disable_ltssm(pp);
> +	if (err) {
> +		dev_err(pp->dev, "disable ltssm failed, %d\n", err);
> +		return;
> +	}
> +
> +	/* Now init the associated miphy */
> +	err = phy_init(pcie->phy);
> +	if (err < 0) {
> +		dev_err(pp->dev, "Cannot init PHY: %d\n", err);
> +		return;
> +	}
> +
> +	/* Now do all the register poking */
> +	st_pcie_hw_setup(pp);
> +
> +	/* Re-enable the link */
> +	err = st_pcie_enable_ltssm(pp);
> +	if (err)
> +		dev_err(pp->dev, "enable ltssm failed, %d\n", err);
> +
> +	if (IS_ENABLED(CONFIG_PCI_MSI))
> +		dw_pcie_msi_init(pp);
> +}
> +
> +static struct pcie_host_ops st_pcie_host_ops = {
> +	.rd_other_conf = st_pcie_rd_other_conf,
> +	.wr_other_conf = st_pcie_wr_other_conf,
> +	.link_up = st_pcie_link_up,
> +	.host_init = st_pcie_host_init,
> +};
> +
> +static const struct of_device_id st_pcie_of_match[] = {
> +	{ .compatible = "st,pcie", },
> +	{ },
> +};
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int st_pcie_suspend(struct device *pcie_dev)
> +{
> +	struct st_pcie *pcie = dev_get_drvdata(pcie_dev);
> +
> +	/* To guarantee a real phy initialization on resume */
> +	phy_exit(pcie->phy);
> +
> +	return 0;
> +}
> +
> +static int st_pcie_resume(struct device *pcie_dev)
> +{
> +	struct st_pcie *pcie = dev_get_drvdata(pcie_dev);
> +
> +	st_pcie_host_init(&pcie->pp);
> +
> +	return 0;
> +}

Just curious to know if you tested suspend/resume with PCIe cards
connected to the RC?

Thanks
Kishon

[PATCH v2 3/5] PCI: st: Provide support for the sti PCIe controller

[Gabriel Fernandez]

Hi Kishon,

I tested with my internal 3.10 ST Kernel but not on the 4.0.
I think i'll implement it when i'm able to test it fully.
Thanks

On 17 March 2015 at 11:35, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Hi,
>
>
> On Monday 16 March 2015 07:50 PM, Gabriel FERNANDEZ wrote:
>>
>> sti pcie is built around a Synopsis Designware PCIe IP.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
>> ---
>>   drivers/pci/host/Kconfig  |   9 +
>>   drivers/pci/host/Makefile |   1 +
>>   drivers/pci/host/pci-st.c | 617
>> ++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 627 insertions(+)
>>   create mode 100644 drivers/pci/host/pci-st.c
>>
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index 7b892a9..af9f9212 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -106,4 +106,13 @@ config PCI_VERSATILE
>>         bool "ARM Versatile PB PCI controller"
>>         depends on ARCH_VERSATILE
>>
>> +config PCI_ST
>> +       bool "ST PCIe controller"
>> +       depends on ARCH_STI || (ARM && COMPILE_TEST)
>> +       select PCIE_DW
>> +       help
>> +         Enable PCIe controller support on ST Socs. This controller is
>> based
>> +         on Designware hardware and therefore the driver re-uses the
>> +         Designware core functions to implement the driver.
>> +
>>   endmenu
>> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
>> index e61d91c..97c6622 100644
>> --- a/drivers/pci/host/Makefile
>> +++ b/drivers/pci/host/Makefile
>> @@ -13,3 +13,4 @@ obj-$(CONFIG_PCIE_XILINX) += pcie-xilinx.o
>>   obj-$(CONFIG_PCI_XGENE) += pci-xgene.o
>>   obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
>>   obj-$(CONFIG_PCI_VERSATILE) += pci-versatile.o
>> +obj-$(CONFIG_PCI_ST) += pci-st.o
>> diff --git a/drivers/pci/host/pci-st.c b/drivers/pci/host/pci-st.c
>> new file mode 100644
>> index 0000000..470000d
>> --- /dev/null
>> +++ b/drivers/pci/host/pci-st.c
>> @@ -0,0 +1,617 @@
>> +/*
>> + * Copyright (C) 2014 STMicroelectronics
>> + *
>> + * STMicroelectronics PCI express Driver for sti SoCs.
>> + * ST PCIe IPs are built around a Synopsys IP Core.
>> + *
>> + * Author: Fabrice Gasnier <fabrice.gasnier@st.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/delay.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/mfd/syscon.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/of_pci.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/regmap.h>
>> +#include <linux/reset.h>
>> +
>> +#include "pcie-designware.h"
>> +
>> +#define TRANSLATION_CONTROL            0x900
>> +/* Controls if area is inclusive or exclusive */
>> +#define RC_PASS_ADDR_RANGE             BIT(1)
>> +
>> +/* Base of area reserved for config accesses. Fixed size of 64K. */
>> +#define CFG_BASE_ADDRESS               0x92c
>> +#define CFG_REGION_SIZE                        65536
>> +#define CFG_SPACE1_OFFSET              0x1000
>> +
>> +/* First 4K of config space has this BDF (bus,device,function) */
>> +#define FUNC0_BDF_NUM                  0x930
>> +
>> +/* Mem regions */
>> +#define IN0_MEM_ADDR_START             0x964
>> +#define IN0_MEM_ADDR_LIMIT             0x968
>> +#define IN1_MEM_ADDR_START             0x974
>> +#define IN1_MEM_ADDR_LIMIT             0x978
>> +
>> +/* This actually contains the LTSSM state machine state */
>> +#define PORT_LOGIC_DEBUG_REG_0         0x728
>> +
>> +/* LTSSM state machine values  */
>> +#define DEBUG_REG_0_LTSSM_MASK         0x1f
>> +#define S_DETECT_QUIET                 0x00
>> +#define S_DETECT_ACT                   0x01
>> +#define S_POLL_ACTIVE                  0x02
>> +#define S_POLL_COMPLIANCE              0x03
>> +#define S_POLL_CONFIG                  0x04
>> +#define S_PRE_DETECT_QUIET             0x05
>> +#define S_DETECT_WAIT                  0x06
>> +#define S_CFG_LINKWD_START             0x07
>> +#define S_CFG_LINKWD_ACEPT             0x08
>> +#define S_CFG_LANENUM_WAIT             0x09
>> +#define S_CFG_LANENUM_ACEPT            0x0A
>> +#define S_CFG_COMPLETE                 0x0B
>> +#define S_CFG_IDLE                     0x0C
>> +#define S_RCVRY_LOCK                   0x0D
>> +#define S_RCVRY_SPEED                  0x0E
>> +#define S_RCVRY_RCVRCFG                        0x0F
>> +#define S_RCVRY_IDLE                   0x10
>> +#define S_L0                           0x11
>> +#define S_L0S                          0x12
>> +#define S_L123_SEND_EIDLE              0x13
>> +#define S_L1_IDLE                      0x14
>> +#define S_L2_IDLE                      0x15
>> +#define S_L2_WAKE                      0x16
>> +#define S_DISABLED_ENTRY               0x17
>> +#define S_DISABLED_IDLE                        0x18
>> +#define S_DISABLED                     0x19
>> +#define S_LPBK_ENTRY                   0x1A
>> +#define S_LPBK_ACTIVE                  0x1B
>> +#define S_LPBK_EXIT                    0x1C
>> +#define S_LPBK_EXIT_TIMEOUT            0x1D
>> +#define S_HOT_RESET_ENTRY              0x1E
>> +#define S_HOT_RESET                    0x1F
>> +
>> +/* syscfg bits */
>> +#define PCIE_SYS_INT                   BIT(5)
>> +#define PCIE_APP_REQ_RETRY_EN          BIT(3)
>> +#define PCIE_APP_LTSSM_ENABLE          BIT(2)
>> +#define PCIE_APP_INIT_RST              BIT(1)
>> +#define PCIE_DEVICE_TYPE               BIT(0)
>> +#define PCIE_DEFAULT_VAL               PCIE_DEVICE_TYPE
>> +
>> +/* Time to wait between testing the link in msecs (hardware poll
>> interval) */
>> +#define LINK_LOOP_DELAY_MS 1
>> +/* Total amount of time to wait for the link to come up in msecs */
>> +#define LINK_WAIT_MS 120
>> +#define LINK_LOOP_COUNT (LINK_WAIT_MS / LINK_LOOP_DELAY_MS)
>> +
>> +/* st,syscfg offsets */
>> +#define SYSCFG0_REG    1
>> +#define SYSCFG1_REG    2
>> +
>> +#define to_st_pcie(x)  container_of(x, struct st_pcie, pp)
>> +
>> +/**
>> + * struct st_pcie - private data of the controller
>> + * @pp: designware pcie port
>> + * @syscfg0: PCIe configuration 0 register, regmap offset
>> + * @syscfg1: PCIe configuration 1 register, regmap offset
>> + * @phy: associated pcie phy
>> + * @lmi: memory made available to the controller
>> + * @regmap: Syscfg registers bank in which PCIe port is configured
>> + * @pwr: power control
>> + * @rst: reset control
>> + * @reset_gpio: optional reset gpio
>> + * @config_window_start: start address of 64K config space area
>> + */
>> +struct st_pcie {
>> +       struct pcie_port pp;
>> +       int syscfg0;
>> +       int syscfg1;
>> +       struct phy *phy;
>> +       struct resource *lmi;
>> +       struct regmap *regmap;
>> +       struct reset_control *pwr;
>> +       struct reset_control *rst;
>> +       int reset_gpio;
>> +       phys_addr_t config_window_start;
>> +};
>> +
>> +/*
>> + * Function to test if the link is in an operational state or not. We
>> must
>> + * ensure the link is operational before we try to do a configuration
>> access.
>> + */
>> +static int st_pcie_link_up(struct pcie_port *pp)
>> +{
>> +       u32 status;
>> +       int link_up;
>> +       int count = 0;
>> +
>> +       /*
>> +        * We have to be careful here. This is used in config read/write,
>> +        * The higher levels switch off interrupts, so you cannot use
>> +        * jiffies to do a timeout, or reschedule
>> +        */
>> +       do {
>> +               /*
>> +                * What about L2? I think software intervention is
>> +                * required to get it out of L2, so in effect the link
>> +                * is down. Requires more work when/if we implement power
>> +                * management
>> +                */
>> +               status = readl_relaxed(pp->dbi_base +
>> PORT_LOGIC_DEBUG_REG_0);
>> +               status &= DEBUG_REG_0_LTSSM_MASK;
>> +
>> +               link_up = (status == S_L0) || (status == S_L0S) ||
>> +                         (status == S_L1_IDLE);
>> +
>> +               /*
>> +                * It can take some considerable time for the link to
>> actually
>> +                * come up, caused by the PLLs. Experiments indicate it
>> takes
>> +                * about 8ms to actually bring the link up, but this can
>> vary
>> +                * considerably according to the specification. This code
>> should
>> +                * allow sufficient time
>> +                */
>> +               if (!link_up)
>> +                       mdelay(LINK_LOOP_DELAY_MS);
>> +
>> +       } while (!link_up && ++count < LINK_LOOP_COUNT);
>> +
>> +       return link_up;
>> +}
>> +
>> +/*
>> + * On ARM platforms, we actually get a bus error returned when the PCIe
>> IP
>> + * returns a UR or CRS instead of an OK.
>> + */
>> +static int st_pcie_abort_handler(unsigned long addr, unsigned int fsr,
>> +                                struct pt_regs *regs)
>> +{
>> +       return 0;
>> +}
>> +
>> +/*
>> + * The PCI express core IP expects the following arrangement on it's
>> address
>> + * bus (slv_haddr) when driving config cycles.
>> + * bus_number          [31:24]
>> + * dev_number          [23:19]
>> + * func_number         [18:16]
>> + * unused              [15:12]
>> + * ext_reg_number      [11:8]
>> + * reg_number          [7:2]
>> + *
>> + * Bits [15:12] are unused.
>> + *
>> + * In the glue logic there is a 64K region of address space that can be
>> + * written/read to generate config cycles. The base address of this is
>> + * controlled by CFG_BASE_ADDRESS. There are 8 16 bit registers called
>> + * FUNC0_BDF_NUM to FUNC8_BDF_NUM. These split the bottom half of the 64K
>> + * window into 8 regions at 4K boundaries. These control the bus,device
>> and
>> + * function number you are trying to talk to.
>> + *
>> + * The decision on whether to generate a type 0 or type 1 access is
>> controlled
>> + * by bits 15:12 of the address you write to.  If they are zero, then a
>> type 0
>> + * is generated, if anything else it will be a type 1. Thus the bottom 4K
>> + * region controlled by FUNC0_BDF_NUM can only generate type 0, all the
>> others
>> + * can only generate type 1.
>> + *
>> + * We only use FUNC0_BDF_NUM and FUNC1_BDF_NUM. Which one you use is
>> selected
>> + * by bit 12 of the address you write to. The selected register is then
>> used
>> + * for the top 16 bits of the slv_haddr to form the bus/dev/func, bit
>> 15:12 are
>> + * wired to zero, and bits 11:2 form the address of the register you want
>> to
>> + * read in config space.
>> + *
>> + * We always write FUNC0_BDF_NUM as a 32 bit write. So if we want type 1
>> + * accesses we have to shift by 16 so in effect we are writing to
>> FUNC1_BDF_NUM
>> + */
>> +static inline u32 bdf_num(int bus, int devfn, int is_root_bus)
>> +{
>> +       return ((bus << 8) | devfn) << (is_root_bus ? 0 : 16);
>> +}
>> +
>> +static int st_pcie_rd_other_conf(struct pcie_port *pp, struct pci_bus
>> *bus,
>> +                                unsigned int devfn, int where, int size,
>> +                                u32 *val)
>> +{
>> +       int ret;
>> +       u32 bdf, addr;
>> +
>> +       addr = where & ~0x3;
>> +       bdf = bdf_num(bus->number, devfn, pci_is_root_bus(bus));
>> +
>> +       /* Set the config packet devfn */
>> +       writel_relaxed(bdf, pp->dbi_base + FUNC0_BDF_NUM);
>> +       readl_relaxed(pp->dbi_base + FUNC0_BDF_NUM);
>> +
>> +       if (bus->parent->number == pp->root_bus_nr)
>> +               ret = dw_pcie_cfg_read(pp->va_cfg0_base + addr, where,
>> size,
>> +                                      val);
>> +       else
>> +               ret = dw_pcie_cfg_read(pp->va_cfg1_base + addr, where,
>> size,
>> +                                      val);
>> +       return ret;
>> +}
>> +
>> +static int st_pcie_wr_other_conf(struct pcie_port *pp, struct pci_bus
>> *bus,
>> +                                unsigned int devfn, int where, int size,
>> +                                u32 val)
>> +{
>> +       int ret;
>> +       u32 bdf, addr;
>> +
>> +       addr = where & ~0x3;
>> +       bdf = bdf_num(bus->number, devfn, pci_is_root_bus(bus));
>> +
>> +       /* Set the config packet devfn */
>> +       writel_relaxed(bdf, pp->dbi_base + FUNC0_BDF_NUM);
>> +       readl_relaxed(pp->dbi_base + FUNC0_BDF_NUM);
>> +
>> +       if (bus->parent->number == pp->root_bus_nr)
>> +               ret = dw_pcie_cfg_write(pp->va_cfg0_base + addr, where,
>> size,
>> +                                       val);
>> +       else
>> +               ret = dw_pcie_cfg_write(pp->va_cfg1_base + addr, where,
>> size,
>> +                                       val);
>> +       return ret;
>> +}
>> +
>> +static void st_pcie_board_reset(struct pcie_port *pp)
>> +{
>> +       struct st_pcie *pcie = to_st_pcie(pp);
>> +
>> +       if (!gpio_is_valid(pcie->reset_gpio))
>> +               return;
>> +
>> +       if (gpio_direction_output(pcie->reset_gpio, 0)) {
>> +               dev_err(pp->dev, "Cannot set PERST# (gpio %u) to
>> output\n",
>> +                       pcie->reset_gpio);
>> +               return;
>> +       }
>> +
>> +       /* From PCIe spec */
>> +       msleep(2);
>> +       gpio_direction_output(pcie->reset_gpio, 1);
>> +
>> +       /*
>> +        * PCIe specification states that you should not issue any config
>> +        * requests until 100ms after asserting reset, so we enforce that
>> here
>> +        */
>> +       msleep(100);
>> +}
>> +
>> +static void st_pcie_hw_setup(struct pcie_port *pp)
>> +{
>> +       struct st_pcie *pcie = to_st_pcie(pp);
>> +
>> +       dw_pcie_setup_rc(pp);
>> +
>> +       /* Set up the config window to the top of the PCI address space */
>> +       writel_relaxed(pcie->config_window_start,
>> +                      pp->dbi_base + CFG_BASE_ADDRESS);
>> +
>> +       /*
>> +        * Open up memory to the PCI controller. We could do slightly
>> +        * better than this and exclude the kernel text segment and bss
>> etc.
>> +        * They are base/limit registers so can be of arbitrary alignment
>> +        * presumably
>> +        */
>> +       writel_relaxed(pcie->lmi->start, pp->dbi_base +
>> IN0_MEM_ADDR_START);
>> +       writel_relaxed(pcie->lmi->end, pp->dbi_base + IN0_MEM_ADDR_LIMIT);
>> +
>> +       /* Disable the 2nd region */
>> +       writel_relaxed(~0, pp->dbi_base + IN1_MEM_ADDR_START);
>> +       writel_relaxed(0, pp->dbi_base + IN1_MEM_ADDR_LIMIT);
>> +
>> +       writel_relaxed(RC_PASS_ADDR_RANGE, pp->dbi_base +
>> TRANSLATION_CONTROL);
>> +
>> +       /* Now assert the board level reset to the other PCIe device */
>> +       st_pcie_board_reset(pp);
>> +}
>> +
>> +static irqreturn_t st_pcie_msi_irq_handler(int irq, void *arg)
>> +{
>> +       struct pcie_port *pp = arg;
>> +
>> +       return dw_handle_msi_irq(pp);
>> +}
>> +
>> +static int st_pcie_init(struct pcie_port *pp)
>> +{
>> +       struct st_pcie *pcie = to_st_pcie(pp);
>> +       int ret;
>> +
>> +       ret = reset_control_deassert(pcie->pwr);
>> +       if (ret) {
>> +               dev_err(pp->dev, "unable to bring out of powerdown\n");
>> +               return ret;
>> +       }
>> +
>> +       ret = reset_control_deassert(pcie->rst);
>> +       if (ret) {
>> +               dev_err(pp->dev, "unable to bring out of softreset\n");
>> +               return ret;
>> +       }
>> +
>> +       /* Set device type : Root Complex */
>> +       ret = regmap_write(pcie->regmap, pcie->syscfg0, PCIE_DEVICE_TYPE);
>> +       if (ret < 0) {
>> +               dev_err(pp->dev, "unable to set device type\n");
>> +               return ret;
>> +       }
>> +
>> +       usleep_range(1000, 2000);
>> +       return ret;
>> +}
>> +
>> +static int st_pcie_enable_ltssm(struct pcie_port *pp)
>> +{
>> +       struct st_pcie *pcie = to_st_pcie(pp);
>> +
>> +       return regmap_update_bits(pcie->regmap, pcie->syscfg1,
>> +                                 PCIE_APP_LTSSM_ENABLE,
>> PCIE_APP_LTSSM_ENABLE);
>> +}
>> +
>> +static int st_pcie_disable_ltssm(struct pcie_port *pp)
>> +{
>> +       struct st_pcie *pcie = to_st_pcie(pp);
>> +
>> +       return regmap_update_bits(pcie->regmap, pcie->syscfg1,
>> +                                 PCIE_APP_LTSSM_ENABLE, 0);
>> +}
>> +
>> +static void st_pcie_host_init(struct pcie_port *pp)
>> +{
>> +       struct st_pcie *pcie = to_st_pcie(pp);
>> +       int err;
>> +
>> +       /*
>> +        * We have to initialise the PCIe cell on some hardware before we
>> can
>> +        * talk to the phy
>> +        */
>> +       err = st_pcie_init(pp);
>> +       if (err)
>> +               return;
>> +
>> +       err = st_pcie_disable_ltssm(pp);
>> +       if (err) {
>> +               dev_err(pp->dev, "disable ltssm failed, %d\n", err);
>> +               return;
>> +       }
>> +
>> +       /* Now init the associated miphy */
>> +       err = phy_init(pcie->phy);
>> +       if (err < 0) {
>> +               dev_err(pp->dev, "Cannot init PHY: %d\n", err);
>> +               return;
>> +       }
>> +
>> +       /* Now do all the register poking */
>> +       st_pcie_hw_setup(pp);
>> +
>> +       /* Re-enable the link */
>> +       err = st_pcie_enable_ltssm(pp);
>> +       if (err)
>> +               dev_err(pp->dev, "enable ltssm failed, %d\n", err);
>> +
>> +       if (IS_ENABLED(CONFIG_PCI_MSI))
>> +               dw_pcie_msi_init(pp);
>> +}
>> +
>> +static struct pcie_host_ops st_pcie_host_ops = {
>> +       .rd_other_conf = st_pcie_rd_other_conf,
>> +       .wr_other_conf = st_pcie_wr_other_conf,
>> +       .link_up = st_pcie_link_up,
>> +       .host_init = st_pcie_host_init,
>> +};
>> +
>> +static const struct of_device_id st_pcie_of_match[] = {
>> +       { .compatible = "st,pcie", },
>> +       { },
>> +};
>> +
>> +#ifdef CONFIG_PM_SLEEP
>> +static int st_pcie_suspend(struct device *pcie_dev)
>> +{
>> +       struct st_pcie *pcie = dev_get_drvdata(pcie_dev);
>> +
>> +       /* To guarantee a real phy initialization on resume */
>> +       phy_exit(pcie->phy);
>> +
>> +       return 0;
>> +}
>> +
>> +static int st_pcie_resume(struct device *pcie_dev)
>> +{
>> +       struct st_pcie *pcie = dev_get_drvdata(pcie_dev);
>> +
>> +       st_pcie_host_init(&pcie->pp);
>> +
>> +       return 0;
>> +}
>
>
> Just curious to know if you tested suspend/resume with PCIe cards
> connected to the RC?
>
> Thanks
> Kishon

[PATCH v2 4/5] PCI: designware: Add disable IO support

[Gabriel FERNANDEZ]

ST sti SoCs PCIe IPs are built around DesignWare IP Core.
But in these SoCs PCIe IP doesn't support IO.

This patch adds the possibility to disable it through
a DT property, by creating an empty IO window and by
removing PCI_COMMAND_IO from the setup register.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
---
 .../devicetree/bindings/pci/designware-pcie.txt    |  2 ++
 drivers/pci/host/pcie-designware.c                 | 24 ++++++++++++++++++++--
 drivers/pci/host/pcie-designware.h                 |  1 +
 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index 9f4faa8..40544d4 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -26,3 +26,5 @@ Optional properties:
 - bus-range: PCI bus numbers covered (it is recommended for new devicetrees to
   specify this property, to keep backwards compatibility a range of 0x00-0xff
   is assumed if not present)
+- disable_io_support: set this property for PCIe host controller without IO
+  port access
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 1f4ea6f..f9d70f5 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -471,6 +471,9 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
 		return -EINVAL;
 	}
 
+	pp->disable_io_support = of_property_read_bool(np,
+			"disable_io_support");
+
 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
 		if (!pp->ops->msi_host_init) {
 			pp->irq_domain = irq_domain_add_linear(pp->dev->of_node,
@@ -704,6 +707,7 @@ static struct pci_ops dw_pcie_ops = {
 static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
 {
 	struct pcie_port *pp;
+	struct resource *res;
 
 	pp = sys_to_pcie(sys);
 
@@ -719,6 +723,18 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
 	pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
 	pci_add_resource(&sys->resources, &pp->busn);
 
+	if (pp->disable_io_support) {
+		/* This PCIe controller does not support IO, set an empty one */
+		res = devm_kzalloc(pp->dev, sizeof(*res), GFP_KERNEL);
+		if (res) {
+			res->start = 0;
+			res->end = 0;
+			res->name = "PCIe empty IO space";
+			res->flags = IORESOURCE_IO;
+			pci_add_resource(&sys->resources, res);
+		}
+	}
+
 	return 1;
 }
 
@@ -822,8 +838,12 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
 	/* setup command register */
 	dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
 	val &= 0xffff0000;
-	val |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
-		PCI_COMMAND_MASTER | PCI_COMMAND_SERR;
+
+	if (!pp->disable_io_support)
+		val |= PCI_COMMAND_IO;
+
+	val |= PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | PCI_COMMAND_SERR;
+
 	dw_pcie_writel_rc(pp, val, PCI_COMMAND);
 }
 
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index d0bbd27..027045d 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -52,6 +52,7 @@ struct pcie_port {
 	int			msi_irq;
 	struct irq_domain	*irq_domain;
 	unsigned long		msi_data;
+	bool			disable_io_support;
 	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
 };
 
-- 
1.9.1

[PATCH v2 4/5] PCI: designware: Add disable IO support

[Srinivas Kandagatla]

On 16/03/15 14:20, Gabriel FERNANDEZ wrote:
>   - bus-range: PCI bus numbers covered (it is recommended for new devicetrees to
>     specify this property, to keep backwards compatibility a range of 0x00-0xff
>     is assumed if not present)
> +- disable_io_support: set this property for PCIe host controller without IO
> +  port access
Shouldn't dt properties use hyphens rather than under scores ?

[PATCH v2 4/5] PCI: designware: Add disable IO support

[Gabriel Fernandez]

Hi Srinivas,

Yes, you are right.

Nevertheless i'll try the Kumar and Arnd 's request to not use DT to do that.

BR

Gabriel

On 16 March 2015 at 18:53, Srinivas Kandagatla
<srinivas.kandagatla@linaro.org> wrote:
>
>
> On 16/03/15 14:20, Gabriel FERNANDEZ wrote:
>>
>>   - bus-range: PCI bus numbers covered (it is recommended for new
>> devicetrees to
>>     specify this property, to keep backwards compatibility a range of
>> 0x00-0xff
>>     is assumed if not present)
>> +- disable_io_support: set this property for PCIe host controller without
>> IO
>> +  port access
>
> Shouldn't dt properties use hyphens rather than under scores ?

[PATCH v2 4/5] PCI: designware: Add disable IO support

[Kumar Gala]

On Mar 16, 2015, at 9:20 AM, Gabriel FERNANDEZ <gabriel.fernandez@st.com> wrote:

> ST sti SoCs PCIe IPs are built around DesignWare IP Core.
> But in these SoCs PCIe IP doesn't support IO.
> 
> This patch adds the possibility to disable it through
> a DT property, by creating an empty IO window and by
> removing PCI_COMMAND_IO from the setup register.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
> ---
> .../devicetree/bindings/pci/designware-pcie.txt    |  2 ++
> drivers/pci/host/pcie-designware.c                 | 24 ++++++++++++++++++++--
> drivers/pci/host/pcie-designware.h                 |  1 +
> 3 files changed, 25 insertions(+), 2 deletions(-)

Why not just update the code such that if the ranges doesn?t have an IO space rather than introducing a new DT property?

- k

> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index 9f4faa8..40544d4 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -26,3 +26,5 @@ Optional properties:
> - bus-range: PCI bus numbers covered (it is recommended for new devicetrees to
>   specify this property, to keep backwards compatibility a range of 0x00-0xff
>   is assumed if not present)
> +- disable_io_support: set this property for PCIe host controller without IO
> +  port access
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 1f4ea6f..f9d70f5 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -471,6 +471,9 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
> 		return -EINVAL;
> 	}
> 
> +	pp->disable_io_support = of_property_read_bool(np,
> +			"disable_io_support");
> +
> 	if (IS_ENABLED(CONFIG_PCI_MSI)) {
> 		if (!pp->ops->msi_host_init) {
> 			pp->irq_domain = irq_domain_add_linear(pp->dev->of_node,
> @@ -704,6 +707,7 @@ static struct pci_ops dw_pcie_ops = {
> static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
> {
> 	struct pcie_port *pp;
> +	struct resource *res;
> 
> 	pp = sys_to_pcie(sys);
> 
> @@ -719,6 +723,18 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
> 	pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
> 	pci_add_resource(&sys->resources, &pp->busn);
> 
> +	if (pp->disable_io_support) {
> +		/* This PCIe controller does not support IO, set an empty one */
> +		res = devm_kzalloc(pp->dev, sizeof(*res), GFP_KERNEL);
> +		if (res) {
> +			res->start = 0;
> +			res->end = 0;
> +			res->name = "PCIe empty IO space";
> +			res->flags = IORESOURCE_IO;
> +			pci_add_resource(&sys->resources, res);

Do we really need an empty resource?  What happens if we just dont have one?

> +		}
> +	}
> +
> 	return 1;
> }
> 
> @@ -822,8 +838,12 @@ void dw_pcie_setup_rc(struct pcie_port *pp)
> 	/* setup command register */
> 	dw_pcie_readl_rc(pp, PCI_COMMAND, &val);
> 	val &= 0xffff0000;
> -	val |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
> -		PCI_COMMAND_MASTER | PCI_COMMAND_SERR;
> +
> +	if (!pp->disable_io_support)
> +		val |= PCI_COMMAND_IO;
> +
> +	val |= PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | PCI_COMMAND_SERR;
> +
> 	dw_pcie_writel_rc(pp, val, PCI_COMMAND);
> }
> 
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index d0bbd27..027045d 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -52,6 +52,7 @@ struct pcie_port {
> 	int			msi_irq;
> 	struct irq_domain	*irq_domain;
> 	unsigned long		msi_data;
> +	bool			disable_io_support;
> 	DECLARE_BITMAP(msi_irq_in_use, MAX_MSI_IRQS);
> };
> 
> -- 
> 1.9.1
> 

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH v2 4/5] PCI: designware: Add disable IO support

[Arnd Bergmann]

On Monday 16 March 2015 13:00:51 Kumar Gala wrote:
> On Mar 16, 2015, at 9:20 AM, Gabriel FERNANDEZ <gabriel.fernandez@st.com> wrote:
> 
> > ST sti SoCs PCIe IPs are built around DesignWare IP Core.
> > But in these SoCs PCIe IP doesn't support IO.
> > 
> > This patch adds the possibility to disable it through
> > a DT property, by creating an empty IO window and by
> > removing PCI_COMMAND_IO from the setup register.
> > 
> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
> > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
> > ---
> > .../devicetree/bindings/pci/designware-pcie.txt    |  2 ++
> > drivers/pci/host/pcie-designware.c                 | 24 ++++++++++++++++++++--
> > drivers/pci/host/pcie-designware.h                 |  1 +
> > 3 files changed, 25 insertions(+), 2 deletions(-)
> 
> Why not just update the code such that if the ranges doesn?t have an IO
> space rather than introducing a new DT property?

I suspect we can simplify this now by changing over the designware PCI
code from pci_common_init_dev to calling pci_scan_root_bus() in the
same way that pci-versatile.c does. This would also clean up some
other areas of the driver and let you do proper error handling
in the probe.

	Arnd

[PATCH v2 4/5] PCI: designware: Add disable IO support

[Gabriel Fernandez]

Hi Arnd,

Ok i will try the same way that pci-versatile.c

Thanks.

Gabriel

On 16 March 2015 at 21:00, Arnd Bergmann <arnd@arndb.de> wrote:
> On Monday 16 March 2015 13:00:51 Kumar Gala wrote:
>> On Mar 16, 2015, at 9:20 AM, Gabriel FERNANDEZ <gabriel.fernandez@st.com> wrote:
>>
>> > ST sti SoCs PCIe IPs are built around DesignWare IP Core.
>> > But in these SoCs PCIe IP doesn't support IO.
>> >
>> > This patch adds the possibility to disable it through
>> > a DT property, by creating an empty IO window and by
>> > removing PCI_COMMAND_IO from the setup register.
>> >
>> > Signed-off-by: Fabrice Gasnier <fabrice.gasnier@st.com>
>> > Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
>> > ---
>> > .../devicetree/bindings/pci/designware-pcie.txt    |  2 ++
>> > drivers/pci/host/pcie-designware.c                 | 24 ++++++++++++++++++++--
>> > drivers/pci/host/pcie-designware.h                 |  1 +
>> > 3 files changed, 25 insertions(+), 2 deletions(-)
>>
>> Why not just update the code such that if the ranges doesn?t have an IO
>> space rather than introducing a new DT property?
>
> I suspect we can simplify this now by changing over the designware PCI
> code from pci_common_init_dev to calling pci_scan_root_bus() in the
> same way that pci-versatile.c does. This would also clean up some
> other areas of the driver and let you do proper error handling
> in the probe.
>
>         Arnd

[PATCH v2 5/5] MAINTAINERS: Add pci-st.c to ARCH/STI architecture

[Gabriel FERNANDEZ]

This patch adds the pci-st.c pci driver found on STMicroelectronics
SoC's into the STI arch section of the maintainers file.

Signed-off-by: Gabriel Fernandez <gabriel.fernandez@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index a0dadde..83ed2c3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1455,6 +1455,7 @@ F:	drivers/clocksource/arm_global_timer.c
 F:	drivers/i2c/busses/i2c-st.c
 F:	drivers/media/rc/st_rc.c
 F:	drivers/mmc/host/sdhci-st.c
+F:	drivers/pci/host/pci-st.c
 F:	drivers/phy/phy-stih407-usb.c
 F:	drivers/phy/phy-stih41x-usb.c
 F:	drivers/pinctrl/pinctrl-st.c
-- 
1.9.1