[PATCH] Workaround for a sparse warning in include/asm-mips/io.h

[PATCH] Workaround for a sparse warning in include/asm-mips/io.h

[Atsushi Nemoto]

CKSEG1ADDR() returns unsigned int value on 32bit kernel.  Cast it to
unsigned long to get rid of this warning:

include2/asm/io.h:215:12: warning: cast adds address space to expression (<asn:2>)

Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
---
diff --git a/include/asm-mips/io.h b/include/asm-mips/io.h
index 12bcc1f..7ba9289 100644
--- a/include/asm-mips/io.h
+++ b/include/asm-mips/io.h
@@ -212,7 +212,8 @@ static inline void __iomem * __ioremap_mode(phys_t offset, unsigned long size,
 		 */
 		if (__IS_LOW512(phys_addr) && __IS_LOW512(last_addr) &&
 		    flags == _CACHE_UNCACHED)
-			return (void __iomem *)CKSEG1ADDR(phys_addr);
+			return (void __iomem *)
+				(unsigned long)CKSEG1ADDR(phys_addr);
 	}
 
 	return __ioremap(offset, size, flags);

Re: [PATCH] Workaround for a sparse warning in include/asm-mips/io.h

[Maciej W. Rozycki]

On Wed, 11 Jul 2007, Atsushi Nemoto wrote:

> CKSEG1ADDR() returns unsigned int value on 32bit kernel.  Cast it to
> unsigned long to get rid of this warning:
> 
> include2/asm/io.h:215:12: warning: cast adds address space to expression (<asn:2>)
> 
> Signed-off-by: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
> ---
> diff --git a/include/asm-mips/io.h b/include/asm-mips/io.h
> index 12bcc1f..7ba9289 100644
> --- a/include/asm-mips/io.h
> +++ b/include/asm-mips/io.h
> @@ -212,7 +212,8 @@ static inline void __iomem * __ioremap_mode(phys_t offset, unsigned long size,
>  		 */
>  		if (__IS_LOW512(phys_addr) && __IS_LOW512(last_addr) &&
>  		    flags == _CACHE_UNCACHED)
> -			return (void __iomem *)CKSEG1ADDR(phys_addr);
> +			return (void __iomem *)
> +				(unsigned long)CKSEG1ADDR(phys_addr);
>  	}

 It looks like a bug in sparse.  The result of CKSEG1ADDR() has the same 
size as the pointer.  Perhaps we could append 'L' to the expansion of 
KSEG1 et al, but that should not really matter.

 But -- I have just checked two example calls to this function, one with a 
32-bit configuration and another one with a 64-bit one and sparse did not 
complain.  The cpp expansions of the expression in question are:

return (void *)((((int)(int)(phys_addr)) & 0x1fffffff) | 0xa0000000);

and:

return (void *)((((long int)(int)(phys_addr)) & 0x1fffffff) | 0xffffffffa0000000L);

respectively, so your cast is definitely redundant in these cases.  What 
sort of configuration are you using?  What's the preprocessor output for 
the problematic case?

  Maciej

Re: [PATCH] Workaround for a sparse warning in include/asm-mips/io.h

[Atsushi Nemoto]

On Wed, 11 Jul 2007 15:28:19 +0100 (BST), "Maciej W. Rozycki" <macro@linux-mips.org> wrote:
>  It looks like a bug in sparse.  The result of CKSEG1ADDR() has the same 
> size as the pointer.  Perhaps we could append 'L' to the expansion of 
> KSEG1 et al, but that should not really matter.

Yes, adding 'L' to KSEG1 is another way to silence the warnings.  But
I just thought it was a bit intrusive.  And I'm not sure all code are
OK if KSEG1 is 'signed' ...

>  But -- I have just checked two example calls to this function, one with a 
> 32-bit configuration and another one with a 64-bit one and sparse did not 
> complain.  The cpp expansions of the expression in question are:
> 
> return (void *)((((int)(int)(phys_addr)) & 0x1fffffff) | 0xa0000000);
> 
> and:
> 
> return (void *)((((long int)(int)(phys_addr)) & 0x1fffffff) | 0xffffffffa0000000L);
> 
> respectively, so your cast is definitely redundant in these cases.  What 
> sort of configuration are you using?  What's the preprocessor output for 
> the problematic case?

I see the warnings on 32-bit qemu kernel.  drivers/serial/8250.c,
lib/devres.c, etc.

I think sparse complains on casting to "void __iomem *" from "int".
It looks sparse accepts casting from "long".

---
Atsushi Nemoto

Re: [PATCH] Workaround for a sparse warning in include/asm-mips/io.h

[Ralf Baechle]

On Wed, Jul 11, 2007 at 11:54:20PM +0900, Atsushi Nemoto wrote:

> Yes, adding 'L' to KSEG1 is another way to silence the warnings.  But
> I just thought it was a bit intrusive.  And I'm not sure all code are
> OK if KSEG1 is 'signed' ...

The MIPS architecture's idea of the 32 and 64-bit address space is that
32-bit addresses can be sign-extended to 64-bit addresses.  This if of
course conflicting with the general Linux idea that virtual addresses are
represented as an unsigned long.

  Ralf

Re: [PATCH] Workaround for a sparse warning in include/asm-mips/io.h

[Ralf Baechle]

On Thu, Jul 12, 2007 at 02:19:21PM +0100, Ralf Baechle wrote:

Applied.

  Ralf

Re: [PATCH] Workaround for a sparse warning in include/asm-mips/io.h

[Ralf Baechle]

On Wed, Jul 11, 2007 at 11:54:20PM +0900, Atsushi Nemoto wrote:

Applied.

Re: [PATCH] Workaround for a sparse warning in include/asm-mips/io.h

[Ralf Baechle]

On Wed, Jul 11, 2007 at 11:12:00PM +0900, Atsushi Nemoto wrote:

Applied,

  Ralf