All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Brodkin <Alexey.Brodkin@synopsys.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RESEND] drivers/usb/ehci: Use platform-specific accessors
Date: Mon, 30 Oct 2017 11:47:50 +0000	[thread overview]
Message-ID: <1509364069.3930.18.camel@synopsys.com> (raw)
In-Reply-To: <2c7c3e2a-d930-daad-1f51-4a474000ad57@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

       reply	other threads:[~2017-10-30 11:47 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <1787895307.6043388.1509108146990.ref@mail.yahoo.com>
     [not found] ` <1787895307.6043388.1509108146990@mail.yahoo.com>
     [not found]   ` <2c7c3e2a-d930-daad-1f51-4a474000ad57@denx.de>
2017-10-30 11:47     ` Alexey Brodkin [this message]
2017-10-31 21:27       ` [U-Boot] [RESEND] drivers/usb/ehci: Use platform-specific accessors Vladimir Boroda
2017-10-27  0:20 Vladimir Boroda
  -- strict thread matches above, loose matches on Subject: below --
2017-06-05 19:31 Alexey Brodkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1509364069.3930.18.camel@synopsys.com \
    --to=alexey.brodkin@synopsys.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.