[PATCH] [POWERPC] Move phys_addr_t definition into asm/types.h

[PATCH] [POWERPC] Move phys_addr_t definition into asm/types.h

[Kumar Gala]

Moved phys_addr_t out of mmu-*.h and into asm/types.h so we can use it in
places that before would have caused recursive includes.

For example to use phys_addr_t in <asm/page.h> we would have included
<asm/mmu.h> which would have possibly included <asm/mmu-hash64.h> which
includes <asm/page.h>.  Wheeee recursive include.

Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 include/asm-powerpc/mmu-40x.h       |    2 --
 include/asm-powerpc/mmu-44x.h       |    2 --
 include/asm-powerpc/mmu-8xx.h       |    2 --
 include/asm-powerpc/mmu-fsl-booke.h |    6 ------
 include/asm-powerpc/mmu-hash32.h    |    2 --
 include/asm-powerpc/mmu-hash64.h    |    3 ---
 include/asm-powerpc/types.h         |    7 +++++++
 7 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/include/asm-powerpc/mmu-40x.h b/include/asm-powerpc/mmu-40x.h
index 7d37f77..3d10867 100644
--- a/include/asm-powerpc/mmu-40x.h
+++ b/include/asm-powerpc/mmu-40x.h
@@ -53,8 +53,6 @@

 #ifndef __ASSEMBLY__

-typedef unsigned long phys_addr_t;
-
 typedef struct {
 	unsigned long id;
 	unsigned long vdso_base;
diff --git a/include/asm-powerpc/mmu-44x.h b/include/asm-powerpc/mmu-44x.h
index 62772ae..c8b02d9 100644
--- a/include/asm-powerpc/mmu-44x.h
+++ b/include/asm-powerpc/mmu-44x.h
@@ -53,8 +53,6 @@

 #ifndef __ASSEMBLY__

-typedef unsigned long long phys_addr_t;
-
 typedef struct {
 	unsigned long id;
 	unsigned long vdso_base;
diff --git a/include/asm-powerpc/mmu-8xx.h b/include/asm-powerpc/mmu-8xx.h
index 952bd88..9db877e 100644
--- a/include/asm-powerpc/mmu-8xx.h
+++ b/include/asm-powerpc/mmu-8xx.h
@@ -136,8 +136,6 @@
 #define SPRN_M_TW	799

 #ifndef __ASSEMBLY__
-typedef unsigned long phys_addr_t;
-
 typedef struct {
 	unsigned long id;
 	unsigned long vdso_base;
diff --git a/include/asm-powerpc/mmu-fsl-booke.h b/include/asm-powerpc/mmu-fsl-booke.h
index 3758000..925d93c 100644
--- a/include/asm-powerpc/mmu-fsl-booke.h
+++ b/include/asm-powerpc/mmu-fsl-booke.h
@@ -73,12 +73,6 @@

 #ifndef __ASSEMBLY__

-#ifndef CONFIG_PHYS_64BIT
-typedef unsigned long phys_addr_t;
-#else
-typedef unsigned long long phys_addr_t;
-#endif
-
 typedef struct {
 	unsigned long id;
 	unsigned long vdso_base;
diff --git a/include/asm-powerpc/mmu-hash32.h b/include/asm-powerpc/mmu-hash32.h
index 4bd735b..6e21ca6 100644
--- a/include/asm-powerpc/mmu-hash32.h
+++ b/include/asm-powerpc/mmu-hash32.h
@@ -84,8 +84,6 @@ typedef struct {
 	unsigned long vdso_base;
 } mm_context_t;

-typedef unsigned long phys_addr_t;
-
 #endif /* !__ASSEMBLY__ */

 #endif /* _ASM_POWERPC_MMU_HASH32_H_ */
diff --git a/include/asm-powerpc/mmu-hash64.h b/include/asm-powerpc/mmu-hash64.h
index 2864fa3..0dff767 100644
--- a/include/asm-powerpc/mmu-hash64.h
+++ b/include/asm-powerpc/mmu-hash64.h
@@ -469,9 +469,6 @@ static inline unsigned long get_vsid(unsigned long context, unsigned long ea,
 				 VSID_MODULUS_256M)
 #define KERNEL_VSID(ea)		VSID_SCRAMBLE(GET_ESID(ea))

-/* Physical address used by some IO functions */
-typedef unsigned long phys_addr_t;
-
 #endif /* __ASSEMBLY__ */

 #endif /* _ASM_POWERPC_MMU_HASH64_H_ */
diff --git a/include/asm-powerpc/types.h b/include/asm-powerpc/types.h
index 903fd19..020db52 100644
--- a/include/asm-powerpc/types.h
+++ b/include/asm-powerpc/types.h
@@ -50,6 +50,13 @@ typedef struct {
 	__u32 u[4];
 } __attribute__((aligned(16))) __vector128;

+/* Physical address used by some IO functions */
+#ifndef CONFIG_PHYS_64BIT
+typedef unsigned long phys_addr_t;
+#else
+typedef unsigned long long phys_addr_t;
+#endif
+
 #endif /* __ASSEMBLY__ */

 #ifdef __KERNEL__
-- 
1.5.4.1

Re: [PATCH] [POWERPC] Move phys_addr_t definition into asm/types.h

[Paul Mackerras]

Kumar Gala writes:

> Moved phys_addr_t out of mmu-*.h and into asm/types.h so we can use it in
> places that before would have caused recursive includes.
> 
> For example to use phys_addr_t in <asm/page.h> we would have included
> <asm/mmu.h> which would have possibly included <asm/mmu-hash64.h> which
> includes <asm/page.h>.  Wheeee recursive include.

In general this looks fine.  I wonder if you should use u64 rather
than unsigned long long.  Since CONFIG_PHYS_64BIT=n on 64-bit machines
(which is itself somewhat counterintuitive) we will actually use
unsigned long on 64-bit machines, so it matters less than I originally
thought, but it would be worth explaining that in a comment and/or the
commit message.

Paul.

Re: [PATCH] [POWERPC] Move phys_addr_t definition into asm/types.h

[Kumar Gala]

On Mar 31, 2008, at 10:42 PM, Paul Mackerras wrote:
> Kumar Gala writes:
>
>> Moved phys_addr_t out of mmu-*.h and into asm/types.h so we can use  
>> it in
>> places that before would have caused recursive includes.
>>
>> For example to use phys_addr_t in <asm/page.h> we would have included
>> <asm/mmu.h> which would have possibly included <asm/mmu-hash64.h>  
>> which
>> includes <asm/page.h>.  Wheeee recursive include.
>
> In general this looks fine.  I wonder if you should use u64 rather
> than unsigned long long.  Since CONFIG_PHYS_64BIT=n on 64-bit machines
> (which is itself somewhat counterintuitive) we will actually use
> unsigned long on 64-bit machines, so it matters less than I originally
> thought, but it would be worth explaining that in a comment and/or the
> commit message.

We could change it to be:

/* Physical address used by some IO functions */
#if defined(CONFIG_PPC64) || defined(CONFIG_PHYS_64BIT)
typedef u64 phys_addr_t;
#else
typedef u32 phys_addr_t;
#endif

This seems a bit more self documenting which is always nice (and I can  
add a comment in the commit message about CONFIG_PHYS_64BIT only  
making sense on ppc32).

- k

Re: [PATCH] [POWERPC] Move phys_addr_t definition into asm/types.h

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1929 bytes --]

On Tue, 1 Apr 2008, Kumar Gala wrote:
> On Mar 31, 2008, at 10:42 PM, Paul Mackerras wrote:
> >Kumar Gala writes:
> > >Moved phys_addr_t out of mmu-*.h and into asm/types.h so we can use it in
> > >places that before would have caused recursive includes.
> > >
> > >For example to use phys_addr_t in <asm/page.h> we would have included
> > ><asm/mmu.h> which would have possibly included <asm/mmu-hash64.h> which
> > >includes <asm/page.h>.  Wheeee recursive include.
> >
> >In general this looks fine.  I wonder if you should use u64 rather
> >than unsigned long long.  Since CONFIG_PHYS_64BIT=n on 64-bit machines
> >(which is itself somewhat counterintuitive) we will actually use
> >unsigned long on 64-bit machines, so it matters less than I originally
> >thought, but it would be worth explaining that in a comment and/or the
> >commit message.
> 
> We could change it to be:
> 
> /* Physical address used by some IO functions */
> #if defined(CONFIG_PPC64) || defined(CONFIG_PHYS_64BIT)
> typedef u64 phys_addr_t;
> #else
> typedef u32 phys_addr_t;
> #endif
> 
> This seems a bit more self documenting which is always nice (and I can add a
> comment in the commit message about CONFIG_PHYS_64BIT only making sense on
> ppc32).

If it's counterintuitive that CONFIG_PHYS_64BIT=n on 64-bit machines, why
not set it to =y in Kconfig?

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

Re: [PATCH] [POWERPC] Move phys_addr_t definition into asm/types.h

[Kumar Gala]

On Apr 2, 2008, at 6:53 AM, Geert Uytterhoeven wrote:
> On Tue, 1 Apr 2008, Kumar Gala wrote:
>> On Mar 31, 2008, at 10:42 PM, Paul Mackerras wrote:
>>> Kumar Gala writes:
>>>> Moved phys_addr_t out of mmu-*.h and into asm/types.h so we can  
>>>> use it in
>>>> places that before would have caused recursive includes.
>>>>
>>>> For example to use phys_addr_t in <asm/page.h> we would have  
>>>> included
>>>> <asm/mmu.h> which would have possibly included <asm/mmu-hash64.h>  
>>>> which
>>>> includes <asm/page.h>.  Wheeee recursive include.
>>>
>>> In general this looks fine.  I wonder if you should use u64 rather
>>> than unsigned long long.  Since CONFIG_PHYS_64BIT=n on 64-bit  
>>> machines
>>> (which is itself somewhat counterintuitive) we will actually use
>>> unsigned long on 64-bit machines, so it matters less than I  
>>> originally
>>> thought, but it would be worth explaining that in a comment and/or  
>>> the
>>> commit message.
>>
>> We could change it to be:
>>
>> /* Physical address used by some IO functions */
>> #if defined(CONFIG_PPC64) || defined(CONFIG_PHYS_64BIT)
>> typedef u64 phys_addr_t;
>> #else
>> typedef u32 phys_addr_t;
>> #endif
>>
>> This seems a bit more self documenting which is always nice (and I  
>> can add a
>> comment in the commit message about CONFIG_PHYS_64BIT only making  
>> sense on
>> ppc32).
>
> If it's counterintuitive that CONFIG_PHYS_64BIT=n on 64-bit  
> machines, why
> not set it to =y in Kconfig?

Its not that it =n, its that its not available.  The Kconfig looks like:

config PHYS_64BIT
         bool 'Large physical address support' if E500
         depends on 44x || E500
         select RESOURCES_64BIT
         default y if 44x

We could set it to y on PPC64 as there is no harm there, but Paul  
would need to say he wants that done.

- k