usb: typec: ucsi_ccg: Add retry mechanism in ccg_read
Subsystems (3)
PATCH 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.
Retry the read operation up to three times (four attempts total) when -EREMOTEIO is returned, with a 1-2 ms sleep between retries. 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.
--- drivers/usb/typec/ucsi/ucsi_ccg.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
drivers/usb/typec/ucsi/ucsi_ccg.c +14 -1
@@ -135,6 +135,8 @@ struct version_format {#define NVIDIA_FTB_DP_OFFSET (2)#define NVIDIA_FTB_DBG_OFFSET (3) +#define CCG_READ_MAX_RETRIES 3 +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 ) +break ; + +usleep_range (1000 ,2000 ); + } +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
Heikki Krogerus
maintainerQuoted 46 lines · Wayne Chang
CCG4 firmware occasionally fails to respond to I2C read requests, especially when reading cci and
intr_regfrom 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.
Retry the read operation up to three times (four attempts total) when
-EREMOTEIOis returned, with a 1-2 ms sleep between retries. 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 fromccg_irq_handler(), which runs in threaded IRQ context (registered viarequest_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")Signed-off-by Wayne Chang <[email protected]>--- drivers/usb/typec/ucsi/ucsi_ccg.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)drivers/usb/typec/ucsi/ucsi_ccg.c +3
-0@@ -135,6 +135,8 @@ struct version_format {#define NVIDIA_FTB_DP_OFFSET (2)#define NVIDIA_FTB_DBG_OFFSET (3) +#define CCG_READ_MAX_RETRIES 3 +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)
drivers/usb/typec/ucsi/ucsi_ccg.c static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len) · line 273
Quoted 6 lines
drivers/usb/typec/ucsi/ucsi_ccg.c +2
-1@@ -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++) {
That will run up to 4 times even though your maximum is defined 3?
drivers/usb/typec/ucsi/ucsi_ccg.c static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len) · line 280
Quoted 7 lines
drivers/usb/typec/ucsi/ucsi_ccg.c +7
-0+ 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 ) +break ; + +usleep_range (1000 ,2000 );
fsleep(1000);
Quoted 9 lines
drivers/usb/typec/ucsi/ucsi_ccg.c +2
-0+ } +if (status <0 ) {dev_err (uc->dev,"i2c_transfer failed %d\n" , status);pm_runtime_put_sync (uc->dev);base-commit:
940de590b839f71d6dc846160534bf202401b8b7Signature · 1 line
2.25.1
Signature · 1 line
heikki
Quoted 47 lines · Heikki Krogerus
External email: Use caution opening links or attachments
Quoted 43 lines · Wayne Chang
CCG4 firmware occasionally fails to respond to I2C read requests, especially when reading cci and
intr_regfrom 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.
Retry the read operation up to three times (four attempts total) when
-EREMOTEIOis returned, with a 1-2 ms sleep between retries. 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 fromccg_irq_handler(), which runs in threaded IRQ context (registered viarequest_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")Signed-off-by Wayne Chang <[email protected]>--- drivers/usb/typec/ucsi/ucsi_ccg.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)drivers/usb/typec/ucsi/ucsi_ccg.c +3
-0@@ -135,6 +135,8 @@ struct version_format {#define NVIDIA_FTB_DP_OFFSET (2)#define NVIDIA_FTB_DBG_OFFSET (3) +#define CCG_READ_MAX_RETRIES 3 +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;
Quoted 10 lines
drivers/usb/typec/ucsi/ucsi_ccg.c static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len) · line 273
Quoted 9 lines
drivers/usb/typec/ucsi/ucsi_ccg.c +2
-1/* 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++) {That will run up to 4 times even though your maximum is defined 3?
Thanks for the review. Yes, this could still be somewhat ambiguous here, even though I clarified it in the commit message.
I’m going to change CCG_READ_MAX_RETRIES to 4 to make it consistent.
Quoted 9 lines
drivers/usb/typec/ucsi/ucsi_ccg.c static int ccg_read(struct ucsi_ccg *uc, u16 rab, u8 *data, u32 len) · line 280
Quoted 8 lines
drivers/usb/typec/ucsi/ucsi_ccg.c +7
-0+ 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 ) +break ; + +usleep_range (1000 ,2000 );fsleep(1000);
Thanks for the review. I'll apply it in the v2 patch.
Quoted 13 lines
Quoted 10 lines
drivers/usb/typec/ucsi/ucsi_ccg.c +2
-0+ } +if (status <0 ) {dev_err (uc->dev,"i2c_transfer failed %d\n" , status);pm_runtime_put_sync (uc->dev);base-commit:
940de590b839f71d6dc846160534bf202401b8b7--
2.25.1--
heikki