[PATCH 1/2] musb: add musb support for AM35x

[PATCH 1/2] musb: add musb support for AM35x

[Ajay Kumar Gupta]

AM35x has musb interface and uses CPPI4.1 DMA engine.
Current patch supports only PIO mode and there are on-going
discussions on location of CPPI4.1 DMA.

Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
---
 drivers/usb/musb/Kconfig     |    4 +-
 drivers/usb/musb/Makefile    |    4 +
 drivers/usb/musb/am3517.c    |  536 ++++++++++++++++++++++++++++++++++++++++++
 drivers/usb/musb/musb_core.c |    3 +-
 4 files changed, 544 insertions(+), 3 deletions(-)
 create mode 100644 drivers/usb/musb/am3517.c

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index b4c783c..a29cf84 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -10,7 +10,7 @@ comment "Enable Host or Gadget support to see Inventra options"
 config USB_MUSB_HDRC
 	depends on (USB || USB_GADGET)
 	depends on (ARM || (BF54x && !BF544) || (BF52x && !BF522 && !BF523))
-	select NOP_USB_XCEIV if (ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN)
+	select NOP_USB_XCEIV if (ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN || MACH_OMAP3517EVM)
 	select TWL4030_USB if MACH_OMAP_3430SDP
 	select USB_OTG_UTILS
 	tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
@@ -140,7 +140,7 @@ config USB_MUSB_HDRC_HCD
 config MUSB_PIO_ONLY
 	bool 'Disable DMA (always use PIO)'
 	depends on USB_MUSB_HDRC
-	default y if USB_TUSB6010
+	default USB_TUSB6010 || MACH_OMAP3517EVM
 	help
 	  All data is copied between memory and FIFO by the CPU.
 	  DMA controllers are ignored.
diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile
index 85710cc..9263033 100644
--- a/drivers/usb/musb/Makefile
+++ b/drivers/usb/musb/Makefile
@@ -19,7 +19,11 @@ ifeq ($(CONFIG_ARCH_OMAP2430),y)
 endif
 
 ifeq ($(CONFIG_ARCH_OMAP3430),y)
+   ifeq ($(CONFIG_MACH_OMAP3517EVM),y)
+	musb_hdrc-objs  += am3517.o
+   else
 	musb_hdrc-objs	+= omap2430.o
+   endif
 endif
 
 ifeq ($(CONFIG_BF54x),y)
diff --git a/drivers/usb/musb/am3517.c b/drivers/usb/musb/am3517.c
new file mode 100644
index 0000000..913a294
--- /dev/null
+++ b/drivers/usb/musb/am3517.c
@@ -0,0 +1,536 @@
+/*
+ * Texas Instruments AM3517 "glue layer"
+ *
+ * Copyright (c) 2010, by Texas Instruments
+ *
+ * Based on the DA8xx "glue layer" code.
+ * Copyright (C) 2005-2006 by Texas Instruments
+ * Copyright (c) 2008, MontaVista Software, Inc. <source@mvista.com>
+ *
+ * This file is part of the Inventra Controller Driver for Linux.
+ *
+ * The Inventra Controller Driver for Linux is free software; you
+ * can redistribute it and/or modify it under the terms of the GNU
+ * General Public License version 2 as published by the Free Software
+ * Foundation.
+ *
+ * The Inventra Controller Driver for Linux is distributed in
+ * the hope that it will be useful, but WITHOUT ANY WARRANTY;
+ * without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public
+ * License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with The Inventra Controller Driver for Linux ; if not,
+ * write to the Free Software Foundation, Inc., 59 Temple Place,
+ * Suite 330, Boston, MA  02111-1307  USA
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+
+#include <plat/control.h>
+#include <plat/usb.h>
+
+#include "musb_core.h"
+
+/*
+ * AM3517 specific definitions
+ */
+
+/* CPPI 4.1 queue manager registers */
+#define QMGR_PEND0_REG		0x4090
+#define QMGR_PEND1_REG		0x4094
+#define QMGR_PEND2_REG		0x4098
+
+/* USB 2.0 PHY Control */
+#define CONF2_PHY_GPIOMODE     (1 << 23)
+#define CONF2_OTGMODE          (3 << 14)
+#define CONF2_SESENDEN         (1 << 13)
+#define CONF2_VBDTCTEN         (1 << 12)
+#define CONF2_REFFREQ_24MHZ    (2 << 8)
+#define CONF2_REFFREQ_26MHZ    (7 << 8)
+#define CONF2_REFFREQ_13MHZ    (6 << 8)
+#define CONF2_REFFREQ          (0xf << 8)
+#define CONF2_PHYCLKGD         (1 << 7)
+#define CONF2_VBUSSENSE        (1 << 6)
+#define CONF2_PHY_PLLON        (1 << 5)
+#define CONF2_RESET            (1 << 4)
+#define CONF2_PHYPWRDN         (1 << 3)
+#define CONF2_OTGPWRDN         (1 << 2)
+#define CONF2_DATPOL           (1 << 1)
+
+
+#define AM3517_TX_EP_MASK	0xffff		/* EP0 + 15 Tx EPs */
+#define AM3517_RX_EP_MASK	0xfffe		/* 15 Rx EPs */
+
+#define AM3517_TX_INTR_MASK	(AM3517_TX_EP_MASK << USB_INTR_TX_SHIFT)
+#define AM3517_RX_INTR_MASK	(AM3517_RX_EP_MASK << USB_INTR_RX_SHIFT)
+
+#define A_WAIT_BCON_TIMEOUT	1100	/* in ms */
+
+static inline void phy_on(void)
+{
+	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_OTGMODE | CONF2_REFFREQ | CONF2_PHY_GPIOMODE);
+	devconf2 |= CONF2_SESENDEN | CONF2_VBDTCTEN | CONF2_PHY_PLLON |
+		    CONF2_REFFREQ_13MHZ | CONF2_DATPOL;
+
+	omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2);
+
+	DBG(1, "Waiting for PHY clock good...\n");
+	while (!(omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2)
+			& CONF2_PHYCLKGD))
+		cpu_relax();
+}
+
+static inline void phy_off(void)
+{
+	u32 devconf2;
+
+	/*
+	 * Power down the on-chip PHY.
+	 */
+	devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2);
+
+	devconf2 &= ~CONF2_PHY_PLLON;
+	devconf2 |=  CONF2_PHYPWRDN | CONF2_OTGPWRDN;
+	omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2);
+}
+
+/**
+ * musb_platform_enable - enable interrupts
+ */
+void musb_platform_enable(struct musb *musb)
+{
+	void __iomem *reg_base = musb->ctrl_base;
+	u32 epmask, coremask;
+
+	/* Workaround: setup IRQs through both register sets. */
+	epmask = ((musb->epmask & AM3517_TX_EP_MASK) << USB_INTR_TX_SHIFT) |
+	       ((musb->epmask & AM3517_RX_EP_MASK) << USB_INTR_RX_SHIFT);
+	coremask = (0x01ff << USB_INTR_USB_SHIFT);
+
+	musb_writel(reg_base, EP_INTR_MASK_SET_REG, epmask);
+	musb_writel(reg_base, CORE_INTR_MASK_SET_REG, coremask);
+
+	/* Force the DRVVBUS IRQ so we can start polling for ID change. */
+	if (is_otg_enabled(musb))
+		musb_writel(reg_base, CORE_INTR_SRC_SET_REG,
+			    USB_INTR_DRVVBUS << USB_INTR_USB_SHIFT);
+}
+
+/**
+ * musb_platform_disable - disable HDRC and flush interrupts
+ */
+void musb_platform_disable(struct musb *musb)
+{
+	void __iomem *reg_base = musb->ctrl_base;
+
+	musb_writel(reg_base, CORE_INTR_MASK_CLEAR_REG, USB_INTR_USB_MASK);
+	musb_writel(reg_base, EP_INTR_MASK_CLEAR_REG,
+			 AM3517_TX_INTR_MASK | AM3517_RX_INTR_MASK);
+	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
+	musb_writel(reg_base, USB_END_OF_INTR_REG, 0);
+}
+
+static int vbus_state = -1;
+
+#ifdef CONFIG_USB_MUSB_HDRC_HCD
+#define portstate(stmt) 	stmt
+#else
+#define portstate(stmt)
+#endif
+
+static void am3517_source_power(struct musb *musb, int is_on, int immediate)
+{
+	if (is_on)
+		is_on = 1;
+
+	if (vbus_state == is_on)
+		return;
+	vbus_state = is_on;
+}
+
+static void am3517_set_vbus(struct musb *musb, int is_on)
+{
+	WARN_ON(is_on && is_peripheral_active(musb));
+	am3517_source_power(musb, is_on, 0);
+}
+
+#define	POLL_SECONDS	2
+
+static struct timer_list otg_workaround;
+
+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 AM3517'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_BCON:
+		devctl &= ~MUSB_DEVCTL_SESSION;
+		musb_writeb(musb->mregs, MUSB_DEVCTL, devctl);
+
+		devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
+		if (devctl & MUSB_DEVCTL_BDEVICE) {
+			musb->xceiv->state = OTG_STATE_B_IDLE;
+			MUSB_DEV_MODE(musb);
+		} else {
+			musb->xceiv->state = OTG_STATE_A_IDLE;
+			MUSB_HST_MODE(musb);
+		}
+		break;
+	case OTG_STATE_A_WAIT_VFALL:
+		/*
+		 * Wait till VBUS falls below SessionEnd (~0.2 V); the 1.3
+		 * RTL seems to mis-handle session "start" otherwise (or in
+		 * our case "recover"), in routine "VBUS was valid by the time
+		 * VBUSERR got reported during enumeration" cases.
+		 */
+		if (devctl & MUSB_DEVCTL_VBUS) {
+			mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+			break;
+		}
+		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;
+
+		/*
+		 * There's no ID-changed IRQ, so we have no good way to tell
+		 * when to switch to the A-Default state machine (by setting
+		 * the DEVCTL.SESSION flag).
+		 *
+		 * Workaround:  whenever we're in B_IDLE, try setting the
+		 * session flag every few seconds.  If it works, ID was
+		 * grounded and we're now in the A-Default state machine.
+		 *
+		 * NOTE: setting the session flag is _supposed_ to trigger
+		 * SRP but clearly it doesn't.
+		 */
+		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;
+	default:
+		break;
+	}
+	spin_unlock_irqrestore(&musb->lock, flags);
+}
+
+void musb_platform_try_idle(struct musb *musb, unsigned long timeout)
+{
+	static unsigned long last_timer;
+
+	if (!is_otg_enabled(musb))
+		return;
+
+	if (timeout == 0)
+		timeout = jiffies + msecs_to_jiffies(3);
+
+	/* Never idle if active, or when VBUS timeout is not set as host */
+	if (musb->is_active || (musb->a_wait_bcon == 0 &&
+				musb->xceiv->state == OTG_STATE_A_WAIT_BCON)) {
+		DBG(4, "%s active, deleting timer\n", otg_state_string(musb));
+		del_timer(&otg_workaround);
+		last_timer = jiffies;
+		return;
+	}
+
+	if (time_after(last_timer, timeout) && timer_pending(&otg_workaround)) {
+		DBG(4, "Longer idle timer already pending, ignoring...\n");
+		return;
+	}
+	last_timer = timeout;
+
+	DBG(4, "%s inactive, starting idle timer for %u ms\n",
+	    otg_state_string(musb), jiffies_to_msecs(timeout - jiffies));
+	mod_timer(&otg_workaround, timeout);
+}
+
+static irqreturn_t am3517_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 & AM3517_RX_INTR_MASK) >> USB_INTR_RX_SHIFT;
+		musb->int_tx =
+			(epintr & AM3517_TX_INTR_MASK) >> USB_INTR_TX_SHIFT;
+	}
+
+	/* 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;
+	}
+	/*
+	 * DRVVBUS IRQs are the only proxy we have (a very poor one!) for
+	 * AM3517's missing ID change IRQ.  We need an ID change IRQ to
+	 * switch appropriately between halves of the OTG state machine.
+	 * Managing DEVCTL.SESSION per Mentor docs requires that we know its
+	 * value but DEVCTL.BDEVICE is invalid without DEVCTL.SESSION set.
+	 * Also, DRVVBUS pulses for SRP (but not at 5V) ...
+	 */
+	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;
+
+		err = is_host_enabled(musb) && (musb->int_usb &
+						MUSB_INTR_VBUSERROR);
+		if (err) {
+			/*
+			 * The Mentor core doesn't debounce VBUS as needed
+			 * to cope with device connect current spikes. This
+			 * means it's not uncommon for bus-powered devices
+			 * to get VBUS errors during enumeration.
+			 *
+			 * This is a workaround, but newer RTL from Mentor
+			 * seems to allow a better one: "re"-starting sessions
+			 * without waiting for VBUS to stop registering in
+			 * devctl.
+			 */
+			musb->int_usb &= ~MUSB_INTR_VBUSERROR;
+			musb->xceiv->state = OTG_STATE_A_WAIT_VFALL;
+			mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+			WARNING("VBUS error workaround (delay coming)\n");
+		} else if (is_host_enabled(musb) && drvvbus) {
+			musb->is_active = 1;
+			MUSB_HST_MODE(musb);
+			musb->xceiv->default_a = 1;
+			musb->xceiv->state = OTG_STATE_A_WAIT_VRISE;
+			portstate(musb->port1_status |= USB_PORT_STAT_POWER);
+			del_timer(&otg_workaround);
+		} else {
+			musb->is_active = 0;
+			MUSB_DEV_MODE(musb);
+			musb->xceiv->default_a = 0;
+			musb->xceiv->state = OTG_STATE_B_IDLE;
+			portstate(musb->port1_status &= ~USB_PORT_STAT_POWER);
+		}
+
+		/* NOTE: this must complete power-on within 100 ms. */
+		am3517_source_power(musb, drvvbus, 0);
+		DBG(2, "VBUS %s (%s)%s, devctl %02x\n",
+				drvvbus ? "on" : "off",
+				otg_state_string(musb),
+				err ? " ERROR" : "",
+				devctl);
+		ret = IRQ_HANDLED;
+	}
+
+	if (musb->int_tx || musb->int_rx || musb->int_usb) {
+		irqreturn_t mret;
+
+		mret = musb_interrupt(musb);
+		if (mret == IRQ_HANDLED)
+			ret = IRQ_HANDLED;
+	}
+
+ eoi:
+	/* EOI needs to be written for the IRQ to be re-asserted. */
+	if (ret == IRQ_HANDLED || epintr || usbintr) {
+		/* clear level interrupt */
+		lvl_intr = omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR);
+		lvl_intr |= AM35XX_USBOTGSS_INT_CLR;
+		omap_ctrl_writel(lvl_intr, AM35XX_CONTROL_LVL_INTR_CLEAR);
+		/* write EOI */
+		musb_writel(reg_base, USB_END_OF_INTR_REG, 0);
+	}
+
+	/* Poll for ID change */
+	if (is_otg_enabled(musb) && musb->xceiv->state == OTG_STATE_B_IDLE)
+		mod_timer(&otg_workaround, jiffies + POLL_SECONDS * HZ);
+
+	spin_unlock_irqrestore(&musb->lock, flags);
+
+	if (ret != IRQ_HANDLED) {
+		if (epintr || usbintr)
+			/*
+			 * We sometimes get unhandled IRQs in the peripheral
+			 * mode from EP0 and SOF...
+			 */
+			DBG(2, "Unhandled USB IRQ %08x-%08x\n",
+					 epintr, usbintr);
+		else if (printk_ratelimit())
+			/*
+			 * We've seen series of spurious interrupts in the
+			 * peripheral mode after USB reset and then after some
+			 * time a real interrupt storm starting...
+			 */
+			DBG(2, "Spurious IRQ\n");
+	}
+	return ret;
+}
+
+int musb_platform_set_mode(struct musb *musb, u8 musb_mode)
+{
+	WARNING("FIXME: %s not implemented\n", __func__);
+	return -EIO;
+}
+
+int __init musb_platform_init(struct musb *musb)
+{
+	void __iomem *reg_base = musb->ctrl_base;
+	struct clk              *otg_fck;
+	u32 rev, lvl_intr, sw_reset;
+
+	usb_nop_xceiv_register();
+
+	musb->xceiv = otg_get_transceiver();
+	if (!musb->xceiv)
+		return -ENODEV;
+
+	/* Mentor is at offset of 0x400 in AM3517 */
+	musb->mregs += USB_MENTOR_CORE_OFFSET;
+
+	/* musb->clock is already set from board/arch files */
+	if (IS_ERR(musb->clock))
+		return PTR_ERR(musb->clock);
+
+	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");
+	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;
+
+	if (is_host_enabled(musb))
+		setup_timer(&otg_workaround, otg_timer, (unsigned long) musb);
+
+	musb->board_set_vbus = am3517_set_vbus;
+	am3517_source_power(musb, 0, 1);
+
+	/* global reset */
+	sw_reset = omap_ctrl_readl(AM35XX_CONTROL_IP_SW_RESET);
+
+	sw_reset |= AM35XX_USBOTGSS_SW_RST;
+	omap_ctrl_writel(sw_reset, AM35XX_CONTROL_IP_SW_RESET);
+
+	sw_reset &= ~AM35XX_USBOTGSS_SW_RST;
+	omap_ctrl_writel(sw_reset, AM35XX_CONTROL_IP_SW_RESET);
+
+	/* Reset the controller */
+	musb_writel(reg_base, USB_CTRL_REG, USB_SOFT_RESET_MASK);
+
+	/* Start the on-chip PHY and its PLL. */
+	phy_on();
+
+	msleep(15);
+
+	musb->a_wait_bcon = A_WAIT_BCON_TIMEOUT;
+	musb->isr = am3517_interrupt;
+
+	/* clear level interrupt */
+	lvl_intr = omap_ctrl_readl(AM35XX_CONTROL_LVL_INTR_CLEAR);
+	lvl_intr |= AM35XX_USBOTGSS_INT_CLR;
+	omap_ctrl_writel(lvl_intr, AM35XX_CONTROL_LVL_INTR_CLEAR);
+
+	return 0;
+}
+
+int musb_platform_exit(struct musb *musb)
+{
+	if (is_host_enabled(musb))
+		del_timer_sync(&otg_workaround);
+
+	am3517_source_power(musb, 0 /* off */, 1);
+
+	/* Delay to avoid problems with module reload... */
+	if (is_host_enabled(musb) && musb->xceiv->default_a) {
+		int maxdelay = 30;
+		u8 devctl, warn = 0;
+
+		/*
+		 * If there's no peripheral connected, this can take a
+		 * long time to fall...
+		 */
+		do {
+			devctl = musb_readb(musb->mregs, MUSB_DEVCTL);
+			if (!(devctl & MUSB_DEVCTL_VBUS))
+				break;
+			if ((devctl & MUSB_DEVCTL_VBUS) != warn) {
+				warn = devctl & MUSB_DEVCTL_VBUS;
+				DBG(1, "VBUS %d\n",
+					warn >> MUSB_DEVCTL_VBUS_SHIFT);
+			}
+			msleep(1000);
+			maxdelay--;
+		} while (maxdelay > 0);
+
+		/* In OTG mode, another host might be connected... */
+		if (devctl & MUSB_DEVCTL_VBUS)
+			DBG(1, "VBUS off timeout (devctl %02x)\n", devctl);
+	}
+
+	phy_off();
+
+	usb_nop_xceiv_unregister();
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+void musb_platform_save_context(struct musb_context_registers
+		*musb_context)
+{
+	phy_off();
+}
+
+void musb_platform_restore_context(struct musb_context_registers
+		*musb_context)
+{
+	phy_on();
+}
+#endif
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 98fd5b6..f8efe00 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -982,7 +982,8 @@ static void musb_shutdown(struct platform_device *pdev)
  * more than selecting one of a bunch of predefined configurations.
  */
 #if defined(CONFIG_USB_TUSB6010) || \
-	defined(CONFIG_ARCH_OMAP2430) || defined(CONFIG_ARCH_OMAP3)
+	defined(CONFIG_ARCH_OMAP2430) || defined(CONFIG_ARCH_OMAP3) || \
+	defined(CONFIG_MACH_OMAP3517EVM)
 static ushort __initdata fifo_mode = 4;
 #else
 static ushort __initdata fifo_mode = 2;
-- 
1.6.2.4

[PATCH 2/2] AM35x: musb: Workaround for fifo read issue

[Ajay Kumar Gupta]

AM35x supports only 32bit read operations so we need to have
workaround for 8bit and 16bit read operations.

Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
---
 drivers/usb/musb/am3517.c    |   30 ++++++++++++++++++++++++++++++
 drivers/usb/musb/musb_core.c |    2 ++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/musb/am3517.c b/drivers/usb/musb/am3517.c
index 913a294..1326089 100644
--- a/drivers/usb/musb/am3517.c
+++ b/drivers/usb/musb/am3517.c
@@ -534,3 +534,33 @@ void musb_platform_restore_context(struct musb_context_registers
 	phy_on();
 }
 #endif
+
+/* AM35x supports only 32bit read operation */
+void musb_read_fifo(struct musb_hw_ep *hw_ep, u16 len, u8 *dst)
+{
+	void __iomem *fifo = hw_ep->fifo;
+	u32		val;
+	int		i;
+
+	/* Read for 32bit-aligned destination address */
+	if ((likely((0x03 & (unsigned long) dst) == 0)) && len >= 4) {
+		readsl(fifo, dst, len >> 2);
+		dst += (len & ~0x03);
+		len &= 0x03;
+	}
+	/* Now read the rest 1 to 3 bytes or complete length if
+	 * unaligned address.
+	 */
+	if (len > 4) {
+		for (i = 0; i < (len >> 2); i++) {
+			val = musb_readl(fifo, 0);
+			memcpy(dst, &val, 4);
+			dst += 4;
+		}
+		len %= 4;
+	}
+	if (len > 0) {
+		val = musb_readl(fifo, 0);
+		memcpy(dst, &val, len);
+	}
+}
diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index f8efe00..2c1b3ab 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -191,6 +191,7 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 len, const u8 *src)
 	}
 }
 
+#if !defined(CONFIG_MACH_OMAP3517EVM)
 /*
  * Unload an endpoint's FIFO
  */
@@ -228,6 +229,7 @@ void musb_read_fifo(struct musb_hw_ep *hw_ep, u16 len, u8 *dst)
 		readsb(fifo, dst, len);
 	}
 }
+#endif
 
 #endif	/* normal PIO */
 
-- 
1.6.2.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] musb: add musb support for AM35x

[Sergei Shtylyov]

Hello.

Ajay Kumar Gupta wrote:

> AM35x has musb interface and uses CPPI4.1 DMA engine.
> Current patch supports only PIO mode and there are on-going
> discussions on location of CPPI4.1 DMA.
>
> Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
> ---
>  drivers/usb/musb/Kconfig     |    4 +-
>  drivers/usb/musb/Makefile    |    4 +
>  drivers/usb/musb/am3517.c    |  536 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/musb/musb_core.c |    3 +-
>  4 files changed, 544 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/usb/musb/am3517.c
>
> diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
> index b4c783c..a29cf84 100644
> --- a/drivers/usb/musb/Kconfig
> +++ b/drivers/usb/musb/Kconfig
> @@ -10,7 +10,7 @@ comment "Enable Host or Gadget support to see Inventra options"
>  config USB_MUSB_HDRC
>  	depends on (USB || USB_GADGET)
>  	depends on (ARM || (BF54x && !BF544) || (BF52x && !BF522 && !BF523))
> -	select NOP_USB_XCEIV if (ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN)
> +	select NOP_USB_XCEIV if (ARCH_DAVINCI || MACH_OMAP3EVM || BLACKFIN || MACH_OMAP3517EVM)
>  	select TWL4030_USB if MACH_OMAP_3430SDP
>  	select USB_OTG_UTILS
>  	tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
> @@ -140,7 +140,7 @@ config USB_MUSB_HDRC_HCD
>  config MUSB_PIO_ONLY
>  	bool 'Disable DMA (always use PIO)'
>  	depends on USB_MUSB_HDRC
> -	default y if USB_TUSB6010
> +	default USB_TUSB6010 || MACH_OMAP3517EVM
>  	help
>  	  All data is copied between memory and FIFO by the CPU.
>  	  DMA controllers are ignored.
> diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile
> index 85710cc..9263033 100644
> --- a/drivers/usb/musb/Makefile
> +++ b/drivers/usb/musb/Makefile
> @@ -19,7 +19,11 @@ ifeq ($(CONFIG_ARCH_OMAP2430),y)
>  endif
>  
>  ifeq ($(CONFIG_ARCH_OMAP3430),y)
> +   ifeq ($(CONFIG_MACH_OMAP3517EVM),y)
> +	musb_hdrc-objs  += am3517.o
>   

   Isn't there some ARCH-level option for AM3517 SoC? Depending on the 
board type doesn't really scale well...

> +   else
>  	musb_hdrc-objs	+= omap2430.o
> +   endif
>  endif
>  
>  ifeq ($(CONFIG_BF54x),y)
> diff --git a/drivers/usb/musb/am3517.c b/drivers/usb/musb/am3517.c
> new file mode 100644
> index 0000000..913a294
> --- /dev/null
> +++ b/drivers/usb/musb/am3517.c
> @@ -0,0 +1,536 @@
>   
[...]
> +/*
> + * AM3517 specific definitions
> + */
> +
> +/* CPPI 4.1 queue manager registers */
> +#define QMGR_PEND0_REG		0x4090
> +#define QMGR_PEND1_REG		0x4094
> +#define QMGR_PEND2_REG		0x4098
>   

   Those are not used (yet)...

> +static inline void phy_on(void)
> +{
> +	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_OTGMODE | CONF2_REFFREQ | CONF2_PHY_GPIOMODE);
>   

   Shouldn't you manipulate CONF2_OTGMODE in the board code instead? I 
suspect value of 0 doesn't fit the host-only configuration (without 
cable connected, MUSB will think it's a B-device, and the driver will 
fail to initialize IIRC).

> +	devconf2 |= CONF2_SESENDEN | CONF2_VBDTCTEN | CONF2_PHY_PLLON |
> +		    CONF2_REFFREQ_13MHZ | CONF2_DATPOL;
>   

   Reference clock of 13 MHz, not 12? Hmm... Again, shouldn't the board 
code select the reference frequency (clock might be external, IIUC)?

> +static inline void phy_off(void)
> +{
> +	u32 devconf2;
> +
> +	/*
> +	 * Power down the on-chip PHY.
> +	 */
> +	devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2);
> +
> +	devconf2 &= ~CONF2_PHY_PLLON;
> +	devconf2 |=  CONF2_PHYPWRDN | CONF2_OTGPWRDN;
> +	omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2);
> +}
> +
> +/**
> + * musb_platform_enable - enable interrupts
> + */
> +void musb_platform_enable(struct musb *musb)
> +{
> +	void __iomem *reg_base = musb->ctrl_base;
> +	u32 epmask, coremask;
> +
> +	/* Workaround: setup IRQs through both register sets. */
> +	epmask = ((musb->epmask & AM3517_TX_EP_MASK) << USB_INTR_TX_SHIFT) |
> +	       ((musb->epmask & AM3517_RX_EP_MASK) << USB_INTR_RX_SHIFT);
> +	coremask = (0x01ff << USB_INTR_USB_SHIFT);
> +
> +	musb_writel(reg_base, EP_INTR_MASK_SET_REG, epmask);
> +	musb_writel(reg_base, CORE_INTR_MASK_SET_REG, coremask);
>   

   Hm, and I thought all CPPI 4.1 based controllers have the same 
register layout... alas, I was wrong.

> +static int vbus_state = -1;
>   
[...]
> +static void am3517_source_power(struct musb *musb, int is_on, int immediate)
> +{
> +	if (is_on)
> +		is_on = 1;
> +
> +	if (vbus_state == is_on)
> +		return;
> +	vbus_state = is_on;
> +}
> +
>   

   Without the real GPIOs to manipulate, I don't understand the purpose 
of this function...

> +static struct timer_list otg_workaround;
> +
> +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 AM3517's won't expose several OTG-critical
> +	* status change events (from the transceiver) otherwise.
>   

   Need one more space before *...

> +	case OTG_STATE_A_WAIT_VFALL:
> +		/*
> +		 * Wait till VBUS falls below SessionEnd (~0.2 V); the 1.3
> +		 * RTL seems to mis-handle session "start" otherwise (or in
> +		 * our case "recover"), in routine "VBUS was valid by the time
> +		 * VBUSERR got reported during enumeration" cases.
> +		 */
>   

   I wonder if all this still true for RTL 1.8 on which DA8xx (and 
probably AM3517) MUSB is based...

> +void musb_platform_try_idle(struct musb *musb, unsigned long timeout)
> +{
>   

   I wonder how DaVinci gets about without musb_platfrom_try_idle()...

> +static irqreturn_t am3517_interrupt(int irq, void *hci)
> +{

> +		/* NOTE: this must complete power-on within 100 ms. */
> +		am3517_source_power(musb, drvvbus, 0);
>   

   This call is effectively a NOP.

> +	if (musb->int_tx || musb->int_rx || musb->int_usb) {
> +		irqreturn_t mret;
> +
> +		mret = musb_interrupt(musb);
> +		if (mret == IRQ_HANDLED)
> +			ret = IRQ_HANDLED;
>   

   Not sure why you didn't like ret |= musb_interrupt(musb)...

> +	if (ret != IRQ_HANDLED) {
> +		if (epintr || usbintr)
> +			/*
> +			 * We sometimes get unhandled IRQs in the peripheral
> +			 * mode from EP0 and SOF...
> +			 */
> +			DBG(2, "Unhandled USB IRQ %08x-%08x\n",
> +					 epintr, usbintr);
>   

   This check shouldn't be needed any more -- EP0 spurious interrupts 
have been all chased down...

> +		else if (printk_ratelimit())
> +			/*
> +			 * We've seen series of spurious interrupts in the
> +			 * peripheral mode after USB reset and then after some
> +			 * time a real interrupt storm starting...
> +			 */
> +			DBG(2, "Spurious IRQ\n");
>   

   Those turned out to be interrupts caused by CPPI 4.1 descriptor 
starvation (for which the controller didn't even have an interrupt bit). 
I still haven't dealt with them...

> +	}
> +	return ret;
> +}
> +
> +int musb_platform_set_mode(struct musb *musb, u8 musb_mode)
> +{
> +	WARNING("FIXME: %s not implemented\n", __func__);
> +	return -EIO;
>   

   Could be implemented using CONF2_OTGMODE...

> +int __init musb_platform_init(struct musb *musb)
> +{
> +	void __iomem *reg_base = musb->ctrl_base;
> +	struct clk              *otg_fck;
> +	u32 rev, lvl_intr, sw_reset;
> +
> +	usb_nop_xceiv_register();
> +
> +	musb->xceiv = otg_get_transceiver();
> +	if (!musb->xceiv)
> +		return -ENODEV;
> +
> +	/* Mentor is at offset of 0x400 in AM3517 */
> +	musb->mregs += USB_MENTOR_CORE_OFFSET;
> +
> +	/* musb->clock is already set from board/arch files */
> +	if (IS_ERR(musb->clock))
> +		return PTR_ERR(musb->clock);
>   

  This is checked by musb_core.c now, no need to duplicate...

> +	if (musb->set_clock)
> +		musb->set_clock(musb->clock, 1);
>   

   musb->set_clock() is about to be dropped...

> +	else
> +		clk_enable(musb->clock);
> +
> +	DBG(2, "usbotg_ck=%lud\n", clk_get_rate(musb->clock));
>   

   I'd put an empty line after that DBG() rather than before.

> +	otg_fck = clk_get(musb->controller, "fck");
> +	clk_enable(otg_fck);
>   

   Oh, it needs two clocks... Another argument against Felipe dropping 
plat->clock. :-)

> +
> +	DBG(2, "usbotg_phy_ck=%lud\n", clk_get_rate(otg_fck));
>   

   Same about empty line here...

> +	am3517_source_power(musb, 0, 1);
>   

   Effective NOP...

> +	/* Start the on-chip PHY and its PLL. */
> +	phy_on();
> +
> +	msleep(15);
>   

   5 ms is not enough?

> +	musb->a_wait_bcon = A_WAIT_BCON_TIMEOUT;
>   

   Hm... that line kept causing a stream of kernel messages for me, 
until I removed it. Doesn't it for you?

> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 98fd5b6..f8efe00 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -982,7 +982,8 @@ static void musb_shutdown(struct platform_device *pdev)
>   * more than selecting one of a bunch of predefined configurations.
>   */
>  #if defined(CONFIG_USB_TUSB6010) || \
> -	defined(CONFIG_ARCH_OMAP2430) || defined(CONFIG_ARCH_OMAP3)
> +	defined(CONFIG_ARCH_OMAP2430) || defined(CONFIG_ARCH_OMAP3) || \
> +	defined(CONFIG_MACH_OMAP3517EVM)
>   

   Isn't that dependency already covered by CONFIG_ARCH_OMAP3? Or else I 
don't see you adding your #ifdef around musb_platfrom_try_idle() 
declaration...

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] musb: add musb support for AM35x

[Gupta, Ajay Kumar]

Hi,
> > AM35x has musb interface and uses CPPI4.1 DMA engine.
> > Current patch supports only PIO mode and there are on-going
> > discussions on location of CPPI4.1 DMA.
> >
> > Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
> >  	tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
> > @@ -140,7 +140,7 @@ config USB_MUSB_HDRC_HCD
> >  config MUSB_PIO_ONLY
> >  	bool 'Disable DMA (always use PIO)'
> >  	depends on USB_MUSB_HDRC
> > -	default y if USB_TUSB6010
> > +	default USB_TUSB6010 || MACH_OMAP3517EVM
> >  	help
> >  	  All data is copied between memory and FIFO by the CPU.
> >  	  DMA controllers are ignored.
> > diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile
> > index 85710cc..9263033 100644
> > --- a/drivers/usb/musb/Makefile
> > +++ b/drivers/usb/musb/Makefile
> > @@ -19,7 +19,11 @@ ifeq ($(CONFIG_ARCH_OMAP2430),y)
> >  endif
> >
> >  ifeq ($(CONFIG_ARCH_OMAP3430),y)
> > +   ifeq ($(CONFIG_MACH_OMAP3517EVM),y)
> > +	musb_hdrc-objs  += am3517.o
> >
> 
>    Isn't there some ARCH-level option for AM3517 SoC? Depending on the
> board type doesn't really scale well...

AM3517 is actually based on OMAP3 but musb is different. Unfortunately there
is no seperate *_ARCH_* defines for AM3517 alone.

> > + * AM3517 specific definitions
> > + */
> > +
> > +/* CPPI 4.1 queue manager registers */
> > +#define QMGR_PEND0_REG		0x4090
> > +#define QMGR_PEND1_REG		0x4094
> > +#define QMGR_PEND2_REG		0x4098
> >
> 
>    Those are not used (yet)...

Yes, but I added them as they are part of register layout.

> 
> > +static inline void phy_on(void)
> > +{
> > +	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_OTGMODE | CONF2_REFFREQ | CONF2_PHY_GPIOMODE);
> >
> 
>    Shouldn't you manipulate CONF2_OTGMODE in the board code instead? I
> suspect value of 0 doesn't fit the host-only configuration (without
> cable connected, MUSB will think it's a B-device, and the driver will
> fail to initialize IIRC).

I didn't see any issue in Host/Device only and OTG mode configurations
On AM3517? Did you see any issue on DA8xx?

> 
> > +	devconf2 |= CONF2_SESENDEN | CONF2_VBDTCTEN | CONF2_PHY_PLLON |
> > +		    CONF2_REFFREQ_13MHZ | CONF2_DATPOL;
> >
> 
>    Reference clock of 13 MHz, not 12? Hmm... Again, shouldn't the board
> code select the reference frequency (clock might be external, IIUC)?

Yeah, it's 13Mhz.Clock setting from board would be good to have though
Currently there is only one board AM3517EVM.
  
> > +/**
> > + * musb_platform_enable - enable interrupts
> > + */
> > +void musb_platform_enable(struct musb *musb)
> > +{
> > +	void __iomem *reg_base = musb->ctrl_base;
> > +	u32 epmask, coremask;
> > +
> > +	/* Workaround: setup IRQs through both register sets. */
> > +	epmask = ((musb->epmask & AM3517_TX_EP_MASK) << USB_INTR_TX_SHIFT) |
> > +	       ((musb->epmask & AM3517_RX_EP_MASK) << USB_INTR_RX_SHIFT);
> > +	coremask = (0x01ff << USB_INTR_USB_SHIFT);
> > +
> > +	musb_writel(reg_base, EP_INTR_MASK_SET_REG, epmask);
> > +	musb_writel(reg_base, CORE_INTR_MASK_SET_REG, coremask);
> >
> 
>    Hm, and I thought all CPPI 4.1 based controllers have the same
> register layout... alas, I was wrong.

There are changes as AM3517 supports 15Rx/Tx endpoints.

> 
> > +static int vbus_state = -1;
> >
> [...]
> > +static void am3517_source_power(struct musb *musb, int is_on, int
> immediate)
> > +{
> > +	if (is_on)
> > +		is_on = 1;
> > +
> > +	if (vbus_state == is_on)
> > +		return;
> > +	vbus_state = is_on;
> > +}
> > +
> >
> 
>    Without the real GPIOs to manipulate, I don't understand the purpose
> of this function...

Correct, this function can be removed.

> 
> > +static struct timer_list otg_workaround;
> > +
> > +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 AM3517's won't expose several OTG-critical
> > +	* status change events (from the transceiver) otherwise.
> >
> 
>    Need one more space before *...

Right.

> 
> > +	case OTG_STATE_A_WAIT_VFALL:
> > +		/*
> > +		 * Wait till VBUS falls below SessionEnd (~0.2 V); the 1.3
> > +		 * RTL seems to mis-handle session "start" otherwise (or in
> > +		 * our case "recover"), in routine "VBUS was valid by the time
> > +		 * VBUSERR got reported during enumeration" cases.
> > +		 */
> >
> 
>    I wonder if all this still true for RTL 1.8 on which DA8xx (and
> probably AM3517) MUSB is based...

Need to check on this..though I didn't see any issue in my testing.

> 
> > +void musb_platform_try_idle(struct musb *musb, unsigned long timeout)
> > +{
> >
> 
>    I wonder how DaVinci gets about without musb_platfrom_try_idle()...

Hmm..
> 
> > +static irqreturn_t am3517_interrupt(int irq, void *hci)
> > +{
> 
> > +		/* NOTE: this must complete power-on within 100 ms. */
> > +		am3517_source_power(musb, drvvbus, 0);
> >
> 
>    This call is effectively a NOP.

Yes and so should be removed.

> 
> > +	if (musb->int_tx || musb->int_rx || musb->int_usb) {
> > +		irqreturn_t mret;
> > +
> > +		mret = musb_interrupt(musb);
> > +		if (mret == IRQ_HANDLED)
> > +			ret = IRQ_HANDLED;
> >
> 
>    Not sure why you didn't like ret |= musb_interrupt(musb)...

Need to verify/clean it. I remember, was getting some issue initially
On this path.

> 
> > +	if (ret != IRQ_HANDLED) {
> > +		if (epintr || usbintr)
> > +			/*
> > +			 * We sometimes get unhandled IRQs in the peripheral
> > +			 * mode from EP0 and SOF...
> > +			 */
> > +			DBG(2, "Unhandled USB IRQ %08x-%08x\n",
> > +					 epintr, usbintr);
> >
> 
>    This check shouldn't be needed any more -- EP0 spurious interrupts
> have been all chased down...

Ok fine.
> 
> > +		else if (printk_ratelimit())
> > +			/*
> > +			 * We've seen series of spurious interrupts in the
> > +			 * peripheral mode after USB reset and then after some
> > +			 * time a real interrupt storm starting...
> > +			 */
> > +			DBG(2, "Spurious IRQ\n");
> >
> 
>    Those turned out to be interrupts caused by CPPI 4.1 descriptor
> starvation (for which the controller didn't even have an interrupt bit).
> I still haven't dealt with them...

and.. AM3517 has got a bit to disable them, anyways this patch currently
supports PIO only so this check can be removed also.

> 
> > +	}
> > +	return ret;
> > +}
> > +
> > +int musb_platform_set_mode(struct musb *musb, u8 musb_mode)
> > +{
> > +	WARNING("FIXME: %s not implemented\n", __func__);
> > +	return -EIO;
> >
> 
>    Could be implemented using CONF2_OTGMODE...

Ok fine.

> 
> > +int __init musb_platform_init(struct musb *musb)
> > +{
> > +	void __iomem *reg_base = musb->ctrl_base;
> > +	struct clk              *otg_fck;
> > +	u32 rev, lvl_intr, sw_reset;
> > +
> > +	usb_nop_xceiv_register();
> > +
> > +	musb->xceiv = otg_get_transceiver();
> > +	if (!musb->xceiv)
> > +		return -ENODEV;
> > +
> > +	/* Mentor is at offset of 0x400 in AM3517 */
> > +	musb->mregs += USB_MENTOR_CORE_OFFSET;
> > +
> > +	/* musb->clock is already set from board/arch files */
> > +	if (IS_ERR(musb->clock))
> > +		return PTR_ERR(musb->clock);
> >
> 
>   This is checked by musb_core.c now, no need to duplicate...

Ok fine.

> 
> > +	if (musb->set_clock)
> > +		musb->set_clock(musb->clock, 1);
> >
> 
>    musb->set_clock() is about to be dropped...

So there is 'else' part.
> 
> > +	else
> > +		clk_enable(musb->clock);
> > +
> > +	DBG(2, "usbotg_ck=%lud\n", clk_get_rate(musb->clock));
> >
> 
>    I'd put an empty line after that DBG() rather than before.
Ok fine.

> 
> > +	otg_fck = clk_get(musb->controller, "fck");
> > +	clk_enable(otg_fck);
> >
> 
>    Oh, it needs two clocks... Another argument against Felipe dropping
> plat->clock. :-)

..

> 
> > +
> > +	DBG(2, "usbotg_phy_ck=%lud\n", clk_get_rate(otg_fck));
> >
> 
>    Same about empty line here...
> 
> > +	am3517_source_power(musb, 0, 1);
> >
> 
>    Effective NOP...

So need to remove.

> 
> > +	/* Start the on-chip PHY and its PLL. */
> > +	phy_on();
> > +
> > +	msleep(15);
> >
> 
>    5 ms is not enough?

Need to test and correct.

> 
> > +	musb->a_wait_bcon = A_WAIT_BCON_TIMEOUT;
> >
> 
>    Hm... that line kept causing a stream of kernel messages for me,
> until I removed it. Doesn't it for you?

No I didn't get any error messages.. what messages were you getting ?

> 
> > diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> > index 98fd5b6..f8efe00 100644
> > --- a/drivers/usb/musb/musb_core.c
> > +++ b/drivers/usb/musb/musb_core.c
> > @@ -982,7 +982,8 @@ static void musb_shutdown(struct platform_device
> *pdev)
> >   * more than selecting one of a bunch of predefined configurations.
> >   */
> >  #if defined(CONFIG_USB_TUSB6010) || \
> > -	defined(CONFIG_ARCH_OMAP2430) || defined(CONFIG_ARCH_OMAP3)
> > +	defined(CONFIG_ARCH_OMAP2430) || defined(CONFIG_ARCH_OMAP3) || \
> > +	defined(CONFIG_MACH_OMAP3517EVM)
> >
> 
>    Isn't that dependency already covered by CONFIG_ARCH_OMAP3? Or else I
> don't see you adding your #ifdef around musb_platfrom_try_idle()
> declaration...

Right..this can be removed.

> +int musb_platform_exit(struct musb *musb) {
>   
[...]
> +	phy_off();
> +
> +	usb_nop_xceiv_unregister();
> +
> +	return 0;
>   

>>   You forgot the calls to clk_disable() for both your clocks...

Ok fine..need to correct.

Thanks,
Ajay
> 
> WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] musb: add musb support for AM35x

[Sergei Shtylyov]

Gupta, Ajay Kumar wrote:

>>> AM35x has musb interface and uses CPPI4.1 DMA engine.
>>> Current patch supports only PIO mode and there are on-going
>>> discussions on location of CPPI4.1 DMA.
>>>
>>> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>
>>>  	tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
>>> @@ -140,7 +140,7 @@ config USB_MUSB_HDRC_HCD
>>>  config MUSB_PIO_ONLY
>>>  	bool 'Disable DMA (always use PIO)'
>>>  	depends on USB_MUSB_HDRC
>>> -	default y if USB_TUSB6010
>>> +	default USB_TUSB6010 || MACH_OMAP3517EVM
>>>  	help
>>>  	  All data is copied between memory and FIFO by the CPU.
>>>  	  DMA controllers are ignored.
>>> diff --git a/drivers/usb/musb/Makefile b/drivers/usb/musb/Makefile
>>> index 85710cc..9263033 100644
>>> --- a/drivers/usb/musb/Makefile
>>> +++ b/drivers/usb/musb/Makefile
>>> @@ -19,7 +19,11 @@ ifeq ($(CONFIG_ARCH_OMAP2430),y)
>>>  endif
>>>
>>>  ifeq ($(CONFIG_ARCH_OMAP3430),y)
>>> +   ifeq ($(CONFIG_MACH_OMAP3517EVM),y)
>>> +	musb_hdrc-objs  += am3517.o
>>>
>>>       
>>    Isn't there some ARCH-level option for AM3517 SoC? Depending on the
>> board type doesn't really scale well...
>>     
>
> AM3517 is actually based on OMAP3 but musb is different. Unfortunately there
> is no seperate *_ARCH_* defines for AM3517 alone.
>   

   I would really consider adding such... in the current situation the 
thing won't scale with addition of some new AM35x boards.

>>> +static inline void phy_on(void)
>>> +{
>>> +	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_OTGMODE | CONF2_REFFREQ | CONF2_PHY_GPIOMODE);
>>>
>>>       
>>    Shouldn't you manipulate CONF2_OTGMODE in the board code instead? I
>> suspect value of 0 doesn't fit the host-only configuration (without
>> cable connected, MUSB will think it's a B-device, and the driver will
>> fail to initialize IIRC).
>>     

   It will fail to power the port to be precise, so that when you 
finally connect a device, it won't get detected. Note that the comments 
in musb_core.c twice say that the driver expects ID pin to be *forcibly 
grounded* for the host-only mode while CONF2_OTGMODE's value of 0 will 
leave it floating.

> I didn't see any issue in Host/Device only and OTG mode configurations
> On AM3517? Did you see any issue on DA8xx?
>   

   Certainly still seeing it with OTGMODE=0 setting in the host mode -- 
see above.

>>> +/**
>>> + * musb_platform_enable - enable interrupts
>>> + */
>>> +void musb_platform_enable(struct musb *musb)
>>> +{
>>> +	void __iomem *reg_base = musb->ctrl_base;
>>> +	u32 epmask, coremask;
>>> +
>>> +	/* Workaround: setup IRQs through both register sets. */
>>> +	epmask = ((musb->epmask & AM3517_TX_EP_MASK) << USB_INTR_TX_SHIFT) |
>>> +	       ((musb->epmask & AM3517_RX_EP_MASK) << USB_INTR_RX_SHIFT);
>>> +	coremask = (0x01ff << USB_INTR_USB_SHIFT);
>>> +
>>> +	musb_writel(reg_base, EP_INTR_MASK_SET_REG, epmask);
>>> +	musb_writel(reg_base, CORE_INTR_MASK_SET_REG, coremask);
>>>
>>>       
>>    Hm, and I thought all CPPI 4.1 based controllers have the same
>> register layout... alas, I was wrong.
>>     
>
> There are changes as AM3517 supports 15Rx/Tx endpoints.
>   

   Yeah, I need to accomodate cppi41_dma.c to the changes by 
externalizing the code accessing the acceleration registers...

>>> +	case OTG_STATE_A_WAIT_VFALL:
>>> +		/*
>>> +		 * Wait till VBUS falls below SessionEnd (~0.2 V); the 1.3
>>> +		 * RTL seems to mis-handle session "start" otherwise (or in
>>> +		 * our case "recover"), in routine "VBUS was valid by the time
>>> +		 * VBUSERR got reported during enumeration" cases.
>>> +		 */
>>>
>>>       
>>    I wonder if all this still true for RTL 1.8 on which DA8xx (and
>> probably AM3517) MUSB is based...
>>     
>
> Need to check on this..though I didn't see any issue in my testing.
>   

   There should be no issues AFAIU, just the code can be made shorter I 
guess...

>>> +void musb_platform_try_idle(struct musb *musb, unsigned long timeout)
>>> +{
>>>
>>>       
>>    I wonder how DaVinci gets about without musb_platfrom_try_idle()...
>>     
>
> Hmm..
>   

   davinci.c should also define musb_platfrom_try_idle() AFAIU... it 
should be no different from DA8xx in this respect.

>>> +	if (ret != IRQ_HANDLED) {
>>> +		if (epintr || usbintr)
>>> +			/*
>>> +			 * We sometimes get unhandled IRQs in the peripheral
>>> +			 * mode from EP0 and SOF...
>>> +			 */
>>> +			DBG(2, "Unhandled USB IRQ %08x-%08x\n",
>>> +					 epintr, usbintr);
>>>
>>>       
>>    This check shouldn't be needed any more -- EP0 spurious interrupts
>> have been all chased down...
>>     
>
> Ok fine.
>   

   However, I seem to have found a new case of unhandled interrupts 
today: when I disconnect a device, all I get is this message:

da8xx_interrupt 405: Unhandled USB IRQ 00280000

The driver failed to sense the disconnect, it's only reported when I 
re-inserted a device... that's something new. Felipe, are you reading this?

>>> +	musb->a_wait_bcon = A_WAIT_BCON_TIMEOUT;
>>>
>>>       
>>    Hm... that line kept causing a stream of kernel messages for me,
>> until I removed it. Doesn't it for you?
>>     
>
> No I didn't get any error messages.. what messages were you getting ?
>   

   Cannot reproduce them anymore and don't remember which message it was 
-- perhaps this:

musb_bus_suspend 2221: trying to suspend as a_wait_vrise is_active=1

Then it probably got fixed by:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=89368d3d11a5b2eff83ad8e752be67f77a372bad

Look at the below commit however -- it removed such code from omap2430.c:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=f7f9d63eac12b345d6243d1d608b7944a05be921

>> +int musb_platform_exit(struct musb *musb) {  
>>     
> [...]
>   
>> +	phy_off();
>> +
>> +	usb_nop_xceiv_unregister();
>> +
>> +	return 0;  
>>     
>>>   You forgot the calls to clk_disable() for both your clocks...
>>>       

   ... and clk_put() call for the functional clock.

> Ok fine..need to correct.
>
> Thanks,
> Ajay
WBR, Sergei

Re: [PATCH 1/2] musb: add musb support for AM35x

[Sergei Shtylyov]

Ajay Kumar Gupta wrote:

> AM35x has musb interface and uses CPPI4.1 DMA engine.
> Current patch supports only PIO mode and there are on-going
> discussions on location of CPPI4.1 DMA.
>
> Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>

   Overlooked one thing...

[...]

> +int musb_platform_exit(struct musb *musb)
> +{
>   
[...]
> +	phy_off();
> +
> +	usb_nop_xceiv_unregister();
> +
> +	return 0;
>   

   You forgot the calls to clk_disable() for both your clocks...

> +}
> +
>   
WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html