[v4,3/4] usb: musb: Add a quirk flag to skip the phy set mode

Message ID 1478277818-5091-4-git-send-email-abailon@baylibre.com
State New
Headers show

Commit Message

Alexandre Bailon Nov. 4, 2016, 4:43 p.m.
During the init, the driver will use the mode to configure
the controller mode and the phy mode.
The PHY of DA8xx has some issues when the phy is forced in host or device.
Add way to skip the set mode and let the da8xx glue manage the phy mode.

Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

---
 drivers/usb/musb/musb_core.c | 15 ++++++++++-----
 drivers/usb/musb/musb_core.h |  1 +
 2 files changed, 11 insertions(+), 5 deletions(-)

-- 
2.7.3

Comments

David Lechner Nov. 4, 2016, 6:51 p.m. | #1
On 11/04/2016 11:43 AM, Alexandre Bailon wrote:
> During the init, the driver will use the mode to configure

> the controller mode and the phy mode.

> The PHY of DA8xx has some issues when the phy is forced in host or device.

> Add way to skip the set mode and let the da8xx glue manage the phy mode.

>


I think you are on the right track here. But I think a cleaner way to do 
this would be to add a bool parameter to musb_platform_set_mode() that 
is passed musb->ops->set_mode() callback. You could call this parameter 
init or first_call or something like that. This way, the set_mode() 
callback can have a different behavior depending on when it is called.

In musb_init_controller(), for example, you would have...

	status = musb_platform_set_mode(musb, MUSB_HOST, true);

and in musb_mode_store(), you would have...

	status = musb_platform_set_mode(musb, MUSB_HOST, false);



> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

> ---

>  drivers/usb/musb/musb_core.c | 15 ++++++++++-----

>  drivers/usb/musb/musb_core.h |  1 +

>  2 files changed, 11 insertions(+), 5 deletions(-)

>

> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c

> index 27dadc0..6f5f039 100644

> --- a/drivers/usb/musb/musb_core.c

> +++ b/drivers/usb/musb/musb_core.c

> @@ -2046,6 +2046,7 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)

>  {

>  	int			status;

>  	struct musb		*musb;

> +	enum musb_mode		mode = MUSB_UNDEFINED;

>  	struct musb_hdrc_platform_data *plat = dev_get_platdata(dev);

>

>  	/* The driver might handle more features than the board; OK.

> @@ -2261,13 +2262,13 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)

>  		status = musb_host_setup(musb, plat->power);

>  		if (status < 0)

>  			goto fail3;

> -		status = musb_platform_set_mode(musb, MUSB_HOST);

> +		mode = MUSB_HOST;

>  		break;

>  	case MUSB_PORT_MODE_GADGET:

>  		status = musb_gadget_setup(musb);

>  		if (status < 0)

>  			goto fail3;

> -		status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);

> +		mode = MUSB_PERIPHERAL;

>  		break;

>  	case MUSB_PORT_MODE_DUAL_ROLE:

>  		status = musb_host_setup(musb, plat->power);

> @@ -2278,15 +2279,19 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)

>  			musb_host_cleanup(musb);

>  			goto fail3;

>  		}

> -		status = musb_platform_set_mode(musb, MUSB_OTG);

> +		mode = MUSB_OTG;

>  		break;

>  	default:

>  		dev_err(dev, "unsupported port mode %d\n", musb->port_mode);

>  		break;

>  	}

>

> -	if (status < 0)

> -		goto fail3;

> +	if (mode != MUSB_UNDEFINED &&

> +		!(musb->io.quirks & MUSB_SKIP_SET_MODE)) {

> +		status = musb_platform_set_mode(musb, mode);

> +		if (status < 0)

> +			goto fail3;

> +	}

>

>  	status = musb_init_debugfs(musb);

>  	if (status < 0)

> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h

> index 2cb88a49..9bd8b6b 100644

> --- a/drivers/usb/musb/musb_core.h

> +++ b/drivers/usb/musb/musb_core.h

> @@ -172,6 +172,7 @@ struct musb_io;

>   */

>  struct musb_platform_ops {

>

> +#define MUSB_SKIP_SET_MODE	BIT(7)


This would be better described as MUSB_SKIP_SET_MODE_IN_INIT

>  #define MUSB_DMA_UX500		BIT(6)

>  #define MUSB_DMA_CPPI41		BIT(5)

>  #define MUSB_DMA_CPPI		BIT(4)

>
Sergei Shtylyov Nov. 4, 2016, 8:20 p.m. | #2
Hello.

On 11/04/2016 07:43 PM, Alexandre Bailon wrote:

> During the init, the driver will use the mode to configure

> the controller mode and the phy mode.


    PHY -- be consistent please...

> The PHY of DA8xx has some issues when the phy is forced in host or device.


    Again.

> Add way to skip the set mode and let the da8xx glue manage the phy mode.

>

> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>

> ---

>  drivers/usb/musb/musb_core.c | 15 ++++++++++-----

>  drivers/usb/musb/musb_core.h |  1 +

>  2 files changed, 11 insertions(+), 5 deletions(-)

>

> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c

> index 27dadc0..6f5f039 100644

> --- a/drivers/usb/musb/musb_core.c

> +++ b/drivers/usb/musb/musb_core.c

[...]
> @@ -2278,15 +2279,19 @@ musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)

[...]
> -	if (status < 0)

> -		goto fail3;

> +	if (mode != MUSB_UNDEFINED &&

> +		!(musb->io.quirks & MUSB_SKIP_SET_MODE)) {


    Please either add one more tab here or align to 'mode', so it's easier on 
the eyes.

> +		status = musb_platform_set_mode(musb, mode);

> +		if (status < 0)

> +			goto fail3;

> +	}

>

>  	status = musb_init_debugfs(musb);

>  	if (status < 0)

[...]

MBR, Sergei

Patch

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 27dadc0..6f5f039 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -2046,6 +2046,7 @@  musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 {
 	int			status;
 	struct musb		*musb;
+	enum musb_mode		mode = MUSB_UNDEFINED;
 	struct musb_hdrc_platform_data *plat = dev_get_platdata(dev);
 
 	/* The driver might handle more features than the board; OK.
@@ -2261,13 +2262,13 @@  musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 		status = musb_host_setup(musb, plat->power);
 		if (status < 0)
 			goto fail3;
-		status = musb_platform_set_mode(musb, MUSB_HOST);
+		mode = MUSB_HOST;
 		break;
 	case MUSB_PORT_MODE_GADGET:
 		status = musb_gadget_setup(musb);
 		if (status < 0)
 			goto fail3;
-		status = musb_platform_set_mode(musb, MUSB_PERIPHERAL);
+		mode = MUSB_PERIPHERAL;
 		break;
 	case MUSB_PORT_MODE_DUAL_ROLE:
 		status = musb_host_setup(musb, plat->power);
@@ -2278,15 +2279,19 @@  musb_init_controller(struct device *dev, int nIrq, void __iomem *ctrl)
 			musb_host_cleanup(musb);
 			goto fail3;
 		}
-		status = musb_platform_set_mode(musb, MUSB_OTG);
+		mode = MUSB_OTG;
 		break;
 	default:
 		dev_err(dev, "unsupported port mode %d\n", musb->port_mode);
 		break;
 	}
 
-	if (status < 0)
-		goto fail3;
+	if (mode != MUSB_UNDEFINED &&
+		!(musb->io.quirks & MUSB_SKIP_SET_MODE)) {
+		status = musb_platform_set_mode(musb, mode);
+		if (status < 0)
+			goto fail3;
+	}
 
 	status = musb_init_debugfs(musb);
 	if (status < 0)
diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
index 2cb88a49..9bd8b6b 100644
--- a/drivers/usb/musb/musb_core.h
+++ b/drivers/usb/musb/musb_core.h
@@ -172,6 +172,7 @@  struct musb_io;
  */
 struct musb_platform_ops {
 
+#define MUSB_SKIP_SET_MODE	BIT(7)
 #define MUSB_DMA_UX500		BIT(6)
 #define MUSB_DMA_CPPI41		BIT(5)
 #define MUSB_DMA_CPPI		BIT(4)