Skip to content

Conversation

@lihanrui2913
Copy link
Contributor

No description provided.

Copy link
Member

@luojia65 luojia65 left a 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.

Comment on lines 73 to 85
/// 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
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// 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,

@@ -0,0 +1,199 @@
use volatile_register::RW;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
use volatile_register::RW;
//! LEDC peripheral registers.
use volatile_register::RW;

Add module level documentations.

@lihanrui2913
Copy link
Contributor Author

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.

Copy link
Member

@luojia65 luojia65 left a 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.

Comment on lines 447 to 470
/// 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)
}
Copy link
Member

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

Comment on lines 505 to 534
/// 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)
}
Copy link
Member

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

@lihanrui2913 lihanrui2913 force-pushed the main branch 2 times, most recently from d555af4 to 791321b Compare November 25, 2025 14:39
@lihanrui2913 lihanrui2913 marked this pull request as ready for review November 25, 2025 14:39
@luojia65 luojia65 merged commit b3c4caf into rustsbi:main Nov 26, 2025
4 checks passed
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.

2 participants