[PATCH resend 1/3] AM35x: Add musb support

[PATCH resend 1/3] AM35x: Add musb support

[Ajay Kumar Gupta]

AM35x has musb interface (version 1.8) and uses CPPI41 DMA engine.
It has USB phy built inside the IP itself.

Also added ARCH_AM35x which is required to differentiate musb ips
between OMAP3x and AM35x. This config would be used for below purposes,
        - Select am35x.c instead of omap2430.c for compilation
          at drivers/usb/musb directory. Please note there are
          significant differneces in these two files as musb ip
          in quite different on AM35x.
        - Select workaround codes applicable for AM35x musb issues.
          one such workaround is for bytewise read issue on AM35x.

Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
---
Patches created against recent linus's tree.

 arch/arm/mach-omap2/Kconfig           |    1 +
 arch/arm/mach-omap2/board-am3517evm.c |   30 ++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/usb-musb.c        |    4 ++++
 arch/arm/plat-omap/Kconfig            |    8 ++++++++
 arch/arm/plat-omap/include/plat/usb.h |   21 +++++++++++++++++++++
 5 files changed, 64 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index b31b6f1..52a6752 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -84,6 +84,7 @@ config MACH_OMAP3517EVM
 	bool "OMAP3517/ AM3517 EVM board"
 	depends on ARCH_OMAP3
 	select OMAP_PACKAGE_CBB
+	select ARCH_AM35x
 
 config MACH_OMAP3_PANDORA
 	bool "OMAP3 Pandora"
diff --git a/arch/arm/mach-omap2/board-am3517evm.c b/arch/arm/mach-omap2/board-am3517evm.c
index af383a8..7f42a1b 100644
--- a/arch/arm/mach-omap2/board-am3517evm.c
+++ b/arch/arm/mach-omap2/board-am3517evm.c
@@ -35,6 +35,7 @@
 #include <plat/control.h>
 #include <plat/usb.h>
 #include <plat/display.h>
+#include <plat/control.h>
 
 #include "mux.h"
 
@@ -375,6 +376,30 @@ static void __init am3517_evm_init_irq(void)
 	omap_gpio_init();
 }
 
+static struct omap_musb_board_data musb_board_data = {
+	.interface_type         = MUSB_INTERFACE_ULPI,
+	.mode                   = MUSB_OTG,
+	.power                  = 500,
+};
+
+static __init void am3517_evm_musb_init(void)
+{
+	u32 devconf2;
+
+	/*
+	 * Set up USB clock/mode in the DEVCONF2 register.
+	 */
+	devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2);
+
+	/* USB2.0 PHY reference clock is 13 MHz */
+	devconf2 &= ~(CONF2_REFFREQ | CONF2_OTGMODE);
+	devconf2 |=  CONF2_REFFREQ_13MHZ | CONF2_SESENDEN | CONF2_VBDTCTEN;
+
+	omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2);
+
+	usb_musb_init(&musb_board_data);
+}
+
 static const struct ehci_hcd_omap_platform_data ehci_pdata __initconst = {
 	.port_mode[0] = EHCI_HCD_OMAP_MODE_PHY,
 #if defined(CONFIG_PANEL_SHARP_LQ043T1DG01) || \
@@ -393,6 +418,8 @@ static const struct ehci_hcd_omap_platform_data ehci_pdata __initconst = {
 
 #ifdef CONFIG_OMAP_MUX
 static struct omap_board_mux board_mux[] __initdata = {
+	/* USB OTG DRVVBUS offset = 0x212 */
+	OMAP3_MUX(SAD2D_MCAD23, OMAP_MUX_MODE0 | OMAP_PIN_INPUT_PULLDOWN),
 	{ .reg_offset = OMAP_MUX_TERMINATOR },
 };
 #else
@@ -459,6 +486,9 @@ static void __init am3517_evm_init(void)
 				ARRAY_SIZE(am3517evm_i2c1_boardinfo));
 	/*Ethernet*/
 	am3517_evm_ethernet_init(&am3517_evm_emac_pdata);
+
+	/* MUSB */
+	am3517_evm_musb_init();
 }
 
 static void __init am3517_evm_map_io(void)
diff --git a/arch/arm/mach-omap2/usb-musb.c b/arch/arm/mach-omap2/usb-musb.c
index 96f6787..a54f360 100644
--- a/arch/arm/mach-omap2/usb-musb.c
+++ b/arch/arm/mach-omap2/usb-musb.c
@@ -29,6 +29,7 @@
 #include <mach/hardware.h>
 #include <mach/irqs.h>
 #include <plat/mux.h>
+#include <mach/am35xx.h>
 #include <plat/usb.h>
 
 #ifdef CONFIG_USB_MUSB_SOC
@@ -90,6 +91,9 @@ void __init usb_musb_init(struct omap_musb_board_data *board_data)
 {
 	if (cpu_is_omap243x()) {
 		musb_resources[0].start = OMAP243X_HS_BASE;
+	} else if (cpu_is_omap3517() || cpu_is_omap3505()) {
+		musb_resources[0].start = AM35XX_IPSS_USBOTGSS_BASE;
+		musb_resources[1].start = INT_35XX_USBOTG_IRQ;
 	} else if (cpu_is_omap34xx()) {
 		musb_resources[0].start = OMAP34XX_HSUSB_OTG_BASE;
 	} else if (cpu_is_omap44xx()) {
diff --git a/arch/arm/plat-omap/Kconfig b/arch/arm/plat-omap/Kconfig
index 78b49a6..3f33d75 100644
--- a/arch/arm/plat-omap/Kconfig
+++ b/arch/arm/plat-omap/Kconfig
@@ -5,6 +5,14 @@ menu "TI OMAP Implementations"
 config ARCH_OMAP_OTG
 	bool
 
+config ARCH_AM35x
+	bool
+	help
+	  Select this option if your platform is based on AM35x. As
+	  AM35x has an updated MUSB with CPPI4.1 DMA so this config
+	  is introduced to differentiate musb ip between OMAP3x and
+	  AM35x platforms.
+
 choice
 	prompt "OMAP System Type"
 	default ARCH_OMAP2PLUS
diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat-omap/include/plat/usb.h
index 98eef53..76d2e29 100644
--- a/arch/arm/plat-omap/include/plat/usb.h
+++ b/arch/arm/plat-omap/include/plat/usb.h
@@ -191,5 +191,26 @@ void omap_usb_init(struct omap_usb_config *pdata);
 #	define	USBT2TLL5PI		(1 << 17)
 #	define	USB0PUENACTLOI		(1 << 16)
 #	define	USBSTANDBYCTRL		(1 << 15)
+/* AM35x */
+/* USB 2.0 PHY Control */
+#define CONF2_PHY_GPIOMODE	(1 << 23)
+#define CONF2_OTGMODE		(3 << 14)
+#define CONF2_NO_OVERRIDE	(0 << 14)
+#define CONF2_FORCE_HOST	(1 << 14)
+#define CONF2_FORCE_DEVICE	(2 << 14)
+#define CONF2_FORCE_HOST_VBUS_LOW (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)
 
 #endif	/* __ASM_ARCH_OMAP_USB_H */
-- 
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

[PATCH resend 2/3] 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. DMA support can be
added later once basic CPPI4.1 DMA patch is accepted.

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

diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index cfd38ed..87d73fa 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 || ARCH_AM35x)
 	select TWL4030_USB if MACH_OMAP_3430SDP
 	select USB_OTG_UTILS
 	tristate 'Inventra Highspeed Dual Role Controller (TI, ADI, ...)'
@@ -144,7 +144,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 || ARCH_AM35x
 	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 9705f71..f99f675 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_ARCH_AM35x),y)
+	musb_hdrc-objs  += am35x.o
+   else
 	musb_hdrc-objs	+= omap2430.o
+   endif
 endif
 
 ifeq ($(CONFIG_ARCH_OMAP4),y)
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
@@ -0,0 +1,519 @@
+/*
+ * Texas Instruments AM35x "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"
+
+/*
+ * AM35x specific definitions
+ */
+/* USB 2.0 OTG module registers */
+#define USB_REVISION_REG	0x00
+#define USB_CTRL_REG		0x04
+#define USB_STAT_REG		0x08
+#define USB_EMULATION_REG	0x0c
+/* 0x10 Reserved */
+#define USB_AUTOREQ_REG		0x14
+#define USB_SRP_FIX_TIME_REG	0x18
+#define USB_TEARDOWN_REG	0x1c
+#define EP_INTR_SRC_REG		0x20
+#define EP_INTR_SRC_SET_REG	0x24
+#define EP_INTR_SRC_CLEAR_REG	0x28
+#define EP_INTR_MASK_REG	0x2c
+#define EP_INTR_MASK_SET_REG	0x30
+#define EP_INTR_MASK_CLEAR_REG	0x34
+#define EP_INTR_SRC_MASKED_REG	0x38
+#define CORE_INTR_SRC_REG	0x40
+#define CORE_INTR_SRC_SET_REG	0x44
+#define CORE_INTR_SRC_CLEAR_REG	0x48
+#define CORE_INTR_MASK_REG	0x4c
+#define CORE_INTR_MASK_SET_REG	0x50
+#define CORE_INTR_MASK_CLEAR_REG 0x54
+#define CORE_INTR_SRC_MASKED_REG 0x58
+/* 0x5c Reserved */
+#define USB_END_OF_INTR_REG	0x60
+
+/* Control register bits */
+#define USB_SOFT_RESET_MASK	1
+#define A_WAIT_BCON_TIMEOUT	1100		/* in ms */
+
+/* USB interrupt register bits */
+#define USB_INTR_USB_SHIFT	16
+#define USB_INTR_USB_MASK	(0x1ff << USB_INTR_USB_SHIFT)
+#define USB_INTR_DRVVBUS	0x100
+#define USB_INTR_RX_SHIFT	16
+#define USB_INTR_TX_SHIFT	0
+#define AM35X_TX_EP_MASK	0xffff		/* EP0 + 15 Tx EPs */
+#define AM35X_RX_EP_MASK	0xfffe		/* 15 Rx EPs */
+#define AM35X_TX_INTR_MASK	(AM35X_TX_EP_MASK << USB_INTR_TX_SHIFT)
+#define AM35X_RX_INTR_MASK	(AM35X_RX_EP_MASK << USB_INTR_RX_SHIFT)
+
+#define USB_MENTOR_CORE_OFFSET	0x400
+
+static inline void phy_on(void)
+{
+	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);
+	devconf2 |= CONF2_PHY_PLLON | 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();
+
+		if (time_after(jiffies, timeout)) {
+			DBG(1, "musb PHY clock good timed out\n");
+			break;
+		}
+	}
+}
+
+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;
+
+	/* Workaround: setup IRQs through both register sets. */
+	epmask = ((musb->epmask & AM35X_TX_EP_MASK) << USB_INTR_TX_SHIFT) |
+	       ((musb->epmask & AM35X_RX_EP_MASK) << USB_INTR_RX_SHIFT);
+
+	musb_writel(reg_base, EP_INTR_MASK_SET_REG, epmask);
+	musb_writel(reg_base, CORE_INTR_MASK_SET_REG, USB_INTR_USB_MASK);
+
+	/* 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,
+			 AM35X_TX_INTR_MASK | AM35X_RX_INTR_MASK);
+	musb_writeb(musb->mregs, MUSB_DEVCTL, 0);
+	musb_writel(reg_base, USB_END_OF_INTR_REG, 0);
+}
+
+#ifdef CONFIG_USB_MUSB_HDRC_HCD
+#define portstate(stmt)		stmt
+#else
+#define portstate(stmt)
+#endif
+
+static void am35x_set_vbus(struct musb *musb, int is_on)
+{
+	WARN_ON(is_on && is_peripheral_active(musb));
+}
+
+#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 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_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:
+		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;
+
+		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 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;
+	}
+
+	/* 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
+	 * AM35x'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. */
+		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)
+		ret |= musb_interrupt(musb);
+
+ 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);
+
+	return ret;
+}
+
+int musb_platform_set_mode(struct musb *musb, u8 musb_mode)
+{
+	u32 devconf2 = omap_ctrl_readl(AM35XX_CONTROL_DEVCONF2);
+
+	devconf2 &= ~CONF2_OTGMODE;
+	switch (musb_mode) {
+#ifdef	CONFIG_USB_MUSB_HDRC_HCD
+	case MUSB_HOST:		/* Force VBUS valid, ID = 0 */
+		devconf2 |= CONF2_FORCE_HOST;
+		break;
+#endif
+#ifdef	CONFIG_USB_GADGET_MUSB_HDRC
+	case MUSB_PERIPHERAL:	/* Force VBUS valid, ID = 1 */
+		devconf2 |= CONF2_FORCE_DEVICE;
+		break;
+#endif
+#ifdef	CONFIG_USB_MUSB_OTG
+	case MUSB_OTG:		/* Don't override the VBUS/ID comparators */
+		devconf2 |= CONF2_NO_OVERRIDE;
+		break;
+#endif
+	default:
+		DBG(2, "Trying to set unsupported mode %u\n", musb_mode);
+	}
+
+	omap_ctrl_writel(devconf2, AM35XX_CONTROL_DEVCONF2);
+	return 0;
+}
+
+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");
+	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;
+
+	usb_nop_xceiv_register();
+	musb->xceiv = otg_get_transceiver();
+	if (!musb->xceiv)
+		return -ENODEV;
+
+	if (is_host_enabled(musb))
+		setup_timer(&otg_workaround, otg_timer, (unsigned long) musb);
+
+	musb->board_set_vbus = am35x_set_vbus;
+	musb->a_wait_bcon = A_WAIT_BCON_TIMEOUT;
+
+	/* 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(5);
+
+	musb->isr = am35x_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)
+{
+	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... */
+	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...
+		 */
+		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);
+	}
+done:
+	phy_off();
+
+	usb_nop_xceiv_unregister();
+
+	if (musb->set_clock)
+		musb->set_clock(musb->clock, 0);
+	else
+		clk_disable(musb->clock);
+
+	if (otg_fck) {
+		clk_put(otg_fck);
+		clk_disable(otg_fck);
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_PM
+void musb_platform_save_context(struct musb *musb,
+	struct musb_context_registers *musb_context)
+{
+	phy_off();
+}
+
+void musb_platform_restore_context(struct musb *musb,
+	struct musb_context_registers *musb_context)
+{
+	phy_on();
+}
+#endif
-- 
1.6.2.4

[PATCH resend 3/3] musb: AM35x: 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@ti.com>
---
 drivers/usb/musb/am35x.c     |   30 ++++++++++++++++++++++++++++++
 drivers/usb/musb/musb_core.c |    2 ++
 2 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/usb/musb/am35x.c b/drivers/usb/musb/am35x.c
index 0cdc6bf..cdcd6f3 100644
--- a/drivers/usb/musb/am35x.c
+++ b/drivers/usb/musb/am35x.c
@@ -517,3 +517,33 @@ void musb_platform_restore_context(struct musb *musb,
 	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++) {
+			*(u32 *) dst = musb_readl(fifo, 0);
+			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 3b795c5..e22e98b 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -272,6 +272,7 @@ void musb_write_fifo(struct musb_hw_ep *hw_ep, u16 len, const u8 *src)
 	}
 }
 
+#if !defined(CONFIG_ARCH_AM35x)
 /*
  * Unload an endpoint's FIFO
  */
@@ -309,6 +310,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

Re: [PATCH resend 2/3] 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. DMA support can be
> added later once basic CPPI4.1 DMA patch is accepted.
> 
> Signed-off-by: Ajay Kumar Gupta <ajay.gupta@ti.com>

[...]

> 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

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

[Gupta, Ajay Kumar]

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

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

[Sergei Shtylyov]

Hello.

Gupta, Ajay Kumar wrote:

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

    Hm, why GPIOMODE then I wonder... :-)

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

    Ah, I got it reversed: 1 means normal polarity indeed...

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

    I didn't test DaVinci much; I didn't even test DA8xx much in OTG mode. :-/
And in the DA8xx host mode VBUS error would never occur anyway (as the VBUS 
comparator is overridden on the EVM board).

> As such I didn't see issue
> In my normal testing.

    OK.

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

    Do you remember which issue?
    Although I suspect that this code might be related to the comment below: 
"DEVCTL.BDEVICE is invalid without DEVCTL.SESSION set".

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

>> [...]

>>> +	musb->a_wait_bcon = A_WAIT_BCON_TIMEOUT;

>>     This field is set by musb_core.c, so I dropped this line from
>> da8xx.c...

    This has also been dropped from omap2430.c by this commit:

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

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

    Hm, is this related?

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

    Didn't have time to properly look into the OTG mode yet...

> I need to check on this, I guess it's not required though I have tested
> OTG also on AM3517EVM.

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

    Also, you call clk_get() on this clock twice, in musb_platfrom_init() and 
here, but clk_put() only once.

> 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 resend 1/3] AM35x: Add musb support

[Tony Lindgren]

* Ajay Kumar Gupta <ajay.gupta@ti.com> [100702 09:52]:
> AM35x has musb interface (version 1.8) and uses CPPI41 DMA engine.
> It has USB phy built inside the IP itself.
> 
> Also added ARCH_AM35x which is required to differentiate musb ips
> between OMAP3x and AM35x. This config would be used for below purposes,
>         - Select am35x.c instead of omap2430.c for compilation
>           at drivers/usb/musb directory. Please note there are
>           significant differneces in these two files as musb ip
>           in quite different on AM35x.
>         - Select workaround codes applicable for AM35x musb issues.
>           one such workaround is for bytewise read issue on AM35x.
> 

<snip>

> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index b31b6f1..52a6752 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -84,6 +84,7 @@ config MACH_OMAP3517EVM
>  	bool "OMAP3517/ AM3517 EVM board"
>  	depends on ARCH_OMAP3
>  	select OMAP_PACKAGE_CBB
> +	select ARCH_AM35x
>  
>  config MACH_OMAP3_PANDORA
>  	bool "OMAP3 Pandora"

No thanks..

> --- a/arch/arm/plat-omap/Kconfig
> +++ b/arch/arm/plat-omap/Kconfig
> @@ -5,6 +5,14 @@ menu "TI OMAP Implementations"
>  config ARCH_OMAP_OTG
>  	bool
>  
> +config ARCH_AM35x
> +	bool
> +	help
> +	  Select this option if your platform is based on AM35x. As
> +	  AM35x has an updated MUSB with CPPI4.1 DMA so this config
> +	  is introduced to differentiate musb ip between OMAP3x and
> +	  AM35x platforms.
> +
>  choice
>  	prompt "OMAP System Type"
>  	default ARCH_OMAP2PLUS

..this should not be needed.

Regards,

Tony

Re: [PATCH resend 1/3] AM35x: Add musb support

[Sergei Shtylyov]

Hello.

Tony Lindgren wrote:

>> AM35x has musb interface (version 1.8) and uses CPPI41 DMA engine.
>> It has USB phy built inside the IP itself.
>>
>> Also added ARCH_AM35x which is required to differentiate musb ips
>> between OMAP3x and AM35x. This config would be used for below purposes,
>>         - Select am35x.c instead of omap2430.c for compilation
>>           at drivers/usb/musb directory. Please note there are
>>           significant differneces in these two files as musb ip
>>           in quite different on AM35x.
>>         - Select workaround codes applicable for AM35x musb issues.
>>           one such workaround is for bytewise read issue on AM35x.

> <snip>

>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
>> index b31b6f1..52a6752 100644
>> --- a/arch/arm/mach-omap2/Kconfig
>> +++ b/arch/arm/mach-omap2/Kconfig
>> @@ -84,6 +84,7 @@ config MACH_OMAP3517EVM
>>  	bool "OMAP3517/ AM3517 EVM board"
>>  	depends on ARCH_OMAP3
>>  	select OMAP_PACKAGE_CBB
>> +	select ARCH_AM35x
>>  
>>  config MACH_OMAP3_PANDORA
>>  	bool "OMAP3 Pandora"

> No thanks..

>> --- a/arch/arm/plat-omap/Kconfig
>> +++ b/arch/arm/plat-omap/Kconfig
>> @@ -5,6 +5,14 @@ menu "TI OMAP Implementations"
>>  config ARCH_OMAP_OTG
>>  	bool
>>  
>> +config ARCH_AM35x
>> +	bool
>> +	help
>> +	  Select this option if your platform is based on AM35x. As
>> +	  AM35x has an updated MUSB with CPPI4.1 DMA so this config
>> +	  is introduced to differentiate musb ip between OMAP3x and
>> +	  AM35x platforms.
>> +
>>  choice
>>  	prompt "OMAP System Type"
>>  	default ARCH_OMAP2PLUS

> ..this should not be needed.

    I think Ajay has explained why it's needed. The option is necessary in one 
or another form.

> Regards,
> Tony

WBR, Sergei

Re: [PATCH resend 1/3] AM35x: Add musb support

[Tony Lindgren]

* Sergei Shtylyov <sshtylyov-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org> [100705 12:57]:
> Hello.
> 
> Tony Lindgren wrote:
> 
> >>AM35x has musb interface (version 1.8) and uses CPPI41 DMA engine.
> >>It has USB phy built inside the IP itself.
> >>
> >>Also added ARCH_AM35x which is required to differentiate musb ips
> >>between OMAP3x and AM35x. This config would be used for below purposes,
> >>        - Select am35x.c instead of omap2430.c for compilation
> >>          at drivers/usb/musb directory. Please note there are
> >>          significant differneces in these two files as musb ip
> >>          in quite different on AM35x.
> >>        - Select workaround codes applicable for AM35x musb issues.
> >>          one such workaround is for bytewise read issue on AM35x.
> 
> ><snip>
> 
> >>diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> >>index b31b6f1..52a6752 100644
> >>--- a/arch/arm/mach-omap2/Kconfig
> >>+++ b/arch/arm/mach-omap2/Kconfig
> >>@@ -84,6 +84,7 @@ config MACH_OMAP3517EVM
> >> 	bool "OMAP3517/ AM3517 EVM board"
> >> 	depends on ARCH_OMAP3
> >> 	select OMAP_PACKAGE_CBB
> >>+	select ARCH_AM35x
> >> config MACH_OMAP3_PANDORA
> >> 	bool "OMAP3 Pandora"
> 
> >No thanks..
> 
> >>--- a/arch/arm/plat-omap/Kconfig
> >>+++ b/arch/arm/plat-omap/Kconfig
> >>@@ -5,6 +5,14 @@ menu "TI OMAP Implementations"
> >> config ARCH_OMAP_OTG
> >> 	bool
> >>+config ARCH_AM35x
> >>+	bool
> >>+	help
> >>+	  Select this option if your platform is based on AM35x. As
> >>+	  AM35x has an updated MUSB with CPPI4.1 DMA so this config
> >>+	  is introduced to differentiate musb ip between OMAP3x and
> >>+	  AM35x platforms.
> >>+
> >> choice
> >> 	prompt "OMAP System Type"
> >> 	default ARCH_OMAP2PLUS
> 
> >..this should not be needed.
> 
>    I think Ajay has explained why it's needed. The option is
> necessary in one or another form.

It's not needed for omaps, we can already build in support for
omap2, omap3 and omap4 into the same kernel binary.

If a Kconfig option is needed for optionally compiling in the support
for am35x musb, it should be called USB_MUSB_AM35X or similar that
gets selected if the boards using it are selected.

Regards,

Tony
--
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 resend 1/3] AM35x: Add musb support

[Sergei Shtylyov]

Hello.

Tony Lindgren wrote:

>>>> AM35x has musb interface (version 1.8) and uses CPPI41 DMA engine.
>>>> It has USB phy built inside the IP itself.

>>>> Also added ARCH_AM35x which is required to differentiate musb ips
>>>> between OMAP3x and AM35x. This config would be used for below purposes,
>>>>        - Select am35x.c instead of omap2430.c for compilation
>>>>          at drivers/usb/musb directory. Please note there are
>>>>          significant differneces in these two files as musb ip
>>>>          in quite different on AM35x.
>>>>        - Select workaround codes applicable for AM35x musb issues.
>>>>          one such workaround is for bytewise read issue on AM35x.

>>> <snip>

>>>> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
>>>> index b31b6f1..52a6752 100644
>>>> --- a/arch/arm/mach-omap2/Kconfig
>>>> +++ b/arch/arm/mach-omap2/Kconfig
>>>> @@ -84,6 +84,7 @@ config MACH_OMAP3517EVM
>>>> 	bool "OMAP3517/ AM3517 EVM board"
>>>> 	depends on ARCH_OMAP3
>>>> 	select OMAP_PACKAGE_CBB
>>>> +	select ARCH_AM35x
>>>> config MACH_OMAP3_PANDORA
>>>> 	bool "OMAP3 Pandora"

>>> No thanks..

>>>> --- a/arch/arm/plat-omap/Kconfig
>>>> +++ b/arch/arm/plat-omap/Kconfig
>>>> @@ -5,6 +5,14 @@ menu "TI OMAP Implementations"
>>>> config ARCH_OMAP_OTG
>>>> 	bool
>>>> +config ARCH_AM35x
>>>> +	bool
>>>> +	help
>>>> +	  Select this option if your platform is based on AM35x. As
>>>> +	  AM35x has an updated MUSB with CPPI4.1 DMA so this config
>>>> +	  is introduced to differentiate musb ip between OMAP3x and
>>>> +	  AM35x platforms.
>>>> +
>>>> choice
>>>> 	prompt "OMAP System Type"
>>>> 	default ARCH_OMAP2PLUS

>>> ..this should not be needed.

>>    I think Ajay has explained why it's needed. The option is
>> necessary in one or another form.

> It's not needed for omaps, we can already build in support for
> omap2, omap3 and omap4 into the same kernel binary.

    Not with AM35x USB support merged -- at least you won't be able to build 
single kernel with monolithic MUSB support.

> If a Kconfig option is needed for optionally compiling in the support
> for am35x musb, it should be called USB_MUSB_AM35X or similar that
> gets selected if the boards using it are selected.

    Do you mean that we should have this option in drivers/usb/musb/Kconfig?

> Regards,
> Tony

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 resend 1/3] AM35x: Add musb support

[Tony Lindgren]

* Sergei Shtylyov <sshtylyov-Igf4POYTYCDQT0dZR+AlfA@public.gmane.org> [100705 13:29]:

<snip>

> >>>>choice
> >>>>	prompt "OMAP System Type"
> >>>>	default ARCH_OMAP2PLUS
> 
> >>>..this should not be needed.
> 
> >>   I think Ajay has explained why it's needed. The option is
> >>necessary in one or another form.
> 
> >It's not needed for omaps, we can already build in support for
> >omap2, omap3 and omap4 into the same kernel binary.
> 
>    Not with AM35x USB support merged -- at least you won't be able
> to build single kernel with monolithic MUSB support.

Right. I believe musb is pretty much the only remaining driver
that won't behave with multi-omap. But let's not merge code that
would make fixing that even harder.
 
> >If a Kconfig option is needed for optionally compiling in the support
> >for am35x musb, it should be called USB_MUSB_AM35X or similar that
> >gets selected if the boards using it are selected.
> 
>    Do you mean that we should have this option in drivers/usb/musb/Kconfig?

Yeah, it could be set automatically with default y if MACH_AM35X_SOME_BOARD.

Then options like this should not be mutually exclusive like they currently
are for musb, that breaks using musb with multi omap.

Regards,

Tony


--
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 resend 2/3] musb: add musb support for AM35x

[Gupta, Ajay Kumar]

Hi,
> >>> +{
> >>> +	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.
> 
>     Hm, why GPIOMODE then I wonder... :-)
> 
> >>> +	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.
> 
>     Ah, I got it reversed: 1 means normal polarity indeed...

I think, this would again be dependent on boards and therefore
should be moved to board file.

> 
> >>> +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 ?
> 
>     I didn't test DaVinci much; I didn't even test DA8xx much in OTG mode.
> :-/
> And in the DA8xx host mode VBUS error would never occur anyway (as the
> VBUS
> comparator is overridden on the EVM board).
> 
> > As such I didn't see issue
> > In my normal testing.
> 
>     OK.
> 
> >>> +		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.
> 
>     Do you remember which issue?
>     Although I suspect that this code might be related to the comment
> below:
> "DEVCTL.BDEVICE is invalid without DEVCTL.SESSION set".

I will test again and then provide the details. I too feel it has something
to do with the comment you pointed out.

> 
> >>> +		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;
> 
> >> [...]
> 
> >>> +	musb->a_wait_bcon = A_WAIT_BCON_TIMEOUT;
> 
> >>     This field is set by musb_core.c, so I dropped this line from
> >> da8xx.c...
> 
>     This has also been dropped from omap2430.c by this commit:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-
> 2.6.git;a=commit;h=f7f9d63eac12b345d6243d1d608b7944a05be921
> 
> > 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.
> 
>     Hm, is this related?

Not really but let me test again and get the details.

> 
> >>> +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... */
> 
[..]
> 
> >>     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.
> 
>     Also, you call clk_get() on this clock twice, in musb_platfrom_init()
> and
> here, but clk_put() only once.

Ok fine.

Thanks,
Ajay
> 
> > Thanks,
> > Ajay
> 
> WBR, Sergei

RE: [PATCH resend 1/3] AM35x: Add musb support

[Gupta, Ajay Kumar]

> > >>   I think Ajay has explained why it's needed. The option is
> > >>necessary in one or another form.
> >
> > >It's not needed for omaps, we can already build in support for
> > >omap2, omap3 and omap4 into the same kernel binary.
> >
> >    Not with AM35x USB support merged -- at least you won't be able
> > to build single kernel with monolithic MUSB support.
> 
> Right. I believe musb is pretty much the only remaining driver
> that won't behave with multi-omap. But let's not merge code that
> would make fixing that even harder.
> 
> > >If a Kconfig option is needed for optionally compiling in the support
> > >for am35x musb, it should be called USB_MUSB_AM35X or similar that
> > >gets selected if the boards using it are selected.
> >
> >    Do you mean that we should have this option in
> drivers/usb/musb/Kconfig?
> 
> Yeah, it could be set automatically with default y if
> MACH_AM35X_SOME_BOARD.
> 
> Then options like this should not be mutually exclusive like they
> currently are for musb, that breaks using musb with multi omap.

Choosing USB_MUSB_AM35X would anyways compile am35x.c and not omap2430.c
File; thus musb would not work on OMAP3x boards with same binary.

I will update the patches and submit for further reviews.

Thanks,
Ajay
> 
> Regards,
> 
> Tony
> 

Re: [PATCH resend 1/3] AM35x: Add musb support

[Tony Lindgren]

* Gupta, Ajay Kumar <ajay.gupta-l0cyMroinI0@public.gmane.org> [100705 14:48]:
> > > >>   I think Ajay has explained why it's needed. The option is
> > > >>necessary in one or another form.
> > >
> > > >It's not needed for omaps, we can already build in support for
> > > >omap2, omap3 and omap4 into the same kernel binary.
> > >
> > >    Not with AM35x USB support merged -- at least you won't be able
> > > to build single kernel with monolithic MUSB support.
> > 
> > Right. I believe musb is pretty much the only remaining driver
> > that won't behave with multi-omap. But let's not merge code that
> > would make fixing that even harder.
> > 
> > > >If a Kconfig option is needed for optionally compiling in the support
> > > >for am35x musb, it should be called USB_MUSB_AM35X or similar that
> > > >gets selected if the boards using it are selected.
> > >
> > >    Do you mean that we should have this option in
> > drivers/usb/musb/Kconfig?
> > 
> > Yeah, it could be set automatically with default y if
> > MACH_AM35X_SOME_BOARD.
> > 
> > Then options like this should not be mutually exclusive like they
> > currently are for musb, that breaks using musb with multi omap.
> 
> Choosing USB_MUSB_AM35X would anyways compile am35x.c and not omap2430.c
> File; thus musb would not work on OMAP3x boards with same binary.

You should set up things so both can be compiled in, that's standard
behaviour for all Linux drivers :)
 
> I will update the patches and submit for further reviews.

Thanks,

Tony
--
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 resend 1/3] AM35x: Add musb support

[Sergei Shtylyov]

Hello.

Tony Lindgren wrote:

>>>>>>   I think Ajay has explained why it's needed. The option is
>>>>>> necessary in one or another form.

>>>>> It's not needed for omaps, we can already build in support for
>>>>> omap2, omap3 and omap4 into the same kernel binary.

    Don't see how it hinders that.

>>>>    Not with AM35x USB support merged -- at least you won't be able
>>>> to build single kernel with monolithic MUSB support.

>>> Right. I believe musb is pretty much the only remaining driver
>>> that won't behave with multi-omap. But let's not merge code that
>>> would make fixing that even harder.

    I don't see how it's making fixing this harder... (though it's already hard).

>>>>> If a Kconfig option is needed for optionally compiling in the support
>>>>> for am35x musb, it should be called USB_MUSB_AM35X or similar that
>>>>> gets selected if the boards using it are selected.

>>>>    Do you mean that we should have this option in

>>> drivers/usb/musb/Kconfig?

>>> Yeah, it could be set automatically with default y if
>>> MACH_AM35X_SOME_BOARD.

>>> Then options like this should not be mutually exclusive like they
>>> currently are for musb, that breaks using musb with multi omap.

>> Choosing USB_MUSB_AM35X would anyways compile am35x.c and not omap2430.c
>> File; thus musb would not work on OMAP3x boards with same binary.

> You should set up things so both can be compiled in, that's standard
> behaviour for all Linux drivers :)

   This is much easier said than done.

>> I will update the patches and submit for further reviews.

> Thanks,
> Tony

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 resend 1/3] AM35x: Add musb support

[Gupta, Ajay Kumar]

Hi,
> >>>>> If a Kconfig option is needed for optionally compiling in the
> support
> >>>>> for am35x musb, it should be called USB_MUSB_AM35X or similar that
> >>>>> gets selected if the boards using it are selected.
> 
> >>>>    Do you mean that we should have this option in
> 
> >>> drivers/usb/musb/Kconfig?
> 
> >>> Yeah, it could be set automatically with default y if
> >>> MACH_AM35X_SOME_BOARD.
> 
> >>> Then options like this should not be mutually exclusive like they
> >>> currently are for musb, that breaks using musb with multi omap.
> 
> >> Choosing USB_MUSB_AM35X would anyways compile am35x.c and not
> omap2430.c
> >> File; thus musb would not work on OMAP3x boards with same binary.
> 
> > You should set up things so both can be compiled in, that's standard
> > behaviour for all Linux drivers :)
> 
>    This is much easier said than done.

Tony,

As musb controllers are quite different between OMAPs and AM35x so it
would be difficult in terms of software maintenance with such approach.

Some of the major changes in AM35x are,
	- Has builtin USB PHY
	- It doesn't use Mentor DMA but uses CPPI4.1 DMA
	- Has set of wrapper register which are not present on OMAPs.
	- Doesn't have SYSCONFIG registers which are present on OMAPs.
	- Has bytewise read limitation which is not applicable to OMAPs.

Musb on AM35x is actually quite similar to musb on DA8xx family of
Devices.

I can update the patches to use USB_MUSB_AM35X config within 
drivers/usb/musb/ but that would still not be able to compile in
Both omapx and am35x stuff in single binary.

Thanks,
Ajay 
 
> 
> >> I will update the patches and submit for further reviews.
> 
> > Thanks,
> > Tony
> 
> WBR, Sergei

Re: [PATCH resend 1/3] AM35x: Add musb support

[Tony Lindgren]

* Gupta, Ajay Kumar <ajay.gupta@ti.com> [100706 10:17]:
>
> As musb controllers are quite different between OMAPs and AM35x so it
> would be difficult in terms of software maintenance with such approach.

Hmm, this is all pretty standard stuff..
 
> Some of the major changes in AM35x are,
> 	- Has builtin USB PHY

Register the PHY from platform data.

> 	- It doesn't use Mentor DMA but uses CPPI4.1 DMA

Register DMA functions from platform data.

> 	- Has set of wrapper register which are not present on OMAPs.

Yeah tusb6010 has the same problem, but that's already dealt with
I believe.

> 	- Doesn't have SYSCONFIG registers which are present on OMAPs.
> 	- Has bytewise read limitation which is not applicable to OMAPs.

Sounds like this has been mostly dealt with already with tusb6010.
 
> Musb on AM35x is actually quite similar to musb on DA8xx family of
> Devices.
> 
> I can update the patches to use USB_MUSB_AM35X config within 
> drivers/usb/musb/ but that would still not be able to compile in
> Both omapx and am35x stuff in single binary.

Sounds like we should first fix thing before adding new code
that will make fixing the basic issues harder.

Tony

Re: [PATCH resend 1/3] AM35x: Add musb support

[Felipe Balbi]

Hi,

On Tue, Jul 06, 2010 at 09:57:09AM +0200, ext Tony Lindgren wrote:
>Sounds like we should first fix thing before adding new code
>that will make fixing the basic issues harder.

my idea to deal with this is to have a set of "platform glue drivers".  
So omap2430.c, blackfin.c, tusb6010 and davinci.c would become a 
platform driver themselves that would instantiate musb-hdrc 
platform_driver and the correct dma driver (inventra, omap, cppi, etc).

It would look something like:

#include <plat/usb.h>

static struct musb_hdrc_ops omap2430_musb_ops = {
	.readb	= generic_readb,
	.readw	= generic_readw,
	.readl	= generic_readl,
	.writeb	= generic_writeb,
	.writew	= generic_writew,
	.writel	= generic_writel,
};

static struct musb_platform_data omap2430_musb_data = {
	.ops	= &omap2430_musb_ops,
	.mode	= -EINVAL,	/* change it later */
};

static int __init omap2430_musb_probe(struct platform_device *pdev)
{
	struct omap24030_musb_platform_data pdata = pdev->dev.platform_data;

	musb = platform_device_alloc("musb-hdrc", -1);

	/* check error */

	musb->dev.parent = &pdev->dev;
	omap2430_musb_data.mode = pdata->mode;
	/* initialize every other necessary field */

	platform_device_add_data(musb, &omap2430_musb_data);

	dma = platform_device_alloc("dma-inventra", -1);

	/* check error */

	dma->dev.parent = &pdev->dev;

	/* add both devices */

	return 0;
}

static int omap2430_musb_suspend(struct platform_device *pdev,
			pm_message_t state)
{
	return omap2430_musb_save_context(pdev);
}

static int omap2430_musb_resume(stuct platform_device *pdev)
{
	omap2430_musb_restore_context(pdev);
}

this would allow us to factor-out save/restore context, get rid of all 
exported functions (by just adding every necessary function to 
musb_hdrc_ops) and compile in every single musb file in one driver and 
still have it working. Boards would, then, just instantiate 
musb-omap2430/musb-blackfin/musb-tusb6010/musb-davinci 
platform_driver(s) and the rest would be done on runtime, since only the 
driver that matches would actually probe.

How does that sound ??

-- 
balbi

DefectiveByDesign.org
--
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 resend 1/3] AM35x: Add musb support

[Tony Lindgren]

* Felipe Balbi <felipe.balbi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org> [100706 11:40]:
> Hi,
> 
> On Tue, Jul 06, 2010 at 09:57:09AM +0200, ext Tony Lindgren wrote:
> >Sounds like we should first fix thing before adding new code
> >that will make fixing the basic issues harder.
> 
> my idea to deal with this is to have a set of "platform glue
> drivers".  So omap2430.c, blackfin.c, tusb6010 and davinci.c would
> become a platform driver themselves that would instantiate musb-hdrc
> platform_driver and the correct dma driver (inventra, omap, cppi,
> etc).
> 
> It would look something like:
> 
> #include <plat/usb.h>
> 
> static struct musb_hdrc_ops omap2430_musb_ops = {
> 	.readb	= generic_readb,
> 	.readw	= generic_readw,
> 	.readl	= generic_readl,
> 	.writeb	= generic_writeb,
> 	.writew	= generic_writew,
> 	.writel	= generic_writel,
> };
> 
> static struct musb_platform_data omap2430_musb_data = {
> 	.ops	= &omap2430_musb_ops,
> 	.mode	= -EINVAL,	/* change it later */
> };
> 
> static int __init omap2430_musb_probe(struct platform_device *pdev)
> {
> 	struct omap24030_musb_platform_data pdata = pdev->dev.platform_data;
> 
> 	musb = platform_device_alloc("musb-hdrc", -1);
> 
> 	/* check error */
> 
> 	musb->dev.parent = &pdev->dev;
> 	omap2430_musb_data.mode = pdata->mode;
> 	/* initialize every other necessary field */
> 
> 	platform_device_add_data(musb, &omap2430_musb_data);
> 
> 	dma = platform_device_alloc("dma-inventra", -1);
> 
> 	/* check error */
> 
> 	dma->dev.parent = &pdev->dev;
> 
> 	/* add both devices */
> 
> 	return 0;
> }
> 
> static int omap2430_musb_suspend(struct platform_device *pdev,
> 			pm_message_t state)
> {
> 	return omap2430_musb_save_context(pdev);
> }
> 
> static int omap2430_musb_resume(stuct platform_device *pdev)
> {
> 	omap2430_musb_restore_context(pdev);
> }
> 
> this would allow us to factor-out save/restore context, get rid of
> all exported functions (by just adding every necessary function to
> musb_hdrc_ops) and compile in every single musb file in one driver
> and still have it working. Boards would, then, just instantiate
> musb-omap2430/musb-blackfin/musb-tusb6010/musb-davinci
> platform_driver(s) and the rest would be done on runtime, since only
> the driver that matches would actually probe.
> 
> How does that sound ??

That sounds good to me! It would be nice to have usable musb with
soon-to-be-gone omap3_defconfig :)

Tony
--
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 resend 1/3] AM35x: Add musb support

[Gadiyar, Anand]

Felipe Balbi wrote:
> On Tue, Jul 06, 2010 at 09:57:09AM +0200, ext Tony Lindgren wrote:
> >Sounds like we should first fix thing before adding new code
> >that will make fixing the basic issues harder.
> 
> my idea to deal with this is to have a set of "platform glue drivers".  
> So omap2430.c, blackfin.c, tusb6010 and davinci.c would become a 
> platform driver themselves that would instantiate musb-hdrc 
> platform_driver and the correct dma driver (inventra, omap, cppi, etc).
> 
> It would look something like:
> 
> #include <plat/usb.h>
> 
> static struct musb_hdrc_ops omap2430_musb_ops = {
> 	.readb	= generic_readb,
> 	.readw	= generic_readw,
> 	.readl	= generic_readl,
> 	.writeb	= generic_writeb,
> 	.writew	= generic_writew,
> 	.writel	= generic_writel,
> };
> 
> static struct musb_platform_data omap2430_musb_data = {
> 	.ops	= &omap2430_musb_ops,
> 	.mode	= -EINVAL,	/* change it later */
> };
> 
> static int __init omap2430_musb_probe(struct platform_device *pdev)
> {
> 	struct omap24030_musb_platform_data pdata = pdev->dev.platform_data;
> 
> 	musb = platform_device_alloc("musb-hdrc", -1);
> 
> 	/* check error */
> 
> 	musb->dev.parent = &pdev->dev;
> 	omap2430_musb_data.mode = pdata->mode;
> 	/* initialize every other necessary field */
> 
> 	platform_device_add_data(musb, &omap2430_musb_data);
> 
> 	dma = platform_device_alloc("dma-inventra", -1);
> 
> 	/* check error */
> 
> 	dma->dev.parent = &pdev->dev;
> 
> 	/* add both devices */
> 
> 	return 0;
> }
> 
> static int omap2430_musb_suspend(struct platform_device *pdev,
> 			pm_message_t state)
> {
> 	return omap2430_musb_save_context(pdev);
> }
> 
> static int omap2430_musb_resume(stuct platform_device *pdev)
> {
> 	omap2430_musb_restore_context(pdev);
> }
> 
> this would allow us to factor-out save/restore context, get rid of all 
> exported functions (by just adding every necessary function to 
> musb_hdrc_ops) and compile in every single musb file in one driver and 
> still have it working. Boards would, then, just instantiate 
> musb-omap2430/musb-blackfin/musb-tusb6010/musb-davinci 
> platform_driver(s) and the rest would be done on runtime, since only the 
> driver that matches would actually probe.
> 
> How does that sound ??
> 

Like it is done in ehci-hcd.c/ohci-hcd.c?

This looks much easier to maintain.

Do you already have a patch doing this, or would you like me to spin one
for RFC/RFT?

- Anand

Re: [PATCH resend 1/3] AM35x: Add musb support

[Felipe Balbi]

Hi,

On Wed, Jul 07, 2010 at 04:16:04AM +0530, Gadiyar, Anand wrote:
> Like it is done in ehci-hcd.c/ohci-hcd.c?

That's different, they export the platform_driver which gets probed by
ehci-hcd.c, here we would have a parent platform_driver instianting
correct dma device and musb device.

> This looks much easier to maintain.
> 
> Do you already have a patch doing this, or would you like me to spin one
> for RFC/RFT?

I started this yesterday before leaving the office, so not much done. If
you beat me to it, go for it :-)

-- 
balbi