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
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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
