From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761481AbbA3MSu (ORCPT ); Fri, 30 Jan 2015 07:18:50 -0500 Received: from mga09.intel.com ([134.134.136.24]:53026 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761341AbbA3MSr (ORCPT ); Fri, 30 Jan 2015 07:18:47 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,491,1418112000"; d="scan'208,223";a="645038011" Date: Fri, 30 Jan 2015 14:18:42 +0200 From: Heikki Krogerus To: David Cohen Cc: Felipe Balbi , Greg Kroah-Hartman , Baolu Lu , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Kishon Vijay Abraham I Subject: Re: [PATCH 8/8] phy: add driver for TI TUSB1210 ULPI PHY Message-ID: <20150130121842.GH2570@kuha.fi.intel.com> References: <20150124235811.GA24665@psi-dev26.jf.intel.com> <20150126125503.GB28539@kuha.fi.intel.com> <20150126192337.GA13936@psi-dev26.jf.intel.com> <20150127092856.GD28539@kuha.fi.intel.com> <20150127173801.GA8441@psi-dev26.jf.intel.com> <20150128142024.GA2378@kuha.fi.intel.com> <20150128180255.GA7551@psi-dev26.jf.intel.com> <20150129141412.GA2570@kuha.fi.intel.com> <20150129162023.GF21217@saruman.tx.rr.com> <20150129180223.GA2784@psi-dev26.jf.intel.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="XOIedfhf+7KOe/yw" Content-Disposition: inline In-Reply-To: <20150129180223.GA2784@psi-dev26.jf.intel.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --XOIedfhf+7KOe/yw Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 --XOIedfhf+7KOe/yw Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="0001-usb-dwc3-pci-remove-use-of-platform_data.patch" >>From 0afd47616c03d268d1c0e2ad5f4108f7f1a2401d Mon Sep 17 00:00:00 2001 From: Heikki Krogerus 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 --- 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 #include -#include "platform_data.h" - /* FIXME define these in */ #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 --XOIedfhf+7KOe/yw--