[PATCH v2 0/4] Allwinner H6 USB3 support
This series adds PHY and XHCI driver support for the USB3 controller found in the Allwinner H6 SoC. It has been tested and working on both boards enabled in patch 4, although some users experience issues[1].
[1]: https://lists.denx.de/pipermail/u-boot/2021-February/440767.html
Changes from v1: - Dropped patches 1-2 (already in u-boot-sunxi/master) and rebased - Added Andre's Reviewed-by on the PHY driver - Fixed error handling in xhci_pci_probe
Samuel Holland (4): phy: sun50i-usb3: Add a driver for the H6 USB3 PHY usb: xhci-pci: Move reset logic out of XHCI core usb: xhci-dwc3: Add support for clocks/resets configs: Enable USB3 on Allwinner H6 boards
configs/orangepi_3_defconfig | 5 + configs/pine_h64_defconfig | 5 + drivers/phy/allwinner/Kconfig | 8 ++ drivers/phy/allwinner/Makefile | 1 + drivers/phy/allwinner/phy-sun50i-usb3.c | 171 ++++++++++++++++++++++++ drivers/usb/host/xhci-dwc3.c | 56 ++++++++ drivers/usb/host/xhci-mem.c | 2 - drivers/usb/host/xhci-pci.c | 51 ++++++- drivers/usb/host/xhci.c | 35 ----- include/usb/xhci.h | 2 - 10 files changed, 293 insertions(+), 43 deletions(-) create mode 100644 drivers/phy/allwinner/phy-sun50i-usb3.c
This driver is needed for XHCI to work on the Allwinner H6 SoC. The driver is copied from Linux v5.10.
Reviewed-by: Andre Przywara andre.przywara@arm.com Signed-off-by: Samuel Holland samuel@sholland.org --- drivers/phy/allwinner/Kconfig | 8 ++ drivers/phy/allwinner/Makefile | 1 + drivers/phy/allwinner/phy-sun50i-usb3.c | 171 ++++++++++++++++++++++++ 3 files changed, 180 insertions(+) create mode 100644 drivers/phy/allwinner/phy-sun50i-usb3.c
diff --git a/drivers/phy/allwinner/Kconfig b/drivers/phy/allwinner/Kconfig index dba3bae61c..6bfb79cbca 100644 --- a/drivers/phy/allwinner/Kconfig +++ b/drivers/phy/allwinner/Kconfig @@ -11,3 +11,11 @@ config PHY_SUN4I_USB
This driver controls the entire USB PHY block, both the USB OTG parts, as well as the 2 regular USB 2 host PHYs. + +config PHY_SUN50I_USB3 + bool "Allwinner sun50i USB3 PHY driver" + depends on ARCH_SUNXI + select PHY + help + Enable this to support the USB3 transceiver that is part of + Allwinner sun50i SoCs. diff --git a/drivers/phy/allwinner/Makefile b/drivers/phy/allwinner/Makefile index e709fca643..f2b60ce1a6 100644 --- a/drivers/phy/allwinner/Makefile +++ b/drivers/phy/allwinner/Makefile @@ -4,3 +4,4 @@ #
obj-$(CONFIG_PHY_SUN4I_USB) += phy-sun4i-usb.o +obj-$(CONFIG_PHY_SUN50I_USB3) += phy-sun50i-usb3.o diff --git a/drivers/phy/allwinner/phy-sun50i-usb3.c b/drivers/phy/allwinner/phy-sun50i-usb3.c new file mode 100644 index 0000000000..e5a3d2d92e --- /dev/null +++ b/drivers/phy/allwinner/phy-sun50i-usb3.c @@ -0,0 +1,171 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Allwinner sun50i(H6) USB 3.0 phy driver + * + * Copyright (C) 2020 Samuel Holland samuel@sholland.org + * + * Based on the Linux driver, which is: + * + * Copyright (C) 2017 Icenowy Zheng icenowy@aosc.io + * + * Based on phy-sun9i-usb.c, which is: + * + * Copyright (C) 2014-2015 Chen-Yu Tsai wens@csie.org + * + * Based on code from Allwinner BSP, which is: + * + * Copyright (c) 2010-2015 Allwinner Technology Co., Ltd. + */ + +#include <asm/io.h> +#include <clk.h> +#include <dm.h> +#include <dm/device_compat.h> +#include <generic-phy.h> +#include <linux/bitops.h> +#include <reset.h> + +/* Interface Status and Control Registers */ +#define SUNXI_ISCR 0x00 +#define SUNXI_PIPE_CLOCK_CONTROL 0x14 +#define SUNXI_PHY_TUNE_LOW 0x18 +#define SUNXI_PHY_TUNE_HIGH 0x1c +#define SUNXI_PHY_EXTERNAL_CONTROL 0x20 + +/* USB2.0 Interface Status and Control Register */ +#define SUNXI_ISCR_FORCE_VBUS (3 << 12) + +/* PIPE Clock Control Register */ +#define SUNXI_PCC_PIPE_CLK_OPEN (1 << 6) + +/* PHY External Control Register */ +#define SUNXI_PEC_EXTERN_VBUS (3 << 1) +#define SUNXI_PEC_SSC_EN (1 << 24) +#define SUNXI_PEC_REF_SSP_EN (1 << 26) + +/* PHY Tune High Register */ +#define SUNXI_TX_DEEMPH_3P5DB(n) ((n) << 19) +#define SUNXI_TX_DEEMPH_3P5DB_MASK GENMASK(24, 19) +#define SUNXI_TX_DEEMPH_6DB(n) ((n) << 13) +#define SUNXI_TX_DEEMPH_6GB_MASK GENMASK(18, 13) +#define SUNXI_TX_SWING_FULL(n) ((n) << 6) +#define SUNXI_TX_SWING_FULL_MASK GENMASK(12, 6) +#define SUNXI_LOS_BIAS(n) ((n) << 3) +#define SUNXI_LOS_BIAS_MASK GENMASK(5, 3) +#define SUNXI_TXVBOOSTLVL(n) ((n) << 0) +#define SUNXI_TXVBOOSTLVL_MASK GENMASK(2, 0) + +struct sun50i_usb3_phy_priv { + void __iomem *regs; + struct reset_ctl reset; + struct clk clk; +}; + +static void sun50i_usb3_phy_open(struct sun50i_usb3_phy_priv *phy) +{ + u32 val; + + val = readl(phy->regs + SUNXI_PHY_EXTERNAL_CONTROL); + val |= SUNXI_PEC_EXTERN_VBUS; + val |= SUNXI_PEC_SSC_EN | SUNXI_PEC_REF_SSP_EN; + writel(val, phy->regs + SUNXI_PHY_EXTERNAL_CONTROL); + + val = readl(phy->regs + SUNXI_PIPE_CLOCK_CONTROL); + val |= SUNXI_PCC_PIPE_CLK_OPEN; + writel(val, phy->regs + SUNXI_PIPE_CLOCK_CONTROL); + + val = readl(phy->regs + SUNXI_ISCR); + val |= SUNXI_ISCR_FORCE_VBUS; + writel(val, phy->regs + SUNXI_ISCR); + + /* + * All the magic numbers written to the PHY_TUNE_{LOW_HIGH} + * registers are directly taken from the BSP USB3 driver from + * Allwiner. + */ + writel(0x0047fc87, phy->regs + SUNXI_PHY_TUNE_LOW); + + val = readl(phy->regs + SUNXI_PHY_TUNE_HIGH); + val &= ~(SUNXI_TXVBOOSTLVL_MASK | SUNXI_LOS_BIAS_MASK | + SUNXI_TX_SWING_FULL_MASK | SUNXI_TX_DEEMPH_6GB_MASK | + SUNXI_TX_DEEMPH_3P5DB_MASK); + val |= SUNXI_TXVBOOSTLVL(0x7); + val |= SUNXI_LOS_BIAS(0x7); + val |= SUNXI_TX_SWING_FULL(0x55); + val |= SUNXI_TX_DEEMPH_6DB(0x20); + val |= SUNXI_TX_DEEMPH_3P5DB(0x15); + writel(val, phy->regs + SUNXI_PHY_TUNE_HIGH); +} + +static int sun50i_usb3_phy_init(struct phy *phy) +{ + struct sun50i_usb3_phy_priv *priv = dev_get_priv(phy->dev); + int ret; + + ret = clk_prepare_enable(&priv->clk); + if (ret) + return ret; + + ret = reset_deassert(&priv->reset); + if (ret) { + clk_disable_unprepare(&priv->clk); + return ret; + } + + sun50i_usb3_phy_open(priv); + + return 0; +} + +static int sun50i_usb3_phy_exit(struct phy *phy) +{ + struct sun50i_usb3_phy_priv *priv = dev_get_priv(phy->dev); + + reset_assert(&priv->reset); + clk_disable_unprepare(&priv->clk); + + return 0; +} + +static const struct phy_ops sun50i_usb3_phy_ops = { + .init = sun50i_usb3_phy_init, + .exit = sun50i_usb3_phy_exit, +}; + +static int sun50i_usb3_phy_probe(struct udevice *dev) +{ + struct sun50i_usb3_phy_priv *priv = dev_get_priv(dev); + int ret; + + ret = clk_get_by_index(dev, 0, &priv->clk); + if (ret) { + dev_err(dev, "failed to get phy clock\n"); + return ret; + } + + ret = reset_get_by_index(dev, 0, &priv->reset); + if (ret) { + dev_err(dev, "failed to get reset control\n"); + return ret; + } + + priv->regs = (void __iomem *)dev_read_addr(dev); + if (IS_ERR(priv->regs)) + return PTR_ERR(priv->regs); + + return 0; +} + +static const struct udevice_id sun50i_usb3_phy_ids[] = { + { .compatible = "allwinner,sun50i-h6-usb3-phy" }, + { }, +}; + +U_BOOT_DRIVER(sun50i_usb3_phy) = { + .name = "sun50i-usb3-phy", + .id = UCLASS_PHY, + .of_match = sun50i_usb3_phy_ids, + .ops = &sun50i_usb3_phy_ops, + .probe = sun50i_usb3_phy_probe, + .priv_auto = sizeof(struct sun50i_usb3_phy_priv), +};
Resetting an XHCI controller inside xhci_register undoes any register setup performed by the platform driver. And at least on the Allwinner H6, resetting the XHCI controller also resets the PHY, which prevents the controller from working. That means the controller must be taken out of reset before initializing the PHY, which must be done before calling xhci_register.
The logic in the XHCI core was added to support the Raspberry Pi 4 (although this was not mentioned in the commit log!), which uses the xhci-pci platform driver. Move the reset logic to the platform driver, where it belongs, and where it cannot interfere with other platform drivers.
This also fixes a failure to call reset_free if xhci_register failed.
Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") Signed-off-by: Samuel Holland samuel@sholland.org --- drivers/usb/host/xhci-mem.c | 2 -- drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.c | 35 ------------------------- include/usb/xhci.h | 2 -- 4 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 1c11c2e7e0..0d9da62bab 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -180,8 +180,6 @@ void xhci_cleanup(struct xhci_ctrl *ctrl) xhci_free_virt_devices(ctrl); free(ctrl->erst.entries); free(ctrl->dcbaa); - if (reset_valid(&ctrl->reset)) - reset_free(&ctrl->reset); memset(ctrl, '\0', sizeof(struct xhci_ctrl)); }
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index aaa243f291..ea8e8f3211 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -10,9 +10,14 @@ #include <init.h> #include <log.h> #include <pci.h> +#include <reset.h> #include <usb.h> #include <usb/xhci.h>
+struct xhci_pci_plat { + struct reset_ctl reset; +}; + static int xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr, struct xhci_hcor **ret_hcor) { @@ -45,15 +50,53 @@ static int xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr,
static int xhci_pci_probe(struct udevice *dev) { + struct xhci_pci_plat = dev_get_plat(dev); struct xhci_hccr *hccr; struct xhci_hcor *hcor; int ret;
+ ret = reset_get_by_index(dev, 0, &plat->reset); + if (ret && ret != -ENOENT && ret != -ENOTSUPP) { + dev_err(dev, "failed to get reset\n"); + return ret; + } + + if (reset_valid(&plat->reset)) { + ret = reset_assert(&plat->reset); + if (ret) + goto err_reset; + + ret = reset_deassert(&plat->reset); + if (ret) + goto err_reset; + } + ret = xhci_pci_init(dev, &hccr, &hcor); if (ret) - return ret; + goto err_reset; + + ret = xhci_register(dev, hccr, hcor); + if (ret) + goto err_reset; + + return 0; + +err_reset: + if (reset_valid(&plat->reset)) + reset_free(&plat->reset); + + return ret; +} + +static int xhci_pci_remove(struct udevice *dev) +{ + struct xhci_pci_plat = dev_get_plat(dev);
- return xhci_register(dev, hccr, hcor); + xhci_deregister(dev); + if (reset_valid(&plat->reset)) + reset_free(&plat->reset); + + return 0; }
static const struct udevice_id xhci_pci_ids[] = { @@ -65,10 +108,10 @@ U_BOOT_DRIVER(xhci_pci) = { .name = "xhci_pci", .id = UCLASS_USB, .probe = xhci_pci_probe, - .remove = xhci_deregister, + .remove = xhci_pci_remove, .of_match = xhci_pci_ids, .ops = &xhci_usb_ops, - .plat_auto = sizeof(struct usb_plat), + .plat_auto = sizeof(struct xhci_pci_plat), .priv_auto = sizeof(struct xhci_ctrl), .flags = DM_FLAG_OS_PREPARE | DM_FLAG_ALLOC_PRIV_DMA, }; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index d27ac01c83..452dacc0af 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -188,37 +188,6 @@ static int xhci_start(struct xhci_hcor *hcor) return ret; }
-#if CONFIG_IS_ENABLED(DM_USB) -/** - * Resets XHCI Hardware - * - * @param ctrl pointer to host controller - * @return 0 if OK, or a negative error code. - */ -static int xhci_reset_hw(struct xhci_ctrl *ctrl) -{ - int ret; - - ret = reset_get_by_index(ctrl->dev, 0, &ctrl->reset); - if (ret && ret != -ENOENT && ret != -ENOTSUPP) { - dev_err(ctrl->dev, "failed to get reset\n"); - return ret; - } - - if (reset_valid(&ctrl->reset)) { - ret = reset_assert(&ctrl->reset); - if (ret) - return ret; - - ret = reset_deassert(&ctrl->reset); - if (ret) - return ret; - } - - return 0; -} -#endif - /** * Resets the XHCI Controller * @@ -1534,10 +1503,6 @@ int xhci_register(struct udevice *dev, struct xhci_hccr *hccr,
ctrl->dev = dev;
- ret = xhci_reset_hw(ctrl); - if (ret) - goto err; - /* * XHCI needs to issue a Address device command to setup * proper device context structures, before it can interact diff --git a/include/usb/xhci.h b/include/usb/xhci.h index 8d95e213b0..01e63cf0fc 100644 --- a/include/usb/xhci.h +++ b/include/usb/xhci.h @@ -17,7 +17,6 @@ #define HOST_XHCI_H_
#include <phys2bus.h> -#include <reset.h> #include <asm/types.h> #include <asm/cache.h> #include <asm/io.h> @@ -1200,7 +1199,6 @@ struct xhci_ctrl { #if CONFIG_IS_ENABLED(DM_USB) struct udevice *dev; #endif - struct reset_ctl reset; struct xhci_hccr *hccr; /* R/O registers, not need for volatile */ struct xhci_hcor *hcor; struct xhci_doorbell_array *dba;
On Sat, 17 Apr 2021 09:20:57 -0500 Samuel Holland samuel@sholland.org wrote:
Hi,
Resetting an XHCI controller inside xhci_register undoes any register setup performed by the platform driver. And at least on the Allwinner H6, resetting the XHCI controller also resets the PHY, which prevents the controller from working. That means the controller must be taken out of reset before initializing the PHY, which must be done before calling xhci_register.
The logic in the XHCI core was added to support the Raspberry Pi 4 (although this was not mentioned in the commit log!), which uses the xhci-pci platform driver. Move the reset logic to the platform driver, where it belongs, and where it cannot interfere with other platform drivers.
This also fixes a failure to call reset_free if xhci_register failed.
Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") Signed-off-by: Samuel Holland samuel@sholland.org
From my research it looks like no other board should be affected by the change, and they have been no reports from the people I asked a few weeks ago. I'd say we should merge it, to expose it to a wider audience, and keep an eye on bug reports.
Acked-by: Andre Przywara andre.przywara@arm.com
Cheers, Andre
drivers/usb/host/xhci-mem.c | 2 -- drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.c | 35 ------------------------- include/usb/xhci.h | 2 -- 4 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 1c11c2e7e0..0d9da62bab 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -180,8 +180,6 @@ void xhci_cleanup(struct xhci_ctrl *ctrl) xhci_free_virt_devices(ctrl); free(ctrl->erst.entries); free(ctrl->dcbaa);
- if (reset_valid(&ctrl->reset))
reset_free(&ctrl->reset);}
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index aaa243f291..ea8e8f3211 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -10,9 +10,14 @@ #include <init.h> #include <log.h> #include <pci.h> +#include <reset.h> #include <usb.h> #include <usb/xhci.h>
+struct xhci_pci_plat {
- struct reset_ctl reset;
+};
static int xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr, struct xhci_hcor **ret_hcor) { @@ -45,15 +50,53 @@ static int xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr,
static int xhci_pci_probe(struct udevice *dev) {
struct xhci_pci_plat = dev_get_plat(dev); struct xhci_hccr *hccr; struct xhci_hcor *hcor; int ret;
ret = reset_get_by_index(dev, 0, &plat->reset);
if (ret && ret != -ENOENT && ret != -ENOTSUPP) {
dev_err(dev, "failed to get reset\n");return ret;}
if (reset_valid(&plat->reset)) {
ret = reset_assert(&plat->reset);if (ret)goto err_reset;ret = reset_deassert(&plat->reset);if (ret)goto err_reset;}
ret = xhci_pci_init(dev, &hccr, &hcor); if (ret)
return ret;
goto err_reset;- ret = xhci_register(dev, hccr, hcor);
- if (ret)
goto err_reset;- return 0;
+err_reset:
- if (reset_valid(&plat->reset))
reset_free(&plat->reset);- return ret;
+}
+static int xhci_pci_remove(struct udevice *dev) +{
- struct xhci_pci_plat = dev_get_plat(dev);
- return xhci_register(dev, hccr, hcor);
- xhci_deregister(dev);
- if (reset_valid(&plat->reset))
reset_free(&plat->reset);- return 0;
}
static const struct udevice_id xhci_pci_ids[] = { @@ -65,10 +108,10 @@ U_BOOT_DRIVER(xhci_pci) = { .name = "xhci_pci", .id = UCLASS_USB, .probe = xhci_pci_probe,
- .remove = xhci_deregister,
- .remove = xhci_pci_remove, .of_match = xhci_pci_ids, .ops = &xhci_usb_ops,
- .plat_auto = sizeof(struct usb_plat),
- .plat_auto = sizeof(struct xhci_pci_plat), .priv_auto = sizeof(struct xhci_ctrl), .flags = DM_FLAG_OS_PREPARE | DM_FLAG_ALLOC_PRIV_DMA,
}; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index d27ac01c83..452dacc0af 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -188,37 +188,6 @@ static int xhci_start(struct xhci_hcor *hcor) return ret; }
-#if CONFIG_IS_ENABLED(DM_USB) -/**
- Resets XHCI Hardware
- @param ctrl pointer to host controller
- @return 0 if OK, or a negative error code.
- */
-static int xhci_reset_hw(struct xhci_ctrl *ctrl) -{
- int ret;
- ret = reset_get_by_index(ctrl->dev, 0, &ctrl->reset);
- if (ret && ret != -ENOENT && ret != -ENOTSUPP) {
dev_err(ctrl->dev, "failed to get reset\n");return ret;- }
- if (reset_valid(&ctrl->reset)) {
ret = reset_assert(&ctrl->reset);if (ret)return ret;ret = reset_deassert(&ctrl->reset);if (ret)return ret;- }
- return 0;
-} -#endif
/**
- Resets the XHCI Controller
@@ -1534,10 +1503,6 @@ int xhci_register(struct udevice *dev, struct xhci_hccr *hccr,
ctrl->dev = dev;
- ret = xhci_reset_hw(ctrl);
- if (ret)
goto err;- /*
- XHCI needs to issue a Address device command to setup
- proper device context structures, before it can interact
diff --git a/include/usb/xhci.h b/include/usb/xhci.h index 8d95e213b0..01e63cf0fc 100644 --- a/include/usb/xhci.h +++ b/include/usb/xhci.h @@ -17,7 +17,6 @@ #define HOST_XHCI_H_
#include <phys2bus.h> -#include <reset.h> #include <asm/types.h> #include <asm/cache.h> #include <asm/io.h> @@ -1200,7 +1199,6 @@ struct xhci_ctrl { #if CONFIG_IS_ENABLED(DM_USB) struct udevice *dev; #endif
- struct reset_ctl reset; struct xhci_hccr *hccr; /* R/O registers, not need for volatile */ struct xhci_hcor *hcor; struct xhci_doorbell_array *dba;
On 4/21/21 5:36 AM, Andre Przywara wrote:
On Sat, 17 Apr 2021 09:20:57 -0500 Samuel Holland samuel@sholland.org wrote:
Hi,
Resetting an XHCI controller inside xhci_register undoes any register setup performed by the platform driver. And at least on the Allwinner H6, resetting the XHCI controller also resets the PHY, which prevents the controller from working. That means the controller must be taken out of reset before initializing the PHY, which must be done before calling xhci_register.
The logic in the XHCI core was added to support the Raspberry Pi 4 (although this was not mentioned in the commit log!), which uses the xhci-pci platform driver. Move the reset logic to the platform driver, where it belongs, and where it cannot interfere with other platform drivers.
This also fixes a failure to call reset_free if xhci_register failed.
Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") Signed-off-by: Samuel Holland samuel@sholland.org
From my research it looks like no other board should be affected by the change, and they have been no reports from the people I asked a few weeks ago. I'd say we should merge it, to expose it to a wider audience, and keep an eye on bug reports.
Acked-by: Andre Przywara andre.przywara@arm.com
Thanks!
Note that after a pending device tree change[1], this patch won't be strictly necessary for this platform anymore, since the DWC3 node will no longer have a resets property. However I still think it's a good change to make.
Regards, Samuel
[1]: https://lore.kernel.org/linux-sunxi/20210421042834.27309-3-samuel@sholland.o...
Cheers, Andre
drivers/usb/host/xhci-mem.c | 2 -- drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.c | 35 ------------------------- include/usb/xhci.h | 2 -- 4 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 1c11c2e7e0..0d9da62bab 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -180,8 +180,6 @@ void xhci_cleanup(struct xhci_ctrl *ctrl) xhci_free_virt_devices(ctrl); free(ctrl->erst.entries); free(ctrl->dcbaa);
- if (reset_valid(&ctrl->reset))
reset_free(&ctrl->reset);}
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index aaa243f291..ea8e8f3211 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -10,9 +10,14 @@ #include <init.h> #include <log.h> #include <pci.h> +#include <reset.h> #include <usb.h> #include <usb/xhci.h>
+struct xhci_pci_plat {
- struct reset_ctl reset;
+};
static int xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr, struct xhci_hcor **ret_hcor) { @@ -45,15 +50,53 @@ static int xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr,
static int xhci_pci_probe(struct udevice *dev) {
struct xhci_pci_plat = dev_get_plat(dev); struct xhci_hccr *hccr; struct xhci_hcor *hcor; int ret;
ret = reset_get_by_index(dev, 0, &plat->reset);
if (ret && ret != -ENOENT && ret != -ENOTSUPP) {
dev_err(dev, "failed to get reset\n");return ret;}
if (reset_valid(&plat->reset)) {
ret = reset_assert(&plat->reset);if (ret)goto err_reset;ret = reset_deassert(&plat->reset);if (ret)goto err_reset;}
ret = xhci_pci_init(dev, &hccr, &hcor); if (ret)
return ret;
goto err_reset;- ret = xhci_register(dev, hccr, hcor);
- if (ret)
goto err_reset;- return 0;
+err_reset:
- if (reset_valid(&plat->reset))
reset_free(&plat->reset);- return ret;
+}
+static int xhci_pci_remove(struct udevice *dev) +{
- struct xhci_pci_plat = dev_get_plat(dev);
- return xhci_register(dev, hccr, hcor);
- xhci_deregister(dev);
- if (reset_valid(&plat->reset))
reset_free(&plat->reset);- return 0;
}
static const struct udevice_id xhci_pci_ids[] = { @@ -65,10 +108,10 @@ U_BOOT_DRIVER(xhci_pci) = { .name = "xhci_pci", .id = UCLASS_USB, .probe = xhci_pci_probe,
- .remove = xhci_deregister,
- .remove = xhci_pci_remove, .of_match = xhci_pci_ids, .ops = &xhci_usb_ops,
- .plat_auto = sizeof(struct usb_plat),
- .plat_auto = sizeof(struct xhci_pci_plat), .priv_auto = sizeof(struct xhci_ctrl), .flags = DM_FLAG_OS_PREPARE | DM_FLAG_ALLOC_PRIV_DMA,
}; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index d27ac01c83..452dacc0af 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -188,37 +188,6 @@ static int xhci_start(struct xhci_hcor *hcor) return ret; }
-#if CONFIG_IS_ENABLED(DM_USB) -/**
- Resets XHCI Hardware
- @param ctrl pointer to host controller
- @return 0 if OK, or a negative error code.
- */
-static int xhci_reset_hw(struct xhci_ctrl *ctrl) -{
- int ret;
- ret = reset_get_by_index(ctrl->dev, 0, &ctrl->reset);
- if (ret && ret != -ENOENT && ret != -ENOTSUPP) {
dev_err(ctrl->dev, "failed to get reset\n");return ret;- }
- if (reset_valid(&ctrl->reset)) {
ret = reset_assert(&ctrl->reset);if (ret)return ret;ret = reset_deassert(&ctrl->reset);if (ret)return ret;- }
- return 0;
-} -#endif
/**
- Resets the XHCI Controller
@@ -1534,10 +1503,6 @@ int xhci_register(struct udevice *dev, struct xhci_hccr *hccr,
ctrl->dev = dev;
- ret = xhci_reset_hw(ctrl);
- if (ret)
goto err;- /*
- XHCI needs to issue a Address device command to setup
- proper device context structures, before it can interact
diff --git a/include/usb/xhci.h b/include/usb/xhci.h index 8d95e213b0..01e63cf0fc 100644 --- a/include/usb/xhci.h +++ b/include/usb/xhci.h @@ -17,7 +17,6 @@ #define HOST_XHCI_H_
#include <phys2bus.h> -#include <reset.h> #include <asm/types.h> #include <asm/cache.h> #include <asm/io.h> @@ -1200,7 +1199,6 @@ struct xhci_ctrl { #if CONFIG_IS_ENABLED(DM_USB) struct udevice *dev; #endif
- struct reset_ctl reset; struct xhci_hccr *hccr; /* R/O registers, not need for volatile */ struct xhci_hcor *hcor; struct xhci_doorbell_array *dba;
On Wed, 21 Apr 2021 08:36:26 -0500 Samuel Holland samuel@sholland.org wrote:
Hi,
On 4/21/21 5:36 AM, Andre Przywara wrote:
On Sat, 17 Apr 2021 09:20:57 -0500 Samuel Holland samuel@sholland.org wrote:
Hi,
Resetting an XHCI controller inside xhci_register undoes any register setup performed by the platform driver. And at least on the Allwinner H6, resetting the XHCI controller also resets the PHY, which prevents the controller from working. That means the controller must be taken out of reset before initializing the PHY, which must be done before calling xhci_register.
The logic in the XHCI core was added to support the Raspberry Pi 4 (although this was not mentioned in the commit log!), which uses the xhci-pci platform driver. Move the reset logic to the platform driver, where it belongs, and where it cannot interfere with other platform drivers.
This also fixes a failure to call reset_free if xhci_register failed.
Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") Signed-off-by: Samuel Holland samuel@sholland.org
From my research it looks like no other board should be affected by the change, and they have been no reports from the people I asked a few weeks ago. I'd say we should merge it, to expose it to a wider audience, and keep an eye on bug reports.
Acked-by: Andre Przywara andre.przywara@arm.com
Thanks!
Note that after a pending device tree change[1], this patch won't be strictly necessary for this platform anymore, since the DWC3 node will no longer have a resets property. However I still think it's a good change to make.
Yes, I saw that, but those DT changes won't make it into U-Boot for a while, so we should go ahead with those patches anyway. This would also allow us the get the PHY driver tested already.
And I think we need another U-Boot patch, to teach it about the new(ly used) compatible string? I don't find "allwinner,sun50i-h6-dwc3" anywhere in the current U-Boot code.
Cheers, Andre
Cheers, Andre
drivers/usb/host/xhci-mem.c | 2 -- drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.c | 35 ------------------------- include/usb/xhci.h | 2 -- 4 files changed, 47 insertions(+), 43 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index 1c11c2e7e0..0d9da62bab 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -180,8 +180,6 @@ void xhci_cleanup(struct xhci_ctrl *ctrl) xhci_free_virt_devices(ctrl); free(ctrl->erst.entries); free(ctrl->dcbaa);
- if (reset_valid(&ctrl->reset))
reset_free(&ctrl->reset);}
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index aaa243f291..ea8e8f3211 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -10,9 +10,14 @@ #include <init.h> #include <log.h> #include <pci.h> +#include <reset.h> #include <usb.h> #include <usb/xhci.h>
+struct xhci_pci_plat {
- struct reset_ctl reset;
+};
static int xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr, struct xhci_hcor **ret_hcor) { @@ -45,15 +50,53 @@ static int xhci_pci_init(struct udevice *dev, struct xhci_hccr **ret_hccr,
static int xhci_pci_probe(struct udevice *dev) {
struct xhci_pci_plat = dev_get_plat(dev); struct xhci_hccr *hccr; struct xhci_hcor *hcor; int ret;
ret = reset_get_by_index(dev, 0, &plat->reset);
if (ret && ret != -ENOENT && ret != -ENOTSUPP) {
dev_err(dev, "failed to get reset\n");return ret;}
if (reset_valid(&plat->reset)) {
ret = reset_assert(&plat->reset);if (ret)goto err_reset;ret = reset_deassert(&plat->reset);if (ret)goto err_reset;}
ret = xhci_pci_init(dev, &hccr, &hcor); if (ret)
return ret;
goto err_reset;- ret = xhci_register(dev, hccr, hcor);
- if (ret)
goto err_reset;- return 0;
+err_reset:
- if (reset_valid(&plat->reset))
reset_free(&plat->reset);- return ret;
+}
+static int xhci_pci_remove(struct udevice *dev) +{
- struct xhci_pci_plat = dev_get_plat(dev);
- return xhci_register(dev, hccr, hcor);
- xhci_deregister(dev);
- if (reset_valid(&plat->reset))
reset_free(&plat->reset);- return 0;
}
static const struct udevice_id xhci_pci_ids[] = { @@ -65,10 +108,10 @@ U_BOOT_DRIVER(xhci_pci) = { .name = "xhci_pci", .id = UCLASS_USB, .probe = xhci_pci_probe,
- .remove = xhci_deregister,
- .remove = xhci_pci_remove, .of_match = xhci_pci_ids, .ops = &xhci_usb_ops,
- .plat_auto = sizeof(struct usb_plat),
- .plat_auto = sizeof(struct xhci_pci_plat), .priv_auto = sizeof(struct xhci_ctrl), .flags = DM_FLAG_OS_PREPARE | DM_FLAG_ALLOC_PRIV_DMA,
}; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index d27ac01c83..452dacc0af 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -188,37 +188,6 @@ static int xhci_start(struct xhci_hcor *hcor) return ret; }
-#if CONFIG_IS_ENABLED(DM_USB) -/**
- Resets XHCI Hardware
- @param ctrl pointer to host controller
- @return 0 if OK, or a negative error code.
- */
-static int xhci_reset_hw(struct xhci_ctrl *ctrl) -{
- int ret;
- ret = reset_get_by_index(ctrl->dev, 0, &ctrl->reset);
- if (ret && ret != -ENOENT && ret != -ENOTSUPP) {
dev_err(ctrl->dev, "failed to get reset\n");return ret;- }
- if (reset_valid(&ctrl->reset)) {
ret = reset_assert(&ctrl->reset);if (ret)return ret;ret = reset_deassert(&ctrl->reset);if (ret)return ret;- }
- return 0;
-} -#endif
/**
- Resets the XHCI Controller
@@ -1534,10 +1503,6 @@ int xhci_register(struct udevice *dev, struct xhci_hccr *hccr,
ctrl->dev = dev;
- ret = xhci_reset_hw(ctrl);
- if (ret)
goto err;- /*
- XHCI needs to issue a Address device command to setup
- proper device context structures, before it can interact
diff --git a/include/usb/xhci.h b/include/usb/xhci.h index 8d95e213b0..01e63cf0fc 100644 --- a/include/usb/xhci.h +++ b/include/usb/xhci.h @@ -17,7 +17,6 @@ #define HOST_XHCI_H_
#include <phys2bus.h> -#include <reset.h> #include <asm/types.h> #include <asm/cache.h> #include <asm/io.h> @@ -1200,7 +1199,6 @@ struct xhci_ctrl { #if CONFIG_IS_ENABLED(DM_USB) struct udevice *dev; #endif
- struct reset_ctl reset; struct xhci_hccr *hccr; /* R/O registers, not need for volatile */ struct xhci_hcor *hcor; struct xhci_doorbell_array *dba;
On Sat, Apr 17, 2021 at 10:21 PM Samuel Holland samuel@sholland.org wrote:
Resetting an XHCI controller inside xhci_register undoes any register setup performed by the platform driver. And at least on the Allwinner H6, resetting the XHCI controller also resets the PHY, which prevents the controller from working. That means the controller must be taken out of reset before initializing the PHY, which must be done before calling xhci_register.
The logic in the XHCI core was added to support the Raspberry Pi 4 (although this was not mentioned in the commit log!), which uses the xhci-pci platform driver. Move the reset logic to the platform driver, where it belongs, and where it cannot interfere with other platform drivers.
This also fixes a failure to call reset_free if xhci_register failed.
Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") Signed-off-by: Samuel Holland samuel@sholland.org
drivers/usb/host/xhci-mem.c | 2 -- drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.c | 35 ------------------------- include/usb/xhci.h | 2 -- 4 files changed, 47 insertions(+), 43 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
On 7/5/21 10:04 AM, Bin Meng wrote:
On Sat, Apr 17, 2021 at 10:21 PM Samuel Holland samuel@sholland.org wrote:
Resetting an XHCI controller inside xhci_register undoes any register setup performed by the platform driver. And at least on the Allwinner H6, resetting the XHCI controller also resets the PHY, which prevents the controller from working. That means the controller must be taken out of reset before initializing the PHY, which must be done before calling xhci_register.
The logic in the XHCI core was added to support the Raspberry Pi 4 (although this was not mentioned in the commit log!), which uses the xhci-pci platform driver. Move the reset logic to the platform driver, where it belongs, and where it cannot interfere with other platform drivers.
This also fixes a failure to call reset_free if xhci_register failed.
Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") Signed-off-by: Samuel Holland samuel@sholland.org
drivers/usb/host/xhci-mem.c | 2 -- drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.c | 35 ------------------------- include/usb/xhci.h | 2 -- 4 files changed, 47 insertions(+), 43 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
So shall we apply this whole thing for 2021.10 ?
On Mon, Jul 5, 2021 at 4:19 PM Marek Vasut marex@denx.de wrote:
On 7/5/21 10:04 AM, Bin Meng wrote:
On Sat, Apr 17, 2021 at 10:21 PM Samuel Holland samuel@sholland.org wrote:
Resetting an XHCI controller inside xhci_register undoes any register setup performed by the platform driver. And at least on the Allwinner H6, resetting the XHCI controller also resets the PHY, which prevents the controller from working. That means the controller must be taken out of reset before initializing the PHY, which must be done before calling xhci_register.
The logic in the XHCI core was added to support the Raspberry Pi 4 (although this was not mentioned in the commit log!), which uses the xhci-pci platform driver. Move the reset logic to the platform driver, where it belongs, and where it cannot interfere with other platform drivers.
This also fixes a failure to call reset_free if xhci_register failed.
Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") Signed-off-by: Samuel Holland samuel@sholland.org
drivers/usb/host/xhci-mem.c | 2 -- drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.c | 35 ------------------------- include/usb/xhci.h | 2 -- 4 files changed, 47 insertions(+), 43 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
So shall we apply this whole thing for 2021.10 ?
Yes. Andre wanted to get this in 2021.07 which is too late.
Regards, Bin
On Mon, 5 Jul 2021 16:38:29 +0800 Bin Meng bmeng.cn@gmail.com wrote:
Hi,
On Mon, Jul 5, 2021 at 4:19 PM Marek Vasut marex@denx.de wrote:
On 7/5/21 10:04 AM, Bin Meng wrote:
On Sat, Apr 17, 2021 at 10:21 PM Samuel Holland samuel@sholland.org wrote:
Resetting an XHCI controller inside xhci_register undoes any register setup performed by the platform driver. And at least on the Allwinner H6, resetting the XHCI controller also resets the PHY, which prevents the controller from working. That means the controller must be taken out of reset before initializing the PHY, which must be done before calling xhci_register.
The logic in the XHCI core was added to support the Raspberry Pi 4 (although this was not mentioned in the commit log!), which uses the xhci-pci platform driver. Move the reset logic to the platform driver, where it belongs, and where it cannot interfere with other platform drivers.
This also fixes a failure to call reset_free if xhci_register failed.
Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") Signed-off-by: Samuel Holland samuel@sholland.org
drivers/usb/host/xhci-mem.c | 2 -- drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.c | 35 ------------------------- include/usb/xhci.h | 2 -- 4 files changed, 47 insertions(+), 43 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
So shall we apply this whole thing for 2021.10 ?
Yes. Andre wanted to get this in 2021.07 which is too late.
Ah, sorry, I didn't mean into this release, but into the 2021.10 merge window. I was preparing the sunxi patches for the PR, so stumbled upon this.
So I'd be grateful if you could push this into the MW, I can then finish up the sunxi side of things (patch 4/4).
Thanks! Andre
Hi Andre,
On Mon, Jul 5, 2021 at 5:07 PM Andre Przywara andre.przywara@arm.com wrote:
On Mon, 5 Jul 2021 16:38:29 +0800 Bin Meng bmeng.cn@gmail.com wrote:
Hi,
On Mon, Jul 5, 2021 at 4:19 PM Marek Vasut marex@denx.de wrote:
On 7/5/21 10:04 AM, Bin Meng wrote:
On Sat, Apr 17, 2021 at 10:21 PM Samuel Holland samuel@sholland.org wrote:
Resetting an XHCI controller inside xhci_register undoes any register setup performed by the platform driver. And at least on the Allwinner H6, resetting the XHCI controller also resets the PHY, which prevents the controller from working. That means the controller must be taken out of reset before initializing the PHY, which must be done before calling xhci_register.
The logic in the XHCI core was added to support the Raspberry Pi 4 (although this was not mentioned in the commit log!), which uses the xhci-pci platform driver. Move the reset logic to the platform driver, where it belongs, and where it cannot interfere with other platform drivers.
This also fixes a failure to call reset_free if xhci_register failed.
Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") Signed-off-by: Samuel Holland samuel@sholland.org
drivers/usb/host/xhci-mem.c | 2 -- drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.c | 35 ------------------------- include/usb/xhci.h | 2 -- 4 files changed, 47 insertions(+), 43 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
So shall we apply this whole thing for 2021.10 ?
Yes. Andre wanted to get this in 2021.07 which is too late.
Ah, sorry, I didn't mean into this release, but into the 2021.10 merge window. I was preparing the sunxi patches for the PR, so stumbled upon this.
Ah, 2021.01 is not a problem.
So I'd be grateful if you could push this into the MW, I can then finish up the sunxi side of things (patch 4/4).
Marek can pick up this soon.
Regards, Bin
On 7/5/21 11:18 AM, Bin Meng wrote:
Hi Andre,
On Mon, Jul 5, 2021 at 5:07 PM Andre Przywara andre.przywara@arm.com wrote:
On Mon, 5 Jul 2021 16:38:29 +0800 Bin Meng bmeng.cn@gmail.com wrote:
Hi,
On Mon, Jul 5, 2021 at 4:19 PM Marek Vasut marex@denx.de wrote:
On 7/5/21 10:04 AM, Bin Meng wrote:
On Sat, Apr 17, 2021 at 10:21 PM Samuel Holland samuel@sholland.org wrote:
Resetting an XHCI controller inside xhci_register undoes any register setup performed by the platform driver. And at least on the Allwinner H6, resetting the XHCI controller also resets the PHY, which prevents the controller from working. That means the controller must be taken out of reset before initializing the PHY, which must be done before calling xhci_register.
The logic in the XHCI core was added to support the Raspberry Pi 4 (although this was not mentioned in the commit log!), which uses the xhci-pci platform driver. Move the reset logic to the platform driver, where it belongs, and where it cannot interfere with other platform drivers.
This also fixes a failure to call reset_free if xhci_register failed.
Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") Signed-off-by: Samuel Holland samuel@sholland.org
drivers/usb/host/xhci-mem.c | 2 -- drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.c | 35 ------------------------- include/usb/xhci.h | 2 -- 4 files changed, 47 insertions(+), 43 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
So shall we apply this whole thing for 2021.10 ?
Yes. Andre wanted to get this in 2021.07 which is too late.
Ah, sorry, I didn't mean into this release, but into the 2021.10 merge window. I was preparing the sunxi patches for the PR, so stumbled upon this.
Ah, 2021.01 is not a problem.
So I'd be grateful if you could push this into the MW, I can then finish up the sunxi side of things (patch 4/4).
Marek can pick up this soon.
U-Boot CI seems to fail on this, please recheck that.
On Mon, 5 Jul 2021 12:45:59 +0200 Marek Vasut marex@denx.de wrote:
Hi,
On 7/5/21 11:18 AM, Bin Meng wrote:
Hi Andre,
On Mon, Jul 5, 2021 at 5:07 PM Andre Przywara andre.przywara@arm.com wrote:
On Mon, 5 Jul 2021 16:38:29 +0800 Bin Meng bmeng.cn@gmail.com wrote:
Hi,
On Mon, Jul 5, 2021 at 4:19 PM Marek Vasut marex@denx.de wrote:
On 7/5/21 10:04 AM, Bin Meng wrote:
On Sat, Apr 17, 2021 at 10:21 PM Samuel Holland samuel@sholland.org wrote: > > Resetting an XHCI controller inside xhci_register undoes any register > setup performed by the platform driver. And at least on the Allwinner > H6, resetting the XHCI controller also resets the PHY, which prevents > the controller from working. That means the controller must be taken out > of reset before initializing the PHY, which must be done before calling > xhci_register. > > The logic in the XHCI core was added to support the Raspberry Pi 4 > (although this was not mentioned in the commit log!), which uses the > xhci-pci platform driver. Move the reset logic to the platform driver, > where it belongs, and where it cannot interfere with other platform > drivers. > > This also fixes a failure to call reset_free if xhci_register failed. > > Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") > Signed-off-by: Samuel Holland samuel@sholland.org > --- > drivers/usb/host/xhci-mem.c | 2 -- > drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- > drivers/usb/host/xhci.c | 35 ------------------------- > include/usb/xhci.h | 2 -- > 4 files changed, 47 insertions(+), 43 deletions(-) >
Reviewed-by: Bin Meng bmeng.cn@gmail.com
So shall we apply this whole thing for 2021.10 ?
Yes. Andre wanted to get this in 2021.07 which is too late.
Ah, sorry, I didn't mean into this release, but into the 2021.10 merge window. I was preparing the sunxi patches for the PR, so stumbled upon this.
Ah, 2021.01 is not a problem.
So I'd be grateful if you could push this into the MW, I can then finish up the sunxi side of things (patch 4/4).
Marek can pick up this soon.
U-Boot CI seems to fail on this, please recheck that.
Ouch, thanks for the heads up! It's xhci-pci.c failing, as used by the RPi4, for instance.
I will send a v3 ASAP.
Cheers, Andre
On 7/5/21 10:38 AM, Bin Meng wrote:
On Mon, Jul 5, 2021 at 4:19 PM Marek Vasut marex@denx.de wrote:
On 7/5/21 10:04 AM, Bin Meng wrote:
On Sat, Apr 17, 2021 at 10:21 PM Samuel Holland samuel@sholland.org wrote:
Resetting an XHCI controller inside xhci_register undoes any register setup performed by the platform driver. And at least on the Allwinner H6, resetting the XHCI controller also resets the PHY, which prevents the controller from working. That means the controller must be taken out of reset before initializing the PHY, which must be done before calling xhci_register.
The logic in the XHCI core was added to support the Raspberry Pi 4 (although this was not mentioned in the commit log!), which uses the xhci-pci platform driver. Move the reset logic to the platform driver, where it belongs, and where it cannot interfere with other platform drivers.
This also fixes a failure to call reset_free if xhci_register failed.
Fixes: 0b80371b350e ("usb: xhci: Add reset controller support") Signed-off-by: Samuel Holland samuel@sholland.org
drivers/usb/host/xhci-mem.c | 2 -- drivers/usb/host/xhci-pci.c | 51 ++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.c | 35 ------------------------- include/usb/xhci.h | 2 -- 4 files changed, 47 insertions(+), 43 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
So shall we apply this whole thing for 2021.10 ?
Yes. Andre wanted to get this in 2021.07 which is too late.
Applied to usb/next, thanks
Some platforms, like the Allwinner H6, do not have a separate glue layer around the dwc3. Instead, they rely on the clocks/resets/phys referenced from the dwc3 DT node itself. Add support for enabling the clocks/resets referenced from the dwc3 DT node.
Signed-off-by: Samuel Holland samuel@sholland.org --- drivers/usb/host/xhci-dwc3.c | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c index 3e0ae80cec..5b12d1358e 100644 --- a/drivers/usb/host/xhci-dwc3.c +++ b/drivers/usb/host/xhci-dwc3.c @@ -7,10 +7,12 @@ * Author: Ramneek Mehreshramneek.mehresh@freescale.com */
+#include <clk.h> #include <common.h> #include <dm.h> #include <generic-phy.h> #include <log.h> +#include <reset.h> #include <usb.h> #include <dwc3-uboot.h> #include <linux/delay.h> @@ -21,7 +23,9 @@ #include <linux/usb/otg.h>
struct xhci_dwc3_plat { + struct clk_bulk clks; struct phy_bulk phys; + struct reset_ctl_bulk resets; };
void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode) @@ -111,6 +115,46 @@ void dwc3_set_fladj(struct dwc3 *dwc3_reg, u32 val) }
#if CONFIG_IS_ENABLED(DM_USB) +static int xhci_dwc3_reset_init(struct udevice *dev, + struct xhci_dwc3_plat *plat) +{ + int ret; + + ret = reset_get_bulk(dev, &plat->resets); + if (ret == -ENOTSUPP || ret == -ENOENT) + return 0; + else if (ret) + return ret; + + ret = reset_deassert_bulk(&plat->resets); + if (ret) { + reset_release_bulk(&plat->resets); + return ret; + } + + return 0; +} + +static int xhci_dwc3_clk_init(struct udevice *dev, + struct xhci_dwc3_plat *plat) +{ + int ret; + + ret = clk_get_bulk(dev, &plat->clks); + if (ret == -ENOSYS || ret == -ENOENT) + return 0; + if (ret) + return ret; + + ret = clk_enable_bulk(&plat->clks); + if (ret) { + clk_release_bulk(&plat->clks); + return ret; + } + + return 0; +} + static int xhci_dwc3_probe(struct udevice *dev) { struct xhci_hcor *hcor; @@ -122,6 +166,14 @@ static int xhci_dwc3_probe(struct udevice *dev) u32 reg; int ret;
+ ret = xhci_dwc3_reset_init(dev, plat); + if (ret) + return ret; + + ret = xhci_dwc3_clk_init(dev, plat); + if (ret) + return ret; + hccr = (struct xhci_hccr *)((uintptr_t)dev_remap_addr(dev)); hcor = (struct xhci_hcor *)((uintptr_t)hccr + HC_LENGTH(xhci_readl(&(hccr)->cr_capbase))); @@ -171,6 +223,10 @@ static int xhci_dwc3_remove(struct udevice *dev)
dwc3_shutdown_phy(dev, &plat->phys);
+ clk_release_bulk(&plat->clks); + + reset_release_bulk(&plat->resets); + return xhci_deregister(dev); }
On Sat, 17 Apr 2021 09:20:58 -0500 Samuel Holland samuel@sholland.org wrote:
Some platforms, like the Allwinner H6, do not have a separate glue layer around the dwc3. Instead, they rely on the clocks/resets/phys referenced from the dwc3 DT node itself. Add support for enabling the clocks/resets referenced from the dwc3 DT node.
Signed-off-by: Samuel Holland samuel@sholland.org
Looks good, and even if they are redundancies with the other kind of binding, it should not hurt to have it in, to cover our DTs.
Reviewed-by: Andre Przywara andre.przywara@arm.com
Cheers, Andre
drivers/usb/host/xhci-dwc3.c | 56 ++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/drivers/usb/host/xhci-dwc3.c b/drivers/usb/host/xhci-dwc3.c index 3e0ae80cec..5b12d1358e 100644 --- a/drivers/usb/host/xhci-dwc3.c +++ b/drivers/usb/host/xhci-dwc3.c @@ -7,10 +7,12 @@
- Author: Ramneek Mehreshramneek.mehresh@freescale.com
*/
+#include <clk.h> #include <common.h> #include <dm.h> #include <generic-phy.h> #include <log.h> +#include <reset.h> #include <usb.h> #include <dwc3-uboot.h> #include <linux/delay.h> @@ -21,7 +23,9 @@ #include <linux/usb/otg.h>
struct xhci_dwc3_plat {
- struct clk_bulk clks; struct phy_bulk phys;
- struct reset_ctl_bulk resets;
};
void dwc3_set_mode(struct dwc3 *dwc3_reg, u32 mode) @@ -111,6 +115,46 @@ void dwc3_set_fladj(struct dwc3 *dwc3_reg, u32 val) }
#if CONFIG_IS_ENABLED(DM_USB) +static int xhci_dwc3_reset_init(struct udevice *dev,
struct xhci_dwc3_plat *plat)+{
- int ret;
- ret = reset_get_bulk(dev, &plat->resets);
- if (ret == -ENOTSUPP || ret == -ENOENT)
return 0;- else if (ret)
return ret;- ret = reset_deassert_bulk(&plat->resets);
- if (ret) {
reset_release_bulk(&plat->resets);return ret;- }
- return 0;
+}
+static int xhci_dwc3_clk_init(struct udevice *dev,
struct xhci_dwc3_plat *plat)+{
- int ret;
- ret = clk_get_bulk(dev, &plat->clks);
- if (ret == -ENOSYS || ret == -ENOENT)
return 0;- if (ret)
return ret;- ret = clk_enable_bulk(&plat->clks);
- if (ret) {
clk_release_bulk(&plat->clks);return ret;- }
- return 0;
+}
static int xhci_dwc3_probe(struct udevice *dev) { struct xhci_hcor *hcor; @@ -122,6 +166,14 @@ static int xhci_dwc3_probe(struct udevice *dev) u32 reg; int ret;
- ret = xhci_dwc3_reset_init(dev, plat);
- if (ret)
return ret;- ret = xhci_dwc3_clk_init(dev, plat);
- if (ret)
return ret;- hccr = (struct xhci_hccr *)((uintptr_t)dev_remap_addr(dev)); hcor = (struct xhci_hcor *)((uintptr_t)hccr + HC_LENGTH(xhci_readl(&(hccr)->cr_capbase)));
@@ -171,6 +223,10 @@ static int xhci_dwc3_remove(struct udevice *dev)
dwc3_shutdown_phy(dev, &plat->phys);
- clk_release_bulk(&plat->clks);
- reset_release_bulk(&plat->resets);
- return xhci_deregister(dev);
}
Pine H64 and Orange Pi 3 both provide a USB3 type A port. Enable it in U-Boot.
Signed-off-by: Samuel Holland samuel@sholland.org --- configs/orangepi_3_defconfig | 5 +++++ configs/pine_h64_defconfig | 5 +++++ 2 files changed, 10 insertions(+)
diff --git a/configs/orangepi_3_defconfig b/configs/orangepi_3_defconfig index 82b9815205..eb25bd9f50 100644 --- a/configs/orangepi_3_defconfig +++ b/configs/orangepi_3_defconfig @@ -8,5 +8,10 @@ CONFIG_MMC_SUNXI_SLOT_EXTRA=2 CONFIG_BLUETOOTH_DT_DEVICE_FIXUP="brcm,bcm4345c5" CONFIG_DEFAULT_DEVICE_TREE="sun50i-h6-orangepi-3" # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set +CONFIG_PHY_SUN50I_USB3=y +CONFIG_USB_XHCI_HCD=y +CONFIG_USB_XHCI_DWC3=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_OHCI_HCD=y +CONFIG_USB_DWC3=y +# CONFIG_USB_DWC3_GADGET is not set diff --git a/configs/pine_h64_defconfig b/configs/pine_h64_defconfig index 2fa66f3834..0095fb222e 100644 --- a/configs/pine_h64_defconfig +++ b/configs/pine_h64_defconfig @@ -12,5 +12,10 @@ CONFIG_SPL_SPI_SUNXI=y CONFIG_DEFAULT_DEVICE_TREE="sun50i-h6-pine-h64" # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_SUN8I_EMAC=y +CONFIG_PHY_SUN50I_USB3=y +CONFIG_USB_XHCI_HCD=y +CONFIG_USB_XHCI_DWC3=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_OHCI_HCD=y +CONFIG_USB_DWC3=y +# CONFIG_USB_DWC3_GADGET is not set
On Sat, 17 Apr 2021 09:20:59 -0500 Samuel Holland samuel@sholland.org wrote:
Pine H64 and Orange Pi 3 both provide a USB3 type A port. Enable it in U-Boot.
Any reason to enable it only for those two boards? Just because those are the H6 devices you have access to?
I wonder if we should enable this in Kconfig for all H6 boards? Or at least reduce the number of lines in defconfig, by adding select's in Kconfig?
Also: this doesn't work out of the box for the Pine H64, right? Because the official DT keeps the dwc3 and usb3phy nodes disabled. I was planing on sending a DT fix to the kernel.
But with those nodes enabled it indeed works for me, and adds another usable USB port to those H6 boards.
Cheers, Andre
Signed-off-by: Samuel Holland samuel@sholland.org
configs/orangepi_3_defconfig | 5 +++++ configs/pine_h64_defconfig | 5 +++++ 2 files changed, 10 insertions(+)
diff --git a/configs/orangepi_3_defconfig b/configs/orangepi_3_defconfig index 82b9815205..eb25bd9f50 100644 --- a/configs/orangepi_3_defconfig +++ b/configs/orangepi_3_defconfig @@ -8,5 +8,10 @@ CONFIG_MMC_SUNXI_SLOT_EXTRA=2 CONFIG_BLUETOOTH_DT_DEVICE_FIXUP="brcm,bcm4345c5" CONFIG_DEFAULT_DEVICE_TREE="sun50i-h6-orangepi-3" # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set +CONFIG_PHY_SUN50I_USB3=y +CONFIG_USB_XHCI_HCD=y +CONFIG_USB_XHCI_DWC3=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_OHCI_HCD=y +CONFIG_USB_DWC3=y +# CONFIG_USB_DWC3_GADGET is not set diff --git a/configs/pine_h64_defconfig b/configs/pine_h64_defconfig index 2fa66f3834..0095fb222e 100644 --- a/configs/pine_h64_defconfig +++ b/configs/pine_h64_defconfig @@ -12,5 +12,10 @@ CONFIG_SPL_SPI_SUNXI=y CONFIG_DEFAULT_DEVICE_TREE="sun50i-h6-pine-h64" # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set CONFIG_SUN8I_EMAC=y +CONFIG_PHY_SUN50I_USB3=y +CONFIG_USB_XHCI_HCD=y +CONFIG_USB_XHCI_DWC3=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_OHCI_HCD=y +CONFIG_USB_DWC3=y +# CONFIG_USB_DWC3_GADGET is not set
On Sat, 17 Apr 2021 09:20:55 -0500 Samuel Holland samuel@sholland.org wrote:
Hi,
This series adds PHY and XHCI driver support for the USB3 controller found in the Allwinner H6 SoC.
Thanks for the update!
It has been tested and working on both boards enabled in patch 4, although some users experience issues[1].
So I could not reproduce those issues either, it works for me fine on my Pine-H64. I'd suggest we merge those patches, and check for more reports from more users.
Bin, Marek: can you push patches 1, 2 and 3 to the USB tree, to get them into the current merge window, still? I would then push 4/4 (pending possible fixes) once the first three reached mainline.
And btw: the first two patches of the original v1 series (adding the sunxi clocks and reset bits) have been merged into master last week already.
Thanks, Andre
Changes from v1:
- Dropped patches 1-2 (already in u-boot-sunxi/master) and rebased
- Added Andre's Reviewed-by on the PHY driver
- Fixed error handling in xhci_pci_probe
Samuel Holland (4): phy: sun50i-usb3: Add a driver for the H6 USB3 PHY usb: xhci-pci: Move reset logic out of XHCI core usb: xhci-dwc3: Add support for clocks/resets configs: Enable USB3 on Allwinner H6 boards
configs/orangepi_3_defconfig | 5 + configs/pine_h64_defconfig | 5 + drivers/phy/allwinner/Kconfig | 8 ++ drivers/phy/allwinner/Makefile | 1 + drivers/phy/allwinner/phy-sun50i-usb3.c | 171 ++++++++++++++++++++++++ drivers/usb/host/xhci-dwc3.c | 56 ++++++++ drivers/usb/host/xhci-mem.c | 2 - drivers/usb/host/xhci-pci.c | 51 ++++++- drivers/usb/host/xhci.c | 35 ----- include/usb/xhci.h | 2 - 10 files changed, 293 insertions(+), 43 deletions(-) create mode 100644 drivers/phy/allwinner/phy-sun50i-usb3.c
On 4/21/21 12:43 PM, Andre Przywara wrote:
On Sat, 17 Apr 2021 09:20:55 -0500 Samuel Holland samuel@sholland.org wrote:
Hi,
This series adds PHY and XHCI driver support for the USB3 controller found in the Allwinner H6 SoC.
Thanks for the update!
It has been tested and working on both boards enabled in patch 4, although some users experience issues[1].
So I could not reproduce those issues either, it works for me fine on my Pine-H64. I'd suggest we merge those patches, and check for more reports from more users.
Bin, Marek: can you push patches 1, 2 and 3 to the USB tree, to get them into the current merge window, still? I would then push 4/4 (pending possible fixes) once the first three reached mainline.
And btw: the first two patches of the original v1 series (adding the sunxi clocks and reset bits) have been merged into master last week already.
I will pick these once I get ack from Bin.
On Wed, 21 Apr 2021 12:53:42 +0200 Marek Vasut marex@denx.de wrote:
Hi,
On 4/21/21 12:43 PM, Andre Przywara wrote:
On Sat, 17 Apr 2021 09:20:55 -0500 Samuel Holland samuel@sholland.org wrote:
Hi,
This series adds PHY and XHCI driver support for the USB3 controller found in the Allwinner H6 SoC.
Thanks for the update!
It has been tested and working on both boards enabled in patch 4, although some users experience issues[1].
So I could not reproduce those issues either, it works for me fine on my Pine-H64. I'd suggest we merge those patches, and check for more reports from more users.
Bin, Marek: can you push patches 1, 2 and 3 to the USB tree, to get them into the current merge window, still? I would then push 4/4 (pending possible fixes) once the first three reached mainline.
And btw: the first two patches of the original v1 series (adding the sunxi clocks and reset bits) have been merged into master last week already.
I will pick these once I get ack from Bin.
Bin, did you have a chance to have a look at those patches? I wonder if we could merge them in the MW, to expose them to a wider audience?
Cheers, Andre
Hi Andre,
On Mon, Jul 5, 2021 at 7:10 AM Andre Przywara andre.przywara@arm.com wrote:
On Wed, 21 Apr 2021 12:53:42 +0200 Marek Vasut marex@denx.de wrote:
Hi,
On 4/21/21 12:43 PM, Andre Przywara wrote:
On Sat, 17 Apr 2021 09:20:55 -0500 Samuel Holland samuel@sholland.org wrote:
Hi,
This series adds PHY and XHCI driver support for the USB3 controller found in the Allwinner H6 SoC.
Thanks for the update!
It has been tested and working on both boards enabled in patch 4, although some users experience issues[1].
So I could not reproduce those issues either, it works for me fine on my Pine-H64. I'd suggest we merge those patches, and check for more reports from more users.
Bin, Marek: can you push patches 1, 2 and 3 to the USB tree, to get them into the current merge window, still? I would then push 4/4 (pending possible fixes) once the first three reached mainline.
And btw: the first two patches of the original v1 series (adding the sunxi clocks and reset bits) have been merged into master last week already.
I will pick these once I get ack from Bin.
Bin, did you have a chance to have a look at those patches? I wonder if we could merge them in the MW, to expose them to a wider audience?
It looks only this patch is not applied? http://patchwork.ozlabs.org/project/uboot/patch/20210417142059.45337-3-samue...
I am afraid this is too late as the release date is today.
Regards, Bin
participants (4)
-
Andre Przywara -
Bin Meng -
Marek Vasut -
Samuel Holland