Skip to content

Commit 8d1c28f

Browse files
committed
[otlib] Deprecate CLI flag for setting reset delay
Reset delays are now configured once in configuration files and cannot be changed per-test on the command line. This commit switches all uses of the `reset_target` function to use either `reset` or `reset_with_delay` where appropriate. All delays should be exactly the same after this commit. Signed-off-by: James Wainwright <[email protected]>
1 parent 177e5eb commit 8d1c28f

File tree

64 files changed

+228
-394
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

64 files changed

+228
-394
lines changed

sw/host/opentitanlib/src/app/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -987,6 +987,7 @@ impl TransportWrapper {
987987
Ok(())
988988
}
989989

990+
#[deprecated = "use [`reset`] or [`reset_with_delay`]"]
990991
pub fn reset_target(&self, reset_delay: Duration, clear_uart_rx: bool) -> Result<()> {
991992
let uart_rx = match clear_uart_rx {
992993
true => UartRx::Clear,

sw/host/opentitanlib/src/bootstrap/legacy_rescue.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::time::{Duration, Instant};
88
use thiserror::Error;
99
use zerocopy::{Immutable, IntoBytes};
1010

11-
use crate::app::TransportWrapper;
11+
use crate::app::{TransportWrapper, UartRx};
1212
use crate::bootstrap::{Bootstrap, BootstrapOptions, UpdateProtocol};
1313
use crate::impl_serializable_error;
1414
use crate::io::uart::Uart;
@@ -253,17 +253,12 @@ impl LegacyRescue {
253253

254254
/// Reset the chip and send the magic 'r' character at the opportune moment during boot in
255255
/// order to enter rescue more, repeat if necessary.
256-
fn enter_rescue_mode(
257-
&self,
258-
transport: &TransportWrapper,
259-
container: &Bootstrap,
260-
uart: &dyn Uart,
261-
) -> Result<()> {
256+
fn enter_rescue_mode(&self, transport: &TransportWrapper, uart: &dyn Uart) -> Result<()> {
262257
// Attempt getting the attention of the bootloader.
263258
let timeout = Duration::from_millis(2000);
264259
for _ in 0..Self::MAX_CONSECUTIVE_ERRORS {
265260
eprint!("Resetting...");
266-
transport.reset_target(container.reset_delay, true)?;
261+
transport.reset(UartRx::Clear)?;
267262

268263
let stopwatch = Instant::now();
269264
while stopwatch.elapsed() < timeout {
@@ -321,7 +316,7 @@ impl UpdateProtocol for LegacyRescue {
321316
let frames = Frame::from_payload(payload)?;
322317
let uart = container.uart_params.create(transport)?;
323318

324-
self.enter_rescue_mode(transport, container, &*uart)?;
319+
self.enter_rescue_mode(transport, &*uart)?;
325320

326321
// Send frames one at a time.
327322
progress.new_stage("", frames.len() * Frame::DATA_LEN);
@@ -367,7 +362,7 @@ impl UpdateProtocol for LegacyRescue {
367362
if container.leave_in_reset {
368363
container.reset_pin.write(false)?; // Low active
369364
} else {
370-
transport.reset_target(container.reset_delay, false)?;
365+
transport.reset(UartRx::Keep)?;
371366
}
372367

373368
progress.progress(frames.len() * Frame::DATA_LEN);

sw/host/opentitanlib/src/bootstrap/mod.rs

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use humantime::parse_duration;
1111
use serde::{Deserialize, Serialize};
1212
use thiserror::Error;
1313

14-
use crate::app::{NoProgressBar, TransportWrapper};
14+
use crate::app::{NoProgressBar, TransportWrapper, UartRx};
1515
use crate::impl_serializable_error;
1616
use crate::io::gpio::GpioPin;
1717
use crate::io::spi::SpiParams;
@@ -86,9 +86,6 @@ pub struct BootstrapOptions {
8686
/// Whether to reset target and clear UART RX buffer after bootstrap. For Chip Whisperer board only.
8787
#[arg(long)]
8888
pub clear_uart: Option<bool>,
89-
/// Duration of the reset pulse.
90-
#[arg(long, value_parser = parse_duration, default_value = "100ms")]
91-
pub reset_delay: Duration,
9289
/// If set, keep the bootstrap strapping applied and do not perform the post-bootstrap reset
9390
/// sequence.
9491
#[arg(long)]
@@ -111,7 +108,6 @@ pub struct Bootstrap<'a> {
111108
pub uart_params: &'a UartParams,
112109
pub spi_params: &'a SpiParams,
113110
reset_pin: Rc<dyn GpioPin>,
114-
reset_delay: Duration,
115111
leave_in_reset: bool,
116112
leave_in_bootstrap: bool,
117113
}
@@ -164,7 +160,6 @@ impl<'a> Bootstrap<'a> {
164160
uart_params: &options.uart_params,
165161
spi_params: &options.spi_params,
166162
reset_pin: transport.gpio_pin("RESET")?,
167-
reset_delay: options.reset_delay,
168163
leave_in_reset: options.leave_in_reset,
169164
leave_in_bootstrap: options.leave_in_bootstrap,
170165
}
@@ -185,7 +180,11 @@ impl<'a> Bootstrap<'a> {
185180
if perform_bootstrap_reset {
186181
log::info!("Asserting bootstrap pins...");
187182
rom_boot_strapping.apply()?;
188-
transport.reset_target(self.reset_delay, self.clear_uart_rx)?;
183+
let uart_rx = match self.clear_uart_rx {
184+
true => UartRx::Clear,
185+
false => UartRx::Keep,
186+
};
187+
transport.reset(uart_rx)?;
189188
log::info!("Performing bootstrap...");
190189
}
191190
let result = updater.update(self, transport, payload, progress);
@@ -204,7 +203,7 @@ impl<'a> Bootstrap<'a> {
204203
rom_boot_strapping.remove()?;
205204
// Don't clear the UART RX buffer after bootstrap to preserve the bootstrap
206205
// output.
207-
transport.reset_target(self.reset_delay, false)?;
206+
transport.reset(UartRx::Keep)?;
208207
}
209208
}
210209
result

sw/host/opentitanlib/src/rescue/serial.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::cell::Cell;
77
use std::rc::Rc;
88
use std::time::Duration;
99

10-
use crate::app::TransportWrapper;
10+
use crate::app::{TransportWrapper, UartRx};
1111
use crate::io::uart::Uart;
1212
use crate::rescue::xmodem::Xmodem;
1313
use crate::rescue::{EntryMode, Rescue, RescueError, RescueMode};
@@ -53,7 +53,7 @@ impl Rescue for RescueSerial {
5353
match mode {
5454
EntryMode::Reset => {
5555
self.uart.set_break(true)?;
56-
transport.reset_target(self.reset_delay, /*clear_uart=*/ true)?;
56+
transport.reset_with_delay(UartRx::Clear, self.reset_delay)?;
5757
}
5858
EntryMode::Reboot => {
5959
self.reboot()?;

sw/host/opentitanlib/src/rescue/spidfu.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use std::rc::Rc;
88
use std::time::{Duration, Instant};
99
use zerocopy::{Immutable, IntoBytes};
1010

11-
use crate::app::TransportWrapper;
11+
use crate::app::{TransportWrapper, UartRx};
1212
use crate::chip::rom_error::RomError;
1313
use crate::io::spi::Target;
1414
use crate::rescue::dfu::*;
@@ -75,9 +75,7 @@ impl Rescue for SpiDfu {
7575
);
7676
self.params.set_trigger(transport, true)?;
7777
match mode {
78-
EntryMode::Reset => {
79-
transport.reset_target(self.reset_delay, /*clear_uart=*/ false)?
80-
}
78+
EntryMode::Reset => transport.reset_with_delay(UartRx::Keep, self.reset_delay)?,
8179
EntryMode::Reboot => {
8280
self.reboot()?;
8381
// Give the chip a chance to reset before attempting to re-read

sw/host/opentitanlib/src/rescue/usbdfu.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use anyhow::{Result, bail};
66
use std::cell::{Cell, Ref, RefCell};
77
use std::time::Duration;
88

9-
use crate::app::TransportWrapper;
9+
use crate::app::{TransportWrapper, UartRx};
1010
use crate::rescue::dfu::*;
1111
use crate::rescue::{EntryMode, Rescue, RescueError, RescueMode, RescueParams};
1212
use crate::util::usb::UsbBackend;
@@ -49,9 +49,7 @@ impl Rescue for UsbDfu {
4949
self.params.set_trigger(transport, true)?;
5050

5151
match mode {
52-
EntryMode::Reset => {
53-
transport.reset_target(self.reset_delay, /*clear_uart=*/ false)?
54-
}
52+
EntryMode::Reset => transport.reset_with_delay(UartRx::Keep, self.reset_delay)?,
5553
EntryMode::Reboot => self.reboot()?,
5654
EntryMode::None => {}
5755
}

sw/host/opentitanlib/src/test_utils/lc.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,22 +7,21 @@ use std::time::Duration;
77
use anyhow::Result;
88
use arrayvec::ArrayVec;
99

10-
use crate::app::TransportWrapper;
10+
use crate::app::{TransportWrapper, UartRx};
1111
use crate::dif::lc_ctrl::{DifLcCtrlState, LcCtrlReg, LcCtrlStatus};
1212
use crate::io::jtag::{JtagParams, JtagTap};
1313
use crate::test_utils::lc_transition::wait_for_status;
1414

1515
pub fn read_lc_state(
1616
transport: &TransportWrapper,
1717
jtag_params: &JtagParams,
18-
reset_delay: Duration,
1918
) -> Result<DifLcCtrlState> {
2019
transport.pin_strapping("PINMUX_TAP_LC")?.apply()?;
2120

2221
// Apply bootstrap pin to be able to connect to JTAG when ROM execution is
2322
// enabled.
2423
transport.pin_strapping("ROM_BOOTSTRAP")?.apply()?;
25-
transport.reset_target(reset_delay, true)?;
24+
transport.reset(UartRx::Clear)?;
2625
let mut jtag = jtag_params.create(transport)?.connect(JtagTap::LcTap)?;
2726
// We must wait for the lc_ctrl to initialize before the LC state is exposed.
2827
wait_for_status(
@@ -40,14 +39,13 @@ pub fn read_lc_state(
4039
pub fn read_device_id(
4140
transport: &TransportWrapper,
4241
jtag_params: &JtagParams,
43-
reset_delay: Duration,
4442
) -> Result<ArrayVec<u32, 8>> {
4543
transport.pin_strapping("PINMUX_TAP_LC")?.apply()?;
4644

4745
// Apply bootstrap pin to be able to connect to JTAG when ROM execution is
4846
// enabled.
4947
transport.pin_strapping("ROM_BOOTSTRAP")?.apply()?;
50-
transport.reset_target(reset_delay, true)?;
48+
transport.reset(UartRx::Clear)?;
5149
let mut jtag = jtag_params.create(transport)?.connect(JtagTap::LcTap)?;
5250
// We must wait for the lc_ctrl to initialize before the LC state is exposed.
5351
wait_for_status(

sw/host/opentitanlib/src/test_utils/lc_transition.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use anyhow::{Context, Result, bail};
99
use serde::{Deserialize, Serialize};
1010
use thiserror::Error;
1111

12-
use crate::app::TransportWrapper;
12+
use crate::app::{TransportWrapper, UartRx};
1313
use crate::chip::boolean::MultiBitBool8;
1414
use crate::dif::lc_ctrl::{
1515
DifLcCtrlState, LcCtrlReg, LcCtrlStatus, LcCtrlTransitionCmd, LcCtrlTransitionCtrl,
@@ -142,7 +142,7 @@ fn setup_lc_transition(
142142
/// tap_lc_strapping.apply().expect("failed to apply strapping");
143143
///
144144
/// // Reset into the new strapping.
145-
/// transport.reset_target(init.bootstrap.options.reset_delay, true).unwrap();
145+
/// transport.reset(UartRx::Clear).unwrap();
146146
///
147147
/// // Connect to the LC controller TAP.
148148
/// let mut jtag = transport
@@ -159,7 +159,6 @@ fn setup_lc_transition(
159159
/// DifLcCtrlState::Prod,
160160
/// Some(test_exit_token.into_register_values()),
161161
/// true,
162-
/// init.bootstrap.options.reset_delay,
163162
/// Some(JtagTap::LcTap),
164163
/// ).expect("failed to trigger transition to prod");
165164
///
@@ -180,7 +179,6 @@ pub fn trigger_lc_transition(
180179
target_lc_state: DifLcCtrlState,
181180
token: Option<[u32; 4]>,
182181
use_external_clk: bool,
183-
reset_delay: Duration,
184182
reset_tap_straps: Option<JtagTap>,
185183
) -> Result<()> {
186184
// Wait for the lc_ctrl to become initialized, claim the mutex, and program the target state
@@ -222,7 +220,7 @@ pub fn trigger_lc_transition(
222220
JtagTap::RiscvTap => transport.pin_strapping("PINMUX_TAP_RISCV")?.apply()?,
223221
}
224222
}
225-
transport.reset_target(reset_delay, true)?;
223+
transport.reset(UartRx::Clear)?;
226224

227225
Ok(())
228226
}

0 commit comments

Comments
 (0)