Re: Allwinner H616 USB support woes

[Andre Przywara <andre.przywara@arm.com>]

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