Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove code setting IC_DATA_CMD.RESTART bit #827

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Remove code setting IC_DATA_CMD.RESTART bit #827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16fc26c
Remove code setting IC_DATA_CMD.RESTART bit
jannic Aug 15, 2024
82f076e
Increase rtt buffer size for on-target tests
jannic Aug 15, 2024
5046f14
Fix on-target tests
jannic Aug 25, 2024
e7c6d14
Also apply changes to rp235x-hal
jannic Aug 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions on-target-tests/run_tests.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
@rem We need to specify environment variables here to control build since we aren't able to override them in Cargo.toml

@SET "CARGO_TARGET_THUMBV6M_NONE_EABI_RUNNER=probe-rs run"
@SET "DEFMT_RTT_BUFFER_SIZE=4096"

cargo test --no-fail-fast -- --chip rp2040
2 changes: 1 addition & 1 deletion on-target-tests/run_tests.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
# Keep running tests even if one of them fails
# We need to specify probe-rs as our runner via environment variables here
# to control build since we aren't able to override them in config.toml
CARGO_TARGET_THUMBV6M_NONE_EABI_RUNNER="probe-rs run" cargo test --no-fail-fast -- --chip rp2040
DEFMT_RTT_BUFFER_SIZE=4096 CARGO_TARGET_THUMBV6M_NONE_EABI_RUNNER="probe-rs run" cargo test --no-fail-fast -- --chip rp2040
13 changes: 7 additions & 6 deletions on-target-tests/tests/i2c_loopback.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ mod tests {
#[test]
fn write_iter_read(state: &mut State) {
i2c_tests::blocking::write_iter_read(state, ADDR_7BIT, 1..=1);
i2c_tests::blocking::write_iter_read(state, ADDR_10BIT, 2..=2);
i2c_tests::blocking::write_iter_read(state, ADDR_10BIT, 1..=1);
}

#[test]
fn write_read(state: &mut State) {
i2c_tests::blocking::write_read(state, ADDR_7BIT, 1..=1);
i2c_tests::blocking::write_read(state, ADDR_10BIT, 2..=2);
i2c_tests::blocking::write_read(state, ADDR_10BIT, 1..=1);
}

#[test]
Expand All @@ -89,25 +89,26 @@ mod tests {
#[test]
fn transactions_read_write(state: &mut State) {
i2c_tests::blocking::transactions_read_write(state, ADDR_7BIT, 1..=1);
// An initial read in 10 bit mode contains an implicit restart condition
i2c_tests::blocking::transactions_read_write(state, ADDR_10BIT, 2..=2);
}

#[test]
fn transactions_write_read(state: &mut State) {
i2c_tests::blocking::transactions_write_read(state, ADDR_7BIT, 1..=1);
i2c_tests::blocking::transactions_write_read(state, ADDR_10BIT, 2..=2);
i2c_tests::blocking::transactions_write_read(state, ADDR_10BIT, 1..=1);
}

#[test]
fn transaction(state: &mut State) {
i2c_tests::blocking::transaction(state, ADDR_7BIT, 7..=9);
i2c_tests::blocking::transaction(state, ADDR_10BIT, 7..=9);
i2c_tests::blocking::transaction(state, ADDR_7BIT, 5..=5);
i2c_tests::blocking::transaction(state, ADDR_10BIT, 5..=5);
}

#[test]
fn transactions_iter(state: &mut State) {
i2c_tests::blocking::transactions_iter(state, ADDR_7BIT, 1..=1);
i2c_tests::blocking::transactions_iter(state, ADDR_10BIT, 2..=2);
i2c_tests::blocking::transactions_iter(state, ADDR_10BIT, 1..=1);
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions on-target-tests/tests/i2c_loopback_async.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,8 @@ mod tests {

#[test]
fn transactions_iter(state: &mut State) {
run_test(non_blocking::transaction(state, ADDR_7BIT, 7..=9));
run_test(non_blocking::transaction(state, ADDR_10BIT, 7..=14));
run_test(non_blocking::transaction(state, ADDR_7BIT, 5..=5));
run_test(non_blocking::transaction(state, ADDR_10BIT, 5..=5));
}

#[test]
Expand Down
23 changes: 14 additions & 9 deletions on-target-tests/tests/i2c_tests/blocking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ pub fn transactions_read_write<T: ValidAddress>(
restart_count: RangeInclusive<u32>,
) {
use embedded_hal::i2c::{I2c, Operation};
let controller = reset(state, addr, true);
let controller = reset(state, addr, false);

let samples_seq: FIFOBuffer = Generator::seq().take(25).collect();
let samples_fib: FIFOBuffer = Generator::fib().take(25).collect();
Expand Down Expand Up @@ -381,25 +381,25 @@ pub fn transaction<T: ValidAddress>(
// does not "waste" bytes that would be discarded otherwise.
//
// One down side of this is that the Target implementation is unable to detect restarts
// between consicutive write operations
// between consecutive write operations
use embedded_hal::i2c::{I2c, Operation};
let controller = reset(state, addr, true);
let controller = reset(state, addr, false);

let mut v = ([0u8; 14], [0u8; 25], [0u8; 25], [0u8; 14], [0u8; 14]);
let samples: FIFOBuffer = Generator::seq().take(25).collect();
controller
.transaction(
addr,
&mut [
Operation::Write(&samples), // goes to v2
Operation::Write(&samples),
Operation::Read(&mut v.0),
Operation::Read(&mut v.1),
Operation::Read(&mut v.2),
Operation::Write(&samples), // goes to v3
Operation::Write(&samples),
Operation::Read(&mut v.3),
Operation::Write(&samples), // goes to v4
Operation::Write(&samples), // remains in buffer
Operation::Write(&samples), // remains in buffer
Operation::Write(&samples),
Operation::Write(&samples),
Operation::Write(&samples),
Operation::Read(&mut v.4),
],
)
Expand All @@ -423,7 +423,12 @@ pub fn transaction<T: ValidAddress>(
assert_vec_eq!(e);

// assert reads
let g: FIFOBuffer = Generator::seq().take(92).collect();
let g: FIFOBuffer = itertools::chain!(
Generator::seq().take(14 + 25 + 25),
Generator::fib().take(14),
Generator::seq().take(14),
)
.collect();
let h: FIFOBuffer = itertools::chain!(
v.0.into_iter(),
v.1.into_iter(),
Expand Down
5 changes: 4 additions & 1 deletion on-target-tests/tests/i2c_tests/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,10 @@ fn target_handler(
} = payload;
match evt {
Event::Start => *first = true,
Event::Restart => *restart_cnt += 1,
Event::Restart => {
*first = true;
*restart_cnt += 1;
}
Event::TransferRead => {
let n = throttle.then_some(1).unwrap_or(target.tx_fifo_available());
let v: FIFOBuffer = gen.take(n.into()).collect();
Expand Down
8 changes: 3 additions & 5 deletions on-target-tests/tests/i2c_tests/non_blocking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,11 +342,9 @@ pub async fn transaction<A: ValidAddress>(
assert_eq!(e, state.payload.borrow().vec);
// assert reads
let g: FIFOBuffer = itertools::chain!(
Generator::fib().take(25),
Generator::fib().skip(32).take(25),
Generator::fib().skip(64).take(25),
Generator::fib().skip(96).take(14),
Generator::fib().skip(112).take(14),
Generator::fib().take(25 * 3),
Generator::seq().take(14),
Generator::fib().take(14),
)
.collect();
let h: FIFOBuffer = itertools::chain!(
Expand Down
15 changes: 0 additions & 15 deletions rp2040-hal/src/i2c/controller.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,21 +227,13 @@ impl<T: Deref<Target = Block>, PINS> I2C<T, PINS, Controller> {
)?;

let lastindex = buffer.len() - 1;
let mut first_byte = true;
for (i, byte) in buffer.iter_mut().enumerate() {
let last_byte = i == lastindex;

// wait until there is space in the FIFO to write the next byte
while self.i2c.ic_status().read().tfnf().bit_is_clear() {}

self.i2c.ic_data_cmd().write(|w| {
if first_byte {
if !first_transaction {
w.restart().enable();
}
first_byte = false;
}

w.stop().bit(do_stop && last_byte);
w.cmd().read()
});
Expand Down Expand Up @@ -270,7 +262,6 @@ impl<T: Deref<Target = Block>, PINS> I2C<T, PINS, Controller> {
)?;

let mut abort_reason = Ok(());
let mut first_byte = true;
'outer: while let Some(byte) = peekable.next() {
if self.tx_fifo_full() {
// wait for more room in the fifo
Expand All @@ -289,12 +280,6 @@ impl<T: Deref<Target = Block>, PINS> I2C<T, PINS, Controller> {
// else enqueue
let last = peekable.peek().is_none();
self.i2c.ic_data_cmd().write(|w| {
if first_byte {
if !first_transaction {
w.restart().enable();
}
first_byte = false;
}
w.stop().bit(do_stop && last);
unsafe { w.dat().bits(byte) }
});
Expand Down
15 changes: 0 additions & 15 deletions rp2040-hal/src/i2c/controller/non_blocking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ where
)?;

let lastindex = buffer.len() - 1;
let mut first_byte = true;
for (i, byte) in buffer.iter_mut().enumerate() {
let last_byte = i == lastindex;

Expand All @@ -135,13 +134,6 @@ where
.await;

self.i2c.ic_data_cmd().write(|w| {
if first_byte {
if !first_transaction {
w.restart().enable();
}
first_byte = false;
}

w.stop().bit(do_stop && last_byte);
w.cmd().read()
});
Expand Down Expand Up @@ -174,7 +166,6 @@ where
)?;

let mut abort_reason = Ok(());
let mut first_byte = true;
while let Some(byte) = peekable.next() {
if self.tx_fifo_full() {
// wait for more room in the fifo
Expand All @@ -193,12 +184,6 @@ where
// else enqueue
let last = peekable.peek().is_none();
self.i2c.ic_data_cmd().write(|w| {
if first_byte {
if !first_transaction {
w.restart().enable();
}
first_byte = false;
}
w.stop().bit(do_stop && last);
unsafe { w.dat().bits(byte) }
});
Expand Down
15 changes: 0 additions & 15 deletions rp235x-hal/src/i2c/controller.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,21 +227,13 @@ impl<T: Deref<Target = Block>, PINS> I2C<T, PINS, Controller> {
)?;

let lastindex = buffer.len() - 1;
let mut first_byte = true;
for (i, byte) in buffer.iter_mut().enumerate() {
let last_byte = i == lastindex;

// wait until there is space in the FIFO to write the next byte
while self.i2c.ic_status().read().tfnf().bit_is_clear() {}

self.i2c.ic_data_cmd().write(|w| {
if first_byte {
if !first_transaction {
w.restart().enable();
}
first_byte = false;
}

w.stop().bit(do_stop && last_byte);
w.cmd().read()
});
Expand Down Expand Up @@ -270,7 +262,6 @@ impl<T: Deref<Target = Block>, PINS> I2C<T, PINS, Controller> {
)?;

let mut abort_reason = Ok(());
let mut first_byte = true;
'outer: while let Some(byte) = peekable.next() {
if self.tx_fifo_full() {
// wait for more room in the fifo
Expand All @@ -289,12 +280,6 @@ impl<T: Deref<Target = Block>, PINS> I2C<T, PINS, Controller> {
// else enqueue
let last = peekable.peek().is_none();
self.i2c.ic_data_cmd().write(|w| {
if first_byte {
if !first_transaction {
w.restart().enable();
}
first_byte = false;
}
w.stop().bit(do_stop && last);
unsafe { w.dat().bits(byte) }
});
Expand Down
15 changes: 0 additions & 15 deletions rp235x-hal/src/i2c/controller/non_blocking.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ where
)?;

let lastindex = buffer.len() - 1;
let mut first_byte = true;
for (i, byte) in buffer.iter_mut().enumerate() {
let last_byte = i == lastindex;

Expand All @@ -135,13 +134,6 @@ where
.await;

self.i2c.ic_data_cmd().write(|w| {
if first_byte {
if !first_transaction {
w.restart().enable();
}
first_byte = false;
}

w.stop().bit(do_stop && last_byte);
w.cmd().read()
});
Expand Down Expand Up @@ -174,7 +166,6 @@ where
)?;

let mut abort_reason = Ok(());
let mut first_byte = true;
while let Some(byte) = peekable.next() {
if self.tx_fifo_full() {
// wait for more room in the fifo
Expand All @@ -193,12 +184,6 @@ where
// else enqueue
let last = peekable.peek().is_none();
self.i2c.ic_data_cmd().write(|w| {
if first_byte {
if !first_transaction {
w.restart().enable();
}
first_byte = false;
}
w.stop().bit(do_stop && last);
unsafe { w.dat().bits(byte) }
});
Expand Down