Hi Sergei
Thank you for pointing it
> > + /* set platform specific port settings */
> > + iowrite32(0x00000000, (reg0 + USBPCTRL0));
>
> This register contains completely board specific setting,
> hard-coding it to 0 is wrong.
> Shouldn't you have passed its value as platform data instead?
(snip)
> > + iowrite32(0x00ff0040, (reg0 + EIIBC1));
> > + iowrite32(0x00ff0040, (reg1 + EIIBC1));
> > +
> > + iowrite32(0x00000001, (reg0 + EIIBC2));
> > + iowrite32(0x00000001, (reg1 + EIIBC2));
>
> Why write each of these register twice, at different bases? The USB
> section of
> the R-Car H1 manual doesn't seem to mention that they are dual mapped...
(snip)
> > + if (priv->counter-- == 1) { /* last user */
> > + iowrite32(0x00000000, (reg0 + USBPCTRL0));
>
> Why change this register at all at shutdown?
Sorry, I didn't mention about these settings value on this patch.
This driver is using the fixed value for particular applications at this point,
because of prototype driver at that point.
This means it should use platform/chip specific callback function
Best regards
---
Kuninori Morimoto
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
