Question About __REG32 marco

Question About __REG32 marco

[colorant]

[-- Attachment #1: Type: text/plain, Size: 909 bytes --]

Hi

    In include/asm-arm/arch-omap/io.h

    The __REG32 macro is defined as following:

typedef struct { volatile u32 offset[4096]; } __regbase32;
#define __REGV32(vaddr)         ((__regbase32 *)((vaddr)&~4095)) \
                                        ->offset[((vaddr)&4095)>>2]
#define __REG32(paddr)          __REGV32(io_p2v(paddr))

    My question is :

    1:
    Why this macro need to be defined in this way ?  Why borther to use an array ?

    Can't we just use something like :

    *(volatile u32 *)(vaddr&~0x3) to get the value from vaddr with word align ?

    2:
    Why this offset array need to be 4096 in size ? shouldn't it be 1024 ? since it already point to a u32 value. and the >>2 make the macro never got chance to reach a index great than 1024 !

    I can only find this macro in omap arch, so I put up the question here 8 )

    Thanks

Raymond



[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

RE: Question About __REG32 marco

[Woodruff, Richard]

>     1:
>     Why this macro need to be defined in this way ?  Why borther to use an
> array ?
> 
>     Can't we just use something like :
> 
>     *(volatile u32 *)(vaddr&~0x3) to get the value from vaddr with word
> align ?
> 
>     2:
>     Why this offset array need to be 4096 in size ? shouldn't it be 1024 ?
> since it already point to a u32 value. and the >>2 make the macro never
> got chance to reach a index great than 1024 !
> 
>     I can only find this macro in omap arch, so I put up the question here

IIRC seems like David Brownell introduced this macro usage a couple years back.  I seem to recall the generated code was better using that scheme.

Regards,
Richard W.

Re: Question About __REG32 marco

[David Brownell]

On Tuesday 20 November 2007, colorant wrote:
> 
>     1:
>     Why this macro need to be defined in this way ?  Why borther to use an array ?
>     Can't we just use something like :
> 
>     *(volatile u32 *)(vaddr&~0x3) to get the value from vaddr with word align ?

It generalizes __REG2() from arch-pxa/hardware.h ... where,
as the comment notes, GCC would otherwise be incapable of
generating decent code.  It's possible GCC code generation
has gotten smarter since then.

Note that such macros are supposed to be used only with
constant addresses, so code can treat registers as if they
were global variables.  Some people dislike that style;
others find it more understandable than __raw_writel()
style accessors.

When it works right, driver code will have a register with
the base address of a controller, and all accesses to that
controller will use single word load/store instructions which
embed the offsets of the various registers being accessed
against that base register.

The main alternative addressing styles were notably larger
(I recall observing 1-2 KBytes per driver) and slower.


>     2:
>     Why this offset array need to be 4096 in size ? shouldn't it be 1024 ?
>     since it already point to a u32 value. and the >>2 make the macro never
>     got chance to reach a index great than 1024 !  

Look at the ARM instructions that are used.  The address
range may be 4KBytes, but the values are 4 bytes each.
So the range of their indices is just 1K.  (As implied
by the comments adjacent to those macro definitions, which
point out the use of LDR/STR for 8 and 32 bit values...)

That's the case for 32 bit values.  For other value sizes,
the logic is necessarily a bit different.

- Dave

Re: Question About __REG32 marco

[colorant]

[-- Attachment #1: Type: text/plain, Size: 2023 bytes --]

Hi Dave

> On Tuesday 20 November 2007, colorant wrote:
>> 
>>     1:
>>     Why this macro need to be defined in this way ?  Why borther to use an array ?
>>     Can't we just use something like :
>> 
>>     *(volatile u32 *)(vaddr&~0x3) to get the value from vaddr with word align ?
> 
> It generalizes __REG2() from arch-pxa/hardware.h ... where,
> as the comment notes, GCC would otherwise be incapable of
> generating decent code.  It's possible GCC code generation
> has gotten smarter since then.
> 
> Note that such macros are supposed to be used only with
> constant addresses, so code can treat registers as if they
> were global variables.  Some people dislike that style;
> others find it more understandable than __raw_writel()
> style accessors.
> 
> When it works right, driver code will have a register with
> the base address of a controller, and all accesses to that
> controller will use single word load/store instructions which
> embed the offsets of the various registers being accessed
> against that base register.
> 
> The main alternative addressing styles were notably larger
> (I recall observing 1-2 KBytes per driver) and slower.
> 

Yes, I can see that from the obj-code. Thanks.

> 
>>     2:
>>     Why this offset array need to be 4096 in size ? shouldn't it be 1024 ?
>>     since it already point to a u32 value. and the >>2 make the macro never
>>     got chance to reach a index great than 1024 !  
> 
> Look at the ARM instructions that are used.  The address
> range may be 4KBytes, but the values are 4 bytes each.
> So the range of their indices is just 1K.  (As implied
> by the comments adjacent to those macro definitions, which
> point out the use of LDR/STR for 8 and 32 bit values...)
> 

This is what I am puzzled, since the index is just 1K, the following definition :

typedef struct { volatile u32 offset[4096]; } __regbase32;

shouldn't be:

 typedef struct { volatile u32 offset[1024]; } __regbase32; ?

Raymond


[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: Question About __REG32 marco

[David Brownell]

On Tuesday 20 November 2007, colorant wrote:
> 	since the index is just 1K, the following definition :
> 
> typedef struct { volatile u32 offset[4096]; } __regbase32;
> 
> shouldn't be:
> 
>  typedef struct { volatile u32 offset[1024]; } __regbase32; ?

Would changing that make a difference in the generated code,
or turn up any source code errors?

I'm not sure changing that would matter at all.  And in any
case, those multiboot patches show the downside of this
particular idiom... someone may need to take another look
at the code generation issue, and maybe file some bugs/rfes
against current GCC versions.

- Dave

Re: Question About __REG32 marco

[colorant]

[-- Attachment #1: Type: text/plain, Size: 782 bytes --]

Hi Dave


> On Tuesday 20 November 2007, colorant wrote:
>> since the index is just 1K, the following definition :
>> 
>> typedef struct { volatile u32 offset[4096]; } __regbase32;
>> 
>> shouldn't be:
>> 
>> typedef struct { volatile u32 offset[1024]; } __regbase32; ?
> 
> Would changing that make a difference in the generated code,
> or turn up any source code errors?
> 
> I'm not sure changing that would matter at all.  And in any
> case, those multiboot patches show the downside of this
> particular idiom... someone may need to take another look
> at the code generation issue, and maybe file some bugs/rfes
> against current GCC versions.
> 
> - Dave
> 

No, the generated code is the same , and I can run up the kernel without problem.

Raymond

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]