Re: [PATCH 7/8] Add EHCI support for MX27 and MX31 based boards

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

 



Daniel Mack wrote:
Hi Sascha,

On Fri, Jun 26, 2009 at 10:45:50AM +0200, Sascha Hauer wrote:
On Thu, Jun 25, 2009 at 02:11:56PM +0200, Daniel Mack wrote:
+   /* set USBMODE to host mode */
+   temp = readl(hcd->regs + USBMODE_OFFSET);
+   writel(temp | USBMODE_CM_HOST, hcd->regs + USBMODE_OFFSET);
+
+   /* set up the PORTSCx register */
+   writel(pdata->portsc, hcd->regs + PORTSC_OFFSET);
+   mdelay(10);
+
+   /* setup USBCONTROL. Bits for all 3 ports are MUXed into that
+    * single register, hence we have to access memory outside the
+    * resource window */
+   temp = readl(IO_ADDRESS(OTG_BASE_ADDR + USBCTRL_OTGBASE_OFFSET));
This is not good. Currently we can build a kernel for i.MX31 and i.MX35.
Problem is that the OTG base addresses differ. Maybe we should add a
function which encapsulates accesses to this register. We might even
want to add locking to this function.
As a general note I don't want to depend on defines like OTG_BASE_ADDR
in drivers. This all comes back to us when we try to build kernels for
multiple i.MXs.

Yes, fully agreed. See the new patch below. Better now?

There is need to add the special cases CPUs other than MX31, but at
least the skeleton is there now, and it is serparated from the generic
host controller driver.

+   /* call platform specific init function */
+   if (pdata->init) {
+           ret = pdata->init(pdev);
+           if (ret) {
+                   dev_err(dev, "platform init failed\n");
+                   goto err_init;
+           }
+   }
+
My board needs an mdelay here. I can put this into my board code, but
does anybody else see this?

I still got trouble with that on my board and I can't really figure out
what the problem is. Adding an mdelay() here gives some improvement, but
for the OTG port, I still get 0x24e4/0x2524 as ID in the xvcr's init
function, but I'm not totally sure I can rule out the hardware. That's
why I was asking for more testers.

I also need a mdelay(10) on my board at the exact same place. I think it would be better to put it into the driver code than into the platform code (if all platforms need it).



+   /* Initialize the transceiver */
+   if (pdata->xcvr) {
+           pdata->xcvr->access_priv = hcd->regs + ULPI_VIEWPORT_OFFSET;
+           if (usb_xcvr_init(pdata->xcvr) != 0)
+                   dev_err(dev, "unable to init transceiver\n");
+           else if (usb_xcvr_set_vbus(pdata->xcvr, 1) != 0)
+                   dev_err(dev, "unable to enable vbus on transceiver\n");
We should probably bail out here.

In my case, this isn't really a fatal situation since the VBUS power is
not even switched by th 1504. So if this fails, USB will still work
normally. Hence I removed the error handling for this condition.

Daniel


Val

--
Valentin Longchamp, PhD Student, EPFL-STI-LSRO1
valentin.longchamp@xxxxxxx, Phone: +41216937827
http://people.epfl.ch/valentin.longchamp
MEA3485, Station 9, CH-1015 Lausanne
--
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