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

[David Lechner]

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@xxxxxxxxxxxx>
> ---
>  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)