On Mon, Apr 13, 2020 at 03:32:24PM +0200, Michał Mirosław wrote:
> Apply some DRY-ing to elants_i2c_execute_command() callers. This pulls
> polling and error printk()s into a single function.
>
> Signed-off-by: Michał Mirosław <mirq-linux@xxxxxxxxxxxx>
> ---
> drivers/input/touchscreen/elants_i2c.c | 189 +++++++++++++------------
> 1 file changed, 96 insertions(+), 93 deletions(-)
>
> diff --git a/drivers/input/touchscreen/elants_i2c.c b/drivers/input/touchscreen/elants_i2c.c
> index 87d686ce08f2..b0f083f7f2a9 100644
> --- a/drivers/input/touchscreen/elants_i2c.c
> +++ b/drivers/input/touchscreen/elants_i2c.c
> @@ -205,7 +205,8 @@ static int elants_i2c_read(struct i2c_client *client, void *data, size_t size)
>
> static int elants_i2c_execute_command(struct i2c_client *client,
> const u8 *cmd, size_t cmd_size,
> - u8 *resp, size_t resp_size)
> + u8 *resp, size_t resp_size,
> + int retries, const char *cmd_name)
> {
> struct i2c_msg msgs[2];
> int ret;
> @@ -225,30 +226,55 @@ static int elants_i2c_execute_command(struct i2c_client *client,
> break;
>
> default:
> - dev_err(&client->dev, "%s: invalid command %*ph\n",
> - __func__, (int)cmd_size, cmd);
> + dev_err(&client->dev, "(%s): invalid command: %*ph\n",
> + cmd_name, (int)cmd_size, cmd);
> return -EINVAL;
> }
>
> - msgs[0].addr = client->addr;
> - msgs[0].flags = client->flags & I2C_M_TEN;
> - msgs[0].len = cmd_size;
> - msgs[0].buf = (u8 *)cmd;
> + for (;;) {
> + msgs[0].addr = client->addr;
> + msgs[0].flags = client->flags & I2C_M_TEN;
> + msgs[0].len = cmd_size;
> + msgs[0].buf = (u8 *)cmd;
>
> - msgs[1].addr = client->addr;
> - msgs[1].flags = client->flags & I2C_M_TEN;
> - msgs[1].flags |= I2C_M_RD;
> - msgs[1].len = resp_size;
> - msgs[1].buf = resp;
> + msgs[1].addr = client->addr;
> + msgs[1].flags = client->flags & I2C_M_TEN;
> + msgs[1].flags |= I2C_M_RD;
> + msgs[1].len = resp_size;
> + msgs[1].buf = resp;
>
> - ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> - if (ret < 0)
> - return ret;
> + ret = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs));
> + if (ret < 0) {
> + if (--retries > 0) {
> + dev_dbg(&client->dev,
> + "(%s) I2C transfer failed: %d (retrying)\n",
> + cmd_name, ret);
> + continue;
> + }
>
> - if (ret != ARRAY_SIZE(msgs) || resp[FW_HDR_TYPE] != expected_response)
> - return -EIO;
> + dev_err(&client->dev,
> + "(%s) I2C transfer failed: %d\n",
> + cmd_name, ret);
> + return ret;
> + }
>
> - return 0;
> + if (ret != ARRAY_SIZE(msgs) ||
> + resp[FW_HDR_TYPE] != expected_response) {
> + if (--retries > 0) {
> + dev_dbg(&client->dev,
> + "(%s) unexpected response: %*ph (retrying)\n",
> + cmd_name, ret, resp);
> + continue;
> + }
> +
> + dev_err(&client->dev,
> + "(%s) unexpected response: %*ph\n",
> + cmd_name, ret, resp);
> + return -EIO;
> + }
> +
> + return --retries;
I'd prefer if we returned 0 on success and I'd be OK when flashing
firmware to have separate retry counters for the command themselves and
for checking the response.
Thanks.
--
Dmitry
