Skip to contentSkip to conversation

Question About __REG32 marco

DISCUSSION colorant 3 participants 6 messages Nov 20, 2007 – Nov 21, 2007

lore
Share & tools
lore

colorant

author
#0 lore raw

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

#1 lore raw
Quoted 9 lines

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 ?

Quoted 6 lines

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.

#2 lore raw
In reply to colorant
Quoted 6 lines · colorant

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.

Quoted 4 lines

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

author
#3 lore raw

Hi Dave

Quoted 19 lines · colorant
Quoted 6 lines

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.

Quoted 9 lines

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.

Quoted 6 lines
Quoted 5 lines

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 !

Quoted 6 lines

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

#4 lore raw
Quoted 7 lines · colorant

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

author
#5 lore raw

Hi Dave

Quoted 12 lines · colorant
Quoted 7 lines

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?

Quoted 8 lines

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