Hi Sinan,
About the commit in the $subject 87fe2d543f81, would there be any
specific reason why the logic pio versions of these functions did not
get the same treatment or should not? I'm talking about lib/logic_pio.c
here - commit 031e3601869c ("lib: Add generic PIO mapping method")
introduced this.
In fact, logic pio will override these for arm64 with the vanilla
defconfig these days.
It seems that your change was made just after that logic pio stuff went
into the kernel.
Thanks,
John
Hi John,
On 2/28/2020 4:52 AM, John Garry wrote:
> About the commit in the $subject 87fe2d543f81, would there be any
> specific reason why the logic pio versions of these functions did not
> get the same treatment or should not? I'm talking about lib/logic_pio.c
> here - commit 031e3601869c ("lib: Add generic PIO mapping method")
> introduced this.
>
> In fact, logic pio will override these for arm64 with the vanilla
> defconfig these days.
We only looked at inX()/inY() and readX()/writeX() API because the
semantics of these API are defined in the kernel documentation.
We looked at how to generalize this so that there is a uniform
behavior across different architectures.
Is logic PIO subject to ordering issues?
How is the behavior on different architectures?
As long as the expectations are set, I see no reason why it shouldn't
but, I'll let Arnd comment on it too.
Sinan
Hi Sinan,
Thanks for getting back to me.
> On 2/28/2020 4:52 AM, John Garry wrote:
>> About the commit in the $subject 87fe2d543f81, would there be any
>> specific reason why the logic pio versions of these functions did not
>> get the same treatment
In fact, your changes and the logic PIO changes went in at the same time.
or should not? I'm talking about lib/logic_pio.c
>> here - commit 031e3601869c ("lib: Add generic PIO mapping method")
>> introduced this.
>>
>> In fact, logic pio will override these for arm64 with the vanilla
>> defconfig these days.
>
> We only looked at inX()/inY() and readX()/writeX() API because the
> semantics of these API are defined in the kernel documentation.
Could we consider adding __io_pbr() et al to the kernel Documentation? I
couldn't find them and I had to rely on checking 64e2c67738 ("io: define
several IO & PIO barrier types for the asm-generic version") commit
message to find the definition.
> We looked at how to generalize this so that there is a uniform
> behavior across different architectures.
>
> Is logic PIO subject to ordering issues?
Well the point is that we're still concerned here with using
readX/writeX for MMIO-based IO port accesses, see *** from logic_pio.c:
#define BUILD_LOGIC_IO(bw, type)
type logic_in##bw(unsigned long addr)
{
type ret = (type)~0;
if (addr < MMIO_UPPER_LIMIT) {
ret = read##bw(PCI_IOBASE + addr); ***
} else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {
struct logic_pio_hwaddr *entry = find_io_range(addr);
if (entry)
ret = entry->ops->in(entry->hostdata,
addr, sizeof(type));
else
WARN_ON_ONCE(1);
}
return ret;
}
> How is the behavior on different architectures?
So today only ARM64 uses it for this relevant code, above. But maybe
others in future will want to use it - any arch without native IO port
access is a candidate.
>
> As long as the expectations are set, I see no reason why it shouldn't
> but, I'll let Arnd comment on it too.
ok, so it looks reasonable consider replicating your change for ***, above.
Thanks,
John
On 3/2/2020 7:35 AM, John Garry wrote:
> Hi Sinan,
>
> Thanks for getting back to me.
>
>> On 2/28/2020 4:52 AM, John Garry wrote:
>>> About the commit in the $subject 87fe2d543f81, would there be any
>>> specific reason why the logic pio versions of these functions did not
>>> get the same treatment
>
> In fact, your changes and the logic PIO changes went in at the same time.
>
> or should not? I'm talking about lib/logic_pio.c
I think your change missed "cross-architecture" category.
>
> #define BUILD_LOGIC_IO(bw, type)
> type logic_in##bw(unsigned long addr)
> {
> type ret = (type)~0;
> if (addr < MMIO_UPPER_LIMIT) {
> ret = read##bw(PCI_IOBASE + addr); ***
> } else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {
> struct logic_pio_hwaddr *entry = find_io_range(addr);
>
> if (entry)
> ret = entry->ops->in(entry->hostdata,
> addr, sizeof(type));
> else
> WARN_ON_ONCE(1);
> }
> return ret;
> }
>
>> How is the behavior on different architectures?
>
> So today only ARM64 uses it for this relevant code, above. But maybe
> others in future will want to use it - any arch without native IO port
> access is a candidate.
I'm looking at Arnd here for help.
>
>>
>> As long as the expectations are set, I see no reason why it shouldn't
>> but, I'll let Arnd comment on it too.
>
> ok, so it looks reasonable consider replicating your change for ***, above.
Arnd is the maintainer here. We should consult first.
I believe there is also a linux-arch mailing list. Going there with this
question makes sense IMO.
>
> Thanks,
> John
+ linux-arch
For background, see
https://lore.kernel.org/lkml/[email protected]/
>>
>> So today only ARM64 uses it for this relevant code, above. But maybe
>> others in future will want to use it - any arch without native IO port
>> access is a candidate.
>
> I'm looking at Arnd here for help.
>
>>
>>>
>>> As long as the expectations are set, I see no reason why it shouldn't
>>> but, I'll let Arnd comment on it too.
>>
>> ok, so it looks reasonable consider replicating your change for ***, above.
To be clear, I would make this change in lib/logic_pio.c since
__io_pbr() can be overridden per-arch:
#define BUILD_LOGIC_IO(bw, type)
type logic_in##bw(unsigned long addr)
{
type ret = (type)~0;
if (addr < MMIO_UPPER_LIMIT) {
- ret = read##bw(PCI_IOBASE + addr);
+ __io_pbr();
+ ret = __raw_read##bw(PCI_IOBASE + addr);
+ __io_pbr();
} else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {
struct logic_pio_hwaddr *entry = find_io_range(addr);
...
(forgetting leX_to_cpu for the moment)
>
> Arnd is the maintainer here. We should consult first.
ok, fine.
> I believe there is also a linux-arch mailing list. Going there with this
> question makes sense IMO.
Cheers,
John
On Tue, Mar 3, 2020 at 2:18 PM John Garry <[email protected]> wrote:
>
> + linux-arch
>
> For background, see
> https://lore.kernel.org/lkml/[email protected]/
>
> >>
> >> So today only ARM64 uses it for this relevant code, above. But maybe
> >> others in future will want to use it - any arch without native IO port
> >> access is a candidate.
> >
> > I'm looking at Arnd here for help.
> >
> >>
> >>>
> >>> As long as the expectations are set, I see no reason why it shouldn't
> >>> but, I'll let Arnd comment on it too.
> >>
> >> ok, so it looks reasonable consider replicating your change for ***, above.
>
> To be clear, I would make this change in lib/logic_pio.c since
> __io_pbr() can be overridden per-arch:
>
> #define BUILD_LOGIC_IO(bw, type)
> type logic_in##bw(unsigned long addr)
> {
> type ret = (type)~0;
> if (addr < MMIO_UPPER_LIMIT) {
> - ret = read##bw(PCI_IOBASE + addr);
> + __io_pbr();
> + ret = __raw_read##bw(PCI_IOBASE + addr);
> + __io_pbr();
__io_par();
> } else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {
> struct logic_pio_hwaddr *entry = find_io_range(addr);
>
> ...
>
> (forgetting leX_to_cpu for the moment)
Yes, I suppose this is required to get consistent behavior on arm64,
which overrides __io_par() but not __io_ar(), with the current code
the barrier after read is weaker when LOGIC_PIO is enabled than it
is otherwise.
For other architectures, I suppose we would need another indirection
level, as those can also override the default inb() itself to do something
other than readb(PCI_IOBASE + addr), and that is not handled
here either. We can do that if we need LOGIC_PIO on a second
architecture.
Arnd
On 03/03/2020 16:40, Arnd Bergmann wrote:
> On Tue, Mar 3, 2020 at 2:18 PM John Garry <[email protected]> wrote:
>>
>> + linux-arch
>>
>> For background, see
>> https://lore.kernel.org/lkml/[email protected]/
>>
>>>>
>>>> So today only ARM64 uses it for this relevant code, above. But maybe
>>>> others in future will want to use it - any arch without native IO port
>>>> access is a candidate.
>>>
>>> I'm looking at Arnd here for help.
>>>
>>>>
>>>>>
>>>>> As long as the expectations are set, I see no reason why it shouldn't
>>>>> but, I'll let Arnd comment on it too.
>>>>
>>>> ok, so it looks reasonable consider replicating your change for ***, above.
>>
>> To be clear, I would make this change in lib/logic_pio.c since
>> __io_pbr() can be overridden per-arch:
>>
>> #define BUILD_LOGIC_IO(bw, type)
>> type logic_in##bw(unsigned long addr)
>> {
>> type ret = (type)~0;
>> if (addr < MMIO_UPPER_LIMIT) {
>> - ret = read##bw(PCI_IOBASE + addr);
>> + __io_pbr();
>> + ret = __raw_read##bw(PCI_IOBASE + addr);
>> + __io_pbr();
>
> __io_par();
>
>> } else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {
>> struct logic_pio_hwaddr *entry = find_io_range(addr);
>>
>> ...
>>
>> (forgetting leX_to_cpu for the moment)
>
> Yes, I suppose this is required to get consistent behavior on arm64,
> which overrides __io_par() but not __io_ar(), with the current code
> the barrier after read is weaker when LOGIC_PIO is enabled than it
> is otherwise.
Ok.
Apart from that, this code is somewhat hidden. I mean, most people would
consider generic IO port accessors come from asm-generic/io.h only,
which is not the case here. Maybe this can be better integrated into
asm-generic/io.h, the only hint today being the logic_pio.h include half
way through the file.
>
> For other architectures, I suppose we would need another indirection
> level, as those can also override the default inb() itself to do something
> other than readb(PCI_IOBASE + addr), and that is not handled
> here either. We can do that if we need LOGIC_PIO on a second
> architecture.
Jiaxun Yang did mention that MIPS may want to move away from its own IO
space management.
Thanks,
John
On 3/3/2020 11:40 AM, Arnd Bergmann wrote:
>> - ret = read##bw(PCI_IOBASE + addr);
>> + __io_pbr();
>> + ret = __raw_read##bw(PCI_IOBASE + addr);
>> + __io_pbr();
> __io_par();
>
Why do we need to change read##bw above?
read##bw already provides strong ordering guarantees across multiple
architectures.
On Fri, Mar 6, 2020 at 4:44 AM Sinan Kaya <[email protected]> wrote:
>
> On 3/3/2020 11:40 AM, Arnd Bergmann wrote:
> >> - ret = read##bw(PCI_IOBASE + addr);
> >> + __io_pbr();
> >> + ret = __raw_read##bw(PCI_IOBASE + addr);
> >> + __io_pbr();
> > __io_par();
> >
>
> Why do we need to change read##bw above?
>
> read##bw already provides strong ordering guarantees across multiple
> architectures.
The exact semantics of inl() and readl() are slightly different, so they
have distinct sets of barriers in the asm-generic/io.h implementation.
For instance, the arm64 architectures defines in_par() as '__iormb(v)',
but defines __io_ar() as a '__rmb()'. Similarly, riscv defines them
as "fence i,ior" and "fence i,r".
You could argue that the definitions are wrong (I have not checked the
history of the definitions), but as long as the inb() in asm-generic/io.h
uses those, the implementation in lib/logic_pio.c uses the same ones
to make the two behave the same way.
Arnd
On 06/03/2020 07:54, Arnd Bergmann wrote:
> On Fri, Mar 6, 2020 at 4:44 AM Sinan Kaya <[email protected]> wrote:
>>
>> On 3/3/2020 11:40 AM, Arnd Bergmann wrote:
>>>> - ret = read##bw(PCI_IOBASE + addr);
>>>> + __io_pbr();
>>>> + ret = __raw_read##bw(PCI_IOBASE + addr);
>>>> + __io_pbr();
>>> __io_par();
>>>
>>
>> Why do we need to change read##bw above?
>>
>> read##bw already provides strong ordering guarantees across multiple
>> architectures.
>
> The exact semantics of inl() and readl() are slightly different, so they
> have distinct sets of barriers in the asm-generic/io.h implementation.
>
> For instance, the arm64 architectures defines in_par() as '__iormb(v)',
> but defines __io_ar() as a '__rmb()'. Similarly, riscv defines them
> as "fence i,ior" and "fence i,r".
>
> You could argue that the definitions are wrong (I have not checked the
> history of the definitions), but as long as the inb() in asm-generic/io.h
> uses those, the implementation in lib/logic_pio.c uses the same ones
> to make the two behave the same way.
>
So the change would look like:
-- a/lib/logic_pio.c
+++ b/lib/logic_pio.c
@@ -229,13 +229,21 @@ unsigned long
logic_pio_trans_cpuaddr(resource_size_t addr)
}
#if defined(CONFIG_INDIRECT_PIO) && defined(PCI_IOBASE)
+
+#define logic_in_to_cpu_b(x) (x)
+#define logic_in_to_cpu_w(x) __le16_to_cpu(x)
+#define logic_in_to_cpu_l(x) __le32_to_cpu(x)
+
#define BUILD_LOGIC_IO(bw, type) \
type logic_in##bw(unsigned long addr) \
{ \
type ret = (type)~0; \
\
if (addr < MMIO_UPPER_LIMIT) { \
- ret = read##bw(PCI_IOBASE + addr); \
+ void __iomem *_addr = PCI_IOBASE + addr; \
+ __io_pbr(); \
+ ret = logic_in_to_cpu_##bw(__raw_read##bw(_addr)); \
+ __io_par(ret); \
} else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {\
struct logic_pio_hwaddr *entry = find_io_rang
We could prob combine the le_to_cpu and __raw_read into a single macro.
John
On Fri, Mar 6, 2020 at 11:40 AM John Garry <[email protected]> wrote:
> On 06/03/2020 07:54, Arnd Bergmann wrote:
> > On Fri, Mar 6, 2020 at 4:44 AM Sinan Kaya <[email protected]> wrote:
> -- a/lib/logic_pio.c
> +++ b/lib/logic_pio.c
> @@ -229,13 +229,21 @@ unsigned long
> logic_pio_trans_cpuaddr(resource_size_t addr)
> }
>
> #if defined(CONFIG_INDIRECT_PIO) && defined(PCI_IOBASE)
> +
> +#define logic_in_to_cpu_b(x) (x)
> +#define logic_in_to_cpu_w(x) __le16_to_cpu(x)
> +#define logic_in_to_cpu_l(x) __le32_to_cpu(x)
> +
> #define BUILD_LOGIC_IO(bw, type) \
> type logic_in##bw(unsigned long addr) \
> { \
> type ret = (type)~0; \
> \
> if (addr < MMIO_UPPER_LIMIT) { \
> - ret = read##bw(PCI_IOBASE + addr); \
> + void __iomem *_addr = PCI_IOBASE + addr; \
> + __io_pbr(); \
> + ret = logic_in_to_cpu_##bw(__raw_read##bw(_addr)); \
> + __io_par(ret); \
> } else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {\
> struct logic_pio_hwaddr *entry = find_io_rang
>
> We could prob combine the le_to_cpu and __raw_read into a single macro.
What is the purpose of splitting out the byteswap rather than leaving the
open-coded rather than __le16_to_cpu(__raw_readw(PCI_IOBASE + addr))?
If this is needed to work across architectures, how about adding
an intermediate __raw_inw() etc in asm-generic/io.h like
#ifndef __raw_inw
#define __raw_inw(a) __raw_readw(PCI_IOBASE + addr));
#endif
#include <linux/logic_pio.h>
#ifndef inw
static inline u16 inw(unsigned long addr)
{
u16 val;
__io_pbr();
val = __le16_to_cpu(__raw_inw(addr));
__io_par(val);
return val;
}
#endif
Arnd
On 06/03/2020 15:16, Arnd Bergmann wrote:
> On Fri, Mar 6, 2020 at 11:40 AM John Garry <[email protected]> wrote:
>> On 06/03/2020 07:54, Arnd Bergmann wrote:
>>> On Fri, Mar 6, 2020 at 4:44 AM Sinan Kaya <[email protected]> wrote:
>> -- a/lib/logic_pio.c
>> +++ b/lib/logic_pio.c
>> @@ -229,13 +229,21 @@ unsigned long
>> logic_pio_trans_cpuaddr(resource_size_t addr)
>> }
>>
>> #if defined(CONFIG_INDIRECT_PIO) && defined(PCI_IOBASE)
>> +
>> +#define logic_in_to_cpu_b(x) (x)
>> +#define logic_in_to_cpu_w(x) __le16_to_cpu(x)
>> +#define logic_in_to_cpu_l(x) __le32_to_cpu(x)
>> +
>> #define BUILD_LOGIC_IO(bw, type) \
Note: The "bw" argument name could be improved to "bwl", since this
macro is used for building inl() also.
>> type logic_in##bw(unsigned long addr) \
>> { \
>> type ret = (type)~0; \
>> \
>> if (addr < MMIO_UPPER_LIMIT) { \
>> - ret = read##bw(PCI_IOBASE + addr); \
>> + void __iomem *_addr = PCI_IOBASE + addr; \
>> + __io_pbr(); \
>> + ret = logic_in_to_cpu_##bw(__raw_read##bw(_addr)); \
>> + __io_par(ret); \
>> } else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {\
>> struct logic_pio_hwaddr *entry = find_io_rang
>>
>> We could prob combine the le_to_cpu and __raw_read into a single macro.
>
> What is the purpose of splitting out the byteswap rather than leaving the
> open-coded rather than __le16_to_cpu(__raw_readw(PCI_IOBASE + addr))?
I'm just copying what is in asm-generic io.h, which uses the 16b and 32b
byteswaps in the w and l variants, respectively.
>
> If this is needed to work across architectures, how about adding
> an intermediate __raw_inw() etc in asm-generic/io.h like
>
> #ifndef __raw_inw
> #define __raw_inw(a) __raw_readw(PCI_IOBASE + addr));
> #endif
>
> #include <linux/logic_pio.h>
>
> #ifndef inw
> static inline u16 inw(unsigned long addr)
> {
> u16 val;
>
> __io_pbr();
> val = __le16_to_cpu(__raw_inw(addr));
> __io_par(val);
> return val;
> }
> #endif
>
The idea is good, but it would be nice if we just somehow use a common
asm-generic io.h definition directly in logic_pio.c, like:
asm-generic io.h:
#ifndef __raw_inw // name?
#define __raw_inw __raw_inw
static inline u16 __raw_inw(unsigned long addr)
{
u16 val;
__io_pbr();
val = __le16_to_cpu(__raw_readw(addr));
__io_par(val);
return val;
}
#endif
#include <linux/logic_pio.h>
#ifndef inw
#define inw __raw_inw
#endif
logic_pio.c:
#define BUILD_LOGIC_IO(bw, type) \
type logic_in##bw(unsigned long addr) \
{ \
type ret = (type)~0; \
\
if (addr < MMIO_UPPER_LIMIT) { \
- ret = read##bw(PCI_IOBASE + addr); \
+ ret = __raw_in##bw(PCI_IOBASE + addr); \
} else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {\
struct logic_pio_hwaddr *entry = find_io_rang
Thanks,
John
On Fri, Mar 6, 2020 at 5:18 PM John Garry <[email protected]> wrote:
> On 06/03/2020 15:16, Arnd Bergmann wrote:
> > On Fri, Mar 6, 2020 at 11:40 AM John Garry <[email protected]> wrote:
> >> On 06/03/2020 07:54, Arnd Bergmann wrote:
> >>> On Fri, Mar 6, 2020 at 4:44 AM Sinan Kaya <[email protected]> wrote:
> >> -- a/lib/logic_pio.c
> >> +++ b/lib/logic_pio.c
> >> @@ -229,13 +229,21 @@ unsigned long
> >> logic_pio_trans_cpuaddr(resource_size_t addr)
> >> }
> >>
> >> #if defined(CONFIG_INDIRECT_PIO) && defined(PCI_IOBASE)
> >> +
> >> +#define logic_in_to_cpu_b(x) (x)
> >> +#define logic_in_to_cpu_w(x) __le16_to_cpu(x)
> >> +#define logic_in_to_cpu_l(x) __le32_to_cpu(x)
> >> +
> >> #define BUILD_LOGIC_IO(bw, type) \
>
> Note: The "bw" argument name could be improved to "bwl", since this
> macro is used for building inl() also.
>
> >> type logic_in##bw(unsigned long addr) \
> >> { \
> >> type ret = (type)~0; \
> >> \
> >> if (addr < MMIO_UPPER_LIMIT) { \
> >> - ret = read##bw(PCI_IOBASE + addr); \
> >> + void __iomem *_addr = PCI_IOBASE + addr; \
> >> + __io_pbr(); \
> >> + ret = logic_in_to_cpu_##bw(__raw_read##bw(_addr)); \
> >> + __io_par(ret); \
> >> } else if (addr >= MMIO_UPPER_LIMIT && addr < IO_SPACE_LIMIT) {\
> >> struct logic_pio_hwaddr *entry = find_io_rang
> >>
> >> We could prob combine the le_to_cpu and __raw_read into a single macro.
> >
> > What is the purpose of splitting out the byteswap rather than leaving the
> > open-coded rather than __le16_to_cpu(__raw_readw(PCI_IOBASE + addr))?
>
> I'm just copying what is in asm-generic io.h, which uses the 16b and 32b
> byteswaps in the w and l variants, respectively.
Sure, but I don't think that needs another macro.
>
> The idea is good, but it would be nice if we just somehow use a common
> asm-generic io.h definition directly in logic_pio.c, like:
>
> asm-generic io.h:
>
> #ifndef __raw_inw // name?
> #define __raw_inw __raw_inw
> static inline u16 __raw_inw(unsigned long addr)
> {
> u16 val;
>
> __io_pbr();
> val = __le16_to_cpu(__raw_readw(addr));
> __io_par(val);
> return val;
> }
> #endif
>
> #include <linux/logic_pio.h>
>
> #ifndef inw
> #define inw __raw_inw
> #endif
Yes, makes sense. Maybe __arch_inw() then? Not great either, but I think
that's better than __raw_inw() because __raw_* would sound like it
mirrors __raw_readl() that lacks the barriers and byteswaps.
Arnd
On 06/03/2020 16:29, Arnd Bergmann wrote:
>> The idea is good, but it would be nice if we just somehow use a common
>> asm-generic io.h definition directly in logic_pio.c, like:
>>
>> asm-generic io.h:
>>
>> #ifndef __raw_inw // name?
>> #define __raw_inw __raw_inw
>> static inline u16 __raw_inw(unsigned long addr)
>> {
>> u16 val;
>>
>> __io_pbr();
>> val = __le16_to_cpu(__raw_readw(addr));
>> __io_par(val);
>> return val;
>> }
>> #endif
>>
>> #include <linux/logic_pio.h>
>>
>> #ifndef inw
>> #define inw __raw_inw
>> #endif
> Yes, makes sense. Maybe __arch_inw() then? Not great either, but I think
> that's better than __raw_inw() because __raw_* would sound like it
> mirrors __raw_readl() that lacks the barriers and byteswaps.
Right, I had the same concern. And maybe the "arch" prefix is
misleading. Just __inw could be ok, and hopefully not conflict with the
arch/arm/mach-* definitions.
Thanks,
John
On 3/6/2020 2:54 AM, Arnd Bergmann wrote:
> The exact semantics of inl() and readl() are slightly different, so they
> have distinct sets of barriers in the asm-generic/io.h implementation.
>
> For instance, the arm64 architectures defines in_par() as '__iormb(v)',
> but defines __io_ar() as a '__rmb()'. Similarly, riscv defines them
> as "fence i,ior" and "fence i,r".
makes sense
On 06/03/2020 16:43, John Garry wrote:
> On 06/03/2020 16:29, Arnd Bergmann wrote:
>>> The idea is good, but it would be nice if we just somehow use a common
>>> asm-generic io.h definition directly in logic_pio.c, like:
>>>
>>> asm-generic io.h:
>>>
>>> #ifndef __raw_inw // name?
>>> #define __raw_inw __raw_inw
>>> static inline u16 __raw_inw(unsigned long addr)
>>> {
>>> u16 val;
>>>
>>> __io_pbr();
>>> val = __le16_to_cpu(__raw_readw(addr));
>>> __io_par(val);
>>> return val;
>>> }
>>> #endif
>>>
>>> #include <linux/logic_pio.h>
>>>
>>> #ifndef inw
>>> #define inw __raw_inw
>>> #endif
>> Yes, makes sense. Maybe __arch_inw() then? Not great either, but I think
>> that's better than __raw_inw() because __raw_* would sound like it
>> mirrors __raw_readl() that lacks the barriers and byteswaps.
>
> Right, I had the same concern. And maybe the "arch" prefix is
> misleading. Just __inw could be ok, and hopefully not conflict with the
> arch/arm/mach-* definitions.
>
I think that it hasn't been mentioned already, but it looks like the
outX methods also need the same treatment, from a7851aa54c.
thanks,
John