[PATCH 0/8] i.MX6 PCIe binding change and MSI support

[PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Lucas Stach]

While working on MSI support for the i.MX6 PCIe host driver
it has been discovered that the binding for this host controller
is broken in many ways (refer to the patch descriptions for more
info) and was introduced without proper discussion about what
should/should not be in the binding.

This series fixes this and minimizes the difference of the
i.MX6 binding to the common designware PCIe binding. I'm aware
that this is a quite radical change, but I think it's justified
to do this as long as there aren't many user of the old binding
(most of the optional properties in the binding aren't even
implemented).

Looking forward to your feedback.

Lucas Stach (8):
  ARM: imx6q-clk: parent lvds_gate from lvds_sel
  PCI: designware: split Exynos and i.MX bindings
  ARM: dts: imx6: update pcie to bring in line with new binding
  PCI: imx6: use new clock names
  PCI: imx6: drop old irq mapping
  PCI: imx6: rip out optional (and unused) irqs
  PCI: designware: make MSI isr shared irq aware
  PCI: imx6: add support for MSI

 .../devicetree/bindings/pci/designware-pcie.txt    |  74 +----------
 .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     |  38 ++++++
 .../bindings/pci/samsung,exynos5440-pcie.txt       |  65 +++++++++
 arch/arm/boot/dts/imx6qdl.dtsi                     |   7 +-
 arch/arm/mach-imx/clk-imx6q.c                      |   4 +-
 drivers/pci/host/pci-exynos.c                      |   4 +-
 drivers/pci/host/pci-imx6.c                        | 145 ++++++++-------------
 drivers/pci/host/pcie-designware.c                 |   6 +-
 drivers/pci/host/pcie-designware.h                 |   2 +-
 9 files changed, 176 insertions(+), 169 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
 create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt

-- 
1.9.0

[PATCH 1/8] ARM: imx6q-clk: parent lvds_gate from lvds_sel

[Lucas Stach]

Allows fror proper refcounting of the parent clocks
when enabling the clock output on CLK1/2 pads.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm/mach-imx/clk-imx6q.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index af2e582d2b74..f98a6bb98145 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -208,8 +208,8 @@ static void __init imx6q_clocks_init(struct device_node *ccm_node)
 	 * the "output_enable" bit as a gate, even though it's really just
 	 * enabling clock output.
 	 */
-	clk[lvds1_gate] = imx_clk_gate("lvds1_gate", "dummy", base + 0x160, 10);
-	clk[lvds2_gate] = imx_clk_gate("lvds2_gate", "dummy", base + 0x160, 11);
+	clk[lvds1_gate] = imx_clk_gate("lvds1_gate", "lvds1_sel", base + 0x160, 10);
+	clk[lvds2_gate] = imx_clk_gate("lvds2_gate", "lvds2_sel", base + 0x160, 11);
 
 	/*                                name              parent_name        reg       idx */
 	clk[pll2_pfd0_352m] = imx_clk_pfd("pll2_pfd0_352m", "pll2_bus",     base + 0x100, 0);
-- 
1.9.0

Re: [PATCH 1/8] ARM: imx6q-clk: parent lvds_gate from lvds_sel

[Marek Vasut]

On Friday, March 28, 2014 at 05:52:52 PM, Lucas Stach wrote:
> Allows fror proper refcounting of the parent clocks
> when enabling the clock output on CLK1/2 pads.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Looks OK:

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

RE: [PATCH 1/8] ARM: imx6q-clk: parent lvds_gate from lvds_sel

[Hong-Xing.Zhu-KZfg59tc24xl57MIdRCFDg]

Hi Lucas:
Thanks for your re-structure the management of the output on CLK1/2 pads.


> -----Original Message-----
> From: linux-pci-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-pci-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org]
> On Behalf Of Lucas Stach
> Sent: Saturday, March 29, 2014 12:53 AM
> To: linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Sean Cross; Marek Vasut; Zhu Richard-R65037; Bjorn
> Helgaas; Jingoo Han; Shawn Guo; Ian Campbell; Mark Rutland; Pawel Moll; Rob
> Herring; Arnd Bergmann; Tim Harvey; kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org
> Subject: [PATCH 1/8] ARM: imx6q-clk: parent lvds_gate from lvds_sel
> 
> Allows fror proper refcounting of the parent clocks when enabling the clock
> output on CLK1/2 pads.
> 
> Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Acked-by: Richard Zhu <r65037-KZfg59tc24xl57MIdRCFDg@public.gmane.org>

> ---
>  arch/arm/mach-imx/clk-imx6q.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> index af2e582d2b74..f98a6bb98145 100644
> --- a/arch/arm/mach-imx/clk-imx6q.c
> +++ b/arch/arm/mach-imx/clk-imx6q.c
> @@ -208,8 +208,8 @@ static void __init imx6q_clocks_init(struct device_node
> *ccm_node)
>  	 * the "output_enable" bit as a gate, even though it's really just
>  	 * enabling clock output.
>  	 */
> -	clk[lvds1_gate] = imx_clk_gate("lvds1_gate", "dummy", base + 0x160, 10);
> -	clk[lvds2_gate] = imx_clk_gate("lvds2_gate", "dummy", base + 0x160, 11);
> +	clk[lvds1_gate] = imx_clk_gate("lvds1_gate", "lvds1_sel", base + 0x160,
> 10);
> +	clk[lvds2_gate] = imx_clk_gate("lvds2_gate", "lvds2_sel", base +
> +0x160, 11);
> 
>  	/*                                name              parent_name
> reg       idx */
>  	clk[pll2_pfd0_352m] = imx_clk_pfd("pll2_pfd0_352m", "pll2_bus",     base
> + 0x100, 0);
> --
> 1.9.0
> 
> --
> 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
> 


Best Regards
Richard Zhu


--
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

Re: [PATCH 1/8] ARM: imx6q-clk: parent lvds_gate from lvds_sel

[Shawn Guo]

On Fri, Mar 28, 2014 at 05:52:52PM +0100, Lucas Stach wrote:
> Allows fror proper refcounting of the parent clocks
> when enabling the clock output on CLK1/2 pads.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Applied, thanks.

[PATCH 2/8] PCI: designware: split Exynos and i.MX bindings

[Lucas Stach]

The glue around the core designware IP is significantly
different between the Exynos and i.MX implementation,
which is reflected in the DT bindings.

This changes the i.MX6 binding to reuse as much as
possible from the common designware binding and
removes old cruft.

I removed the optional GPIOs with the following reasoning:
- disable-gpio: endpoint specific GPIO, not currently
  wired up in any code. Should be handled by the PCI device
  driver, not the host controller driver.
- wake-up-gpio: same as above.
- power-on-gpio: No user in any upstream DT. This should
  be handled by a regulator which shouldn't be controlled
  by the host driver, but rather by the PCI device driver.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 .../devicetree/bindings/pci/designware-pcie.txt    | 74 ++--------------------
 .../devicetree/bindings/pci/fsl,imx6q-pcie.txt     | 38 +++++++++++
 .../bindings/pci/samsung,exynos5440-pcie.txt       | 65 +++++++++++++++++++
 3 files changed, 109 insertions(+), 68 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
 create mode 100644 Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index d6fae13ff062..228b37684305 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -1,15 +1,7 @@
 * Synopsys Designware PCIe interface
 
 Required properties:
-- compatible: should contain "snps,dw-pcie" to identify the
-	core, plus an identifier for the specific instance, such
-	as "samsung,exynos5440-pcie" or "fsl,imx6q-pcie".
-- reg: base addresses and lengths of the pcie controller,
-	the phy controller, additional register for the phy controller.
-- interrupts: interrupt values for level interrupt,
-	pulse interrupt, special interrupt.
-- clocks: from common clock binding: handle to pci clock.
-- clock-names: from common clock binding: should be "pcie" and "pcie_bus".
+- compatible: should contain "snps,dw-pcie" to identify the core.
 - #address-cells: set to <3>
 - #size-cells: set to <2>
 - device_type: set to "pci"
@@ -19,65 +11,11 @@ Required properties:
 	to define the mapping of the PCIe interface to interrupt
 	numbers.
 - num-lanes: number of lanes to use
+- clocks: Must contain an entry for each entry in clock-names.
+	See ../clocks/clock-bindings.txt for details.
+- clock-names: Must include the following entries: 
+	- "pcie"
+	- "pcie_bus"
 
 Optional properties:
 - reset-gpio: gpio pin number of power good signal
-
-Optional properties for fsl,imx6q-pcie
-- power-on-gpio: gpio pin number of power-enable signal
-- wake-up-gpio: gpio pin number of incoming wakeup signal
-- disable-gpio: gpio pin number of outgoing rfkill/endpoint disable signal
-
-Example:
-
-SoC specific DT Entry:
-
-	pcie@290000 {
-		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
-		reg = <0x290000 0x1000
-			0x270000 0x1000
-			0x271000 0x40>;
-		interrupts = <0 20 0>, <0 21 0>, <0 22 0>;
-		clocks = <&clock 28>, <&clock 27>;
-		clock-names = "pcie", "pcie_bus";
-		#address-cells = <3>;
-		#size-cells = <2>;
-		device_type = "pci";
-		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000   /* configuration space */
-			  0x81000000 0 0	  0x40001000 0 0x00010000   /* downstream I/O */
-			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>; /* non-prefetchable memory */
-		#interrupt-cells = <1>;
-		interrupt-map-mask = <0 0 0 0>;
-		interrupt-map = <0x0 0 &gic 53>;
-		num-lanes = <4>;
-	};
-
-	pcie@2a0000 {
-		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
-		reg = <0x2a0000 0x1000
-			0x272000 0x1000
-			0x271040 0x40>;
-		interrupts = <0 23 0>, <0 24 0>, <0 25 0>;
-		clocks = <&clock 29>, <&clock 27>;
-		clock-names = "pcie", "pcie_bus";
-		#address-cells = <3>;
-		#size-cells = <2>;
-		device_type = "pci";
-		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000   /* configuration space */
-			  0x81000000 0 0	  0x60001000 0 0x00010000   /* downstream I/O */
-			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>; /* non-prefetchable memory */
-		#interrupt-cells = <1>;
-		interrupt-map-mask = <0 0 0 0>;
-		interrupt-map = <0x0 0 &gic 56>;
-		num-lanes = <4>;
-	};
-
-Board specific DT Entry:
-
-	pcie@290000 {
-		reset-gpio = <&pin_ctrl 5 0>;
-	};
-
-	pcie@2a0000 {
-		reset-gpio = <&pin_ctrl 22 0>;
-	};
diff --git a/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
new file mode 100644
index 000000000000..cb61578bd346
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/fsl,imx6q-pcie.txt
@@ -0,0 +1,38 @@
+* Freescale i.MX6 PCIe interface
+
+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: "fsl,imx6q-pcie"
+- reg: base addresse and length of the pcie controller
+- interrupts: A list of interrupt outputs of the controller. Must contain an
+  entry for each entry in the interrupt-names property.
+- interrupt-names: Must include the following entries:
+	- "msi": The interrupt that is asserted when an MSI is received
+- clock-names: Must include the following additional entries: 
+	- "pcie_phy"
+
+Example:
+
+	pcie@0x01000000 {
+		compatible = "fsl,imx6q-pcie", "snps,dw-pcie";
+		reg = <0x01ffc000 0x4000>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x00000800 0 0x01f00000 0x01f00000 0 0x00080000
+			  0x81000000 0 0          0x01f80000 0 0x00010000
+			  0x82000000 0 0x01000000 0x01000000 0 0x00f00000>;
+		num-lanes = <1>;
+		interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+		interrupt-names = "msi";
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0x7>;
+		interrupt-map = <0 0 0 1 &intc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
+		                <0 0 0 2 &intc GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
+		                <0 0 0 3 &intc GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>,
+		                <0 0 0 4 &intc GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&clks 144>, <&clks 206>, <&clks 189>;
+		clock-names = "pcie", "pcie_bus", "pcie_phy";
+	};
diff --git a/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt b/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
new file mode 100644
index 000000000000..4f9d23d2ed67
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/samsung,exynos5440-pcie.txt
@@ -0,0 +1,65 @@
+* Samsung Exynos 5440 PCIe interface
+
+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: "samsung,exynos5440-pcie"
+- reg: base addresses and lengths of the pcie controller,
+	the phy controller, additional register for the phy controller.
+- interrupts: A list of interrupt outputs for level interrupt,
+	pulse interrupt, special interrupt.
+
+Example:
+
+SoC specific DT Entry:
+
+	pcie@290000 {
+		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
+		reg = <0x290000 0x1000
+			0x270000 0x1000
+			0x271000 0x40>;
+		interrupts = <0 20 0>, <0 21 0>, <0 22 0>;
+		clocks = <&clock 28>, <&clock 27>;
+		clock-names = "pcie", "pcie_bus";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x00000800 0 0x40000000 0x40000000 0 0x00001000   /* configuration space */
+			  0x81000000 0 0	  0x40001000 0 0x00010000   /* downstream I/O */
+			  0x82000000 0 0x40011000 0x40011000 0 0x1ffef000>; /* non-prefetchable memory */
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0 0 0 0 &gic GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+		num-lanes = <4>;
+	};
+
+	pcie@2a0000 {
+		compatible = "samsung,exynos5440-pcie", "snps,dw-pcie";
+		reg = <0x2a0000 0x1000
+			0x272000 0x1000
+			0x271040 0x40>;
+		interrupts = <0 23 0>, <0 24 0>, <0 25 0>;
+		clocks = <&clock 29>, <&clock 27>;
+		clock-names = "pcie", "pcie_bus";
+		#address-cells = <3>;
+		#size-cells = <2>;
+		device_type = "pci";
+		ranges = <0x00000800 0 0x60000000 0x60000000 0 0x00001000   /* configuration space */
+			  0x81000000 0 0	  0x60001000 0 0x00010000   /* downstream I/O */
+			  0x82000000 0 0x60011000 0x60011000 0 0x1ffef000>; /* non-prefetchable memory */
+		#interrupt-cells = <1>;
+		interrupt-map-mask = <0 0 0 0>;
+		interrupt-map = <0 0 0 0 &gic GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
+		num-lanes = <4>;
+	};
+
+Board specific DT Entry:
+
+	pcie@290000 {
+		reset-gpio = <&pin_ctrl 5 0>;
+	};
+
+	pcie@2a0000 {
+		reset-gpio = <&pin_ctrl 22 0>;
+	};
-- 
1.9.0

Re: [PATCH 2/8] PCI: designware: split Exynos and i.MX bindings

[Marek Vasut]

On Friday, March 28, 2014 at 05:52:53 PM, Lucas Stach wrote:
> The glue around the core designware IP is significantly
> different between the Exynos and i.MX implementation,
> which is reflected in the DT bindings.
> 
> This changes the i.MX6 binding to reuse as much as
> possible from the common designware binding and
> removes old cruft.
> 
> I removed the optional GPIOs with the following reasoning:
> - disable-gpio: endpoint specific GPIO, not currently
>   wired up in any code. Should be handled by the PCI device
>   driver, not the host controller driver.
> - wake-up-gpio: same as above.
> - power-on-gpio: No user in any upstream DT. This should
>   be handled by a regulator which shouldn't be controlled
>   by the host driver, but rather by the PCI device driver.

This power-on-gpio should indeed be handled by the regulator, but the regulator 
cannot be handled by the PCIe device driver. This power-on-gpio must be operated 
on per-slot basis if I understand it correctly, so it cannot be controlled by 
the host controller driver either.

The reason why this cannot be controlled by the device driver is that if the 
device is powered down, it won't be detected on the PCIe bus, thus it cannot 
enable the regulator which will power up the slot the device is sitting in.

[...]

btw. am I blind or do I just not see devicetree-discuss on CC ?

Best regards,
Marek Vasut

Re: [PATCH 2/8] PCI: designware: split Exynos and i.MX bindings

[Lucas Stach]

Am Sonntag, den 30.03.2014, 19:36 +0200 schrieb Marek Vasut:
> On Friday, March 28, 2014 at 05:52:53 PM, Lucas Stach wrote:
> > The glue around the core designware IP is significantly
> > different between the Exynos and i.MX implementation,
> > which is reflected in the DT bindings.
> > 
> > This changes the i.MX6 binding to reuse as much as
> > possible from the common designware binding and
> > removes old cruft.
> > 
> > I removed the optional GPIOs with the following reasoning:
> > - disable-gpio: endpoint specific GPIO, not currently
> >   wired up in any code. Should be handled by the PCI device
> >   driver, not the host controller driver.
> > - wake-up-gpio: same as above.
> > - power-on-gpio: No user in any upstream DT. This should
> >   be handled by a regulator which shouldn't be controlled
> >   by the host driver, but rather by the PCI device driver.
> 
> This power-on-gpio should indeed be handled by the regulator, but the regulator 
> cannot be handled by the PCIe device driver. This power-on-gpio must be operated 
> on per-slot basis if I understand it correctly, so it cannot be controlled by 
> the host controller driver either.
> 
> The reason why this cannot be controlled by the device driver is that if the 
> device is powered down, it won't be detected on the PCIe bus, thus it cannot 
> enable the regulator which will power up the slot the device is sitting in.
> 
So we are on the same page with regard to a GPIO being the wrong
abstraction for this, I think.

For the regulator part I would argue that PCI is a bus that is built
around the ability to inspect the bus and detect devices on the bus at
probe time, so any regulator that's powering a PCI device should be
boot-on.

Only after the device driver is loaded it should be able to fetch the
regulator to power down/up the device as it wishes. In the x86 world
this is AFAIK done using ACPI methods.

I think the host controller driver has no business in controlling the
device power, more so as there possibly could be a lot of devices on the
bus even with a single host lane.

> 
> btw. am I blind or do I just not see devicetree-discuss on CC ?
> 
Hm, there is devicetree@vger.kernel.org on CC, which MAINTAINER says is
the right list for this stuff.

Regards,
Lucas

-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/8] PCI: designware: split Exynos and i.MX bindings

[Marek Vasut]

On Monday, March 31, 2014 at 11:28:29 AM, Lucas Stach wrote:
> Am Sonntag, den 30.03.2014, 19:36 +0200 schrieb Marek Vasut:
> > On Friday, March 28, 2014 at 05:52:53 PM, Lucas Stach wrote:
> > > The glue around the core designware IP is significantly
> > > different between the Exynos and i.MX implementation,
> > > which is reflected in the DT bindings.
> > > 
> > > This changes the i.MX6 binding to reuse as much as
> > > possible from the common designware binding and
> > > removes old cruft.
> > > 
> > > I removed the optional GPIOs with the following reasoning:
> > > - disable-gpio: endpoint specific GPIO, not currently
> > > 
> > >   wired up in any code. Should be handled by the PCI device
> > >   driver, not the host controller driver.
> > > 
> > > - wake-up-gpio: same as above.
> > > - power-on-gpio: No user in any upstream DT. This should
> > > 
> > >   be handled by a regulator which shouldn't be controlled
> > >   by the host driver, but rather by the PCI device driver.
> > 
> > This power-on-gpio should indeed be handled by the regulator, but the
> > regulator cannot be handled by the PCIe device driver. This
> > power-on-gpio must be operated on per-slot basis if I understand it
> > correctly, so it cannot be controlled by the host controller driver
> > either.
> > 
> > The reason why this cannot be controlled by the device driver is that if
> > the device is powered down, it won't be detected on the PCIe bus, thus
> > it cannot enable the regulator which will power up the slot the device
> > is sitting in.
> 
> So we are on the same page with regard to a GPIO being the wrong
> abstraction for this, I think.

Yes.

> For the regulator part I would argue that PCI is a bus that is built
> around the ability to inspect the bus and detect devices on the bus at
> probe time, so any regulator that's powering a PCI device should be
> boot-on.

This thing about regulator being boot-on should really be documented.

Moreover, I think it's a waste of power to keep the devices ON on boot even if 
the PCIe bus was not started (yet). The bus might not be started at all and the 
regulators would still be ON, which would be quite a waste.

> Only after the device driver is loaded it should be able to fetch the
> regulator to power down/up the device as it wishes. In the x86 world
> this is AFAIK done using ACPI methods.
> 
> I think the host controller driver has no business in controlling the
> device power, more so as there possibly could be a lot of devices on the
> bus even with a single host lane.

The power should be controlled per-slot, but I don't know how to model that. 
Note that there might be a PCIe device with a switch popped into a single slot, 
which makes things much more interesting. In such case, you need to power up the 
slot and neither of the downstream devices should control the power regulator of 
that slot.

> > btw. am I blind or do I just not see devicetree-discuss on CC ?
> 
> Hm, there is devicetree@vger.kernel.org on CC, which MAINTAINER says is
> the right list for this stuff.

OK, I was blind, sorry.

Best regards,
Marek Vasut

Re: [PATCH 2/8] PCI: designware: split Exynos and i.MX bindings

[Lucas Stach]

Am Montag, den 31.03.2014, 11:36 +0200 schrieb Marek Vasut:
> On Monday, March 31, 2014 at 11:28:29 AM, Lucas Stach wrote:
> > Am Sonntag, den 30.03.2014, 19:36 +0200 schrieb Marek Vasut:
> > > On Friday, March 28, 2014 at 05:52:53 PM, Lucas Stach wrote:
> > > > The glue around the core designware IP is significantly
> > > > different between the Exynos and i.MX implementation,
> > > > which is reflected in the DT bindings.
> > > > 
> > > > This changes the i.MX6 binding to reuse as much as
> > > > possible from the common designware binding and
> > > > removes old cruft.
> > > > 
> > > > I removed the optional GPIOs with the following reasoning:
> > > > - disable-gpio: endpoint specific GPIO, not currently
> > > > 
> > > >   wired up in any code. Should be handled by the PCI device
> > > >   driver, not the host controller driver.
> > > > 
> > > > - wake-up-gpio: same as above.
> > > > - power-on-gpio: No user in any upstream DT. This should
> > > > 
> > > >   be handled by a regulator which shouldn't be controlled
> > > >   by the host driver, but rather by the PCI device driver.
> > > 
> > > This power-on-gpio should indeed be handled by the regulator, but the
> > > regulator cannot be handled by the PCIe device driver. This
> > > power-on-gpio must be operated on per-slot basis if I understand it
> > > correctly, so it cannot be controlled by the host controller driver
> > > either.
> > > 
> > > The reason why this cannot be controlled by the device driver is that if
> > > the device is powered down, it won't be detected on the PCIe bus, thus
> > > it cannot enable the regulator which will power up the slot the device
> > > is sitting in.
> > 
> > So we are on the same page with regard to a GPIO being the wrong
> > abstraction for this, I think.
> 
> Yes.
> 
> > For the regulator part I would argue that PCI is a bus that is built
> > around the ability to inspect the bus and detect devices on the bus at
> > probe time, so any regulator that's powering a PCI device should be
> > boot-on.
> 
> This thing about regulator being boot-on should really be documented.
> 
> Moreover, I think it's a waste of power to keep the devices ON on boot even if 
> the PCIe bus was not started (yet). The bus might not be started at all and the 
> regulators would still be ON, which would be quite a waste.
> 

It's the exact same behavior that you get on x86: all devices are
powered after boot, once you loaded a device driver it may choose to
turn the device off. I don't think it makes sense to deviate here for
the sake of being embedded and special.

> > Only after the device driver is loaded it should be able to fetch the
> > regulator to power down/up the device as it wishes. In the x86 world
> > this is AFAIK done using ACPI methods.
> > 
> > I think the host controller driver has no business in controlling the
> > device power, more so as there possibly could be a lot of devices on the
> > bus even with a single host lane.
> 
> The power should be controlled per-slot, but I don't know how to model that. 
> Note that there might be a PCIe device with a switch popped into a single slot, 
> which makes things much more interesting. In such case, you need to power up the 
> slot and neither of the downstream devices should control the power regulator of 
> that slot.
> 
We could just add the regulator to the PCI hierarchy in the DT and
handle it similar to IRQs where we just walk up the DT starting from the
device until we find the matching IRQ/regulator.

Still for this to work with a regulator shared between several devices,
all the device drivers have to be aware of the regulator. Otherwise a
single device may choose to power down and erroneously cut power to a
sibling device, where the driver hasn't requested the regulator.

In all those scenarios the host controller driver still would not have
any business dealing with the regulator. Same situation as with the
legacy IRQs, that aren't handled by the host driver, but just routed to
the right instance through the DT.

Regards,
Lucas

-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/8] PCI: designware: split Exynos and i.MX bindings

[Marek Vasut]

On Monday, March 31, 2014 at 12:38:01 PM, Lucas Stach wrote:
> Am Montag, den 31.03.2014, 11:36 +0200 schrieb Marek Vasut:
> > On Monday, March 31, 2014 at 11:28:29 AM, Lucas Stach wrote:
> > > Am Sonntag, den 30.03.2014, 19:36 +0200 schrieb Marek Vasut:
> > > > On Friday, March 28, 2014 at 05:52:53 PM, Lucas Stach wrote:
> > > > > The glue around the core designware IP is significantly
> > > > > different between the Exynos and i.MX implementation,
> > > > > which is reflected in the DT bindings.
> > > > > 
> > > > > This changes the i.MX6 binding to reuse as much as
> > > > > possible from the common designware binding and
> > > > > removes old cruft.
> > > > > 
> > > > > I removed the optional GPIOs with the following reasoning:
> > > > > - disable-gpio: endpoint specific GPIO, not currently
> > > > > 
> > > > >   wired up in any code. Should be handled by the PCI device
> > > > >   driver, not the host controller driver.
> > > > > 
> > > > > - wake-up-gpio: same as above.
> > > > > - power-on-gpio: No user in any upstream DT. This should
> > > > > 
> > > > >   be handled by a regulator which shouldn't be controlled
> > > > >   by the host driver, but rather by the PCI device driver.
> > > > 
> > > > This power-on-gpio should indeed be handled by the regulator, but the
> > > > regulator cannot be handled by the PCIe device driver. This
> > > > power-on-gpio must be operated on per-slot basis if I understand it
> > > > correctly, so it cannot be controlled by the host controller driver
> > > > either.
> > > > 
> > > > The reason why this cannot be controlled by the device driver is that
> > > > if the device is powered down, it won't be detected on the PCIe bus,
> > > > thus it cannot enable the regulator which will power up the slot the
> > > > device is sitting in.
> > > 
> > > So we are on the same page with regard to a GPIO being the wrong
> > > abstraction for this, I think.
> > 
> > Yes.
> > 
> > > For the regulator part I would argue that PCI is a bus that is built
> > > around the ability to inspect the bus and detect devices on the bus at
> > > probe time, so any regulator that's powering a PCI device should be
> > > boot-on.
> > 
> > This thing about regulator being boot-on should really be documented.
> > 
> > Moreover, I think it's a waste of power to keep the devices ON on boot
> > even if the PCIe bus was not started (yet). The bus might not be started
> > at all and the regulators would still be ON, which would be quite a
> > waste.
> 
> It's the exact same behavior that you get on x86: all devices are
> powered after boot, once you loaded a device driver it may choose to
> turn the device off. I don't think it makes sense to deviate here for
> the sake of being embedded and special.
> 
> > > Only after the device driver is loaded it should be able to fetch the
> > > regulator to power down/up the device as it wishes. In the x86 world
> > > this is AFAIK done using ACPI methods.
> > > 
> > > I think the host controller driver has no business in controlling the
> > > device power, more so as there possibly could be a lot of devices on
> > > the bus even with a single host lane.
> > 
> > The power should be controlled per-slot, but I don't know how to model
> > that. Note that there might be a PCIe device with a switch popped into a
> > single slot, which makes things much more interesting. In such case, you
> > need to power up the slot and neither of the downstream devices should
> > control the power regulator of that slot.
> 
> We could just add the regulator to the PCI hierarchy in the DT and
> handle it similar to IRQs where we just walk up the DT starting from the
> device until we find the matching IRQ/regulator.
> 
> Still for this to work with a regulator shared between several devices,
> all the device drivers have to be aware of the regulator. Otherwise a
> single device may choose to power down and erroneously cut power to a
> sibling device, where the driver hasn't requested the regulator.
> 
> In all those scenarios the host controller driver still would not have
> any business dealing with the regulator. Same situation as with the
> legacy IRQs, that aren't handled by the host driver, but just routed to
> the right instance through the DT.

OK, it all makes sense. What I find a bit unfortunate is that we loose the Plug-
n-Play nature of the PCIe here, but I guess that cannot be helped.

[PATCH 3/8] ARM: dts: imx6: update pcie to bring in line with new binding

[Lucas Stach]

The new bindings drops one clock, renames the others and
drops the old interrupt mapping.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm/boot/dts/imx6qdl.dtsi | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index c1c06d25decc..1ea1adb6d8c6 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -128,15 +128,16 @@
 				  0x81000000 0 0          0x01f80000 0 0x00010000 /* downstream I/O */
 				  0x82000000 0 0x01000000 0x01000000 0 0x00f00000>; /* non-prefetchable memory */
 			num-lanes = <1>;
-			interrupts = <0 123 0x04>;
+			interrupts = <GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "msi";
 			#interrupt-cells = <1>;
 			interrupt-map-mask = <0 0 0 0x7>;
 			interrupt-map = <0 0 0 1 &intc GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>,
 			                <0 0 0 2 &intc GIC_SPI 122 IRQ_TYPE_LEVEL_HIGH>,
 			                <0 0 0 3 &intc GIC_SPI 121 IRQ_TYPE_LEVEL_HIGH>,
 			                <0 0 0 4 &intc GIC_SPI 120 IRQ_TYPE_LEVEL_HIGH>;
-			clocks = <&clks 189>, <&clks 187>, <&clks 206>, <&clks 144>;
-			clock-names = "pcie_ref_125m", "sata_ref_100m", "lvds_gate", "pcie_axi";
+			clocks = <&clks 144>, <&clks 206>, <&clks 189>;
+			clock-names = "pcie", "pcie_bus", "pcie_phy";
 			status = "disabled";
 		};
 
-- 
1.9.0

Re: [PATCH 3/8] ARM: dts: imx6: update pcie to bring in line with new binding

[Shawn Guo]

On Fri, Mar 28, 2014 at 05:52:54PM +0100, Lucas Stach wrote:
> The new bindings drops one clock, renames the others and
> drops the old interrupt mapping.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

It does not apply on my branch, and I had to apply it manually.  Please
check my imx/dt branch to see if I did it right.

Shawn

Re: [PATCH 3/8] ARM: dts: imx6: update pcie to bring in line with new binding

[Lucas Stach]

Am Mittwoch, den 30.04.2014, 14:02 +0800 schrieb Shawn Guo:
> On Fri, Mar 28, 2014 at 05:52:54PM +0100, Lucas Stach wrote:
> > The new bindings drops one clock, renames the others and
> > drops the old interrupt mapping.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> 
> It does not apply on my branch, and I had to apply it manually.  Please
> check my imx/dt branch to see if I did it right.
> 
The diff as applied in your imx/dt looks good to me.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

[PATCH 5/8] PCI: imx6: drop old irq mapping

[Lucas Stach]

We don't need this anymore. The irqs are now
properly mapped through the DT.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/pci/host/pci-imx6.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 71730bbcd9a2..259a73687526 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -483,12 +483,6 @@ static int imx6_add_pcie_port(struct pcie_port *pp,
 {
 	int ret;
 
-	pp->irq = platform_get_irq(pdev, 0);
-	if (!pp->irq) {
-		dev_err(&pdev->dev, "failed to get irq\n");
-		return -ENODEV;
-	}
-
 	pp->root_bus_nr = -1;
 	pp->ops = &imx6_pcie_host_ops;
 
-- 
1.9.0

[PATCH 6/8] PCI: imx6: rip out optional (and unused) irqs

[Lucas Stach]

They are dropped with the new binding.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/pci/host/pci-imx6.c | 42 ------------------------------------------
 1 file changed, 42 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 259a73687526..4c20fc4795e5 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -32,9 +32,6 @@
 
 struct imx6_pcie {
 	int			reset_gpio;
-	int			power_on_gpio;
-	int			wake_up_gpio;
-	int			disable_gpio;
 	struct clk		*pcie_bus;
 	struct clk		*pcie_phy;
 	struct clk		*pcie;
@@ -230,9 +227,6 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
 	int ret;
 
-	if (gpio_is_valid(imx6_pcie->power_on_gpio))
-		gpio_set_value(imx6_pcie->power_on_gpio, 1);
-
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_TEST_PD, 0 << 18);
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
@@ -531,42 +525,6 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 		}
 	}
 
-	imx6_pcie->power_on_gpio = of_get_named_gpio(np, "power-on-gpio", 0);
-	if (gpio_is_valid(imx6_pcie->power_on_gpio)) {
-		ret = devm_gpio_request_one(&pdev->dev,
-					imx6_pcie->power_on_gpio,
-					GPIOF_OUT_INIT_LOW,
-					"PCIe power enable");
-		if (ret) {
-			dev_err(&pdev->dev, "unable to get power-on gpio\n");
-			return ret;
-		}
-	}
-
-	imx6_pcie->wake_up_gpio = of_get_named_gpio(np, "wake-up-gpio", 0);
-	if (gpio_is_valid(imx6_pcie->wake_up_gpio)) {
-		ret = devm_gpio_request_one(&pdev->dev,
-					imx6_pcie->wake_up_gpio,
-					GPIOF_IN,
-					"PCIe wake up");
-		if (ret) {
-			dev_err(&pdev->dev, "unable to get wake-up gpio\n");
-			return ret;
-		}
-	}
-
-	imx6_pcie->disable_gpio = of_get_named_gpio(np, "disable-gpio", 0);
-	if (gpio_is_valid(imx6_pcie->disable_gpio)) {
-		ret = devm_gpio_request_one(&pdev->dev,
-					imx6_pcie->disable_gpio,
-					GPIOF_OUT_INIT_HIGH,
-					"PCIe disable endpoint");
-		if (ret) {
-			dev_err(&pdev->dev, "unable to get disable-ep gpio\n");
-			return ret;
-		}
-	}
-
 	/* Fetch clocks */
 	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
 	if (IS_ERR(imx6_pcie->pcie_phy)) {
-- 
1.9.0

[PATCH 4/8] PCI: imx6: use new clock names

[Lucas Stach]

As defined in the new binding.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/pci/host/pci-imx6.c | 74 ++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 45 deletions(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index ee082509b0ba..71730bbcd9a2 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -35,10 +35,9 @@ struct imx6_pcie {
 	int			power_on_gpio;
 	int			wake_up_gpio;
 	int			disable_gpio;
-	struct clk		*lvds_gate;
-	struct clk		*sata_ref_100m;
-	struct clk		*pcie_ref_125m;
-	struct clk		*pcie_axi;
+	struct clk		*pcie_bus;
+	struct clk		*pcie_phy;
+	struct clk		*pcie;
 	struct pcie_port	pp;
 	struct regmap		*iomuxc_gpr;
 	void __iomem		*mem_base;
@@ -239,28 +238,22 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 	regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR1,
 			IMX6Q_GPR1_PCIE_REF_CLK_EN, 1 << 16);
 
-	ret = clk_prepare_enable(imx6_pcie->sata_ref_100m);
+	ret = clk_prepare_enable(imx6_pcie->pcie_phy);
 	if (ret) {
-		dev_err(pp->dev, "unable to enable sata_ref_100m\n");
-		goto err_sata_ref;
+		dev_err(pp->dev, "unable to enable pcie_phy clock\n");
+		goto err_pcie_phy;
 	}
 
-	ret = clk_prepare_enable(imx6_pcie->pcie_ref_125m);
+	ret = clk_prepare_enable(imx6_pcie->pcie_bus);
 	if (ret) {
-		dev_err(pp->dev, "unable to enable pcie_ref_125m\n");
-		goto err_pcie_ref;
+		dev_err(pp->dev, "unable to enable pcie_bus clock\n");
+		goto err_pcie_bus;
 	}
 
-	ret = clk_prepare_enable(imx6_pcie->lvds_gate);
+	ret = clk_prepare_enable(imx6_pcie->pcie);
 	if (ret) {
-		dev_err(pp->dev, "unable to enable lvds_gate\n");
-		goto err_lvds_gate;
-	}
-
-	ret = clk_prepare_enable(imx6_pcie->pcie_axi);
-	if (ret) {
-		dev_err(pp->dev, "unable to enable pcie_axi\n");
-		goto err_pcie_axi;
+		dev_err(pp->dev, "unable to enable pcie clock\n");
+		goto err_pcie;
 	}
 
 	/* allow the clocks to stabilize */
@@ -274,13 +267,11 @@ static int imx6_pcie_deassert_core_reset(struct pcie_port *pp)
 	}
 	return 0;
 
-err_pcie_axi:
-	clk_disable_unprepare(imx6_pcie->lvds_gate);
-err_lvds_gate:
-	clk_disable_unprepare(imx6_pcie->pcie_ref_125m);
-err_pcie_ref:
-	clk_disable_unprepare(imx6_pcie->sata_ref_100m);
-err_sata_ref:
+err_pcie:
+	clk_disable_unprepare(imx6_pcie->pcie_bus);
+err_pcie_bus:
+	clk_disable_unprepare(imx6_pcie->pcie_phy);
+err_pcie_phy:
 	return ret;
 
 }
@@ -583,32 +574,25 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 	}
 
 	/* Fetch clocks */
-	imx6_pcie->lvds_gate = devm_clk_get(&pdev->dev, "lvds_gate");
-	if (IS_ERR(imx6_pcie->lvds_gate)) {
-		dev_err(&pdev->dev,
-			"lvds_gate clock select missing or invalid\n");
-		return PTR_ERR(imx6_pcie->lvds_gate);
-	}
-
-	imx6_pcie->sata_ref_100m = devm_clk_get(&pdev->dev, "sata_ref_100m");
-	if (IS_ERR(imx6_pcie->sata_ref_100m)) {
+	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
+	if (IS_ERR(imx6_pcie->pcie_phy)) {
 		dev_err(&pdev->dev,
-			"sata_ref_100m clock source missing or invalid\n");
-		return PTR_ERR(imx6_pcie->sata_ref_100m);
+			"pcie_phy clock source missing or invalid\n");
+		return PTR_ERR(imx6_pcie->pcie_phy);
 	}
 
-	imx6_pcie->pcie_ref_125m = devm_clk_get(&pdev->dev, "pcie_ref_125m");
-	if (IS_ERR(imx6_pcie->pcie_ref_125m)) {
+	imx6_pcie->pcie_bus = devm_clk_get(&pdev->dev, "pcie_bus");
+	if (IS_ERR(imx6_pcie->pcie_bus)) {
 		dev_err(&pdev->dev,
-			"pcie_ref_125m clock source missing or invalid\n");
-		return PTR_ERR(imx6_pcie->pcie_ref_125m);
+			"pcie_bus clock source missing or invalid\n");
+		return PTR_ERR(imx6_pcie->pcie_bus);
 	}
 
-	imx6_pcie->pcie_axi = devm_clk_get(&pdev->dev, "pcie_axi");
-	if (IS_ERR(imx6_pcie->pcie_axi)) {
+	imx6_pcie->pcie = devm_clk_get(&pdev->dev, "pcie");
+	if (IS_ERR(imx6_pcie->pcie)) {
 		dev_err(&pdev->dev,
-			"pcie_axi clock source missing or invalid\n");
-		return PTR_ERR(imx6_pcie->pcie_axi);
+			"pcie clock source missing or invalid\n");
+		return PTR_ERR(imx6_pcie->pcie);
 	}
 
 	/* Grab GPR config register range */
-- 
1.9.0

--
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

Re: [PATCH 4/8] PCI: imx6: use new clock names

[Shawn Guo]

On Fri, Mar 28, 2014 at 05:52:55PM +0100, Lucas Stach wrote:
> As defined in the new binding.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/pci/host/pci-imx6.c | 74 ++++++++++++++++++---------------------------
>  1 file changed, 29 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index ee082509b0ba..71730bbcd9a2 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
...
> @@ -583,32 +574,25 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
>  	}
>  
>  	/* Fetch clocks */
> -	imx6_pcie->lvds_gate = devm_clk_get(&pdev->dev, "lvds_gate");
> -	if (IS_ERR(imx6_pcie->lvds_gate)) {
> -		dev_err(&pdev->dev,
> -			"lvds_gate clock select missing or invalid\n");
> -		return PTR_ERR(imx6_pcie->lvds_gate);
> -	}
> -
> -	imx6_pcie->sata_ref_100m = devm_clk_get(&pdev->dev, "sata_ref_100m");
> -	if (IS_ERR(imx6_pcie->sata_ref_100m)) {
> +	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
> +	if (IS_ERR(imx6_pcie->pcie_phy)) {

Won't this break the existing DTBs?

Shawn

>  		dev_err(&pdev->dev,
> -			"sata_ref_100m clock source missing or invalid\n");
> -		return PTR_ERR(imx6_pcie->sata_ref_100m);
> +			"pcie_phy clock source missing or invalid\n");
> +		return PTR_ERR(imx6_pcie->pcie_phy);
>  	}
>  
> -	imx6_pcie->pcie_ref_125m = devm_clk_get(&pdev->dev, "pcie_ref_125m");
> -	if (IS_ERR(imx6_pcie->pcie_ref_125m)) {
> +	imx6_pcie->pcie_bus = devm_clk_get(&pdev->dev, "pcie_bus");
> +	if (IS_ERR(imx6_pcie->pcie_bus)) {
>  		dev_err(&pdev->dev,
> -			"pcie_ref_125m clock source missing or invalid\n");
> -		return PTR_ERR(imx6_pcie->pcie_ref_125m);
> +			"pcie_bus clock source missing or invalid\n");
> +		return PTR_ERR(imx6_pcie->pcie_bus);
>  	}
>  
> -	imx6_pcie->pcie_axi = devm_clk_get(&pdev->dev, "pcie_axi");
> -	if (IS_ERR(imx6_pcie->pcie_axi)) {
> +	imx6_pcie->pcie = devm_clk_get(&pdev->dev, "pcie");
> +	if (IS_ERR(imx6_pcie->pcie)) {
>  		dev_err(&pdev->dev,
> -			"pcie_axi clock source missing or invalid\n");
> -		return PTR_ERR(imx6_pcie->pcie_axi);
> +			"pcie clock source missing or invalid\n");
> +		return PTR_ERR(imx6_pcie->pcie);
>  	}
>  
>  	/* Grab GPR config register range */
> -- 
> 1.9.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/8] PCI: imx6: use new clock names

[Lucas Stach]

Hi Shawn,

Am Donnerstag, den 03.04.2014, 14:50 +0800 schrieb Shawn Guo:
> On Fri, Mar 28, 2014 at 05:52:55PM +0100, Lucas Stach wrote:
> > As defined in the new binding.
> > 
> > Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> >  drivers/pci/host/pci-imx6.c | 74 ++++++++++++++++++---------------------------
> >  1 file changed, 29 insertions(+), 45 deletions(-)
> > 
> > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > index ee082509b0ba..71730bbcd9a2 100644
> > --- a/drivers/pci/host/pci-imx6.c
> > +++ b/drivers/pci/host/pci-imx6.c
> ...
> > @@ -583,32 +574,25 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
> >  	}
> >  
> >  	/* Fetch clocks */
> > -	imx6_pcie->lvds_gate = devm_clk_get(&pdev->dev, "lvds_gate");
> > -	if (IS_ERR(imx6_pcie->lvds_gate)) {
> > -		dev_err(&pdev->dev,
> > -			"lvds_gate clock select missing or invalid\n");
> > -		return PTR_ERR(imx6_pcie->lvds_gate);
> > -	}
> > -
> > -	imx6_pcie->sata_ref_100m = devm_clk_get(&pdev->dev, "sata_ref_100m");
> > -	if (IS_ERR(imx6_pcie->sata_ref_100m)) {
> > +	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
> > +	if (IS_ERR(imx6_pcie->pcie_phy)) {
> 
> Won't this break the existing DTBs?
> 
Yes, it will. But I think it's justified to break the binding,
considering how much brokenness was in there and the relatively few
number of users right now.

All these patches depend on agreement of the new binding, as laid out in
patch 2/8. I don't think it makes much sense in keeping the old cruft
around given that the old binding was properly reviewed by PCI experts.

Regards,
Lucas
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

--
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

Re: [PATCH 4/8] PCI: imx6: use new clock names

[Lucas Stach]

Am Donnerstag, den 03.04.2014, 10:21 +0200 schrieb Lucas Stach:
> Hi Shawn,
> 
> Am Donnerstag, den 03.04.2014, 14:50 +0800 schrieb Shawn Guo:
> > On Fri, Mar 28, 2014 at 05:52:55PM +0100, Lucas Stach wrote:
> > > As defined in the new binding.
> > > 
> > > Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > > ---
> > >  drivers/pci/host/pci-imx6.c | 74 ++++++++++++++++++---------------------------
> > >  1 file changed, 29 insertions(+), 45 deletions(-)
> > > 
> > > diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> > > index ee082509b0ba..71730bbcd9a2 100644
> > > --- a/drivers/pci/host/pci-imx6.c
> > > +++ b/drivers/pci/host/pci-imx6.c
> > ...
> > > @@ -583,32 +574,25 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
> > >  	}
> > >  
> > >  	/* Fetch clocks */
> > > -	imx6_pcie->lvds_gate = devm_clk_get(&pdev->dev, "lvds_gate");
> > > -	if (IS_ERR(imx6_pcie->lvds_gate)) {
> > > -		dev_err(&pdev->dev,
> > > -			"lvds_gate clock select missing or invalid\n");
> > > -		return PTR_ERR(imx6_pcie->lvds_gate);
> > > -	}
> > > -
> > > -	imx6_pcie->sata_ref_100m = devm_clk_get(&pdev->dev, "sata_ref_100m");
> > > -	if (IS_ERR(imx6_pcie->sata_ref_100m)) {
> > > +	imx6_pcie->pcie_phy = devm_clk_get(&pdev->dev, "pcie_phy");
> > > +	if (IS_ERR(imx6_pcie->pcie_phy)) {
> > 
> > Won't this break the existing DTBs?
> > 
> Yes, it will. But I think it's justified to break the binding,
> considering how much brokenness was in there and the relatively few
> number of users right now.
> 
> All these patches depend on agreement of the new binding, as laid out in
> patch 2/8. I don't think it makes much sense in keeping the old cruft
> around given that the old binding was properly reviewed by PCI experts.
                                    ^ wasn't (obviously)
> 
> Regards,
> Lucas

-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

--
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

[PATCH 7/8] PCI: designware: make MSI isr shared irq aware

[Lucas Stach]

On i.MX6 the host controller MSI irq is shared
with PCI legacy INTD. Make sure we don't bail too
early from the irq handler.

The issue is fairly theoretical as it would require
a system setup with a PCIe switch where one connected
device is using legacy INTD and another one using
MSI, but better fix it now.

Signed-off-by: Lucas Stach <l.stach-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/pci/host/pci-exynos.c      | 4 +---
 drivers/pci/host/pcie-designware.c | 6 +++++-
 drivers/pci/host/pcie-designware.h | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
index 3de6bfbbe8e9..b616d34922d8 100644
--- a/drivers/pci/host/pci-exynos.c
+++ b/drivers/pci/host/pci-exynos.c
@@ -415,9 +415,7 @@ static irqreturn_t exynos_pcie_msi_irq_handler(int irq, void *arg)
 {
 	struct pcie_port *pp = arg;
 
-	dw_handle_msi_irq(pp);
-
-	return IRQ_HANDLED;
+	return dw_handle_msi_irq(pp);
 }
 
 static void exynos_pcie_msi_init(struct pcie_port *pp)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 98c118e04dba..cbce9b04b13d 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -156,15 +156,17 @@ static struct irq_chip dw_msi_irq_chip = {
 };
 
 /* MSI int handler */
-void dw_handle_msi_irq(struct pcie_port *pp)
+irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 {
 	unsigned long val;
 	int i, pos, irq;
+	irqreturn_t ret = IRQ_NONE;
 
 	for (i = 0; i < MAX_MSI_CTRLS; i++) {
 		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4,
 				(u32 *)&val);
 		if (val) {
+			ret = IRQ_HANDLED;
 			pos = 0;
 			while ((pos = find_next_bit(&val, 32, pos)) != 32) {
 				irq = irq_find_mapping(pp->irq_domain,
@@ -177,6 +179,8 @@ void dw_handle_msi_irq(struct pcie_port *pp)
 			}
 		}
 	}
+
+	return ret;
 }
 
 void dw_pcie_msi_init(struct pcie_port *pp)
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index 3063b3594d88..a169d22d517e 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -68,7 +68,7 @@ struct pcie_host_ops {
 
 int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val);
 int dw_pcie_cfg_write(void __iomem *addr, int where, int size, u32 val);
-void dw_handle_msi_irq(struct pcie_port *pp);
+irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
 void dw_pcie_msi_init(struct pcie_port *pp);
 int dw_pcie_link_up(struct pcie_port *pp);
 void dw_pcie_setup_rc(struct pcie_port *pp);
-- 
1.9.0

--
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

Re: [PATCH 7/8] PCI: designware: make MSI isr shared irq aware

[Jingoo Han]

On Saturday, March 29, 2014 1:53 AM, Lucas Stach wrote:
> 
> On i.MX6 the host controller MSI irq is shared
> with PCI legacy INTD. Make sure we don't bail too
> early from the irq handler.
> 
> The issue is fairly theoretical as it would require
> a system setup with a PCIe switch where one connected
> device is using legacy INTD and another one using
> MSI, but better fix it now.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

(+cc Mohit KUMAR DCG, Pratyush ANAND)

Acked-by: Jingoo Han <jg1.han@samsung.com>

Best regards,
Jingoo Han

> ---
>  drivers/pci/host/pci-exynos.c      | 4 +---
>  drivers/pci/host/pcie-designware.c | 6 +++++-
>  drivers/pci/host/pcie-designware.h | 2 +-
>  3 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
> index 3de6bfbbe8e9..b616d34922d8 100644
> --- a/drivers/pci/host/pci-exynos.c
> +++ b/drivers/pci/host/pci-exynos.c
> @@ -415,9 +415,7 @@ static irqreturn_t exynos_pcie_msi_irq_handler(int irq, void *arg)
>  {
>  	struct pcie_port *pp = arg;
> 
> -	dw_handle_msi_irq(pp);
> -
> -	return IRQ_HANDLED;
> +	return dw_handle_msi_irq(pp);
>  }
> 
>  static void exynos_pcie_msi_init(struct pcie_port *pp)
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 98c118e04dba..cbce9b04b13d 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -156,15 +156,17 @@ static struct irq_chip dw_msi_irq_chip = {
>  };
> 
>  /* MSI int handler */
> -void dw_handle_msi_irq(struct pcie_port *pp)
> +irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
>  {
>  	unsigned long val;
>  	int i, pos, irq;
> +	irqreturn_t ret = IRQ_NONE;
> 
>  	for (i = 0; i < MAX_MSI_CTRLS; i++) {
>  		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS + i * 12, 4,
>  				(u32 *)&val);
>  		if (val) {
> +			ret = IRQ_HANDLED;
>  			pos = 0;
>  			while ((pos = find_next_bit(&val, 32, pos)) != 32) {
>  				irq = irq_find_mapping(pp->irq_domain,
> @@ -177,6 +179,8 @@ void dw_handle_msi_irq(struct pcie_port *pp)
>  			}
>  		}
>  	}
> +
> +	return ret;
>  }
> 
>  void dw_pcie_msi_init(struct pcie_port *pp)
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index 3063b3594d88..a169d22d517e 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -68,7 +68,7 @@ struct pcie_host_ops {
> 
>  int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val);
>  int dw_pcie_cfg_write(void __iomem *addr, int where, int size, u32 val);
> -void dw_handle_msi_irq(struct pcie_port *pp);
> +irqreturn_t dw_handle_msi_irq(struct pcie_port *pp);
>  void dw_pcie_msi_init(struct pcie_port *pp);
>  int dw_pcie_link_up(struct pcie_port *pp);
>  void dw_pcie_setup_rc(struct pcie_port *pp);
> --
> 1.9.0

[PATCH 8/8] PCI: imx6: add support for MSI

[Lucas Stach]

This patch adds support for Message Signaled Interrupts in the
imx6-pcie driver.

Signed-off-by: Harro Haan <hrhaan@gmail.com>
Signed-off-by: Juergen Beisert <jbe@pengutronix.de>
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/pci/host/pci-imx6.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 4c20fc4795e5..41008b7804f9 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -25,6 +25,7 @@
 #include <linux/resource.h>
 #include <linux/signal.h>
 #include <linux/types.h>
+#include <linux/interrupt.h>
 
 #include "pcie-designware.h"
 
@@ -314,6 +315,13 @@ static int imx6_pcie_wait_for_link(struct pcie_port *pp)
 	return 0;
 }
 
+static irqreturn_t imx6_pcie_msi_handler(int irq, void *arg)
+{
+	struct pcie_port *pp = arg;
+
+	return dw_handle_msi_irq(pp);
+}
+
 static int imx6_pcie_start_link(struct pcie_port *pp)
 {
 	struct imx6_pcie *imx6_pcie = to_imx6_pcie(pp);
@@ -388,6 +396,9 @@ static void imx6_pcie_host_init(struct pcie_port *pp)
 	dw_pcie_setup_rc(pp);
 
 	imx6_pcie_start_link(pp);
+
+	if (IS_ENABLED(CONFIG_PCI_MSI))
+		dw_pcie_msi_init(pp);
 }
 
 static void imx6_pcie_reset_phy(struct pcie_port *pp)
@@ -477,6 +488,22 @@ static int imx6_add_pcie_port(struct pcie_port *pp,
 {
 	int 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,
+		                       imx6_pcie_msi_handler,
+		                       IRQF_SHARED, "mx6-pcie-msi", pp);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to request MSI irq\n");
+			return -ENODEV;
+		}
+	}
+
 	pp->root_bus_nr = -1;
 	pp->ops = &imx6_pcie_host_ops;
 
-- 
1.9.0

Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Marek Vasut]

On Friday, March 28, 2014 at 05:52:51 PM, Lucas Stach wrote:
> While working on MSI support for the i.MX6 PCIe host driver
> it has been discovered that the binding for this host controller
> is broken in many ways (refer to the patch descriptions for more
> info) and was introduced without proper discussion about what
> should/should not be in the binding.
> 
> This series fixes this and minimizes the difference of the
> i.MX6 binding to the common designware PCIe binding. I'm aware
> that this is a quite radical change, but I think it's justified
> to do this as long as there aren't many user of the old binding
> (most of the optional properties in the binding aren't even
> implemented).
> 
> Looking forward to your feedback.

Other but 2/8, which needs further discussion,

Reviewed-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Bjorn Helgaas]

On Fri, Mar 28, 2014 at 05:52:51PM +0100, Lucas Stach wrote:
> While working on MSI support for the i.MX6 PCIe host driver
> it has been discovered that the binding for this host controller
> is broken in many ways (refer to the patch descriptions for more
> info) and was introduced without proper discussion about what
> should/should not be in the binding.
> 
> This series fixes this and minimizes the difference of the
> i.MX6 binding to the common designware PCIe binding. I'm aware
> that this is a quite radical change, but I think it's justified
> to do this as long as there aren't many user of the old binding
> (most of the optional properties in the binding aren't even
> implemented).
> 
> Looking forward to your feedback.
> 
> Lucas Stach (8):
>   ARM: imx6q-clk: parent lvds_gate from lvds_sel
>   PCI: designware: split Exynos and i.MX bindings
>   ARM: dts: imx6: update pcie to bring in line with new binding
>   PCI: imx6: use new clock names
>   PCI: imx6: drop old irq mapping
>   PCI: imx6: rip out optional (and unused) irqs
>   PCI: designware: make MSI isr shared irq aware
>   PCI: imx6: add support for MSI

What's the status of all these?  I would normally apply patches 4-8 of this
series through my tree, given the appropriate acks, but I haven't seen
those yet.  And I'm not sure what dependencies there are between the
non-PCI patches and the PCI ones.

Bjorn

Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Lucas Stach]

Am Donnerstag, den 24.04.2014, 11:58 -0600 schrieb Bjorn Helgaas:
> On Fri, Mar 28, 2014 at 05:52:51PM +0100, Lucas Stach wrote:
> > While working on MSI support for the i.MX6 PCIe host driver
> > it has been discovered that the binding for this host controller
> > is broken in many ways (refer to the patch descriptions for more
> > info) and was introduced without proper discussion about what
> > should/should not be in the binding.
> > 
> > This series fixes this and minimizes the difference of the
> > i.MX6 binding to the common designware PCIe binding. I'm aware
> > that this is a quite radical change, but I think it's justified
> > to do this as long as there aren't many user of the old binding
> > (most of the optional properties in the binding aren't even
> > implemented).
> > 
> > Looking forward to your feedback.
> > 
> > Lucas Stach (8):
> >   ARM: imx6q-clk: parent lvds_gate from lvds_sel
This one is already applied.

> >   PCI: designware: split Exynos and i.MX bindings
> >   ARM: dts: imx6: update pcie to bring in line with new binding
> >   PCI: imx6: use new clock names
> >   PCI: imx6: drop old irq mapping
> >   PCI: imx6: rip out optional (and unused) irqs
> >   PCI: designware: make MSI isr shared irq aware
> >   PCI: imx6: add support for MSI
> 
> What's the status of all these?  I would normally apply patches 4-8 of this
> series through my tree, given the appropriate acks, but I haven't seen
> those yet.  And I'm not sure what dependencies there are between the
> non-PCI patches and the PCI ones.
> 
It's a complete binding change, so applying one part without the other
is going to horribly break things.

So I would at least want to see an ack for the binding change on the
i.MX side from Shawn and Richard. This will need some follow on patches,
as some boards adding PCIe using the old binding have cropped up in
linux-next, but I can do the patches on short notice if everyone agrees
to merge this patchset.

The designware part is pretty simple and doesn't change anything for
other users than i.MX. Though I would like to see an ack from Jingoo for
those.

I have some more stuff in the pipes regarding multiple MSI irqs, that
depend on this series and also the Keystone people are waiting for this
to be applied in order to consolidate the clock handling of the
designware core driver, so it would be nice to get this moving again.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Bjorn Helgaas]

On Fri, Apr 25, 2014 at 4:21 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Am Donnerstag, den 24.04.2014, 11:58 -0600 schrieb Bjorn Helgaas:
>> On Fri, Mar 28, 2014 at 05:52:51PM +0100, Lucas Stach wrote:
>> > While working on MSI support for the i.MX6 PCIe host driver
>> > it has been discovered that the binding for this host controller
>> > is broken in many ways (refer to the patch descriptions for more
>> > info) and was introduced without proper discussion about what
>> > should/should not be in the binding.
>> >
>> > This series fixes this and minimizes the difference of the
>> > i.MX6 binding to the common designware PCIe binding. I'm aware
>> > that this is a quite radical change, but I think it's justified
>> > to do this as long as there aren't many user of the old binding
>> > (most of the optional properties in the binding aren't even
>> > implemented).
>> >
>> > Looking forward to your feedback.
>> >
>> > Lucas Stach (8):
>> >   ARM: imx6q-clk: parent lvds_gate from lvds_sel
> This one is already applied.
>
>> >   PCI: designware: split Exynos and i.MX bindings
>> >   ARM: dts: imx6: update pcie to bring in line with new binding
>> >   PCI: imx6: use new clock names
>> >   PCI: imx6: drop old irq mapping
>> >   PCI: imx6: rip out optional (and unused) irqs
>> >   PCI: designware: make MSI isr shared irq aware
>> >   PCI: imx6: add support for MSI
>>
>> What's the status of all these?  I would normally apply patches 4-8 of this
>> series through my tree, given the appropriate acks, but I haven't seen
>> those yet.  And I'm not sure what dependencies there are between the
>> non-PCI patches and the PCI ones.
>>
> It's a complete binding change, so applying one part without the other
> is going to horribly break things.
>
> So I would at least want to see an ack for the binding change on the
> i.MX side from Shawn and Richard. This will need some follow on patches,
> as some boards adding PCIe using the old binding have cropped up in
> linux-next, but I can do the patches on short notice if everyone agrees
> to merge this patchset.
>
> The designware part is pretty simple and doesn't change anything for
> other users than i.MX. Though I would like to see an ack from Jingoo for
> those.
>
> I have some more stuff in the pipes regarding multiple MSI irqs, that
> depend on this series and also the Keystone people are waiting for this
> to be applied in order to consolidate the clock handling of the
> designware core driver, so it would be nice to get this moving again.

OK, just poke me again when you're ready for me to do something with these.

Bjorn

RE: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Hong-Xing.Zhu]

> -----Original Message-----
> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
> Sent: Friday, April 25, 2014 10:40 PM
> To: Lucas Stach
> Cc: Zhu Richard-R65037; Jingoo Han; Shawn Guo; linux-pci@vger.kernel.org;
> linux-arm; linux-samsung-soc@vger.kernel.org; devicetree@vger.kernel.org; Sean
> Cross; Marek Vasut; Ian Campbell; Mark Rutland; Pawel Moll; Rob Herring; Arnd
> Bergmann; Tim Harvey; kernel@pengutronix.de
> Subject: Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support
> 
> On Fri, Apr 25, 2014 at 4:21 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> > Am Donnerstag, den 24.04.2014, 11:58 -0600 schrieb Bjorn Helgaas:
> >> On Fri, Mar 28, 2014 at 05:52:51PM +0100, Lucas Stach wrote:
> >> > While working on MSI support for the i.MX6 PCIe host driver it has
> >> > been discovered that the binding for this host controller is broken
> >> > in many ways (refer to the patch descriptions for more
> >> > info) and was introduced without proper discussion about what
> >> > should/should not be in the binding.
> >> >
> >> > This series fixes this and minimizes the difference of the
> >> > i.MX6 binding to the common designware PCIe binding. I'm aware that
> >> > this is a quite radical change, but I think it's justified to do
> >> > this as long as there aren't many user of the old binding (most of
> >> > the optional properties in the binding aren't even implemented).
> >> >
> >> > Looking forward to your feedback.
> >> >
> >> > Lucas Stach (8):
> >> >   ARM: imx6q-clk: parent lvds_gate from lvds_sel
> > This one is already applied.
> >
> >> >   PCI: designware: split Exynos and i.MX bindings
> >> >   ARM: dts: imx6: update pcie to bring in line with new binding
> >> >   PCI: imx6: use new clock names
> >> >   PCI: imx6: drop old irq mapping
> >> >   PCI: imx6: rip out optional (and unused) irqs
> >> >   PCI: designware: make MSI isr shared irq aware
> >> >   PCI: imx6: add support for MSI
> >>
> >> What's the status of all these?  I would normally apply patches 4-8
> >> of this series through my tree, given the appropriate acks, but I
> >> haven't seen those yet.  And I'm not sure what dependencies there are
> >> between the non-PCI patches and the PCI ones.
> >>
> > It's a complete binding change, so applying one part without the other
> > is going to horribly break things.
> >
> > So I would at least want to see an ack for the binding change on the
> > i.MX side from Shawn and Richard. This will need some follow on
> > patches, as some boards adding PCIe using the old binding have cropped
> > up in linux-next, but I can do the patches on short notice if everyone
> > agrees to merge this patchset.
> >
> > The designware part is pretty simple and doesn't change anything for
> > other users than i.MX. Though I would like to see an ack from Jingoo
> > for those.
> >
> > I have some more stuff in the pipes regarding multiple MSI irqs, that
> > depend on this series and also the Keystone people are waiting for
> > this to be applied in order to consolidate the clock handling of the
> > designware core driver, so it would be nice to get this moving again.
[Richard] Hi Lucas, thanks. I'm ok for this patch-set.
Acked-by: Richard Zhu <r65037@freescale.com>

> 
> OK, just poke me again when you're ready for me to do something with these.
> 
> Bjorn
> 


Best Regards
Richard Zhu

Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Lucas Stach]

Hi Bjorn,

Am Freitag, den 25.04.2014, 08:39 -0600 schrieb Bjorn Helgaas:
[...]
> >> >   PCI: designware: split Exynos and i.MX bindings
> >> >   ARM: dts: imx6: update pcie to bring in line with new binding
> >> >   PCI: imx6: use new clock names
> >> >   PCI: imx6: drop old irq mapping
> >> >   PCI: imx6: rip out optional (and unused) irqs
> >> >   PCI: designware: make MSI isr shared irq aware
> >> >   PCI: imx6: add support for MSI
> >>
> >> What's the status of all these?  I would normally apply patches 4-8 of this
> >> series through my tree, given the appropriate acks, but I haven't seen
> >> those yet.  And I'm not sure what dependencies there are between the
> >> non-PCI patches and the PCI ones.
> >>
> > It's a complete binding change, so applying one part without the other
> > is going to horribly break things.
> >
> > So I would at least want to see an ack for the binding change on the
> > i.MX side from Shawn and Richard. This will need some follow on patches,
> > as some boards adding PCIe using the old binding have cropped up in
> > linux-next, but I can do the patches on short notice if everyone agrees
> > to merge this patchset.
> >
> > The designware part is pretty simple and doesn't change anything for
> > other users than i.MX. Though I would like to see an ack from Jingoo for
> > those.
> >
> > I have some more stuff in the pipes regarding multiple MSI irqs, that
> > depend on this series and also the Keystone people are waiting for this
> > to be applied in order to consolidate the clock handling of the
> > designware core driver, so it would be nice to get this moving again.
> 
> OK, just poke me again when you're ready for me to do something with these.
> 

As both Richard and Jingoo gave their ack for the respective patches I
think this is good to go in and I would expect all the PCI patches to go
through your tree for 3.16.

Shawn, if you are not okay with this change, please speak up now.
Otherwise please pick the dts change for 3.16. I'll go over linux-next
and prepare the patches to fix up the boards there.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Lucas Stach]

Hi Bjorn,

just a friendly reminder. It would be nice if you could pull those in.

Shawn already pulled the DT change and as it is a binding change this
means PCIe on i.MX6 is broken in -next, as long as the remaining patches
are missing.

Regards,
Lucas
 
Am Dienstag, den 29.04.2014, 14:31 +0200 schrieb Lucas Stach:
> Hi Bjorn,
> 
> Am Freitag, den 25.04.2014, 08:39 -0600 schrieb Bjorn Helgaas:
> [...]
> > >> >   PCI: designware: split Exynos and i.MX bindings
> > >> >   ARM: dts: imx6: update pcie to bring in line with new binding
> > >> >   PCI: imx6: use new clock names
> > >> >   PCI: imx6: drop old irq mapping
> > >> >   PCI: imx6: rip out optional (and unused) irqs
> > >> >   PCI: designware: make MSI isr shared irq aware
> > >> >   PCI: imx6: add support for MSI
> > >>
> > >> What's the status of all these?  I would normally apply patches 4-8 of this
> > >> series through my tree, given the appropriate acks, but I haven't seen
> > >> those yet.  And I'm not sure what dependencies there are between the
> > >> non-PCI patches and the PCI ones.
> > >>
> > > It's a complete binding change, so applying one part without the other
> > > is going to horribly break things.
> > >
> > > So I would at least want to see an ack for the binding change on the
> > > i.MX side from Shawn and Richard. This will need some follow on patches,
> > > as some boards adding PCIe using the old binding have cropped up in
> > > linux-next, but I can do the patches on short notice if everyone agrees
> > > to merge this patchset.
> > >
> > > The designware part is pretty simple and doesn't change anything for
> > > other users than i.MX. Though I would like to see an ack from Jingoo for
> > > those.
> > >
> > > I have some more stuff in the pipes regarding multiple MSI irqs, that
> > > depend on this series and also the Keystone people are waiting for this
> > > to be applied in order to consolidate the clock handling of the
> > > designware core driver, so it would be nice to get this moving again.
> > 
> > OK, just poke me again when you're ready for me to do something with these.
> > 
> 
> As both Richard and Jingoo gave their ack for the respective patches I
> think this is good to go in and I would expect all the PCI patches to go
> through your tree for 3.16.
> 
> Shawn, if you are not okay with this change, please speak up now.
> Otherwise please pick the dts change for 3.16. I'll go over linux-next
> and prepare the patches to fix up the boards there.
> 


-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Fabio Estevam]

Hi Bjorn,

On Mon, May 12, 2014 at 5:59 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Hi Bjorn,
>
> just a friendly reminder. It would be nice if you could pull those in.
>
> Shawn already pulled the DT change and as it is a binding change this
> means PCIe on i.MX6 is broken in -next, as long as the remaining patches
> are missing.

We still have imx6q pcie broken in linux-next:

imx6q-pcie 1ffc000.pcie: lvds_gate clock select missing or invalid
imx6q-pcie: probe of 1ffc000.pcie failed with error -2


Could you please apply Lucas' series?

Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Bjorn Helgaas]

On Mon, May 12, 2014 at 10:59:43AM +0200, Lucas Stach wrote:
> Hi Bjorn,
> 
> just a friendly reminder. It would be nice if you could pull those in.
> 
> Shawn already pulled the DT change and as it is a binding change this
> means PCIe on i.MX6 is broken in -next, as long as the remaining patches
> are missing.

I applied these to pci/host-imx6 for v3.16.

Sorry I didn't do this sooner; I get a lot of email and it's easy to
overlook things.  I mostly work from patchwork
(http://patchwork.ozlabs.org/project/linux-pci/list/?order=date), and I had
dropped this series from there because there was enough discussion that I
expected a re-post, so it wasn't on my radar screen anymore.

Bjorn

> Am Dienstag, den 29.04.2014, 14:31 +0200 schrieb Lucas Stach:
> > Hi Bjorn,
> > 
> > Am Freitag, den 25.04.2014, 08:39 -0600 schrieb Bjorn Helgaas:
> > [...]
> > > >> >   PCI: designware: split Exynos and i.MX bindings
> > > >> >   ARM: dts: imx6: update pcie to bring in line with new binding
> > > >> >   PCI: imx6: use new clock names
> > > >> >   PCI: imx6: drop old irq mapping
> > > >> >   PCI: imx6: rip out optional (and unused) irqs
> > > >> >   PCI: designware: make MSI isr shared irq aware
> > > >> >   PCI: imx6: add support for MSI
> > > >>
> > > >> What's the status of all these?  I would normally apply patches 4-8 of this
> > > >> series through my tree, given the appropriate acks, but I haven't seen
> > > >> those yet.  And I'm not sure what dependencies there are between the
> > > >> non-PCI patches and the PCI ones.
> > > >>
> > > > It's a complete binding change, so applying one part without the other
> > > > is going to horribly break things.
> > > >
> > > > So I would at least want to see an ack for the binding change on the
> > > > i.MX side from Shawn and Richard. This will need some follow on patches,
> > > > as some boards adding PCIe using the old binding have cropped up in
> > > > linux-next, but I can do the patches on short notice if everyone agrees
> > > > to merge this patchset.
> > > >
> > > > The designware part is pretty simple and doesn't change anything for
> > > > other users than i.MX. Though I would like to see an ack from Jingoo for
> > > > those.
> > > >
> > > > I have some more stuff in the pipes regarding multiple MSI irqs, that
> > > > depend on this series and also the Keystone people are waiting for this
> > > > to be applied in order to consolidate the clock handling of the
> > > > designware core driver, so it would be nice to get this moving again.
> > > 
> > > OK, just poke me again when you're ready for me to do something with these.
> > > 
> > 
> > As both Richard and Jingoo gave their ack for the respective patches I
> > think this is good to go in and I would expect all the PCI patches to go
> > through your tree for 3.16.
> > 
> > Shawn, if you are not okay with this change, please speak up now.
> > Otherwise please pick the dts change for 3.16. I'll go over linux-next
> > and prepare the patches to fix up the boards there.
> > 
> 
> 
> -- 
> Pengutronix e.K.             | Lucas Stach                 |
> Industrial Linux Solutions   | http://www.pengutronix.de/  |
> 

Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Lucas Stach]

Hello Bjorn,

Am Freitag, den 30.05.2014, 11:30 -0600 schrieb Bjorn Helgaas:
> On Mon, May 12, 2014 at 10:59:43AM +0200, Lucas Stach wrote:
> > Hi Bjorn,
> > 
> > just a friendly reminder. It would be nice if you could pull those in.
> > 
> > Shawn already pulled the DT change and as it is a binding change this
> > means PCIe on i.MX6 is broken in -next, as long as the remaining patches
> > are missing.
> 
> I applied these to pci/host-imx6 for v3.16.
> 
> Sorry I didn't do this sooner; I get a lot of email and it's easy to
> overlook things.  I mostly work from patchwork
> (http://patchwork.ozlabs.org/project/linux-pci/list/?order=date), and I had
> dropped this series from there because there was enough discussion that I
> expected a re-post, so it wasn't on my radar screen anymore.
> 
> Bjorn
> 
> > Am Dienstag, den 29.04.2014, 14:31 +0200 schrieb Lucas Stach:
> > > Hi Bjorn,
> > > 
> > > Am Freitag, den 25.04.2014, 08:39 -0600 schrieb Bjorn Helgaas:
> > > [...]
> > > > >> >   PCI: designware: split Exynos and i.MX bindings

It seems you missed this one patch. This isn't too urgent, as it is just
the doc update, but it should still go into 3.16 along with the other
changes. I would have expected that is ok for this to go through your
tree.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

--
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

Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

[Bjorn Helgaas]

On Tue, Jun 3, 2014 at 7:22 AM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Hello Bjorn,
>
> Am Freitag, den 30.05.2014, 11:30 -0600 schrieb Bjorn Helgaas:
>> On Mon, May 12, 2014 at 10:59:43AM +0200, Lucas Stach wrote:
>> > Hi Bjorn,
>> >
>> > just a friendly reminder. It would be nice if you could pull those in.
>> >
>> > Shawn already pulled the DT change and as it is a binding change this
>> > means PCIe on i.MX6 is broken in -next, as long as the remaining patches
>> > are missing.
>>
>> I applied these to pci/host-imx6 for v3.16.
>>
>> Sorry I didn't do this sooner; I get a lot of email and it's easy to
>> overlook things.  I mostly work from patchwork
>> (http://patchwork.ozlabs.org/project/linux-pci/list/?order=date), and I had
>> dropped this series from there because there was enough discussion that I
>> expected a re-post, so it wasn't on my radar screen anymore.
>>
>> Bjorn
>>
>> > Am Dienstag, den 29.04.2014, 14:31 +0200 schrieb Lucas Stach:
>> > > Hi Bjorn,
>> > >
>> > > Am Freitag, den 25.04.2014, 08:39 -0600 schrieb Bjorn Helgaas:
>> > > [...]
>> > > > >> >   PCI: designware: split Exynos and i.MX bindings
>
> It seems you missed this one patch. This isn't too urgent, as it is just
> the doc update, but it should still go into 3.16 along with the other
> changes. I would have expected that is ok for this to go through your
> tree.

Oops, sorry.  I had mentioned patches 4-8 earlier in the conversation,
so that's what I merged.

I added patch 2 to pci/host-imx6, and I'll ask Linus to pull it later
along with some other things I forgot.  Thanks for the reminder!

Bjorn