diff mbox

[v2,10/30] USB: ehci-omap: Use PHY APIs to get the PHY device and put it out of suspend

Message ID 1359372631-8180-11-git-send-email-rogerq@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Roger Quadros Jan. 28, 2013, 11:30 a.m. UTC
For each port that is in PHY mode we obtain a PHY device using the USB PHY
library and put it out of suspend.

It is upto platform code to associate the PHY to the controller's
port and it is upto the PHY driver to manage the PHY's resources.

Signed-off-by: Roger Quadros <rogerq@ti.com>
---
 drivers/usb/host/ehci-omap.c |   54 ++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 54 insertions(+), 0 deletions(-)

Comments

Alan Stern Jan. 28, 2013, 3:40 p.m. UTC | #1
On Mon, 28 Jan 2013, Roger Quadros wrote:

> For each port that is in PHY mode we obtain a PHY device using the USB PHY
> library and put it out of suspend.
> 
> It is upto platform code to associate the PHY to the controller's
> port and it is upto the PHY driver to manage the PHY's resources.

"up to" is two words, not one.

> Signed-off-by: Roger Quadros <rogerq@ti.com>
> ---
>  drivers/usb/host/ehci-omap.c |   54 ++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 54 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index fd2f5450..a35e44e 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -70,6 +70,11 @@ static const char hcd_name[] = "ehci-omap";
>  
>  /*-------------------------------------------------------------------------*/
>  
> +struct omap_hcd {
> +	struct usb_hcd *hcd;

This pointer is not needed any more.

> +	struct usb_phy **phy;	/* one PHY for each port */
> +	int nports;

Is there a reasonable upper limit on the number of ports?  If there is 
you could just put a fixed-size array here.

> @@ -233,6 +240,39 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
>  	hcd->rsrc_len = resource_size(res);
>  	hcd->regs = regs;
>  
> +	omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv;
> +	omap->nports = pdata->nports;
> +	i = sizeof(struct usb_phy *) * omap->nports;
> +	omap->phy = devm_kzalloc(&pdev->dev, i, GFP_KERNEL);
> +	if (!omap->phy) {
> +		dev_err(dev, "Memory allocation failed\n");
> +		return -ENOMEM;

You have to put a "goto" here, not a "return".  The hcd must be cleaned 
up properly.

> @@ -295,11 +342,18 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
>  {
>  	struct device *dev				= &pdev->dev;
>  	struct usb_hcd *hcd				= dev_get_drvdata(dev);
> +	struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv;

What's with the weird spacing in the first two declarations?  This 
isn't a typeset document where the paragraphs need to be fully 
justified.  :-)

> +	int i;
>  
>  	usb_remove_hcd(hcd);
>  	disable_put_regulator(dev->platform_data);
>  	usb_put_hcd(hcd);

This line must be moved down.  When the hcd is deallocated, the
omap_hcd structure goes with it.

> +	for (i = 0; i < omap->nports; i++) {
> +		if (omap->phy[i])
> +			usb_phy_shutdown(omap->phy[i]);
> +	}
> +

Alan Stern
Roger Quadros Jan. 28, 2013, 3:58 p.m. UTC | #2
On 01/28/2013 05:40 PM, Alan Stern wrote:
> On Mon, 28 Jan 2013, Roger Quadros wrote:
> 
>> For each port that is in PHY mode we obtain a PHY device using the USB PHY
>> library and put it out of suspend.
>>
>> It is upto platform code to associate the PHY to the controller's
>> port and it is upto the PHY driver to manage the PHY's resources.
> 
> "up to" is two words, not one.

right :P

> 
>> Signed-off-by: Roger Quadros <rogerq@ti.com>
>> ---
>>  drivers/usb/host/ehci-omap.c |   54 ++++++++++++++++++++++++++++++++++++++++++
>>  1 files changed, 54 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
>> index fd2f5450..a35e44e 100644
>> --- a/drivers/usb/host/ehci-omap.c
>> +++ b/drivers/usb/host/ehci-omap.c
>> @@ -70,6 +70,11 @@ static const char hcd_name[] = "ehci-omap";
>>  
>>  /*-------------------------------------------------------------------------*/
>>  
>> +struct omap_hcd {
>> +	struct usb_hcd *hcd;
> 
> This pointer is not needed any more.
> 
OK.

>> +	struct usb_phy **phy;	/* one PHY for each port */
>> +	int nports;
> 
> Is there a reasonable upper limit on the number of ports?  If there is 
> you could just put a fixed-size array here.

For now there are only 3 which is defined in OMAP3_HS_USB_PORTS. 

> 
>> @@ -233,6 +240,39 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
>>  	hcd->rsrc_len = resource_size(res);
>>  	hcd->regs = regs;
>>  
>> +	omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv;
>> +	omap->nports = pdata->nports;
>> +	i = sizeof(struct usb_phy *) * omap->nports;
>> +	omap->phy = devm_kzalloc(&pdev->dev, i, GFP_KERNEL);
>> +	if (!omap->phy) {
>> +		dev_err(dev, "Memory allocation failed\n");
>> +		return -ENOMEM;
> 
> You have to put a "goto" here, not a "return".  The hcd must be cleaned 
> up properly.

good catch!

> 
>> @@ -295,11 +342,18 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
>>  {
>>  	struct device *dev				= &pdev->dev;
>>  	struct usb_hcd *hcd				= dev_get_drvdata(dev);
>> +	struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv;
> 
> What's with the weird spacing in the first two declarations?  This 
> isn't a typeset document where the paragraphs need to be fully 
> justified.  :-)

Funny that it is all over the place ;). I'll fix it around whatever I touch.
> 
>> +	int i;
>>  
>>  	usb_remove_hcd(hcd);
>>  	disable_put_regulator(dev->platform_data);
>>  	usb_put_hcd(hcd);
> 
> This line must be moved down.  When the hcd is deallocated, the
> omap_hcd structure goes with it.

Good one. Will fix.
> 
>> +	for (i = 0; i < omap->nports; i++) {
>> +		if (omap->phy[i])
>> +			usb_phy_shutdown(omap->phy[i]);
>> +	}
>> +
> 

regards,
-roger
diff mbox

Patch

diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index fd2f5450..a35e44e 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -70,6 +70,11 @@  static const char hcd_name[] = "ehci-omap";
 
 /*-------------------------------------------------------------------------*/
 
+struct omap_hcd {
+	struct usb_hcd *hcd;
+	struct usb_phy **phy;	/* one PHY for each port */
+	int nports;
+};
 
 static inline void ehci_write(void __iomem *base, u32 reg, u32 val)
 {
@@ -179,6 +184,7 @@  static struct hc_driver __read_mostly ehci_omap_hc_driver;
 
 static const struct ehci_driver_overrides ehci_omap_overrides __initdata = {
 	.reset =		omap_ehci_init,
+	.extra_priv_size =	sizeof(struct omap_hcd),
 };
 
 /**
@@ -194,6 +200,7 @@  static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	struct usbhs_omap_platform_data		*pdata = dev->platform_data;
 	struct resource				*res;
 	struct usb_hcd				*hcd;
+	struct omap_hcd				*omap;
 	void __iomem				*regs;
 	int					ret = -ENODEV;
 	int					irq;
@@ -233,6 +240,39 @@  static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	hcd->rsrc_len = resource_size(res);
 	hcd->regs = regs;
 
+	omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv;
+	omap->nports = pdata->nports;
+	i = sizeof(struct usb_phy *) * omap->nports;
+	omap->phy = devm_kzalloc(&pdev->dev, i, GFP_KERNEL);
+	if (!omap->phy) {
+		dev_err(dev, "Memory allocation failed\n");
+		return -ENOMEM;
+	}
+
+	platform_set_drvdata(pdev, hcd);
+
+	/* get the PHY devices if needed */
+	for (i = 0 ; i < omap->nports ; i++) {
+		struct usb_phy *phy;
+
+		if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY)
+			continue;
+
+		/* get the PHY device */
+		phy = devm_usb_get_phy_dev(dev, i);
+		if (IS_ERR(phy) || !phy) {
+			ret = IS_ERR(phy) ? PTR_ERR(phy) : -ENODEV;
+			dev_err(dev, "Can't get PHY device for port %d: %d\n",
+					i, ret);
+			goto err_phy;
+		}
+
+		omap->phy[i] = phy;
+		usb_phy_init(omap->phy[i]);
+		/* bring PHY out of suspend */
+		usb_phy_set_suspend(omap->phy[i], 0);
+	}
+
 	/* get ehci regulator and enable */
 	for (i = 0 ; i < OMAP3_HS_USB_PORTS ; i++) {
 		if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY) {
@@ -277,6 +317,13 @@  static int ehci_hcd_omap_probe(struct platform_device *pdev)
 err_pm_runtime:
 	disable_put_regulator(pdata);
 	pm_runtime_put_sync(dev);
+
+err_phy:
+	for (i = 0; i < omap->nports; i++) {
+		if (omap->phy[i])
+			usb_phy_shutdown(omap->phy[i]);
+	}
+
 	usb_put_hcd(hcd);
 
 	return ret;
@@ -295,11 +342,18 @@  static int ehci_hcd_omap_remove(struct platform_device *pdev)
 {
 	struct device *dev				= &pdev->dev;
 	struct usb_hcd *hcd				= dev_get_drvdata(dev);
+	struct omap_hcd *omap = (struct omap_hcd *)hcd_to_ehci(hcd)->priv;
+	int i;
 
 	usb_remove_hcd(hcd);
 	disable_put_regulator(dev->platform_data);
 	usb_put_hcd(hcd);
 
+	for (i = 0; i < omap->nports; i++) {
+		if (omap->phy[i])
+			usb_phy_shutdown(omap->phy[i]);
+	}
+
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);