i.MX6 PCIe binding change and MSI support

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

Signed-off-by Lucas Stach <[email protected]>

-----Original Message-----
From: linux-pci-owner at vger.kernel.org [mailto:linux-pci-owner at vger.kernel.org] On Behalf Of Lucas Stach
Sent: Saturday, March 29, 2014 12:53 AM
To: linux-pci at vger.kernel.org
Cc: linux-arm-kernel at lists.infradead.org; linux-samsung-soc at vger.kernel.org; devicetree at vger.kernel.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 at pengutronix.de
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 <[email protected]>

---
 arch/arm/mach-imx/clk-imx6q.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/mach-imx/clk-imx6q.c +3 -2

index af2e582d2b74..f98a6bb98145 100644
@@ -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 at vger.kernel.org More majordomo info at
http://vger.kernel.org/majordomo-info.html

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

Signed-off-by Lucas Stach <[email protected]>

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.

Quoted 8 lines

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.

Quoted 8 lines

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 ?

Am Sonntag, den 30.03.2014, 19:36 +0200 schrieb Marek Vasut:

Quoted 27 lines · Lucas Stach

Quoted 11 lines

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

Quoted 8 lines

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.

Quoted 4 lines

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.

Quoted 1 line

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

Hm, there is devicetree at vger.kernel.org on CC, which MAINTAINER says is the right list for this stuff.

Quoted 28 lines

Am Sonntag, den 30.03.2014, 19:36 +0200 schrieb Marek Vasut:

Quoted 27 lines · Lucas Stach

Quoted 11 lines

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

Quoted 8 lines

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.

Quoted 7 lines

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 7 lines

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.

Am Montag, den 31.03.2014, 11:36 +0200 schrieb Marek Vasut:

Quoted 44 lines · Lucas Stach

Quoted 28 lines

Am Sonntag, den 30.03.2014, 19:36 +0200 schrieb Marek Vasut:

Quoted 27 lines · Lucas Stach

Quoted 11 lines

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

Quoted 8 lines

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.

Quoted 7 lines

Quoted 4 lines

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.

Quoted 4 lines

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.

Quoted 6 lines

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.

Quoted 8 lines

Quoted 7 lines

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.

Quoted 5 lines

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.

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

Signed-off-by Lucas Stach <[email protected]>

Quoted 4 lines

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

Signed-off-by Lucas Stach <[email protected]>

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.

As defined in the new binding.

Signed-off-by Lucas Stach <[email protected]>

---
 drivers/pci/host/pci-imx6.c | 74 ++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 45 deletions(-)

drivers/pci/host/pci-imx6.c +0 -0

index ee082509b0ba..71730bbcd9a2 100644

drivers/pci/host/pci-imx6.c +0 -1

@@ -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");

drivers/pci/host/pci-imx6.c +10 -10

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

Signature · 6 lines

1.9.0

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

Quoted 11 lines

As defined in the new binding.

Signed-off-by Lucas Stach <[email protected]>

---
 drivers/pci/host/pci-imx6.c | 74 ++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 45 deletions(-)

drivers/pci/host/pci-imx6.c +0 -0

index ee082509b0ba..71730bbcd9a2 100644

...

Quoted 3 lines

drivers/pci/host/pci-imx6.c +0 -0

@@ -583,32 +574,25 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 	}
 

drivers/pci/host/pci-imx6.c static int __init imx6_pcie_probe(struct platform_device *pdev) · line 578

Quoted 12 lines

drivers/pci/host/pci-imx6.c +2 -9

 	/* 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?

Hi Shawn,

Am Donnerstag, den 03.04.2014, 14:50 +0800 schrieb Shawn Guo:

Quoted 16 lines · Lucas Stach

Quoted 11 lines

As defined in the new binding.

Signed-off-by Lucas Stach <[email protected]>

---
 drivers/pci/host/pci-imx6.c | 74 ++++++++++++++++++---------------------------
 1 file changed, 29 insertions(+), 45 deletions(-)

drivers/pci/host/pci-imx6.c +0 -0

index ee082509b0ba..71730bbcd9a2 100644

...

Quoted 3 lines

drivers/pci/host/pci-imx6.c +0 -0

@@ -583,32 +574,25 @@ static int __init imx6_pcie_probe(struct platform_device *pdev)
 	}
 

Quoted 15 lines

drivers/pci/host/pci-imx6.c static int __init imx6_pcie_probe(struct platform_device *pdev) · line 578

Quoted 12 lines

drivers/pci/host/pci-imx6.c +2 -9

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

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 <[email protected]>

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

drivers/pci/host/pci-exynos.c +1 -3

index 3de6bfbbe8e9..b616d34922d8 100644
@@ -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)

drivers/pci/host/pcie-designware.c +5 -1

index 98c118e04dba..cbce9b04b13d 100644
@@ -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)

drivers/pci/host/pcie-designware.h +1 -2

index 3063b3594d88..a169d22d517e 100644
@@ -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

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.

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

Quoted 13 lines

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

Quoted 4 lines

Looking forward to your feedback.

Lucas Stach (8):
ARM: imx6q-clk: parent lvds_gate from lvds_sel

Quoted 7 lines

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.

Am Donnerstag, den 24.04.2014, 11:58 -0600 schrieb Bjorn Helgaas:

Quoted 14 lines · Lucas Stach

Quoted 13 lines

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

Quoted 4 lines

Quoted 4 lines

Looking forward to your feedback.

Lucas Stach (8):
ARM: imx6q-clk: parent lvds_gate from lvds_sel

This one is already applied.

Quoted 8 lines

Quoted 7 lines

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

Quoted 5 lines

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.

-----Original Message-----
From: Bjorn Helgaas [mailto:bhelgaas at google.com]
Sent: Friday, April 25, 2014 10:40 PM
To: Lucas Stach
Cc: Zhu Richard-R65037; Jingoo Han; Shawn Guo; linux-pci at vger.kernel.org; linux-arm; linux-samsung-soc at vger.kernel.org; devicetree at vger.kernel.org; Sean Cross; Marek Vasut; Ian Campbell; Mark Rutland; Pawel Moll; Rob Herring; Arnd Bergmann; Tim Harvey; kernel at pengutronix.de
Subject: Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support

Quoted 50 lines · Lucas Stach

Am Donnerstag, den 24.04.2014, 11:58 -0600 schrieb Bjorn Helgaas:

Quoted 13 lines · Lucas Stach

Quoted 12 lines

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

Quoted 4 lines

Quoted 4 lines

Looking forward to your feedback.

Lucas Stach (8):
ARM: imx6q-clk: parent lvds_gate from lvds_sel

This one is already applied.

Quoted 8 lines

Quoted 7 lines

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

Quoted 5 lines

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

Quoted 26 lines

Quoted 8 lines

Quoted 7 lines

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

Quoted 5 lines

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.

Quoted 4 lines

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.

Hi Bjorn,

Am Freitag, den 25.04.2014, 08:39 -0600 schrieb Bjorn Helgaas:
[...]

Quoted 26 lines

Quoted 26 lines

Quoted 8 lines

Quoted 7 lines

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

Quoted 5 lines

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.

Quoted 7 lines

Quoted 4 lines

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.

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.

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.

Am Dienstag, den 29.04.2014, 14:31 +0200 schrieb Lucas Stach:

Quoted 46 lines

Hi Bjorn,

Am Freitag, den 25.04.2014, 08:39 -0600 schrieb Bjorn Helgaas:
[...]

Quoted 26 lines

Quoted 26 lines

Quoted 8 lines

Quoted 7 lines

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

Quoted 5 lines

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.

Quoted 7 lines

Quoted 4 lines

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.

Signature · 2 lines

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

Quoted 7 lines

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

Quoted 6 lines

Am Dienstag, den 29.04.2014, 14:31 +0200 schrieb Lucas Stach:

Quoted 5 lines

Hi Bjorn,

Am Freitag, den 25.04.2014, 08:39 -0600 schrieb Bjorn Helgaas:
[...]

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

PCI: designware: split Exynos and i.MX bindings

Hello Bjorn,

Am Freitag, den 30.05.2014, 11:30 -0600 schrieb Bjorn Helgaas:

Quoted 22 lines · Lucas Stach

Quoted 7 lines

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

Quoted 3 lines

Am Dienstag, den 29.04.2014, 14:31 +0200 schrieb Lucas Stach:

Quoted 2 lines

Hi Bjorn,

Quoted 3 lines

Quoted 3 lines

Quoted 3 lines

Am Freitag, den 25.04.2014, 08:39 -0600 schrieb Bjorn Helgaas:
[...]

Quoted 1 line

Quoted 1 line

Quoted 1 line

Quoted 1 line

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.