[U-Boot] [PATCH] sunxi: A64: OHCI: prevent turning off shared USB clock

[Andre Przywara]

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.przywara at 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?

Cheers,
Andre.