-
Notifications
You must be signed in to change notification settings - Fork 13
feat(hal): Add preliminary support for LEDC #34
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
Conversation
luojia65
left a comment
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.
Please add Signed-off-by for each of your comments.
allwinner-hal/src/ledc/register.rs
Outdated
| /// LEDC MSB control bits. | ||
| #[inline] | ||
| pub const fn is_red_msb(self) -> bool { | ||
| (self.0 & Self::LED_MSB_R) != 0 | ||
| } | ||
| #[inline] | ||
| pub const fn is_blue_msb(self) -> bool { | ||
| (self.0 & Self::LED_MSB_B) != 0 | ||
| } | ||
| #[inline] | ||
| pub const fn is_green_msb(self) -> bool { | ||
| (self.0 & Self::LED_MSB_G) != 0 | ||
| } |
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.
| /// LEDC MSB control bits. | |
| #[inline] | |
| pub const fn is_red_msb(self) -> bool { | |
| (self.0 & Self::LED_MSB_R) != 0 | |
| } | |
| #[inline] | |
| pub const fn is_blue_msb(self) -> bool { | |
| (self.0 & Self::LED_MSB_B) != 0 | |
| } | |
| #[inline] | |
| pub const fn is_green_msb(self) -> bool { | |
| (self.0 & Self::LED_MSB_G) != 0 | |
| } | |
| /// Get the red LEDC MSB control bit. | |
| #[inline] | |
| pub const fn is_red_msb(self) -> bool { | |
| (self.0 & Self::LED_MSB_R) != 0 | |
| } | |
| /// Get the blue LEDC MSB control bit. | |
| #[inline] | |
| pub const fn is_blue_msb(self) -> bool { | |
| (self.0 & Self::LED_MSB_B) != 0 | |
| } | |
| /// Get the green LEDC MSB control bit. | |
| #[inline] | |
| pub const fn is_green_msb(self) -> bool { | |
| (self.0 & Self::LED_MSB_G) != 0 | |
| } |
Documentation comments are for each function, we can't write only one documentation comment for a list of functions. Use cargo doc and you will find out that the following functions other than the first one does not have a proper documentation comment,
allwinner-hal/src/ledc/register.rs
Outdated
| @@ -0,0 +1,199 @@ | |||
| use volatile_register::RW; | |||
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.
| use volatile_register::RW; | |
| //! LEDC peripheral registers. | |
| use volatile_register::RW; |
Add module level documentations.
|
I have finished writing all the LEDC registers. Please take a look at where they need to be modified. If you think there are too many commits, I will merge them. |
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.
LGTM with minor modifications applicable. Please squash your commits.
| /// Get the wait time0 enable state. | ||
| #[inline] | ||
| pub const fn wait_time0_enabled(self) -> bool { | ||
| ((self.0 >> Self::WAIT_TIME0_EN_OFFSET) & Self::WAIT_TIME0_EN_MASK) != 0 | ||
| } | ||
|
|
||
| /// Set the wait time0 enable state. | ||
| #[inline] | ||
| pub const fn set_wait_time0_enable(self, enabled: bool) -> Self { | ||
| let mut value = self.0; | ||
| value &= !(Self::WAIT_TIME0_EN_MASK << Self::WAIT_TIME0_EN_OFFSET); | ||
| value |= ((if enabled { 1 } else { 0 }) & Self::WAIT_TIME0_EN_MASK) | ||
| << Self::WAIT_TIME0_EN_OFFSET; | ||
| Self(value) | ||
| } |
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.
For enable/disable bit, write like:
pub const fn is_wait_time0_enabled(self) -> bool
pub const fn enable_wait_time0(self) -> Self
pub const fn disable_wait_time0(self) -> Self| /// Get the LEDC DMA enable state. | ||
| #[inline] | ||
| pub const fn dma_enabled(self) -> bool { | ||
| ((self.0 >> Self::LEDC_DMA_EN_OFFSET) & Self::LEDC_DMA_EN_MASK) != 0 | ||
| } | ||
|
|
||
| /// Set the LEDC DMA enable state. | ||
| #[inline] | ||
| pub const fn set_dma_enable(self, enabled: bool) -> Self { | ||
| let mut value = self.0; | ||
| value &= !(Self::LEDC_DMA_EN_MASK << Self::LEDC_DMA_EN_OFFSET); | ||
| value |= | ||
| ((if enabled { 1 } else { 0 }) & Self::LEDC_DMA_EN_MASK) << Self::LEDC_DMA_EN_OFFSET; | ||
| Self(value) | ||
| } |
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.
is_dma_enabled, enable_dma, disable_dma
d555af4 to
791321b
Compare
Signed-off-by: Hanrui Li <[email protected]>
No description provided.