Re: [PATCH] usb: typec: ucsi_ccg: Add retry mechanism in ccg_read

[Wayne Chang <waynec@nvidia.com>]

On 9/11/26 19:09, Heikki Krogerus wrote:
> External email: Use caution opening links or attachments
>
>
> On Wed, Sep 09, 2026 at 11:28:05PM +0800, Wayne Chang wrote:
>> 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.
>>
>> Fixes: 247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Wayne Chang <waynec@nvidia.com>
>> ---
>>   drivers/usb/typec/ucsi/ucsi_ccg.c | 15 ++++++++++++++-
>>   1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/typec/ucsi/ucsi_ccg.c b/drivers/usb/typec/ucsi/ucsi_ccg.c
>> index 91c2958a708c..da8beb14d84c 100644
>> --- a/drivers/usb/typec/ucsi/ucsi_ccg.c
>> +++ b/drivers/usb/typec/ucsi/ucsi_ccg.c
>> @@ -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++) {
> 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.
>
>> +                     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.
>
>> +             }
>> +
>>                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
>