Skip to content

Commit 46fbe4f

Browse files
committed
generic: port: Fix pin toggle implementation
Only the bit for the pin to be toggled should be set in the PIN register. The RMW-operation was incorrect here as it would also toggle unrelated pins which happen to read HIGH at that moment. This went unnoticed because the code for the non-dynamic pin-variant was optimized down to an `sbi` instruction - thus "accidentally" behaving correctly. I do not think it is a good idea to rely on this behavior though. If the non-dynamic code should be optimized to an `sbi` again at some point in the future, I would prefer an implementation using inline assembly. Ref: #284
1 parent 0fdbc49 commit 46fbe4f

File tree

1 file changed

+3
-5
lines changed

1 file changed

+3
-5
lines changed

avr-hal-generic/src/port.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -548,8 +548,8 @@ macro_rules! impl_port_traditional {
548548
#[inline]
549549
unsafe fn out_toggle(&mut self) {
550550
match self.port {
551-
$(DynamicPort::$PortName => (*<$Port>::ptr()).$port_pin_reg.modify(|r, w| {
552-
w.bits(r.bits() | self.mask)
551+
$(DynamicPort::$PortName => (*<$Port>::ptr()).$port_pin_reg.write(|w| {
552+
w.bits(self.mask)
553553
}),)+
554554
}
555555
}
@@ -623,9 +623,7 @@ macro_rules! impl_port_traditional {
623623

624624
#[inline]
625625
unsafe fn out_toggle(&mut self) {
626-
(*<$PinPort>::ptr()).$pin_pin_reg.modify(|r, w| {
627-
w.bits(r.bits() | (1 << $pin_num))
628-
})
626+
(*<$PinPort>::ptr()).$pin_pin_reg.write(|w| w.bits(1 << $pin_num))
629627
}
630628

631629
#[inline]

0 commit comments

Comments
 (0)