Hi, On 04/07/18 12:10, Marek Vasut wrote: > On 07/04/2018 12:03 PM, Andre Przywara wrote: >> Hi, >> >> On 04/07/18 08:14, Marek Vasut wrote: >>> On 07/04/2018 02:05 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 *not* turning off any clock for OHCI0, but both clocks when >>>> OHCI1 is brought down. >>>> >>>> Signed-off-by: Andre Przywara <andre.przyw...@arm.com> >>>> --- >>>> Hi, >>>> >>>> this is a new approach to fix the USB hang we see with mainline U-Boot. >>>> Compared to the previous patch it just deals with the USB clock (the AHB >>>> gate was a red herring), and it eventually turns both clocks off instead >>>> of leaving them running. Please have a test on A64 boards! >>>> >>>> Cheers, >>>> Andre. >>>> >>>> drivers/usb/host/ohci-sunxi.c | 13 ++++++++++++- >>>> 1 file changed, 12 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c >>>> index 0ddbdbe460..8f108b48a8 100644 >>>> --- a/drivers/usb/host/ohci-sunxi.c >>>> +++ b/drivers/usb/host/ohci-sunxi.c >>>> @@ -114,6 +114,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 +131,17 @@ 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 bring down none for OHCI0, but both for OHCI1. >>>> + */ >>>> + if (!priv->cfg->extra_usb_gate_mask || base_addr >= SUNXI_USB2_BASE) { >>>> + 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; >>>> >>> >>> What about boards which only enable OHCI0 and do not enable OHCI1 ? >> >> Ah, c'mon, thanks for spoiling my short patch ;-) >> The A64 has just two USB controllers, on dedicated pins, so mostly you >> want both of them. But I see your point, the clocks would stay on if >> only the first controller is ever dealt with. Maybe we can live with >> that, at least for the next release? >> Or do you have a clever idea how to deal with that? I think it's hard to >> determine how many USB controller we have enabled? > > I'd prefer approach which works in all cases. > > Can't you check if both controllers are enabled by traversing the DT ?
Nah, that sound's awful. But I could count the number of controllers during initialisation and store this in a static variable. That might also help to avoid the ugly comparison against SUNXI_USB2_BASE. Cheers, Andre. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot