-
Notifications
You must be signed in to change notification settings - Fork 3k
F improvements for teensy3 1 to master #12835
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
F improvements for teensy3 1 to master #12835
Conversation
…t so it works correctly in mbed os 5
… period amount overflows
…non-portB pins Note: previous code here used to say: "enable open drain for I2C pins, only port b available" And the proceeded to strip off the Port letter, and modify the open drain setting for a pin with the same number on PORT B. If the pin configured as i2c wasn't on PORTB, then the previous code was touching the open-drain settings for the wrong pin, which is just plain wrong. Anyway, the comment suggests that the other ports might not have open drain operation, however a review of documentation shows: 1) For the "MK20DX128VLH5" chip used by the K20D50M board, neither the datasheet nor the reference manual seems to say anything about inexistent open drain capability 2) For the "MK20DX256VLH7" chip used by the TEENSY3_1 board, there is a mention of a pin potentially not supporting open-drain, without listing which pins are like that, however it also says that the ODE bit would be read-only in that case. I have tested some I2C-enabled PORTC pins on TEENSY3_1, and the bits are not read-only, and changing them has an effect on the physical hardware.
… of the PCR register is calculated This makes it simmetric with how it's done in pin_mode(), and saves about 2 instructions too in pin_functions().
…ng, but luckily they were never found, because they were not the first in the list with the same pin.
…lso don't have PWM.
…n the comment, and PTD5's comment was wrong. Note: tested on hardware: PTD4 was not useable, and now it is.
* added support for module FTM2 * added 4 missing pwm-compatible pins * renamed PWMn keys to match documentation from https://os.mbed.com/platforms/Teensy-3-1/ (this was done, rather then changing documentation, bedcause these pins weren't working anyway because of missing FTM2 support) * tested on hardware
Question: I created two PRs: one to 5.15 and one to master. Is this what I'm supposed to be doing? |
@purdeaandrei, thank you for your changes. |
targets/targets.json
Outdated
@@ -1712,10 +1712,12 @@ | |||
"SLEEP", | |||
"SPI", | |||
"SPISLAVE", | |||
"STDIO_MESSAGES" | |||
"STDIO_MESSAGES", | |||
"USBDEVICE" |
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.
how's enabling USBDEVICE related to Mbed OS 5 ? It should be a separate commit, also includes test logs for this in the PR itself
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.
Well, it's related in the sense that USB Device was already supported in Mbed OS 2, and for some reason it worked even though USBDEVICE was not declared in the .json file. If you notice, I declared USBDEVICE but I didn't have to add any .c code to support it, it's because it's all already there. However if you still think I should separate it out, I'm fine with it. Should I force-push to this branch?
Regarding test logs -- Is there some specific testing that I should do that I should read up on? I tried both USBSerial, and USBMSD with a dummy buffer, and they worked fine.
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've checked ,correct was already implemented. All fine then
This PR cannot be merged due to conflicts. Please rebase to resolve them. |
Teensy 3.1 was removed in #12810 |
Correct. I wanted to get this in prior that PR. We need to move Teensy 3 to be custom target outside of this repository tree then. |
@purdeaandrei as commented, we won't support the KL20D50 family (latest PR #12864), therefore it makes sense to close this PR and add support for the TEENSY3_1 as a custom target. |
Summary of changes
Impact of changes
Migration actions required
Documentation
Documentation should mention that mbed os 5 is now supported for TEENSY3_1.
Pull request type
Test results
Reviewers