From b7c9e19da46e459368306eb173951b0c6a42f459 Mon Sep 17 00:00:00 2001 From: okhsunrog Date: Mon, 8 Dec 2025 23:30:56 +0300 Subject: [PATCH 1/2] feat: add HAS_AUTO_RETRY constant to Driver trait MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add support for hardware automatic retries in USB PD drivers. When HAS_AUTO_RETRY is true, the protocol layer skips its software retry loop and calls driver.transmit() directly — Discarded means all hardware retries are exhausted, so no software retry is needed. The TX message counter is maintained directly without calling wait_for_good_crc(), since hardware already verified GoodCRC reception (e.g. via I_TXSENT on FUSB302B). Also updates transmit_chunk_request() with the same HAS_AUTO_RETRY handling for chunked extended message flows. --- usbpd-traits/src/lib.rs | 6 +++ usbpd/src/protocol_layer/mod.rs | 75 ++++++++++++++++++++++++--------- 2 files changed, 60 insertions(+), 21 deletions(-) diff --git a/usbpd-traits/src/lib.rs b/usbpd-traits/src/lib.rs index ba511c1..d93607e 100644 --- a/usbpd-traits/src/lib.rs +++ b/usbpd-traits/src/lib.rs @@ -33,6 +33,12 @@ pub trait Driver { /// GoodCRC messages and will instead rely on the hardware. const HAS_AUTO_GOOD_CRC: bool = false; + /// If this is `true`, the hardware automatically retries transmission + /// when no GoodCRC is received. The protocol layer will skip its own + /// retry loop but still call wait_for_good_crc() to consume the GoodCRC + /// from the FIFO and validate the message ID. + const HAS_AUTO_RETRY: bool = false; + /// Wait for availability of VBus voltage. fn wait_for_vbus(&self) -> impl Future; diff --git a/usbpd/src/protocol_layer/mod.rs b/usbpd/src/protocol_layer/mod.rs index 290f592..df01c13 100644 --- a/usbpd/src/protocol_layer/mod.rs +++ b/usbpd/src/protocol_layer/mod.rs @@ -296,29 +296,50 @@ impl ProtocolLayer { Self::validate_outgoing_message(&message)?; trace!("Transmit message: {:?}", message); - self.counters.retry.reset(); let mut buffer = Self::get_message_buffer(); let size = message.to_bytes(&mut buffer); - loop { - match self.transmit_inner(&buffer[..size]).await { - Ok(_) => match self.wait_for_good_crc().await { - Ok(()) => { - trace!("Transmit success"); - return Ok(()); - } - Err(RxError::ReceiveTimeout) => match self.counters.retry.increment() { - Ok(_) => { - // Retry transmission, until the retry counter is exceeded. - } - Err(CounterError::Exceeded) => { - return Err(ProtocolError::TransmitRetriesExceeded(self.counters.retry.max_value())); + if DRIVER::HAS_AUTO_RETRY { + // Hardware handles retries and verifies GoodCRC reception. + // Call driver.transmit() directly (not transmit_inner()) because + // Discarded here means all hardware retries exhausted — no point + // retrying in software. + match self.driver.transmit(&buffer[..size]).await { + Ok(()) => { + self.counters.retry.reset(); + _ = self.counters.tx_message.increment(); + trace!("Transmit success (hardware retry)"); + Ok(()) + } + Err(DriverTxError::HardReset) => Err(TxError::HardReset.into()), + Err(DriverTxError::Discarded) => { + Err(ProtocolError::TransmitRetriesExceeded(self.counters.retry.max_value())) + } + } + } else { + // Software retry loop + self.counters.retry.reset(); + + loop { + match self.transmit_inner(&buffer[..size]).await { + Ok(_) => match self.wait_for_good_crc().await { + Ok(()) => { + trace!("Transmit success"); + return Ok(()); } + Err(RxError::ReceiveTimeout) => match self.counters.retry.increment() { + Ok(_) => { + // Retry transmission, until the retry counter is exceeded. + } + Err(CounterError::Exceeded) => { + return Err(ProtocolError::TransmitRetriesExceeded(self.counters.retry.max_value())); + } + }, + Err(other) => return Err(other.into()), }, Err(other) => return Err(other.into()), - }, - Err(other) => return Err(other.into()), + } } } } @@ -721,11 +742,23 @@ impl ProtocolLayer { offset += 2; // Transmit and wait for GoodCRC - match self.transmit_inner(&buffer[..offset]).await { - Ok(_) => self.wait_for_good_crc().await, - Err(TxError::HardReset) => Err(RxError::HardReset), - Err(TxError::UnchunkedExtendedMessagesNotSupported | TxError::AvsVoltageAlignmentInvalid) => { - unreachable!("validation should happen before transmit_inner") + if DRIVER::HAS_AUTO_RETRY { + match self.driver.transmit(&buffer[..offset]).await { + Ok(()) => { + self.counters.retry.reset(); + _ = self.counters.tx_message.increment(); + Ok(()) + } + Err(DriverTxError::HardReset) => Err(RxError::HardReset), + Err(DriverTxError::Discarded) => Err(RxError::ReceiveTimeout), + } + } else { + match self.transmit_inner(&buffer[..offset]).await { + Ok(_) => self.wait_for_good_crc().await, + Err(TxError::HardReset) => Err(RxError::HardReset), + Err(TxError::UnchunkedExtendedMessagesNotSupported | TxError::AvsVoltageAlignmentInvalid) => { + unreachable!("validation should happen before transmit_inner") + } } } } From 08b1d8c73b1329a0183491b97d38e2cf59a35efd Mon Sep 17 00:00:00 2001 From: okhsunrog Date: Fri, 6 Feb 2026 03:11:55 +0300 Subject: [PATCH 2/2] refactor: change wait_for_vbus(&self) to &mut self MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Some PHYs (e.g. FUSB302B) need I2C access to poll VBUS status, which requires &mut self. Existing implementations that don't need mutability are unaffected — they just change the signature. Also update HAS_AUTO_RETRY doc comment to reflect the current implementation (no longer calls wait_for_good_crc). --- examples/embassy-nucleo-h563zi/src/power.rs | 2 +- examples/embassy-stm32-g431cb-epr/src/power.rs | 2 +- examples/embassy-stm32-g431cb/src/power.rs | 2 +- usbpd-traits/src/lib.rs | 7 ++++--- usbpd/src/dummy.rs | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/examples/embassy-nucleo-h563zi/src/power.rs b/examples/embassy-nucleo-h563zi/src/power.rs index a24055d..1c38f05 100644 --- a/examples/embassy-nucleo-h563zi/src/power.rs +++ b/examples/embassy-nucleo-h563zi/src/power.rs @@ -50,7 +50,7 @@ impl<'d> UcpdSinkDriver<'d> { } impl SinkDriver for UcpdSinkDriver<'_> { - async fn wait_for_vbus(&self) { + async fn wait_for_vbus(&mut self) { // The sink policy engine is only running when attached. Therefore VBus is present. } diff --git a/examples/embassy-stm32-g431cb-epr/src/power.rs b/examples/embassy-stm32-g431cb-epr/src/power.rs index c463e23..6595f12 100644 --- a/examples/embassy-stm32-g431cb-epr/src/power.rs +++ b/examples/embassy-stm32-g431cb-epr/src/power.rs @@ -163,7 +163,7 @@ impl<'d> UcpdSinkDriver<'d> { } impl SinkDriver for UcpdSinkDriver<'_> { - async fn wait_for_vbus(&self) { + async fn wait_for_vbus(&mut self) { // The sink policy engine is only running when attached. Therefore VBus is present. } diff --git a/examples/embassy-stm32-g431cb/src/power.rs b/examples/embassy-stm32-g431cb/src/power.rs index 77e4f78..26f9c65 100644 --- a/examples/embassy-stm32-g431cb/src/power.rs +++ b/examples/embassy-stm32-g431cb/src/power.rs @@ -47,7 +47,7 @@ impl<'d> UcpdSinkDriver<'d> { } impl SinkDriver for UcpdSinkDriver<'_> { - async fn wait_for_vbus(&self) { + async fn wait_for_vbus(&mut self) { // The sink policy engine is only running when attached. Therefore VBus is present. } diff --git a/usbpd-traits/src/lib.rs b/usbpd-traits/src/lib.rs index d93607e..89ad6e5 100644 --- a/usbpd-traits/src/lib.rs +++ b/usbpd-traits/src/lib.rs @@ -35,12 +35,13 @@ pub trait Driver { /// If this is `true`, the hardware automatically retries transmission /// when no GoodCRC is received. The protocol layer will skip its own - /// retry loop but still call wait_for_good_crc() to consume the GoodCRC - /// from the FIFO and validate the message ID. + /// retry loop and call driver.transmit() directly. On success, it + /// maintains protocol state (TX message counter) without calling + /// wait_for_good_crc(), since the hardware already verified GoodCRC. const HAS_AUTO_RETRY: bool = false; /// Wait for availability of VBus voltage. - fn wait_for_vbus(&self) -> impl Future; + fn wait_for_vbus(&mut self) -> impl Future; /// Receive a packet. fn receive(&mut self, buffer: &mut [u8]) -> impl Future>; diff --git a/usbpd/src/dummy.rs b/usbpd/src/dummy.rs index 03b8f7f..f43b91f 100644 --- a/usbpd/src/dummy.rs +++ b/usbpd/src/dummy.rs @@ -198,7 +198,7 @@ impl Driver for DummyDriver { Ok(()) } - async fn wait_for_vbus(&self) { + async fn wait_for_vbus(&mut self) { // Do nothing. } }