Skip to contentSkip to conversation

drivers/usb/ehci: Use platform-specific accessors

PATCH New Alexey Brodkin 3 participants 14 messages Feb 10, 2017 – Apr 17, 2017

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Current implementation doesn't allow utilization of platform-specific reads and writes.

But some arches or platforms may want to use their accessors that do some extra work like adding barriers for data serialization etc.

Interesting enough OHCI accessors already do that so just aligning EHCI to it now.

Cc Marek Vasut <[email protected]>
Cc Simon Glass <[email protected]>
Cc Mateusz Kulikowski <[email protected]>
---
 drivers/usb/host/ehci.h | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/usb/host/ehci.h +4 -6
index 734d7f036278..2ab830df5155 100644
@@ -102,13 +102,11 @@ struct usb_linux_config_descriptor {
 } __attribute__ ((packed));
 
 #if defined CONFIG_EHCI_DESC_BIG_ENDIAN
-#define ehci_readl(x)		cpu_to_be32((*((volatile u32 *)(x))))
-#define ehci_writel(a, b)	(*((volatile u32 *)(a)) = \
-					cpu_to_be32(((volatile u32)b)))
+#define ehci_readl(x)		cpu_to_be32(readl(x))
+#define ehci_writel(a, b)	writel(cpu_to_be32(b), a)
 #else
-#define ehci_readl(x)		cpu_to_le32((*((volatile u32 *)(x))))
-#define ehci_writel(a, b)	(*((volatile u32 *)(a)) = \
-					cpu_to_le32(((volatile u32)b)))
+#define ehci_readl(x)		cpu_to_le32(readl(x))
+#define ehci_writel(a, b)	writel(cpu_to_le32(b), a)
 #endif
 
 #if defined CONFIG_EHCI_MMIO_BIG_ENDIAN
Signature · 1 line
2.7.4
#1 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Quoted 6 lines · Alexey Brodkin

Current implementation doesn't allow utilization of platform-specific reads and writes.

But some arches or platforms may want to use their accessors that do some extra work like adding barriers for data serialization etc.

Quoted 7 lines

Interesting enough OHCI accessors already do that so just aligning EHCI to it now.

Cc Marek Vasut <[email protected]>
Cc Simon Glass <[email protected]>
Cc Mateusz Kulikowski <[email protected]>

IMO looks OK,

I'd like to have a few more reviews of this before applying it for 2017.05 . Also CCing Wills, it'd be great to get a test on atheros MIPS .

Thanks!

Quoted 27 lines
---
 drivers/usb/host/ehci.h | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/usb/host/ehci.h +4 -6
index 734d7f036278..2ab830df5155 100644
@@ -102,13 +102,11 @@ struct usb_linux_config_descriptor {
 } __attribute__ ((packed));
 
 #if defined CONFIG_EHCI_DESC_BIG_ENDIAN
-#define ehci_readl(x)		cpu_to_be32((*((volatile u32 *)(x))))
-#define ehci_writel(a, b)	(*((volatile u32 *)(a)) = \
-					cpu_to_be32(((volatile u32)b)))
+#define ehci_readl(x)		cpu_to_be32(readl(x))
+#define ehci_writel(a, b)	writel(cpu_to_be32(b), a)
 #else
-#define ehci_readl(x)		cpu_to_le32((*((volatile u32 *)(x))))
-#define ehci_writel(a, b)	(*((volatile u32 *)(a)) = \
-					cpu_to_le32(((volatile u32)b)))
+#define ehci_readl(x)		cpu_to_le32(readl(x))
+#define ehci_writel(a, b)	writel(cpu_to_le32(b), a)
 #endif
 
 #if defined CONFIG_EHCI_MMIO_BIG_ENDIAN
Signature · 2 lines
Best regards,
Marek Vasut
#2 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Hi Marek,

Quoted 16 lines · Marek Vasut
Quoted 7 lines · Alexey Brodkin

Current implementation doesn't allow utilization of platform-specific reads and writes.

But some arches or platforms may want to use their accessors that do some extra work like adding barriers for data serialization etc.

Quoted 7 lines

Interesting enough OHCI accessors already do that so just aligning EHCI to it now.

Cc Marek Vasut <[email protected]>
Cc Simon Glass <[email protected]>
Cc Mateusz Kulikowski <[email protected]>
Quoted 7 lines

IMO looks OK,

I'd like to have a few more reviews of this before applying it for 2017.05 . Also CCing Wills, it'd be great to get a test on atheros MIPS .

It's been quite some time since your request to try the patch on other systems and so far nobody answered. May we consider that one for application then?

Essentially I'm not talking about current Tom's master branch but would be good if the patch lands somewhere and once 2017.03 gets cut is merged in the main branch?

-Alexey

#3 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Quoted 27 lines · Alexey Brodkin

Hi Marek,

Quoted 16 lines · Marek Vasut
Quoted 7 lines · Alexey Brodkin

Current implementation doesn't allow utilization of platform-specific reads and writes.

But some arches or platforms may want to use their accessors that do some extra work like adding barriers for data serialization etc.

Quoted 7 lines

Interesting enough OHCI accessors already do that so just aligning EHCI to it now.

Cc Marek Vasut <[email protected]>
Cc Simon Glass <[email protected]>
Cc Mateusz Kulikowski <[email protected]>
Quoted 7 lines

IMO looks OK,

I'd like to have a few more reviews of this before applying it for 2017.05 . Also CCing Wills, it'd be great to get a test on atheros MIPS .

Quoted 7 lines

It's been quite some time since your request to try the patch on other systems and so far nobody answered. May we consider that one for application then?

Essentially I'm not talking about current Tom's master branch but would be good if the patch lands somewhere and once 2017.03 gets cut is merged in the main branch?

Yeah, it's not happening for 2017.03, it'll go into the next one.

Can you just rebase and repost the patch please ? I'll get to it next week, I'm traveling until the 10th.

Quoted 2 lines

-Alexey

#4 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Hi Marek,

Quoted 35 lines · Marek Vasut
Quoted 34 lines · Alexey Brodkin

Hi Marek,

Quoted 18 lines · Marek Vasut
Quoted 8 lines · Alexey Brodkin

Current implementation doesn't allow utilization of platform-specific reads and writes.

But some arches or platforms may want to use their accessors that do some extra work like adding barriers for data serialization etc.

Quoted 7 lines

Interesting enough OHCI accessors already do that so just aligning EHCI to it now.

Cc Marek Vasut <[email protected]>
Cc Simon Glass <[email protected]>
Cc Mateusz Kulikowski <[email protected]>
Quoted 7 lines

IMO looks OK,

I'd like to have a few more reviews of this before applying it for 2017.05 . Also CCing Wills, it'd be great to get a test on atheros MIPS .

It's been quite some time since your request to try the patch on other systems and so far nobody answered. May we consider that one for application then?

Quoted 8 lines
Quoted 3 lines

Essentially I'm not talking about current Tom's master branch but would be good if the patch lands somewhere and once 2017.03 gets cut is merged in the main branch?

Yeah, it's not happening for 2017.03, it'll go into the next one.

Can you just rebase and repost the patch please ? I'll get to it next week, I'm traveling until the 10th.

I just tried and the same .mbox applies cleanly on top of today's U-Boot master so I believe there's no point in resending it.

Are you OK to just grab existing one?

-Alexey

#5 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Quoted 44 lines · Alexey Brodkin

Hi Marek,

Quoted 41 lines · Marek Vasut
Quoted 30 lines · Alexey Brodkin

Hi Marek,

Quoted 18 lines · Marek Vasut
Quoted 8 lines · Alexey Brodkin

Current implementation doesn't allow utilization of platform-specific reads and writes.

But some arches or platforms may want to use their accessors that do some extra work like adding barriers for data serialization etc.

Quoted 7 lines

Interesting enough OHCI accessors already do that so just aligning EHCI to it now.

Cc Marek Vasut <[email protected]>
Cc Simon Glass <[email protected]>
Cc Mateusz Kulikowski <[email protected]>
Quoted 7 lines

IMO looks OK,

I'd like to have a few more reviews of this before applying it for 2017.05 . Also CCing Wills, it'd be great to get a test on atheros MIPS .

Quoted 7 lines

It's been quite some time since your request to try the patch on other systems and so far nobody answered. May we consider that one for application then?

Essentially I'm not talking about current Tom's master branch but would be good if the patch lands somewhere and once 2017.03 gets cut is merged in the main branch?

Yeah, it's not happening for 2017.03, it'll go into the next one.

Quoted 7 lines
Quoted 2 lines

Can you just rebase and repost the patch please ? I'll get to it next week, I'm traveling until the 10th.

I just tried and the same .mbox applies cleanly on top of today's U-Boot master so I believe there's no point in resending it.

Are you OK to just grab existing one?

I no longer have the patch in my mailbox, which is why I asked you to resend it. Moreover, did you just test that it applies or did you actually build-test and retest the patch on real hardware too ?

#6 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Hi Marek,

Quoted 53 lines · Marek Vasut
Quoted 52 lines · Alexey Brodkin

Hi Marek,

Quoted 47 lines · Marek Vasut
Quoted 33 lines · Alexey Brodkin

Hi Marek,

Quoted 20 lines · Marek Vasut
Quoted 9 lines · Alexey Brodkin

Current implementation doesn't allow utilization of platform-specific reads and writes.

But some arches or platforms may want to use their accessors that do some extra work like adding barriers for data serialization etc.

Quoted 7 lines

Interesting enough OHCI accessors already do that so just aligning EHCI to it now.

Cc Marek Vasut <[email protected]>
Cc Simon Glass <[email protected]>
Cc Mateusz Kulikowski <[email protected]>
Quoted 7 lines

IMO looks OK,

I'd like to have a few more reviews of this before applying it for 2017.05 . Also CCing Wills, it'd be great to get a test on atheros MIPS .

Quoted 7 lines

It's been quite some time since your request to try the patch on other systems and so far nobody answered. May we consider that one for application then?

Essentially I'm not talking about current Tom's master branch but would be good if the patch lands somewhere and once 2017.03 gets cut is merged in the main branch?

Yeah, it's not happening for 2017.03, it'll go into the next one.

Can you just rebase and repost the patch please ? I'll get to it next week, I'm traveling until the 10th.

Quoted 8 lines
Quoted 4 lines

I just tried and the same .mbox applies cleanly on top of today's U-Boot master so I believe there's no point in resending it.

Are you OK to just grab existing one?

I no longer have the patch in my mailbox, which is why I asked you to resend it. Moreover, did you just test that it applies or did you actually build-test and retest the patch on real hardware too ?

I did test it on my boards again on top of today's master branch and everything seem to work perfectly fine so please consider applying this one.

You may use mbox from here http://patchwork.ozlabs.org/patch/726714/mbox/
Al least it worked for me this way:
--------------------->8-----------------------
curl http://patchwork.ozlabs.org/patch/726714/mbox/ | git am --------------------->8-----------------------

Regards,
Alexey

#7 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Quoted 65 lines · Alexey Brodkin

Hi Marek,

Quoted 53 lines · Marek Vasut
Quoted 52 lines · Alexey Brodkin

Hi Marek,

Quoted 47 lines · Marek Vasut
Quoted 33 lines · Alexey Brodkin

Hi Marek,

Quoted 20 lines · Marek Vasut
Quoted 9 lines · Alexey Brodkin

Current implementation doesn't allow utilization of platform-specific reads and writes.

But some arches or platforms may want to use their accessors that do some extra work like adding barriers for data serialization etc.

Quoted 7 lines

Interesting enough OHCI accessors already do that so just aligning EHCI to it now.

Cc Marek Vasut <[email protected]>
Cc Simon Glass <[email protected]>
Cc Mateusz Kulikowski <[email protected]>
Quoted 7 lines

IMO looks OK,

I'd like to have a few more reviews of this before applying it for 2017.05 . Also CCing Wills, it'd be great to get a test on atheros MIPS .

Quoted 7 lines

It's been quite some time since your request to try the patch on other systems and so far nobody answered. May we consider that one for application then?

Essentially I'm not talking about current Tom's master branch but would be good if the patch lands somewhere and once 2017.03 gets cut is merged in the main branch?

Yeah, it's not happening for 2017.03, it'll go into the next one.

Can you just rebase and repost the patch please ? I'll get to it next week, I'm traveling until the 10th.

Quoted 8 lines
Quoted 4 lines

I just tried and the same .mbox applies cleanly on top of today's U-Boot master so I believe there's no point in resending it.

Are you OK to just grab existing one?

I no longer have the patch in my mailbox, which is why I asked you to resend it. Moreover, did you just test that it applies or did you actually build-test and retest the patch on real hardware too ?

Quoted 8 lines

I did test it on my boards again on top of today's master branch and everything seem to work perfectly fine so please consider applying this one.

You may use mbox from here http://patchwork.ozlabs.org/patch/726714/mbox/
Al least it worked for me this way:
--------------------->8-----------------------
curl http://patchwork.ozlabs.org/patch/726714/mbox/ | git am --------------------->8-----------------------

Thank you for NOT resending the patch and making my life harder. Applied.

Signature · 2 lines
Best regards,
Marek Vasut
#8 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Quoted 69 lines · Marek Vasut
Quoted 68 lines · Alexey Brodkin

Hi Marek,

Quoted 53 lines · Marek Vasut
Quoted 52 lines · Alexey Brodkin

Hi Marek,

Quoted 47 lines · Marek Vasut
Quoted 33 lines · Alexey Brodkin

Hi Marek,

Quoted 20 lines · Marek Vasut
Quoted 9 lines · Alexey Brodkin

Current implementation doesn't allow utilization of platform-specific reads and writes.

But some arches or platforms may want to use their accessors that do some extra work like adding barriers for data serialization etc.

Quoted 7 lines

Interesting enough OHCI accessors already do that so just aligning EHCI to it now.

Cc Marek Vasut <[email protected]>
Cc Simon Glass <[email protected]>
Cc Mateusz Kulikowski <[email protected]>
Quoted 7 lines

IMO looks OK,

I'd like to have a few more reviews of this before applying it for 2017.05 . Also CCing Wills, it'd be great to get a test on atheros MIPS .

Quoted 7 lines

It's been quite some time since your request to try the patch on other systems and so far nobody answered. May we consider that one for application then?

Essentially I'm not talking about current Tom's master branch but would be good if the patch lands somewhere and once 2017.03 gets cut is merged in the main branch?

Yeah, it's not happening for 2017.03, it'll go into the next one.

Can you just rebase and repost the patch please ? I'll get to it next week, I'm traveling until the 10th.

Quoted 8 lines
Quoted 4 lines

I just tried and the same .mbox applies cleanly on top of today's U-Boot master so I believe there's no point in resending it.

Are you OK to just grab existing one?

I no longer have the patch in my mailbox, which is why I asked you to resend it. Moreover, did you just test that it applies or did you actually build-test and retest the patch on real hardware too ?

I did test it on my boards again on top of today's master branch and everything seem to work perfectly fine so please consider applying this one.

Quoted 8 lines
Quoted 5 lines

You may use mbox from here http://patchwork.ozlabs.org/patch/726714/mbox/
Al least it worked for me this way:
--------------------->8-----------------------
curl http://patchwork.ozlabs.org/patch/726714/mbox/ | git am --------------------->8-----------------------

Thank you for NOT resending the patch and making my life harder. Applied.

Even better, this patch breaks powerpc board lwmon5, so dropped.

Signature · 2 lines
Best regards,
Marek Vasut
#9 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Hi Marek,

Quoted 3 lines · Marek Vasut
Quoted 1 line · Marek Vasut

Â

Even better, this patch breaks powerpc board lwmon5, so dropped.

Are you sure the problem is in my patch?

Maybe this PPC board just needs to UNdefine CONFIG_EHCI_DESC_BIG_ENDIAN or there's a problem in its either cpu_to_XXX() or readl()/writel()?

Stefan, could you please look at what's wrong here if you have the board handy?

-Alexey

#10 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Quoted 1 line · Alexey Brodkin

Hi Marek,

Hi,

Quoted 6 lines · Marek Vasut
Quoted 3 lines · Marek Vasut
Quoted 1 line

Even better, this patch breaks powerpc board lwmon5, so dropped.

Are you sure the problem is in my patch?

Builds without the patch, does not build with the patch.

Patch which modifies common code which affects a lot of different architectures should be at least compile-tested . We already have buildman for that. Clearly it was not, otherwise this would have popped up.

Quoted 6 lines

Maybe this PPC board just needs to UNdefine CONFIG_EHCI_DESC_BIG_ENDIAN or there's a problem in its either cpu_to_XXX() or readl()/writel()?

Stefan, could you please look at what's wrong here if you have the board handy?

The board is pretty ancient. I'd rather appreciate if you could look into this and fix the issue, then re-submit. Thanks

Quoted 2 lines

-Alexey

Signature · 2 lines
Best regards,
Marek Vasut
#11 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

In reply to Alexey Brodkin
Quoted 8 lines · Alexey Brodkin

Hi Marek,

Quoted 3 lines · Marek Vasut
Quoted 1 line · Marek Vasut

Â

Even better, this patch breaks powerpc board lwmon5, so dropped.

Are you sure the problem is in my patch?

Yes, it is. Dropping this from the USB PR gets everything going again.

Quoted 6 lines

Maybe this PPC board just needs to UNdefine CONFIG_EHCI_DESC_BIG_ENDIAN or there's a problem in its either cpu_to_XXX() or readl()/writel()?

Stefan, could you please look at what's wrong here if you have the board handy?

Well, since this is your set of changes, you should have some idea what's right/wrong here given what things are before/after. Also, for wide changes like this, please give things a build either in travis-ci (so that most of the world is built) or do most of the world locally before posting, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170414/5041838d/attachment.sig>
#12 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Hi Tom, Marek,

Quoted 6 lines · Tom Rini
Quoted 5 lines · Alexey Brodkin

Hi Marek,

Quoted 1 line · Marek Vasut
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines · Marek Vasut
Quoted 2 lines

Even better, this patch breaks powerpc board lwmon5, so dropped.

Are you sure the problem is in my patch?

Yes, it is.  Dropping this from the USB PR gets everything going again.

Ok the problem was obvious missing inclusion f "asm/io.h". Fix is submitted here http://patchwork.ozlabs.org/patch/751397/

Please consider for applying.

Quoted 4 lines
Quoted 4 lines

Maybe this PPC board just needs to UNdefine CONFIG_EHCI_DESC_BIG_ENDIAN or there's a problem in its either cpu_to_XXX() or readl()/writel()?

Quoted 8 lines
Quoted 2 lines

Stefan, could you please look at what's wrong here if you have the board handy?

Well, since this is your set of changes, you should have some idea what's right/wrong here given what things are before/after.  Also, for wide changes like this, please give things a build either in travis-ci (so that most of the world is built) or do most of the world locally before posting, thanks!

Well I tried to run a buildman locally (thanks to machine resources I have) but out of 1243 boards 154 have fatal problems if I use toolchains obtained via buildbot. Which is barely useful for capturing issues introduced by incremental patches :(

-Alexey

#13 lore raw

PATCH 1/1 drivers/usb/ehci: Use platform-specific accessors

Quoted 14 lines · Alexey Brodkin

Hi Tom, Marek,

Quoted 11 lines · Tom Rini
Quoted 10 lines · Alexey Brodkin

Hi Marek,

Quoted 5 lines · Marek Vasut
Quoted 2 lines · Marek Vasut

Even better, this patch breaks powerpc board lwmon5, so dropped.

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

Are you sure the problem is in my patch?

Yes, it is.  Dropping this from the USB PR gets everything going again.

Ok the problem was obvious missing inclusion f "asm/io.h". Fix is submitted here http://patchwork.ozlabs.org/patch/751397/

Please consider for applying.

Thanks.

Quoted 13 lines
Quoted 4 lines
Quoted 4 lines

Maybe this PPC board just needs to UNdefine CONFIG_EHCI_DESC_BIG_ENDIAN or there's a problem in its either cpu_to_XXX() or readl()/writel()?

Quoted 8 lines
Quoted 2 lines

Stefan, could you please look at what's wrong here if you have the board handy?

Well, since this is your set of changes, you should have some idea what's right/wrong here given what things are before/after.  Also, for wide changes like this, please give things a build either in travis-ci (so that most of the world is built) or do most of the world locally before posting, thanks!

Quoted 4 lines

Well I tried to run a buildman locally (thanks to machine resources I have) but out of 1243 boards 154 have fatal problems if I use toolchains obtained via buildbot. Which is barely useful for capturing issues introduced by incremental patches :(

Did you mean buildman? If you look at .travis.yml that builds all of those boards (well, with the exception of some we need gcc-6 in order to have small enough binaries for) and that's pretty close to 1200 I think.

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170417/d86c6f32/attachment.sig>