[U-Boot] [RESEND] drivers/usb/ehci: Use platform-specific accessors

Alexey Brodkin Alexey.Brodkin at synopsys.com
Mon Oct 30 11:47:50 UTC 2017


Hi Marek, Vladimir,

On Sun, 2017-10-29 at 11:00 +0100, Marek Vasut wrote:
> On 10/27/2017 02:42 PM, Vladimir Boroda wrote:
> > 
> > Alexey/Marek,
> > 
> > It appears that the "drivers/usb/ehci: Use platform-specific accessors"
> > patch that was submitted in June (and currently adopted in U-Boot
> > releases) kills USB EHCI functionality on PowerPC and likely all other
> > big-endian platforms.  The readl() and writel() accessors already
> > perform endian port to cpu conversion, so no extra conversion was
> > necessary.  The double conversion caused incorrect reading of USB EHCI
> > registers.
> 
> Give Alexey a few days, I've met him at a conference a few days ago so
> he's probably still traveling.

Yep indeed I was recovering after the last trip.

> > I can propose a patch to fix this issue, currently not sure how to
> > submit it for U-Boot approval.  Or you may want to pull the previous
> > patch (or replace the readl() and writel() with some endian-agnostic
> > register reading functions).

Indeed I do see a problem with existing implementation of ehci_{readl|writel}.
Could you please try the following fix which I believe is right now:
------------------------------------->8----------------------------------
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 7c39becd247e..7080ae8bded2 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -101,11 +101,11 @@ struct usb_linux_config_descriptor {
 } __attribute__ ((packed));
 
 #if defined CONFIG_EHCI_DESC_BIG_ENDIAN
-#define ehci_readl(x)          cpu_to_be32(readl(x))
-#define ehci_writel(a, b)      writel(cpu_to_be32(b), a)
+#define ehci_readl(x)          be32_to_cpu(__raw_readl(x))
+#define ehci_writel(a, b)      __raw_writel(cpu_to_be32(a), b)
 #else
-#define ehci_readl(x)          cpu_to_le32(readl(x))
-#define ehci_writel(a, b)      writel(cpu_to_le32(b), a)
+#define ehci_readl(x)          readl(x)
+#define ehci_writel(a, b)      writel(b, a)
 #endif
 
 #if defined CONFIG_EHCI_MMIO_BIG_ENDIAN
------------------------------------->8----------------------------------

I don't have BE platform handy now so your "Tested-by" will be very
nice to have. On LE platform the change above doesn't cause any problems
[as expected] :)

-Alexey


More information about the U-Boot mailing list