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

Don't tie USART clock to specific USART instance; cleanups #223

Merged
merged 19 commits into from Mar 17, 2020
Merged

Don't tie USART clock to specific USART instance; cleanups #223

merged 19 commits into from Mar 17, 2020

Conversation

hannobraun
Copy link
Member

Based on #222. I recommend reviewing/merging that one first.

Originally I just meant to remove the connection between a USART clock a specific USART instance, as that's not necessary, and prevents re-use of configuration between multiple USART instances (which is a bit annoying). It turned into a meandering cleanup operation of USART clock code.

With the added space and changed grouping, I find this much easier to
read.
This follows the typical Rust naming style better.
I find imports easier to read when the different types are separated,
and in a consistent order: From most general (`core`), to most specific
(`self`, `super`).
I think it's easier to find there. I also think this is more clear than
having all the various clock in one module.
It's already in the `usart` module, so the name is redundant.
It seems like the logical place. When I searched for it, I was surprised
to find it in `syscon`.
It's neither used nor implemented on LPC82x, but having it available
everywhere will make some of the following clean-ups easier.
There's really no reason to do so. It just prevents the same
configuration from being reused for multiple instances, requiring more
lines of code.
It's not stricly required for LPC82x, but having it generally available
will make some clean-ups possible, that are going to reduce code
duplication.
This reduces code duplication between the different targets, and makes
them a bit easier to find.
This makes their names and definition consistent with the style used in
the rest of the `usart` module.
@hannobraun hannobraun marked this pull request as ready for review March 17, 2020 10:29
@hannobraun
Copy link
Member Author

Rebased, ready for review.

Copy link
Member

@david-sawatzke david-sawatzke left a comment

Choose a reason for hiding this comment

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

Looks good!

Parts of it were really needed

@david-sawatzke david-sawatzke merged commit 415ab13 into lpc-rs:master Mar 17, 2020
@hannobraun hannobraun deleted the usart-clock branch March 18, 2020 09:58
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.

None yet

2 participants