2018-04-16 22:03:17

by Sinan Kaya

[permalink] [raw]
Subject: [PATCH 1/2] parisc: use the asm-generic version for writeX()

parisc architecture seems to be mapping writeX() and writeX_relaxed() APIs
to __raw_writeX() API.

__raw_writeX() API doesn't provide any kind of ordering guarantees.
commit 755bd04aaf4b ("io: define stronger ordering for the default writeX()
implementation") changed asm-generic implementation to use a more
conservative approach towards the writeX() API.

Drop the arch specific version and rely on the asm-generic version for
parisc since parisc version doesn't seem to do anything special with these
macros.

Signed-off-by: Sinan Kaya <[email protected]>
---
arch/parisc/include/asm/io.h | 27 ---------------------------
1 file changed, 27 deletions(-)

diff --git a/arch/parisc/include/asm/io.h b/arch/parisc/include/asm/io.h
index afe493b..ef04864 100644
--- a/arch/parisc/include/asm/io.h
+++ b/arch/parisc/include/asm/io.h
@@ -194,41 +194,14 @@ static inline unsigned long long readq(const volatile void __iomem *addr)
return le64_to_cpu((__le64 __force) __raw_readq(addr));
}

-static inline void writeb(unsigned char b, volatile void __iomem *addr)
-{
- __raw_writeb(b, addr);
-}
-static inline void writew(unsigned short w, volatile void __iomem *addr)
-{
- __raw_writew((__u16 __force) cpu_to_le16(w), addr);
-}
-static inline void writel(unsigned int l, volatile void __iomem *addr)
-{
- __raw_writel((__u32 __force) cpu_to_le32(l), addr);
-}
-static inline void writeq(unsigned long long q, volatile void __iomem *addr)
-{
- __raw_writeq((__u64 __force) cpu_to_le64(q), addr);
-}
-
#define readb readb
#define readw readw
#define readl readl
#define readq readq
-#define writeb writeb
-#define writew writew
-#define writel writel
-#define writeq writeq
-
#define readb_relaxed(addr) readb(addr)
#define readw_relaxed(addr) readw(addr)
#define readl_relaxed(addr) readl(addr)
#define readq_relaxed(addr) readq(addr)
-#define writeb_relaxed(b, addr) writeb(b, addr)
-#define writew_relaxed(w, addr) writew(w, addr)
-#define writel_relaxed(l, addr) writel(l, addr)
-#define writeq_relaxed(q, addr) writeq(q, addr)
-
#define mmiowb() do { } while (0)

void memset_io(volatile void __iomem *addr, unsigned char val, int count);
--
2.7.4



2018-04-16 22:03:58

by Sinan Kaya

[permalink] [raw]
Subject: [PATCH 2/2] parisc: use the asm-generic version for readX()

parisc architecture seems to be mapping readX() and readX_relaxed() APIs
to __raw_readX() API.

__raw_readX() API doesn't provide any kind of ordering guarantees.
commit 032d59e1cde9 ("io: define stronger ordering for the default readX()
implementation") changed asm-generic implementation to use a more
conservative approach towards the readX() API.

Drop the arch specific version and rely on the asm-generic version for
parisc since it doesn't seem to do anything special with these macros.

Signed-off-by: Sinan Kaya <[email protected]>
---
arch/parisc/include/asm/io.h | 25 -------------------------
1 file changed, 25 deletions(-)

diff --git a/arch/parisc/include/asm/io.h b/arch/parisc/include/asm/io.h
index ef04864..7e60642 100644
--- a/arch/parisc/include/asm/io.h
+++ b/arch/parisc/include/asm/io.h
@@ -177,31 +177,6 @@ static inline void __raw_writeq(unsigned long long b, volatile void __iomem *add
*(volatile unsigned long long __force *) addr = b;
}

-static inline unsigned char readb(const volatile void __iomem *addr)
-{
- return __raw_readb(addr);
-}
-static inline unsigned short readw(const volatile void __iomem *addr)
-{
- return le16_to_cpu((__le16 __force) __raw_readw(addr));
-}
-static inline unsigned int readl(const volatile void __iomem *addr)
-{
- return le32_to_cpu((__le32 __force) __raw_readl(addr));
-}
-static inline unsigned long long readq(const volatile void __iomem *addr)
-{
- return le64_to_cpu((__le64 __force) __raw_readq(addr));
-}
-
-#define readb readb
-#define readw readw
-#define readl readl
-#define readq readq
-#define readb_relaxed(addr) readb(addr)
-#define readw_relaxed(addr) readw(addr)
-#define readl_relaxed(addr) readl(addr)
-#define readq_relaxed(addr) readq(addr)
#define mmiowb() do { } while (0)

void memset_io(volatile void __iomem *addr, unsigned char val, int count);
--
2.7.4


2018-04-16 23:18:19

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH 1/2] parisc: use the asm-generic version for writeX()

On 4/16/2018 7:09 PM, John David Anglin wrote:
> On 2018-04-16 6:01 PM, Sinan Kaya wrote:
>> parisc architecture seems to be mapping writeX() and writeX_relaxed() APIs
>> to __raw_writeX() API.
>>
>> __raw_writeX() API doesn't provide any kind of ordering guarantees.
>> commit 755bd04aaf4b ("io: define stronger ordering for the default writeX()
>> implementation") changed asm-generic implementation to use a more
>> conservative approach towards the writeX() API.
>>
>> Drop the arch specific version and rely on the asm-generic version for
>> parisc since parisc version doesn't seem to do anything special with these
>> macros.
>   HOSTLD  scripts/mod/modpost
> In file included from ./arch/parisc/include/asm/hardirq.h:13:0,
>                  from ./include/linux/hardirq.h:9,
>                  from arch/parisc/kernel/asm-offsets.c:34:
> ./include/linux/irq.h: In function 'irq_reg_writel':
> ./include/linux/irq.h:1114:3: error: implicit declaration of function 'writel' [-Werror=implicit-function-declaration]
>    writel(val, gc->reg_base + reg_offset);
>    ^~~~~~
> ./include/linux/irq.h: In function 'irq_reg_readl':
> ./include/linux/irq.h:1123:10: error: implicit declaration of function 'readl' [-Werror=implicit-function-declaration]
>    return readl(gc->reg_base + reg_offset);
>           ^~~~~
> cc1: some warnings being treated as errors
> make[1]: *** [Kbuild:58: arch/parisc/kernel/asm-offsets.s] Error 1
>
> Dave
>

Thanks for testing. Can you add this on top and see if it helps?

--- a/arch/parisc/include/asm/io.h
+++ b/arch/parisc/include/asm/io.h
@@ -259,6 +259,7 @@ extern void outsl (unsigned long port, const void *src, unsigned long count);
* value for either 32 or 64 bit mode */
#define F_EXTEND(x) ((unsigned long)((x) | (0xffffffff00000000ULL)))

+#include <asm-generic/io.h>
#include <asm-generic/iomap.h>


--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

2018-04-16 23:19:17

by John David Anglin

[permalink] [raw]
Subject: Re: [PATCH 1/2] parisc: use the asm-generic version for writeX()

On 2018-04-16 6:01 PM, Sinan Kaya wrote:
> parisc architecture seems to be mapping writeX() and writeX_relaxed() APIs
> to __raw_writeX() API.
>
> __raw_writeX() API doesn't provide any kind of ordering guarantees.
> commit 755bd04aaf4b ("io: define stronger ordering for the default writeX()
> implementation") changed asm-generic implementation to use a more
> conservative approach towards the writeX() API.
>
> Drop the arch specific version and rely on the asm-generic version for
> parisc since parisc version doesn't seem to do anything special with these
> macros.
  HOSTLD  scripts/mod/modpost
In file included from ./arch/parisc/include/asm/hardirq.h:13:0,
                 from ./include/linux/hardirq.h:9,
                 from arch/parisc/kernel/asm-offsets.c:34:
./include/linux/irq.h: In function 'irq_reg_writel':
./include/linux/irq.h:1114:3: error: implicit declaration of function
'writel' [-Werror=implicit-function-declaration]
   writel(val, gc->reg_base + reg_offset);
   ^~~~~~
./include/linux/irq.h: In function 'irq_reg_readl':
./include/linux/irq.h:1123:10: error: implicit declaration of function
'readl' [-Werror=implicit-function-declaration]
   return readl(gc->reg_base + reg_offset);
          ^~~~~
cc1: some warnings being treated as errors
make[1]: *** [Kbuild:58: arch/parisc/kernel/asm-offsets.s] Error 1

Dave

--
John David Anglin [email protected]


2018-04-16 23:39:28

by John David Anglin

[permalink] [raw]
Subject: Re: [PATCH 1/2] parisc: use the asm-generic version for writeX()

On 2018-04-16 7:15 PM, Sinan Kaya wrote:
> Thanks for testing. Can you add this on top and see if it helps?
>
> --- a/arch/parisc/include/asm/io.h
> +++ b/arch/parisc/include/asm/io.h
> @@ -259,6 +259,7 @@ extern void outsl (unsigned long port, const void *src, unsigned long count);
> * value for either 32 or 64 bit mode */
> #define F_EXTEND(x) ((unsigned long)((x) | (0xffffffff00000000ULL)))
>
> +#include <asm-generic/io.h>
> #include <asm-generic/iomap.h>
Still lots of problems.

Dave

--
John David Anglin [email protected]


Attachments:
io.log (37.77 kB)

2018-04-16 23:46:09

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH 1/2] parisc: use the asm-generic version for writeX()

On 4/16/2018 7:37 PM, John David Anglin wrote:
>>   +#include <asm-generic/io.h>
>>   #include <asm-generic/iomap.h>
> Still lots of problems.
>
> Dave
>
> -- 
> John David Anglin  [email protected]
>
>
> io.log
>
>
> CC arch/parisc/kernel/asm-offsets.s
> In file included from ./arch/parisc/include/asm/io.h:262:0,
> from ./include/linux/io.h:25,
> from ./include/linux/irq.h:25,
> from ./arch/parisc/include/asm/hardirq.h:13,
> from ./include/linux/hardirq.h:9,
> from arch/parisc/kernel/asm-offsets.c:34:
> ./include/asm-generic/io.h:37:21: error: redefinition of '__raw_readb'
> #define __raw_readb __raw_readb
> ^

This one is easy to fix:

https://elixir.bootlin.com/linux/latest/source/arch/parisc/include/asm/io.h#L146

Remove 146..179.

--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

2018-04-16 23:49:54

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH 1/2] parisc: use the asm-generic version for writeX()

On 4/16/2018 7:44 PM, Sinan Kaya wrote:
>> John David Anglin  [email protected]
>>
>>
>> io.log
>>
>>
>> CC arch/parisc/kernel/asm-offsets.s
>> In file included from ./arch/parisc/include/asm/io.h:262:0,
>> from ./include/linux/io.h:25,
>> from ./include/linux/irq.h:25,
>> from ./arch/parisc/include/asm/hardirq.h:13,
>> from ./include/linux/hardirq.h:9,
>> from arch/parisc/kernel/asm-offsets.c:34:
>> ./include/asm-generic/io.h:37:21: error: redefinition of '__raw_readb'
>> #define __raw_readb __raw_readb
>> ^
> This one is easy to fix:
>
> https://elixir.bootlin.com/linux/latest/source/arch/parisc/include/asm/io.h#L146
>
> Remove 146..179.

I'll try to get a hold of a cross compiler and re-test. The above is a hack.
I need to do a better job on this.

Thanks for the help. I'll post V2 when it is ready.

--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

2018-04-17 04:13:36

by Sinan Kaya

[permalink] [raw]
Subject: Re: [PATCH 1/2] parisc: use the asm-generic version for writeX()

On 4/16/2018 7:48 PM, Sinan Kaya wrote:
> On 4/16/2018 7:44 PM, Sinan Kaya wrote:
>>> John David Anglin  [email protected]
>>>
>>>
>>> io.log
>>>
>>>
>>> CC arch/parisc/kernel/asm-offsets.s
>>> In file included from ./arch/parisc/include/asm/io.h:262:0,
>>> from ./include/linux/io.h:25,
>>> from ./include/linux/irq.h:25,
>>> from ./arch/parisc/include/asm/hardirq.h:13,
>>> from ./include/linux/hardirq.h:9,
>>> from arch/parisc/kernel/asm-offsets.c:34:
>>> ./include/asm-generic/io.h:37:21: error: redefinition of '__raw_readb'
>>> #define __raw_readb __raw_readb
>>> ^
>> This one is easy to fix:
>>
>> https://elixir.bootlin.com/linux/latest/source/arch/parisc/include/asm/io.h#L146
>>
>> Remove 146..179.
>
> I'll try to get a hold of a cross compiler and re-test. The above is a hack.
> I need to do a better job on this.
>
> Thanks for the help. I'll post V2 when it is ready.
>

Converting this to asm-generic turned out to be a much bigger task. I placed
compiler barriers instead to existing version and posted as follows:

[PATCH v2 1/2] parisc: define stronger ordering for the default writeX()
[PATCH v2 2/2] parisc: define stronger ordering for the default readX()

--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

2018-04-17 04:33:33

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 1/2] parisc: use the asm-generic version for writeX()

Hi Sinan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on hp-parisc/for-next]
[also build test ERROR on v4.17-rc1 next-20180416]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Sinan-Kaya/parisc-use-the-asm-generic-version-for-writeX/20180417-103119
base: https://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git for-next
config: parisc-c3000_defconfig (attached as .config)
compiler: hppa-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=parisc

All errors (new ones prefixed by >>):

In file included from arch/parisc/include/asm/hardirq.h:13:0,
from include/linux/hardirq.h:9,
from arch/parisc/kernel/asm-offsets.c:34:
include/linux/irq.h: In function 'irq_reg_writel':
>> include/linux/irq.h:1126:3: error: implicit declaration of function 'writel'; did you mean 'iowrite8'? [-Werror=implicit-function-declaration]
writel(val, gc->reg_base + reg_offset);
^~~~~~
iowrite8
cc1: some warnings being treated as errors
make[2]: *** [arch/parisc/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2

vim +1126 include/linux/irq.h

7d828062 Thomas Gleixner 2011-04-03 1109
ebf9ff75 Boris Brezillon 2016-09-13 1110 /*
ebf9ff75 Boris Brezillon 2016-09-13 1111 * The irqsave variants are for usage in non interrupt code. Do not use
ebf9ff75 Boris Brezillon 2016-09-13 1112 * them in irq_chip callbacks. Use irq_gc_lock() instead.
ebf9ff75 Boris Brezillon 2016-09-13 1113 */
ebf9ff75 Boris Brezillon 2016-09-13 1114 #define irq_gc_lock_irqsave(gc, flags) \
ebf9ff75 Boris Brezillon 2016-09-13 1115 raw_spin_lock_irqsave(&(gc)->lock, flags)
ebf9ff75 Boris Brezillon 2016-09-13 1116
ebf9ff75 Boris Brezillon 2016-09-13 1117 #define irq_gc_unlock_irqrestore(gc, flags) \
ebf9ff75 Boris Brezillon 2016-09-13 1118 raw_spin_unlock_irqrestore(&(gc)->lock, flags)
ebf9ff75 Boris Brezillon 2016-09-13 1119
332fd7c4 Kevin Cernekee 2014-11-06 1120 static inline void irq_reg_writel(struct irq_chip_generic *gc,
332fd7c4 Kevin Cernekee 2014-11-06 1121 u32 val, int reg_offset)
332fd7c4 Kevin Cernekee 2014-11-06 1122 {
2b280376 Kevin Cernekee 2014-11-06 1123 if (gc->reg_writel)
2b280376 Kevin Cernekee 2014-11-06 1124 gc->reg_writel(val, gc->reg_base + reg_offset);
2b280376 Kevin Cernekee 2014-11-06 1125 else
332fd7c4 Kevin Cernekee 2014-11-06 @1126 writel(val, gc->reg_base + reg_offset);
332fd7c4 Kevin Cernekee 2014-11-06 1127 }
332fd7c4 Kevin Cernekee 2014-11-06 1128

:::::: The code at line 1126 was first introduced by commit
:::::: 332fd7c4fef5f3b166e93decb07fd69eb24f7998 genirq: Generic chip: Change irq_reg_{readl,writel} arguments

:::::: TO: Kevin Cernekee <[email protected]>
:::::: CC: Jason Cooper <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.44 kB)
.config.gz (13.97 kB)
Download all attachments

2018-04-17 04:59:51

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH 2/2] parisc: use the asm-generic version for readX()

Hi Sinan,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on hp-parisc/for-next]
[also build test ERROR on v4.17-rc1 next-20180416]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url: https://github.com/0day-ci/linux/commits/Sinan-Kaya/parisc-use-the-asm-generic-version-for-writeX/20180417-103119
base: https://git.kernel.org/pub/scm/linux/kernel/git/deller/parisc-linux.git for-next
config: parisc-c3000_defconfig (attached as .config)
compiler: hppa-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=parisc

All errors (new ones prefixed by >>):

In file included from arch/parisc/include/asm/hardirq.h:13:0,
from include/linux/hardirq.h:9,
from arch/parisc/kernel/asm-offsets.c:34:
include/linux/irq.h: In function 'irq_reg_writel':
include/linux/irq.h:1126:3: error: implicit declaration of function 'writel'; did you mean 'iowrite8'? [-Werror=implicit-function-declaration]
writel(val, gc->reg_base + reg_offset);
^~~~~~
iowrite8
include/linux/irq.h: In function 'irq_reg_readl':
>> include/linux/irq.h:1135:10: error: implicit declaration of function 'readl'; did you mean 'ioread8'? [-Werror=implicit-function-declaration]
return readl(gc->reg_base + reg_offset);
^~~~~
ioread8
cc1: some warnings being treated as errors
make[2]: *** [arch/parisc/kernel/asm-offsets.s] Error 1
make[2]: Target '__build' not remade because of errors.
make[1]: *** [prepare0] Error 2
make[1]: Target 'prepare' not remade because of errors.
make: *** [sub-make] Error 2

vim +1135 include/linux/irq.h

7d828062 Thomas Gleixner 2011-04-03 1109
ebf9ff75 Boris Brezillon 2016-09-13 1110 /*
ebf9ff75 Boris Brezillon 2016-09-13 1111 * The irqsave variants are for usage in non interrupt code. Do not use
ebf9ff75 Boris Brezillon 2016-09-13 1112 * them in irq_chip callbacks. Use irq_gc_lock() instead.
ebf9ff75 Boris Brezillon 2016-09-13 1113 */
ebf9ff75 Boris Brezillon 2016-09-13 1114 #define irq_gc_lock_irqsave(gc, flags) \
ebf9ff75 Boris Brezillon 2016-09-13 1115 raw_spin_lock_irqsave(&(gc)->lock, flags)
ebf9ff75 Boris Brezillon 2016-09-13 1116
ebf9ff75 Boris Brezillon 2016-09-13 1117 #define irq_gc_unlock_irqrestore(gc, flags) \
ebf9ff75 Boris Brezillon 2016-09-13 1118 raw_spin_unlock_irqrestore(&(gc)->lock, flags)
ebf9ff75 Boris Brezillon 2016-09-13 1119
332fd7c4 Kevin Cernekee 2014-11-06 1120 static inline void irq_reg_writel(struct irq_chip_generic *gc,
332fd7c4 Kevin Cernekee 2014-11-06 1121 u32 val, int reg_offset)
332fd7c4 Kevin Cernekee 2014-11-06 1122 {
2b280376 Kevin Cernekee 2014-11-06 1123 if (gc->reg_writel)
2b280376 Kevin Cernekee 2014-11-06 1124 gc->reg_writel(val, gc->reg_base + reg_offset);
2b280376 Kevin Cernekee 2014-11-06 1125 else
332fd7c4 Kevin Cernekee 2014-11-06 @1126 writel(val, gc->reg_base + reg_offset);
332fd7c4 Kevin Cernekee 2014-11-06 1127 }
332fd7c4 Kevin Cernekee 2014-11-06 1128
332fd7c4 Kevin Cernekee 2014-11-06 1129 static inline u32 irq_reg_readl(struct irq_chip_generic *gc,
332fd7c4 Kevin Cernekee 2014-11-06 1130 int reg_offset)
332fd7c4 Kevin Cernekee 2014-11-06 1131 {
2b280376 Kevin Cernekee 2014-11-06 1132 if (gc->reg_readl)
2b280376 Kevin Cernekee 2014-11-06 1133 return gc->reg_readl(gc->reg_base + reg_offset);
2b280376 Kevin Cernekee 2014-11-06 1134 else
332fd7c4 Kevin Cernekee 2014-11-06 @1135 return readl(gc->reg_base + reg_offset);
332fd7c4 Kevin Cernekee 2014-11-06 1136 }
332fd7c4 Kevin Cernekee 2014-11-06 1137

:::::: The code at line 1135 was first introduced by commit
:::::: 332fd7c4fef5f3b166e93decb07fd69eb24f7998 genirq: Generic chip: Change irq_reg_{readl,writel} arguments

:::::: TO: Kevin Cernekee <[email protected]>
:::::: CC: Jason Cooper <[email protected]>

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (4.31 kB)
.config.gz (13.97 kB)
Download all attachments