Refactoring fault handler implementation

[SenRamakri]

Description

Mbed Fault Handler was originally placed under rtos folder because of its dependency on some of the RTX data structures for capturing the thread info. But with the new error handling in place thread info collection on hardfaults has been moved to Mbed_error handler. There is no point for fault handler implementation to exist under rtos and can be used for RTOS-less builds as well. So moving under platform folder. Also removing some references to RTX data structs like osRtxInfo from fault handler implementation.

Pull request type

[ ] Fix
[x] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

[0xc0170]

Changes look good. Refactor like this should contain more info in the commit msg

Mbed Fault Handler was originally placed under rtos folder because of its dependency on some of the RTX data structures for capturing the thread info. But with the new error handling in place thread info collection on hardfaults has been moved to Mbed_error handler. There is no point for fault handler implementation to exist under rtos and can be used for RTOS-less builds as well. So moving under platform folder. Also removing some references to RTX data structs like osRtxInfo from fault handler implementation.

Can this be added there?

[studavekar]

LGTM

[SenRamakri]

@0xc0170 - Commit msg has been updated, please review.

Rereview requested

[deepikabhavnani]

I am not sure if adding TARGET_ and TOOLCHAIN_ specific folders in platform is a good idea. I would rather move Cortex-M specific toolchain dependent code to CMSIS folder (if not dependent on RTOS)

[0xc0170]

I am not sure if adding TARGET_ and TOOLCHAIN_ specific folders in platform is a good idea. I would rather move Cortex-M specific toolchain dependent code to CMSIS folder (if not dependent on RTOS)

I would not expect to have these folders in platform or drivers. CMSIS sounds as a good place to have these in. What others think?

[SenRamakri]

I don't mind moving these under CMSIS, but the exception handler is no longer RTOS specific or CMSIS-specific and can be used by RTOS-less or CMSIS-less builds as well. The only reason I can think of is usage of SCB struct which we can get rid of as well. Should we still move it out?

[deepikabhavnani]

Yes @SenRamakri though it is not Cmsis dependent but core specific code is in CMSIS..

[kjbracey]

I don't see any particular reason why we couldn't have TARGET_ folders under platform, and this code is more closely related to platform than anything in cmsis.

But then the start up code which calls mbed_main and main is over in there, and this exception stuff calling mbed_fault_handler probably belongs alongside it.

[kjbracey]

Just to save a couple of words, I'm wondering why

LDR R3,=mbed_fault_handler
BLX R3

and not just

BL mbed_fault_handler

(Actually, there are tons of potential savings in this assembler, but that's a straightforward de-obfustication.)

Changes addressed. Please re-review.

[SenRamakri]

Please review, I have addressed the review comments.

[deepikabhavnani]

Looks good to me

[cmonr]

I don't see any particular reason why we couldn't have TARGET_ folders under platform, and this code is more closely related to platform than anything in cmsis.

I think the particular reason for that would be that it would require a tweak of the tools to handle the updated structure. In any case, it looks like your optimization comments have been addressed.

/morph build

[mbed-ci]

Build : SUCCESS

Build number : 3447
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8332/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

[mbed-ci]

Exporter Build : SUCCESS

Build number : 3068
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/8332/

[cmonr]

New greentea tests should be complete within two hours.

[mbed-ci]

Test : SUCCESS

Build number : 3238
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8332/3238

[0xc0170]

New greentea tests should be complete within two hours.

Yes not they are, waiting for CI !