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

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

[Ralf Baechle]

On Thu, Jul 12, 2007 at 03:47:04PM +0100, Maciej W. Rozycki wrote:

> > Author: Atsushi Nemoto <anemo@mba.ocn.ne.jp> Wed Jul 11 23:12:00 2007 +0900
> > Comitter: Ralf Baechle <ralf@linux-mips.org> Thu Jul 12 14:39:44 2007 +0100
> > Commit: 57be612bf3815728ad29f39a09a1c70d71bd279c
> > Gitweb: http://www.linux-mips.org/g/linux/57be612b
> > Branch: master
> > 
> > CKSEG1ADDR() returns unsigned int value on 32bit kernel.  Cast it to
> 
>  This is not true.  With a 32-bit kernel CKSEG1ADDR(), quite 
> intentionally, returns a *signed* int.
> 
>  Since you have decided to fix the symptom rather than the bug I would at 
> least suggest to cast the result to "long" first and only then drop the 
> signedness.  Otherwise it looks misleading to a casual reader.

More a general comment on the use of KSEG0, KSEG1, KSEG1ADDR and similar
macros.  They've been used in about every piece of MIPS UNIX OS kernel
and driver code I ever touched.  But generally we want to abstract such
architecture specific knowledge away from drivers, even platform-specific
drivers.  So Linux code should prefer to use the standard Linux interfaces
such as ioremap, readb, writeb etc. over those old macros.

  Ralf

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

[Maciej W. Rozycki]

On Thu, 12 Jul 2007, linux-mips@linux-mips.org wrote:

> Author: Atsushi Nemoto <anemo@mba.ocn.ne.jp> Wed Jul 11 23:12:00 2007 +0900
> Comitter: Ralf Baechle <ralf@linux-mips.org> Thu Jul 12 14:39:44 2007 +0100
> Commit: 57be612bf3815728ad29f39a09a1c70d71bd279c
> Gitweb: http://www.linux-mips.org/g/linux/57be612b
> Branch: master
> 
> CKSEG1ADDR() returns unsigned int value on 32bit kernel.  Cast it to

 This is not true.  With a 32-bit kernel CKSEG1ADDR(), quite 
intentionally, returns a *signed* int.

 Since you have decided to fix the symptom rather than the bug I would at 
least suggest to cast the result to "long" first and only then drop the 
signedness.  Otherwise it looks misleading to a casual reader.

  Maciej

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

[Atsushi Nemoto]

On Thu, 12 Jul 2007 15:47:04 +0100 (BST), "Maciej W. Rozycki" <macro@linux-mips.org> wrote:
> > CKSEG1ADDR() returns unsigned int value on 32bit kernel.  Cast it to
> 
>  This is not true.  With a 32-bit kernel CKSEG1ADDR(), quite 
> intentionally, returns a *signed* int.

Yes, the comment was wrong.  Thanks.

>  Since you have decided to fix the symptom rather than the bug I would at 
> least suggest to cast the result to "long" first and only then drop the 
> signedness.  Otherwise it looks misleading to a casual reader.

OK, I added cast to "long", and a comment to why the cast was
introduced.


Subject: [MIPS] Workaround for a sparse warning in include/asm-mips/io.h (part 2)

Since CKSEG1ADDR() returns "signed int" (on 32bit), cast it to "long"
first to avoid misleading.  Also add a comment why the cast to
"unsigned long" was introduced.

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 7ba9289..ad60863 100644
--- a/include/asm-mips/io.h
+++ b/include/asm-mips/io.h
@@ -212,8 +212,9 @@ static inline void __iomem * __ioremap_mode(phys_t offset, unsigned long size,
 		 */
 		if (__IS_LOW512(phys_addr) && __IS_LOW512(last_addr) &&
 		    flags == _CACHE_UNCACHED)
+			/* The cast to unsigned long makes sparse happy */
 			return (void __iomem *)
-				(unsigned long)CKSEG1ADDR(phys_addr);
+				(unsigned long)(long)CKSEG1ADDR(phys_addr);
 	}
 
 	return __ioremap(offset, size, flags);