ARM: mvebu: Enable XHCI on the A385-AP
Subsystems (3)
cover ARM: mvebu: Enable XHCI on the A385-AP
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(-)
Signature · 1 line
2.2.1
PATCH 1/4 usb: phy: Fix deferred probing
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.
--- drivers/usb/phy/phy.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/usb/phy/phy.c +3 -3
@@ -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;
Signature · 1 line
2.2.1
PATCH 1/4 usb: phy: Fix deferred probing
Hi,
Quoted 5 lines · Maxime Ripard
Commit
1290a958d48e("usb: phy: propagate __of_usb_find_phy()'s error on failure") actually broke the deferred probing mechanism, since it now returnsEPROBE_DEFERonly when thetry_module_getcall fails, but not when the phy lookup does.
Quoted 8 lines
All the other similar functions seem to return ENODEV when
try_module_getfails, and the error code of either__usb_find_phyor__of_usb_find_phyotherwise.In order to have a consistent behaviour, and a meaningful
EPROBE_DEFER, always returnEPROBE_DEFERwhen __(of_)usb_find_phy fails to look up the requested phy, that will be propagated by the caller, and ENODEV iftry_module_getfails.
on failure")
Quoted 1 line
Signed-off-by Maxime Ripard <[email protected]>
This also fixes USB on tegra/trimslice here.
Given that the regression went in during 3.19 merge window, this should go in as a fix during -rc, please.
-Olof
PATCH 1/4 usb: phy: Fix deferred probing
Hi,
Quoted 19 lines · Olof Johansson
Quoted 5 lines · Maxime Ripard
Commit
1290a958d48e("usb: phy: propagate __of_usb_find_phy()'s error on failure") actually broke the deferred probing mechanism, since it now returnsEPROBE_DEFERonly when thetry_module_getcall fails, but not when the phy lookup does.Quoted 8 lines
All the other similar functions seem to return ENODEV when
try_module_getfails, and the error code of either__usb_find_phyor__of_usb_find_phyotherwise.In order to have a consistent behaviour, and a meaningful
EPROBE_DEFER, always returnEPROBE_DEFERwhen __(of_)usb_find_phy fails to look up the requested phy, that will be propagated by the caller, and ENODEV iftry_module_getfails.Fixes 1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s erroron failure")
Quoted 8 lines
Quoted 1 line
Signed-off-by Maxime Ripard <[email protected]>This also fixes USB on tegra/trimslice here.
Tested-by Olof Johansson <[email protected]>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
Signature · 2 lines
regards, -grygorii
PATCH 1/4 usb: phy: Fix deferred probing
Quoted 31 lines · Grygorii.Strashko at linaro.org
Hi,
Quoted 19 lines · Olof Johansson
Quoted 5 lines · Maxime Ripard
Commit
1290a958d48e("usb: phy: propagate __of_usb_find_phy()'s error on failure") actually broke the deferred probing mechanism, since it now returnsEPROBE_DEFERonly when thetry_module_getcall fails, but not when the phy lookup does.Quoted 8 lines
All the other similar functions seem to return ENODEV when
try_module_getfails, and the error code of either__usb_find_phyor__of_usb_find_phyotherwise.In order to have a consistent behaviour, and a meaningful
EPROBE_DEFER, always returnEPROBE_DEFERwhen __(of_)usb_find_phy fails to look up the requested phy, that will be propagated by the caller, and ENODEV iftry_module_getfails.Fixes 1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s erroron failure")
Quoted 8 lines
Quoted 1 line
Signed-off-by Maxime Ripard <[email protected]>This also fixes USB on tegra/trimslice here.
Tested-by Olof Johansson <[email protected]>Given that the regression went in during 3.19 merge window, this should go in as a fix during -rc, please.
Quoted 6 lines
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 <[email protected]>
Are we asking that I drop Thierry's Patch or just apply the other hunks?
Signature · 8 lines
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>
PATCH 1/4 usb: phy: Fix deferred probing
Quoted 34 lines · Felipe Balbi
* PGP Signed by an unknown key
Quoted 31 lines · Grygorii.Strashko at linaro.org
Hi,
Quoted 27 lines · Olof Johansson
Quoted 5 lines · Maxime Ripard
Commit
1290a958d48e("usb: phy: propagate __of_usb_find_phy()'s error on failure") actually broke the deferred probing mechanism, since it now returnsEPROBE_DEFERonly when thetry_module_getcall fails, but not when the phy lookup does.Quoted 8 lines
All the other similar functions seem to return ENODEV when
try_module_getfails, and the error code of either__usb_find_phyor__of_usb_find_phyotherwise.In order to have a consistent behaviour, and a meaningful
EPROBE_DEFER, always returnEPROBE_DEFERwhen __(of_)usb_find_phy fails to look up the requested phy, that will be propagated by the caller, and ENODEV iftry_module_getfails.Fixes 1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s erroron failure")
Quoted 1 line
Signed-off-by Maxime Ripard <[email protected]>This also fixes USB on tegra/trimslice here.
Tested-by Olof Johansson <[email protected]>Given that the regression went in during 3.19 merge window, this should go in as a fix during -rc, please.
Quoted 8 lines
Quoted 6 lines
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 <[email protected]>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>
PATCH 1/4 usb: phy: Fix deferred probing
Quoted 52 lines · Thierry Reding
Quoted 34 lines · Felipe Balbi
* PGP Signed by an unknown key
Quoted 31 lines · Grygorii.Strashko at linaro.org
Hi,
Quoted 27 lines · Olof Johansson
Quoted 5 lines · Maxime Ripard
Commit
1290a958d48e("usb: phy: propagate __of_usb_find_phy()'s error on failure") actually broke the deferred probing mechanism, since it now returnsEPROBE_DEFERonly when thetry_module_getcall fails, but not when the phy lookup does.Quoted 8 lines
All the other similar functions seem to return ENODEV when
try_module_getfails, and the error code of either__usb_find_phyor__of_usb_find_phyotherwise.In order to have a consistent behaviour, and a meaningful
EPROBE_DEFER, always returnEPROBE_DEFERwhen __(of_)usb_find_phy fails to look up the requested phy, that will be propagated by the caller, and ENODEV iftry_module_getfails.Fixes 1290a958d48e ("usb: phy: propagate __of_usb_find_phy()'s erroron failure")
Quoted 1 line
Signed-off-by Maxime Ripard <[email protected]>This also fixes USB on tegra/trimslice here.
Tested-by Olof Johansson <[email protected]>Given that the regression went in during 3.19 merge window, this should go in as a fix during -rc, please.
Quoted 8 lines
Quoted 6 lines
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 <[email protected]>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.htmlThat 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.
Quoted 7 lines
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
Signature · 8 lines
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>
PATCH 2/4 usb: XHCI: platform: Move the Marvell quirks after the enabling the clocks
In reply to Maxime RipardThe 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.
--- drivers/usb/host/xhci-plat.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-)
drivers/usb/host/xhci-plat.c +9 -10
@@ -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;
Signature · 1 line
2.2.1
PATCH 2/4 usb: XHCI: platform: Move the Marvell quirks after the enabling the clocks
Dear Maxime Ripard,
drivers/usb/host/xhci-plat.c static int xhci_plat_probe(struct platform_device *pdev) · line 127
Quoted 8 lines · Maxime Ripard
drivers/usb/host/xhci-plat.c +8
-0+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
Signature · 3 lines
Thomas Petazzoni, CTO, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com
PATCH 3/4 usb: xhci: plat: Add USB phy support
In reply to Maxime RipardThe 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.
--- drivers/usb/host/xhci-plat.c | 13 +++++++++++++ drivers/usb/host/xhci.h | 2 ++ 2 files changed, 15 insertions(+)
drivers/usb/host/xhci-plat.c +13 -0
@@ -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;
drivers/usb/host/xhci.h +2 -0
@@ -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;
Signature · 1 line
2.2.1
PATCH 3/4 usb: xhci: plat: Add USB phy support
Hello.
Quoted 5 lines · Maxime Ripard
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 <[email protected]>
[...]
drivers/usb/host/xhci.h struct xhci_hcd { · line 1478
Quoted 10 lines
drivers/usb/host/xhci.h +2
-0@@ -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
PATCH 4/4 ARM: mvebu: armada-385-ap: Enable USB3 port
In reply to Maxime RipardThe 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.
--- arch/arm/boot/dts/armada-385-ap.dts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
arch/arm/boot/dts/armada-385-ap.dts +28 -0
@@ -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 = <®_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>; + }; };
Signature · 1 line
2.2.1