public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH 0/4] ARM: mvebu: Enable XHCI on the A385-AP
@ 2015-01-06 15:45 Maxime Ripard
  2015-01-06 15:45 ` [PATCH 1/4] usb: phy: Fix deferred probing Maxime Ripard
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Maxime Ripard @ 2015-01-06 15:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hi all,

This serie enables the Armada 385 AP XHCI controller.

Since the controller uses a GPIO-controlled VBUS, we used the
phy-generic driver, and made the needed additions to the xhci-plat
driver to retrieve a USB phy.

Unfortunately, some glitches were also found along the way, mostly
because of the probe deferring that was introduced by this phy
retrieval.

First, the commit 1290a958d48e ("usb: phy: propagate
__of_usb_find_phy()'s error on failure") introduced in 3.19 broke the
deferred probing for consumer drivers. The first patch attempts at
fixing this while keeping the original intention of the author, and
should probably go in as a fix for 3.19.

Then, since the introduction of the Armada 38x support in 3.16, the
driver was attempting to write into registers while the clock wasn't
enabled yet. This was working because the bootloader left it enabled,
but in the case of a deferred probing, the clock would have been
disabled by the error path of our driver, and this would fail. This
should go in 3.19 as well, and any stable kernel for 3.16+.

The two patches remaining are "regular" patches, and are aimed at
3.20. The last patch depend on my previous serie to introduce support
for the the A385 AP board.

Thanks,
Maxime

Maxime Ripard (4):
  usb: phy: Fix deferred probing
  usb: XHCI: platform: Move the Marvell quirks after the enabling the
    clocks
  usb: xhci: plat: Add USB phy support
  ARM: mvebu: armada-385-ap: Enable USB3 port

 arch/arm/boot/dts/armada-385-ap.dts | 28 ++++++++++++++++++++++++++++
 drivers/usb/host/xhci-plat.c        | 32 ++++++++++++++++++++++----------
 drivers/usb/host/xhci.h             |  2 ++
 drivers/usb/phy/phy.c               |  6 +++---
 4 files changed, 55 insertions(+), 13 deletions(-)

-- 
2.2.1

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

* [PATCH 1/4] usb: phy: Fix deferred probing
  2015-01-06 15:45 [PATCH 0/4] ARM: mvebu: Enable XHCI on the A385-AP Maxime Ripard
@ 2015-01-06 15:45 ` Maxime Ripard
  2015-01-07  4:41   ` Olof Johansson
  2015-01-06 15:45 ` [PATCH 2/4] usb: XHCI: platform: Move the Marvell quirks after the enabling the clocks Maxime Ripard
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Maxime Ripard @ 2015-01-06 15:45 UTC (permalink / raw)
  To: linux-arm-kernel

Commit 1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s error on
failure") actually broke the deferred probing mechanism, since it now returns
EPROBE_DEFER only when the try_module_get call fails, but not when the phy
lookup does.

All the other similar functions seem to return ENODEV when try_module_get
fails, and the error code of either __usb_find_phy or __of_usb_find_phy
otherwise.

In order to have a consistent behaviour, and a meaningful EPROBE_DEFER, always
return EPROBE_DEFER when __(of_)usb_find_phy fails to look up the requested
phy, that will be propagated by the caller, and ENODEV if try_module_get fails.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/usb/phy/phy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index b4066a001ba0..353c686498d4 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -34,7 +34,7 @@ static struct usb_phy *__usb_find_phy(struct list_head *list,
 		return phy;
 	}
 
-	return ERR_PTR(-ENODEV);
+	return ERR_PTR(-EPROBE_DEFER);
 }
 
 static struct usb_phy *__usb_find_phy_dev(struct device *dev,
@@ -66,7 +66,7 @@ static struct usb_phy *__of_usb_find_phy(struct device_node *node)
 		return phy;
 	}
 
-	return ERR_PTR(-ENODEV);
+	return ERR_PTR(-EPROBE_DEFER);
 }
 
 static void devm_usb_phy_release(struct device *dev, void *res)
@@ -192,7 +192,7 @@ struct usb_phy *devm_usb_get_phy_by_phandle(struct device *dev,
 	phy = __of_usb_find_phy(node);
 	if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
 		if (!IS_ERR(phy))
-			phy = ERR_PTR(-EPROBE_DEFER);
+			phy = ERR_PTR(-ENODEV);
 
 		devres_free(ptr);
 		goto err1;
-- 
2.2.1

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

* [PATCH 2/4] usb: XHCI: platform: Move the Marvell quirks after the enabling the clocks
  2015-01-06 15:45 [PATCH 0/4] ARM: mvebu: Enable XHCI on the A385-AP Maxime Ripard
  2015-01-06 15:45 ` [PATCH 1/4] usb: phy: Fix deferred probing Maxime Ripard
@ 2015-01-06 15:45 ` Maxime Ripard
  2015-01-06 15:58   ` Thomas Petazzoni
  2015-01-06 15:45 ` [PATCH 3/4] usb: xhci: plat: Add USB phy support Maxime Ripard
  2015-01-06 15:45 ` [PATCH 4/4] ARM: mvebu: armada-385-ap: Enable USB3 port Maxime Ripard
  3 siblings, 1 reply; 12+ messages in thread
From: Maxime Ripard @ 2015-01-06 15:45 UTC (permalink / raw)
  To: linux-arm-kernel

The commit 973747928514 ("usb: host: xhci-plat: add support for the Armada
375/38x XHCI controllers") extended the xhci-plat driver to support the Armada
375/38x SoCs, mostly by adding a quirk configuring the MBUS window.

However, that quirk was run before the clock the controllers needs has been
enabled. This usually worked because the clock was first enabled by the
bootloader, and left as such until the driver is probe, where it tries to
access the MBUS configuration registers before enabling the clock.

Things get messy when EPROBE_DEFER is involved during the probe, since as part
of its error path, the driver will rightfully disable the clock. When the
driver will be reprobed, it will retry to access the MBUS registers, but this
time with the clock disabled, which hangs forever.

Fix this by running the quirks after the clock has been enabled by the driver.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Cc: <stable@vger.kernel.org> # v3.16+
---
 drivers/usb/host/xhci-plat.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 08d402b15482..34c2f6d3e5d0 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -83,16 +83,6 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return -ENODEV;
 
-
-	if (of_device_is_compatible(pdev->dev.of_node,
-				    "marvell,armada-375-xhci") ||
-	    of_device_is_compatible(pdev->dev.of_node,
-				    "marvell,armada-380-xhci")) {
-		ret = xhci_mvebu_mbus_init_quirk(pdev);
-		if (ret)
-			return ret;
-	}
-
 	/* Initialize dma_mask and coherent_dma_mask to 32-bits */
 	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
 	if (ret)
@@ -127,6 +117,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
 			goto put_hcd;
 	}
 
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "marvell,armada-375-xhci") ||
+	    of_device_is_compatible(pdev->dev.of_node,
+				    "marvell,armada-380-xhci")) {
+		ret = xhci_mvebu_mbus_init_quirk(pdev);
+		if (ret)
+			return ret;
+	}
+
 	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
 	if (ret)
 		goto disable_clk;
-- 
2.2.1

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

* [PATCH 3/4] usb: xhci: plat: Add USB phy support
  2015-01-06 15:45 [PATCH 0/4] ARM: mvebu: Enable XHCI on the A385-AP Maxime Ripard
  2015-01-06 15:45 ` [PATCH 1/4] usb: phy: Fix deferred probing Maxime Ripard
  2015-01-06 15:45 ` [PATCH 2/4] usb: XHCI: platform: Move the Marvell quirks after the enabling the clocks Maxime Ripard
@ 2015-01-06 15:45 ` Maxime Ripard
  2015-01-06 19:39   ` Sergei Shtylyov
  2015-01-06 15:45 ` [PATCH 4/4] ARM: mvebu: armada-385-ap: Enable USB3 port Maxime Ripard
  3 siblings, 1 reply; 12+ messages in thread
From: Maxime Ripard @ 2015-01-06 15:45 UTC (permalink / raw)
  To: linux-arm-kernel

The Marvell Armada 385 AP needs a dumb phy in order to enable the USB3 VBUS.

Add a call to retrieve a USB PHY to XHCI plat in order to support this.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/usb/host/xhci-plat.c | 13 +++++++++++++
 drivers/usb/host/xhci.h      |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 34c2f6d3e5d0..284ab70f13a3 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -16,6 +16,7 @@
 #include <linux/module.h>
 #include <linux/of.h>
 #include <linux/platform_device.h>
+#include <linux/usb/phy.h>
 #include <linux/slab.h>
 #include <linux/usb/xhci_pdriver.h>
 
@@ -155,6 +156,18 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
 		xhci->shared_hcd->can_do_streams = 1;
 
+	xhci->phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
+	if (IS_ERR(xhci->phy)) {
+		ret = PTR_ERR(xhci->phy);
+		if (ret == -EPROBE_DEFER)
+			goto put_usb3_hcd;
+		xhci->phy = NULL;
+	} else {
+		ret = usb_phy_init(xhci->phy);
+		if (ret)
+			goto put_usb3_hcd;
+	}
+
 	ret = usb_add_hcd(xhci->shared_hcd, irq, IRQF_SHARED);
 	if (ret)
 		goto put_usb3_hcd;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index cc7c5bb7cbcf..d9468af2a8e4 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1474,6 +1474,8 @@ struct xhci_hcd {
 	struct msix_entry	*msix_entries;
 	/* optional clock */
 	struct clk		*clk;
+	/* optional phy */
+	struct usb_phy		*phy;
 	/* data structures */
 	struct xhci_device_context_array *dcbaa;
 	struct xhci_ring	*cmd_ring;
-- 
2.2.1

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

* [PATCH 4/4] ARM: mvebu: armada-385-ap: Enable USB3 port
  2015-01-06 15:45 [PATCH 0/4] ARM: mvebu: Enable XHCI on the A385-AP Maxime Ripard
                   ` (2 preceding siblings ...)
  2015-01-06 15:45 ` [PATCH 3/4] usb: xhci: plat: Add USB phy support Maxime Ripard
@ 2015-01-06 15:45 ` Maxime Ripard
  3 siblings, 0 replies; 12+ messages in thread
From: Maxime Ripard @ 2015-01-06 15:45 UTC (permalink / raw)
  To: linux-arm-kernel

The Armada 385 AP board has a USB3 port exposed that uses a GPIO to drive the
VBUS line. Enable the needed drivers to support this.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/armada-385-ap.dts | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/arm/boot/dts/armada-385-ap.dts b/arch/arm/boot/dts/armada-385-ap.dts
index ab88dc4be636..a9dea82ef34b 100644
--- a/arch/arm/boot/dts/armada-385-ap.dts
+++ b/arch/arm/boot/dts/armada-385-ap.dts
@@ -88,6 +88,13 @@
 				status = "okay";
 			};
 
+			pinctrl at 18000 {
+				xhci0_vbus_pins: xhci0-vbus-pins {
+					marvell,pins = "mpp44";
+					marvell,function = "gpio";
+				};
+			};
+
 			ethernet at 30000 {
 				status = "okay";
 				phy = <&phy1>;
@@ -112,6 +119,11 @@
 				phy = <&phy0>;
 				phy-mode = "rgmii-id";
 			};
+
+			usb3 at f0000 {
+				status = "okay";
+				usb-phy = <&usb3_phy>;
+			};
 		};
 
 		pcie-controller {
@@ -137,4 +149,20 @@
 			};
 		};
 	};
+
+	usb3_phy: usb3_phy {
+		compatible = "usb-nop-xceiv";
+		vcc-supply = <&reg_xhci0_vbus>;
+	};
+
+	reg_xhci0_vbus: xhci0-vbus {
+		compatible = "regulator-fixed";
+		pinctrl-names = "default";
+		pinctrl-0 = <&xhci0_vbus_pins>;
+		regulator-name = "xhci0-vbus";
+		regulator-min-microvolt = <5000000>;
+		regulator-max-microvolt = <5000000>;
+		enable-active-high;
+		gpio = <&gpio1 12 GPIO_ACTIVE_HIGH>;
+	};
 };
-- 
2.2.1

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

* [PATCH 2/4] usb: XHCI: platform: Move the Marvell quirks after the enabling the clocks
  2015-01-06 15:45 ` [PATCH 2/4] usb: XHCI: platform: Move the Marvell quirks after the enabling the clocks Maxime Ripard
@ 2015-01-06 15:58   ` Thomas Petazzoni
  0 siblings, 0 replies; 12+ messages in thread
From: Thomas Petazzoni @ 2015-01-06 15:58 UTC (permalink / raw)
  To: linux-arm-kernel

Dear Maxime Ripard,

On Tue,  6 Jan 2015 16:45:08 +0100, Maxime Ripard wrote:

> +	if (of_device_is_compatible(pdev->dev.of_node,
> +				    "marvell,armada-375-xhci") ||
> +	    of_device_is_compatible(pdev->dev.of_node,
> +				    "marvell,armada-380-xhci")) {
> +		ret = xhci_mvebu_mbus_init_quirk(pdev);
> +		if (ret)
> +			return ret;
> +	}

So on error, you're leaking the struct usb_hcd now if I'm not mistaken.
When moving code around, the error handling should also be fixed. You
probably need "goto put_hcd;" instead of "return ret;".

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

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

* [PATCH 3/4] usb: xhci: plat: Add USB phy support
  2015-01-06 15:45 ` [PATCH 3/4] usb: xhci: plat: Add USB phy support Maxime Ripard
@ 2015-01-06 19:39   ` Sergei Shtylyov
  0 siblings, 0 replies; 12+ messages in thread
From: Sergei Shtylyov @ 2015-01-06 19:39 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

On 01/06/2015 06:45 PM, Maxime Ripard wrote:

> The Marvell Armada 385 AP needs a dumb phy in order to enable the USB3 VBUS.

> Add a call to retrieve a USB PHY to XHCI plat in order to support this.

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

[...]

> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index cc7c5bb7cbcf..d9468af2a8e4 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -1474,6 +1474,8 @@ struct xhci_hcd {
>   	struct msix_entry	*msix_entries;
>   	/* optional clock */
>   	struct clk		*clk;
> +	/* optional phy */
> +	struct usb_phy		*phy;

    Can't you reuse the one in 'struct usb_hcd'?

[...]

WBR, Sergei

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

* [PATCH 1/4] usb: phy: Fix deferred probing
  2015-01-06 15:45 ` [PATCH 1/4] usb: phy: Fix deferred probing Maxime Ripard
@ 2015-01-07  4:41   ` Olof Johansson
  2015-01-08 16:32     ` Grygorii.Strashko@linaro.org
  0 siblings, 1 reply; 12+ messages in thread
From: Olof Johansson @ 2015-01-07  4:41 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Tue, Jan 6, 2015 at 7:45 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Commit 1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s error on
> failure") actually broke the deferred probing mechanism, since it now returns
> EPROBE_DEFER only when the try_module_get call fails, but not when the phy
> lookup does.
>
> All the other similar functions seem to return ENODEV when try_module_get
> fails, and the error code of either __usb_find_phy or __of_usb_find_phy
> otherwise.
>
> In order to have a consistent behaviour, and a meaningful EPROBE_DEFER, always
> return EPROBE_DEFER when __(of_)usb_find_phy fails to look up the requested
> phy, that will be propagated by the caller, and ENODEV if try_module_get fails.
>

Fixes:  1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s error
on failure")

> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

This also fixes USB on tegra/trimslice here.

Tested-by: Olof Johansson <olof@lixom.net>

Given that the regression went in during 3.19 merge window, this
should go in as a fix during -rc, please.


-Olof

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

* [PATCH 1/4] usb: phy: Fix deferred probing
  2015-01-07  4:41   ` Olof Johansson
@ 2015-01-08 16:32     ` Grygorii.Strashko@linaro.org
  2015-01-08 17:24       ` Felipe Balbi
  0 siblings, 1 reply; 12+ messages in thread
From: Grygorii.Strashko@linaro.org @ 2015-01-08 16:32 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On 01/07/2015 06:41 AM, Olof Johansson wrote:
> On Tue, Jan 6, 2015 at 7:45 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
>> Commit 1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s error on
>> failure") actually broke the deferred probing mechanism, since it now returns
>> EPROBE_DEFER only when the try_module_get call fails, but not when the phy
>> lookup does.
>>
>> All the other similar functions seem to return ENODEV when try_module_get
>> fails, and the error code of either __usb_find_phy or __of_usb_find_phy
>> otherwise.
>>
>> In order to have a consistent behaviour, and a meaningful EPROBE_DEFER, always
>> return EPROBE_DEFER when __(of_)usb_find_phy fails to look up the requested
>> phy, that will be propagated by the caller, and ENODEV if try_module_get fails.
>>
> 
> Fixes:  1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s error
> on failure")
> 
>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> This also fixes USB on tegra/trimslice here.
> 
> Tested-by: Olof Johansson <olof@lixom.net>
> 
> Given that the regression went in during 3.19 merge window, this
> should go in as a fix during -rc, please.

1) Pls note, there is a duplication patch https://www.mail-archive.com/linux-usb at vger.kernel.org/msg53893.html
but this one is more correct as for me. 

2) This patch fixes USB regression on Keystone 2, so

 Tested-by: Grygorii Strashko <grygorii.strashko@linaro.org>

-- 
regards,
-grygorii

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

* [PATCH 1/4] usb: phy: Fix deferred probing
  2015-01-08 16:32     ` Grygorii.Strashko@linaro.org
@ 2015-01-08 17:24       ` Felipe Balbi
  2015-01-08 18:04         ` Thierry Reding
  0 siblings, 1 reply; 12+ messages in thread
From: Felipe Balbi @ 2015-01-08 17:24 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jan 08, 2015 at 06:32:27PM +0200, Grygorii.Strashko at linaro.org wrote:
> Hi,
> 
> On 01/07/2015 06:41 AM, Olof Johansson wrote:
> > On Tue, Jan 6, 2015 at 7:45 AM, Maxime Ripard
> > <maxime.ripard@free-electrons.com> wrote:
> >> Commit 1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s error on
> >> failure") actually broke the deferred probing mechanism, since it now returns
> >> EPROBE_DEFER only when the try_module_get call fails, but not when the phy
> >> lookup does.
> >>
> >> All the other similar functions seem to return ENODEV when try_module_get
> >> fails, and the error code of either __usb_find_phy or __of_usb_find_phy
> >> otherwise.
> >>
> >> In order to have a consistent behaviour, and a meaningful EPROBE_DEFER, always
> >> return EPROBE_DEFER when __(of_)usb_find_phy fails to look up the requested
> >> phy, that will be propagated by the caller, and ENODEV if try_module_get fails.
> >>
> > 
> > Fixes:  1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s error
> > on failure")
> > 
> >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > 
> > This also fixes USB on tegra/trimslice here.
> > 
> > Tested-by: Olof Johansson <olof@lixom.net>
> > 
> > Given that the regression went in during 3.19 merge window, this
> > should go in as a fix during -rc, please.
> 
> 1) Pls note, there is a duplication patch https://www.mail-archive.com/linux-usb at vger.kernel.org/msg53893.html
> but this one is more correct as for me. 
> 
> 2) This patch fixes USB regression on Keystone 2, so
> 
>  Tested-by: Grygorii Strashko <grygorii.strashko@linaro.org>

Are we asking that I drop Thierry's Patch or just apply the other hunks?

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150108/fc39e5b5/attachment.sig>

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

* [PATCH 1/4] usb: phy: Fix deferred probing
  2015-01-08 17:24       ` Felipe Balbi
@ 2015-01-08 18:04         ` Thierry Reding
  2015-01-08 18:09           ` Felipe Balbi
  0 siblings, 1 reply; 12+ messages in thread
From: Thierry Reding @ 2015-01-08 18:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jan 08, 2015 at 11:24:11AM -0600, Felipe Balbi wrote:
> * PGP Signed by an unknown key
> 
> On Thu, Jan 08, 2015 at 06:32:27PM +0200, Grygorii.Strashko at linaro.org wrote:
> > Hi,
> > 
> > On 01/07/2015 06:41 AM, Olof Johansson wrote:
> > > On Tue, Jan 6, 2015 at 7:45 AM, Maxime Ripard
> > > <maxime.ripard@free-electrons.com> wrote:
> > >> Commit 1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s error on
> > >> failure") actually broke the deferred probing mechanism, since it now returns
> > >> EPROBE_DEFER only when the try_module_get call fails, but not when the phy
> > >> lookup does.
> > >>
> > >> All the other similar functions seem to return ENODEV when try_module_get
> > >> fails, and the error code of either __usb_find_phy or __of_usb_find_phy
> > >> otherwise.
> > >>
> > >> In order to have a consistent behaviour, and a meaningful EPROBE_DEFER, always
> > >> return EPROBE_DEFER when __(of_)usb_find_phy fails to look up the requested
> > >> phy, that will be propagated by the caller, and ENODEV if try_module_get fails.
> > >>
> > > 
> > > Fixes:  1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s error
> > > on failure")
> > > 
> > >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > 
> > > This also fixes USB on tegra/trimslice here.
> > > 
> > > Tested-by: Olof Johansson <olof@lixom.net>
> > > 
> > > Given that the regression went in during 3.19 merge window, this
> > > should go in as a fix during -rc, please.
> > 
> > 1) Pls note, there is a duplication patch https://www.mail-archive.com/linux-usb at vger.kernel.org/msg53893.html
> > but this one is more correct as for me. 
> > 
> > 2) This patch fixes USB regression on Keystone 2, so
> > 
> >  Tested-by: Grygorii Strashko <grygorii.strashko@linaro.org>
> 
> Are we asking that I drop Thierry's Patch or just apply the other hunks?

The above doesn't like to my patch. The original is here:

	http://www.spinics.net/lists/linux-usb/msg118358.html

That has the advantage of also handling the case where the PHY provider
is disabled and will therefore never become available, thus preventing
infinite deferred probe.

My patch doesn't handle the non-OF case, though I'm not sure that
it's safe to defer probe for those, since we don't have any indication
that the probe will ever succeed. In that case it might actually be
better to keep returning -ENODEV. Also the change to __usb_find_phy() is
unrelated to the commit that causes the regression, so if we want to
defer probe in that case, too it might be better to do so in a separate
patch.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150108/aaca6d34/attachment.sig>

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

* [PATCH 1/4] usb: phy: Fix deferred probing
  2015-01-08 18:04         ` Thierry Reding
@ 2015-01-08 18:09           ` Felipe Balbi
  0 siblings, 0 replies; 12+ messages in thread
From: Felipe Balbi @ 2015-01-08 18:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jan 08, 2015 at 07:04:18PM +0100, Thierry Reding wrote:
> On Thu, Jan 08, 2015 at 11:24:11AM -0600, Felipe Balbi wrote:
> > * PGP Signed by an unknown key
> > 
> > On Thu, Jan 08, 2015 at 06:32:27PM +0200, Grygorii.Strashko at linaro.org wrote:
> > > Hi,
> > > 
> > > On 01/07/2015 06:41 AM, Olof Johansson wrote:
> > > > On Tue, Jan 6, 2015 at 7:45 AM, Maxime Ripard
> > > > <maxime.ripard@free-electrons.com> wrote:
> > > >> Commit 1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s error on
> > > >> failure") actually broke the deferred probing mechanism, since it now returns
> > > >> EPROBE_DEFER only when the try_module_get call fails, but not when the phy
> > > >> lookup does.
> > > >>
> > > >> All the other similar functions seem to return ENODEV when try_module_get
> > > >> fails, and the error code of either __usb_find_phy or __of_usb_find_phy
> > > >> otherwise.
> > > >>
> > > >> In order to have a consistent behaviour, and a meaningful EPROBE_DEFER, always
> > > >> return EPROBE_DEFER when __(of_)usb_find_phy fails to look up the requested
> > > >> phy, that will be propagated by the caller, and ENODEV if try_module_get fails.
> > > >>
> > > > 
> > > > Fixes:  1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s error
> > > > on failure")
> > > > 
> > > >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > > > 
> > > > This also fixes USB on tegra/trimslice here.
> > > > 
> > > > Tested-by: Olof Johansson <olof@lixom.net>
> > > > 
> > > > Given that the regression went in during 3.19 merge window, this
> > > > should go in as a fix during -rc, please.
> > > 
> > > 1) Pls note, there is a duplication patch https://www.mail-archive.com/linux-usb at vger.kernel.org/msg53893.html
> > > but this one is more correct as for me. 
> > > 
> > > 2) This patch fixes USB regression on Keystone 2, so
> > > 
> > >  Tested-by: Grygorii Strashko <grygorii.strashko@linaro.org>
> > 
> > Are we asking that I drop Thierry's Patch or just apply the other hunks?
> 
> The above doesn't like to my patch. The original is here:
> 
> 	http://www.spinics.net/lists/linux-usb/msg118358.html
> 
> That has the advantage of also handling the case where the PHY provider
> is disabled and will therefore never become available, thus preventing
> infinite deferred probe.
> 
> My patch doesn't handle the non-OF case, though I'm not sure that
> it's safe to defer probe for those, since we don't have any indication
> that the probe will ever succeed. In that case it might actually be
> better to keep returning -ENODEV. Also the change to __usb_find_phy() is
> unrelated to the commit that causes the regression, so if we want to
> defer probe in that case, too it might be better to do so in a separate
> patch.

my thoughts exactly. I'll send your patch and expect another patch for
3.20 for the other case. Thanks

-- 
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150108/daf33c2e/attachment-0001.sig>

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

end of thread, other threads:[~2015-01-08 18:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-06 15:45 [PATCH 0/4] ARM: mvebu: Enable XHCI on the A385-AP Maxime Ripard
2015-01-06 15:45 ` [PATCH 1/4] usb: phy: Fix deferred probing Maxime Ripard
2015-01-07  4:41   ` Olof Johansson
2015-01-08 16:32     ` Grygorii.Strashko@linaro.org
2015-01-08 17:24       ` Felipe Balbi
2015-01-08 18:04         ` Thierry Reding
2015-01-08 18:09           ` Felipe Balbi
2015-01-06 15:45 ` [PATCH 2/4] usb: XHCI: platform: Move the Marvell quirks after the enabling the clocks Maxime Ripard
2015-01-06 15:58   ` Thomas Petazzoni
2015-01-06 15:45 ` [PATCH 3/4] usb: xhci: plat: Add USB phy support Maxime Ripard
2015-01-06 19:39   ` Sergei Shtylyov
2015-01-06 15:45 ` [PATCH 4/4] ARM: mvebu: armada-385-ap: Enable USB3 port Maxime Ripard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox