From 21e26f08d18a54ed601b812659c36f13e6614b78 Mon Sep 17 00:00:00 2001 From: Daniel Schaefer Date: Thu, 1 Oct 2026 16:56:55 +0800 Subject: [PATCH 01/10] i2c_passthrough: Add i2c_read_addr16 for 16-bit register addresses i2c_read picks the register address width based on the value: one byte if it fits, two bytes otherwise. Devices like the CCGX PD controllers always use 16-bit register addresses, even for small values, so add i2c_read_addr16 that always sends two bytes (LSB first). Both share the same implementation, which now takes the already encoded address bytes and logs them instead of the numeric address. Signed-off-by: Daniel Schaefer --- .../src/chromium_ec/i2c_passthrough.rs | 38 +++++++++++++++---- 1 file changed, 30 insertions(+), 8 deletions(-) diff --git a/framework_lib/src/chromium_ec/i2c_passthrough.rs b/framework_lib/src/chromium_ec/i2c_passthrough.rs index 3649ac2..8836a10 100644 --- a/framework_lib/src/chromium_ec/i2c_passthrough.rs +++ b/framework_lib/src/chromium_ec/i2c_passthrough.rs @@ -63,18 +63,45 @@ impl EcI2cPassthruResponse { /// Indicate that it's a read, not a write const I2C_READ_FLAG: u16 = 1 << 15; +/// Read from a device with a one byte register address (two bytes if addr > 0xFF) pub fn i2c_read( ec: &CrosEc, i2c_port: u8, i2c_addr: u16, addr: u16, len: u16, +) -> EcResult { + let addr_bytes = if addr < 0xFF { + vec![addr as u8] + } else { + u16::to_le_bytes(addr).to_vec() + }; + i2c_read_impl(ec, i2c_port, i2c_addr, &addr_bytes, len) +} + +/// Read from a device that always uses two byte register addresses (LSB first) +pub fn i2c_read_addr16( + ec: &CrosEc, + i2c_port: u8, + i2c_addr: u16, + addr: u16, + len: u16, +) -> EcResult { + i2c_read_impl(ec, i2c_port, i2c_addr, &u16::to_le_bytes(addr), len) +} + +fn i2c_read_impl( + ec: &CrosEc, + i2c_port: u8, + i2c_addr: u16, + addr_bytes: &[u8], + len: u16, ) -> EcResult { trace!( - "i2c_read(i2c_port: 0x{:X}, i2c_addr: 0x{:X}, addr: 0x{:X}, len: 0x{:X})", + "i2c_read(i2c_port: 0x{:X}, i2c_addr: 0x{:X}, addr: {:X?}, len: 0x{:X})", i2c_port, i2c_addr, - addr, + addr_bytes, len ); if usize::from(len) > MAX_I2C_CHUNK { @@ -83,11 +110,6 @@ pub fn i2c_read( len ))); } - let addr_bytes = if addr < 0xFF { - vec![addr as u8] - } else { - u16::to_le_bytes(addr).to_vec() - }; let messages = vec![ EcParamsI2cPassthruMsg { addr_and_flags: i2c_addr, @@ -112,7 +134,7 @@ pub fn i2c_read( let mut buffer: Vec = vec![0; params_len + msgs_len + addr_bytes.len()]; buffer[0..params_len].copy_from_slice(params_buffer); buffer[params_len..params_len + msgs_len].copy_from_slice(msgs_buffer); - buffer[params_len + msgs_len..].copy_from_slice(&addr_bytes); + buffer[params_len + msgs_len..].copy_from_slice(addr_bytes); let data = ec.send_command(EcCommands::I2cPassthrough as u16, 0, &buffer)?; let res: _EcI2cPassthruResponse = unsafe { std::ptr::read(data.as_ptr() as *const _) }; From 6a0f4d8eb069d35ff351208214a5aa46009d0359 Mon Sep 17 00:00:00 2001 From: Daniel Schaefer Date: Thu, 1 Oct 2026 17:52:57 +0800 Subject: [PATCH 02/10] i2c_passthrough: Send register 0xFF as a one byte address The one byte form was chosen for addr < 0xFF, so 0xFF was sent as two bytes [0xFF, 0x00] and the device saw the wrong register. Signed-off-by: Daniel Schaefer --- framework_lib/src/chromium_ec/i2c_passthrough.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework_lib/src/chromium_ec/i2c_passthrough.rs b/framework_lib/src/chromium_ec/i2c_passthrough.rs index 8836a10..b6db51d 100644 --- a/framework_lib/src/chromium_ec/i2c_passthrough.rs +++ b/framework_lib/src/chromium_ec/i2c_passthrough.rs @@ -71,7 +71,7 @@ pub fn i2c_read( addr: u16, len: u16, ) -> EcResult { - let addr_bytes = if addr < 0xFF { + let addr_bytes = if addr <= 0xFF { vec![addr as u8] } else { u16::to_le_bytes(addr).to_vec() From b728154afafed42e951a3e7895b44f8ae65b99f2 Mon Sep 17 00:00:00 2001 From: Daniel Schaefer Date: Wed, 14 Oct 2026 10:35:33 +0800 Subject: [PATCH 03/10] chromium_ec: Fix response length from the Windows driver The driver's IOCTL_CROSEC_XCMD reply length is sizeof(CROSEC_COMMAND) plus the EC response, where CROSEC_COMMAND is a 20 byte (5x UINT32) header followed by a flexible Data[] array: requiredReplySize = sizeof(CROSEC_COMMAND) + res; WdfRequestSetInformation(Request, requiredReplySize); But we sliced cmd.buffer, which already starts after that header, with the full reply length. So every command returned 20 bytes of leftover request data after the response. With METHOD_BUFFERED the input and output buffer are the same, so those bytes are whatever we just sent. That's where the mysterious +20 in assert_win_len and in send_command_extra came from. Subtract the header instead, so the Windows driver returns the same length as portio and cros_ec do, and drop both workarounds. It also means a failed I2C read no longer looks like it returned data. On a Laptop 16 whose dGPU retimer is asleep, the read NAKs and returns nothing, but `framework_tool --versions` reported Parade Retimers dGPU: 11.0.1.0 which is just the first four bytes of our own request buffer (0x11, 0x00, 0x01, 0x00) read back. Signed-off-by: Daniel Schaefer --- framework_lib/src/ccgx/device.rs | 12 ++++++------ framework_lib/src/chromium_ec/command.rs | 7 +------ framework_lib/src/chromium_ec/i2c_passthrough.rs | 4 ++-- framework_lib/src/chromium_ec/mod.rs | 6 +++--- framework_lib/src/chromium_ec/windows.rs | 13 +++++++++---- framework_lib/src/util.rs | 10 ---------- 6 files changed, 21 insertions(+), 31 deletions(-) diff --git a/framework_lib/src/ccgx/device.rs b/framework_lib/src/ccgx/device.rs index 91376c6..ba4cc96 100644 --- a/framework_lib/src/ccgx/device.rs +++ b/framework_lib/src/ccgx/device.rs @@ -10,7 +10,7 @@ use core::prelude::rust_2021::derive; use crate::ccgx::{AppVersion, BaseVersion, ControllerVersion}; use crate::chromium_ec::i2c_passthrough::*; use crate::chromium_ec::{CrosEc, EcError, EcResult}; -use crate::util::{assert_win_len, Config, Platform}; +use crate::util::{Config, Platform}; use super::*; @@ -281,7 +281,7 @@ impl PdController { pub fn get_silicon_id(&self) -> EcResult { let data = self.ccgx_read(ControlRegisters::SiliconId, 2)?; - assert_win_len(data.len(), 2); + assert_eq!(data.len(), 2); Ok(u16::from_le_bytes([data[0], data[1]])) } @@ -342,7 +342,7 @@ impl PdController { } }; - assert_win_len(data.len(), 8); + assert_eq!(data.len(), 8); let base_ver = BaseVersion::from(&data[..4]); let app_ver = AppVersion::from(&data[4..]); println!( @@ -352,7 +352,7 @@ impl PdController { let data = self.ccgx_read(ControlRegisters::Firmware1Version, 8); let data = data.unwrap(); - assert_win_len(data.len(), 8); + assert_eq!(data.len(), 8); let base_ver = BaseVersion::from(&data[..4]); let app_ver = AppVersion::from(&data[4..]); println!( @@ -362,7 +362,7 @@ impl PdController { let data = self.ccgx_read(ControlRegisters::Firmware2Version, 8); let data = data.unwrap(); - assert_win_len(data.len(), 8); + assert_eq!(data.len(), 8); let base_ver = BaseVersion::from(&data[..4]); let app_ver = AppVersion::from(&data[4..]); println!( @@ -387,7 +387,7 @@ impl PdController { pub fn get_port_status(&self) -> EcResult { let data = self.ccgx_read(ControlRegisters::PdPortsEnable, 1)?; - assert_win_len(data.len(), 1); + assert_eq!(data.len(), 1); Ok(data[0]) } } diff --git a/framework_lib/src/chromium_ec/command.rs b/framework_lib/src/chromium_ec/command.rs index ea608b7..288dab4 100644 --- a/framework_lib/src/chromium_ec/command.rs +++ b/framework_lib/src/chromium_ec/command.rs @@ -201,12 +201,7 @@ pub trait EcRequestRaw { Self: Sized, { let response = self.send_command_vec_extra(ec, extra_data)?; - // TODO: The Windows driver seems to return 20 more bytes than expected - #[cfg(windows)] - let expected = response.len() != std::mem::size_of::() + 20; - #[cfg(not(windows))] - let expected = response.len() != std::mem::size_of::(); - if expected { + if response.len() != std::mem::size_of::() { return Err(EcError::DeviceError(format!( "Returned data size ({}) is not the expted size: {}", response.len(), diff --git a/framework_lib/src/chromium_ec/i2c_passthrough.rs b/framework_lib/src/chromium_ec/i2c_passthrough.rs index b6db51d..b72e862 100644 --- a/framework_lib/src/chromium_ec/i2c_passthrough.rs +++ b/framework_lib/src/chromium_ec/i2c_passthrough.rs @@ -253,7 +253,7 @@ pub fn i2c_write_block( let data = ec.send_command(EcCommands::I2cPassthrough as u16, 0, &buffer)?; let res: _EcI2cPassthruResponse = unsafe { std::ptr::read(data.as_ptr() as *const _) }; - util::assert_win_len(data.len(), size_of::<_EcI2cPassthruResponse>()); // No extra data other than the header + assert_eq!(data.len(), size_of::<_EcI2cPassthruResponse>()); // No extra data other than the header // Only assert message count if the transfer was successful // On error (NAK/timeout), the EC may return 0 messages processed @@ -305,7 +305,7 @@ pub fn i2c_write( let data = ec.send_command(EcCommands::I2cPassthrough as u16, 0, &buffer)?; let res: _EcI2cPassthruResponse = unsafe { std::ptr::read(data.as_ptr() as *const _) }; - util::assert_win_len(data.len(), size_of::<_EcI2cPassthruResponse>()); // No extra data other than the header + assert_eq!(data.len(), size_of::<_EcI2cPassthruResponse>()); // No extra data other than the header // Only assert message count if the transfer was successful // On error (NAK/timeout), the EC may return 0 messages processed diff --git a/framework_lib/src/chromium_ec/mod.rs b/framework_lib/src/chromium_ec/mod.rs index b90ad58..4ac88b0 100644 --- a/framework_lib/src/chromium_ec/mod.rs +++ b/framework_lib/src/chromium_ec/mod.rs @@ -451,7 +451,7 @@ impl CrosEc { let limits = &[ChargeLimitControlModes::Set as u8, max, min]; let data = self.send_command(EcCommands::ChargeLimitControl as u16, 0, limits)?; - util::assert_win_len(data.len(), 0); + assert_eq!(data.len(), 0); Ok(()) } @@ -515,7 +515,7 @@ impl CrosEc { let limits = &[percentage, 0x00]; let data = self.send_command(EcCommands::FpLedLevelControl as u16, 1, limits)?; - util::assert_win_len(data.len(), 0); + assert_eq!(data.len(), 0); Ok(()) } @@ -526,7 +526,7 @@ impl CrosEc { let limits = &[level as u8, 0x00]; let data = self.send_command(EcCommands::FpLedLevelControl as u16, 0, limits)?; - util::assert_win_len(data.len(), 0); + assert_eq!(data.len(), 0); Ok(()) } diff --git a/framework_lib/src/chromium_ec/windows.rs b/framework_lib/src/chromium_ec/windows.rs index 936243c..55cf35e 100644 --- a/framework_lib/src/chromium_ec/windows.rs +++ b/framework_lib/src/chromium_ec/windows.rs @@ -169,10 +169,15 @@ pub fn send_command(command: u16, command_version: u8, data: &[u8]) -> EcResult< Some(status) => return Err(EcError::Response(status)), } - // TODO: Figure out why that's sometimes bigger - let end = std::cmp::min(returned, CROSEC_CMD_MAX_REQUEST as u32); - - let out_buffer = &cmd.buffer[..(end as usize)]; + // The driver reports how many bytes of CROSEC_COMMAND it filled in, which includes its + // 20 byte header. But cmd.buffer already starts after that header, so subtract it to get + // the length of the EC response itself. + // Otherwise we'd return 20 bytes of leftover request data after the response. + let crosec_header_len = buf_size - CROSEC_CMD_MAX_REQUEST; + let data_len = (returned as usize).saturating_sub(crosec_header_len); + let end = std::cmp::min(data_len, CROSEC_CMD_MAX_REQUEST); + + let out_buffer = &cmd.buffer[..end]; Ok(out_buffer.to_vec()) } diff --git a/framework_lib/src/util.rs b/framework_lib/src/util.rs index 03cc7cc..307ab31 100644 --- a/framework_lib/src/util.rs +++ b/framework_lib/src/util.rs @@ -1,6 +1,5 @@ //! Miscellaneous utility functions to use across modules -use num::{Num, NumCast}; use std::prelude::v1::*; #[cfg(feature = "uefi")] @@ -267,15 +266,6 @@ pub fn find_sequence(haystack: &[u8], needle: &[u8]) -> Option { .position(|window| window == needle) } -/// Assert length of an EC response from the windows driver -/// It's always 20 more than expected. TODO: Figure out why -pub fn assert_win_len(left: N, right: N) { - #[cfg(windows)] - assert_eq!(left, right + NumCast::from(20).unwrap()); - #[cfg(not(windows))] - assert_eq!(left, right); -} - pub fn format_duration(duration: &Duration) -> String { let hours = duration.as_secs() / 3600; let minutes = (duration.as_secs() % 3600) / 60; From 7b7b014e7a4c6367c984c18288a1a06ee4809cde Mon Sep 17 00:00:00 2001 From: Daniel Schaefer Date: Wed, 14 Oct 2026 10:35:51 +0800 Subject: [PATCH 04/10] i2c_passthrough: Deduplicate i2c_read_16bit_addr It only differs from i2c_read_impl in the byte order of the register address, so let it share the implementation. This also drops its unconditional truncation to the requested length. That was a workaround for the Windows driver returning extra bytes, which is fixed now. Worse, it could panic: when the device NAKs, the EC stops before the read message and returns only the 2 byte header, see i2c_command_passthru() in common/i2c_passthru.c: args->response_size = sizeof(*resp) + in_len; Returning the short response as-is is what the callers expect anyway, they check i2c_status before looking at the data. Signed-off-by: Daniel Schaefer --- .../src/chromium_ec/i2c_passthrough.rs | 70 ++++--------------- 1 file changed, 12 insertions(+), 58 deletions(-) diff --git a/framework_lib/src/chromium_ec/i2c_passthrough.rs b/framework_lib/src/chromium_ec/i2c_passthrough.rs index b72e862..8c0ced1 100644 --- a/framework_lib/src/chromium_ec/i2c_passthrough.rs +++ b/framework_lib/src/chromium_ec/i2c_passthrough.rs @@ -138,17 +138,26 @@ fn i2c_read_impl( let data = ec.send_command(EcCommands::I2cPassthrough as u16, 0, &buffer)?; let res: _EcI2cPassthruResponse = unsafe { std::ptr::read(data.as_ptr() as *const _) }; + // The EC only returns as many bytes as it managed to read. + // On NAK or timeout that's less than requested, possibly nothing at all. let res_data = &data[size_of::<_EcI2cPassthruResponse>()..]; // TODO: Seems to be either one, non-deterministically debug_assert!(res.messages as usize == messages.len() || res.messages == 0); + let i2c_status = res.i2c_status; + trace!( + " i2c_read response (status: 0x{:02X}, len: {}, data: {:#04X?})", + i2c_status, + res_data.len(), + res_data + ); Ok(EcI2cPassthruResponse { - i2c_status: res.i2c_status, + i2c_status, data: res_data.to_vec(), }) } /// I2C read with 16-bit addressing (for larger EEPROMs like 24C32+) -/// Always sends a 2-byte address, even for addr=0 +/// Always sends a 2-byte address (MSB first), even for addr=0 pub fn i2c_read_16bit_addr( ec: &CrosEc, i2c_port: u8, @@ -156,62 +165,7 @@ pub fn i2c_read_16bit_addr( addr: u16, len: u16, ) -> EcResult { - trace!( - "i2c_read_16bit_addr(i2c_port: 0x{:X}, i2c_addr: 0x{:X}, addr: 0x{:X}, len: 0x{:X})", - i2c_port, - i2c_addr, - addr, - len - ); - if usize::from(len) > MAX_I2C_CHUNK { - return EcResult::Err(EcError::DeviceError(format!( - "i2c_read too long. Must be <128, is: {}", - len - ))); - } - // Always use 16-bit addressing (big-endian for EEPROM) - let addr_bytes = u16::to_be_bytes(addr).to_vec(); - let messages = vec![ - EcParamsI2cPassthruMsg { - addr_and_flags: i2c_addr, - transfer_len: addr_bytes.len() as u16, - }, - EcParamsI2cPassthruMsg { - addr_and_flags: i2c_addr + I2C_READ_FLAG, - transfer_len: len, // How much to read - }, - ]; - let msgs_len = size_of::() * messages.len(); - let msgs_buffer: &[u8] = unsafe { util::any_vec_as_u8_slice(&messages) }; - - let params = EcParamsI2cPassthru { - port: i2c_port, - messages: messages.len() as u8, - msg: [], // Messages are copied right after this struct - }; - let params_len = size_of::(); - let params_buffer: &[u8] = unsafe { util::any_as_u8_slice(¶ms) }; - - let mut buffer: Vec = vec![0; params_len + msgs_len + addr_bytes.len()]; - buffer[0..params_len].copy_from_slice(params_buffer); - buffer[params_len..params_len + msgs_len].copy_from_slice(msgs_buffer); - buffer[params_len + msgs_len..].copy_from_slice(&addr_bytes); - - let data = ec.send_command(EcCommands::I2cPassthrough as u16, 0, &buffer)?; - let res: _EcI2cPassthruResponse = unsafe { std::ptr::read(data.as_ptr() as *const _) }; - let header_len: usize = size_of::<_EcI2cPassthruResponse>(); - /* Note on windows, you can get extra bytes back, so truncate this to the requested size */ - let res_data = &data[header_len..(len as usize + header_len)]; - debug_assert!(res.messages as usize == messages.len() || res.messages == 0); - trace!( - " i2c_read_16bit_addr response (len: {}, data: {:#04X?})", - res_data.len(), - res_data.to_vec() - ); - Ok(EcI2cPassthruResponse { - i2c_status: res.i2c_status, - data: res_data.to_vec(), - }) + i2c_read_impl(ec, i2c_port, i2c_addr, &u16::to_be_bytes(addr), len) } /* Write address and bytes in a single I2C transfer */ From 9b77a714e63f78e73d8eb9dee3bb66296bc88095 Mon Sep 17 00:00:00 2001 From: Daniel Schaefer Date: Wed, 14 Oct 2026 10:36:03 +0800 Subject: [PATCH 05/10] i2c_passthrough: Don't loop forever if a chunked read returns nothing Both chunked readers loop until they have as many bytes as requested. Before, every read grew the buffer because the response was padded to the requested length, on Windows even past it. Now a read can come back short, so bail out instead of retrying the same chunk forever. The EC only returns a short response when it also sets i2c_status, so the preceding is_successful() check should already catch it. This is just to make sure we can't hang. Signed-off-by: Daniel Schaefer --- framework_lib/src/ccgx/device.rs | 6 ++++++ framework_lib/src/chromium_ec/mod.rs | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/framework_lib/src/ccgx/device.rs b/framework_lib/src/ccgx/device.rs index ba4cc96..5765501 100644 --- a/framework_lib/src/ccgx/device.rs +++ b/framework_lib/src/ccgx/device.rs @@ -244,6 +244,12 @@ impl PdController { err ))); } + // Don't loop forever if the EC doesn't return any data + if i2c_response.data.is_empty() { + return Err(EcError::DeviceError( + "I2C read returned no data".to_string(), + )); + } data.extend(i2c_response.data); } diff --git a/framework_lib/src/chromium_ec/mod.rs b/framework_lib/src/chromium_ec/mod.rs index 4ac88b0..a4e4cd8 100644 --- a/framework_lib/src/chromium_ec/mod.rs +++ b/framework_lib/src/chromium_ec/mod.rs @@ -1460,6 +1460,12 @@ impl CrosEc { err ))); } + // Don't loop forever if the EC doesn't return any data + if i2c_response.data.is_empty() { + return Err(EcError::DeviceError( + "I2C read returned no data".to_string(), + )); + } data.extend(i2c_response.data); } From f6ac5284694cf7c39711253a325529355d42310e Mon Sep 17 00:00:00 2001 From: Daniel Schaefer Date: Wed, 14 Oct 2026 10:51:34 +0800 Subject: [PATCH 06/10] parade_retimer: Report a retimer that doesn't answer as unknown get_version() returns Ok(None) for "no retimer on this system", which makes --versions print nothing at all. It also took that path when the retimer did not return any data, so a dGPU whose retimer is asleep looked like a system without one. Decide it by the I2C status the EC reports instead of by how many bytes came back. Once the EC has told us there's a compatible dGPU we know the retimer exists, so a NAK is an error, and the caller already prints Parade Retimers Unknown for that. The length based check only worked before because a failed read still came back padded to the requested size. Signed-off-by: Daniel Schaefer --- framework_lib/src/parade_retimer.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/framework_lib/src/parade_retimer.rs b/framework_lib/src/parade_retimer.rs index 6f0cf8c..7af14c7 100644 --- a/framework_lib/src/parade_retimer.rs +++ b/framework_lib/src/parade_retimer.rs @@ -1,10 +1,11 @@ +use alloc::string::ToString; use alloc::vec::Vec; use num_traits::FromPrimitive; use crate::chromium_ec::command::EcRequestRaw; use crate::chromium_ec::commands::{EcRequestGetGpuPcie, GpuVendor}; use crate::chromium_ec::i2c_passthrough::*; -use crate::chromium_ec::{CrosEc, EcResult}; +use crate::chromium_ec::{CrosEc, EcError, EcResult}; use crate::os_specific; pub fn get_version(ec: &CrosEc) -> EcResult>> { @@ -35,13 +36,16 @@ pub fn get_version(ec: &CrosEc) -> EcResult>> { i2c_response.i2c_status, i2c_response.data.len() ); - if i2c_response.i2c_status == 0x01 { - // NAK + // We know the retimer is there, because the EC reported a compatible dGPU. + // So if it doesn't answer, report that instead of pretending it doesn't exist. + if let Err(err) = i2c_response.is_successful() { warn!("Unable to communicate with dGPU Retimer. Try to force it on by plugging a cable into the dGPU"); + return Err(err); } let Some(&safe_mode) = i2c_response.data.first() else { - info!("Failed to read retimer safe mode status (empty response)"); - return Ok(None); + return Err(EcError::DeviceError( + "Retimer did not return the safe mode status".to_string(), + )); }; info!("Retimer safe mode status: {}", safe_mode); if safe_mode == 0 { @@ -61,6 +65,7 @@ pub fn get_version(ec: &CrosEc) -> EcResult>> { // Read version let i2c_response = i2c_read(ec, i2c_port, (i2c_addr + 18) >> 1, 0x01, 0x04)?; + i2c_response.is_successful()?; Ok(Some(i2c_response.data)) } From 51b4f6d294ea3b6464116631ad886df99dbdc01b Mon Sep 17 00:00:00 2001 From: Daniel Schaefer Date: Wed, 14 Oct 2026 11:00:11 +0800 Subject: [PATCH 07/10] i2c_passthrough: Don't assert an exact message count on reads The EC processes passthru messages in order and stops at the first one that fails, see i2c_command_passthru() in common/i2c_passthru.c. So on a read (one write message plus one read message) num_msgs can also be 1, when the register address write is ACKed but the read is NAKed. The old assert allowed only 0 or all messages and would panic a debug build in that case. Assert the only real invariant instead: the EC can't process more messages than we sent. That matches what the kernel checks in ec_i2c_parse_response(), see drivers/i2c/busses/i2c-cros-ec-tunnel.c: /* Other side could send us back fewer messages, but not more */ if (resp->num_msgs > *num) return -EPROTO; Signed-off-by: Daniel Schaefer --- framework_lib/src/chromium_ec/i2c_passthrough.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/framework_lib/src/chromium_ec/i2c_passthrough.rs b/framework_lib/src/chromium_ec/i2c_passthrough.rs index 8c0ced1..c91f719 100644 --- a/framework_lib/src/chromium_ec/i2c_passthrough.rs +++ b/framework_lib/src/chromium_ec/i2c_passthrough.rs @@ -141,8 +141,9 @@ fn i2c_read_impl( // The EC only returns as many bytes as it managed to read. // On NAK or timeout that's less than requested, possibly nothing at all. let res_data = &data[size_of::<_EcI2cPassthruResponse>()..]; - // TODO: Seems to be either one, non-deterministically - debug_assert!(res.messages as usize == messages.len() || res.messages == 0); + // The EC stops at the first message that fails, so it can have processed + // any number of messages up to the requested count + debug_assert!(res.messages as usize <= messages.len()); let i2c_status = res.i2c_status; trace!( " i2c_read response (status: 0x{:02X}, len: {}, data: {:#04X?})", From 45b760f7c995e713d40a775f1887a7651c9de820 Mon Sep 17 00:00:00 2001 From: Daniel Schaefer Date: Wed, 14 Oct 2026 11:02:03 +0800 Subject: [PATCH 08/10] i2c_passthrough: Fix off-by-one in the i2c_read length error message The check rejects len > MAX_I2C_CHUNK, so 128 is allowed. Say <=128 and reference the constant instead of hardcoding it. Signed-off-by: Daniel Schaefer --- framework_lib/src/chromium_ec/i2c_passthrough.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/framework_lib/src/chromium_ec/i2c_passthrough.rs b/framework_lib/src/chromium_ec/i2c_passthrough.rs index c91f719..405627f 100644 --- a/framework_lib/src/chromium_ec/i2c_passthrough.rs +++ b/framework_lib/src/chromium_ec/i2c_passthrough.rs @@ -106,8 +106,8 @@ fn i2c_read_impl( ); if usize::from(len) > MAX_I2C_CHUNK { return EcResult::Err(EcError::DeviceError(format!( - "i2c_read too long. Must be <128, is: {}", - len + "i2c_read too long. Must be <={}, is: {}", + MAX_I2C_CHUNK, len ))); } let messages = vec![ From fcd5863704374dc2bde1f067f2042285082c4251 Mon Sep 17 00:00:00 2001 From: Daniel Schaefer Date: Wed, 14 Oct 2026 11:02:19 +0800 Subject: [PATCH 09/10] chromium_ec: Fix typo in response size error message Signed-off-by: Daniel Schaefer --- framework_lib/src/chromium_ec/command.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework_lib/src/chromium_ec/command.rs b/framework_lib/src/chromium_ec/command.rs index 288dab4..1d2688f 100644 --- a/framework_lib/src/chromium_ec/command.rs +++ b/framework_lib/src/chromium_ec/command.rs @@ -203,7 +203,7 @@ pub trait EcRequestRaw { let response = self.send_command_vec_extra(ec, extra_data)?; if response.len() != std::mem::size_of::() { return Err(EcError::DeviceError(format!( - "Returned data size ({}) is not the expted size: {}", + "Returned data size ({}) is not the expected size: {}", response.len(), std::mem::size_of::() ))); From 5e06c480b70a5ed11ea5c289fde65542c9652ef7 Mon Sep 17 00:00:00 2001 From: Daniel Schaefer Date: Wed, 14 Oct 2026 11:03:13 +0800 Subject: [PATCH 10/10] chromium_ec: Use offset_of to compute the driver header length Express "where does the data buffer start inside CROSEC_COMMAND" directly instead of deriving it from the struct and buffer sizes. No functional change. Signed-off-by: Daniel Schaefer --- framework_lib/src/chromium_ec/windows.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/framework_lib/src/chromium_ec/windows.rs b/framework_lib/src/chromium_ec/windows.rs index 55cf35e..050d734 100644 --- a/framework_lib/src/chromium_ec/windows.rs +++ b/framework_lib/src/chromium_ec/windows.rs @@ -173,7 +173,7 @@ pub fn send_command(command: u16, command_version: u8, data: &[u8]) -> EcResult< // 20 byte header. But cmd.buffer already starts after that header, so subtract it to get // the length of the EC response itself. // Otherwise we'd return 20 bytes of leftover request data after the response. - let crosec_header_len = buf_size - CROSEC_CMD_MAX_REQUEST; + let crosec_header_len = std::mem::offset_of!(CrosEcCommand, buffer); let data_len = (returned as usize).saturating_sub(crosec_header_len); let end = std::cmp::min(data_len, CROSEC_CMD_MAX_REQUEST);