From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ed1-f65.google.com (mail-ed1-f65.google.com. [209.85.208.65]) by gmr-mx.google.com with ESMTPS id y185si50856wmd.2.2020.01.08.01.15.54 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 08 Jan 2020 01:15:54 -0800 (PST) Received: by mail-ed1-f65.google.com with SMTP id l8so1994132edw.1 for ; Wed, 08 Jan 2020 01:15:54 -0800 (PST) Return-Path: Date: Wed, 8 Jan 2020 10:15:49 +0100 From: Krzysztof Kozlowski Subject: Re: [RFT 00/13] iomap: Constify ioreadX() iomem argument Message-ID: <20200108091549.GB10145@pi3> References: <1578415992-24054-1-git-send-email-krzk@kernel.org> <2355489c-a207-1927-54cf-85c04b62f18f@c-s.fr> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: To: Arnd Bergmann Cc: Christophe Leroy , Geert Uytterhoeven , Rich Felker , Jiri Slaby , "Michael S. Tsirkin" , David Airlie , Jason Wang , DRI Development , virtualization@lists.linux-foundation.org, "James E.J. Bottomley" , netdev , Paul Mackerras , Linux-Arch , Dave Jiang , Yoshinori Sato , Helge Deller , Linux-sh list , Alexey Brodkin , Ben Skeggs , ML nouveau , Dave Airlie , Matt Turner , arcml , Nick Kossifidis , Allen Hubbe , alpha , Ivan Kokshaysky , Thomas Gleixner , Mauro Carvalho Chehab , Kalle Valo , Richard Henderson , Parisc List , Vineet Gupta , linux-wireless , Linux Kernel Mailing List , Luis Chamberlain , Daniel Vetter , Jon Mason , linux-ntb@googlegroups.com, Andrew Morton , Linux Media Mailing List , linuxppc-dev , "David S. Miller" List-ID: On Wed, Jan 08, 2020 at 09:44:36AM +0100, Arnd Bergmann wrote: > On Wed, Jan 8, 2020 at 9:36 AM Christophe Leroy = wrote: > > Le 08/01/2020 =C3=A0 09:18, Krzysztof Kozlowski a =C3=A9crit : > > > On Wed, 8 Jan 2020 at 09:13, Geert Uytterhoeven wrote: > > > I'll add to this one also changes to ioreadX_rep() and add another > > > patch for volatile for reads and writes. I guess your review will be > > > appreciated once more because of ioreadX_rep() > > > > > > > volatile should really only be used where deemed necessary: > > > > https://www.kernel.org/doc/html/latest/process/volatile-considered-harm= ful.html > > > > It is said: " ... accessor functions might use volatile on > > architectures where direct I/O memory access does work. Essentially, > > each accessor call becomes a little critical section on its own and > > ensures that the access happens as expected by the programmer." >=20 > The I/O accessors are one of the few places in which 'volatile' generally > makes sense, at least for the implementations that do a plain pointer > dereference (probably none of the ones in question here). >=20 > In case of readl/writel, this is what we do in asm-generic: >=20 > static inline u32 __raw_readl(const volatile void __iomem *addr) > { > return *(const volatile u32 __force *)addr; > } SuperH is another example: 1. ioread8_rep(void __iomem *addr, void *dst, unsigned long count) calls mmio_insb() 2. static inline void mmio_insb(void __iomem *addr, u8 *dst, int count) calls __raw_readb() 3. #define __raw_readb(a) (__chk_io_ptr(a), *(volatile u8 __force= *)(a)) Even if interface was not marked as volatile, in fact its implementation was casting to volatile. > The __force-cast that removes the __iomem here also means that > the 'volatile' keyword could be dropped from the argument list, > as it has no real effect any more, but then there are a few drivers > that mark their iomem pointers as either 'volatile void __iomem*' or > (worse) 'volatile void *', so we keep it in the argument list to not > add warnings for those drivers. >=20 > It may be time to change these drivers to not use volatile for __iomem > pointers, but that seems out of scope for what Krzysztof is trying > to do. Ideally we would be consistent here though, either using volatile > all the time or never. Indeed. I guess there are no objections around const so let me send v2 for const only. Best regards, Krzysztof