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

UART v2 #443

Merged
merged 54 commits into from Jul 31, 2021
Merged

UART v2 #443

merged 54 commits into from Jul 31, 2021

Conversation

jbeaurivage
Copy link
Contributor

@jbeaurivage jbeaurivage commented May 19, 2021

This PR aims to provide a new UART driver with an API similar to sercom::v2::spi.

In fact, most of the type-level code was shamelessly copied from @bradleyharden's SPI driver. Thanks for doing most of the legwork!

Supported

  • Basic UART functionality including different frame configurations: parity, stop bits and character size are configurable.
  • Fractional or Arithmetic baud mode
  • Flow control (CTS/RTS pins).
  • Rx + Tx, Rx-only or Tx-only
  • Splitting of Rx and Tx halves
  • IrDA encoding

Not supported

  • LIN mode (thumbv7em devices)
  • 32-bit extension mode (thumbv7em devices)
  • Future-mode - Use DMA transfers instead
  • Synchronous (USART) operation

To Do

  • thumbv7em support
  • Add baudrate flexibility (fractional baud + oversampling)
  • Refine Rx and Tx splitting
  • Finish IrDA implementation
  • DMAC support
  • Add feather_m0 and feather_m4 examples

Dependencies

This PR is dependent on the new Pads API (#434).

Related issues

Closes #329

@jbeaurivage jbeaurivage force-pushed the uart-v2 branch 2 times, most recently from d903cf4 to a7ead1e Compare June 15, 2021 22:51
@bradleyharden
Copy link
Contributor

bradleyharden commented Jun 16, 2021

Some comments:

  • I would define the Registers trait directly on instances of Sercom. That would get rid of the associated type. Both UartRx and UartTx store a Sercom already, so you have direct access to it. The Registers trait is just a layer of convenience functions for the Sercom.
  • You might want to rethink how splitting works. Right now, you let users take out the UartRx and UartTx pieces. But if they do that and then drop the rest, they are dropping the Config, which contains the GPIO pins. To retrieve the pins, you would have to do partial moves out of the Uart and then back in, before calling the disable function.
    • One approach would be to copy the Config struct into each half. You would have split and join methods to handle the logic. The Config struct is zero-sized except for the freq field, so it's minimal memory overhead. Doing so would also save you from having to copy the Sercom directly.
    • However, with this approach, would wouldn't want to let people free one half directly, because that could let you duplicate GPIO pins. You would need to force a join, then disable, then free.
  • You don't need to reexport anything from the uart_dma module. It only contains impls, so there's nothing to export.
    • Speaking of, you might add this as a separate file. I'm not sure. The uart module is already pretty big, and there is a somewhat clear distinction between it and pieces for DMA. But then again, they are still pretty tightly coupled. I'm not sure.

@bradleyharden
Copy link
Contributor

By the way, I think the type-level stuff meshed really well here. You were able to use the Word type to define the size of your Beat. I like that.

@jbeaurivage jbeaurivage force-pushed the uart-v2 branch 2 times, most recently from f562eeb to 9b7dc71 Compare June 16, 2021 20:11
@bradleyharden bradleyharden marked this pull request as draft June 19, 2021 20:31
@bradleyharden
Copy link
Contributor

@jbeaurivage, since you still have boxes unchecked, I converted this to a draft PR. I hope that's ok. It helps me identify merge candidates when browsing the PR list.

@jbeaurivage
Copy link
Contributor Author

@bradleyharden no problem. Hopefully it should be ready soon!

Improve CharSize API
Registers methods take Flags and Status structs instead of raw bits
Remove From implementations for StopBits, BitOrder, Baud, etc.
RXEN/TXEN will only be enabled if the underlying Pads have RX/TX
capability.
Move BITS constant from CharSize to FixedCharSize trait and rename to
SIZE
@bradleyharden bradleyharden merged commit e885cfb into atsamd-rs:master Jul 31, 2021
@jbeaurivage jbeaurivage deleted the uart-v2 branch August 2, 2021 13:36
kaizensparc pushed a commit to kaizensparc/atsamd that referenced this pull request Dec 24, 2021
Provide a new UART driver with an API similar to `sercom::v2::spi`

Co-authored-by: Ian Rees <code@ianrees.nz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor UART implementation
4 participants