[PATCH v2 0/4] Allwinner H6 USB3 support

[PATCH v2 0/4] Allwinner H6 USB3 support

[Samuel Holland]

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

-- 
2.26.2

[PATCH v2 1/4] phy: sun50i-usb3: Add a driver for the H6 USB3 PHY

[Samuel Holland]

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),
+};
-- 
2.26.2

[PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Samuel Holland]

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

[PATCH v2 3/4] usb: xhci-dwc3: Add support for clocks/resets

[Samuel Holland]

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 Mehresh<ramneek.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);
 }
 
-- 
2.26.2

[PATCH v2 4/4] configs: Enable USB3 on Allwinner H6 boards

[Samuel Holland]

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

[PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Andre Przywara]

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

[PATCH v2 3/4] usb: xhci-dwc3: Add support for clocks/resets

[Andre Przywara]

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 Mehresh<ramneek.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);
>  }
>  

[PATCH v2 4/4] configs: Enable USB3 on Allwinner H6 boards

[Andre Przywara]

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

[PATCH v2 0/4] Allwinner H6 USB3 support

[Andre Przywara]

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].
> 
> [1]: https://lists.denx.de/pipermail/u-boot/2021-February/440767.html

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
> 

[PATCH v2 0/4] Allwinner H6 USB3 support

[Marek Vasut]

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].
>>
>> [1]: https://lists.denx.de/pipermail/u-boot/2021-February/440767.html
> 
> 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.

[PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Samuel Holland]

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 at sholland.org/

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

[PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Andre Przywara]

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

> 
> [1]:
> https://lore.kernel.org/linux-sunxi/20210421042834.27309-3-samuel at sholland.org/
> 
> > 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);
> >>  	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;  
> >   
> 

Re: [PATCH v2 0/4] Allwinner H6 USB3 support

[Andre Przywara]

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].
> >>
> >> [1]: https://lists.denx.de/pipermail/u-boot/2021-February/440767.html  
> > 
> > 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

Re: [PATCH v2 0/4] Allwinner H6 USB3 support

[Bin Meng]

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].
> > >>
> > >> [1]: https://lists.denx.de/pipermail/u-boot/2021-February/440767.html
> > >
> > > 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-samuel@sholland.org/

I am afraid this is too late as the release date is today.

Regards,
Bin

Re: [PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Bin Meng]

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>

Re: [PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Marek Vasut]

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 ?

Re: [PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Bin Meng]

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

Re: [PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Andre Przywara]

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

Re: [PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Bin Meng]

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

Re: [PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Marek Vasut]

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

Re: [PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Marek Vasut]

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.

Re: [PATCH v2 2/4] usb: xhci-pci: Move reset logic out of XHCI core

[Andre Przywara]

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