What about making the Libmaple interrupt handlers "__weak"?

Post Reply
edogaldo
Posts: 283
Joined: Fri Jun 03, 2016 8:19 am

What about making the Libmaple interrupt handlers "__weak"?

Post by edogaldo » Mon Oct 30, 2017 3:56 pm

This could allow overriding them if needed..

victor_pv
Posts: 1682
Joined: Mon Apr 27, 2015 12:12 pm

Re: What about making the Libmaple interrupt handlers "__weak"?

Post by victor_pv » Tue Oct 31, 2017 3:03 am

edogaldo wrote:
Mon Oct 30, 2017 3:56 pm
This could allow overriding them if needed..
I thought some of them were weak already. This same thing was discussed a while back about the USART handlers, but I don't think anyone submitted a PR.
As far as myself, I see the benefit, since this same subject pops up in the forum every once in a while, and likely doesn't pop up more often because some people just goes and modify them without posting about it.

Let's see if you get more responses and no one has any objection.

Riva
Posts: 42
Joined: Fri May 06, 2016 6:42 am

Re: What about making the Libmaple interrupt handlers "__weak"?

Post by Riva » Wed Nov 01, 2017 8:57 am

I would like the __weak references as I suffered this same problem with the USART interrupts but the last posted suggestion was to just document the changes in the wiki and leave as is.

User avatar
ahull
Posts: 1633
Joined: Mon Apr 27, 2015 11:04 pm
Location: Sunny Scotland
Contact:

Re: What about making the Libmaple interrupt handlers "__weak"?

Post by ahull » Wed Nov 01, 2017 9:02 am

What (if any) downside is there to making them "__weak"?
- Andy Hull -

User avatar
RogerClark
Posts: 7179
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: What about making the Libmaple interrupt handlers "__weak"?

Post by RogerClark » Wed Nov 01, 2017 9:11 pm

ahull wrote:
Wed Nov 01, 2017 9:02 am
What (if any) downside is there to making them "__weak"?
I'm not sure if this is the case, but in the past we've had issues with the compiler / linker not including functions because it didt think they were needed.

From what I recall this may have been something to do with weak references

I think it had something to do with the timers, but it was a long time a go, so my memory is a bit hazy. I think the solution at one time was to use the WHOLE ARCHIVE switch in the compiler settings, but we don't use that any more, (as it resulted in loads of unused code being compiled into the binary)

Perhaps someone else can remember the problem and the solution, ... Sounds like something Rick may remember..

User avatar
Rick Kimball
Posts: 1043
Joined: Tue Apr 28, 2015 1:26 am
Location: Eastern NC, US
Contact:

Re: What about making the Libmaple interrupt handlers "__weak"?

Post by Rick Kimball » Thu Nov 02, 2017 12:21 am

viewtopic.php?f=16&t=270

that is a long thread discussing issues. I came to the personal conclusion that people should use attachInterrupt.
-rick

User avatar
RogerClark
Posts: 7179
Joined: Mon Apr 27, 2015 10:36 am
Location: Melbourne, Australia
Contact:

Re: What about making the Libmaple interrupt handlers "__weak"?

Post by RogerClark » Thu Nov 02, 2017 2:35 am

Rick.. Thanks

Looks like a big can of worms....

edogaldo
Posts: 283
Joined: Fri Jun 03, 2016 8:19 am

Re: What about making the Libmaple interrupt handlers "__weak"?

Post by edogaldo » Thu Nov 02, 2017 9:45 am

Hi, I tried this:
  • took the latest Libmaple core
  • commented out "__irq_usart1" section in "STM32F1\cores\maple\libmaple\stm32f1\performance\isrs.S":

    Code: Select all

    /*
    	.weak	__irq_usart1
    	.globl	__irq_usart1
    	.set	__irq_usart1, __default_handler
    */
    
  • defined "__irq_usart1()" as "__weak" in "STM32F1\cores\maple\libmaple\usart_f1.c":

    Code: Select all

    __weak void __irq_usart1(void) {
        usart_irq(&usart1_rb, &usart1_wb, USART1_BASE);
    }
    
  • compiled an empty sketch
Looking at the output asm, the reference to "__irq_usart1" in the vector table is correctly pointing to the Libmaple "__irq_usart1()" definition:

Code: Select all

08002000 <__stm32_vector_table>:
 8002000:	00 50 00 20 45 21 00 08 d1 23 00 08 d5 23 00 08     .P. E!...#...#..
 8002010:	d9 23 00 08 dd 23 00 08 e1 23 00 08 31 24 00 08     .#...#...#..1$..
 8002020:	31 24 00 08 31 24 00 08 31 24 00 08 31 24 00 08     1$..1$..1$..1$..
 8002030:	31 24 00 08 31 24 00 08 31 24 00 08 09 28 00 08     1$..1$..1$...(..
 8002040:	31 24 00 08 31 24 00 08 31 24 00 08 31 24 00 08     1$..1$..1$..1$..
 8002050:	31 24 00 08 31 24 00 08 31 24 00 08 31 24 00 08     1$..1$..1$..1$..
 8002060:	31 24 00 08 31 24 00 08 31 24 00 08 31 24 00 08     1$..1$..1$..1$..
 8002070:	31 24 00 08 31 24 00 08 31 24 00 08 31 24 00 08     1$..1$..1$..1$..
 8002080:	31 24 00 08 31 24 00 08 89 24 00 08 31 24 00 08     1$..1$...$..1$..
 8002090:	e1 32 00 08 31 24 00 08 31 24 00 08 31 24 00 08     .2..1$..1$..1$..
 80020a0:	b1 28 00 08 d5 28 00 08 f9 28 00 08 31 29 00 08     .(...(...(..1)..
 80020b0:	85 29 00 08 f5 29 00 08 65 2a 00 08 31 24 00 08     .)...)..e*..1$..
 80020c0:	31 24 00 08 31 24 00 08 31 24 00 08 31 24 00 08     1$..1$..1$..1$..
 80020d0:	31 24 00 08 >>>>>4d 2b 00 08<<<<< a1 2b 00 08 f5 2b 00 08     1$..M+...+...+..
 80020e0:	31 24 00 08 31 24 00 08 31 24 00 08                 1$..1$..1$..
[...]
__default_handler:
	b .
 8002430:	e7fe      	b.n	8002430 <__default_handler>
[...]
08002b4c <__irq_usart1>:

/*
 * Interrupt handlers.
 */

__weak void __irq_usart1(void) {
 8002b4c:	b538      	push	{r3, r4, r5, lr}
    /* Handling RXNEIE and TXEIE interrupts. 
     * RXNE signifies availability of a byte in DR.
     *
     * See table 198 (sec 27.4, p809) in STM document RM0008 rev 15.
     * We enable RXNEIE. */
    if ((regs->CR1 & USART_CR1_RXNEIE) && (regs->SR & USART_SR_RXNE)) {
 8002b4e:	4d11      	ldr	r5, [pc, #68]	; (8002b94 <__irq_usart1+0x48>)
 8002b50:	68eb      	ldr	r3, [r5, #12]
 8002b52:	0698      	lsls	r0, r3, #26
 8002b54:	d507      	bpl.n	8002b66 <__irq_usart1+0x1a>
 8002b56:	682b      	ldr	r3, [r5, #0]
So it seems to me that it's enough not to map the default handler for the Libmaple handled interrupt handlers in "isrs.S" to make the compiler use the Libmaple handler even if defined as "__weak".
This, by the way, makes sense to me as there is no need to map the default handler for library handled ISRs.

One could then redefine the ISR in the sketch with:

Code: Select all

extern "C" void __irq_usart1(void) {
  // put your interrupt handler code here:
}
Best, E.

stevestrong
Posts: 1749
Joined: Mon Oct 19, 2015 12:06 am
Location: Munich, Germany

Re: What about making the Libmaple interrupt handlers "__weak"?

Post by stevestrong » Thu Nov 02, 2017 12:01 pm

It seems that it is already declared as "weak" in the .S file, so wouldn't be enough just to comment out the set line?

Code: Select all

	.weak	__irq_usart1
	.globl	__irq_usart1
//	.set	__irq_usart1, __default_handler

ag123
Posts: 775
Joined: Thu Jul 21, 2016 4:24 pm

Re: What about making the Libmaple interrupt handlers "__weak"?

Post by ag123 » Fri Nov 17, 2017 3:43 pm

i think edogaldo's code snippets actually looked interesting, it would perhaps allow an isr from the sketch to live in flash rather than occupy some memory via attach_interrupt and often it means an additional call from the original isr with attach_interrupt
i've not really studied libmaple's codes in that aspects in much detail, i'd leave it as a casual observation.
isr are important in arm cortex-m mcus, but i'd guess myself and most arduino users never quite explored that far when starting out, finding them rather complex to deal with. there are things for various exceptions / faults handling which most of us i'd guess won't bother to hook the isrs till some difficult freezes / stalls is encountered. even then my guess is like me the first thoughts is fire up eclipse, openocd, gdb and debug away (if possible) :lol:

Post Reply