Skip to content

Commit

Permalink
Add from_installed_program with correct shift direction
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jannic committed Nov 23, 2023
1 parent 1406331 commit 50583b2
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 9 deletions.
2 changes: 1 addition & 1 deletion rp2040-hal/examples/pio_blink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion rp2040-hal/examples/pio_dma.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion rp2040-hal/examples/pio_proc_blink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion rp2040-hal/examples/pio_side_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions rp2040-hal/examples/pio_synchronized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() },
)
Expand All @@ -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);
Expand Down
38 changes: 35 additions & 3 deletions rp2040-hal/src/pio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,7 +281,7 @@ impl<P: PIOExt> PIO<P> {
/// ).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.
Expand Down Expand Up @@ -1931,12 +1931,44 @@ pub enum InstallError {
}

impl<P: PIOExt> PIOBuilder<P> {
/// 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<P>) -> 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<P>) -> Self {
PIOBuilder {
clock_divisor: (1, 0),
Expand Down

0 comments on commit 50583b2

Please sign in to comment.