linux-kernel - Re: [PATCH] spi/xilinx: Cast ioread32/iowrite32 function pointers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 29 Jan 2015 23:11:47 +0100
From:	Arnd Bergmann <arnd@...db.de>
To:	linux-arm-kernel@...ts.infradead.org
Cc:	Ricardo Ribalda Delgado <ricardo.ribalda@...il.com>,
	Geert Uytterhoeven <geert@...ux-m68k.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Michal Simek <michal.simek@...inx.com>,
	Mark Brown <broonie@...nel.org>,
	Sören Brinkmann <soren.brinkmann@...inx.com>,
	linux-spi <linux-spi@...r.kernel.org>
Subject: Re: [PATCH] spi/xilinx: Cast ioread32/iowrite32 function pointers

On Thursday 29 January 2015 17:39:08 Ricardo Ribalda Delgado wrote:
> > I think the definitions in include/asm-generic/iomap.h are actually wrong,
> > as they lack a const:
> >
> >     extern unsigned int ioread8(void __iomem *);
> >     extern unsigned int ioread16(void __iomem *);
> >     extern unsigned int ioread16be(void __iomem *);
> >     extern unsigned int ioread32(void __iomem *);
> >     extern unsigned int ioread32be(void __iomem *);
> >
> > Note that the definitions in include/asm-generic/io.h do have the const:
> >
> >     static inline u8 ioread8(const volatile void __iomem *addr)
> >     static inline u16 ioread16(const volatile void __iomem *addr)
> >     static inline u32 ioread32(const volatile void __iomem *addr)
> >     static inline u16 ioread16be(const volatile void __iomem *addr)
> >     static inline u32 ioread32be(const volatile void __iomem *addr)
> 
> 
> I think you are right: ioread/iowrite is poorly implemented in many arches 
> 
> Would you mind sending the patch for asm-generic/iomap ? Or you want
> me to do it.
> 

I think we don't need the 'volatile' keyword here. The main reason
we have it on readl() is to shut up the compiler when dealing with
ancient driver code that annotates iomem pointers as volatile.

This is generally considered a (minor) driver mistake though, and
modern drivers that for some reason use ioread*() typically don't
do that (or they get a warning).

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/