[ECOS] Type of alarm-function

[ECOS] Type of alarm-function

[Thomas Klöber]

Hello all,

I am porting eCos to the Infineon TriCore TC1796 as part of my studies. I am 
using the kernel testcases to verify it's working, most ones are running just 
fine. During stepping throug kclock0.c as it wouldn't work, I found the 
following:

The prototype for an alarm handler function in the C kernel api is

void alarm_handler(cyg_handle_t alarm, cyg_addrword_t data)

with cyg_handle_t being a cyg_uint32. The internal kernel prototype for 
calling the function however is

void cyg_alarm_fn(Cyg_Alarm *alarm, CYG_ADDRWORD data)

Note  that the first argument is a pointer in this case. While on most 
architectures this does not make any difference, it circumvents passing of 
parameters to the function.

The TC1796 has separate registers for addresses and for data which are used 
for passing parameters. When calling a function of type cyg_alarm_fn, the 
compiler puts the first argument into an address register (%a4) and the 
second one into a data register (%d4). Unfortunately, the called function has 
the type alarm_handler, so the compiler trys to read both values from data 
registers (%d4 and %d5) when entering the function.

The result is that neither of the arguments is passed correctly to the 
function as the first one lies in %a4 instead of %d4 and the second one lies 
in %d4 instead of %d5.

imho one of the prototypes for the function should be corrected to guarantee 
correct behaviour on all platforms.

Any opinions on this?


Tom

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Type of alarm-function

[Andrew Lunn]

On Wed, Mar 05, 2008 at 01:52:16PM +0100, Thomas Kl??ber wrote:
> Hello all,
> 
> I am porting eCos to the Infineon TriCore TC1796 as part of my studies. I am 
> using the kernel testcases to verify it's working, most ones are running just 
> fine. During stepping throug kclock0.c as it wouldn't work, I found the 
> following:
> 
> The prototype for an alarm handler function in the C kernel api is
> 
> void alarm_handler(cyg_handle_t alarm, cyg_addrword_t data)
> 
> with cyg_handle_t being a cyg_uint32. The internal kernel prototype for 
> calling the function however is
> 
> void cyg_alarm_fn(Cyg_Alarm *alarm, CYG_ADDRWORD data)
> 
> Note  that the first argument is a pointer in this case. While on most 
> architectures this does not make any difference, it circumvents passing of 
> parameters to the function.
> 
> The TC1796 has separate registers for addresses and for data which are used 
> for passing parameters. When calling a function of type cyg_alarm_fn, the 
> compiler puts the first argument into an address register (%a4) and the 
> second one into a data register (%d4). Unfortunately, the called function has 
> the type alarm_handler, so the compiler trys to read both values from data 
> registers (%d4 and %d5) when entering the function.
> 
> The result is that neither of the arguments is passed correctly to the 
> function as the first one lies in %a4 instead of %d4 and the second one lies 
> in %d4 instead of %d5.

Interesting behaviour. Never heard of this before. 

I suspect there is more broken than just alarms. In the C world a
cyg_handle_t is an abstract type used to represent an object. Inside
the kernel in the C++ world the handle is cast to a C++ object.

The alarm code uses the following to call the alarm function:

 // Call alarm function
alarm->alarm(alarm, alarm->data);

ie it passes the C++ alarm object to the C function and relies on the
fact that a handle and the object are the same thing. So probably all
callbacks from the kernel are broken for you. One that springs to mind
is the entry point to a thread. 

One fix might be to change the type of cyg_handle_t. 
typedef cyg_addrword_t cyg_handle_t;        /* Object handle
typedef CYG_ADDRWORD   cyg_addrword_t;      /* May hold pointer or word
typedef cyg_haladdrword CYG_ADDRWORD;

and in /packages/infra/current/include/cyg_type.h

#ifndef cyg_haladdrword
# define cyg_haladdrword cyg_uint32
#endif

Your HAL is allowed to override this definition. 

Does the compiler ABI have a type which can hold both an 32 bit word
and a pointer? 

You probably also want to look at 

#ifndef cyg_haladdress
# define cyg_haladdress cyg_uint32
#endif

in the same file.

   Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] Type of alarm-function

[Fabian Scheler]

I propose the following "fix"

>  typedef cyg_addrword_t* cyg_handle_t;

define cyg_handle_t as pointer to cyg_addrword_t. That way
cyg_handle_t would always be a real pointer and that is how it is
used. cyg_handle_t should only be used for pointers to kernel objects
(i.e. threads, mutexes, ...) , right?

We (Thomas to be more precise) checked this change by running the
kernel test cases and did not encounter any problems.

Ciao, Fabian

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss