Jump to content
Sign in to follow this  
energia

declare ISR to be weak

Recommended Posts

Is there any way to make an ISR weak so that it can be overridden by user code?

The problem I have is that in the Energia core I have a set of timer ISR's that are used for PWM. I would like to make those weak so that a library can declare the same ISR which would then override the ISR in the core.

Without this I am getting the "multiple definition of `__isr_XX'" as one would expect.

 

Thanks a bunch!

 

Robert

 

Share this post


Link to post
Share on other sites

I don't think __isr_XX is defined. It is in the crt0 object that gets linked in. As far as I can see __isr_XX is not visible at compile time.

#if defined __isr_53
#undef __isr_53
#error "__isr_53_is defined"
#endif
The above does not generate a compilation error.
 
Thanks for brainstorming with me on this though!

Share this post


Link to post
Share on other sites

You don't really need a separate declaration in a header; it should be enough to put __attribute__((__weak__)) on the definition of the function. I do this sort of thing in my Cortex-M infrastructure. If you do have both, it may be necessary that they be consistent.

 

For mspgcc, the compiler detects that the function has an __interrupt__ attribute and adds a second name to the assembly source which corresponds to the name crt0 is looking for, with the value being the address of the handler. crt0 in turn has a weak definition for that symbol that resolves to the default handler, so you do need to be careful about ordering your object files so the right definition is found first.

 

You can also do something like the following, so you could make the PWM interrupt implementation available in a function named (e.g.) PWM_handler, then select which timer interrupt invokes it in an application-specific implementation file. You might need to use the actual symbol name __irq_# for this to work.

/* Provide a weak alias that will resolve to the unlimitedstack
 * implementation in the case where _sbrk() is requested.  This
 * matches the nosys behavior of newlib. */
void * _sbrk (ptrdiff_t increment) __attribute__((__weak__,__alias__("_bspacm_sbrk_unlimitedstack")));

Share this post


Link to post
Share on other sites

It would be nice to have this on the LM4F version of Energia as well.

I have been porting a library that needs a Timer.

I thought having weak timer handlers that I could just override would be handier than having to use IntRegister

 

For the weak handling there one can just use the examples provided by CMSIS.

Share this post


Link to post
Share on other sites

Thanks for the feedback @@pabigot and thanks Rick for getting this further along.

 

I thought I had overcome the issue by placing the declaration in a header file. I was looking at the wrong elf file so the issue remains. Below is what I tried:

 

1: Attribute the function declaration like below.

void TA0_CCR_updater() __attribute__((interrupt(TIMER0_A0_VECTOR),__weak__));

2: Attribute the function definition like below:

__attribute__((interrupt(TIMER0_A0_VECTOR),__weak__))
void TA0_CCR_updater()
{
  /* ... */
}

3: Also tried with replacing __weak__ with weak.

 

All these attempts still result in multiple definition of `__isr_53'.

 

The weak attributed function is in an archive (core.a) and the user library function is in an object file. The linker order is user files and then the core archive file.

 

I also tried working with the __alias__ attribute but this is a beyond my knowledge of how the internals work and what to place where.

 

Any further insight / help would be greatly appreciated!

 

Robert

Share this post


Link to post
Share on other sites

I suspect that as long as the __interrupt__ attribute is used mspgcc will generate a non-weak global symbol corresponding to the specific interrupt. The only solution that comes immediately to mind is to not provide that attribute, but instead use the alias syntax with the required specific symbol, which would be something like:

void TA0_CCR_updater () { }
#if TIMER0_A0_VECTOR == 0x6A
void __isr_53 (void) __attribute__((__weak__, __alias__("TA0_CCR_updater")));
#elif TIMER0_A0_VECTOR == 0x60
void __isr_48 (void) __attribute__((__weak__, __alias__("TA0_CCR_updater")));
#else
...
#endif
The ISR number should be the vector number divided by 2; I don't remember that there are exceptions to that rule.

 

I believe this should go into an implementation file; compile with -S and look at the generated code to ensure the weak and alias declarations are present. The assembly directives will look something like this:

        .weak   __isr_53
        .set    __isr_53,TA0_CCR_updater

Share this post


Link to post
Share on other sites

@@pabigot thanks for the pointer! I gave this a try and I am indeed seeing the .set and I end up with my __isr_53.

However There must be something else that the __interrupt__ attribute is doing to it since the function is missing the reti and instead it has a ret. See objdump output below:

 

Function with __attribute__((interrupt(TIMER0_A0_VECTOR)))

000046f0 <TA0_CCR_updater>:

#if defined(__MSP430_HAS_TA5__) || defined(__MSP430_HAS_T0A5__) 
volatile uint16_t timera0_ccr_dblbuf[4];
__attribute__((interrupt(TIMER0_A0_VECTOR)))
void TA0_CCR_updater()
{
    46f0:       2f 15           pushm   #3,     r15     
        TA0CCR1 = timera0_ccr_dblbuf[0];
    46f2:       3e 40 1a 24     mov     #9242,  r14     ;#0x241a
    46f6:       0f 4e           mov     r14,    r15     
    46f8:       3d 4f           mov     @r15+,  r13     
    46fa:       82 4d 54 03     mov     r13,    &0x0354 
        TA0CCR2 = timera0_ccr_dblbuf[1];
    46fe:       a2 4f 56 03     mov     @r15,   &0x0356 
        TA0CCR3 = timera0_ccr_dblbuf[2];
    4702:       0f 4e           mov     r14,    r15     
    4704:       2f 52           add     #4,     r15     ;r2 As==10
    4706:       a2 4f 58 03     mov     @r15,   &0x0358 
        TA0CCR4 = timera0_ccr_dblbuf[3];
    470a:       3e 50 06 00     add     #6,     r14     ;#0x0006
    470e:       a2 4e 5a 03     mov     @r14,   &0x035a 
        TA0CCTL0 &= ~CCIE;
    4712:       1f 42 42 03     mov     &0x0342,r15     
    4716:       3f f0 ef ff     and     #-17,   r15     ;#0xffef
    471a:       82 4f 42 03     mov     r15,    &0x0342 
}
    471e:       2d 17           popm    #3,     r15     
    4720:       00 13           reti
#endif

Same function but this time aliased with void __isr_53 (void) __attribute__((__weak__, __alias__("TA0_CCR_updater")));

000046f0 <TA0_CCR_updater>:

#if defined(__MSP430_HAS_TA5__) || defined(__MSP430_HAS_T0A5__) 
volatile uint16_t timera0_ccr_dblbuf[4];
__attribute__((interrupt(TIMER0_A0_VECTOR)))
void TA0_CCR_updater()
{
    46f0:       3e 40 1a 24     mov     #9242,  r14     ;#0x241a
    46f4:       0f 4e           mov     r14,    r15     
    46f6:       3d 4f           mov     @r15+,  r13     
    46f8:       82 4d 54 03     mov     r13,    &0x0354 
        TA0CCR1 = timera0_ccr_dblbuf[0];
    46fc:       a2 4f 56 03     mov     @r15,   &0x0356 
        TA0CCR2 = timera0_ccr_dblbuf[1];
    4700:       0f 4e           mov     r14,    r15     
    4702:       2f 52           add     #4,     r15     ;r2 As==10
    4704:       a2 4f 58 03     mov     @r15,   &0x0358 
        TA0CCR3 = timera0_ccr_dblbuf[2];
    4708:       3e 50 06 00     add     #6,     r14     ;#0x0006
    470c:       a2 4e 5a 03     mov     @r14,   &0x035a 
        TA0CCR4 = timera0_ccr_dblbuf[3];
    4710:       b2 f0 ef ff     and     #-17,   &0x0342 ;#0xffef
    4714:       42 03 
        TA0CCTL0 &= ~CCIE;
    4716:       30 41           ret                     

00004718 <TA1_CCR_updater>:
#endif

Thanks,

 

Robert

Share this post


Link to post
Share on other sites

@@energia Try this version. I remembered that if you don't provide a specific interrupt, I had decided to skip the global symbol part, with the idea that you need to put it into the vector table explicitly (which can be done with the weak alias).

#include <msp430.h>
__attribute__((__interrupt__))
void TA0_CCR_updater()
{
  /* ... */
}

void __isr_53 (void) __attribute__((__weak__,__alias__("TA0_CCR_updater")));

Share this post


Link to post
Share on other sites

@@pabigot that did the trick to make it look like an isr. We are getting closer. The issue now is that for whatever reason the __weak__ attribute places the function at the right address but it seems like it is never called. I do not know how to interpret the interrupt vector table but could it be that it does not get installed?

 

Looking at the vector table without the weak attribute I see "f0 46" at offset 0x6A (__isr_53?) which is the address of my ISR:

0000ff80 <__ivtbl_64>:
    ff80:       ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff     ................
    ff90:       ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff     ................
    ffa0:       ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff     ................
    ffb0:       ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff     ................
    ffc0:       5e 44 5e 44 5e 44 5e 44 5e 44 5e 44 5e 44 5e 44     ^D^D^D^D^D^D^D^D
    ffd0:       5e 44 5e 44 5e 44 5e 44 42 47 5e 44 5e 44 5e 44     ^D^D^D^DBG^D^D^D
    ffe0:       5e 44 22 47 5e 44 5e 44 5e 44 f0 46 5c 4a 5e 44     ^D"G^D^D^D.F\J^D
    fff0:       5e 44 7e 46 5e 44 5e 44 5e 44 5e 44 5e 44 00 44     ^D~F^D^D^D^D^D.D

However, the one with the weak attribute seems to have the default at that offset:

0000ff80 <__ivtbl_64>:
    ff80:       ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff     ................
    ff90:       ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff     ................
    ffa0:       ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff     ................
    ffb0:       ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff     ................
    ffc0:       5e 44 5e 44 5e 44 5e 44 5e 44 5e 44 5e 44 5e 44     ^D^D^D^D^D^D^D^D
    ffd0:       5e 44 5e 44 5e 44 5e 44 42 47 5e 44 5e 44 5e 44     ^D^D^D^DBG^D^D^D
    ffe0:       5e 44 22 47 5e 44 5e 44 5e 44 5e 44 5c 4a 5e 44     ^D"G^D^D^D^D\J^D
    fff0:       5e 44 7e 46 5e 44 5e 44 5e 44 5e 44 5e 44 00 44     ^D~F^D^D^D^D^D.D

Share this post


Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Sign in to follow this  

×
×
  • Create New...