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

[Vladimir Boroda]

Alexey,
I tested the patch on PowerPC.  Seems to work fine, but you have to make sure to set the proper order of arguments for when the EHCI registers are big endian:  __raw_writel(cpu_to_be32(b), a)
To test this patch I had to verify that __raw_ functions are supported for PowerPC, so I created another patch, more in line with your original intent.  Here it is:------------------------------------------------------diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 7c39bec..aeb9745 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -101,11 +101,15 @@ 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)
+/*
+ * On big-endian platforms readl() and writel() perform automatic conversion
+ * from and to little endian.
+ */
+#define ehci_readl(x)          be32_to_cpu(__raw_readl(x))
+#define ehci_writel(a, b)      __raw_writel(cpu_to_be32(b), a)
 #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)          le32_to_cpu(__raw_readl(x))
+#define ehci_writel(a, b)      __raw_writel(cpu_to_le32(b), a)
 #endif

 #if defined CONFIG_EHCI_MMIO_BIG_ENDIAN
------------------------------------------------------Thanks,Vladimir
 

    On Monday, October 30, 2017, 7:49:02 AM EDT, Alexey Brodkin <Alexey.Brodkin at synopsys.com> wrote:  
 
 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