cpu: centralized cortex initialization and some defines by haukepetersen · Pull Request #3100 · RIOT-OS/RIOT

haukepetersen · 2015-05-27T21:16:00Z

Based on #3023

added a centralized core implementation for all cortex CPUs
moved default stack size defines to cpu.h in cortexm_common
moved uart0 bufsize define to cpu.h in cortexm_common
moved typed of panic_t to cpu.h in cortexm_common
adapted all effected CPUs

EDIT:
Forgot to explain one feature: The device specific interrupt priorities are now initialized on startup to a default value. As we are using only one interrupt priority for everything to prevent interrupt preemtion, this saves the need to set the interrupt priority in during peripheral initialization -> helps to keep the periph code smaller.

jnohlgard · 2015-05-27T22:39:19Z

Impressive work

jnohlgard · 2015-05-27T23:01:36Z

is this WIP?
cortexm_init() should be called from all of the affected CPUs during cpu_init().

haukepetersen · 2015-05-28T09:25:05Z

not really WIP, I just forgot the cortexm_init()... Thx for reminding me, I will fix this ASAP

jnohlgard · 2015-05-28T09:30:32Z

Let's try to get this merged asap so that we don't accumulate more dependencies.

haukepetersen · 2015-05-28T12:53:25Z

+1!

haukepetersen · 2015-05-28T13:34:41Z

added those missing calls to cortexm_init(). Will rebase this PR once #3023 is merged...

jnohlgard · 2015-05-28T16:47:25Z

OK to squash.

haukepetersen · 2015-05-28T17:30:25Z

will do when I rebase after the merge of #3023.

jnohlgard · 2015-05-29T07:55:10Z

#3023 is merged, please rebase

haukepetersen · 2015-05-29T09:47:42Z

did, but broke something... Fixing it as we speak.

haukepetersen · 2015-05-29T09:59:27Z

fixed. Ready for final review, Travis and hopefully merging.

jnohlgard · 2015-05-29T10:14:05Z

cpu/cortexm_common/include/cpu.h

This TODO reference is something that I have been curious about for some time, what does it mean? Is there an open issue for this? Is this related to the COREIF_NG definition found in most (all?) platform makefiles?

yes, there is: #1892

Once in a time, boards/cpus used dINT and eINT macros for enabling/disabling interrupts. Then the current API with enableIRQ and disableIRQ was introduced. But there are still some occurances of the old calls left, so we were not able to remove them so far...

jnohlgard · 2015-05-29T10:19:50Z

How are the Cortex-M common IRQs affected by this change? Will they have a higher priority than the vendor specific IRQs since the default IRQ prio is set to 1?

jnohlgard · 2015-05-29T10:24:52Z

I think the code looks good, waiting for Travis. The question of IRQ priorities is something to keep in mind though, since the preemption order of the ISR may have been affected.

@haukepetersen Thank you for doing this large and somewhat ungrateful work for the long term greater good of the OS.

haukepetersen · 2015-05-29T10:25:21Z

They are not. We use only two of them, PendSV and SVC, which are both set explicitly in cortexm_init. The others we do not use (or even panic when they encounter, so their prios to not really matter...).

haukepetersen · 2015-05-29T10:27:03Z

@gebart: by the way, did you notice the implicitly included solution on handling cortex-M4 CPUs without FPU? This should be taken care of for once and all...

jnohlgard · 2015-05-29T10:28:25Z

@haukepetersen

They are not. We use only two of them, PendSV and SVC, which are both set explicitly in cortexm_init. The others we do not use (or even panic when they encounter, so their prios to not really matter...).

That clears things up. Thanks for the explanation

@gebart: by the way, did you notice the implicitly included solution on handling cortex-M4 CPUs without FPU? This should be taken care of for once and all...

I did see some special handling of M4F in the makefiles, but the context switching is still broken with regards to floating point registers, right?

jnohlgard · 2015-05-29T10:31:18Z

@haukepetersen but the M4F stuff was in #3023, I am not sure what you mean.

haukepetersen · 2015-05-29T10:36:53Z

oh yep, it was in #3023. I just had looked at your comment in #1366 in wanted to point out that that concern is solved...

But you are right, the handling of the FPU registers on context switching is still not implement. This could lead to corrupted data if more than one thread is using the FPU simultaneously...

kaspar030 · 2015-05-29T10:39:46Z

the handling of the FPU registers on context switching is still not implement. This could lead to corrupted data if more than one thread is using the FPU simultaneously...

Shouldn't we then disable hard FPU until this is sorted out?

haukepetersen · 2015-05-29T10:55:59Z

might not be the worst idea. But unrelated to this PR... How about we move that discussion to #1366?

jnohlgard · 2015-05-29T10:57:41Z

cpu/cortexm_common/cortexm_init.c

Could we leave this in but commented out, until #1366 is resolved?

Not really. If we leave this out, the CPU goes into hardfault as soon as it encounters a 'float' somewhere. We can only leave this out, as soon as we disabled the FPU compiler options or changed them to use soft float explicitly... I will prepare another PR for this.

And remember, as long as there is only one thread using the FPU at a time, everything works fine at the moment... :-)

I smell hard-to-find bugs...

IMHO we should make this an opt-in pseudomodule until this is sorted out.

Thats why I'm just preparing a PR to fix this...

done: #3112

Regarding the FPU initialization above: I makes much sense to leave it in. With #3112 the FPU should not be used. But if it is under some unforeseeable circumstances is used (though I don't know how this could happen), this initialization would prevent the CPU from going into hard-fault.

OK, I see your points.

haukepetersen · 2015-05-29T11:16:44Z

fixed issues with uart0

jnohlgard · 2015-05-29T12:13:30Z

sys/include/board_uart0.h

is not cpu_conf.h always included by cpu.h?

not on the arm7 and the msp430...

on these platforms it will be as soon as we clean them up and move them to the current structure...

- added a centralized core implementation for all cortex CPUs - moved default stack size defines to cpu.h in cortexm_common - moved uart0 bufsize define to cpu.h in cortexm_common - moved typed of panic_t to cpu.h in cortexm_common

haukepetersen · 2015-05-29T14:44:35Z

something went wrong while rebasing with the saml21-xpro, fixed now.

Some CPUs (e.g. cortex-m based ones) define the UART0_BUFSIZE in cpu.h. So also include this file here.

jnohlgard · 2015-05-30T07:21:28Z

ACK
Travis is green, go

cpu: centralized cortex initialization and some defines

haukepetersen assigned jnohlgard May 27, 2015

haukepetersen added this to the Release 2015.06 milestone May 27, 2015

haukepetersen mentioned this pull request May 28, 2015

cpu: RIOT port for Freescale Kinetis CPUs #2188

Closed

45 tasks

jfischer-no mentioned this pull request May 29, 2015

kinetis: adaption for common makefiles #3099

Merged

haukepetersen force-pushed the opt_cortexm_init branch from 7f2ebd1 to 8b739fa Compare May 29, 2015 09:46

haukepetersen force-pushed the opt_cortexm_init branch from 8b739fa to 8c81e59 Compare May 29, 2015 09:58

haukepetersen added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label May 29, 2015

jnohlgard reviewed May 29, 2015
View reviewed changes

haukepetersen force-pushed the opt_cortexm_init branch from d948843 to 9ea6b83 Compare May 29, 2015 11:16

jnohlgard reviewed May 29, 2015
View reviewed changes

haukepetersen added 8 commits May 29, 2015 16:42

cpu/cortexm_common: centralized init and defines

78d65a4

- added a centralized core implementation for all cortex CPUs - moved default stack size defines to cpu.h in cortexm_common - moved uart0 bufsize define to cpu.h in cortexm_common - moved typed of panic_t to cpu.h in cortexm_common

cpu/cc2538: adapted to centralized cpu conf

52e51bf

cpu/k60: adapted to centralized cpu conf

d4d34a7

cpu/kinetis_common: fixes to fault_handlers

de31e1f

cpu/kw2x: adapted to centralized cpu conf

6102896

cpu/lpc1768: adapted to centralized cpu conf

aa728e0

cpu/nrf51822: adapted to centralized cpu conf

a97cdaa

cpu/sam3x8e: adapted to centralized cpu conf

4d3b147

haukepetersen force-pushed the opt_cortexm_init branch from 9ea6b83 to e2cb8e7 Compare May 29, 2015 14:44

haukepetersen added 8 commits May 29, 2015 16:44

cpu/saml21: adapted to centralized cpu conf

3c48722

cpu/samd21: adapted to centralized cpu conf

dfb717c

cpu/stm32f0: adapted to centralized cpu conf

cb476da

cpu/stm32f3: adapted to centralized cpu conf

c6d65fe

cpu/stm32f4: adapted to centralized cpu conf

79d9391

cpu/stm32f1: adapted to centralized cpu conf

0e61ec0

cpu/stm32l1: adapted to centralized cpu conf

f48ea33

sys/uart0: include cpu.h in board_uart0.h

e2cb8e7

Some CPUs (e.g. cortex-m based ones) define the UART0_BUFSIZE in cpu.h. So also include this file here.

jnohlgard pushed a commit that referenced this pull request May 30, 2015

Merge pull request #3100 from haukepetersen/opt_cortexm_init

41e1b57

cpu: centralized cortex initialization and some defines

jnohlgard merged commit 41e1b57 into RIOT-OS:master May 30, 2015

haukepetersen deleted the opt_cortexm_init branch May 30, 2015 09:46

Conversation

haukepetersen commented May 27, 2015

Uh oh!

jnohlgard commented May 27, 2015

Uh oh!

jnohlgard commented May 27, 2015

Uh oh!

haukepetersen commented May 28, 2015

Uh oh!

jnohlgard commented May 28, 2015

Uh oh!

haukepetersen commented May 28, 2015

Uh oh!

haukepetersen commented May 28, 2015

Uh oh!

jnohlgard commented May 28, 2015

Uh oh!

haukepetersen commented May 28, 2015

Uh oh!

jnohlgard commented May 29, 2015

Uh oh!

haukepetersen commented May 29, 2015

Uh oh!

haukepetersen commented May 29, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnohlgard commented May 29, 2015

Uh oh!

jnohlgard commented May 29, 2015

Uh oh!

haukepetersen commented May 29, 2015

Uh oh!

haukepetersen commented May 29, 2015

Uh oh!

jnohlgard commented May 29, 2015

Uh oh!

jnohlgard commented May 29, 2015

Uh oh!

haukepetersen commented May 29, 2015

Uh oh!

kaspar030 commented May 29, 2015

Uh oh!

haukepetersen commented May 29, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haukepetersen commented May 29, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haukepetersen commented May 29, 2015

Uh oh!

jnohlgard commented May 30, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone