From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexey Brodkin Date: Mon, 30 Oct 2017 11:47:50 +0000 Subject: [U-Boot] [RESEND] drivers/usb/ehci: Use platform-specific accessors In-Reply-To: <2c7c3e2a-d930-daad-1f51-4a474000ad57@denx.de> References: <1787895307.6043388.1509108146990.ref@mail.yahoo.com> <1787895307.6043388.1509108146990@mail.yahoo.com> <2c7c3e2a-d930-daad-1f51-4a474000ad57@denx.de> Message-ID: <1509364069.3930.18.camel@synopsys.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de 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