Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Silicon Labs] Adding support for EFR32MG1 wireless SoC #3157

Merged
merged 17 commits into from
Dec 2, 2016

Conversation

stevew817
Copy link
Contributor

Description

This pull request is adding support for the EFR32MG1 family of wireless SoCs. Functionality-wise, this port should be complete. We have also created a driver for using the radio wih Nanostack.

Status

READY

Migrations

NO

Deploy notes

This PR touches the mbed-added platform header for mbed TLS, in order to provide support for hardware acceleration. The configuration for that is the same as for enabling TRNG, i.e. through a "device_has" entry.

Steps to test or reproduce

Compile for THUNDERBOARD_SENSE

Adding target definitions and the HAL implementation for EFR32 Mighty Gecko
Initial check-in of hardware acceleration support on EFR32 for mbed TLS (AES, SHA and ECC).
First check-in of the EFR32 radio driver for Nanostack
@0xc0170
Copy link
Contributor

0xc0170 commented Oct 28, 2016

@stevew817 Can you share the tests results please?

@stevew817
Copy link
Contributor Author

@0xc0170 I'll get you the test results after the weekend.

@stevew817
Copy link
Contributor Author

Test results pasted under. I opened #3215 to cover the two failing tests, since IMO the two sub-test fails are due to a testing methodology issue.

RESULTS2.txt

Silicon Labs targets use a default, unchangeable baud rate of 115200 on the stdio serial-USB bridge.
@stevew817
Copy link
Contributor Author

@0xc0170 @bridadan Please review.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 7, 2016

@sbutcher-arm @yanesca : please review, regarding mbedtls - commit c0301b1

Copy link
Contributor

@bridadan bridadan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because of the size of the diff I can't comment on the specific line, but in targets.json, you've added the target EFR32MG1P132f256GM48, which contains a lowercase f in it. All target names should use capital letters, could you make this change please?

Also, I see that you've added the extra label 256K to the target however I can't see from the diff where it's used (GitHub hiding half the diff isn't helping here). Can you explain this a bit?

And lastly, I see that you've added the Greentea change here to respect the stdout baud rate. This is a good change, however do you think you could submit it as a separate PR so it can have a bit more visibility since it affects the testing of all platforms?

@bridadan
Copy link
Contributor

bridadan commented Nov 7, 2016

Also, do you have test results for IAR and ARM compilers as well?

@stevew817
Copy link
Contributor Author

@bridadan Sure.

  • I fixed the stray lowercase. Good catch!
  • The extra 256K label is to switch between different linker scripts depending on the flash size (as we're doing for EFM32). Currently, the EFR32 series only has 256K parts, but this is not going to remain the case in the future.
  • It shouldn't affect any other platforms, since MBED_CONF_PLATFORM_STDIO_BAUD_RATE is defined to 9600 for all platforms except EFM32/EFR32?
  • I'll run through ARM and IAR tonight

@stevew817
Copy link
Contributor Author

@bridadan Here are the test results. Same deepsleep timer/ticker tests are failing (see my previous comment about those), but there's a few rtos fails on IAR. Do you know more about those? It seems strange to me that these would only fail on IAR.

RESULTS-IAR.txt
RESULTS-ARM.txt

@stevew817
Copy link
Contributor Author

Update on IAR: the tests-mbedmicro-rtos-mbed-basic test is passing on multiple subsequent runs. I think the fail exhibited on the documented run is due to something else that was running on my machine, messing with the input timing (since the failure was due to a drift check).

@stevew817
Copy link
Contributor Author

Update 2: All tests are passing on IAR too, after reducing the stack size on the failing tests. I keep wondering, why doesn't mbed set the stack size inside of the tests to something more bite-sized instead of DEFAULT_STACK_SIZE?

@bridadan
Copy link
Contributor

bridadan commented Nov 8, 2016

Thanks for the changes! The targets.json fix looks good!

The extra 256K label is to switch between different linker scripts depending on the flash size (as we're doing for EFM32). Currently, the EFR32 series only has 256K parts, but this is not going to remain the case in the future.

OK good to know!

It shouldn't affect any other platforms, since MBED_CONF_PLATFORM_STDIO_BAUD_RATE is defined to 9600 for all platforms except EFM32/EFR32?

It shouldn't, yes. I also suggested putting it in a separate PR so if we needed to revert it it'd be easy to revert just that change and not this whole PR. It's just a preference of mine, but it doesn't need to block this PR.

Update on IAR: the tests-mbedmicro-rtos-mbed-basic test is passing on multiple subsequent runs. I think the fail exhibited on the documented run is due to something else that was running on my machine, messing with the input timing (since the failure was due to a drift check).

Yes, unfortunately the timing tests are susceptible to failures if the host machine is under load. If it passed on subsequent runs then that should be ok.

Update 2: All tests are passing on IAR too, after reducing the stack size on the failing tests

Can you update your PR with these changes?

I keep wondering, why doesn't mbed set the stack size inside of the tests to something more bite-sized instead of DEFAULT_STACK_SIZE?

Any thoughts @c1728p9 ?

@stevew817
Copy link
Contributor Author

stevew817 commented Nov 8, 2016

@bridadan Updated results after last changes:
RESULTS-ARM2.txt
RESULTS-IAR2.txt

@stevew817
Copy link
Contributor Author

@bridadan If you're happy with the current PR, can you remove your change request so that the PR can be merged?

Copy link
Contributor

@bridadan bridadan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bridadan
Copy link
Contributor

bridadan commented Nov 9, 2016

/morph test

@stevew817
Copy link
Contributor Author

Kinda. Thinking around TLS acceleration happening in a directory like feature/mbedtls/targets/TARGET_MCU_CRYPTO where a MCU can add that label and get support. This is how LwIP was done. Given these hal interfaces are from mbedtls we shouldn't use DEVICE_ until (or unless) there is a mbed c hal for symmetric crypto operations.

I feel I'm lacking some kind of context here to parse that statement...
@sg- IMO, it is cleaner in terms of PRs (you can directly see that no common code has been touched if all changes are in targets\) to have all partner-code inside the targets/ directory. That code can then be guarded by the relevant FEATURE_ folder if necessary for header inclusion (such as with nanostack drivers) instead of spreading out hardware implementations accross a bunch of different modules. Just my $.02, and the reason I submitted the PR as such.

@mbed-bot
Copy link

mbed-bot commented Nov 9, 2016

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1035

All builds and test passed!

@sg-
Copy link
Contributor

sg- commented Nov 10, 2016

IMO, it is cleaner in terms of PRs (you can directly see that no common code has been touched if all changes are in targets\

Makes perfect sense.

That code can then be guarded by the relevant FEATURE_ folder if necessary for header inclusion

Currently there can only be a single FEATURE_X folder in any build.

Context is that mbed TLS team are looking at asymmetric acceleration. As implemented, there is already symmetric acceleration interfaces available. Either way, DEVICE_ should be reserved for mbed OS HAL implementations for the time being. It maybe best for mbed TLS specific config hooks. All under consideration.

@sg-
Copy link
Contributor

sg- commented Nov 14, 2016

@stevew817 to move this forward, can we remove the TLS acceleration for now. @sbutcher-arm will provide more guidance on how to fit acceleration in when ready.

@0xc0170
Copy link
Contributor

0xc0170 commented Nov 16, 2016

@stevew817 bump

mbed compile doesn't support two different FEATURE_X folders being merged, so we'll have to move our nanostack driver into the Nanostack folder for the time being.
@stevew817
Copy link
Contributor Author

@0xc0170 @sg- Done.

@stevew817
Copy link
Contributor Author

@0xc0170 Poke.

@stevew817
Copy link
Contributor Author

@0xc0170 What is taking so long?

@adbridge
Copy link
Contributor

/morph test

@mbed-bot
Copy link

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1174

All builds and test passed!

@stevew817
Copy link
Contributor Author

Anything else needed for this PR?

@0xc0170 0xc0170 removed the needs: CI label Dec 1, 2016
@0xc0170
Copy link
Contributor

0xc0170 commented Dec 1, 2016

LGTM

@0xc0170 0xc0170 merged commit ab2e869 into ARMmbed:master Dec 2, 2016
aisair pushed a commit to aisair/mbed that referenced this pull request Apr 30, 2024
Ports for Upcoming Targets

3241: Add support for FRDM-KW41 ARMmbed/mbed-os#3241
3291: Adding mbed enabled Maker board with NINA-B1 and EVA-M8Q ARMmbed/mbed-os#3291

Fixes and Changes

3062: TARGET_STM :USB device FS  ARMmbed/mbed-os#3062
3213: STM32: Refactor us_ticker.c + hal_tick.c files ARMmbed/mbed-os#3213
3288: Dev spi asynch l0l1 ARMmbed/mbed-os#3288
3289: Bug fix of initial value of interrupt edge in "gpio_irq_init" function. ARMmbed/mbed-os#3289
3302: STM32F4 AnalogIn - Clear VBATE and TSVREFE bits before configuring ADC channels ARMmbed/mbed-os#3302
3320: STM32 - Add ADC_VREF label ARMmbed/mbed-os#3320
3321: no HSE available by default for NUCLEO_L432KC ARMmbed/mbed-os#3321
3352: ublox eva nina - fix line endings ARMmbed/mbed-os#3352
3322: DISCO_L053C8 doesn't support LSE ARMmbed/mbed-os#3322
3345: STM32 - Remove TIM_IT_UPDATE flag in HAL_Suspend/ResumeTick functions ARMmbed/mbed-os#3345
3309: [NUC472/M453] Fix CI failed tests ARMmbed/mbed-os#3309
3157: [Silicon Labs] Adding support for EFR32MG1 wireless SoC ARMmbed/mbed-os#3157
3301: I2C - correct return values for write functions (docs) - part 1 ARMmbed/mbed-os#3301
3303: Fix #2956 #2939 #2957 #2959 #2960: Add HAL_DeInit function in gpio_irq destructor ARMmbed/mbed-os#3303
3304: STM32L476: no HSE is present in NUCLEO and DISCO boards ARMmbed/mbed-os#3304
3318: Register map changes for RevG ARMmbed/mbed-os#3318
3317: NUCLEO_F429ZI has integrated LSE ARMmbed/mbed-os#3317
3312: K64F: SPI Asynch API implementation ARMmbed/mbed-os#3312
3324: Dev i2c common code ARMmbed/mbed-os#3324
3369: Add CAN2 missing pins for connector CN12 ARMmbed/mbed-os#3369
3377: STM32 NUCLEO-L152RE Update system core clock to 32MHz ARMmbed/mbed-os#3377
3378: K66F: Enable LWIP feature ARMmbed/mbed-os#3378
3382: [MAX32620] Fixing serial readable function. ARMmbed/mbed-os#3382
3399: NUCLEO_F103RB - Add SERIAL_FC feature ARMmbed/mbed-os#3399
3409: STM32L1 : map ST HAL assert into MBED assert ARMmbed/mbed-os#3409
3416: Renames i2c_api.c for STM32F1 targets to fix IAR exporter ARMmbed/mbed-os#3416
3348: Fix frequency function of CAN driver. ARMmbed/mbed-os#3348
3366: NUCLEO_F412ZG - Add new platform ARMmbed/mbed-os#3366
3379: STM32F0 : map ST HAL assert into MBED assert ARMmbed/mbed-os#3379
3393: ISR register never re-evaluated in HAL_DMA_PollForTransfer for STM32F4 ARMmbed/mbed-os#3393
3408: STM32F7 : map ST HAL assert into MBED assert ARMmbed/mbed-os#3408
3411: STM32L0 : map ST HAL assert into MBED assert ARMmbed/mbed-os#3411
3424: STM32F4 - FIX to add the update of hdma->State variable ARMmbed/mbed-os#3424
3427: Fix stm i2c slave ARMmbed/mbed-os#3427
3429: Fix stm i2c fix init ARMmbed/mbed-os#3429
3434: [NUC472/M453] Fix stuck in lp_ticker_init and other updates ARMmbed/mbed-os#3434
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants