Hello.

Ajay Kumar Gupta wrote:

AM35x has musb interface and uses CPPI4.1 DMA engine.
Current patch supports only PIO mode. DMA support can be
added later once basic CPPI4.1 DMA patch is accepted.

Signed-off-by: Ajay Kumar Gupta <[email protected]>

[...]

diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
new file mode 100644
index 0000000..0cdc6bf
--- /dev/null
+++ b/drivers/usb/musb/am35x.c

[...]

+#define USB_SOFT_RESET_MASK    1

   Need a empty line here.

+#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?

+       devconf2 |= CONF2_PHY_PLLON | CONF2_DATPOL;

   So, AM35x always uses the reversed data 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?

+               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?

+               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...

+       /* 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

[...]

+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?

+       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...

+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.

+       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.

+               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.

+               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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to