On Sat, Sep 12, 2020 at 3:55 AM Jiada Wang <jiada_wang@xxxxxxxxxx> wrote:
>
> From: Nick Dyer <nick.dyer@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> [gdavis: Forward port and fix conflicts.]
> Signed-off-by: George G. Davis <george_davis@xxxxxxxxxx>
> [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@xxxxxxxxxx>
> Reviewed-by: Dmitry Osipenko <digetx@xxxxxxxxx>
> Tested-by: Dmitry Osipenko <digetx@xxxxxxxxx>
...
> + 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
