Message-ID: <20150130121842.GH2570@kuha.fi.intel.com>
Date:	Fri, 30 Jan 2015 14:18:42 +0200
From:	Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:	David Cohen <david.a.cohen@...ux.intel.com>
Cc:	Felipe Balbi <balbi@...com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Baolu Lu <baolu.lu@...ux.intel.com>, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Kishon Vijay Abraham I <kishon@...com>
Subject: Re: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY

Hi,

> > > 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?

> Let's propose this non-BYT-CR scenario:
> You have dwc3/phy powered on by BIOS and dwc3 + phy as modules. You
> probe the modules for the first time and it works because phy was
> accessible. Then we remove the modules and load it again. By removing
> the modules phy is not functional anymore (remove operation will put
> them in reset state). Then you load the modules again. It certainly will
> fail to access ULPI bus during phy's probe this time. BYT-CR is just an
> example where BIOS lets phy in reset during probe, but you can get this
> same behavior on other platforms too.

You have a point here. I'm curious now what you reply to my question
about the possibility to modify DSDT in my previous mail. Because the
properties would be a way out of the PHY powering issue.

You know, even if we can't make changes to the DSDT (like I suspect)
we can still use the properties once this is accepted:
http://www.spinics.net/lists/linux-acpi/msg55337.html

So we can pass the "ulpi,vendor" and "ulpi,product" also with these
generic properties. I'm attaching a patch where I'm converting the
platform data AMD uses to them as an example how they could be used.

> > > BYT-CR's USB is not supported in mainline yet unless I'm completely
> > > mistaken, but we have the plan for it. Instead of trying to take any
> > > shortcuts, let's follow that plan.
> 
> It is supported, as I stated above. I don't know where you got the idea
> it wasn't. I made BYT-T (and BYT-CR) + Merrifield USB device layer work
> with mainline since 3.14. But this patch made the regression to stop
> working:
> https://www.mail-archive.com/linux-usb@vger.kernel.org/msg54400.html

How can it work with mainline if there was nothing toggling the gpios
yet?

> I'd prefer to go back to platform device case (which is less ugly) for
> products on market (i.e. legacy and unwanted way for future platforms).

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.


Have nice weekend guys,

-- 
heikki

View attachment "0001-usb-dwc3-pci-remove-use-of-platform_data.patch" of type "text/plain" (2859 bytes)

Powered by blists - more mailing lists