Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates
On 07/03/2018 12:08 PM, Andre Przywara wrote: > Hi, 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 >> >> 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. Some temporary enable/disable_clock() function shared by those two drivers might be doable in this short timeframe, no ? > 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). Good. Last time I asked why there is no proper clock driver, I got quite a lot of flak in that I am asking for too much. > 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. I want something which stops the clock properly for this release. So if you can come up with something like that, great. And then the clock driver would be awesome for the next release or so. [...] -- Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates
On 07/03/2018 12:11 PM, Andre Przywara wrote:
> Hi,
>
> On 03/07/18 07:19, Jagan Teki wrote:
>> On Tue, Jul 3, 2018 at 5:14 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
>>> ---
>>> 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(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>>> - clrbits_le32(>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).
>>
>> I've tried this "not to disable usb_clk_cfg" before sending "arm64:
>> allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi" patch.
>> But, I thought improper shutdown may effect other issue may be with
>> Linux.
>
> That's probably true. Did you try to prevent shutdown at all? Or just
> for *one* of OHCI and EHCI?
> My patch does it only for OHCI, so the EHCI still does the shutdown.
> This somewhat relies on some shutdown ordering, though.
>
>> Initially I found an issue of disabling OHCI1 gate clock during OHCI0
>> shutdown. ohci driver is trying to clear BIT(16) for OHCI0, but it
>> also clears BIT(17), which is for OHCI1. By adding proper shutdown
>> support for musb and other things I came to a situation where all
>> shutdown happen properly. But ofcourse I still see hang.
>
> I believe with all those shared gates and dependencies we would need
> refcounting per *bit*, for the gates, USB clocks and resets. As
> mentioned, I am not sure it's worth the effort, if a DM clock driver
> would solve this much more elegantly, and in one place.
> I just need this fix here to make the kernel DTs work, so that I can
> send those out.
> After this I would revive the DM_CLK work.
Yes, the clock driver would solve this issue, since it would be able to
do some sort of reference counting for the shared clock.
--
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates
On Tue, Jul 3, 2018 at 3:41 PM, Andre Przywara wrote:
> Hi,
>
> On 03/07/18 07:19, Jagan Teki wrote:
>> On Tue, Jul 3, 2018 at 5:14 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
>>> ---
>>> 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(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>>> - clrbits_le32(>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).
>>
>> I've tried this "not to disable usb_clk_cfg" before sending "arm64:
>> allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi" patch.
>> But, I thought improper shutdown may effect other issue may be with
>> Linux.
>
> That's probably true. Did you try to prevent shutdown at all? Or just
> for *one* of OHCI and EHCI?
> My patch does it only for OHCI, so the EHCI still does the shutdown.
> This somewhat relies on some shutdown ordering, though.
Yes I understand it, but here I think we can turn-off usb_clk_cfg no
need to turn-off gates as well.
>
>> Initially I found an issue of disabling OHCI1 gate clock during OHCI0
>> shutdown. ohci driver is trying to clear BIT(16) for OHCI0, but it
>> also clears BIT(17), which is for OHCI1. By adding proper shutdown
>> support for musb and other things I came to a situation where all
>> shutdown happen properly. But ofcourse I still see hang.
>
> I believe with all those shared gates and dependencies we would need
> refcounting per *bit*, for the gates, USB clocks and resets. As
> mentioned, I am not sure it's worth the effort, if a DM clock driver
> would solve this much more elegantly, and in one place.
> I just need this fix here to make the kernel DTs work, so that I can
> send those out.
> After this I would revive the DM_CLK work.
This is what my next plan would be. Once we have dm_clk ready we can
use ehci/ohci generic drivers directly since we have PHY handling
driver.
thanks,
Jagan.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates
Hi,
On 03/07/18 07:19, Jagan Teki wrote:
> On Tue, Jul 3, 2018 at 5:14 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
>> ---
>> 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(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>> - clrbits_le32(>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).
>
> I've tried this "not to disable usb_clk_cfg" before sending "arm64:
> allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi" patch.
> But, I thought improper shutdown may effect other issue may be with
> Linux.
That's probably true. Did you try to prevent shutdown at all? Or just
for *one* of OHCI and EHCI?
My patch does it only for OHCI, so the EHCI still does the shutdown.
This somewhat relies on some shutdown ordering, though.
> Initially I found an issue of disabling OHCI1 gate clock during OHCI0
> shutdown. ohci driver is trying to clear BIT(16) for OHCI0, but it
> also clears BIT(17), which is for OHCI1. By adding proper shutdown
> support for musb and other things I came to a situation where all
> shutdown happen properly. But ofcourse I still see hang.
I believe with all those shared gates and dependencies we would need
refcounting per *bit*, for the gates, USB clocks and resets. As
mentioned, I am not sure it's worth the effort, if a DM clock driver
would solve this much more elegantly, and in one place.
I just need this fix here to make the kernel DTs work, so that I can
send those out.
After this I would revive the DM_CLK work.
Cheers,
Andre.
> => usb reset
> resetting USB...
>
> PHY0: mask = 0x101, usb_clk_cfg = 0x30202
> sunxi_musb_exit: ahb_gate0 = 0x33004540, reset0_cfg = 0x33004540
> EHCI failed to shut down host controller.
> ehci_usb_remove: ahb_gate0 = 0x32004540, reset0_cfg = 0x32004540
> ohci_usb_remove: ahb_gate0 = 0x22004540, reset0_cfg = 0x22004540
> ohci_usb_remove: mask = 0x1, usb_clk_cfg = 0x20202
>
> PHY1: mask = 0x202, usb_clk_cfg = 0x0
> ehci_usb_remove: ahb_gate0 = 0x20004540, reset0_cfg = 0x20004540
> << hang >>
>
> Do you think this is still an issue with clock? considering proper
> shutdown sequence happened above.
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates
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
>
> 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(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>> -clrbits_le32(>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(>ccm->ahb_gate0, priv->ahb_gate_mask);
>> +}
>> +if (!priv->cfg->extra_usb_gate_mask)
>> +clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>>
>> return 0;
>> }
>>
>
>
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates
On Tue, Jul 3, 2018 at 1:54 PM, 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 > > What about adding some enable/disable counter to those clock somehow and > then turning them off when the counter reaches zero ? Like disable all controller clocks, at the end of last controller shutdown? right now it is doing opposite for A64. Disabling all clocks at the end work for me, with preserving previous mask bit. Jagan. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates
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
What about adding some enable/disable counter to those clock somehow and
then turning them off when the counter reaches zero ?
> ---
> 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(>ccm->usb_clk_cfg, priv->usb_gate_mask);
> - clrbits_le32(>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(>ccm->ahb_gate0, priv->ahb_gate_mask);
> + }
> + if (!priv->cfg->extra_usb_gate_mask)
> + clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
>
> return 0;
> }
>
--
Best regards,
Marek Vasut
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates
On Tue, Jul 3, 2018 at 5:14 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
> ---
> 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(>ccm->usb_clk_cfg, priv->usb_gate_mask);
> - clrbits_le32(>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).
I've tried this "not to disable usb_clk_cfg" before sending "arm64:
allwinner: a64: Disable ehci1 and ohci1 for bananapi, nanopi" patch.
But, I thought improper shutdown may effect other issue may be with
Linux.
Initially I found an issue of disabling OHCI1 gate clock during OHCI0
shutdown. ohci driver is trying to clear BIT(16) for OHCI0, but it
also clears BIT(17), which is for OHCI1. By adding proper shutdown
support for musb and other things I came to a situation where all
shutdown happen properly. But ofcourse I still see hang.
=> usb reset
resetting USB...
PHY0: mask = 0x101, usb_clk_cfg = 0x30202
sunxi_musb_exit: ahb_gate0 = 0x33004540, reset0_cfg = 0x33004540
EHCI failed to shut down host controller.
ehci_usb_remove: ahb_gate0 = 0x32004540, reset0_cfg = 0x32004540
ohci_usb_remove: ahb_gate0 = 0x22004540, reset0_cfg = 0x22004540
ohci_usb_remove: mask = 0x1, usb_clk_cfg = 0x20202
PHY1: mask = 0x202, usb_clk_cfg = 0x0
ehci_usb_remove: ahb_gate0 = 0x20004540, reset0_cfg = 0x20004540
<< hang >>
Do you think this is still an issue with clock? considering proper
shutdown sequence happened above.
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH] sunxi: H3/H5/A64: OHCI: prevent turning off shared gates
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
---
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(>ccm->usb_clk_cfg, priv->usb_gate_mask);
- clrbits_le32(>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(>ccm->ahb_gate0, priv->ahb_gate_mask);
+ }
+ if (!priv->cfg->extra_usb_gate_mask)
+ clrbits_le32(>ccm->usb_clk_cfg, priv->usb_gate_mask);
return 0;
}
--
2.14.4
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
