| Message ID | 87lfqhscac.wl-kuninori.morimoto.gx@renesas.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [repost] sh: clkfwk: remove r8/r16/r32 | expand |
Hi Morimoto-san, On Thu, Jan 9, 2020 at 1:29 AM Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > SH will get below warning > > ${LINUX}/drivers/sh/clk/cpg.c: In function 'r8': > ${LINUX}/drivers/sh/clk/cpg.c:41:17: warning: passing argument 1 of 'ioread8' > discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] > return ioread8(addr); > ^~~~ > In file included from ${LINUX}/arch/sh/include/asm/io.h:21, > from ${LINUX}/include/linux/io.h:13, > from ${LINUX}/drivers/sh/clk/cpg.c:14: > ${LINUX}/include/asm-generic/iomap.h:29:29: note: expected 'void *' but > argument is of type 'const void *' > extern unsigned int ioread8(void __iomem *); > ^~~~~~~~~~~~~~ > > We don't need "const" for r8/r16/r32. > And we don't need r8/r16/r32 themselvs. > This patch cleanup these. > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> One note below. > --- a/drivers/sh/clk/cpg.c > +++ b/drivers/sh/clk/cpg.c > @@ -36,36 +36,21 @@ static void sh_clk_write(int value, struct clk *clk) > iowrite32(value, clk->mapped_reg); > } > > -static unsigned int r8(const void __iomem *addr) > -{ > - return ioread8(addr); > -} > - > -static unsigned int r16(const void __iomem *addr) > -{ > - return ioread16(addr); > -} > - > -static unsigned int r32(const void __iomem *addr) > -{ > - return ioread32(addr); > -} > - > 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); While it is good to get rid of the wrappers, the change above will conflict with [PATCH v2 1/9] iomap: Constify ioreadX() iomem argument (as in generic implementation) (https://lore.kernel.org/lkml/20200108200528.4614-2-krzk@kernel.org/), which will add const to ioread*(). > int i; > void __iomem *mapped_status = (phys_addr_t)clk->status_reg - > (phys_addr_t)clk->enable_reg + clk->mapped_reg; > > if (clk->flags & CLK_ENABLE_REG_8BIT) > - read = r8; > + read = ioread8; > else if (clk->flags & CLK_ENABLE_REG_16BIT) > - read = r16; > + read = ioread16; > else > - read = r32; > + read = ioread32; > > for (i = 1000; > (read(mapped_status) & (1 << clk->enable_bit)) && i; Gr{oetje,eeting}s, Geert
Hi Geert > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > > > SH will get below warning > > > > ${LINUX}/drivers/sh/clk/cpg.c: In function 'r8': > > ${LINUX}/drivers/sh/clk/cpg.c:41:17: warning: passing argument 1 of 'ioread8' > > discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers] > > return ioread8(addr); > > ^~~~ > > In file included from ${LINUX}/arch/sh/include/asm/io.h:21, > > from ${LINUX}/include/linux/io.h:13, > > from ${LINUX}/drivers/sh/clk/cpg.c:14: > > ${LINUX}/include/asm-generic/iomap.h:29:29: note: expected 'void *' but > > argument is of type 'const void *' > > extern unsigned int ioread8(void __iomem *); > > ^~~~~~~~~~~~~~ > > > > We don't need "const" for r8/r16/r32. > > And we don't need r8/r16/r32 themselvs. > > This patch cleanup these. > > > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thanks > > 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); > > While it is good to get rid of the wrappers, the change above will conflict with > [PATCH v2 1/9] iomap: Constify ioreadX() iomem argument (as in generic > implementation) > (https://lore.kernel.org/lkml/20200108200528.4614-2-krzk@kernel.org/), > which will add const to ioread*(). Oh, I see. Will consider about it, and post v2 Thank you for your help !! Best regards --- Kuninori Morimoto
diff --git a/drivers/sh/clk/cpg.c b/drivers/sh/clk/cpg.c index eeb028b9..a5cacfe 100644 --- a/drivers/sh/clk/cpg.c +++ b/drivers/sh/clk/cpg.c @@ -36,36 +36,21 @@ static void sh_clk_write(int value, struct clk *clk) iowrite32(value, clk->mapped_reg); } -static unsigned int r8(const void __iomem *addr) -{ - return ioread8(addr); -} - -static unsigned int r16(const void __iomem *addr) -{ - return ioread16(addr); -} - -static unsigned int r32(const void __iomem *addr) -{ - return ioread32(addr); -} - 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; if (clk->flags & CLK_ENABLE_REG_8BIT) - read = r8; + read = ioread8; else if (clk->flags & CLK_ENABLE_REG_16BIT) - read = r16; + read = ioread16; else - read = r32; + read = ioread32; for (i = 1000; (read(mapped_status) & (1 << clk->enable_bit)) && i;