u-boot.git/drivers/i2c/rk_i2c.c, branch main

rockchip: i2c: fix illegal I2C START/STOP condition

2025-12-13T16:02:10Z

In the last message sent in rockchip_i2c_xfer, the controller is disabled (see rk_i2c_disable() in rk_i2c_read()/rk_i2c_write()), then the STOP condition is sent (see rk_i2c_send_stop_bit() in rockchip_i2c_xfer()) and the controller is disabled once again (see rk_i2c_disable() right after). The issue is that re-enabling the controller just to send the STOP condition doesn't work. When, the controller is disabled, the SCL and SDA lanes are not driven anymore and thus enter the idle mode where they are kept high by the external HW pull-up. To send a STOP condition, one needs to drive the SDA line so that a rising edge happens while SCL is high. Experimentally (on PX30 and RK3399), when enabling the controller to send a STOP condition after it's been disabled, the controller only drives the SDA line to trigger the rising edge for the STOP condition, leaving SCL undriven (and thus, high). This means, that because SDA is high before this happens and that we need a rising edge, the controller drives the SDA line low and then releases it, meaning we trigger a START condition followed by a STOP condition: SCL _________ _____... __ _____ _____... \/ SDA ^ STOP ^ START This is illegal in I2C protocol[1]: 5. A START condition immediately followed by a STOP condition (void message) is an illegal format. Many devices however are designed to operate properly under this condition. My guess is that the I2C controller IP knows that it makes only sense to send a STOP condition after a START condition, meaning the controller is already driving the SCL line low and neither the device nor controller drive the SDA line after the last ACK/NACK as there's no need to, then it needs to drive SDA, release SCL to make it high and then release the SDA line. However, after it's been disabled, the SCL is already released so the controller only essentially drives SDA and then releases it. It happens that this seems to be breaking the SE050 Secure Element after a few transfers in the middle of a transfer where it starts clock stretching the bus forever. It may be related to Errata 3.2[2] but the description of the setup isn't an exact match to the current situation. It seems to be required to disable the I2C controller between messages as the Linux kernel states that "The HW is actually not capable of REPEATED START. But we can get the intended effect by resetting its internal state and issuing an ordinary START.". Between messages, this logic seems fine as I get an Sr (repeated START condition) before starting the next message in the transfer without a STOP condition. However, we should NOT disable the controller after the last message in the transfer otherwise we do this illegal START condition followed by the STOP condition, hence the added check. [1] https://www.nxp.com/docs/en/user-guide/UM10204.pdf 3.1.10 The target address and R/W bit point 5 [2] https://www.nxp.com/docs/en/errata/SE050_Erratasheet.pdf Fixes: c9fca5ec8849 ("rockchip: i2c: don't sent stop bit after each message") Signed-off-by: Quentin Schulz Reviewed-by: Heiko Schocher Reviewed-by: Kever Yang

rockchip: i2c: move ACK comment where it applies

2025-12-13T16:02:10Z

The I2C_CON_LASTACK is kind of a misnomer as setting it means sending a NACK as last byte acknowledge when the controller is in receive mode. It should therefore be used only when there's no more data to transfer after this. Move the comment in the proper if block. Sync the comment with the Linux kernel's while at it so it's more explicit. Fixes: 5deaa530280f ("rockchip: i2c: fix >32 byte reads") Signed-off-by: Quentin Schulz Reviewed-by: Heiko Schocher Reviewed-by: Kever Yang

rockchip: i2c: fix incorrect STOP flag for the interrupt enable register

2025-12-13T16:02:10Z

I2C_CON_STOP is a flag to be used for the con register, where it is bit 4 to send the STOP condition. To enable the interrupt the controller sends to tell it's finished sending the STOP condition, it's the ien register at bit 5. Let's use the proper offset. My hunch is that enabling the interrupt is useless as the interrupt status register is always up-to-date and enabling the interrupt is just so that the interrupt is available via the GIC. However, U-Boot has no interrupt support and the logic was working well before this patch. This is just so people aren't side-tracked when debugging I2C issues on Rockchip by checking all writes are proper. Fixes: 3437469985df ("rockchip: Add I2C driver") Signed-off-by: Quentin Schulz Reviewed-by: Heiko Schocher Reviewed-by: Kever Yang

Restore patch series "arm: dts: am62-beagleplay: Fix Beagleplay Ethernet"

2024-05-20T19:35:03Z

As part of bringing the master branch back in to next, we need to allow for all of these changes to exist here. Reported-by: Jonas Karlman Signed-off-by: Tom Rini

Revert "Merge patch series "arm: dts: am62-beagleplay: Fix Beagleplay Ethernet""

2024-05-19T14:16:36Z

When bringing in the series 'arm: dts: am62-beagleplay: Fix Beagleplay Ethernet"' I failed to notice that b4 noticed it was based on next and so took that as the base commit and merged that part of next to master. This reverts commit c8ffd1356d42223cbb8c86280a083cc3c93e6426, reversing changes made to 2ee6f3a5f7550de3599faef9704e166e5dcace35. Reported-by: Jonas Karlman Signed-off-by: Tom Rini

i2c: Remove and add needed includes

2024-05-07T14:00:55Z

Remove from this driver directory and when needed add missing include files directly. Reviewed-by: Heiko Schocher Signed-off-by: Tom Rini

i2c: rockchip: De-initialize the bus after start bit failure

2023-06-05T04:52:46Z

Failure can happen when i2c is used without initializing pinctrl properly, which U-Boot happily allows in SPL. Without this fix, further I2C access would fail, even after proper pinctrl initialization. Signed-off-by: Ondrej Jirman Cc: Heiko Schocher Reviewed-by: Kever Yang Reviewed-by: Heiko Schocher

dm: Rename U_BOOT_DRIVER_ALIAS to DM_DRIVER_ALIAS

2021-01-05T19:26:35Z

We use the U_BOOT_ prefix (i.e. U_BOOT_DRIVER) to declare a driver but in every other case we just use DM_. Update the alias macros to use the DM_ prefix. We could perhaps rename U_BOOT_DRIVER() to DM_DRIVER(), but this macro is widely used and there is at least some benefit to indicating it us a U-Boot driver, particularly for code ported from Linux. So for now, let's keep that name. Signed-off-by: Simon Glass

dm: treewide: Rename ofdata_to_platdata() to of_to_plat()

2020-12-13T23:51:09Z

This name is far too long. Rename it to remove the 'data' bits. This makes it consistent with the platdata->plat rename. Signed-off-by: Simon Glass

dm: treewide: Rename auto_alloc_size members to be shorter

2020-12-13T15:00:25Z

This construct is quite long-winded. In earlier days it made some sense since auto-allocation was a strange concept. But with driver model now used pretty universally, we can shorten this to 'auto'. This reduces verbosity and makes it easier to read. Coincidentally it also ensures that every declaration is on one line, thus making dtoc's job easier. Signed-off-by: Simon Glass