Messages in this thread | | | From | Andy Shevchenko <> | Date | Sun, 13 Sep 2020 11:43:41 +0300 | Subject | Re: [PATCH v4 1/1] Input: atmel_mxt_ts - implement I2C retries |
| |
On Sat, Sep 12, 2020 at 3:55 AM Jiada Wang <jiada_wang@mentor.com> wrote: > > From: Nick Dyer <nick.dyer@itdev.co.uk> > > Some maXTouch chips (eg mXT1386) will not respond on the first I2C request > when they are in a sleep state. It must be retried after a delay for the > chip to wake up. > > Signed-off-by: Nick Dyer <nick.dyer@itdev.co.uk> > [gdavis: Forward port and fix conflicts.] > Signed-off-by: George G. Davis <george_davis@mentor.com> > [jiada: return exact errno when i2c_transfer & i2c_master_send fails > rename "retry" to "retried" and keep its order in length > set "ret" to correct errno before calling dev_err()
> remove reduntant conditional]
redundant
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com> > Reviewed-by: Dmitry Osipenko <digetx@gmail.com> > Tested-by: Dmitry Osipenko <digetx@gmail.com>
...
> + bool retried = false;
I thought Dmitry wants that to be retry
> u8 buf[2]; > int ret;
> - ret = i2c_transfer(client->adapter, xfer, 2); > - if (ret == 2) { > - ret = 0; > - } else { > - if (ret >= 0) > - ret = -EIO; > +retry_read:
> + ret = i2c_transfer(client->adapter, xfer, ARRAY_SIZE(xfer)); > + if (ret != ARRAY_SIZE(xfer)) {
I'm wondering why you can't leave 2 as is and change it to ARRAY_SIZE in a separate patch? Also why switch from positive to negative conditional?
> + if (!retried) { > + dev_dbg(&client->dev, "i2c retry\n"); > + msleep(MXT_WAKEUP_TIME); > + retried = true; > + goto retry_read; > + } > + ret = ret < 0 ? ret : -EIO; > dev_err(&client->dev, "%s: i2c transfer failed (%d)\n", > __func__, ret); > + return ret; > } > > - return ret; > + return 0; > }
..
> + bool retried = false;
Same comments here, in this function.
> +retry_write: > ret = i2c_master_send(client, buf, count); > - if (ret == count) { > - ret = 0; > - } else { > - if (ret >= 0) > - ret = -EIO; > + if (ret != count) { > + if (!retried) { > + dev_dbg(&client->dev, "i2c retry\n"); > + msleep(MXT_WAKEUP_TIME); > + retried = true; > + goto retry_write; > + } > + ret = ret < 0 ? ret : -EIO; > dev_err(&client->dev, "%s: i2c send failed (%d)\n", > __func__, ret); > + } else { > + ret = 0; > }
-- With Best Regards, Andy Shevchenko
| |