diff mbox

[V7,02/16] usb: gadget: mv_udc: use PHY driver for udc

Message ID 1361167838-22929-3-git-send-email-chao.xie@marvell.com (mailing list archive)
State New, archived
Headers show

Commit Message

Chao Xie Feb. 18, 2013, 6:10 a.m. UTC 
Originaly, udc driver will call the callbacks in platform data
for PHY initialization and shut down.
With PHY driver, it will call the APIs provided by PHY driver
for PHY initialization and shut down. It removes the callbacks
in platform data, and at same time it removes one block in the
way of enabling device tree for udc driver.

Signed-off-by: Chao Xie <chao.xie@marvell.com>
---
 drivers/usb/gadget/mv_udc.h      |    2 +-
 drivers/usb/gadget/mv_udc_core.c |   37 +++++++++++++++----------------------
 2 files changed, 16 insertions(+), 23 deletions(-)

Comments

Kishon Vijay Abraham I Feb. 18, 2013, 8:20 a.m. UTC | #1 
Hi,

On Monday 18 February 2013 11:40 AM, Chao Xie wrote:
> Originaly, udc driver will call the callbacks in platform data
> for PHY initialization and shut down.
> With PHY driver, it will call the APIs provided by PHY driver
> for PHY initialization and shut down. It removes the callbacks
> in platform data, and at same time it removes one block in the
> way of enabling device tree for udc driver.
>
> Signed-off-by: Chao Xie <chao.xie@marvell.com>
> ---
>   drivers/usb/gadget/mv_udc.h      |    2 +-
>   drivers/usb/gadget/mv_udc_core.c |   37 +++++++++++++++----------------------
>   2 files changed, 16 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/usb/gadget/mv_udc.h b/drivers/usb/gadget/mv_udc.h
> index 9073436..f339df4 100644
> --- a/drivers/usb/gadget/mv_udc.h
> +++ b/drivers/usb/gadget/mv_udc.h
> @@ -180,7 +180,6 @@ struct mv_udc {
>
>   	struct mv_cap_regs __iomem	*cap_regs;
>   	struct mv_op_regs __iomem	*op_regs;
> -	void __iomem                    *phy_regs;
>   	unsigned int			max_eps;
>   	struct mv_dqh			*ep_dqh;
>   	size_t				ep_dqh_size;
> @@ -217,6 +216,7 @@ struct mv_udc {
>   	struct work_struct	vbus_work;
>   	struct workqueue_struct *qwork;
>
> +	struct usb_phy		*phy;
>   	struct usb_phy		*transceiver;
>
>   	struct mv_usb_platform_data     *pdata;
> diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c
> index c8cf959..d3ce1aa 100644
> --- a/drivers/usb/gadget/mv_udc_core.c
> +++ b/drivers/usb/gadget/mv_udc_core.c
> @@ -35,6 +35,7 @@
>   #include <linux/platform_device.h>
>   #include <linux/clk.h>
>   #include <linux/platform_data/mv_usb.h>
> +#include <linux/usb/mv_usb2.h>
>   #include <asm/unaligned.h>
>
>   #include "mv_udc.h"
> @@ -1121,15 +1122,14 @@ static int mv_udc_enable_internal(struct mv_udc *udc)
>
>   	dev_dbg(&udc->dev->dev, "enable udc\n");
>   	udc_clock_enable(udc);
> -	if (udc->pdata->phy_init) {
> -		retval = udc->pdata->phy_init(udc->phy_regs);
> -		if (retval) {
> -			dev_err(&udc->dev->dev,
> -				"init phy error %d\n", retval);
> -			udc_clock_disable(udc);
> -			return retval;
> -		}
> +	retval = usb_phy_init(udc->phy);
> +	if (retval) {
> +		dev_err(&udc->dev->dev,
> +			"init phy error %d\n", retval);
> +		udc_clock_disable(udc);
> +		return retval;
>   	}
> +
>   	udc->active = 1;
>
>   	return 0;
> @@ -1147,8 +1147,7 @@ static void mv_udc_disable_internal(struct mv_udc *udc)
>   {
>   	if (udc->active) {
>   		dev_dbg(&udc->dev->dev, "disable udc\n");
> -		if (udc->pdata->phy_deinit)
> -			udc->pdata->phy_deinit(udc->phy_regs);
> +		usb_phy_shutdown(udc->phy);
>   		udc_clock_disable(udc);
>   		udc->active = 0;
>   	}
> @@ -2175,8 +2174,8 @@ static int mv_udc_probe(struct platform_device *pdev)
>
>   #ifdef CONFIG_USB_OTG_UTILS
>   	if (pdata->mode == MV_USB_MODE_OTG) {
> -		udc->transceiver = devm_usb_get_phy(&pdev->dev,
> -					USB_PHY_TYPE_USB2);
> +		udc->transceiver = devm_usb_get_phy_dev(&pdev->dev,
> +					MV_USB2_OTG_PHY_INDEX);
>   		if (IS_ERR_OR_NULL(udc->transceiver)) {
>   			udc->transceiver = NULL;
>   			return -ENODEV;
> @@ -2194,7 +2193,7 @@ static int mv_udc_probe(struct platform_device *pdev)
>   		}
>   	}
>
> -	r = platform_get_resource_byname(udc->dev, IORESOURCE_MEM, "capregs");
> +	r = platform_get_resource(udc->dev, IORESOURCE_MEM, 0);
>   	if (r == NULL) {
>   		dev_err(&pdev->dev, "no I/O memory resource defined\n");
>   		return -ENODEV;
> @@ -2207,18 +2206,12 @@ static int mv_udc_probe(struct platform_device *pdev)
>   		return -EBUSY;
>   	}
>
> -	r = platform_get_resource_byname(udc->dev, IORESOURCE_MEM, "phyregs");
> -	if (r == NULL) {
> -		dev_err(&pdev->dev, "no phy I/O memory resource defined\n");
> +	udc->phy = devm_usb_get_phy_dev(&pdev->dev, MV_USB2_PHY_INDEX);
> +	if (IS_ERR_OR_NULL(udc->phy)) {
> +		udc->phy = NULL;

Ideally you shouldn't do this. Consider the case where 
devm_usb_get_phy_dev returns -EPROBE_DEFER. In that case you should 
return -EPROBE_DEFER from probe instead of manually writing the return 
value to -ENODEV.

This comment is applicable for the remaining patches that uses 
devm_usb_get_phy_dev.

Thanks
Kishon
diff mbox

Patch

diff --git a/drivers/usb/gadget/mv_udc.h b/drivers/usb/gadget/mv_udc.h
index 9073436..f339df4 100644
--- a/drivers/usb/gadget/mv_udc.h
+++ b/drivers/usb/gadget/mv_udc.h
@@ -180,7 +180,6 @@  struct mv_udc {
 
 	struct mv_cap_regs __iomem	*cap_regs;
 	struct mv_op_regs __iomem	*op_regs;
-	void __iomem                    *phy_regs;
 	unsigned int			max_eps;
 	struct mv_dqh			*ep_dqh;
 	size_t				ep_dqh_size;
@@ -217,6 +216,7 @@  struct mv_udc {
 	struct work_struct	vbus_work;
 	struct workqueue_struct *qwork;
 
+	struct usb_phy		*phy;
 	struct usb_phy		*transceiver;
 
 	struct mv_usb_platform_data     *pdata;
diff --git a/drivers/usb/gadget/mv_udc_core.c b/drivers/usb/gadget/mv_udc_core.c
index c8cf959..d3ce1aa 100644
--- a/drivers/usb/gadget/mv_udc_core.c
+++ b/drivers/usb/gadget/mv_udc_core.c
@@ -35,6 +35,7 @@ 
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/platform_data/mv_usb.h>
+#include <linux/usb/mv_usb2.h>
 #include <asm/unaligned.h>
 
 #include "mv_udc.h"
@@ -1121,15 +1122,14 @@  static int mv_udc_enable_internal(struct mv_udc *udc)
 
 	dev_dbg(&udc->dev->dev, "enable udc\n");
 	udc_clock_enable(udc);
-	if (udc->pdata->phy_init) {
-		retval = udc->pdata->phy_init(udc->phy_regs);
-		if (retval) {
-			dev_err(&udc->dev->dev,
-				"init phy error %d\n", retval);
-			udc_clock_disable(udc);
-			return retval;
-		}
+	retval = usb_phy_init(udc->phy);
+	if (retval) {
+		dev_err(&udc->dev->dev,
+			"init phy error %d\n", retval);
+		udc_clock_disable(udc);
+		return retval;
 	}
+
 	udc->active = 1;
 
 	return 0;
@@ -1147,8 +1147,7 @@  static void mv_udc_disable_internal(struct mv_udc *udc)
 {
 	if (udc->active) {
 		dev_dbg(&udc->dev->dev, "disable udc\n");
-		if (udc->pdata->phy_deinit)
-			udc->pdata->phy_deinit(udc->phy_regs);
+		usb_phy_shutdown(udc->phy);
 		udc_clock_disable(udc);
 		udc->active = 0;
 	}
@@ -2175,8 +2174,8 @@  static int mv_udc_probe(struct platform_device *pdev)
 
 #ifdef CONFIG_USB_OTG_UTILS
 	if (pdata->mode == MV_USB_MODE_OTG) {
-		udc->transceiver = devm_usb_get_phy(&pdev->dev,
-					USB_PHY_TYPE_USB2);
+		udc->transceiver = devm_usb_get_phy_dev(&pdev->dev,
+					MV_USB2_OTG_PHY_INDEX);
 		if (IS_ERR_OR_NULL(udc->transceiver)) {
 			udc->transceiver = NULL;
 			return -ENODEV;
@@ -2194,7 +2193,7 @@  static int mv_udc_probe(struct platform_device *pdev)
 		}
 	}
 
-	r = platform_get_resource_byname(udc->dev, IORESOURCE_MEM, "capregs");
+	r = platform_get_resource(udc->dev, IORESOURCE_MEM, 0);
 	if (r == NULL) {
 		dev_err(&pdev->dev, "no I/O memory resource defined\n");
 		return -ENODEV;
@@ -2207,18 +2206,12 @@  static int mv_udc_probe(struct platform_device *pdev)
 		return -EBUSY;
 	}
 
-	r = platform_get_resource_byname(udc->dev, IORESOURCE_MEM, "phyregs");
-	if (r == NULL) {
-		dev_err(&pdev->dev, "no phy I/O memory resource defined\n");
+	udc->phy = devm_usb_get_phy_dev(&pdev->dev, MV_USB2_PHY_INDEX);
+	if (IS_ERR_OR_NULL(udc->phy)) {
+		udc->phy = NULL;
 		return -ENODEV;
 	}
 
-	udc->phy_regs = ioremap(r->start, resource_size(r));
-	if (udc->phy_regs == NULL) {
-		dev_err(&pdev->dev, "failed to map phy I/O memory\n");
-		return -EBUSY;
-	}
-
 	/* we will acces controller register, so enable the clk */
 	retval = mv_udc_enable_internal(udc);
 	if (retval)