On 10/28/2016 07:39 AM, Alexandre Bailon wrote:
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 <abai...@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.
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.
After having slept on this, I am realizing that the "correct" thing to
do here is highly hardware dependent. If you look at musb_probe() in
musb_core.c, you will see the true purpose of musb->port_mode. In host
mode, it only sets up a host device, in peripheral mode, it only sets up
a peripheral device and in otg mode, it sets up both.
So really, this musb->port_mode setting does not say anything about what
the hardware is actually capable of. It is just telling which devices
you want registered in the kernel. (As a side note, this means that the
dr_mode property is really not appropriate for device tree in your other
patch series - even though many existing USB devices use dr_mode anyway).
The CFGCHIP2_OTGMODE_* options are to work around hardware deficiencies.
They are only needed when a port is missing the required external
circuitry to function correctly. I think it is wrong to assume that if
someone selects a specific musb->port_mode then they need to enable one
of CFGCHIP2_OTGMODE_FORCE_*.
If the port has the proper circuitry for OTG, then one should be able to
select any of host, peripheral or otg mode without needing to set any of
CFGCHIP2_OTGMODE_FORCE_*.
So, I think if we want to be able to use CFGCHIP2_OTGMODE_FORCE_* for
special cases, then we need to add a module parameter (or this might fit
in device tree if we can figure out how to express it as "describing the
hardware"). The parameter will basically say "override PHY VBUS/ID in
host mode if and only if this parameter is enabled". We could also have
a parameter for peripheral mode that says "override PHY VBUS/ID in
peripheral mode if and only if this parameter is enabled".
As an example, on LEGO MINDSTORMS EV3, the USB port is wired for
peripheral only. There is nothing connected to the VBUSDRV or ID pins.
Furthermore, the VBUS pin is only connected to the USB jack and there is
not a way to generate VBUS power. So, we can set musb->port_mode =
MUSB_PORT_MODE_GADGET and everything will work as expected as long as we
don't set CFGCHIP2_OTGMODE_FORCE_PERIPHERAL. Overriding the PHY breaks
VBUS sense and we never get back to b_idle after a device disconnect.
(In fact, the only time one would ever need to set
CFGCHIP2_OTGMODE_FORCE_PERIPHERAL is if the VBUS pin is not connected at
all and/or the ID pin is hardwired to ground).
If we want to be crazy though and be able to switch between host and
peripheral mode anyway, even though the required circuits are missing,
we can set musb->port_mode = MUSB_PORT_MODE_OTG. Then we can write
"host" to the "mode" sysfs attribute to force the port into host mode.
However, in order for this to work, it requires that
CFGCHIP2_OTGMODE_FORCE_HOST is set because of the missing circuitry for
host mode. You have to supply your own external VBUS, but it does work.
TL;DR
I think you fill find that if we remove the da8xx_musb_set_mode()
callback completely, that both host and peripheral mode will work for
you. Overriding the PHY is only needed for unusual cases, like my
example where we are forcing host mode when the required circuitry is
missing.
