From 50583b2c9347254b5f04a2b676f88b3e7ee0c1ec Mon Sep 17 00:00:00 2001 From: Jan Niehusmann Date: Thu, 23 Nov 2023 07:39:24 +0000 Subject: [PATCH] Add from_installed_program with correct shift direction The existing function from_program uses the "wrong" shift direction, the default is different from what the RP2040 does. This is confusing and can lead to difficult to debug errors. (This is how this difference was found.) To avoid a subtle breaking change by just fixing the default, introduce a new function from_installed_program with the correct default value, and deprecate the old one. --- rp2040-hal/examples/pio_blink.rs | 2 +- rp2040-hal/examples/pio_dma.rs | 2 +- rp2040-hal/examples/pio_proc_blink.rs | 2 +- rp2040-hal/examples/pio_side_set.rs | 2 +- rp2040-hal/examples/pio_synchronized.rs | 4 +-- rp2040-hal/src/pio.rs | 38 +++++++++++++++++++++++-- 6 files changed, 41 insertions(+), 9 deletions(-) diff --git a/rp2040-hal/examples/pio_blink.rs b/rp2040-hal/examples/pio_blink.rs index 18e69220b..67ac6777f 100644 --- a/rp2040-hal/examples/pio_blink.rs +++ b/rp2040-hal/examples/pio_blink.rs @@ -66,7 +66,7 @@ fn main() -> ! { let (mut pio, sm0, _, _, _) = pac.PIO0.split(&mut pac.RESETS); let installed = pio.install(&program).unwrap(); let (int, frac) = (0, 0); // as slow as possible (0 is interpreted as 65536) - let (sm, _, _) = rp2040_hal::pio::PIOBuilder::from_program(installed) + let (sm, _, _) = rp2040_hal::pio::PIOBuilder::from_installed_program(installed) .set_pins(led_pin_id, 1) .clock_divisor_fixed_point(int, frac) .build(sm0); diff --git a/rp2040-hal/examples/pio_dma.rs b/rp2040-hal/examples/pio_dma.rs index 78e37cd69..a40b33b47 100644 --- a/rp2040-hal/examples/pio_dma.rs +++ b/rp2040-hal/examples/pio_dma.rs @@ -62,7 +62,7 @@ fn main() -> ! { // Initialize and start PIO let (mut pio, sm0, _, _, _) = pac.PIO0.split(&mut pac.RESETS); let installed = pio.install(&program.program).unwrap(); - let (mut sm, rx, tx) = rp2040_hal::pio::PIOBuilder::from_program(installed) + let (mut sm, rx, tx) = rp2040_hal::pio::PIOBuilder::from_installed_program(installed) .out_pins(led_pin_id, 1) .clock_divisor_fixed_point(0, 0) // as slow as possible (0 is interpreted as 65536) .autopull(true) diff --git a/rp2040-hal/examples/pio_proc_blink.rs b/rp2040-hal/examples/pio_proc_blink.rs index 7f5c9d245..75cdebca6 100644 --- a/rp2040-hal/examples/pio_proc_blink.rs +++ b/rp2040-hal/examples/pio_proc_blink.rs @@ -52,7 +52,7 @@ fn main() -> ! { let (mut pio, sm0, _, _, _) = pac.PIO0.split(&mut pac.RESETS); let installed = pio.install(&program.program).unwrap(); let (int, frac) = (0, 0); // as slow as possible (0 is interpreted as 65536) - let (mut sm, _, _) = rp2040_hal::pio::PIOBuilder::from_program(installed) + let (mut sm, _, _) = rp2040_hal::pio::PIOBuilder::from_installed_program(installed) .set_pins(led_pin_id, 1) .clock_divisor_fixed_point(int, frac) .build(sm0); diff --git a/rp2040-hal/examples/pio_side_set.rs b/rp2040-hal/examples/pio_side_set.rs index 89f01b940..5db05f260 100644 --- a/rp2040-hal/examples/pio_side_set.rs +++ b/rp2040-hal/examples/pio_side_set.rs @@ -55,7 +55,7 @@ fn main() -> ! { let (mut pio, sm0, _, _, _) = pac.PIO0.split(&mut pac.RESETS); let installed = pio.install(&program.program).unwrap(); let (int, frac) = (0, 0); // as slow as possible (0 is interpreted as 65536) - let (mut sm, _, _) = rp2040_hal::pio::PIOBuilder::from_program(installed) + let (mut sm, _, _) = rp2040_hal::pio::PIOBuilder::from_installed_program(installed) .side_set_pin_base(led_pin_id) .clock_divisor_fixed_point(int, frac) .build(sm0); diff --git a/rp2040-hal/examples/pio_synchronized.rs b/rp2040-hal/examples/pio_synchronized.rs index 9759162b5..2e19edc35 100644 --- a/rp2040-hal/examples/pio_synchronized.rs +++ b/rp2040-hal/examples/pio_synchronized.rs @@ -64,7 +64,7 @@ fn main() -> ! { let (int, frac) = (256, 0); let installed = pio.install(&program.program).unwrap(); - let (mut sm0, _, _) = rp2040_hal::pio::PIOBuilder::from_program( + let (mut sm0, _, _) = rp2040_hal::pio::PIOBuilder::from_installed_program( // Safety: We won't uninstall the program, ever unsafe { installed.share() }, ) @@ -74,7 +74,7 @@ fn main() -> ! { // The GPIO pin needs to be configured as an output. sm0.set_pindirs([(pin0, hal::pio::PinDir::Output)]); - let (mut sm1, _, _) = rp2040_hal::pio::PIOBuilder::from_program(installed) + let (mut sm1, _, _) = rp2040_hal::pio::PIOBuilder::from_installed_program(installed) .set_pins(pin1, 1) .clock_divisor_fixed_point(int, frac) .build(sm1); diff --git a/rp2040-hal/src/pio.rs b/rp2040-hal/src/pio.rs index 6343a1845..5fe048176 100644 --- a/rp2040-hal/src/pio.rs +++ b/rp2040-hal/src/pio.rs @@ -281,7 +281,7 @@ impl PIO

{ /// ).program; /// let installed = pio.install(&program).unwrap(); /// // Configure a state machine to use the program. -/// let (sm, rx, tx) = PIOBuilder::from_program(installed).build(sm0); +/// let (sm, rx, tx) = PIOBuilder::from_installed_program(installed).build(sm0); /// // Uninitialize the state machine again, freeing the program. /// let (sm, installed) = sm.uninit(rx, tx); /// // Uninstall the program to free instruction memory. @@ -1931,12 +1931,44 @@ pub enum InstallError { } impl PIOBuilder

{ - /// Set config settings based on information from the given [`pio::Program`]. + /// Set config settings based on information from the given [`InstalledProgram`]. + /// Additional configuration may be needed in addition to this. + /// + /// Note: This was formerly called `from_program`. The new function has + /// a different default shift direction, `ShiftDirection::Right`, matching + /// the hardware reset value. + pub fn from_installed_program(p: InstalledProgram

) -> Self { + PIOBuilder { + clock_divisor: (1, 0), + program: p, + jmp_pin: 0, + out_sticky: false, + inline_out: None, + mov_status: MovStatusConfig::Tx(0), + fifo_join: Buffers::RxTx, + pull_threshold: 0, + push_threshold: 0, + out_shiftdir: ShiftDirection::Right, + in_shiftdir: ShiftDirection::Right, + autopull: false, + autopush: false, + set_count: 5, + out_count: 0, + in_base: 0, + side_set_base: 0, + set_base: 0, + out_base: 0, + } + } + + /// Set config settings based on information from the given [`InstalledProgram`]. /// Additional configuration may be needed in addition to this. /// /// Note: The shift direction for both input and output shift registers /// defaults to `ShiftDirection::Left`, which is different from the - /// rp2040 reset value. + /// rp2040 reset value. The alternative [`Self::from_installed_program`], + /// fixes this. + #[deprecated(note = "please use `from_installed_program` instead")] pub fn from_program(p: InstalledProgram

) -> Self { PIOBuilder { clock_divisor: (1, 0),