| Message ID | 1578399963-2229-1-git-send-email-krzk@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sh: clk: Fix discarding const qualifier warning | expand |
Hi Krzysztof, On Tue, Jan 7, 2020 at 1:26 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > ioreadX() accepts pointer to non-const memory. This fixes warnings > like: > > drivers/sh/clk/cpg.c: In function ‘r8’: > drivers/sh/clk/cpg.c:41:17: warning: passing argument 1 of ‘ioread8’ > discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> Thanks for your patch! > --- a/drivers/sh/clk/cpg.c > +++ b/drivers/sh/clk/cpg.c > @@ -36,17 +36,17 @@ static void sh_clk_write(int value, struct clk *clk) > iowrite32(value, clk->mapped_reg); > } > > -static unsigned int r8(const void __iomem *addr) > +static unsigned int r8(void __iomem *addr) > { > return ioread8(addr); > } Isn't the real issue that some implementations of ioreadX() take const, while others don't? Even the generic ones disagree: include/asm-generic/io.h:static inline u8 ioread8(const volatile void __iomem *addr) include/asm-generic/iomap.h:extern unsigned int ioread8(void __iomem *); IMHO they all should take "const volatile void __iomem *". Gr{oetje,eeting}s, Geert
On Tue, 7 Jan 2020 at 14:00, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > Hi Krzysztof, > > On Tue, Jan 7, 2020 at 1:26 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > ioreadX() accepts pointer to non-const memory. This fixes warnings > > like: > > > > drivers/sh/clk/cpg.c: In function ‘r8’: > > drivers/sh/clk/cpg.c:41:17: warning: passing argument 1 of ‘ioread8’ > > discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > Thanks for your patch! > > > --- a/drivers/sh/clk/cpg.c > > +++ b/drivers/sh/clk/cpg.c > > @@ -36,17 +36,17 @@ static void sh_clk_write(int value, struct clk *clk) > > iowrite32(value, clk->mapped_reg); > > } > > > > -static unsigned int r8(const void __iomem *addr) > > +static unsigned int r8(void __iomem *addr) > > { > > return ioread8(addr); > > } > > Isn't the real issue that some implementations of ioreadX() take const, > while others don't? > > Even the generic ones disagree: > > include/asm-generic/io.h:static inline u8 ioread8(const volatile > void __iomem *addr) > include/asm-generic/iomap.h:extern unsigned int ioread8(void __iomem *); > > IMHO they all should take "const volatile void __iomem *". Since this is a SuperH driver, I adjusted it to the SuperH implementation - lack of const. However iIndeed it makes sense to have them all taking "const"... Let me check, if I can fix it (without the real HW). Best regards, Krzysztof
On Tue, Jan 07, 2020 at 02:05:14PM +0100, Krzysztof Kozlowski wrote: > On Tue, 7 Jan 2020 at 14:00, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > Hi Krzysztof, > > > > On Tue, Jan 7, 2020 at 1:26 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > > ioreadX() accepts pointer to non-const memory. This fixes warnings > > > like: > > > > > > drivers/sh/clk/cpg.c: In function ‘r8’: > > > drivers/sh/clk/cpg.c:41:17: warning: passing argument 1 of ‘ioread8’ > > > discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] > > > > > > Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> > > > > Thanks for your patch! > > > > > --- a/drivers/sh/clk/cpg.c > > > +++ b/drivers/sh/clk/cpg.c > > > @@ -36,17 +36,17 @@ static void sh_clk_write(int value, struct clk *clk) > > > iowrite32(value, clk->mapped_reg); > > > } > > > > > > -static unsigned int r8(const void __iomem *addr) > > > +static unsigned int r8(void __iomem *addr) > > > { > > > return ioread8(addr); > > > } > > > > Isn't the real issue that some implementations of ioreadX() take const, > > while others don't? > > > > Even the generic ones disagree: > > > > include/asm-generic/io.h:static inline u8 ioread8(const volatile > > void __iomem *addr) > > include/asm-generic/iomap.h:extern unsigned int ioread8(void __iomem *); > > > > IMHO they all should take "const volatile void __iomem *". > > Since this is a SuperH driver, I adjusted it to the SuperH > implementation - lack of const. However iIndeed it makes sense to have > them all taking "const"... Let me check, if I can fix it (without the > real HW). That will be non-trivial because many platforms define ioreadX() with non-const. For example entire alpha with many its implementations of ioread(). Even include/asm-generic/iomap.h defines them as non-const... Best regards, Krzysztof
arch/powerpc/kernel/iomap.cOn Tue, Jan 7, 2020 at 2:33 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > On Tue, Jan 07, 2020 at 02:05:14PM +0100, Krzysztof Kozlowski wrote: > > On Tue, 7 Jan 2020 at 14:00, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > Since this is a SuperH driver, I adjusted it to the SuperH > > implementation - lack of const. However iIndeed it makes sense to have > > them all taking "const"... Let me check, if I can fix it (without the > > real HW). > > That will be non-trivial because many platforms define ioreadX() with > non-const. For example entire alpha with many its implementations of > ioread(). Even include/asm-generic/iomap.h defines them as non-const... I found these instances: arch/alpha/include/asm/io.h arch/alpha/kernel/io.c arch/parisc/include/asm/io.h arch/parisc/lib/iomap.c arch/sh/kernel/iomap.c arch/powerpc/kernel/iomap.c lib/iomap.c include/asm-generic/iomap.h At least the last four file would have to be done at the same time as the header is shared, but the actual conversion should be trivial. Arnd
On Tue, 7 Jan 2020 at 14:54, Arnd Bergmann <arnd@arndb.de> wrote: > > arch/powerpc/kernel/iomap.cOn Tue, Jan 7, 2020 at 2:33 PM Krzysztof > Kozlowski <krzk@kernel.org> wrote: > > On Tue, Jan 07, 2020 at 02:05:14PM +0100, Krzysztof Kozlowski wrote: > > > On Tue, 7 Jan 2020 at 14:00, Geert Uytterhoeven <geert@linux-m68k.org> wrote: > > > > > > Since this is a SuperH driver, I adjusted it to the SuperH > > > implementation - lack of const. However iIndeed it makes sense to have > > > them all taking "const"... Let me check, if I can fix it (without the > > > real HW). > > > > That will be non-trivial because many platforms define ioreadX() with > > non-const. For example entire alpha with many its implementations of > > ioread(). Even include/asm-generic/iomap.h defines them as non-const... > > I found these instances: > > arch/alpha/include/asm/io.h > arch/alpha/kernel/io.c > arch/parisc/include/asm/io.h > arch/parisc/lib/iomap.c > arch/sh/kernel/iomap.c > arch/powerpc/kernel/iomap.c > lib/iomap.c > include/asm-generic/iomap.h > > At least the last four file would have to be done at the same time as > the header is shared, but the actual conversion should be trivial. Yes, assuming that I did not screw up some specific arch-implementation, it seems easy. I have patchset ready - just need to build test it and I'll share for kbuild & company to test. Best regards, Krzysztof
diff --git a/drivers/sh/clk/cpg.c b/drivers/sh/clk/cpg.c index eeb028b9cdb3..4f3d99d37809 100644 --- a/drivers/sh/clk/cpg.c +++ b/drivers/sh/clk/cpg.c @@ -36,17 +36,17 @@ static void sh_clk_write(int value, struct clk *clk) iowrite32(value, clk->mapped_reg); } -static unsigned int r8(const void __iomem *addr) +static unsigned int r8(void __iomem *addr) { return ioread8(addr); } -static unsigned int r16(const void __iomem *addr) +static unsigned int r16(void __iomem *addr) { return ioread16(addr); } -static unsigned int r32(const void __iomem *addr) +static unsigned int r32(void __iomem *addr) { return ioread32(addr); } @@ -55,7 +55,7 @@ static int sh_clk_mstp_enable(struct clk *clk) { sh_clk_write(sh_clk_read(clk) & ~(1 << clk->enable_bit), clk); if (clk->status_reg) { - unsigned int (*read)(const void __iomem *addr); + unsigned int (*read)(void __iomem *addr); int i; void __iomem *mapped_status = (phys_addr_t)clk->status_reg - (phys_addr_t)clk->enable_reg + clk->mapped_reg;
ioreadX() accepts pointer to non-const memory. This fixes warnings like: drivers/sh/clk/cpg.c: In function ‘r8’: drivers/sh/clk/cpg.c:41:17: warning: passing argument 1 of ‘ioread8’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers] Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org> --- drivers/sh/clk/cpg.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)