Re: Fixing "int-to-pointer-cast" warning in J2 code

Hi Geert!

On Wed, 2023-05-03 at 11:08 +0200, Geert Uytterhoeven wrote:
> On Wed, May 3, 2023 at 10:14 AM John Paul Adrian Glaubitz
> <[email protected]> wrote:
> > When building j2_defconfig, the following warning is issued:
> > 
> > arch/sh/kernel/cpu/sh2/probe.c: In function 'scan_cache':
> > arch/sh/kernel/cpu/sh2/probe.c:24:16: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
> >    24 |  j2_ccr_base = (u32 __iomem *)of_flat_dt_translate_address(node);
> >       |
> > 
> > Reading the code and look how other users of of_flat_dt_translate_address()
> > used the return code, I came up with the following patch which fixes the issue:
> > 
> > diff --git a/arch/sh/kernel/cpu/sh2/probe.c b/arch/sh/kernel/cpu/sh2/probe.c
> > index d342ea08843f..a0dc3675fc68 100644
> > --- a/arch/sh/kernel/cpu/sh2/probe.c
> > +++ b/arch/sh/kernel/cpu/sh2/probe.c
> > @@ -14,14 +14,14 @@
> >  #include <asm/cache.h>
> > 
> >  #if defined(CONFIG_CPU_J2)
> > -extern u32 __iomem *j2_ccr_base;
> > +extern phys_addr_t j2_ccr_base;
> >  static int __init scan_cache(unsigned long node, const char *uname,
> >                              int depth, void *data)
> >  {
> >         if (!of_flat_dt_is_compatible(node, "jcore,cache"))
> >                 return 0;
> > 
> > -       j2_ccr_base = (u32 __iomem *)of_flat_dt_translate_address(node);
> > +       j2_ccr_base = of_flat_dt_translate_address(node);
> 
> of_flat_dt_translate_address() indeed returns a CPU physical address
> (perhaps its return type should be changed from u64 to phys_addr_t?)...
> 
> > 
> >         return 1;
> >  }
> > diff --git a/arch/sh/mm/cache-j2.c b/arch/sh/mm/cache-j2.c
> > index f277862a11f5..2bc6d38d6f7c 100644
> > --- a/arch/sh/mm/cache-j2.c
> > +++ b/arch/sh/mm/cache-j2.c
> > @@ -22,7 +22,7 @@
> >  #define DCACHE_FLUSH   0x200
> >  #define CACHE_FLUSH    (ICACHE_FLUSH | DCACHE_FLUSH)
> > 
> > -u32 __iomem *j2_ccr_base;
> > +phys_addr_t j2_ccr_base;
> 
> ... however, all other users of j2_ccr_base use this with __raw_*()
> I/O accessors, so "u32 __iomem *" is correct.
> 
> What is missing is a proper conversion from physical to virtual
> addresses, using e.g. ioremap().
> 
> As this is nommu, the identity mapping in ioremap() in
> arch/sh/include/asm/io.h should do, and cannot fail.
> 
> So just:
> 
>     j2_ccr_base = ioremap(of_flat_dt_translate_address(node), 4);
> 
> should be fine.

Thanks, that actually makes much more sense. I was actually looking for
such a function after reading what the __iomap attribute is for.

> BTW, "jcore,cache" does not have any DT binding documentation.

Jeff or Rob should look into this.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913