From: David Cohen <david.a.cohen@linux.intel.com>
To: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Cc: Felipe Balbi <balbi@ti.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Baolu Lu <baolu.lu@linux.intel.com>,
linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
Kishon Vijay Abraham I <kishon@ti.com>
Subject: Re: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY
Date: Fri, 30 Jan 2015 08:09:59 -0800 [thread overview]
Message-ID: <20150130160959.GA20689@psi-dev26.jf.intel.com> (raw)
In-Reply-To: <20150130121842.GH2570@kuha.fi.intel.com>
On Fri, Jan 30, 2015 at 02:18:42PM +0200, Heikki Krogerus wrote:
> 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?
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.
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.
>
> > 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 missed my question. Have you tried to remove and reload dwc3 and phy
modules with your test case?
>
> 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?
Again, that's board related, not BYT-CR. See my reply above and redo
your tests using BYT-T reference board.
>
> > 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.
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.
>
>
> Have nice weekend guys,
>
You too :)
Br, David
> --
> heikki
> From 0afd47616c03d268d1c0e2ad5f4108f7f1a2401d Mon Sep 17 00:00:00 2001
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Date: Mon, 12 Jan 2015 16:15:10 +0200
> Subject: [PATCH] usb: dwc3: pci: remove use of platform_data
>
> This replaces the platform_data used on AMD Nolan SoC with
> generic device property.
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> ---
> drivers/usb/dwc3/dwc3-pci.c | 53 ++++++++++++++++++++++++---------------------
> 1 file changed, 28 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> index 8d95056..0f20139 100644
> --- a/drivers/usb/dwc3/dwc3-pci.c
> +++ b/drivers/usb/dwc3/dwc3-pci.c
> @@ -22,8 +22,6 @@
> #include <linux/pci.h>
> #include <linux/platform_device.h>
>
> -#include "platform_data.h"
> -
> /* FIXME define these in <linux/pci_ids.h> */
> #define PCI_VENDOR_ID_SYNOPSYS 0x16c3
> #define PCI_DEVICE_ID_SYNOPSYS_HAPSUSB3 0xabcd
> @@ -37,34 +35,39 @@ static int dwc3_pci_quirks(struct pci_dev *pdev)
> {
> if (pdev->vendor == PCI_VENDOR_ID_AMD &&
> pdev->device == PCI_DEVICE_ID_AMD_NL_USB) {
> - struct dwc3_platform_data pdata;
> -
> - memset(&pdata, 0, sizeof(pdata));
> -
> - pdata.has_lpm_erratum = true;
> - pdata.lpm_nyet_threshold = 0xf;
> -
> - pdata.u2exit_lfps_quirk = true;
> - pdata.u2ss_inp3_quirk = true;
> - pdata.req_p1p2p3_quirk = true;
> - pdata.del_p1p2p3_quirk = true;
> - pdata.del_phy_power_chg_quirk = true;
> - pdata.lfps_filter_quirk = true;
> - pdata.rx_detect_poll_quirk = true;
> -
> - pdata.tx_de_emphasis_quirk = true;
> - pdata.tx_de_emphasis = 1;
> -
> + struct platform_device *dwc3 = pci_get_drvdata(pdev);
> + struct dev_gen_prop *prop;
> +
> + prop = devm_kcalloc(&pdev->dev, 14, sizeof(*prop), GFP_KERNEL);
> + if (!prop)
> + return -ENOMEM;
> +
> + prop[1].num = devm_kzalloc(&pdev->dev, sizeof(u8 *),
> + GFP_KERNEL);
> + if (!prop[1].num)
> + return -ENOMEM;
> +
> + prop[0].name = "snps,has-lpm-erratum";
> + prop[1].name = "snps,lpm-nyet-threshold";
> + prop[1].nval = 1;
> + prop[1].num[0] = 0xf;
> + prop[2].name = "snps,u2exit_lfps_quirk";
> + prop[3].name = "snps,u2ss_inp3_quirk";
> + prop[4].name = "snps,req_p1p2p3_quirk";
> + prop[5].name = "snps,del_p1p2p3_quirk";
> + prop[6].name = "snps,del_phy_power_chg_quirk";
> + prop[7].name = "snps,lfps_filter_quirk";
> + prop[8].name = "snps,rx_detect_poll_quirk";
> + prop[9].name = "snps,tx_de_emphasis_quirk";
> /*
> * FIXME these quirks should be removed when AMD NL
> * taps out
> */
> - pdata.disable_scramble_quirk = true;
> - pdata.dis_u3_susphy_quirk = true;
> - pdata.dis_u2_susphy_quirk = true;
> + prop[10].name = "snps,disable_scramble_quirk";
> + prop[11].name = "snps,dis_u3_susphy_quirk";
> + prop[12].name = "snps,dis_u2_susphy_quirk";
>
> - return platform_device_add_data(pci_get_drvdata(pdev), &pdata,
> - sizeof(pdata));
> + dwc3->dev.gen_prop = prop;
> }
>
> return 0;
> --
> 2.1.4
>
next prev parent reply other threads:[~2015-01-30 16:08 UTC|newest]
Thread overview: 70+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-23 15:12 [PATCH 0/8] usb: ulpi bus Heikki Krogerus
2015-01-23 15:12 ` [PATCH 1/8] usb: add bus type for USB ULPI Heikki Krogerus
2015-01-29 5:02 ` Felipe Balbi
2015-01-29 14:18 ` Heikki Krogerus
2015-02-13 1:44 ` Stephen Boyd
2015-02-13 11:24 ` Heikki Krogerus
2015-01-23 15:12 ` [PATCH 2/8] usb: dwc3: USB2 PHY register access bits Heikki Krogerus
2015-01-23 15:12 ` [PATCH 3/8] usb: dwc3: store driver data earlier Heikki Krogerus
2015-01-23 15:12 ` [PATCH 4/8] usb: dwc3: cache hwparams earlier Heikki Krogerus
2015-01-23 15:12 ` [PATCH 5/8] usb: dwc3: ULPI or UTMI+ select Heikki Krogerus
2015-01-23 15:12 ` [PATCH 6/8] usb: dwc3: add ULPI interface support Heikki Krogerus
2015-01-23 16:24 ` Felipe Balbi
2015-01-26 11:46 ` Heikki Krogerus
2015-01-26 19:35 ` Felipe Balbi
2015-01-27 11:09 ` Heikki Krogerus
2015-01-27 15:24 ` Felipe Balbi
2015-02-11 19:34 ` David Cohen
2015-02-12 12:12 ` Heikki Krogerus
2015-02-13 1:41 ` David Cohen
2015-02-13 1:54 ` David Cohen
2015-02-13 13:16 ` Heikki Krogerus
2015-02-13 22:03 ` David Cohen
2015-02-13 22:04 ` Felipe Balbi
2015-01-23 15:12 ` [PATCH 7/8] phy: helpers for USB ULPI PHY registering Heikki Krogerus
2015-01-29 5:03 ` Felipe Balbi
2015-01-29 14:34 ` Heikki Krogerus
2015-01-29 16:11 ` Felipe Balbi
2015-01-30 10:33 ` Heikki Krogerus
2015-01-30 16:03 ` Felipe Balbi
2015-01-23 15:12 ` [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY Heikki Krogerus
2015-01-24 23:58 ` David Cohen
2015-01-26 12:55 ` Heikki Krogerus
2015-01-26 19:23 ` David Cohen
2015-01-27 9:28 ` Heikki Krogerus
2015-01-27 12:57 ` Heikki Krogerus
2015-01-27 17:38 ` David Cohen
2015-01-28 14:20 ` Heikki Krogerus
2015-01-28 18:02 ` David Cohen
2015-01-29 14:14 ` Heikki Krogerus
2015-01-29 16:20 ` Felipe Balbi
2015-01-29 18:02 ` David Cohen
2015-01-30 12:18 ` Heikki Krogerus
2015-01-30 16:09 ` David Cohen [this message]
2015-02-02 12:50 ` Heikki Krogerus
2015-01-30 9:29 ` Heikki Krogerus
2015-01-30 16:14 ` Felipe Balbi
2015-01-30 16:25 ` David Cohen
2015-01-30 16:30 ` Felipe Balbi
2015-01-30 16:20 ` David Cohen
2015-01-30 16:33 ` Felipe Balbi
2015-02-02 12:59 ` Heikki Krogerus
2015-02-03 11:37 ` Heikki Krogerus
2015-02-10 18:33 ` David Cohen
2015-02-10 19:05 ` David Cohen
2015-02-10 19:23 ` David Cohen
2015-02-11 13:12 ` Heikki Krogerus
2015-02-11 19:36 ` David Cohen
2015-02-13 22:02 ` David Cohen
2015-02-13 22:03 ` Felipe Balbi
2015-02-13 22:13 ` David Cohen
2015-01-29 5:09 ` Felipe Balbi
2015-01-29 14:30 ` Heikki Krogerus
2015-01-29 16:20 ` Felipe Balbi
2015-01-29 18:04 ` David Cohen
2015-01-29 18:25 ` David Cohen
2015-01-29 18:47 ` David Cohen
2015-01-30 10:30 ` Heikki Krogerus
2015-02-13 1:52 ` David Cohen
2015-02-13 12:35 ` Heikki Krogerus
2015-02-13 16:01 ` Felipe Balbi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150130160959.GA20689@psi-dev26.jf.intel.com \
--to=david.a.cohen@linux.intel.com \
--cc=balbi@ti.com \
--cc=baolu.lu@linux.intel.com \
--cc=gregkh@linuxfoundation.org \
--cc=heikki.krogerus@linux.intel.com \
--cc=kishon@ti.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).