> > +#define USB_SOFT_RESET_MASK 1 > > Need a empty line here. Hmm, ok. > > > +#define A_WAIT_BCON_TIMEOUT 1100 /* in ms */ > > I think this #define should be dropped -- see below... > > > +{ > > + unsigned long timeout = jiffies + msecs_to_jiffies(100); > > + u32 devconf2; > > + > > + /* > > + * Start the on-chip PHY and its PLL. > > + */ > > + devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2); > > + > > + devconf2 &= ~(CONF2_RESET | CONF2_PHYPWRDN | CONF2_OTGPWRDN | > > + CONF2_PHY_GPIOMODE); > > BWT, what's thet GPIOMODE bit for? If GPIO Mode is set to '1' then UART3 Tx/Rx would come on DP and DM pins of USBPHY. > > > + devconf2 |= CONF2_PHY_PLLON | CONF2_DATPOL; > > So, AM35x always uses the reversed data polarity? It's getting set to '1' so its always normal polarity. > > > +static void otg_timer(unsigned long _musb) > > +{ > > + struct musb *musb = (void *)_musb; > > + void __iomem *mregs = musb->mregs; > > + u8 devctl; > > + unsigned long flags; > > + > > + /* > > + * We poll because AM35x's won't expose several OTG-critical > > + * status change events (from the transceiver) otherwise. > > + */ > > + devctl = musb_readb(mregs, MUSB_DEVCTL); > > + DBG(7, "Poll devctl %02x (%s)\n", devctl, otg_state_string(musb)); > > + > > + spin_lock_irqsave(&musb->lock, flags); > > + switch (musb->xceiv->state) { > [...] > > + case OTG_STATE_A_WAIT_VFALL: > > So, are you sure there's no need to call mod_timer() here for RTL 1.8? What issue did you see on earlier RTLs ? As such I didn't see issue In my normal testing. > > > + musb->xceiv->state = OTG_STATE_A_WAIT_VRISE; > > + musb_writel(musb->ctrl_base, CORE_INTR_SRC_SET_REG, > > + MUSB_INTR_VBUSERROR << USB_INTR_USB_SHIFT); > > + break; > > + case OTG_STATE_B_IDLE: > > + if (!is_peripheral_enabled(musb)) > > + break; > > Hm, davinci.c sets DevCtl.Session here and I'm also doing it in > da8xx.c -- > can you comment why you don't do that too? I remember this was causing some issue in OTG testing. > > > + devctl = musb_readb(mregs, MUSB_DEVCTL); > > + if (devctl & MUSB_DEVCTL_BDEVICE) > > + mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ); > > + else > > + musb->xceiv->state = OTG_STATE_A_IDLE; > > + break; > > [...] > > > +static irqreturn_t am35x_interrupt(int irq, void *hci) > > +{ > > + struct musb *musb = hci; > > + void __iomem *reg_base = musb->ctrl_base; > > + unsigned long flags; > > + irqreturn_t ret = IRQ_NONE; > > + u32 epintr, usbintr, lvl_intr; > > + > > + spin_lock_irqsave(&musb->lock, flags); > > + > > + /* Get endpoint interrupts */ > > + epintr = musb_readl(reg_base, EP_INTR_SRC_MASKED_REG); > > + > > + if (epintr) { > > + musb_writel(reg_base, EP_INTR_SRC_CLEAR_REG, epintr); > > + > > + musb->int_rx = > > + (epintr & AM35X_RX_INTR_MASK) >> USB_INTR_RX_SHIFT; > > + musb->int_tx = > > + (epintr & AM35X_TX_INTR_MASK) >> USB_INTR_TX_SHIFT; > > + } > > Perhaps this should be placed after the following check... Hmm, ok. > > > + /* Get usb core interrupts */ > > + usbintr = musb_readl(reg_base, CORE_INTR_SRC_MASKED_REG); > > + if (!usbintr && !epintr) > > + goto eoi; > > + > > + if (usbintr) { > > + musb_writel(reg_base, CORE_INTR_SRC_CLEAR_REG, usbintr); > > + > > + musb->int_usb = > > + (usbintr & USB_INTR_USB_MASK) >> USB_INTR_USB_SHIFT; > > + } > [...] > > + if (usbintr & (USB_INTR_DRVVBUS << USB_INTR_USB_SHIFT)) { > > + int drvvbus = musb_readl(reg_base, USB_STAT_REG); > > + void __iomem *mregs = musb->mregs; > > + u8 devctl = musb_readb(mregs, MUSB_DEVCTL); > > + int err; > [...] > > + } else if (is_host_enabled(musb) && drvvbus) { > > + musb->is_active = 1; > > I dropped this line from da8xx.c as per this change to davinci.c: > > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux- > 2.6.git;a=commit;h=89368d3d11a5b2eff83ad8e752be67f77a372bad Would test on this. > > [...] > > > +int __init musb_platform_init(struct musb *musb, void *board_data) > > +{ > > + void __iomem *reg_base = musb->ctrl_base; > > + struct clk *otg_fck; > > + u32 rev, lvl_intr, sw_reset; > > + > > + musb->mregs += USB_MENTOR_CORE_OFFSET; > > + > > + if (musb->set_clock) > > + musb->set_clock(musb->clock, 1); > > + else > > + clk_enable(musb->clock); > > + DBG(2, "usbotg_ck=%lud\n", clk_get_rate(musb->clock)); > > + > > + otg_fck = clk_get(musb->controller, "fck"); > > Can't this fail? Yup, need to correct. > > > + clk_enable(otg_fck); > > + DBG(2, "usbotg_phy_ck=%lud\n", clk_get_rate(otg_fck)); > > + > > + /* Returns zero if e.g. not clocked */ > > + rev = musb_readl(reg_base, USB_REVISION_REG); > > + if (!rev) > > + return -ENODEV; > > You forgot to disable the clocks you just enabled above. > > > + usb_nop_xceiv_register(); > > + musb->xceiv = otg_get_transceiver(); > > + if (!musb->xceiv) > > + return -ENODEV; > > Ditto. > > > + musb->a_wait_bcon = A_WAIT_BCON_TIMEOUT; > > This field is set by musb_core.c, so I dropped this line from > da8xx.c... Need to test again but I was facing a issue where OTG build image Was either working in host only mode or device only. Role were Not changing based on ID pin status. > > > +int musb_platform_exit(struct musb *musb) > > +{ > > + struct clk *otg_fck = clk_get(musb->controller, "fck"); > > + > > + if (is_host_enabled(musb)) > > + del_timer_sync(&otg_workaround); > > + > > + /* Delay to avoid problems with module reload... */ > > Are you sure this is needed? For DA8xx, I'm not sure: at least in host > mode it just causes pointless delay for me (VBUS comparator is overridden > in > host mode); I was unable to test the OTG mode properly -- for some reason > USB > device didn't get recongnized and VBUS has probably stayed low. I need to check on this, I guess it's not required though I have tested OTG also on AM3517EVM. > > > + if (is_host_enabled(musb) && musb->xceiv->default_a) { > > + u8 devctl, warn = 0; > > + int delay; > > + > > + /* > > + * If there's no peripheral connected, VBUS can take a > > + * long time to fall... > > + */ > > Well, if you have a large capacitor on VBUS... DA8xx seems to have one > (as > I was unable to get the disconnect interrupts in the gadget mode), so > probably > the delay is still needed... don't know about your board. Will test and update. > > > + for (delay = 30; delay > 0; delay--) { > > + devctl = musb_readb(musb->mregs, MUSB_DEVCTL); > > + if (!(devctl & MUSB_DEVCTL_VBUS)) > > + goto done; > > + if ((devctl & MUSB_DEVCTL_VBUS) != warn) { > > + warn = devctl & MUSB_DEVCTL_VBUS; > > + DBG(1, "VBUS %d\n", > > + warn >> MUSB_DEVCTL_VBUS_SHIFT); > > + } > > + msleep(1000); > > + } > > + > > + /* In OTG mode, another host might be connected... */ > > + DBG(1, "VBUS off timeout (devctl %02x)\n", devctl); > > + } > > + if (otg_fck) { > > musb_platfrom_init() suggests that otg_fck can't be NULL. Also, > clk_get > returns error code, not NULL, so should use IS_ERR() here. Correct, would clean it. Thanks, Ajay > > > + clk_put(otg_fck); > > + clk_disable(otg_fck); > > + } > > + > > + return 0; > > +} > > WBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html