Re: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

Hi David,

> > > > > What exactly are we breaking here? The USB on BYT-CR does not work yet
> > > > > with the mainline kernel, or does it? To enable it, I already
> > > > > suggested the BYT quirk (attached again).
> > > 
> > > It used to work with mainline with minor restrictions. It stopped
> > > working (and I failed to catch it during patch review) when NOP phy
> > > enumeration was removed from dwc3-pci.c (but my understanding is that
> > > Felipe is expecting to add it back as default phy in case no phy is
> > > found by dwc3).
> > > 
> > > BYT-CR worked well except for operations that needed to access phy's
> > > registers via ULPI bus (e.g. eye optimization). But to power on i.e.
> > > TUSB1210 all you need it to toggle GPIOs, which is done by generic phy.
> > > The need for ULPI bus was to complement this missing features, but
> > > instead we're breaking BYT-CR :/
> > 
> > So what you are saying that if I get one of those devices you
> > mentioned and try vanilla kernel on it, the USB will work without any
> > modifications to the kernel?
> 
> You're misunderstanding BYT-CR SoC and external board components. The
> only reason that prevents most of BYT-CR boards' USB device work
> out-of-the-box is a switch device muxing D+/D- between host and device
> controllers (they depend on a single GPIO level to be toggled to get USB
> device working). I started discussion on how to upstream this case, but
> this is board related, not BYT-CR related. Some boards have also an i2c
> switch which requires extra i2c driver to get USB device working. But it
> doesn't mean the phy/otg controllers aren't fully functional with dwc3 +
> generic phy drivers.

OK, so after we add driver for the mux, are you saying that USB device
mode works without need for any patches to dwc3 or the nop phy driver
for example with v3.18?

In the code that we had in v3.18, the nop phy platform data had the
reset gpio value set to -1 (invalid) by the dwc3-pci, so there was
nothing toggling the reset gpio yet and the cs gpio was not handled at
all. So unless you are saying we don't need to toggle the gpios before
the USB became operational, you would have had to patch both dwc3-pci
and phy-generic in order to get it operational. And of course if we
didn't need to toggle them, there would not be any need for the nop
phy at all.

> FWIW if you test a board without such switch (e.g. a reference BYT-T
> board called FFRD8 - BYT-CR is a derivation of BYT-T) it will work
> out-of-the-box.

And it continues to work out-of-the-box even after we removed the
creation of the PHY platform device because it does not need to
toggle the gpios, right?

BYT-T boards are actually one of the reason why we would really need
the ulpi bus, because most them have tusb1211 (so not tusb1210) as the
phy and they use it to detect the charger among other things.

> You missed my question. Have you tried to remove and reload dwc3 and phy
> modules with your test case?

I do test unloading all the modules and reloading them back every
time, and with the hack I suggested everything works just fine.

> > We really can't go back to what we had. Please keep in mind that it
> > tied us to the USB PHY framework, possibly forever, and we shouldn't
> > have to depend on two different PHY frameworks. If we have to register
> > the PHY device in dwc3-pci.c then you should create new nop phy for
> > the generic phy framework and use that instead. But before you do
> > that, we better be damn sure there is no way to make ulpi bus work,
> > and we are not there yet.
> 
> You have a point. But the transition should happen without creating
> regressions. You cannot remove previous design while we don't have the
> next one merged and functional.

But I still don't see any regressions?


Thanks,

-- 
heikki