Skip to content

Commit 00e6f28

Browse files
bugadanibugadani
authored
ESP32: Fix chunked I2C transfers (#4318)
1 parent c290d5e commit 00e6f28

File tree

2 files changed

+127
-112
lines changed

2 files changed

+127
-112
lines changed

‎esp-hal/src/i2c/master/mod.rs‎

Lines changed: 72 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,12 @@ impl I2cAddress {
184184

185185
Ok(())
186186
}
187+
188+
fn bytes(self) -> usize {
189+
match self {
190+
I2cAddress::SevenBit(_) => 1,
191+
}
192+
}
187193
}
188194

189195
impl From<u8> for I2cAddress {
@@ -1976,12 +1982,14 @@ impl Driver<'_> {
19761982
/// - `bytes` is the data two be sent.
19771983
/// - `start` indicates whether the operation should start by a START condition and sending the
19781984
/// address.
1985+
/// - `stop` indicates whether the operation will end with a STOP condition.
19791986
/// - `cmd_iterator` is an iterator over the command registers.
19801987
fn setup_write<'a, I>(
19811988
&self,
19821989
addr: I2cAddress,
19831990
bytes: &[u8],
19841991
start: bool,
1992+
stop: bool,
19851993
cmd_iterator: &mut I,
19861994
) -> Result<(), Error>
19871995
where
@@ -2005,60 +2013,32 @@ impl Driver<'_> {
20052013
let write_len = if start { bytes.len() + 1 } else { bytes.len() };
20062014
// don't issue write if there is no data to write
20072015
if write_len > 0 {
2008-
if cfg!(any(esp32, esp32s2)) {
2009-
// try to place END at the same index
2010-
if write_len < 2 {
2011-
add_cmd(
2012-
cmd_iterator,
2013-
Command::Write {
2014-
ack_exp: Ack::Ack,
2015-
ack_check_en: true,
2016-
length: write_len as u8,
2017-
},
2018-
)?;
2019-
} else if start {
2020-
add_cmd(
2021-
cmd_iterator,
2022-
Command::Write {
2023-
ack_exp: Ack::Ack,
2024-
ack_check_en: true,
2025-
length: (write_len as u8) - 1,
2026-
},
2027-
)?;
2028-
add_cmd(
2029-
cmd_iterator,
2030-
Command::Write {
2031-
ack_exp: Ack::Ack,
2032-
ack_check_en: true,
2033-
length: 1,
2034-
},
2035-
)?;
2036-
} else {
2037-
add_cmd(
2038-
cmd_iterator,
2039-
Command::Write {
2040-
ack_exp: Ack::Ack,
2041-
ack_check_en: true,
2042-
length: (write_len as u8) - 2,
2043-
},
2044-
)?;
2045-
add_cmd(
2046-
cmd_iterator,
2047-
Command::Write {
2048-
ack_exp: Ack::Ack,
2049-
ack_check_en: true,
2050-
length: 1,
2051-
},
2052-
)?;
2053-
add_cmd(
2054-
cmd_iterator,
2055-
Command::Write {
2056-
ack_exp: Ack::Ack,
2057-
ack_check_en: true,
2058-
length: 1,
2059-
},
2060-
)?;
2061-
}
2016+
// ESP32 can't alter the position of END, so we need to split the chunk always into
2017+
// 3-command sequences. Chunking makes sure not to place a 1-byte
2018+
// command at the end, which would cause an arithmetic underflow.
2019+
// The sequences we can generate are:
2020+
// - START-WRITE-STOP
2021+
// - START-WRITE-END-WRITE-STOP
2022+
// - START-WRITE-END-(WRITE-WRITE-END)*-WRITE-STOP sequence.
2023+
if cfg!(esp32) && !(start || stop) {
2024+
// Chunks that do not have a START or STOP command need to be split into multiple
2025+
// commands.
2026+
add_cmd(
2027+
cmd_iterator,
2028+
Command::Write {
2029+
ack_exp: Ack::Ack,
2030+
ack_check_en: true,
2031+
length: (write_len as u8) - 1,
2032+
},
2033+
)?;
2034+
add_cmd(
2035+
cmd_iterator,
2036+
Command::Write {
2037+
ack_exp: Ack::Ack,
2038+
ack_check_en: true,
2039+
length: 1,
2040+
},
2041+
)?;
20622042
} else {
20632043
add_cmd(
20642044
cmd_iterator,
@@ -2091,6 +2071,7 @@ impl Driver<'_> {
20912071
/// - `buffer` is the buffer to store the read data.
20922072
/// - `start` indicates whether the operation should start by a START condition and sending the
20932073
/// address.
2074+
/// - `stop` indicates whether the operation will end with a STOP condition.
20942075
/// - `will_continue` indicates whether there is another read operation following this one and
20952076
/// we should not nack the last byte.
20962077
/// - `cmd_iterator` is an iterator over the command registers.
@@ -2099,6 +2080,7 @@ impl Driver<'_> {
20992080
addr: I2cAddress,
21002081
buffer: &mut [u8],
21012082
start: bool,
2083+
stop: bool,
21022084
will_continue: bool,
21032085
cmd_iterator: &mut I,
21042086
) -> Result<(), Error>
@@ -2119,7 +2101,7 @@ impl Driver<'_> {
21192101

21202102
if start {
21212103
add_cmd(cmd_iterator, Command::Start)?;
2122-
// WRITE command
2104+
// WRITE 7-bit address
21232105
add_cmd(
21242106
cmd_iterator,
21252107
Command::Write {
@@ -2131,60 +2113,32 @@ impl Driver<'_> {
21312113
}
21322114

21332115
if initial_len > 0 {
2134-
if cfg!(any(esp32, esp32s2)) {
2135-
// try to place END at the same index
2136-
if initial_len < 2 || start {
2137-
add_cmd(
2138-
cmd_iterator,
2139-
Command::Read {
2140-
ack_value: Ack::Ack,
2141-
length: initial_len as u8,
2142-
},
2143-
)?;
2144-
} else if !will_continue {
2145-
add_cmd(
2146-
cmd_iterator,
2147-
Command::Read {
2148-
ack_value: Ack::Ack,
2149-
length: (initial_len as u8) - 1,
2150-
},
2151-
)?;
2152-
add_cmd(
2153-
cmd_iterator,
2154-
Command::Read {
2155-
ack_value: Ack::Ack,
2156-
length: 1,
2157-
},
2158-
)?;
2159-
} else {
2160-
add_cmd(
2161-
cmd_iterator,
2162-
Command::Read {
2163-
ack_value: Ack::Ack,
2164-
length: (initial_len as u8) - 2,
2165-
},
2166-
)?;
2167-
add_cmd(
2168-
cmd_iterator,
2169-
Command::Read {
2170-
ack_value: Ack::Ack,
2171-
length: 1,
2172-
},
2173-
)?;
2174-
add_cmd(
2175-
cmd_iterator,
2176-
Command::Read {
2177-
ack_value: Ack::Ack,
2178-
length: 1,
2179-
},
2180-
)?;
2116+
let extra_commands = if cfg!(esp32) {
2117+
match (start, will_continue) {
2118+
// No chunking (START-WRITE-READ-STOP) or first chunk (START-WRITE-READ-END)
2119+
(true, _) => 0,
2120+
// Middle chunk - (READ-READ-READ-END)
2121+
(false, true) => 2,
2122+
// Last chunk - (READ-READ-STOP-END)
2123+
(false, false) => 1 - stop as u8,
21812124
}
21822125
} else {
2126+
0
2127+
};
2128+
2129+
add_cmd(
2130+
cmd_iterator,
2131+
Command::Read {
2132+
ack_value: Ack::Ack,
2133+
length: initial_len as u8 - extra_commands,
2134+
},
2135+
)?;
2136+
for _ in 0..extra_commands {
21832137
add_cmd(
21842138
cmd_iterator,
21852139
Command::Read {
21862140
ack_value: Ack::Ack,
2187-
length: initial_len as u8,
2141+
length: 1,
21882142
},
21892143
)?;
21902144
}
@@ -2437,24 +2391,28 @@ impl Driver<'_> {
24372391
fn start_write_operation(
24382392
&self,
24392393
address: I2cAddress,
2440-
bytes: &[u8],
2394+
buffer: &[u8],
24412395
start: bool,
24422396
stop: bool,
24432397
deadline: Deadline,
24442398
) -> Result<Option<Instant>, Error> {
24452399
let cmd_iterator = &mut self.regs().comd_iter();
24462400

2447-
self.setup_write(address, bytes, start, cmd_iterator)?;
2401+
self.setup_write(address, buffer, start, stop, cmd_iterator)?;
24482402

24492403
if stop {
24502404
add_cmd(cmd_iterator, Command::Stop)?;
2451-
} else {
2405+
}
2406+
if !(start && stop) {
2407+
// Multi-chunk write, terminate with END. ESP32 TRM suggests a write should work with
2408+
// only a STOP at the end, but STOP does not seem to generate a TX_COMPLETE interrupt
2409+
// without END.
24522410
add_cmd(cmd_iterator, Command::End)?;
24532411
}
2412+
24542413
self.start_transmission();
2455-
let deadline = deadline.start(bytes.len() + start as usize);
24562414

2457-
Ok(deadline)
2415+
Ok(deadline.start(buffer.len() + address.bytes()))
24582416
}
24592417

24602418
/// Executes an I2C read operation.
@@ -2482,17 +2440,19 @@ impl Driver<'_> {
24822440

24832441
let cmd_iterator = &mut self.regs().comd_iter();
24842442

2485-
self.setup_read(address, buffer, start, will_continue, cmd_iterator)?;
2443+
self.setup_read(address, buffer, start, stop, will_continue, cmd_iterator)?;
24862444

24872445
if stop {
24882446
add_cmd(cmd_iterator, Command::Stop)?;
2489-
} else {
2447+
}
2448+
if !(start && stop) {
2449+
// Multi-chunk read, terminate with END. On ESP32, assume same limitation as writes.
24902450
add_cmd(cmd_iterator, Command::End)?;
24912451
}
2452+
24922453
self.start_transmission();
2493-
let deadline = deadline.start(buffer.len() + start as usize);
24942454

2495-
Ok(deadline)
2455+
Ok(deadline.start(buffer.len() + address.bytes()))
24962456
}
24972457

24982458
/// Executes an I2C write operation.

‎qa-test/src/bin/i2c_ssd1306_length_test.rs‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
//! Test an SSD1306 display with various read/write data lengths.
2+
//!
3+
//! The following wiring is assumed:
4+
//! - SDA => GPIO4
5+
//! - SCL => GPIO5
6+
7+
//% CHIPS: esp32 esp32c2 esp32c3 esp32c6 esp32h2 esp32s2 esp32s3
8+
//% TAG: ssd1306
9+
10+
#![no_std]
11+
#![no_main]
12+
13+
use esp_backtrace as _;
14+
use esp_hal::{
15+
i2c::master::{Config, I2c},
16+
main,
17+
};
18+
use esp_println::println;
19+
20+
esp_bootloader_esp_idf::esp_app_desc!();
21+
22+
#[main]
23+
fn main() -> ! {
24+
esp_println::logger::init_logger_from_env();
25+
let peripherals = esp_hal::init(esp_hal::Config::default());
26+
27+
// Create a new peripheral object with the described wiring and standard
28+
// I2C clock speed:
29+
let mut i2c = I2c::new(peripherals.I2C0, Config::default())
30+
.unwrap()
31+
.with_sda(peripherals.GPIO4)
32+
.with_scl(peripherals.GPIO5);
33+
34+
let mut data = [0u8; 1024];
35+
36+
println!("Testing writes");
37+
for limit in 0..data.len() {
38+
if let Err(e) = i2c.write(0x3c, &data[0..limit]) {
39+
println!("Error with len {}: {}", limit, e);
40+
}
41+
println!("{} bytes ok", limit);
42+
}
43+
44+
println!("Testing reads");
45+
for limit in 0..data.len() {
46+
if let Err(e) = i2c.read(0x3c, &mut data[0..limit]) {
47+
println!("Error with len {}: {}", limit, e);
48+
}
49+
println!("{} bytes ok", limit);
50+
}
51+
52+
println!("Done");
53+
54+
loop {}
55+
}

0 commit comments

Comments
 (0)