* [PATCH 00/24] ARM: readl/writel conversion fallout @ 2012-09-14 21:34 Arnd Bergmann 2012-09-14 21:34 ` [PATCH 22/24] scsi: eesox: use __iomem pointers for MMIO Arnd Bergmann 0 siblings, 1 reply; 8+ messages in thread From: Arnd Bergmann @ 2012-09-14 21:34 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-fbdev, linux-sh, Tony Lindgren, Linus Walleij, Will Deacon, Wolfram Sang, Roland Stigge, Kukjin Kim, linux-scsi, Nicolas Pitre, Magnus Damm, linux-input, Russell King, David Brown, Arnd Bergmann, Nicolas Ferre, spear-devel, Florian Tobias Schandinat, Shiraz Hashim, Simon Horman, Shawn Guo, Ryan Mallon, Greg Kroah-Hartman, Dmitry Torokhov, linux-kernel Linux-next currently contains 195bbcac "ARM: 7500/1: io: avoid writeback addressing modes for __raw_ accessors" from Will Deacon. While this patch does a number of very useful things, it also causes a lot of new build warnings in ARM specific code that was passing an integer as the address into readl/writel or similar functions. Most architectures have never allowed this, and my feeling is that it's time for ARM to do the same, so instead of changing the readl/writel behavior back, we should fix all code that uses incorrect addressing. A few people have already posted platform specific patches, this should take care of the rest that is needed for all defconfig builds. The majority of the warnings was in the shmobile platform, so those patches are by far the largest. I'm happy to have these patches go through individual subsystem maintainers, especially for the device drivers and those that have conflicts with other changes (ixp4xx, integrator, shmobile), but I can carry the reamining ones in one branch for arm-soc. Right now, the whole set is available in the testing/__iomem branch. Arnd Cc: "David S. Miller" <davem@davemloft.net> Cc: "James E.J. Bottomley" <JBottomley@parallels.com> Cc: David Brown <davidb@codeaurora.org> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: Florian Tobias Schandinat <FlorianSchandinat@gmx.de> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Cc: Hartley Sweeten <hsweeten@visionengravers.com> Cc: Imre Kaloz <kaloz@openwrt.org> Cc: Krzysztof Halasa <khc@pm.waw.pl> Cc: Kukjin Kim <kgene.kim@samsung.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Magnus Damm <magnus.damm@gmail.com> Cc: Nicolas Ferre <nicolas.ferre@atmel.com> Cc: Paul Mundt <lethal@linux-sh.org> Cc: Roland Stigge <stigge@antcom.de> Cc: Ryan Mallon <rmallon@gmail.com> Cc: Sascha Hauer <kernel@pengutronix.de> Cc: Shawn Guo <shawn.guo@linaro.org> Cc: Shiraz Hashim <shiraz.hashim@st.com> Cc: Simon Horman <horms@verge.net.au> Cc: Tony Lindgren <tony@atomide.com> Cc: Wolfram Sang <w.sang@pengutronix.de> Cc: STEricsson_nomadik_linux@list.st.com> Cc: linux-fbdev@vger.kernel.org Cc: linux-input@vger.kernel.org Cc: linux-scsi@vger.kernel.org Cc: linux-sh@vger.kernel.org Cc: linux-sh@vger.kernel.org Cc: netdev@vger.kernel.org Cc: spear-devel@list.st.com Arnd Bergmann (24): ARM: shmobile: use __iomem pointers for MMIO ARM: at91: use __iomem pointers for MMIO ARM: ebsa: use __iomem pointers for MMIO ARM: ep93xx: use _iomem pointers for MMIO ARM: imx: use __iomem pointers for MMIO ARM: integrator: use __iomem pointers for MMIO ARM: iop13xx: use __iomem pointers for MMIO ARM: iop32x: use __iomem pointers for MMIO ARM: ixp4xx: use __iomem pointers for MMIO ARM: ks8695: use __iomem pointers for MMIO ARM: lpc32xx: use __iomem pointers for MMIO ARM: msm: use __iomem pointers for MMIO ARM: nomadik: use __iomem pointers for MMIO ARM: prima2: use __iomem pointers for MMIO ARM: sa1100: use __iomem pointers for MMIO ARM: spear13xx: use __iomem pointers for MMIO ARM: OMAP: use __iomem pointers for MMIO ARM: samsung: use __iomem pointers for MMIO sh: use __iomem pointers for MMIO input: rpcmouse: use __iomem pointers for MMIO serial: ks8695: use __iomem pointers for MMIO scsi: eesox: use __iomem pointers for MMIO video: da8xx-fb: use __iomem pointers for MMIO net: seeq: use __iomem pointers for MMIO arch/arm/mach-at91/at91x40.c | 2 +- arch/arm/mach-at91/at91x40_time.c | 4 +- arch/arm/mach-at91/include/mach/hardware.h | 4 +- arch/arm/mach-at91/include/mach/uncompress.h | 6 +- arch/arm/mach-at91/setup.c | 4 +- arch/arm/mach-ebsa110/core.c | 8 +-- arch/arm/mach-ebsa110/core.h | 12 ++-- arch/arm/mach-ep93xx/include/mach/ts72xx.h | 10 ++-- arch/arm/mach-ep93xx/ts72xx.c | 10 ++-- arch/arm/mach-imx/mach-armadillo5x0.c | 2 +- arch/arm/mach-imx/mach-kzm_arm11_01.c | 4 +- arch/arm/mach-imx/mach-mx31ads.c | 2 +- arch/arm/mach-imx/mach-mx31lite.c | 2 +- arch/arm/mach-integrator/core.c | 4 +- arch/arm/mach-integrator/cpu.c | 8 +-- arch/arm/mach-integrator/integrator_ap.c | 12 ++-- arch/arm/mach-integrator/integrator_cp.c | 6 +- arch/arm/mach-integrator/pci_v3.c | 12 ++-- arch/arm/mach-iop13xx/include/mach/iop13xx.h | 20 +++---- arch/arm/mach-iop13xx/include/mach/memory.h | 14 ++--- arch/arm/mach-iop13xx/io.c | 12 ++-- arch/arm/mach-iop13xx/pci.c | 16 +++--- arch/arm/mach-iop13xx/pci.h | 4 +- arch/arm/mach-iop13xx/setup.c | 10 ++-- arch/arm/mach-iop32x/glantank.c | 2 +- arch/arm/mach-ixp4xx/common.c | 8 +-- arch/arm/mach-ixp4xx/include/mach/cpu.h | 5 +- arch/arm/mach-ixp4xx/include/mach/ixp4xx-regs.h | 10 ++-- arch/arm/mach-ks8695/cpu.c | 2 +- arch/arm/mach-ks8695/include/mach/hardware.h | 2 +- arch/arm/mach-ks8695/include/mach/uncompress.h | 6 +- arch/arm/mach-lpc32xx/common.c | 8 +-- arch/arm/mach-lpc32xx/include/mach/hardware.h | 2 +- arch/arm/mach-msm/smd.c | 19 +++--- arch/arm/mach-nomadik/board-nhk8815.c | 3 +- arch/arm/mach-nomadik/include/mach/hardware.h | 2 +- arch/arm/mach-nomadik/include/mach/uncompress.h | 8 +-- arch/arm/mach-prima2/include/mach/uncompress.h | 4 +- arch/arm/mach-sa1100/include/mach/simpad.h | 2 +- arch/arm/mach-sa1100/simpad.c | 2 +- arch/arm/mach-shmobile/board-ap4evb.c | 12 ++-- arch/arm/mach-shmobile/board-armadillo800eva.c | 6 +- arch/arm/mach-shmobile/board-bonito.c | 8 +-- arch/arm/mach-shmobile/board-g3evm.c | 12 ++-- arch/arm/mach-shmobile/board-g4evm.c | 30 +++++----- arch/arm/mach-shmobile/board-kzm9g.c | 8 +-- arch/arm/mach-shmobile/board-mackerel.c | 22 +++---- arch/arm/mach-shmobile/clock-r8a7740.c | 46 +++++++-------- arch/arm/mach-shmobile/clock-sh7367.c | 44 +++++++------- arch/arm/mach-shmobile/clock-sh7372.c | 60 +++++++++---------- arch/arm/mach-shmobile/clock-sh7377.c | 50 ++++++++-------- arch/arm/mach-shmobile/clock-sh73a0.c | 70 +++++++++++------------ arch/arm/mach-shmobile/include/mach/gpio.h | 6 +- arch/arm/mach-shmobile/intc-r8a7779.c | 14 ++--- arch/arm/mach-shmobile/intc-sh7372.c | 27 +++++---- arch/arm/mach-shmobile/intc-sh73a0.c | 20 ++++--- arch/arm/mach-shmobile/pm-rmobile.c | 6 +- arch/arm/mach-shmobile/pm-sh7372.c | 57 +++++++++--------- arch/arm/mach-shmobile/setup-sh7367.c | 2 +- arch/arm/mach-shmobile/setup-sh7377.c | 2 +- arch/arm/mach-shmobile/setup-sh73a0.c | 2 +- arch/arm/mach-spear13xx/include/mach/spear.h | 14 ++--- arch/arm/mach-spear13xx/spear13xx.c | 6 +- arch/arm/plat-mxc/include/mach/mx31.h | 6 +- arch/arm/plat-omap/include/plat/hardware.h | 18 +++--- arch/arm/plat-samsung/s5p-irq-gpioint.c | 4 +- drivers/input/mouse/rpcmouse.c | 2 +- drivers/net/ethernet/seeq/ether3.c | 4 +- drivers/scsi/arm/eesox.c | 2 +- drivers/sh/intc/access.c | 56 +++++++++--------- drivers/sh/intc/chip.c | 8 +-- drivers/sh/intc/core.c | 6 +- drivers/sh/intc/handle.c | 6 +- drivers/sh/intc/internals.h | 18 +++--- drivers/sh/intc/virq.c | 3 +- drivers/tty/serial/serial_ks8695.c | 4 +- drivers/video/da8xx-fb.c | 8 +-- include/linux/serial_sci.h | 2 +- include/linux/sh_clk.h | 4 +- 79 files changed, 477 insertions(+), 471 deletions(-) -- 1.7.10 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 22/24] scsi: eesox: use __iomem pointers for MMIO 2012-09-14 21:34 [PATCH 00/24] ARM: readl/writel conversion fallout Arnd Bergmann @ 2012-09-14 21:34 ` Arnd Bergmann 2012-09-14 23:27 ` Russell King - ARM Linux 0 siblings, 1 reply; 8+ messages in thread From: Arnd Bergmann @ 2012-09-14 21:34 UTC (permalink / raw) To: linux-arm-kernel Cc: linux-kernel, Will Deacon, Russell King, Nicolas Pitre, Arnd Bergmann, James E.J. Bottomley, linux-scsi ARM is moving to stricter checks on readl/write functions, so we need to use the correct types everywhere. Cc: "James E.J. Bottomley" <JBottomley@parallels.com> Cc: linux-scsi@vger.kernel.org Signed-off-by: Arnd Bergmann <arnd@arndb.de> --- drivers/scsi/arm/eesox.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/arm/eesox.c b/drivers/scsi/arm/eesox.c index edfd12b..968d083 100644 --- a/drivers/scsi/arm/eesox.c +++ b/drivers/scsi/arm/eesox.c @@ -273,7 +273,7 @@ static void eesoxscsi_buffer_out(void *buf, int length, void __iomem *base) { const void __iomem *reg_fas = base + EESOX_FAS216_OFFSET; const void __iomem *reg_dmastat = base + EESOX_DMASTAT; - const void __iomem *reg_dmadata = base + EESOX_DMADATA; + void __iomem *reg_dmadata = base + EESOX_DMADATA; do { unsigned int status; -- 1.7.10 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 22/24] scsi: eesox: use __iomem pointers for MMIO 2012-09-14 21:34 ` [PATCH 22/24] scsi: eesox: use __iomem pointers for MMIO Arnd Bergmann @ 2012-09-14 23:27 ` Russell King - ARM Linux 2012-09-15 8:00 ` Arnd Bergmann 0 siblings, 1 reply; 8+ messages in thread From: Russell King - ARM Linux @ 2012-09-14 23:27 UTC (permalink / raw) To: Arnd Bergmann Cc: linux-arm-kernel, linux-kernel, Will Deacon, Nicolas Pitre, James E.J. Bottomley, linux-scsi On Fri, Sep 14, 2012 at 11:34:50PM +0200, Arnd Bergmann wrote: > ARM is moving to stricter checks on readl/write functions, > so we need to use the correct types everywhere. There's nothing wrong with const iomem pointers. If you think otherwise, patch x86 not to use const in its accessor implementation and watch the reaction: #define build_mmio_read(name, size, type, reg, barrier) \ static inline type name(const volatile void __iomem *addr) \ { type ret; asm volatile("mov" size " %1,%0":reg (ret) \ :"m" (*(volatile type __force *)addr) barrier); return ret; } build_mmio_read(readb, "b", unsigned char, "=q", :"memory") build_mmio_read(readw, "w", unsigned short, "=r", :"memory") build_mmio_read(readl, "l", unsigned int, "=r", :"memory") ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 22/24] scsi: eesox: use __iomem pointers for MMIO 2012-09-14 23:27 ` Russell King - ARM Linux @ 2012-09-15 8:00 ` Arnd Bergmann 2012-09-15 8:57 ` Russell King - ARM Linux 0 siblings, 1 reply; 8+ messages in thread From: Arnd Bergmann @ 2012-09-15 8:00 UTC (permalink / raw) To: Russell King - ARM Linux Cc: linux-arm-kernel, linux-kernel, Will Deacon, Nicolas Pitre, James E.J. Bottomley, linux-scsi On Friday 14 September 2012, Russell King - ARM Linux wrote: > On Fri, Sep 14, 2012 at 11:34:50PM +0200, Arnd Bergmann wrote: > > ARM is moving to stricter checks on readl/write functions, > > so we need to use the correct types everywhere. > > There's nothing wrong with const iomem pointers. If you think > otherwise, patch x86 not to use const in its accessor implementation > and watch the reaction: > > #define build_mmio_read(name, size, type, reg, barrier) \ > static inline type name(const volatile void __iomem *addr) \ > { type ret; asm volatile("mov" size " %1,%0":reg (ret) \ > :"m" (*(volatile type __force *)addr) barrier); return ret; } > > build_mmio_read(readb, "b", unsigned char, "=q", :"memory") > build_mmio_read(readw, "w", unsigned short, "=r", :"memory") > build_mmio_read(readl, "l", unsigned int, "=r", :"memory") Ok, fair enough. Can you fold the patch below into "ARM: 7500/1: io: avoid writeback addressing modes for __raw_ accessors", or apply on top then? Signed-off-by: Arnd Bergmann <arnd@arndb.de> diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h index 35c1ed8..4c5f708 100644 --- a/arch/arm/include/asm/io.h +++ b/arch/arm/include/asm/io.h @@ -39,9 +39,9 @@ * Generic IO read/write. These perform native-endian accesses. Note * that some architectures will want to re-define __raw_{read,write}w. */ -extern void __raw_writesb(void __iomem *addr, const void *data, int bytelen); -extern void __raw_writesw(void __iomem *addr, const void *data, int wordlen); -extern void __raw_writesl(void __iomem *addr, const void *data, int longlen); +extern void __raw_writesb(const void __iomem *addr, const void *data, int bytelen); +extern void __raw_writesw(const void __iomem *addr, const void *data, int wordlen); +extern void __raw_writesl(const void __iomem *addr, const void *data, int longlen); extern void __raw_readsb(const void __iomem *addr, void *data, int bytelen); extern void __raw_readsw(const void __iomem *addr, void *data, int wordlen); @@ -61,7 +61,7 @@ extern void __raw_readsl(const void __iomem *addr, void *data, int longlen); * writeback addressing modes as these incur a significant performance * overhead (the address generation must be emulated in software). */ -static inline void __raw_writew(u16 val, volatile void __iomem *addr) +static inline void __raw_writew(u16 val, const volatile void __iomem *addr) { asm volatile("strh %1, %0" : "+Qo" (*(volatile u16 __force *)addr) @@ -78,14 +78,14 @@ static inline u16 __raw_readw(const volatile void __iomem *addr) } #endif -static inline void __raw_writeb(u8 val, volatile void __iomem *addr) +static inline void __raw_writeb(u8 val, const volatile void __iomem *addr) { asm volatile("strb %1, %0" : "+Qo" (*(volatile u8 __force *)addr) : "r" (val)); } -static inline void __raw_writel(u32 val, volatile void __iomem *addr) +static inline void __raw_writel(u32 val, const volatile void __iomem *addr) { asm volatile("str %1, %0" : "+Qo" (*(volatile u32 __force *)addr) ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 22/24] scsi: eesox: use __iomem pointers for MMIO 2012-09-15 8:00 ` Arnd Bergmann @ 2012-09-15 8:57 ` Russell King - ARM Linux 2012-09-15 10:30 ` Arnd Bergmann 0 siblings, 1 reply; 8+ messages in thread From: Russell King - ARM Linux @ 2012-09-15 8:57 UTC (permalink / raw) To: Arnd Bergmann Cc: linux-scsi, Nicolas Pitre, Will Deacon, linux-kernel, James E.J. Bottomley, linux-arm-kernel On Sat, Sep 15, 2012 at 08:00:35AM +0000, Arnd Bergmann wrote: > On Friday 14 September 2012, Russell King - ARM Linux wrote: > > On Fri, Sep 14, 2012 at 11:34:50PM +0200, Arnd Bergmann wrote: > > > ARM is moving to stricter checks on readl/write functions, > > > so we need to use the correct types everywhere. > > > > There's nothing wrong with const iomem pointers. If you think > > otherwise, patch x86 not to use const in its accessor implementation > > and watch the reaction: > > > > #define build_mmio_read(name, size, type, reg, barrier) \ > > static inline type name(const volatile void __iomem *addr) \ > > { type ret; asm volatile("mov" size " %1,%0":reg (ret) \ > > :"m" (*(volatile type __force *)addr) barrier); return ret; } > > > > build_mmio_read(readb, "b", unsigned char, "=q", :"memory") > > build_mmio_read(readw, "w", unsigned short, "=r", :"memory") > > build_mmio_read(readl, "l", unsigned int, "=r", :"memory") > > Ok, fair enough. Can you fold the patch below into > "ARM: 7500/1: io: avoid writeback addressing modes for __raw_ > accessors", or apply on top then? No - const is not appropriate for the write accessors. Again, this puts us at odds with x86: #define build_mmio_write(name, size, type, reg, barrier) \ static inline void name(type val, volatile void __iomem *addr) \ { asm volatile("mov" size " %0,%1": :reg (val), \ "m" (*(volatile type __force *)addr) barrier); } build_mmio_write(writeb, "b", unsigned char, "q", :"memory") build_mmio_write(writew, "w", unsigned short, "r", :"memory") build_mmio_write(writel, "l", unsigned int, "r", :"memory") So, readl etc are all const volatile void __iomem *, but writel etc are all volatile void __iomem *. How they're defined on ARM after 7500/1 copies how they're defined on x86. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 22/24] scsi: eesox: use __iomem pointers for MMIO 2012-09-15 8:57 ` Russell King - ARM Linux @ 2012-09-15 10:30 ` Arnd Bergmann 2012-09-17 22:03 ` Russell King - ARM Linux 0 siblings, 1 reply; 8+ messages in thread From: Arnd Bergmann @ 2012-09-15 10:30 UTC (permalink / raw) To: Russell King - ARM Linux Cc: linux-arm-kernel, linux-kernel, Will Deacon, Nicolas Pitre, James E.J. Bottomley, linux-scsi On Saturday 15 September 2012, Russell King - ARM Linux wrote: > On Sat, Sep 15, 2012 at 08:00:35AM +0000, Arnd Bergmann wrote: > > On Friday 14 September 2012, Russell King - ARM Linux wrote: > > > On Fri, Sep 14, 2012 at 11:34:50PM +0200, Arnd Bergmann wrote: > > > > ARM is moving to stricter checks on readl/write functions, > > > > so we need to use the correct types everywhere. > > > > > > There's nothing wrong with const iomem pointers. If you think > > > otherwise, patch x86 not to use const in its accessor implementation > > > and watch the reaction: > > > > > > #define build_mmio_read(name, size, type, reg, barrier) \ > > > static inline type name(const volatile void __iomem *addr) \ > > > { type ret; asm volatile("mov" size " %1,%0":reg (ret) \ > > > :"m" (*(volatile type __force *)addr) barrier); return ret; } > > > > > > build_mmio_read(readb, "b", unsigned char, "=q", :"memory") > > > build_mmio_read(readw, "w", unsigned short, "=r", :"memory") > > > build_mmio_read(readl, "l", unsigned int, "=r", :"memory") > > > > Ok, fair enough. Can you fold the patch below into > > "ARM: 7500/1: io: avoid writeback addressing modes for __raw_ > > accessors", or apply on top then? > > No - const is not appropriate for the write accessors. Again, this puts > us at odds with x86: > > #define build_mmio_write(name, size, type, reg, barrier) \ > static inline void name(type val, volatile void __iomem *addr) \ > { asm volatile("mov" size " %0,%1": :reg (val), \ > "m" (*(volatile type __force *)addr) barrier); } > > build_mmio_write(writeb, "b", unsigned char, "q", :"memory") > build_mmio_write(writew, "w", unsigned short, "r", :"memory") > build_mmio_write(writel, "l", unsigned int, "r", :"memory") > > So, readl etc are all const volatile void __iomem *, but writel etc are > all volatile void __iomem *. > > How they're defined on ARM after 7500/1 copies how they're defined on > x86. Well, you have to make up your mind what you want. Right now, we get these warnings in rpc_defconfig: Generating include/generated/mach-types.h /home/arnd/linux-arm/drivers/net/ethernet/seeq/ether3.c: In function 'ether3_outb': /home/arnd/linux-arm/drivers/net/ethernet/seeq/ether3.c:104:2: error: passing argument 2 of '__raw_writeb' discards 'const' qualifier from pointer target type [-Werror] /home/arnd/linux-arm/arch/arm/include/asm/io.h:81:91: note: expected 'volatile void *' but argument is of type 'const void *' /home/arnd/linux-arm/drivers/scsi/arm/eesox.c: In function 'eesoxscsi_buffer_out': /home/arnd/linux-arm/drivers/scsi/arm/eesox.c:310:4: error: passing argument 2 of '__raw_writel' discards 'const' qualifier from pointer target type [-Werror] /home/arnd/linux-arm/arch/arm/include/asm/io.h:88:91: note: expected 'volatile void *' but argument is of type 'const void *' /home/arnd/linux-arm/drivers/scsi/arm/eesox.c:324:4: error: passing argument 2 of '__raw_writel' discards 'const' qualifier from pointer target type [-Werror] /home/arnd/linux-arm/arch/arm/include/asm/io.h:88:91: note: expected 'volatile void *' but argument is of type 'const void *' /home/arnd/linux-arm/drivers/scsi/arm/eesox.c:325:4: error: passing argument 2 of '__raw_writel' discards 'const' qualifier from pointer target type [-Werror] /home/arnd/linux-arm/arch/arm/include/asm/io.h:88:91: note: expected 'volatile void *' but argument is of type 'const void *' Either we allow drivers to write to const __iomem pointers or we don't. I don't care which way we do it, but just saying both patches are wrong is not very helpful. Arnd ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 22/24] scsi: eesox: use __iomem pointers for MMIO 2012-09-15 10:30 ` Arnd Bergmann @ 2012-09-17 22:03 ` Russell King - ARM Linux 2012-09-18 8:09 ` Arnd Bergmann 0 siblings, 1 reply; 8+ messages in thread From: Russell King - ARM Linux @ 2012-09-17 22:03 UTC (permalink / raw) To: Arnd Bergmann Cc: linux-arm-kernel, linux-kernel, Will Deacon, Nicolas Pitre, James E.J. Bottomley, linux-scsi On Sat, Sep 15, 2012 at 10:30:43AM +0000, Arnd Bergmann wrote: > On Saturday 15 September 2012, Russell King - ARM Linux wrote: > > On Sat, Sep 15, 2012 at 08:00:35AM +0000, Arnd Bergmann wrote: > > > On Friday 14 September 2012, Russell King - ARM Linux wrote: > > > > On Fri, Sep 14, 2012 at 11:34:50PM +0200, Arnd Bergmann wrote: > > > > > ARM is moving to stricter checks on readl/write functions, > > > > > so we need to use the correct types everywhere. > > > > > > > > There's nothing wrong with const iomem pointers. If you think > > > > otherwise, patch x86 not to use const in its accessor implementation > > > > and watch the reaction: > > > > > > > > #define build_mmio_read(name, size, type, reg, barrier) \ > > > > static inline type name(const volatile void __iomem *addr) \ > > > > { type ret; asm volatile("mov" size " %1,%0":reg (ret) \ > > > > :"m" (*(volatile type __force *)addr) barrier); return ret; } > > > > > > > > build_mmio_read(readb, "b", unsigned char, "=q", :"memory") > > > > build_mmio_read(readw, "w", unsigned short, "=r", :"memory") > > > > build_mmio_read(readl, "l", unsigned int, "=r", :"memory") > > > > > > Ok, fair enough. Can you fold the patch below into > > > "ARM: 7500/1: io: avoid writeback addressing modes for __raw_ > > > accessors", or apply on top then? > > > > No - const is not appropriate for the write accessors. Again, this puts > > us at odds with x86: > > > > #define build_mmio_write(name, size, type, reg, barrier) \ > > static inline void name(type val, volatile void __iomem *addr) \ > > { asm volatile("mov" size " %0,%1": :reg (val), \ > > "m" (*(volatile type __force *)addr) barrier); } > > > > build_mmio_write(writeb, "b", unsigned char, "q", :"memory") > > build_mmio_write(writew, "w", unsigned short, "r", :"memory") > > build_mmio_write(writel, "l", unsigned int, "r", :"memory") > > > > So, readl etc are all const volatile void __iomem *, but writel etc are > > all volatile void __iomem *. > > > > How they're defined on ARM after 7500/1 copies how they're defined on > > x86. > > Well, you have to make up your mind what you want. Right now, we get these > warnings in rpc_defconfig: > > Generating include/generated/mach-types.h > /home/arnd/linux-arm/drivers/net/ethernet/seeq/ether3.c: In function 'ether3_outb': > /home/arnd/linux-arm/drivers/net/ethernet/seeq/ether3.c:104:2: error: passing argument 2 of '__raw_writeb' discards 'const' qualifier from pointer target type [-Werror] > /home/arnd/linux-arm/arch/arm/include/asm/io.h:81:91: note: expected 'volatile void *' but argument is of type 'const void *' > /home/arnd/linux-arm/drivers/scsi/arm/eesox.c: In function 'eesoxscsi_buffer_out': > /home/arnd/linux-arm/drivers/scsi/arm/eesox.c:310:4: error: passing argument 2 of '__raw_writel' discards 'const' qualifier from pointer target type [-Werror] > /home/arnd/linux-arm/arch/arm/include/asm/io.h:88:91: note: expected 'volatile void *' but argument is of type 'const void *' > /home/arnd/linux-arm/drivers/scsi/arm/eesox.c:324:4: error: passing argument 2 of '__raw_writel' discards 'const' qualifier from pointer target type [-Werror] > /home/arnd/linux-arm/arch/arm/include/asm/io.h:88:91: note: expected 'volatile void *' but argument is of type 'const void *' > /home/arnd/linux-arm/drivers/scsi/arm/eesox.c:325:4: error: passing argument 2 of '__raw_writel' discards 'const' qualifier from pointer target type [-Werror] > /home/arnd/linux-arm/arch/arm/include/asm/io.h:88:91: note: expected 'volatile void *' but argument is of type 'const void *' > > Either we allow drivers to write to const __iomem pointers or we don't. I > don't care which way we do it, but just saying both patches are wrong is > not very helpful. In both of my replies, I've said "as x86 does". We need to follow x86's behaviour here, which is as I've quoted - it's not a matter that "I need to make up my mind" - my mind is already well made up. That is, we need to follow x86 here. That is, const volatile void __iomem * for reads, and volatile void __iomem * for writes. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 22/24] scsi: eesox: use __iomem pointers for MMIO 2012-09-17 22:03 ` Russell King - ARM Linux @ 2012-09-18 8:09 ` Arnd Bergmann 0 siblings, 0 replies; 8+ messages in thread From: Arnd Bergmann @ 2012-09-18 8:09 UTC (permalink / raw) To: Russell King - ARM Linux Cc: linux-arm-kernel, linux-kernel, Will Deacon, Nicolas Pitre, James E.J. Bottomley, linux-scsi On Monday 17 September 2012, Russell King - ARM Linux wrote: > In both of my replies, I've said "as x86 does". We need to follow > x86's behaviour here, which is as I've quoted - it's not a matter > that "I need to make up my mind" - my mind is already well made up. > That is, we need to follow x86 here. > > That is, const volatile void __iomem * for reads, and volatile void > __iomem * for writes. Ok, I'll keep the patch in the series then, as it only changes the pointer that we do an MMIO write on, not the ones for MMIO read. Arnd ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2012-09-18 8:09 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-09-14 21:34 [PATCH 00/24] ARM: readl/writel conversion fallout Arnd Bergmann 2012-09-14 21:34 ` [PATCH 22/24] scsi: eesox: use __iomem pointers for MMIO Arnd Bergmann 2012-09-14 23:27 ` Russell King - ARM Linux 2012-09-15 8:00 ` Arnd Bergmann 2012-09-15 8:57 ` Russell King - ARM Linux 2012-09-15 10:30 ` Arnd Bergmann 2012-09-17 22:03 ` Russell King - ARM Linux 2012-09-18 8:09 ` Arnd Bergmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).