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

Commit Message

John Paul Adrian Glaubitz May 3, 2023, 8:14 a.m. UTC

Hi!

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:


Does that look reasonable?

Adrian

Comments

Geert Uytterhoeven May 3, 2023, 9:08 a.m. UTC | #1

Hi Adrian,

CC dt

On Wed, May 3, 2023 at 10:14 AM John Paul Adrian Glaubitz
<[e-mail]> 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.

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

>
>  static void j2_flush_icache(void *args)
>  {
>
> Does that look reasonable?

Gr{oetje,eeting}s,

                        Geert

John Paul Adrian Glaubitz May 3, 2023, 10:50 a.m. UTC | #2

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
> <[e-mail]> 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

Rob Landley May 3, 2023, 2:53 p.m. UTC | #3

On 5/3/23 05:50, John Paul Adrian Glaubitz wrote:
>> BTW, "jcore,cache" does not have any DT binding documentation.
> 
> Jeff or Rob should look into this.

I'm looking at Documentation/devicetree/bindings/cache... (yaml files?) but am
not as of yet enlightened. I can poke Jeff in the morning.

Rob

Patch

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);
 
        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;
 
 static void j2_flush_icache(void *args)
 {