[U-Boot] [PATCH v2] sunxi: A64: OHCI: prevent turning off shared USB clock
On the A64 the clock for the first USB controller is actually the parent of the clock for the second controller, so turning them off in that order makes the system hang. Fix this by only turning off *both* clocks when the *last* OHCI controller is brought down. This covers the case when only one controller is used.
Signed-off-by: Andre Przywara andre.przywara@arm.com --- Hi,
as requested by Marek, a second version to address the problem of only one controller instantiated. I tested all cases: - only [EO]HCI1 enabled (current U-Boot master DT) - both controllers enabled (mainline Linux DT) - only [EO]HCI0 enabled (DT hack) In all cases the system booted without hanging, plus I confirmed that the USB clocks were disabled in all cases (early in the kernel).
Cheers, Andre.
P.S. I found the MMC0, EMAC and USB-OTG AHB gates and resets still running, but this is an unrelated issue and no regression. Just in case somebody feels bored ...
drivers/usb/host/ohci-sunxi.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c index 0ddbdbe460..bb3c2475df 100644 --- a/drivers/usb/host/ohci-sunxi.c +++ b/drivers/usb/host/ohci-sunxi.c @@ -44,6 +44,8 @@ struct ohci_sunxi_priv { const struct ohci_sunxi_cfg *cfg; };
+static fdt_addr_t last_ohci_addr = 0; + static int ohci_usb_probe(struct udevice *dev) { struct usb_bus_priv *bus_priv = dev_get_uclass_priv(dev); @@ -53,6 +55,9 @@ static int ohci_usb_probe(struct udevice *dev) u8 reg_mask = 0; int phys, ret;
+ if ((fdt_addr_t)regs > last_ohci_addr) + last_ohci_addr = (fdt_addr_t)regs; + priv->cfg = (const struct ohci_sunxi_cfg *)dev_get_driver_data(dev); priv->ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; if (IS_ERR(priv->ccm)) @@ -114,6 +119,7 @@ no_phy: static int ohci_usb_remove(struct udevice *dev) { struct ohci_sunxi_priv *priv = dev_get_priv(dev); + fdt_addr_t base_addr = devfdt_get_addr(dev); int ret;
if (generic_phy_valid(&priv->phy)) { @@ -130,7 +136,18 @@ static int ohci_usb_remove(struct udevice *dev)
if (priv->cfg->has_reset) clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask); - clrbits_le32(&priv->ccm->usb_clk_cfg, priv->usb_gate_mask); + /* + * On the A64 CLK_USB_OHCI0 is the parent of CLK_USB_OHCI1, so + * we have to wait with bringing down any clock until the last + * OHCI controller is removed. + */ + if (!priv->cfg->extra_usb_gate_mask || base_addr == last_ohci_addr) { + u32 usb_gate_mask = priv->usb_gate_mask; + + usb_gate_mask |= priv->cfg->extra_usb_gate_mask; + clrbits_le32(&priv->ccm->usb_clk_cfg, usb_gate_mask); + } + clrbits_le32(&priv->ccm->ahb_gate0, priv->ahb_gate_mask);
return 0;
On Thu, Jul 5, 2018 at 5:27 AM, Andre Przywara andre.przywara@arm.com wrote:
On the A64 the clock for the first USB controller is actually the parent of the clock for the second controller, so turning them off in that order makes the system hang. Fix this by only turning off *both* clocks when the *last* OHCI controller is brought down. This covers the case when only one controller is used.
Signed-off-by: Andre Przywara andre.przywara@arm.com
Hi,
as requested by Marek, a second version to address the problem of only one controller instantiated. I tested all cases:
- only [EO]HCI1 enabled (current U-Boot master DT)
- both controllers enabled (mainline Linux DT)
- only [EO]HCI0 enabled (DT hack)
In all cases the system booted without hanging, plus I confirmed that the USB clocks were disabled in all cases (early in the kernel).
Cheers, Andre.
P.S. I found the MMC0, EMAC and USB-OTG AHB gates and resets still running,
Yes MUSB doesn't have shutdown support yet for UCLASS_USB_DEV_GENERIC so .remove is not able to call. But I've verified this with some change[1] and it's working as usual. will send it soon.
but this is an unrelated issue and no regression. Just in case somebody feels bored ...
drivers/usb/host/ohci-sunxi.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c index 0ddbdbe460..bb3c2475df 100644 --- a/drivers/usb/host/ohci-sunxi.c +++ b/drivers/usb/host/ohci-sunxi.c @@ -44,6 +44,8 @@ struct ohci_sunxi_priv { const struct ohci_sunxi_cfg *cfg; };
+static fdt_addr_t last_ohci_addr = 0;
static int ohci_usb_probe(struct udevice *dev) { struct usb_bus_priv *bus_priv = dev_get_uclass_priv(dev); @@ -53,6 +55,9 @@ static int ohci_usb_probe(struct udevice *dev) u8 reg_mask = 0; int phys, ret;
if ((fdt_addr_t)regs > last_ohci_addr)last_ohci_addr = (fdt_addr_t)regs;priv->cfg = (const struct ohci_sunxi_cfg *)dev_get_driver_data(dev); priv->ccm = (struct sunxi_ccm_reg *)SUNXI_CCM_BASE; if (IS_ERR(priv->ccm))@@ -114,6 +119,7 @@ no_phy: static int ohci_usb_remove(struct udevice *dev) { struct ohci_sunxi_priv *priv = dev_get_priv(dev);
fdt_addr_t base_addr = devfdt_get_addr(dev); int ret; if (generic_phy_valid(&priv->phy)) {@@ -130,7 +136,18 @@ static int ohci_usb_remove(struct udevice *dev)
if (priv->cfg->has_reset) clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
clrbits_le32(&priv->ccm->usb_clk_cfg, priv->usb_gate_mask);
/** On the A64 CLK_USB_OHCI0 is the parent of CLK_USB_OHCI1, so* we have to wait with bringing down any clock until the last* OHCI controller is removed.*/if (!priv->cfg->extra_usb_gate_mask || base_addr == last_ohci_addr) {u32 usb_gate_mask = priv->usb_gate_mask;usb_gate_mask |= priv->cfg->extra_usb_gate_mask;clrbits_le32(&priv->ccm->usb_clk_cfg, usb_gate_mask);}clrbits_le32(&priv->ccm->ahb_gate0, priv->ahb_gate_mask); return 0;--
Reviewed-by: Jagan Teki jagan@openedev.com Tested-by: Jagan Teki jagan@openedev.com
[1] https://github.com/amarula/u-boot-amarula/commit/401f74ebfb21bb92f793d50e2d3...
On 07/05/2018 01:57 AM, Andre Przywara wrote:
On the A64 the clock for the first USB controller is actually the parent of the clock for the second controller, so turning them off in that order makes the system hang. Fix this by only turning off *both* clocks when the *last* OHCI controller is brought down. This covers the case when only one controller is used.
Signed-off-by: Andre Przywara andre.przywara@arm.com
Hi,
as requested by Marek, a second version to address the problem of only one controller instantiated. I tested all cases:
- only [EO]HCI1 enabled (current U-Boot master DT)
- both controllers enabled (mainline Linux DT)
- only [EO]HCI0 enabled (DT hack)
In all cases the system booted without hanging, plus I confirmed that the USB clocks were disabled in all cases (early in the kernel).
Cheers, Andre.
P.S. I found the MMC0, EMAC and USB-OTG AHB gates and resets still running, but this is an unrelated issue and no regression. Just in case somebody feels bored ...
Applied, thanks for all the effort you put into this .
Hi,
On 05/07/18 10:25, Marek Vasut wrote:
On 07/05/2018 01:57 AM, Andre Przywara wrote:
On the A64 the clock for the first USB controller is actually the parent of the clock for the second controller, so turning them off in that order makes the system hang. Fix this by only turning off *both* clocks when the *last* OHCI controller is brought down. This covers the case when only one controller is used.
Signed-off-by: Andre Przywara andre.przywara@arm.com
Hi,
as requested by Marek, a second version to address the problem of only one controller instantiated. I tested all cases:
- only [EO]HCI1 enabled (current U-Boot master DT)
- both controllers enabled (mainline Linux DT)
- only [EO]HCI0 enabled (DT hack)
In all cases the system booted without hanging, plus I confirmed that the USB clocks were disabled in all cases (early in the kernel).
Cheers, Andre.
P.S. I found the MMC0, EMAC and USB-OTG AHB gates and resets still running, but this is an unrelated issue and no regression. Just in case somebody feels bored ...
Applied, thanks for all the effort you put into this .
Thank _you_ for staying on this, and insisting on the more robust solution!
Cheers, Andre.
participants (3)
-
Andre Przywara -
Jagan Teki -
Marek Vasut