This is the mail archive of the ecos-devel@sources.redhat.com mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: CYG_HAL_TABLE_END alignment


>>>>> "Peter" == Peter Korsgaard <jacmet@sunsite.dk> writes:

    Peter> Why is CYG_HAL_TABLE_END aligned? It seems to be a
    Peter> conscious decission according to
    Peter> hal/common/current/ChangeLog:

    Peter> 2000-09-04  Jonathan Larmour  <jlarmour@redhat.com>

    Peter> * include/hal_tables.h (CYG_HAL_TABLE_END): Use CYGARC_P2ALIGNMENT
    Peter>   to align label

    Peter> But this gives errors if the size of the table elements
    Peter> aren't a multiple of the alignment (8 bytes).

    Peter> I had an issue with that today using the I2C
    Peter> infrastructure, since the cyg_i2c_bus structure is 44 bytes
    Peter> big with assertions enabled - causing an never ending loop
    Peter> in cyg_i2c_init::cyg_i2c_init.

I had not seen that before, but I think I understand the issues.
Suppose you are putting a structure like the following in a table:

    struct fred {
        cyg_uint32	a;
	cyg_uint8	b[3];
    }

On most architectures the compiler and linker will arrange for the
start of this structure to be aligned to a 4-byte boundary, i.e. a
byte of padding gets inserted. When the compiler iterates through the
table it thinks it has a fred[] array with each entry properly
aligned. The linker does not see an array, it gets a number of
individual variables, but it will align each one's start to the
appropriate boundary. So no problems so far.

Linker alignment only applies to the start of a structure, not the
end. Therefore when the linker places the end of the table label this
would appear immediately after the last entry, without the padding.
That is a bad idea so the end is explicitly aligned in the code.

Now, ideally the table end would be aligned to the same boundary as a
struct fred. Unfortunately the CYG_HAL_TABLE_END() was not written to
take an alignment argument, and instead it uses CYGARC_P2ALIGNMENT. On
many architectures (including the ones I have run the I2C code on) the
natural alignment is 4 bytes and there are no problems. However
CYGARC_P2ALIGNMENT refers to the largest alignment that may be needed
on the current architecture. It is possible that a struct fred will
work happily with something smaller, and the compiler will assume the
smaller alignment. Hence the linker has inserted too much padding, the
table is too large, and the i2c init code will try to init a
non-existant entry and fail to detect the end of the table.

Ideally the table macros would take an extra argument to indicate the
actual alignment needed for the structures in each table, i.e.
sizeof(struct fred), but since the tables are defined via inline
assembler that could get tricky. Also changing the macros at this
stage is probably not a good idea, they get used in many places.

Instead I suspect the cyg_i2c_bus structure should have been given an
appropriate attribute to increase its alignment to the maximum, at the
cost of a small amount of memory. That way the compiler and the linker
will agree on how all the table entries and the end get aligned, and
things should work. The following would probably do the trick.

Index: i2c.h
===================================================================
RCS file: /cvs/ecos/ecos/packages/io/i2c/current/include/i2c.h,v
retrieving revision 1.1
diff -u -r1.1 i2c.h
--- i2c.h	20 Apr 2005 12:25:30 -0000	1.1
+++ i2c.h	28 Apr 2005 19:24:08 -0000
@@ -92,7 +92,7 @@
     void                    (*i2c_stop_fn)(const cyg_i2c_device*);
     // A spare field for use by the driver
     void*                   i2c_extra;
-} cyg_i2c_bus;
+} CYG_HAL_TABLE_TYPE cyg_i2c_bus;
 
 #define CYG_I2C_BUS(_name_, _init_fn_, _tx_fn_, _rx_fn_, _stop_fn_, _extra_)    \
     cyg_i2c_bus _name_  CYG_HAL_TABLE_ENTRY( i2c_buses ) = {                    \

Bart

P.S. I'll take a look at your I2C patches when I get a chance,
probably this weekend.

-- 
Bart Veer                       eCos Configuration Architect
http://www.ecoscentric.com/     The eCos and RedBoot experts


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]