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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxx>
 	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


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

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux