Support Nuvoton's new target NUMAKER_PFM_M487 #4608
Conversation
|
@ccli8 It looks like the device you entered in the "device_name" key in targets.json is not a valid part in a CMSIS pack. Could you remove or correct that entry? |
|
@theotherjimmy I remove |
|
@ccli8 I suppose we should get this in now, and turn on uvision later when the pack is released. Is that acceptable to you? |
|
|
@ccli8 I am reviewing this patch at the moment, will provide feedback soon. Can you please rebase to resolve the conflict? |
Can this be reported as an issue, I would like to see more details for this drift. @yanesca @andresag01 mbedtls additions here, please review |
| if (carry && minor_us > (US_PER_TMR0HIRES_INT / 2)) { | ||
| major_minor_us = (counter_major + 1) * US_PER_TMR0HIRES_INT; | ||
| } | ||
| else { |
There was a problem hiding this comment.
else should be on the same line as previous }. Its on more lines in this file, please align
|
|
||
| core_util_critical_section_exit(); | ||
| } | ||
| while (minor_us == 0 || minor_us == US_PER_TMR0HIRES_INT); |
|
|
||
| // Reset 8-bit PSC counter, 24-bit up counter value and CNTEN bit | ||
| // NUC472/M451 | ||
| //timer1_base->CTL |= TIMER_CTL_RSTCNT_Msk; |
| {NC, 0, 0, 0, 0, (IRQn_Type) 0, NULL} | ||
| }; | ||
|
|
||
| void spi_init(spi_t *obj, PinName mosi, PinName miso, PinName sclk, PinName ssel) { |
There was a problem hiding this comment.
{ on the new line for function body
There was a problem hiding this comment.
@0xc0170 Same as above.
|
|
||
|
|
||
| // Configure the SPI data format and frequency | ||
| //spi_format(obj, 8, 0, SPI_MSB); // 8 bits, mode 0 |
There was a problem hiding this comment.
@0xc0170 Same as above.
| MBED_ASSERT(modinit != NULL); | ||
| MBED_ASSERT(modinit->modname == obj->spi.spi); | ||
|
|
||
| void (*vec)(void) = (void (*)(void)) NVIC_GetVector(modinit->irq_n); |
There was a problem hiding this comment.
what is this magic with vec function pointer?
There was a problem hiding this comment.
@0xc0170 This is SPI vector, which is cast to function type to be called in DMA vector.
| // Flush Tx FIFO. Otherwise, output data may get lost on this change. | ||
| while (! UART_IS_TX_EMPTY((UART_T *) NU_MODBASE(obj->serial.uart))); | ||
|
|
||
| // TODO: Assert for not supported parity and data bits |
There was a problem hiding this comment.
this todo can be easily fixed?
| @@ -0,0 +1,29 @@ | |||
| { | |||
There was a problem hiding this comment.
shall this be part of target configuration ? (in targets.json) file? it would be all in one place. we already have there config for some targets.
There was a problem hiding this comment.
@0xc0170 There is incompatibility issue with some sample code. I will move to targets.json later.
| LED1 = LED_RED, | ||
| LED2 = LED_YELLOW, | ||
| LED3 = LED_GREEN, | ||
| LED4 = D0, // No real LED. Just for passing ATS. |
There was a problem hiding this comment.
wouldnt be better to haver this as another LED duplicate rather than D0?
|
|
||
| #include "analogin_api.h" | ||
|
|
||
| // NOTE: Ensure mbed_sdk_init() will get called before C++ global object constructor. |
There was a problem hiding this comment.
is this needed? I believe for mbed OS 5, we have boot unified and should be called as needed here?
@0xc0170 I've fixed it. |
|
@0xc0170 Could you rereview? |
| return (elapsed >= timeout); | ||
| } | ||
|
|
||
| #if 0 |
There was a problem hiding this comment.
please remove any dead code like this
|
@0xc0170 Dead code removed. Could you review? |
|
@ccli8 Thanks for the updates. this looks good to me. There is one more change that either this PR gets or I'll based it on top of this, as it was merged recently - fire now functionality for tickers. Can you rebase to get that API addition or we can help. Let us know |
|
@0xc0170 Yes. This dropped out of my todo list, sorry about the delay.
You should be able to do |
@0xc0170 @theotherjimmy I've updated CMSIS pack. Please check 1e02309. |
@0xc0170 M487's |
|
/morph test |
|
It's unfortunate that the mbed OS serial API doesn't have a way to wait until all serial data is sent. My applications were written assuming that M487's serial_putc's behavior is within the HAL specification. I'll have to figure out another way to wait for all data to be sent than |
Yep. Looks good. Travis is now happy. |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputExample Build failed! |
|
Failure is not related (again IAR for one platform that I can not reproduce :( ) /morph test |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputExample Build failed! |
|
/morph test |
1 similar comment
|
/morph test |
Result: FAILUREYour command has finished executing! Here's what you wrote!
OutputExample Build failed! |
|
/morph test |
|
The above job was aborted, will be restarted soon once the release candidate gets tested cc @studavekar |
|
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
|
/morph export-build |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
Outputmbed Build Number: 124 All exports and builds passed! |
There was a problem hiding this comment.
Personally I don't like the fact that this PR includes so many files. The new driver files should have been pushed in a separate PR.
Also, this PR has a lot of duplications between HW an SW implementations, but I guess this is a necessity due to current mbed TLS architecture, but this will change in the future
| #define MBEDTLS_SHA256_ALT | ||
| #define MBEDTLS_SHA512_ALT | ||
|
|
||
| #define MBEDTLS_AES_ALT |
There was a problem hiding this comment.
if MBEDTLS_AES_ALT is defined, no need to define other AES alternative function definitions, as MBEDTLS_AES_ALT defines alternative implementation for the whole functionality
| int ret; | ||
|
|
||
| mbedtls_trace("=== %s keybits[%d]\r\n", __FUNCTION__, keybits); | ||
| dumpHex((uint8_t *)key,keybits/8); |
There was a problem hiding this comment.
it is better to remove the prints
There was a problem hiding this comment.
@RonEld Same as above.
| { | ||
| unsigned int i; | ||
|
|
||
| mbedtls_trace("=== %s keybits[%d]\r\n", __FUNCTION__, keybits); |
There was a problem hiding this comment.
It is better to remove the prints
| unsigned char* pOut; | ||
|
|
||
| // mbedtls_trace("=== %s \r\n", __FUNCTION__); | ||
| dumpHex(input,16); |
There was a problem hiding this comment.
It is better to remove the prints
There was a problem hiding this comment.
@RonEld Same as above.
| #ifndef MBEDTLS_AES_ALT_H | ||
| #define MBEDTLS_AES_ALT_H | ||
|
|
||
| #if !defined(MBEDTLS_CONFIG_FILE) |
There was a problem hiding this comment.
no real need, as aes.h includes config.h before including aes_alt.h , and aes_alt.h should not be included in any other location
| ivl = nu_get32_be(iv + 4); | ||
| TDES_SetInitVect(0, ivh, ivl); | ||
|
|
||
| memcpy(dmabuf_in, in_pos, data_len); |
There was a problem hiding this comment.
Is this thread safe? dmabuf_in and dmabuf_out are static variables, which can be used by anyone calling this function
There was a problem hiding this comment.
@RonEld Same as above.
|
|
||
|
|
||
|
|
||
| static int mbedtls_des_docrypt(uint16_t keyopt, uint8_t key[3][MBEDTLS_DES_KEY_SIZE], int enc, uint32_t tdes_opmode, size_t length, |
There was a problem hiding this comment.
This function doesn't seem to be thread safe
| #ifndef MBEDTLS_DES_ALT_H | ||
| #define MBEDTLS_DES_ALT_H | ||
|
|
||
| #if !defined(MBEDTLS_CONFIG_FILE) |
There was a problem hiding this comment.
no need to add config.h, as des.h already includes the configuration file, before including des_alt.h
| #ifndef MBEDTLS_SHA1_ALT_H | ||
| #define MBEDTLS_SHA1_ALT_H | ||
|
|
||
| #if !defined(MBEDTLS_CONFIG_FILE) |
There was a problem hiding this comment.
no need to include config.h as sha1.h already includes the configuration file before including sha1_alt.h file?
There was a problem hiding this comment.
@RonEld Same as above.
| #ifndef MBEDTLS_SHA512_ALT_H | ||
| #define MBEDTLS_SHA512_ALT_H | ||
|
|
||
| #if !defined(MBEDTLS_CONFIG_FILE) |
There was a problem hiding this comment.
no need to include config.h as sha512.h already includes the configuration file before including sha512_alt.h file
There was a problem hiding this comment.
@RonEld Same as above.
mbed OS 5.5.6 release We are pleased to announce the [mbed OS 5.5.6 release](https://github.com/ARMmbed/mbed-os/releases/tag/mbed-os-5.5.6) is now available. This release includes ... Known Issues The following list of known issues apply to this release: Contents Ports for Upcoming Targets [4608](ARMmbed#4608) Support Nuvoton's new target NUMAKER_PFM_M487 [4840](ARMmbed#4840) Add Support for TOSHIBA TMPM066 board Fixes and Changes [4801](ARMmbed#4801) STM32 CAN: Fix issue with speed function calculation [4808](ARMmbed#4808) Make HAL & US tickers idle safe [4812](ARMmbed#4812) Use DSPI SDK driver API's in SPI HAL driver [4832](ARMmbed#4832) NUC472/M453: Fix several startup and hal bugs [4842](ARMmbed#4842) Add call to DAC_Enable as this is no longer done as part [4849](ARMmbed#4849) Allow using of malloc() for reserving the Nanostack's heap. [4850](ARMmbed#4850) Add list of defines to vscode exporter [4863](ARMmbed#4863) Optimize memory usage of wifi scan for REALTEK_RTW8195AM [4869](ARMmbed#4869) HAL LPCs SPI: Fix mask bits for SPI clock rate [4873](ARMmbed#4873) Fix Cortex-A cache file [4878](ARMmbed#4878) STM32 : Separate internal ADC channels with new pinmap [4392](ARMmbed#4392) Enhance memap, and configure depth level [4895](ARMmbed#4895) Turn on doxygen for DEVICE_* features [4817](ARMmbed#4817) Move RTX error handlers into RTX handler file [4902](ARMmbed#4902) Using CMSIS/RTX Exclusive access macro [4923](ARMmbed#4923) fix export static_files to zip [4844](ARMmbed#4844) bd: Add ProfilingBlockDevice for measuring higher-level applications [4896](ARMmbed#4896) target BLUEPILL_F103C8 compile fix [4921](ARMmbed#4921) Update gcc-arm-embedded PPA in Travis [4926](ARMmbed#4926) STM32L053x8: Refactor NUCLEO_L053R8 and DISCO_L053C8 targets [4831](ARMmbed#4831) Remove excessive use of printf/scanf in mbed_fdopen/_open [4922](ARMmbed#4922) bug fix: xdot clock config [4935](ARMmbed#4935) STM32: fix F410RB vectors size [4940](ARMmbed#4940) Update mbed-coap to version 4.0.9 [4941](ARMmbed#4941) Update of MemoryPool.h header file. You can fetch this release from the [mbed-os GitHub](https://github.com/ARMmbed/mbed-os) repository, using the tag "mbed-os-5.5.6". Please feel free to ask any questions or provide feedback on this release [on the forum](https://forums.mbed.com/), or to contact us at [support@mbed.org](mailto:support@mbed.org).
Description
This PR is to add support for Nuvoton's new target NUMAKER_PFM_M487.
ARM mbed Greentea test
ARM toolchain
mbed-os-tests-mbed_hal-flash/Flash - buffer alignment testfailedThis test item checks performance drift rate < 0.1%. On M487, it could reach 5%.
uARM toolchain
mbed-os-tests-mbed_hal-flash/Flash - buffer alignment testfailedSame cause as above.
GCC toolchain
IAR toolchain
ARM mbed CI test
tests-api-i2c/I2C - LM75B Temperature ReadfailedThis test item fails due to no I2C LM75B sensor at hand.
tests-api-pwm,tests-assumptions-pwm, andtests-assumptions-pwmoutLimited by pinout of this target board, 2 pin pairs out of 4 are bypassed for these test suites.