Subject: Re: [PATCH v2 3/3] usb: musb: da8xx: Only execute the OTG workaround
 when phy in OTG mode
To: Alexandre Bailon <abailon@baylibre.com>, khilman@baylibre.com,
        b-liu@ti.com, balbi@kernel.org
References: <1477479503-5131-1-git-send-email-abailon@baylibre.com>
 <1477479503-5131-4-git-send-email-abailon@baylibre.com>
Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
From: David Lechner <david@lechnology.com>
Message-ID: <4727b41c-ab42-7066-8863-a3799920b79e@lechnology.com>
Date: Thu, 27 Oct 2016 21:56:34 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.3.0
MIME-Version: 1.0
In-Reply-To: <1477479503-5131-4-git-send-email-abailon@baylibre.com>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 4701
Lines: 109

On 10/26/2016 05:58 AM, Alexandre Bailon wrote:
> When the phy is forced in host mode, only the first hot plug and
> hot remove works. That is actually because the driver execute the
> OTG workaround, whereas it is not applicable in host or device mode.
> Indeed, to work correctly, the VBUS sense and session end comparator
> must be enabled, what is only possible when the phy is in OTG mode.
> Only execute the workaround if the phy is in OTG mode.
>
> Signed-off-by: Alexandre Bailon <abailon@baylibre.com>
> ---
>  drivers/usb/musb/da8xx.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c
> index 6749aa1..b8a6b65 100644
> --- a/drivers/usb/musb/da8xx.c
> +++ b/drivers/usb/musb/da8xx.c
> @@ -145,6 +145,17 @@ static void otg_timer(unsigned long _musb)
>  	unsigned long		flags;
>
>  	/*
> +	 * We should only execute the OTG workaround when the phy is in OTG
> +	 * mode. The workaround require the VBUS sense and the session end
> +	 * comparator to be enabled, what is only possible if the phy is in
> +	 * OTG mode. As the workaround is only required to detect if the
> +	 * controller must act as host or device, we can safely exit OTG is
> +	 * not in use.
> +	 */
> +	if (musb->port_mode != MUSB_PORT_MODE_DUAL_ROLE)

musb->port_mode is not valid if we have changed the mode via sysfs. It 
only reflects the mode set during driver probe.

Furthermore, this breaks the host mode completely for me. The first hot 
plug is not even detected.

> +		return;
> +
> +	/*
>  	 * We poll because DaVinci's won't expose several OTG-critical
>  	 * status change events (from the transceiver) otherwise.
>  	 */
>


The way this is working for me (on AM1808) is this:

The problem is not that the OTG workaround is being used. The problem is 
that after disconnect, the VBUSDRV is turned off. If you look at the 
handler for DA8XX_INTR_DRVVBUS in da8xx_musb_interrupt(), you will see 
that if VBUSDRV is off, then drvvbus == 0, which puts the musb state 
back to device mode.

I also ran into a similar problem a while back[1] that if you use a 
self-powered device in host mode, it immediately becomes disconnected. 
This is for the exact same reason. When a port detects a self-powered 
device, it turns of VBUSDRV, which triggers the DA8XX_INTR_DRVVBUS 
interrupt. As we have seen above, this takes the port out of host mode.

The workaround that I have found that seems to fix both cases is to add 
and else if statement that toggles the PHY host override when we are 
forcing host mode and the VBUSDRV is turned off.

Here is a partial diff of drivers/usb/musb/da8xx.c to show what I mean:

@@ -304,10 +309,14 @@ static irqreturn_t da8xx_musb_interrupt(int irq, 
void *hci)
          * Also, DRVVBUS pulses for SRP (but not at 5 V)...
          */
         if (status & (DA8XX_INTR_DRVVBUS << DA8XX_INTR_USB_SHIFT)) {
+               struct da8xx_glue *glue =
+                               dev_get_drvdata(musb->controller->parent);
                 int drvvbus = musb_readl(reg_base, DA8XX_USB_STAT_REG);
                 void __iomem *mregs = musb->mregs;
                 u8 devctl = musb_readb(mregs, MUSB_DEVCTL);
-               int err;
+               int cfgchip2, err;
+
+               regmap_read(glue->cfgchip, CFGCHIP(2), &cfgchip2);

                 err = musb->int_usb & MUSB_INTR_VBUSERROR;
                 if (err) {
@@ -332,10 +341,25 @@ static irqreturn_t da8xx_musb_interrupt(int irq, 
void *hci)
                         musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
                         portstate(musb->port1_status |= 
USB_PORT_STAT_POWER);
                         del_timer(&otg_workaround);
+               } else if ((cfgchip2 & CFGCHIP2_OTGMODE_MASK)
+                          == CFGCHIP2_OTGMODE_FORCE_HOST) {
+                       /*
+                        * If we are forcing host mode, VBUSDRV is 
turned off
+                        * after a device is disconnected. We need to 
toggle the
+                        * VBUS/ID override to trigger turn it back on, 
which
+                        * has the effect of triggering 
DA8XX_INTR_DRVVBUS again.
+                        */
+                       regmap_write_bits(glue->cfgchip, CFGCHIP(2),
+                               CFGCHIP2_OTGMODE_MASK,
+                               CFGCHIP2_OTGMODE_NO_OVERRIDE);
+                       regmap_write_bits(glue->cfgchip, CFGCHIP(2),
+                               CFGCHIP2_OTGMODE_MASK,
+                               CFGCHIP2_OTGMODE_FORCE_HOST);
                 } else {
                         musb->is_active = 0;
                         MUSB_DEV_MODE(musb);