[U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates

[Andre Przywara]

Hi,

On 03/07/18 09:24, Marek Vasut wrote:
> On 07/03/2018 01:44 AM, Andre Przywara wrote:
>> The USB host controllers on the H3, H5 and A64 have the oddity of
>> sharing some clock and reset gates, so both the OHCI and EHCI bits have
>> to be enabled to make only one of them working. We take care of this, and
>> initialisation works fine (due to setting already set bits).
>> However on shutdown we turn the clocks and reset gates off already when
>> deregistering one controller, so the other one is no longer functional.
>> In the result U-Boot complains just before launching the kernel and
>> then hangs.
>> Fix this by not turning off the clocks and resets on the OHCI side, so
>> that the EHCI controller has a chance to properly shut down.
>> This still isn't perfect, but at least prevents the hang.
>>
>> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> 
> What about adding some enable/disable counter to those clock somehow and
> then turning them off when the counter reaches zero ?

Yes, that's what I meant with "proper ref-counting" below. The problem
is that this would need to go across the two files ([oe]hci-sunxi.c),
and be per bit. So it wouldn't be too pretty or easy.
But the clocks and resets should be handled by a proper DM driver, which
is just around the corner (check the link to this branch on my github
below). So I don't like to spend too much time on it. We don't need USB
for the SPL, so we can actually remove this code once we have DM_CLK.
For now I just need to fix that hang to unblock the DT updates.

Cheers,
Andre.

>> ---
>> Hi,
>>
>> commit b62cdbddedc3 ("sunxi: clock: Fix EHCI and OHCI clocks on A64")
>> introduced the proper reset and clock gates for the A64. While the patch
>> itself is correct, it broke Linux as soon as one actually enables USB0 in
>> the DT (in the moment we keep this "disabled" in U-Boot's DT version).
>>
>> I understand that this patch here is somewhat of a hack, but the proper
>> ref-counting is not easy to implement between the separate EHCI and OHCI
>> drivers. Those two files are doomed anyway, since driver model clocks
>> and reset drivers are already on the horizon:
>> https://github.com/apritzel/u-boot/commits/sunxi-dm-WIP
>> So lets fix this up for now so that we can use the Linux kernel DTs with
>> U-Boot itself.
>>
>> Cheers,
>> Andre.
>>
>>  drivers/usb/host/ohci-sunxi.c | 16 ++++++++++++----
>>  1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/host/ohci-sunxi.c b/drivers/usb/host/ohci-sunxi.c
>> index 0ddbdbe460..42ffb6cbcb 100644
>> --- a/drivers/usb/host/ohci-sunxi.c
>> +++ b/drivers/usb/host/ohci-sunxi.c
>> @@ -128,10 +128,18 @@ static int ohci_usb_remove(struct udevice *dev)
>>  	if (ret)
>>  		return ret;
>>  
>> -	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);
>> -	clrbits_le32(&priv->ccm->ahb_gate0, priv->ahb_gate_mask);
>> +	/*
>> +	 * For those SoCs that share the clock and reset gates with the EHCI
>> +	 * controller, we should not turn them off here, to prevent the
>> +	 * other one hanging (when the EHCI driver tries to shut itself down).
>> +	 */
>> +	if (!priv->cfg->extra_ahb_gate_mask) {
>> +		if (priv->cfg->has_reset)
>> +			clrbits_le32(priv->reset0_cfg, priv->ahb_gate_mask);
>> +		clrbits_le32(&priv->ccm->ahb_gate0, priv->ahb_gate_mask);
>> +	}
>> +	if (!priv->cfg->extra_usb_gate_mask)
>> +		clrbits_le32(&priv->ccm->usb_clk_cfg, priv->usb_gate_mask);
>>  
>>  	return 0;
>>  }
>>
> 
>