Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761280AbcJ1MjR (ORCPT ); Fri, 28 Oct 2016 08:39:17 -0400 Received: from mail-wm0-f49.google.com ([74.125.82.49]:38746 "EHLO mail-wm0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761214AbcJ1MjK (ORCPT ); Fri, 28 Oct 2016 08:39:10 -0400 Subject: Re: [PATCH v2 3/3] usb: musb: da8xx: Only execute the OTG workaround when phy in OTG mode To: David Lechner , 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> <4727b41c-ab42-7066-8863-a3799920b79e@lechnology.com> Cc: linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org From: Alexandre Bailon Message-ID: <52da6a0c-6259-6d48-f927-07ce037bf584@baylibre.com> Date: Fri, 28 Oct 2016 14:39:07 +0200 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: <4727b41c-ab42-7066-8863-a3799920b79e@lechnology.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5244 Lines: 117 On 10/28/2016 04:56 AM, David Lechner wrote: > 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 >> --- >> 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. I like this workaround. > > 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); > I haven't thought to this workaround. Actually, my goal with this patch was to prevent VBUSDRV to be turned off. When I hit the issues, I captured some traces and these traces let think VBUSDRV is turned off when the OTG workaround clear the bit MUSB_DEVCTL_SESSION.