[v2,2/3] phy: da8xx-usb: Configure CFGCHIP2 to support OTG workaround

Message ID 1477479503-5131-3-git-send-email-abailon@baylibre.com
State New
Headers show

Commit Message

Alexandre Bailon Oct. 26, 2016, 10:58 a.m.
If we configure the da8xx OTG phy in OTG mode, neither device or host
mode will work. That is because the PHY is not able to detect and notify
the driver that value of ID pin changed.
To work despite this hardware limitation, the da8xx glue implement a
workaround.
But to work, the workaround require the VBUS sense and the session end
comparator to enabled.
Enable them if the phy is configured in OTG mode.

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

---
 drivers/phy/phy-da8xx-usb.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

-- 
2.7.3

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

David Lechner Oct. 26, 2016, 3:40 p.m. | #1
On 10/26/2016 05:58 AM, Alexandre Bailon wrote:
> If we configure the da8xx OTG phy in OTG mode, neither device or host

> mode will work. That is because the PHY is not able to detect and notify

> the driver that value of ID pin changed.

> To work despite this hardware limitation, the da8xx glue implement a

> workaround.

> But to work, the workaround require the VBUS sense and the session end

> comparator to enabled.

> Enable them if the phy is configured in OTG mode.

>

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

> ---

>  drivers/phy/phy-da8xx-usb.c | 17 ++++++++++++-----

>  1 file changed, 12 insertions(+), 5 deletions(-)

>

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

> index 32ae78c..fd39292 100644

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

> +++ b/drivers/phy/phy-da8xx-usb.c

> @@ -93,24 +93,31 @@ static int da8xx_usb20_phy_power_off(struct phy *phy)

>  static int da8xx_usb20_phy_set_mode(struct phy *phy, enum phy_mode mode)

>  {

>  	struct da8xx_usb_phy *d_phy = phy_get_drvdata(phy);

> +	int ret;

>  	u32 val;

>

> +	ret = regmap_read(d_phy->regmap, CFGCHIP(2), &val);

> +	if (ret)

> +		return ret;

> +

> +	val &= ~CFGCHIP2_OTGMODE_MASK;

> +

>  	switch (mode) {

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

> -		val = CFGCHIP2_OTGMODE_FORCE_HOST;

> +		val |= CFGCHIP2_OTGMODE_FORCE_HOST;

>  		break;

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

> -		val = CFGCHIP2_OTGMODE_FORCE_DEVICE;

> +		val |= CFGCHIP2_OTGMODE_FORCE_DEVICE;

>  		break;

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

> -		val = CFGCHIP2_OTGMODE_NO_OVERRIDE;

> +		val |= CFGCHIP2_OTGMODE_NO_OVERRIDE |

> +			CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN;


The AM1808 TRM indicates that CFGCHIP2_SESENDEN and CFGCHIP2_VBDTCTEN 
(and CFGCHIP2_DATPOL) should be on for normal operation of the USB 2.0 
PHY. They do not appear to be associated with the OTG mode specifically.

It seems to me that it would be more appropriate to set these in 
da8xx_usb20_phy_power_on() instead of here in da8xx_usb20_phy_set_mode().


>  		break;

>  	default:

>  		return -EINVAL;

>  	}

>

> -	regmap_write_bits(d_phy->regmap, CFGCHIP(2), CFGCHIP2_OTGMODE_MASK,

> -			  val);

> +	regmap_write(d_phy->regmap, CFGCHIP(2), val);

>

>  	return 0;

>  }

>


Also cc'ing phy maintainer since this is a phy driver.


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Alexandre Bailon Oct. 26, 2016, 4:43 p.m. | #2
On 10/26/2016 05:40 PM, David Lechner wrote:
> On 10/26/2016 05:58 AM, Alexandre Bailon wrote:

>> If we configure the da8xx OTG phy in OTG mode, neither device or host

>> mode will work. That is because the PHY is not able to detect and notify

>> the driver that value of ID pin changed.

>> To work despite this hardware limitation, the da8xx glue implement a

>> workaround.

>> But to work, the workaround require the VBUS sense and the session end

>> comparator to enabled.

>> Enable them if the phy is configured in OTG mode.

>>

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

>> ---

>>  drivers/phy/phy-da8xx-usb.c | 17 ++++++++++++-----

>>  1 file changed, 12 insertions(+), 5 deletions(-)

>>

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

>> index 32ae78c..fd39292 100644

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

>> +++ b/drivers/phy/phy-da8xx-usb.c

>> @@ -93,24 +93,31 @@ static int da8xx_usb20_phy_power_off(struct phy *phy)

>>  static int da8xx_usb20_phy_set_mode(struct phy *phy, enum phy_mode mode)

>>  {

>>      struct da8xx_usb_phy *d_phy = phy_get_drvdata(phy);

>> +    int ret;

>>      u32 val;

>>

>> +    ret = regmap_read(d_phy->regmap, CFGCHIP(2), &val);

>> +    if (ret)

>> +        return ret;

>> +

>> +    val &= ~CFGCHIP2_OTGMODE_MASK;

>> +

>>      switch (mode) {

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

>> -        val = CFGCHIP2_OTGMODE_FORCE_HOST;

>> +        val |= CFGCHIP2_OTGMODE_FORCE_HOST;

>>          break;

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

>> -        val = CFGCHIP2_OTGMODE_FORCE_DEVICE;

>> +        val |= CFGCHIP2_OTGMODE_FORCE_DEVICE;

>>          break;

>>      case PHY_MODE_USB_OTG:    /* Don't override the VBUS/ID

>> comparators */

>> -        val = CFGCHIP2_OTGMODE_NO_OVERRIDE;

>> +        val |= CFGCHIP2_OTGMODE_NO_OVERRIDE |

>> +            CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN;

> 

> The AM1808 TRM indicates that CFGCHIP2_SESENDEN and CFGCHIP2_VBDTCTEN

> (and CFGCHIP2_DATPOL) should be on for normal operation of the USB 2.0

> PHY. They do not appear to be associated with the OTG mode specifically.

I agree but the function assigned to these bit is highly tied to OTG.
And in TI BSP, there is a comment that let me think that is only needed
for OTG.
/*
 * We have to override VBUS/ID signals when MUSB is configured into the
 * host-only mode -- ID pin will float if no cable is connected, so the
 * controller won't be able to drive VBUS thinking that it's a B-device.
 * Otherwise, we want to use the OTG mode and enable VBUS comparators.
 */
	cfgchip2 &= ~CFGCHIP2_OTGMODE;
#ifdef	CONFIG_USB_MUSB_HOST
	cfgchip2 |=  CFGCHIP2_FORCE_HOST;
#else
	cfgchip2 |=  CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN;
#endif
> 

> It seems to me that it would be more appropriate to set these in

> da8xx_usb20_phy_power_on() instead of here in da8xx_usb20_phy_set_mode().

I tried both the phy forced in device or host mode with theses bit set
or clear and I haven't see any issues, so I think we could set them in
da8xx_usb20_phy_power_on() as you suggested.
> 

> 

>>          break;

>>      default:

>>          return -EINVAL;

>>      }

>>

>> -    regmap_write_bits(d_phy->regmap, CFGCHIP(2), CFGCHIP2_OTGMODE_MASK,

>> -              val);

>> +    regmap_write(d_phy->regmap, CFGCHIP(2), val);

>>

>>      return 0;

>>  }

>>

> 

> Also cc'ing phy maintainer since this is a phy driver.

> 

> 


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch hide | download patch | download mbox

diff --git a/drivers/phy/phy-da8xx-usb.c b/drivers/phy/phy-da8xx-usb.c
index 32ae78c..fd39292 100644
--- a/drivers/phy/phy-da8xx-usb.c
+++ b/drivers/phy/phy-da8xx-usb.c
@@ -93,24 +93,31 @@  static int da8xx_usb20_phy_power_off(struct phy *phy)
 static int da8xx_usb20_phy_set_mode(struct phy *phy, enum phy_mode mode)
 {
 	struct da8xx_usb_phy *d_phy = phy_get_drvdata(phy);
+	int ret;
 	u32 val;
 
+	ret = regmap_read(d_phy->regmap, CFGCHIP(2), &val);
+	if (ret)
+		return ret;
+
+	val &= ~CFGCHIP2_OTGMODE_MASK;
+
 	switch (mode) {
 	case PHY_MODE_USB_HOST:		/* Force VBUS valid, ID = 0 */
-		val = CFGCHIP2_OTGMODE_FORCE_HOST;
+		val |= CFGCHIP2_OTGMODE_FORCE_HOST;
 		break;
 	case PHY_MODE_USB_DEVICE:	/* Force VBUS valid, ID = 1 */
-		val = CFGCHIP2_OTGMODE_FORCE_DEVICE;
+		val |= CFGCHIP2_OTGMODE_FORCE_DEVICE;
 		break;
 	case PHY_MODE_USB_OTG:	/* Don't override the VBUS/ID comparators */
-		val = CFGCHIP2_OTGMODE_NO_OVERRIDE;
+		val |= CFGCHIP2_OTGMODE_NO_OVERRIDE |
+			CFGCHIP2_SESENDEN | CFGCHIP2_VBDTCTEN;
 		break;
 	default:
 		return -EINVAL;
 	}
 
-	regmap_write_bits(d_phy->regmap, CFGCHIP(2), CFGCHIP2_OTGMODE_MASK,
-			  val);
+	regmap_write(d_phy->regmap, CFGCHIP(2), val);
 
 	return 0;
 }