[U-Boot] __attribute__((__packed__)) switching to byte-access on ARM
Hi
As I remarked in the opening email to the SMDK6400 patch-series, I've got a strange problem, I cannot understand.
Here're the c-code:
typedef struct { volatile u32 TCNTB; volatile u32 TCMPB; volatile u32 TCNTO; } s3c64xx_timer;
typedef struct { volatile u32 TCFG0; volatile u32 TCFG1; volatile u32 TCON; s3c64xx_timer ch[4]; volatile u32 TCNTB4; volatile u32 TCNTO4; } s3c64xx_timers;
static s3c64xx_timers *s3c64xx_get_base_timers(void) { return (s3c64xx_timers *)ELFIN_TIMER_BASE; }
/* macro to read the 16 bit timer */ static inline ulong read_timer(void) { s3c64xx_timers *const timers = s3c64xx_get_base_timers();
return timers->TCNTO4; }
The respective assembly code is quite simple:
00000000 <read_timer>: 0: e59f3004 ldr r3, [pc, #4] ; c <timer_load_val> 4: e5930040 ldr r0, [r3, #64] 8: e12fff1e bx lr c: 7f006000 swivc 0x00006000
Perfect. Now, if I make the two structs above packed, I get this assembly:
00000000 <read_timer>: 0: e59f301c ldr r3, [pc, #28] ; 24 <.text+0x24> 4: e5d30040 ldrb r0, [r3, #64] 8: e5d32041 ldrb r2, [r3, #65] c: e5d31042 ldrb r1, [r3, #66] 10: e1800402 orr r0, r0, r2, lsl #8 14: e5d33043 ldrb r3, [r3, #67] 18: e1800801 orr r0, r0, r1, lsl #16 1c: e1800c03 orr r0, r0, r3, lsl #24 20: e12fff1e bx lr 24: 7f006000 swivc 0x00006000
which converts all accesses to the structure to 8-bit... Ideas? Toolchain eldk-4.1. U-Boot from nand/testing git with the patches I just sent to the list, configured for smdk6400_config.
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
Dear Guennadi Liakhovetski,
In message Pine.LNX.4.64.0808111737330.30591@axis700.grange you wrote:
As I remarked in the opening email to the SMDK6400 patch-series, I've got a strange problem, I cannot understand.
What exactly is your problem?
which converts all accesses to the structure to 8-bit... Ideas? Toolchain eldk-4.1. U-Boot from nand/testing git with the patches I just sent to the list, configured for smdk6400_config.
Note that you did not exactly ask a question, you just describe your observations so we have to guess what might be puzzeling you. I think that some people with more ARM experience than me will simply not understand what you are asking about, because they are used to GCC turning perfectly valid looking 32 bit accesses into a sequence of four 8 bit accesses.
They consider this "normal".
Best regards,
Wolfgang Denk
On Mon, 11 Aug 2008, Wolfgang Denk wrote:
Dear Guennadi Liakhovetski,
In message Pine.LNX.4.64.0808111737330.30591@axis700.grange you wrote:
As I remarked in the opening email to the SMDK6400 patch-series, I've got a strange problem, I cannot understand.
What exactly is your problem?
Right, sorry, the problem is simple: it doesn't work with the byte access.
which converts all accesses to the structure to 8-bit... Ideas? Toolchain eldk-4.1. U-Boot from nand/testing git with the patches I just sent to the list, configured for smdk6400_config.
Note that you did not exactly ask a question, you just describe your observations so we have to guess what might be puzzeling you. I think that some people with more ARM experience than me will simply not understand what you are asking about, because they are used to GCC turning perfectly valid looking 32 bit accesses into a sequence of four 8 bit accesses.
They consider this "normal".
I cannot think of any such examples... I know access-types can be changed on the hardware, say, on a 16-bit bus a 32-bit access can be split into two 16-bit accesses, but I currently cannot think of any examples of the compiler doing such things. But, I think, I can imagine _why_ the compiler might decide to do that - it cannot guarantee, that the structure is 32-bit aligned, so, it is opting for the "safe" variant... So, the question is: is the compiler right and the c-code was wrong, or the other way round?
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
On Mon, 2008-08-11 at 18:23 +0200, Guennadi Liakhovetski wrote:
I cannot think of any such examples... I know access-types can be changed on the hardware, say, on a 16-bit bus a 32-bit access can be split into two 16-bit accesses, but I currently cannot think of any examples of the compiler doing such things. But, I think, I can imagine _why_ the compiler might decide to do that - it cannot guarantee, that the structure is 32-bit aligned, so, it is opting for the "safe" variant... So, the question is: is the compiler right and the c-code was wrong, or the other way round?
Both are "right" :). C has a lot of implementation specific stuff that can make life interesting. Simple thing like if char is signed or unsigned sometimes is really important.
If you have to make sure that the read/write is done in 16 bit the only option is to use assembler code.
On Mon, 11 Aug 2008, Kenneth Johansson wrote:
On Mon, 2008-08-11 at 18:23 +0200, Guennadi Liakhovetski wrote:
I cannot think of any such examples... I know access-types can be changed on the hardware, say, on a 16-bit bus a 32-bit access can be split into two 16-bit accesses, but I currently cannot think of any examples of the compiler doing such things. But, I think, I can imagine _why_ the compiler might decide to do that - it cannot guarantee, that the structure is 32-bit aligned, so, it is opting for the "safe" variant... So, the question is: is the compiler right and the c-code was wrong, or the other way round?
Both are "right" :). C has a lot of implementation specific stuff that can make life interesting. Simple thing like if char is signed or unsigned sometimes is really important.
If you have to make sure that the read/write is done in 16 bit the only option is to use assembler code.
I think, properly written mmapped-register access functions like readl() work perfectly too.
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
On Mon, 2008-08-11 at 19:00 +0200, Guennadi Liakhovetski wrote:
On Mon, 11 Aug 2008, Kenneth Johansson wrote:
On Mon, 2008-08-11 at 18:23 +0200, Guennadi Liakhovetski wrote:
I cannot think of any such examples... I know access-types can be changed on the hardware, say, on a 16-bit bus a 32-bit access can be split into two 16-bit accesses, but I currently cannot think of any examples of the compiler doing such things. But, I think, I can imagine _why_ the compiler might decide to do that - it cannot guarantee, that the structure is 32-bit aligned, so, it is opting for the "safe" variant... So, the question is: is the compiler right and the c-code was wrong, or the other way round?
Both are "right" :). C has a lot of implementation specific stuff that can make life interesting. Simple thing like if char is signed or unsigned sometimes is really important.
If you have to make sure that the read/write is done in 16 bit the only option is to use assembler code.
I think, properly written mmapped-register access functions like readl() work perfectly too.
what does the compiler do if you change the code to
return *(ulong*)&timers->TCNTO4;
On Mon, 11 Aug 2008, Kenneth Johansson wrote:
On Mon, 2008-08-11 at 19:00 +0200, Guennadi Liakhovetski wrote:
On Mon, 11 Aug 2008, Kenneth Johansson wrote:
On Mon, 2008-08-11 at 18:23 +0200, Guennadi Liakhovetski wrote:
I cannot think of any such examples... I know access-types can be changed on the hardware, say, on a 16-bit bus a 32-bit access can be split into two 16-bit accesses, but I currently cannot think of any examples of the compiler doing such things. But, I think, I can imagine _why_ the compiler might decide to do that - it cannot guarantee, that the structure is 32-bit aligned, so, it is opting for the "safe" variant... So, the question is: is the compiler right and the c-code was wrong, or the other way round?
Both are "right" :). C has a lot of implementation specific stuff that can make life interesting. Simple thing like if char is signed or unsigned sometimes is really important.
If you have to make sure that the read/write is done in 16 bit the only option is to use assembler code.
I think, properly written mmapped-register access functions like readl() work perfectly too.
what does the compiler do if you change the code to
return *(ulong*)&timers->TCNTO4;
Strangely, this doesn't help either... Nor does using readl(&timers->TCNTO4).
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
On 17:48 Mon 11 Aug , Guennadi Liakhovetski wrote:
Hi
As I remarked in the opening email to the SMDK6400 patch-series, I've got a strange problem, I cannot understand.
Here're the c-code:
typedef struct { volatile u32 TCNTB; volatile u32 TCMPB; volatile u32 TCNTO; } s3c64xx_timer;
typedef struct { volatile u32 TCFG0; volatile u32 TCFG1; volatile u32 TCON; s3c64xx_timer ch[4]; volatile u32 TCNTB4; volatile u32 TCNTO4; } s3c64xx_timers;
IIRC in gcc 4
you're supposed to declare the struct as packet and not the typedef
struct { volatile u32 TCNTB; volatile u32 TCMPB; volatile u32 TCNTO; } _s3c64xx_timer __attribute__ ((__packed__)); typedef struct _s3c64xx_timer s3c64xx_timer;
struct { volatile u32 TCFG0; volatile u32 TCFG1; volatile u32 TCON; s3c64xx_timer ch[4]; volatile u32 TCNTB4; volatile u32 TCNTO4; } _s3c64xx_timers __attribute__ ((__packed__)); typedef struct _s3c64xx_timers s3c64xx_timers;
Best Regards, J.
On Mon, 11 Aug 2008, Jean-Christophe PLAGNIOL-VILLARD wrote:
On 17:48 Mon 11 Aug , Guennadi Liakhovetski wrote:
Hi
As I remarked in the opening email to the SMDK6400 patch-series, I've got a strange problem, I cannot understand.
Here're the c-code:
typedef struct { volatile u32 TCNTB; volatile u32 TCMPB; volatile u32 TCNTO; } s3c64xx_timer;
typedef struct { volatile u32 TCFG0; volatile u32 TCFG1; volatile u32 TCON; s3c64xx_timer ch[4]; volatile u32 TCNTB4; volatile u32 TCNTO4; } s3c64xx_timers;
IIRC in gcc 4
you're supposed to declare the struct as packet and not the typedef
struct { volatile u32 TCNTB; volatile u32 TCMPB; volatile u32 TCNTO; } _s3c64xx_timer __attribute__ ((__packed__)); typedef struct _s3c64xx_timer s3c64xx_timer;
I believe you meant
struct _s3c64xx_timer { volatile u32 TCNTB; volatile u32 TCMPB; volatile u32 TCNTO; } __attribute__ ((__packed__));
but this doesn't change a thing, I checked it too. Even this
} __attribute__((__packed__,aligned(4)));
doesn't help.
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
On Mon, 11 Aug 2008, Guennadi Liakhovetski wrote:
but this doesn't change a thing, I checked it too. Even this
} __attribute__((__packed__,aligned(4)));
doesn't help.
Sorry, this last statement is wrong, adding aligned(4) does help.
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
On Mon, Aug 11, 2008 at 05:48:21PM +0200, Guennadi Liakhovetski wrote:
The respective assembly code is quite simple:
00000000 <read_timer>: 0: e59f3004 ldr r3, [pc, #4] ; c <timer_load_val> 4: e5930040 ldr r0, [r3, #64] 8: e12fff1e bx lr c: 7f006000 swivc 0x00006000
Perfect. Now, if I make the two structs above packed, I get this assembly:
00000000 <read_timer>: 0: e59f301c ldr r3, [pc, #28] ; 24 <.text+0x24> 4: e5d30040 ldrb r0, [r3, #64] 8: e5d32041 ldrb r2, [r3, #65] c: e5d31042 ldrb r1, [r3, #66] 10: e1800402 orr r0, r0, r2, lsl #8 14: e5d33043 ldrb r3, [r3, #67] 18: e1800801 orr r0, r0, r1, lsl #16 1c: e1800c03 orr r0, r0, r3, lsl #24 20: e12fff1e bx lr 24: 7f006000 swivc 0x00006000
which converts all accesses to the structure to 8-bit... Ideas? Toolchain eldk-4.1. U-Boot from nand/testing git with the patches I just sent to the list, configured for smdk6400_config.
Packed implies a lack of alignment. Since ARM can't do unaligned accesses in a "normal" way, the compiler avoids them.
Why do you need to pack the struct?
-Scott
On Mon, 11 Aug 2008, Scott Wood wrote:
On Mon, Aug 11, 2008 at 05:48:21PM +0200, Guennadi Liakhovetski wrote:
The respective assembly code is quite simple:
00000000 <read_timer>: 0: e59f3004 ldr r3, [pc, #4] ; c <timer_load_val> 4: e5930040 ldr r0, [r3, #64] 8: e12fff1e bx lr c: 7f006000 swivc 0x00006000
Perfect. Now, if I make the two structs above packed, I get this assembly:
00000000 <read_timer>: 0: e59f301c ldr r3, [pc, #28] ; 24 <.text+0x24> 4: e5d30040 ldrb r0, [r3, #64] 8: e5d32041 ldrb r2, [r3, #65] c: e5d31042 ldrb r1, [r3, #66] 10: e1800402 orr r0, r0, r2, lsl #8 14: e5d33043 ldrb r3, [r3, #67] 18: e1800801 orr r0, r0, r1, lsl #16 1c: e1800c03 orr r0, r0, r3, lsl #24 20: e12fff1e bx lr 24: 7f006000 swivc 0x00006000
which converts all accesses to the structure to 8-bit... Ideas? Toolchain eldk-4.1. U-Boot from nand/testing git with the patches I just sent to the list, configured for smdk6400_config.
Packed implies a lack of alignment. Since ARM can't do unaligned accesses in a "normal" way, the compiler avoids them.
Why do you need to pack the struct?
I don't. These structs are already filled with dummy bytes. So it also works without the packed attribute. Why I tried to enable it was because the attribute was in the original code from Samsung, but was commented out, so, I wanted to find out what happens if I re-enable it. Besides, the packed attribute serves as a hint in the source, that this struct has special requirements for its in-memory representation.
Thanks Guennadi --- Guennadi Liakhovetski, Ph.D.
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
participants (5)
-
Guennadi Liakhovetski -
Jean-Christophe PLAGNIOL-VILLARD -
Kenneth Johansson -
Scott Wood -
Wolfgang Denk