[v3,5/5] usb: musb: da8xx: Remove set_mode callback

Message ID 1478186765-19840-6-git-send-email-abailon@baylibre.com
State New
Headers show

Commit Message

Alexandre Bailon Nov. 3, 2016, 3:26 p.m.
The USB PHY is able to operate in OTG, host or peripheral.
Some board may be wired to work act only as host or peripheral.
In such case, the dr_mode property of controller must be set to
host or peripheral. But doing that will also configure the PHY
in host or peripheral mode whereas OTG is able to detect which
role the USB controller should take.
The PHY's host or peripheral mode are actually only useful when
hardware doesn't allow OTG to detect it's role.

The set_mode callback is used by the musb driver to set mode
of the PHY. But in the case of DA8xx, the PHY have some issues.
The OTG mode work correctly but the host and peripheral don't.
In host mode, the PHY stops to work after the first disconnect.
In device mode, the PHY doesn't detect any disconnect.
As the OTG mode is working properly, let the PHY in OTG mode,
whatever is the controller mode.

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

---
 drivers/usb/musb/da8xx.c | 23 -----------------------
 1 file changed, 23 deletions(-)

-- 
2.7.3

Comments

David Lechner Nov. 3, 2016, 5:18 p.m. | #1
On 11/03/2016 10:26 AM, Alexandre Bailon wrote:
> The USB PHY is able to operate in OTG, host or peripheral.

> Some board may be wired to work act only as host or peripheral.

> In such case, the dr_mode property of controller must be set to

> host or peripheral. But doing that will also configure the PHY

> in host or peripheral mode whereas OTG is able to detect which

> role the USB controller should take.

> The PHY's host or peripheral mode are actually only useful when

> hardware doesn't allow OTG to detect it's role.

>

> The set_mode callback is used by the musb driver to set mode

> of the PHY. But in the case of DA8xx, the PHY have some issues.

> The OTG mode work correctly but the host and peripheral don't.

> In host mode, the PHY stops to work after the first disconnect.

> In device mode, the PHY doesn't detect any disconnect.

> As the OTG mode is working properly, let the PHY in OTG mode,

> whatever is the controller mode.

>

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

> ---

>  drivers/usb/musb/da8xx.c | 23 -----------------------

>  1 file changed, 23 deletions(-)

>

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

> index 6749aa1..581f830 100644

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

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

> @@ -335,28 +335,6 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)

>  	return ret;

>  }

>

> -static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)


Bin suggested using some sort of quirks flag. So instead of removing 
this callback, I think this is where to incorporate the quirks flags.

I suppose the quirks could be kernel config options. Perhaps someone 
else has a better idea?

> -{

> -	struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);

> -	enum phy_mode phy_mode;

> -

> -	switch (musb_mode) {

> -	case MUSB_HOST:		/* Force VBUS valid, ID = 0 */

> -		phy_mode = PHY_MODE_USB_HOST;


this should be something like...

		phy_mode = force_host_mode_quirk ? PHY_MODE_USB_HOST
					 	 : PHY_MODE_USB_OTG;


> -		break;

> -	case MUSB_PERIPHERAL:	/* Force VBUS valid, ID = 1 */

> -		phy_mode = PHY_MODE_USB_DEVICE;


and...

		phy_mode = force_peripheral_mode_quirk ?
				PHY_MODE_USB_DEVICE : PHY_MODE_USB_OTG;


> -		break;

> -	case MUSB_OTG:		/* Don't override the VBUS/ID comparators */

> -		phy_mode = PHY_MODE_USB_OTG;

> -		break;

> -	default:

> -		return -EINVAL;

> -	}

> -

> -	return phy_set_mode(glue->phy, phy_mode);

> -}

> -

>  static int da8xx_musb_init(struct musb *musb)

>  {

>  	struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);

> @@ -445,7 +423,6 @@ static const struct musb_platform_ops da8xx_ops = {

>  	.enable		= da8xx_musb_enable,

>  	.disable	= da8xx_musb_disable,

>

> -	.set_mode	= da8xx_musb_set_mode,

>  	.try_idle	= da8xx_musb_try_idle,

>

>  	.set_vbus	= da8xx_musb_set_vbus,

>
Bin Liu Nov. 3, 2016, 5:27 p.m. | #2
On Thu, Nov 03, 2016 at 12:18:53PM -0500, David Lechner wrote:
> On 11/03/2016 10:26 AM, Alexandre Bailon wrote:

> >The USB PHY is able to operate in OTG, host or peripheral.

> >Some board may be wired to work act only as host or peripheral.

> >In such case, the dr_mode property of controller must be set to

> >host or peripheral. But doing that will also configure the PHY

> >in host or peripheral mode whereas OTG is able to detect which

> >role the USB controller should take.

> >The PHY's host or peripheral mode are actually only useful when

> >hardware doesn't allow OTG to detect it's role.

> >

> >The set_mode callback is used by the musb driver to set mode

> >of the PHY. But in the case of DA8xx, the PHY have some issues.

> >The OTG mode work correctly but the host and peripheral don't.

> >In host mode, the PHY stops to work after the first disconnect.

> >In device mode, the PHY doesn't detect any disconnect.

> >As the OTG mode is working properly, let the PHY in OTG mode,

> >whatever is the controller mode.

> >

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

> >---

> > drivers/usb/musb/da8xx.c | 23 -----------------------

> > 1 file changed, 23 deletions(-)

> >

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

> >index 6749aa1..581f830 100644

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

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

> >@@ -335,28 +335,6 @@ static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)

> > 	return ret;

> > }

> >

> >-static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)

> 

> Bin suggested using some sort of quirks flag. So instead of removing

> this callback, I think this is where to incorporate the quirks

> flags.

> 

> I suppose the quirks could be kernel config options. Perhaps someone

> else has a better idea?


I didn't closely follow this thread, but I was thinking about to reuse
musb->io.quirks, and define the quirks in device tree...

I am debugging an issue in dsps, and might need a quirk for the
solution...

Regards,
-Bin.

Patch

diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
index 6749aa1..581f830 100644
--- a/drivers/usb/musb/da8xx.c
+++ b/drivers/usb/musb/da8xx.c
@@ -335,28 +335,6 @@  static irqreturn_t da8xx_musb_interrupt(int irq, void *hci)
 	return ret;
 }
 
-static int da8xx_musb_set_mode(struct musb *musb, u8 musb_mode)
-{
-	struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
-	enum phy_mode phy_mode;
-
-	switch (musb_mode) {
-	case MUSB_HOST:		/* Force VBUS valid, ID = 0 */
-		phy_mode = PHY_MODE_USB_HOST;
-		break;
-	case MUSB_PERIPHERAL:	/* Force VBUS valid, ID = 1 */
-		phy_mode = PHY_MODE_USB_DEVICE;
-		break;
-	case MUSB_OTG:		/* Don't override the VBUS/ID comparators */
-		phy_mode = PHY_MODE_USB_OTG;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	return phy_set_mode(glue->phy, phy_mode);
-}
-
 static int da8xx_musb_init(struct musb *musb)
 {
 	struct da8xx_glue *glue = dev_get_drvdata(musb->controller->parent);
@@ -445,7 +423,6 @@  static const struct musb_platform_ops da8xx_ops = {
 	.enable		= da8xx_musb_enable,
 	.disable	= da8xx_musb_disable,
 
-	.set_mode	= da8xx_musb_set_mode,
 	.try_idle	= da8xx_musb_try_idle,
 
 	.set_vbus	= da8xx_musb_set_vbus,