Re: [PATCH v2 4/6] i2c: pasemi: Improve error recovery

Andi Shyti
2025-04-17T13:08:01Z

Hi Sven, Hector,

...

> +/*
> + * The hardware (supposedly) has a 25ms timeout for clock stretching, thus
> + * use 100ms here which should be plenty.
> + */
> +#define TRANSFER_TIMEOUT_MS 100

Please use the PASEMI prefix here. TRANSFER_TIMEOUT_MS it's not a
naming belonging to this driver.

100ms looks a bit too much to me, but if you say it works, then
it works.

> +

...

> unsigned int status;
>
> if (smbus->use_irq) {
> reinit_completion(&smbus->irq_completion);
> - reg_write(smbus, REG_IMASK, SMSTA_XEN | SMSTA_MTN);
> - ret = wait_for_completion_timeout(&smbus->irq_completion, msecs_to_jiffies(100));
> + /* XEN should be set when a transaction terminates, whether due to error or not */
> + reg_write(smbus, REG_IMASK, SMSTA_XEN);
> + ret = wait_for_completion_timeout(&smbus->irq_completion,
> + msecs_to_jiffies(timeout));
> reg_write(smbus, REG_IMASK, 0);
> status = reg_read(smbus, REG_SMSTA);
>
> @@ -123,9 +150,35 @@ static int pasemi_smb_waitready(struct pasemi_smbus *smbus)
> }
> }
>
> + /* Controller timeout? */
> + if (status & SMSTA_TOM) {
> + dev_warn(smbus->dev, "Controller timeout, status 0x%08x\n", status);
> + return -EIO;

as before, these warnings are treated as errors. Can we just
print error?

The rest looks good.

Andi

> + }
> +
> + /* Peripheral timeout? */
> + if (status & SMSTA_MTO) {
> + dev_warn(smbus->dev, "Peripheral timeout, status 0x%08x\n", status);
> + return -ETIME;
> + }

...