Remove "read-only" from UDRE

bhansconnect commented 2 years ago

This enables writing 0 to UDRE. This is not strictly necessary, but the atmega328p manual suggests that it should be done whenever writing to UCSR A. This is done for future compatibility. It should be the same case for other AVR chips.

This will be used in an update to avr-hal.

Rahix commented 2 years ago

Hi, thanks for the pull-request!

For reference for anyone else, the relevant part of the datasheet is 20.6.3 Transmitter Flags and Interrupts. Indeed, it recommends to set the UDREn bit to zero whenever the UCSRnA register is written.

As the bit is marked read-only right now, the only way for it to be written to 1 is if the reset value from the SVD/ATDF file were to have a 1 in its position. That's because the accessors generated by svd2rust enforce that read-only bits are always written to their reset value. From what I can tell, luckily the reset value does have a 0 for this bit:

// src/devices/atmega328p/usart0/ucsr0a.rs
impl crate::Resettable for UCSR0A_SPEC {
    #[inline(always)]
    fn reset_value() -> Self::Ux {
        0
    }
}

As such, unless you have evidence that this is not actually working as intended, I do not think there is need for action here - we are already doing the right thing.

In any case, marking a read-only bit as write-only should be an option of last resort because it kind of violates the philosophy of having these accessors in the first place. Instead, my suggestion would be to patch the reset value - but as shown above, I think it should already be correct.

bhansconnect commented 2 years ago

I see. I guess I just didn't fully understand the api. Didn't realize that it had a reset value. Still very new to embedded rust. I guess that means this PR can just be closed and my pr on the avr-hal repo needs a small update.

Rahix / avr-device

Remove "read-only" from UDRE #92