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

Sam4l usart new reg interface #766

Merged
merged 1 commit into from Mar 14, 2018

Conversation

Projects
None yet
4 participants
@phil-levis
Collaborator

phil-levis commented Feb 28, 2018

Pull Request Overview

This pull request converts the SAM4L USART implementation to use the new Register interface.

It also extends the Register interface slightly, to better support the complex configurations the USART needs.

Testing Strategy

This pull request was tested by running an application that prints to the serial port and checking that it writes correctly.

TODO or Help Wanted

This pull request still needs a read-over/testing of the SPI part of the USART implementation. I was not sure how to test this.

Documentation Updated

  • Kernel: The relevant files in /docs have been updated or no updates are required.
  • Userland: The application README has been added, updated, or no updates are required.

Formatting

  • make formatall has been run.

@phil-levis phil-levis added the P-Upkeep label Mar 1, 2018

@phil-levis phil-levis requested review from bbbert and ptcrews Mar 1, 2018

@bradjc

bradjc approved these changes Mar 5, 2018

Looks good. I added a commit to remove the pub from the registers (seems that was never necessary).

I also tested with the Hail app on Hail and things seem to be working.

@brghena

There are some places where we could be making better use of the register system.

Show outdated Hide outdated chips/sam4l/src/usart.rs Outdated
Show outdated Hide outdated chips/sam4l/src/usart.rs Outdated
Show outdated Hide outdated chips/sam4l/src/usart.rs Outdated
Show outdated Hide outdated chips/sam4l/src/usart.rs Outdated
Show outdated Hide outdated chips/sam4l/src/usart.rs Outdated
Show outdated Hide outdated kernel/src/common/regs/mod.rs Outdated
Show outdated Hide outdated chips/sam4l/src/usart.rs Outdated

@bradjc bradjc referenced this pull request Mar 7, 2018

Closed

Tracking: SAM4L to New Register Interface #786

18 of 18 tasks complete
@phil-levis

This comment has been minimized.

Show comment
Hide comment
@phil-levis

phil-levis Mar 10, 2018

Collaborator

OK, tried incorporating your suggestions.

Collaborator

phil-levis commented Mar 10, 2018

OK, tried incorporating your suggestions.

@bradjc

bradjc approved these changes Mar 13, 2018

I went through and updated some of the register calls to make them more new-tock-register-formaty.

Tested on hail.

I also moved the changes to the regs code to its own PR since this PR doesn't depend on it.

@brghena

Two small fixes, but otherwise good

Show outdated Hide outdated chips/sam4l/src/usart.rs Outdated
}
ret_val
let regs: &UsartRegisters = unsafe { &*self.registers };
regs.csr.is_set(ChannelStatus::TXRDY)

This comment has been minimized.

@brghena

brghena Mar 14, 2018

Contributor

Wow. It's amazing how much simpler this line is to read than the 5 it replaced

@brghena

brghena Mar 14, 2018

Contributor

Wow. It's amazing how much simpler this line is to read than the 5 it replaced

Show outdated Hide outdated kernel/src/common/regs/mod.rs Outdated
sam4l: usart: new regs
Some of the complex register settings, combined with the encapsulation
of set_mode within a function, called for extending the Register
interface. The core of it is solid, but we're going to want to add more
operators (e.g.  |) and casts over time.

@alevy alevy merged commit 3e4cbd8 into master Mar 14, 2018

3 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@alevy alevy deleted the sam4l-usart-new-reg-interface branch Mar 14, 2018

@ppannuto ppannuto referenced this pull request Mar 14, 2018

Merged

Automatic peripheral clock management #760

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment