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

Adds UART support to SAM platform #430

Merged
merged 4 commits into from
Jul 8, 2020

Conversation

henrikssn
Copy link
Contributor

This adds basic clock generation and uart peripheral for logging.

It works fine, the only thing I haven't figured out is how to run the CPU at full 48MHz speed, configuring the flash for 1 wait states crashes my microcontroller and that is needed to get above 24MHz. I guess running at half speed is a acceptable given that the only capabilities the platform has is to do logging at 9600 baud and blink LEDs :)

Please let me know what you think, feedback is highly appreciated!

src/modm/board/feather_m0/board.hpp Show resolved Hide resolved
src/modm/board/feather_m0/module.lb Show resolved Hide resolved
src/modm/platform/clock/sam/gclk_impl.hpp.in Show resolved Hide resolved
src/modm/platform/uart/sam/uart_base.hpp.in Outdated Show resolved Hide resolved
src/modm/platform/uart/sam/uart_hal_impl.hpp.in Outdated Show resolved Hide resolved
@salkinium
Copy link
Member

salkinium commented Jul 7, 2020

This is a very good start!

  • The PMUX should be implemented using modm-devices, but the PMUX data for each Signal is missing… Since I'm not very familiar with the SAM devices, I didn't know to check for that during the PR review. I hope it's in the raw data.
  • The UART is currently unbuffered, so the IODeviceWrapper will either be completely blocking or discard almost everything. Do you want to polish a little more for an interrupt version with atomic::Queue?
  • Not sure what's going on the with Flash Wait States, I suggest cross-checking their frameworks or other libraries source code before going on a debug spree. Debugging this was a pita for STM32 too, since it locks up the entire chip if you get it wrong.

@henrikssn
Copy link
Contributor Author

henrikssn commented Jul 8, 2020

This is a very good start!

  • The PMUX should be implemented using modm-devices, but the PMUX data for each Signal is missing… Since I'm not very familiar with the SAM devices, I didn't know to check for that during the PR review. I hope it's in the raw data.
  • The UART is currently unbuffered, so the IODeviceWrapper will either be completely blocking or discard almost everything. Do you want to polish a little more for an interrupt version with atomic::Queue?
  • Not sure what's going on the with Flash Wait States, I suggest cross-checking their frameworks or other libraries source code before going on a debug spree. Debugging this was a pita for STM32 too, since it locks up the entire chip if you get it wrong.

Thanks for the feedback!

  1. I agree on the PMUX, I am happy to fix it but could not find it in the device info (please guide me if you find it). Now the hack is at least more contained.
  2. As this is mainly for debug logging, I believe unbuffered UART is a good start, as I would like to focus on the other peripherals (SPI / I2C) before getting back to making UART more awesome. For buffered UART, a DMA implementation would be far superior to interrupt driven.
  3. Flash wait states is fixed

Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

I'm happy with this, let's merge and move on, while I check how to get the PMUX data.

@henrikssn
Copy link
Contributor Author

SG, feel free to merge (I don't seem to be allowed to)

@salkinium
Copy link
Member

salkinium commented Jul 8, 2020

Sorry, I was busy with work. I took the liberty of rebasing and simply regrouping the commits. (#ForTehVanity). Will merge after CI passes.

@salkinium salkinium merged commit 04688bc into modm-io:develop Jul 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants