From 05fae5d00f9c3fb949007d20d4ca3a88cc006530 Mon Sep 17 00:00:00 2001 From: Amaar Ebrahim Date: Fri, 11 Jul 2025 18:30:45 -0500 Subject: [PATCH 1/2] raspberrypi/I2CTarget: Fixed bug where I2C starts were seen as restarts. The Rasperry Pi Pico, based on the RP2040, has a register that maintains the status of I2C interrupt flags called IC_INTR_STAT. The bits of this register are set by hardware and cleared by software. Before this commit, the I2CTarget library did not clear the restart bit (R_RESTART_DET) in this register after an I2C transaction ended, causing the is_restart field of the i2ctarget_i2c_target_request_obj_t struct to always be true after the first I2C transaction. This commit causes the restart and stop bits to get cleared when the I2C transaction ends. Signed-off-by: Amaar Ebrahim --- .../raspberrypi/common-hal/i2ctarget/I2CTarget.c | 15 +++++++++++++++ .../raspberrypi/common-hal/i2ctarget/I2CTarget.h | 2 ++ 2 files changed, 17 insertions(+) diff --git a/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.c b/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.c index d1c92eea9988f..655dfd867ca38 100644 --- a/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.c +++ b/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.c @@ -85,6 +85,8 @@ void common_hal_i2ctarget_i2c_target_deinit(i2ctarget_i2c_target_obj_t *self) { } int common_hal_i2ctarget_i2c_target_is_addressed(i2ctarget_i2c_target_obj_t *self, uint8_t *address, bool *is_read, bool *is_restart) { + common_hal_i2ctarget_i2c_target_is_stop(self); + if (!((self->peripheral->hw->raw_intr_stat & I2C_IC_INTR_STAT_R_RX_FULL_BITS) || (self->peripheral->hw->raw_intr_stat & I2C_IC_INTR_STAT_R_RD_REQ_BITS))) { return 0; } @@ -123,6 +125,19 @@ int common_hal_i2ctarget_i2c_target_write_byte(i2ctarget_i2c_target_obj_t *self, } } +int common_hal_i2ctarget_i2c_target_is_stop(i2ctarget_i2c_target_obj_t *self) { + // Interrupt bits must be cleared by software. Clear the STOP and + // RESTART interrupt bits after an I2C transaction finishes. + if (self->peripheral->hw->raw_intr_stat & I2C_IC_INTR_STAT_R_STOP_DET_BITS) { + self->peripheral->hw->clr_stop_det; + self->peripheral->hw->clr_restart_det; + return 1; + } else { + return 0; + } + +} + void common_hal_i2ctarget_i2c_target_ack(i2ctarget_i2c_target_obj_t *self, bool ack) { return; } diff --git a/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.h b/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.h index 5d4e0690cbffd..67b09c18c2f12 100644 --- a/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.h +++ b/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.h @@ -21,3 +21,5 @@ typedef struct { uint8_t scl_pin; uint8_t sda_pin; } i2ctarget_i2c_target_obj_t; + +int common_hal_i2ctarget_i2c_target_is_stop(i2ctarget_i2c_target_obj_t *self); From e376e2416b84d7b06f3f74843682d1d74d13efd4 Mon Sep 17 00:00:00 2001 From: foamyguy Date: Fri, 10 Apr 2026 15:52:45 -0500 Subject: [PATCH 2/2] refactor to remove common_hal_i2ctarget_i2c_target_is_stop(). Fix separate issue with read_byte returning early. --- .../common-hal/i2ctarget/I2CTarget.c | 38 +++++++++---------- .../common-hal/i2ctarget/I2CTarget.h | 2 - 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.c b/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.c index 655dfd867ca38..a40f6e26608a9 100644 --- a/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.c +++ b/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.c @@ -85,7 +85,12 @@ void common_hal_i2ctarget_i2c_target_deinit(i2ctarget_i2c_target_obj_t *self) { } int common_hal_i2ctarget_i2c_target_is_addressed(i2ctarget_i2c_target_obj_t *self, uint8_t *address, bool *is_read, bool *is_restart) { - common_hal_i2ctarget_i2c_target_is_stop(self); + // Interrupt bits must be cleared by software. Clear the STOP and + // RESTART interrupt bits after an I2C transaction finishes. + if (self->peripheral->hw->raw_intr_stat & I2C_IC_INTR_STAT_R_STOP_DET_BITS) { + self->peripheral->hw->clr_stop_det; + self->peripheral->hw->clr_restart_det; + } if (!((self->peripheral->hw->raw_intr_stat & I2C_IC_INTR_STAT_R_RX_FULL_BITS) || (self->peripheral->hw->raw_intr_stat & I2C_IC_INTR_STAT_R_RD_REQ_BITS))) { return 0; @@ -100,12 +105,20 @@ int common_hal_i2ctarget_i2c_target_is_addressed(i2ctarget_i2c_target_obj_t *sel } int common_hal_i2ctarget_i2c_target_read_byte(i2ctarget_i2c_target_obj_t *self, uint8_t *data) { - if (self->peripheral->hw->status & I2C_IC_STATUS_RFNE_BITS) { - *data = (uint8_t)self->peripheral->hw->data_cmd; - return 1; - } else { - return 0; + // Wait for data to arrive or a STOP condition indicating the transfer is over. + // Without this wait, the CPU can drain the FIFO faster than bytes arrive on the + // I2C bus, causing transfers to be split into multiple partial reads. + for (int t = 0; t < 100; t++) { + if (self->peripheral->hw->status & I2C_IC_STATUS_RFNE_BITS) { + *data = (uint8_t)self->peripheral->hw->data_cmd; + return 1; + } + if (self->peripheral->hw->raw_intr_stat & I2C_IC_INTR_STAT_R_STOP_DET_BITS) { + break; + } + mp_hal_delay_us(10); } + return 0; } int common_hal_i2ctarget_i2c_target_write_byte(i2ctarget_i2c_target_obj_t *self, uint8_t data) { @@ -125,19 +138,6 @@ int common_hal_i2ctarget_i2c_target_write_byte(i2ctarget_i2c_target_obj_t *self, } } -int common_hal_i2ctarget_i2c_target_is_stop(i2ctarget_i2c_target_obj_t *self) { - // Interrupt bits must be cleared by software. Clear the STOP and - // RESTART interrupt bits after an I2C transaction finishes. - if (self->peripheral->hw->raw_intr_stat & I2C_IC_INTR_STAT_R_STOP_DET_BITS) { - self->peripheral->hw->clr_stop_det; - self->peripheral->hw->clr_restart_det; - return 1; - } else { - return 0; - } - -} - void common_hal_i2ctarget_i2c_target_ack(i2ctarget_i2c_target_obj_t *self, bool ack) { return; } diff --git a/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.h b/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.h index 67b09c18c2f12..5d4e0690cbffd 100644 --- a/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.h +++ b/ports/raspberrypi/common-hal/i2ctarget/I2CTarget.h @@ -21,5 +21,3 @@ typedef struct { uint8_t scl_pin; uint8_t sda_pin; } i2ctarget_i2c_target_obj_t; - -int common_hal_i2ctarget_i2c_target_is_stop(i2ctarget_i2c_target_obj_t *self);