Allwinner H616 USB support woes

Allwinner H616 USB support woes

[Andre Przywara]

Hi,

I wanted to ask for some advice on the proper upstream H616 USB
support. This is the only problem left before I can post v6 of the
H616 support series.
The main problem is that only controller 2 (EHCI2+PHY2) works on its
own, all other ports need some help from controller 2.
Just enabling the EHCI2 DT node does the trick, but with some help from
Samuel and Jernej (who were looking into the BROM) I wiggled it down to
the following things actually needed:
- CLK_BUS_PHY2 and RST_BUS_PHY2 need to be enabled.
- PMU2.SIDDQ (0x05310810[3]) needs to be clear.
We can add the clock and reset to the list of clocks and resets in the
other OHCI/EHCI DT nodes, so that's not a real problem.

But this SIDDQ bit is a bit nasty in the details:
It would only need to be flipped once, and wouldn't be changed
back by any reset bits. So what would work is to do this
once in U-Boot. However to actually be able to access this register, we
need CLK_BUS_EHCI2 to be ungated. What works in U-Boot is:
=> mw.l 0x3001a8c 0x40	/* pass USBEHCI2_GATING */
=> mw.l 0x5310810 0	/* clear SIDDQ in PMU2 */
=> mw.l 0x3001a8c 0	/* mask USBEHCI2_GATING again */

Now I would rather see this done in Linux, in the kernel's
phy-sun4i-usb.c, to be self contained and independent.
The easiest way looks like to call sun4i_usb_phy_init() again for PHY2,
after this function is called for another PHY. So at the end of
sun4i_usb_phy_init() (simplified code just for demonstration):

	if (need_phy2 && phy->index != 2)
		sun4i_usb_phy_init(data->phys[2].phy);
(that does a bit more than is really needed, but OK)

However this doesn't work, because CLK_BUS_EHCI2 is still gated, it is
only unmasked in ehci-platform.c, when EHCI2 in initialised.
And annoyingly ehci-platform.c only parses 4 clocks, so adding
CLK_BUS_EHCI2 to the DT would be the fifth in the ehci<x> nodes (three
at the moment plus the still needed CLK_BUS_PHY2 plus CLK_BUS_EHCI2).

Extending this limit is trivial, but sounds a bit like a stretch, since
it's outside of sunxi code and technically the clock is just needed for
initialisation, not during runtime.

Do you guys have any idea how to best solve this?
I would prefer some change confined to phy-sun4i-usb.c, without
bending the DT parts too much. The other alternatives are a U-Boot
solution or extending the limit in ehci-platform.c.

Would be grateful for any input!

Cheers,
Andre

Re: Allwinner H616 USB support woes

[Jernej Škrabec]

Hi Andre!

Dne torek, 18. maj 2021 ob 12:11:18 CEST je Andre Przywara napisal(a):
> Hi,
> 
> I wanted to ask for some advice on the proper upstream H616 USB
> support. This is the only problem left before I can post v6 of the
> H616 support series.
> The main problem is that only controller 2 (EHCI2+PHY2) works on its
> own, all other ports need some help from controller 2.
> Just enabling the EHCI2 DT node does the trick, but with some help from
> Samuel and Jernej (who were looking into the BROM) I wiggled it down to
> the following things actually needed:
> - CLK_BUS_PHY2 and RST_BUS_PHY2 need to be enabled.
> - PMU2.SIDDQ (0x05310810[3]) needs to be clear.
> We can add the clock and reset to the list of clocks and resets in the
> other OHCI/EHCI DT nodes, so that's not a real problem.
> 
> But this SIDDQ bit is a bit nasty in the details:
> It would only need to be flipped once, and wouldn't be changed
> back by any reset bits. So what would work is to do this
> once in U-Boot. However to actually be able to access this register, we
> need CLK_BUS_EHCI2 to be ungated. What works in U-Boot is:
> => mw.l 0x3001a8c 0x40	/* pass USBEHCI2_GATING */
> => mw.l 0x5310810 0	/* clear SIDDQ in PMU2 */
> => mw.l 0x3001a8c 0	/* mask USBEHCI2_GATING again */
> 
> Now I would rather see this done in Linux, in the kernel's
> phy-sun4i-usb.c, to be self contained and independent.
> The easiest way looks like to call sun4i_usb_phy_init() again for PHY2,
> after this function is called for another PHY. So at the end of
> sun4i_usb_phy_init() (simplified code just for demonstration):
> 
> 	if (need_phy2 && phy->index != 2)
> 		sun4i_usb_phy_init(data->phys[2].phy);
> (that does a bit more than is really needed, but OK)
> 
> However this doesn't work, because CLK_BUS_EHCI2 is still gated, it is
> only unmasked in ehci-platform.c, when EHCI2 in initialised.
> And annoyingly ehci-platform.c only parses 4 clocks, so adding
> CLK_BUS_EHCI2 to the DT would be the fifth in the ehci<x> nodes (three
> at the moment plus the still needed CLK_BUS_PHY2 plus CLK_BUS_EHCI2).
> 
> Extending this limit is trivial, but sounds a bit like a stretch, since
> it's outside of sunxi code and technically the clock is just needed for
> initialisation, not during runtime.
> 
> Do you guys have any idea how to best solve this?
> I would prefer some change confined to phy-sun4i-usb.c, without
> bending the DT parts too much. The other alternatives are a U-Boot
> solution or extending the limit in ehci-platform.c.
> 
> Would be grateful for any input!

First, thanks for working on this!

Maybe I'm missing something, but wouldn't be adding CLK_BUS_EHCI2 to usbphy 
node and clearing PHY2 SIDDQ at the end of sun4i_usb_phy_probe() be enough? At 
least judging by your U-Boot commands. PHY driver must be initialized before 
ohci/ehci anyway.

Best regards,
Jernej

Re: Allwinner H616 USB support woes

[Andre Przywara]

On Tue, 18 May 2021 19:32:00 +0200
Jernej Škrabec <jernej.skrabec@gmail.com> wrote:

Hi Jernej,

many thanks for the reply!

> Dne torek, 18. maj 2021 ob 12:11:18 CEST je Andre Przywara napisal(a):
> > Hi,
> > 
> > I wanted to ask for some advice on the proper upstream H616 USB
> > support. This is the only problem left before I can post v6 of the
> > H616 support series.
> > The main problem is that only controller 2 (EHCI2+PHY2) works on its
> > own, all other ports need some help from controller 2.
> > Just enabling the EHCI2 DT node does the trick, but with some help from
> > Samuel and Jernej (who were looking into the BROM) I wiggled it down to
> > the following things actually needed:
> > - CLK_BUS_PHY2 and RST_BUS_PHY2 need to be enabled.
> > - PMU2.SIDDQ (0x05310810[3]) needs to be clear.
> > We can add the clock and reset to the list of clocks and resets in the
> > other OHCI/EHCI DT nodes, so that's not a real problem.
> > 
> > But this SIDDQ bit is a bit nasty in the details:
> > It would only need to be flipped once, and wouldn't be changed
> > back by any reset bits. So what would work is to do this
> > once in U-Boot. However to actually be able to access this register, we
> > need CLK_BUS_EHCI2 to be ungated. What works in U-Boot is:  
> > => mw.l 0x3001a8c 0x40	/* pass USBEHCI2_GATING */
> > => mw.l 0x5310810 0	/* clear SIDDQ in PMU2 */
> > => mw.l 0x3001a8c 0	/* mask USBEHCI2_GATING again */  
> > 
> > Now I would rather see this done in Linux, in the kernel's
> > phy-sun4i-usb.c, to be self contained and independent.
> > The easiest way looks like to call sun4i_usb_phy_init() again for PHY2,
> > after this function is called for another PHY. So at the end of
> > sun4i_usb_phy_init() (simplified code just for demonstration):
> > 
> > 	if (need_phy2 && phy->index != 2)
> > 		sun4i_usb_phy_init(data->phys[2].phy);
> > (that does a bit more than is really needed, but OK)
> > 
> > However this doesn't work, because CLK_BUS_EHCI2 is still gated, it is
> > only unmasked in ehci-platform.c, when EHCI2 in initialised.
> > And annoyingly ehci-platform.c only parses 4 clocks, so adding
> > CLK_BUS_EHCI2 to the DT would be the fifth in the ehci<x> nodes (three
> > at the moment plus the still needed CLK_BUS_PHY2 plus CLK_BUS_EHCI2).
> > 
> > Extending this limit is trivial, but sounds a bit like a stretch, since
> > it's outside of sunxi code and technically the clock is just needed for
> > initialisation, not during runtime.
> > 
> > Do you guys have any idea how to best solve this?
> > I would prefer some change confined to phy-sun4i-usb.c, without
> > bending the DT parts too much. The other alternatives are a U-Boot
> > solution or extending the limit in ehci-platform.c.
> > 
> > Would be grateful for any input!  
> 
> First, thanks for working on this!
> 
> Maybe I'm missing something, but wouldn't be adding CLK_BUS_EHCI2 to usbphy 
> node and clearing PHY2 SIDDQ at the end of sun4i_usb_phy_probe() be enough? At 
> least judging by your U-Boot commands. PHY driver must be initialized before 
> ohci/ehci anyway.

Yes, and I guess I was too busy with finding a nice and generic
solution for this, where this is really a nasty hardware issue and
probably justifies some rather ugly quirk.

So I more or less literally implemented your solution: this doesn't
look super nice, but at least is confined to phy-sun4i-usb.c and has one
little DT addition - the CLK_BUS_EHCI2 clock in the PHY DT node.
And, added bonus: it works ;-)

So many thanks for your help, and watch your inbox for my patches!

Cheers,
Andre