-
Notifications
You must be signed in to change notification settings - Fork 3k
Add support for RapidIoT #8307
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
Add support for RapidIoT #8307
Conversation
0b2cc58
to
6e86296
Compare
needs: preceding PR |
Interesting. @ARMmbed/mbed-os-test |
@cmonr as the board is not part of HW test, no need to update tools. |
@maclobdell @ashok-rao Please review |
|
||
I2C_SCL = PTC10, | ||
I2C_SDA = PTC11, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mmahadevan108 : No SPI brought out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have added SPI defines in the PinNames.h file.
"macros": ["FSL_RTOS_MBED", "USE_EXTERNAL_RTC"], | ||
"default_toolchain": "ARM", | ||
"default_lib": "std", | ||
"release_versions": ["2", "5"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mmahadevan108 : you may want to add a "public: false" here for the parent target..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. I have made this change and updated the PR.
"macros_add": ["CPU_MK64FN1M0VMD12", "TARGET_K64F"], | ||
"is_disk_virtual": true, | ||
"mbed_rom_start": "0x00014000", | ||
"mbed_rom_size": "0xEC000", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@0xc0170, @mmahadevan108 / @maclobdell : Would it be better to move these 2 (ram,rom starts) to the respective linker descriptions (ARMCC / GCC / IAR) rather than here? wrapped in an #ifdef for the rapidiot targets only..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/ARMmbed/mbed-os/blob/master/tools/config/__init__.py#L628 - cant find the documentation so the code reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @mmahadevan108 . Some minor comments added. Otherwise looking good!
@mmahadevan108 Can you review latest comments? This addition is almost ready for CI |
Signed-off-by: Mahesh Mahadevan <[email protected]>
Signed-off-by: Mahesh Mahadevan <[email protected]>
6e86296
to
9a47915
Compare
@ashok-rao @0xc0170 I have updated the PR with changes to include comments received. |
@ashok-rao @maclobdell Could y'all take another look? Ignore the unittest result, as it's not this PR's fault. |
SPI_MOSI = PTC6, | ||
SPI_MISO = PTC7, | ||
SPI_SCK = PTC5, | ||
SPI_PERSISTENT_MEM_CS = PTC4, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mmahadevan108 : Not sure if this pin name for the CS maintains consistency with others? why not SPI_SSEL or SPI_NCS ..?
cc @0xc0170 @cmonr ..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not familiar with the names, I copied this from K64F. These are new additions and I am not familiar with the use-case for which these were added, please suggest and I will make the change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yossi2le Can you review this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a minor comment added on naming convention.. rest all LGTM! Thanks @mmahadevan108 ..
/morph build |
Build : SUCCESSBuild number : 3392 Triggering tests/morph test |
Exporter Build : SUCCESSBuild number : 3023 |
Test : SUCCESSBuild number : 3193 |
Description
This PR requires
Pull request type