Le lundi, 10 mai 2010 20.35:09, Daniel Mack a écrit :
> On Mon, May 10, 2010 at 08:13:40PM +0200, Philippe Rétornaz wrote:
> > On mx31 the OTG host initialisation fail if you need to have
> > an ULPI transfert to initialize the PHY.
> >
> > In order to be able to communicate with the PHY a complete reset
> > of the usb host is needed. After the PHY initialization the host
> > usb configuration registers need to be rewritten to avoid a host
> > controller lockup.
>
> Given that things are wired up correctly on the board, yes. Many boards
> don't, as they copy-pasted the schematics which got it wrong.
Can you elaborate a bit more ? I don't see how you could do it another way
with an isp1504 and an external power switch.
>
> Anyway, if it helps you, it should go in. Some commenty below ...
>
> > + /* i.Mx31 OTG host has a bug, if you don't do a reset, then ULPI
> > + * transfert timeout. */
> > + if (cpu_is_mx31() && pdev->id == 0) {
> > + /* Wait for the controller to go idle */
> > + for (i = 0; i < 10000; i++) {
> > + if (readl(hcd->regs + USBSTS_OFFSET) & USBSTS_HCH)
> > + break;
> > + udelay(1);
> > + }
>
> Please use a #defined value rather than the 10000 magic.
Ok.
> Also, use cpu_relax() instead of the udelay(1).
I don't argee because if I use cpu_relax() it will make the timeout cpu
frequency dependant. With the udelay it is clear that the timeout is about
10ms.
But of course, I can do the change if this is really the way to do it.
>
> > + if (i == 10000) {
> > + dev_err(dev, "Timeout while stopping USB controller\n");
> > + goto err_init;
> > + }
> > +
> > + /* Stop the usb controller */
> > + temp = readl(hcd->regs + USBCMD_OFFSET);
> > + writel(temp & (~USBCMD_RS), hcd->regs + USBCMD_OFFSET);
> > +
> > + for (i = 0; i < 10000; i++) {
> > + if (!(readl(hcd->regs + USBCMD_OFFSET) & USBCMD_RS))
> > + break;
> > + udelay(1);
> > + }
> > +
> > + if (i == 10000) {
> > + dev_err(dev, "Timeout while stopping USB controller\n");
> > + goto err_init;
> > + }
>
> This seems to be done all over the place. Wouldn't a static inline
> function simplify the code a lot here?
Ok.
Philippe
--
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
