diff mbox

[RFC,3/4] xhci: Tune PHY for the DWC3-Exynos host controller

Message ID 1386672926-26885-4-git-send-email-gautam.vivek@samsung.com (mailing list archive)
State New, archived
Headers show

Commit Message

Vivek Gautam Dec. 10, 2013, 10:55 a.m. UTC
The DWC3-exynos eXtensible host controller on Exynos5420 SoC
is quirky in a way that the PHY needs to be tuned to get it
working at SuperSpeed.
Add relevant calls for tuning the PHY for DWC3-Exynos's
host controller, for that matter passing just USB3 PHY
from DWC3 core, which is saved in secondary HCD of XHCI.

Signed-off-by: Vivek Gautam <gautam.vivek@samsung.com>
---
 drivers/usb/dwc3/host.c      |    7 ++++++
 drivers/usb/host/xhci-plat.c |   43 ++++++++++++++++++++++++++++++++++++++++-
 include/linux/usb/hcd.h      |    1 +
 3 files changed, 49 insertions(+), 2 deletions(-)

Comments

Heikki Krogerus Dec. 10, 2013, 1:55 p.m. UTC | #1
Hi,

On Tue, Dec 10, 2013 at 04:25:25PM +0530, Vivek Gautam wrote:
> @@ -170,6 +189,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
>  	}
>  
>  	/*
> +	 * The parent of the xhci-plat device may pass in a PHY via
> +	 * platform data.  If it exists, store it in our struct usb_hcd
> +	 * so that we can use it later.
> +	 */
> +	phy_generic = dev_get_platdata(&pdev->dev);
> +	if (phy_generic)
> +		xhci->shared_hcd->phy_generic = *phy_generic;

Getting the handle to the phy from platform data like this is not
going to work for long. It should be possible to get it normally with
phy_get(). It's not going to be possible to get the handle from the
platform data like this if the xhci-hcd platform device is created
from ACPI or DT. You are also not considering case where you have two
phys.

Vivek, I have made a patch set for the phy framework allowing
associations between the phys and their users to be made in same way
gpios and clk make them. With those you should be able to create a
lookup entry to the phy framework in drivers/usb/dwc3/host.c. Then we
could use phy_get() here already. Please check them. Subject of the
thread:

"phy: remove the need for the phys to know about their users"

The lookup table can then be added in drivers/usb/dwc3/host.c with
something like this:

int dwc3_host_init(struct dwc3 *dwc)
{
        struct platform_device  *xhci;
        struct phy_lookup_table *table;
        ...
        table->dev_id = dev_name(&xhci->dev);
        if (dwc->usb2_generic_phy)
                table->table[0].phy_name = dev_name(&dwc->usb2_generic_phy->dev);
        if (dwc->usb3_generic_phy)
                table->table[1].phy_name = dev_name(&dwc->usb3_generic_phy->dev);
        phy_add_lookup_table(table);
        ...

Br,
Vivek Gautam April 15, 2014, 12:54 p.m. UTC | #2
Hi Heikki,


On Tue, Dec 10, 2013 at 7:25 PM, Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:

Giving life to this thread after long time.

> Hi,
>
> On Tue, Dec 10, 2013 at 04:25:25PM +0530, Vivek Gautam wrote:
>> @@ -170,6 +189,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>       }
>>
>>       /*
>> +      * The parent of the xhci-plat device may pass in a PHY via
>> +      * platform data.  If it exists, store it in our struct usb_hcd
>> +      * so that we can use it later.
>> +      */
>> +     phy_generic = dev_get_platdata(&pdev->dev);
>> +     if (phy_generic)
>> +             xhci->shared_hcd->phy_generic = *phy_generic;
>
> Getting the handle to the phy from platform data like this is not
> going to work for long. It should be possible to get it normally with
> phy_get(). It's not going to be possible to get the handle from the
> platform data like this if the xhci-hcd platform device is created
> from ACPI or DT. You are also not considering case where you have two
> phys.
>
> Vivek, I have made a patch set for the phy framework allowing
> associations between the phys and their users to be made in same way
> gpios and clk make them. With those you should be able to create a
> lookup entry to the phy framework in drivers/usb/dwc3/host.c. Then we
> could use phy_get() here already. Please check them. Subject of the
> thread:
>
> "phy: remove the need for the phys to know about their users"

I had seen your patches in the mailing list, but i don't see any
updated version of these patches.
Are you planning to work on this above mentioned patch-series any time soon ?

Or, should i try to find a different approach for handling the phy
from the host controller (child of DWC3 in this case, which has the
phys).

>
> The lookup table can then be added in drivers/usb/dwc3/host.c with
> something like this:
>
> int dwc3_host_init(struct dwc3 *dwc)
> {
>         struct platform_device  *xhci;
>         struct phy_lookup_table *table;
>         ...
>         table->dev_id = dev_name(&xhci->dev);
>         if (dwc->usb2_generic_phy)
>                 table->table[0].phy_name = dev_name(&dwc->usb2_generic_phy->dev);
>         if (dwc->usb3_generic_phy)
>                 table->table[1].phy_name = dev_name(&dwc->usb3_generic_phy->dev);
>         phy_add_lookup_table(table);
>         ...
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
index 32db328..cc1f6ff 100644
--- a/drivers/usb/dwc3/host.c
+++ b/drivers/usb/dwc3/host.c
@@ -46,6 +46,13 @@  int dwc3_host_init(struct dwc3 *dwc)
 		goto err1;
 	}
 
+	ret = platform_device_add_data(xhci, &dwc->usb3_generic_phy,
+					sizeof(dwc->usb3_generic_phy));
+	if (ret) {
+		dev_err(dwc->dev, "failed to add platform data\n");
+		goto err1;
+	}
+
 	ret = platform_device_add(xhci);
 	if (ret) {
 		dev_err(dwc->dev, "failed to register xHCI device\n");
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 395c9e9..a0f3cbc 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -16,6 +16,7 @@ 
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/dma-mapping.h>
+#include <linux/phy/phy.h>
 
 #include "xhci.h"
 
@@ -51,7 +52,24 @@  static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
 /* called during probe() after chip reset completes */
 static int xhci_plat_setup(struct usb_hcd *hcd)
 {
-	return xhci_gen_setup(hcd, xhci_plat_quirks);
+	struct xhci_hcd	*xhci;
+	int ret = 0;
+
+	ret = xhci_gen_setup(hcd, xhci_plat_quirks);
+	if (ret) {
+		dev_err(hcd->self.controller, "xhci setup failed\n");
+		goto err0;
+	}
+
+	/* Valid for secondary HCD only which gives SuperSpeed ports */
+	if (!usb_hcd_is_primary_hcd(hcd)) {
+		xhci = hcd_to_xhci(hcd);
+		if (xhci->quirks & XHCI_DWC3_EXYNOS)
+			phy_tune(hcd->phy_generic);
+	}
+
+err0:
+	return ret;
 }
 
 static const struct hc_driver xhci_plat_xhci_driver = {
@@ -111,6 +129,7 @@  static int xhci_plat_probe(struct platform_device *pdev)
 	struct usb_hcd		*hcd;
 	int			ret;
 	int			irq;
+	struct phy		**phy_generic;
 
 	if (usb_disabled())
 		return -ENODEV;
@@ -170,6 +189,15 @@  static int xhci_plat_probe(struct platform_device *pdev)
 	}
 
 	/*
+	 * The parent of the xhci-plat device may pass in a PHY via
+	 * platform data.  If it exists, store it in our struct usb_hcd
+	 * so that we can use it later.
+	 */
+	phy_generic = dev_get_platdata(&pdev->dev);
+	if (phy_generic)
+		xhci->shared_hcd->phy_generic = *phy_generic;
+
+	/*
 	 * Set the xHCI pointer before xhci_plat_setup() (aka hcd_driver.reset)
 	 * is called by usb_add_hcd().
 	 */
@@ -229,8 +257,19 @@  static int xhci_plat_resume(struct device *dev)
 {
 	struct usb_hcd	*hcd = dev_get_drvdata(dev);
 	struct xhci_hcd	*xhci = hcd_to_xhci(hcd);
+	int ret;
+
+	ret = xhci_resume(xhci, 0);
+	if (ret)
+		return ret;
 
-	return xhci_resume(xhci, 0);
+	/* Valid for secondary HCD only which gives SuperSpeed ports */
+	if (!usb_hcd_is_primary_hcd(hcd)) {
+		if (xhci->quirks & XHCI_DWC3_EXYNOS)
+			phy_tune(hcd->phy_generic);
+	}
+
+	return 0;
 }
 
 static const struct dev_pm_ops xhci_plat_pm_ops = {
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index b8aba19..241ed2b 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -107,6 +107,7 @@  struct usb_hcd {
 	 * other external phys should be software-transparent
 	 */
 	struct usb_phy	*phy;
+	struct phy	*phy_generic;
 
 	/* Flags that need to be manipulated atomically because they can
 	 * change while the host controller is running.  Always use