usb: typec: ucsi_ccg: Add retry mechanism in ccg_read

PATCH In review Wayne Chang 3 participants 3 messages Sep 16 – Sep 18

Subsystems (4)

Stable branch USB Tegra architecture support USB Type-C class

lore

Share & tools

lore

Wayne Chang

author

2026-09-16 06:19 UTC #0 lore raw

PATCH v2 1/1 usb: typec: ucsi_ccg: Add retry mechanism in ccg_read

CCG4 firmware occasionally fails to respond to I2C read requests, especially when reading cci and intr_reg from the ISR path. In these cases, i2c_transfer() returns -EREMOTEIO.

The CCG4 is EOL, and Infineon no longer supports firmware updates, so a software workaround is necessary.

Attempt the read up to CCG_READ_MAX_RETRIES (4) times in total when -EREMOTEIO is returned, sleeping ~1 ms via fsleep() between attempts. This allows recovery from transient I2C failures without affecting other error paths.

The sleep is safe because ccg_read() is called both from process context and from ccg_irq_handler(), which runs in threaded IRQ context (registered via request_threaded_irq() with a NULL hard-IRQ handler), so sleeping is allowed in both cases.

Fixes 247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")

Cc [email protected]

Signed-off-by Wayne Chang <[email protected]>

---
v1 -> v2: Per Heikki's review:
- Fix off-by-one: CCG_READ_MAX_RETRIES was 3 but the loop ran 4 times (retry_count <= CCG_READ_MAX_RETRIES). Changed the macro to 4 (the actual total attempt count) and the loop condition to retry_count < CCG_READ_MAX_RETRIES, so the name and the loop bound now agree. Adjusted the "skip the sleep on the last attempt" check to retry_count == CCG_READ_MAX_RETRIES - 1 to match.
- Use fsleep(1000) instead of usleep_range(1000, 2000).
- Reworded the commit message to say "up to CCG_READ_MAX_RETRIES (4) times in total" instead of the ambiguous "up to three times (four attempts total)" that prompted the off-by-one question.

 drivers/usb/typec/ucsi/ucsi_ccg.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

drivers/usb/typec/ucsi/ucsi_ccg.c +14 -1

index 91c2958a708c..4b6c53d2a998 100644
@@ -135,6 +135,8 @@ struct version_format {
 #define NVIDIA_FTB_DP_OFFSET	(2)
 #define NVIDIA_FTB_DBG_OFFSET	(3)
 
+#define CCG_READ_MAX_RETRIES	4
+
 struct version_info {
 	struct version_format base;
 	struct version_format app;

@@ -255,6 +257,7 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
 	};
 	u32 rlen, rem_len = len, max_read_len = len;
 	int status;
+	int retry_count;
 
 	/* check any max_read_len limitation on i2c adapter */
 	if (quirks && quirks->max_read_len)

@@ -266,7 +269,17 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
 		rlen = min_t(u16, rem_len, max_read_len);
 		msgs[1].len = rlen;
 		put_unaligned_le16(rab, buf);
-		status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+
+		for (retry_count = 0; retry_count < CCG_READ_MAX_RETRIES; retry_count++) {
+			status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+
+			/* Only retry on -EREMOTEIO, and not after the last attempt */
+			if (status != -EREMOTEIO || retry_count == CCG_READ_MAX_RETRIES - 1)
+				break;
+
+			fsleep(1000);
+		}
+
 		if (status < 0) {
 			dev_err(uc->dev, "i2c_transfer failed %d\n", status);
 			pm_runtime_put_sync(uc->dev);

base-commit: 940de590b839f71d6dc846160534bf202401b8b7

Signature · 1 line

2.25.1

Jon Hunter

maintainer Reviewed-by Tested-by on v2 1/1

2026-09-16 15:00 UTC #1 lore raw

Quoted 66 lines · Wayne Chang

CCG4 firmware occasionally fails to respond to I2C read requests, especially when reading cci and intr_reg from the ISR path. In these cases, i2c_transfer() returns -EREMOTEIO.
The CCG4 is EOL, and Infineon no longer supports firmware updates, so a software workaround is necessary.
Attempt the read up to CCG_READ_MAX_RETRIES (4) times in total when -EREMOTEIO is returned, sleeping ~1 ms via fsleep() between attempts. This allows recovery from transient I2C failures without affecting other error paths.
The sleep is safe because ccg_read() is called both from process context and from ccg_irq_handler(), which runs in threaded IRQ context (registered via request_threaded_irq() with a NULL hard-IRQ handler), so sleeping is allowed in both cases.
Fixes 247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")
Cc [email protected]
Signed-off-by Wayne Chang <[email protected]>
---
v1 -> v2: Per Heikki's review:
- Fix off-by-one: CCG_READ_MAX_RETRIES was 3 but the loop ran 4 times (retry_count <= CCG_READ_MAX_RETRIES). Changed the macro to 4 (the actual total attempt count) and the loop condition to retry_count < CCG_READ_MAX_RETRIES, so the name and the loop bound now agree. Adjusted the "skip the sleep on the last attempt" check to retry_count == CCG_READ_MAX_RETRIES - 1 to match.
- Use fsleep(1000) instead of usleep_range(1000, 2000).
- Reworded the commit message to say "up to CCG_READ_MAX_RETRIES (4) times in total" instead of the ambiguous "up to three times (four attempts total)" that prompted the off-by-one question.
  drivers/usb/typec/ucsi/ucsi_ccg.c | 15 ++++++++++++++-
  1 file changed, 14 insertions(+), 1 deletion(-)
drivers/usb/typec/ucsi/ucsi_ccg.c +7 -1
index 91c2958a708c..4b6c53d2a998 100644
@@ -135,6 +135,8 @@ struct version_format {
  #define NVIDIA_FTB_DP_OFFSET	(2)
  #define NVIDIA_FTB_DBG_OFFSET	(3)
 
+#define CCG_READ_MAX_RETRIES	4
+
  struct version_info {
  	struct version_format base;
  	struct version_format app;
@@ -255,6 +257,7 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
  	};
  	u32 rlen, rem_len = len, max_read_len = len;
  	int status;
+	int retry_count;
 
  	/* check any max_read_len limitation on i2c adapter */
  	if (quirks && quirks->max_read_len)
@@ -266,7 +269,17 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
  		rlen = min_t(u16, rem_len, max_read_len);
  		msgs[1].len = rlen;
  		put_unaligned_le16(rab, buf);
-		status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+
+		for (retry_count = 0; retry_count < CCG_READ_MAX_RETRIES; retry_count++) {
+			status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+

drivers/usb/typec/ucsi/ucsi_ccg.c static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len) · line 285

Quoted 10 lines

drivers/usb/typec/ucsi/ucsi_ccg.c +7 -0

+			/* Only retry on -EREMOTEIO, and not after the last attempt */
+			if (status != -EREMOTEIO || retry_count == CCG_READ_MAX_RETRIES - 1)
+				break;
+
+			fsleep(1000);
+		}
+
  		if (status < 0) {
  			dev_err(uc->dev, "i2c_transfer failed %d\n", status);
  			pm_runtime_put_sync(uc->dev);

I was wondering if we could avoid the extra check inside the loop for 'CCG_READ_MAX_RETRIES - 1' but this does avoid an addition sleep. So ...

Reviewed-by Jon Hunter <[email protected]>

Tested-by Jon Hunter <[email protected]>

Thanks!
Jon

Signature · 1 line

nvpublic

Heikki Krogerus

maintainer Reviewed-by on v2 1/1

2026-09-18 09:45 UTC #2 lore raw

In reply to Wayne Chang

Quoted 12 lines · Wayne Chang

CCG4 firmware occasionally fails to respond to I2C read requests, especially when reading cci and intr_reg from the ISR path. In these cases, i2c_transfer() returns -EREMOTEIO.
The CCG4 is EOL, and Infineon no longer supports firmware updates, so a software workaround is necessary.
Attempt the read up to CCG_READ_MAX_RETRIES (4) times in total when -EREMOTEIO is returned, sleeping ~1 ms via fsleep() between attempts. This allows recovery from transient I2C failures without affecting other error paths.

Quoted 8 lines

The sleep is safe because ccg_read() is called both from process context and from ccg_irq_handler(), which runs in threaded IRQ context (registered via request_threaded_irq() with a NULL hard-IRQ handler), so sleeping is allowed in both cases.
Fixes 247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")
Cc [email protected]
Signed-off-by Wayne Chang <[email protected]>

Reviewed-by Heikki Krogerus <[email protected]>

Quoted 60 lines

---
v1 -> v2: Per Heikki's review:
- Fix off-by-one: CCG_READ_MAX_RETRIES was 3 but the loop ran 4 times (retry_count <= CCG_READ_MAX_RETRIES). Changed the macro to 4 (the actual total attempt count) and the loop condition to retry_count < CCG_READ_MAX_RETRIES, so the name and the loop bound now agree. Adjusted the "skip the sleep on the last attempt" check to retry_count == CCG_READ_MAX_RETRIES - 1 to match.
- Use fsleep(1000) instead of usleep_range(1000, 2000).
- Reworded the commit message to say "up to CCG_READ_MAX_RETRIES (4) times in total" instead of the ambiguous "up to three times (four attempts total)" that prompted the off-by-one question.
 drivers/usb/typec/ucsi/ucsi_ccg.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)
drivers/usb/typec/ucsi/ucsi_ccg.c +14 -1
index 91c2958a708c..4b6c53d2a998 100644
@@ -135,6 +135,8 @@ struct version_format {
 #define NVIDIA_FTB_DP_OFFSET	(2)
 #define NVIDIA_FTB_DBG_OFFSET	(3)
 
+#define CCG_READ_MAX_RETRIES	4
+
 struct version_info {
 	struct version_format base;
 	struct version_format app;
@@ -255,6 +257,7 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
 	};
 	u32 rlen, rem_len = len, max_read_len = len;
 	int status;
+	int retry_count;
 
 	/* check any max_read_len limitation on i2c adapter */
 	if (quirks && quirks->max_read_len)
@@ -266,7 +269,17 @@ static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len)
 		rlen = min_t(u16, rem_len, max_read_len);
 		msgs[1].len = rlen;
 		put_unaligned_le16(rab, buf);
-		status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+
+		for (retry_count = 0; retry_count < CCG_READ_MAX_RETRIES; retry_count++) {
+			status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
+
+			/* Only retry on -EREMOTEIO, and not after the last attempt */
+			if (status != -EREMOTEIO || retry_count == CCG_READ_MAX_RETRIES - 1)
+				break;
+
+			fsleep(1000);
+		}
+
 		if (status < 0) {
 			dev_err(uc->dev, "i2c_transfer failed %d\n", status);
 			pm_runtime_put_sync(uc->dev);
base-commit: 940de590b839f71d6dc846160534bf202401b8b7
Signature · 1 line
2.25.1

Signature · 1 line

heikki