linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] Input: atmel_mxt_ts - refactor i2c error handling
@ 2013-02-15 14:55 Daniel Kurtz
  2013-02-20 20:37 ` Henrik Rydberg
  0 siblings, 1 reply; 2+ messages in thread
From: Daniel Kurtz @ 2013-02-15 14:55 UTC (permalink / raw)
  To: Dmitry Torokhov, Henrik Rydberg, Bill Pemberton, Nick Dyer
  Cc: Benson Leung, linux-input, linux-kernel, Yufeng Shen,
	Daniel Kurtz

A recent patch refactored i2c error handling in the register read/write
path.  This adds similar handling to the other i2c paths used in fw_update
and bootloader state detection.

The generic i2c layer can return values indicating a partial transaction.
>From the atmel_mxt driver's perspective, this is an IO error, so we use
some helper functions to convert these partial transfers to -EIO in a
uniform way.  Other error codes might still be useful, though, so we pass
them up unmodified.  Note that in the partial transfer case, we log the
number of bytes partially transferred, and not -EIO.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
Changes from V1:
- simplified implied logic (thanks Henrik!)
- error print the partial number of bytes received, not -EIO

 drivers/input/touchscreen/atmel_mxt_ts.c | 83 +++++++++++++++++---------------
 1 file changed, 45 insertions(+), 38 deletions(-)

diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index d04f810..3d56d5f 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -324,16 +324,53 @@ static void mxt_dump_message(struct device *dev,
 		message->reportid, 7, message->message);
 }
 
+static int mxt_i2c_recv(struct i2c_client *client, u8 *buf, size_t count)
+{
+	int ret;
+
+	ret = i2c_master_recv(client, buf, count);
+	if (ret == count)
+		return 0;
+
+	dev_err(&client->dev, "i2c recv failed (%d)\n", ret);
+	return (ret < 0) ? ret : -EIO;
+}
+
+static int mxt_i2c_send(struct i2c_client *client, const u8 *buf, size_t count)
+{
+	int ret;
+
+	ret = i2c_master_send(client, buf, count);
+	if (ret == count)
+		return 0;
+
+	dev_err(&client->dev, "i2c send failed (%d)\n", ret);
+	return (ret < 0) ? ret : -EIO;
+}
+
+static int mxt_i2c_transfer(struct i2c_client *client, struct i2c_msg *msgs,
+		size_t count)
+{
+	int ret;
+
+	ret = i2c_transfer(client->adapter, msgs, count);
+	if (ret == count)
+		return 0;
+
+	dev_err(&client->dev, "i2c transfer failed (%d)\n", ret);
+	return (ret < 0) ? ret : -EIO;
+}
+
 static int mxt_check_bootloader(struct i2c_client *client,
 				     unsigned int state)
 {
 	u8 val;
+	int ret;
 
 recheck:
-	if (i2c_master_recv(client, &val, 1) != 1) {
-		dev_err(&client->dev, "%s: i2c recv failed\n", __func__);
-		return -EIO;
-	}
+	ret = mxt_i2c_recv(client, &val, 1);
+	if (ret)
+		return ret;
 
 	switch (state) {
 	case MXT_WAITING_BOOTLOAD_CMD:
@@ -363,23 +400,13 @@ static int mxt_unlock_bootloader(struct i2c_client *client)
 	buf[0] = MXT_UNLOCK_CMD_LSB;
 	buf[1] = MXT_UNLOCK_CMD_MSB;
 
-	if (i2c_master_send(client, buf, 2) != 2) {
-		dev_err(&client->dev, "%s: i2c send failed\n", __func__);
-		return -EIO;
-	}
-
-	return 0;
+	return mxt_i2c_send(client, buf, 2);
 }
 
 static int mxt_fw_write(struct i2c_client *client,
 			     const u8 *data, unsigned int frame_size)
 {
-	if (i2c_master_send(client, data, frame_size) != frame_size) {
-		dev_err(&client->dev, "%s: i2c send failed\n", __func__);
-		return -EIO;
-	}
-
-	return 0;
+	return mxt_i2c_send(client, data, frame_size);
 }
 
 static int __mxt_read_reg(struct i2c_client *client,
@@ -387,7 +414,6 @@ static int __mxt_read_reg(struct i2c_client *client,
 {
 	struct i2c_msg xfer[2];
 	u8 buf[2];
-	int ret;
 
 	buf[0] = reg & 0xff;
 	buf[1] = (reg >> 8) & 0xff;
@@ -404,17 +430,7 @@ static int __mxt_read_reg(struct i2c_client *client,
 	xfer[1].len = len;
 	xfer[1].buf = val;
 
-	ret = i2c_transfer(client->adapter, xfer, 2);
-	if (ret == 2) {
-		ret = 0;
-	} else {
-		if (ret >= 0)
-			ret = -EIO;
-		dev_err(&client->dev, "%s: i2c transfer failed (%d)\n",
-			__func__, ret);
-	}
-
-	return ret;
+	return mxt_i2c_transfer(client, xfer, 2);
 }
 
 static int mxt_read_reg(struct i2c_client *client, u16 reg, u8 *val)
@@ -438,16 +454,7 @@ static int __mxt_write_reg(struct i2c_client *client, u16 reg, u16 len,
 	buf[1] = (reg >> 8) & 0xff;
 	memcpy(&buf[2], val, len);
 
-	ret = i2c_master_send(client, buf, count);
-	if (ret == count) {
-		ret = 0;
-	} else {
-		if (ret >= 0)
-			ret = -EIO;
-		dev_err(&client->dev, "%s: i2c send failed (%d)\n",
-			__func__, ret);
-	}
-
+	ret = mxt_i2c_send(client, buf, count);
 	kfree(buf);
 	return ret;
 }
-- 
1.8.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] Input: atmel_mxt_ts - refactor i2c error handling
  2013-02-15 14:55 [PATCH v2] Input: atmel_mxt_ts - refactor i2c error handling Daniel Kurtz
@ 2013-02-20 20:37 ` Henrik Rydberg
  0 siblings, 0 replies; 2+ messages in thread
From: Henrik Rydberg @ 2013-02-20 20:37 UTC (permalink / raw)
  To: Daniel Kurtz
  Cc: Dmitry Torokhov, Bill Pemberton, Nick Dyer, Benson Leung,
	linux-input, linux-kernel, Yufeng Shen

Hi Daniel,

> A recent patch refactored i2c error handling in the register read/write
> path.  This adds similar handling to the other i2c paths used in fw_update
> and bootloader state detection.
> 
> The generic i2c layer can return values indicating a partial transaction.
> From the atmel_mxt driver's perspective, this is an IO error, so we use
> some helper functions to convert these partial transfers to -EIO in a
> uniform way.  Other error codes might still be useful, though, so we pass
> them up unmodified.  Note that in the partial transfer case, we log the
> number of bytes partially transferred, and not -EIO.
> 
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
> Changes from V1:
> - simplified implied logic (thanks Henrik!)
> - error print the partial number of bytes received, not -EIO
> 
>  drivers/input/touchscreen/atmel_mxt_ts.c | 83 +++++++++++++++++---------------
>  1 file changed, 45 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index d04f810..3d56d5f 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -324,16 +324,53 @@ static void mxt_dump_message(struct device *dev,
>  		message->reportid, 7, message->message);
>  }
>  
> +static int mxt_i2c_recv(struct i2c_client *client, u8 *buf, size_t count)
> +{
> +	int ret;
> +
> +	ret = i2c_master_recv(client, buf, count);
> +	if (ret == count)
> +		return 0;
> +
> +	dev_err(&client->dev, "i2c recv failed (%d)\n", ret);
> +	return (ret < 0) ? ret : -EIO;
> +}
> +
> +static int mxt_i2c_send(struct i2c_client *client, const u8 *buf, size_t count)
> +{
> +	int ret;
> +
> +	ret = i2c_master_send(client, buf, count);
> +	if (ret == count)
> +		return 0;
> +
> +	dev_err(&client->dev, "i2c send failed (%d)\n", ret);
> +	return (ret < 0) ? ret : -EIO;
> +}
> +
> +static int mxt_i2c_transfer(struct i2c_client *client, struct i2c_msg *msgs,
> +		size_t count)
> +{
> +	int ret;
> +
> +	ret = i2c_transfer(client->adapter, msgs, count);
> +	if (ret == count)
> +		return 0;
> +
> +	dev_err(&client->dev, "i2c transfer failed (%d)\n", ret);
> +	return (ret < 0) ? ret : -EIO;
> +}
> +
>  static int mxt_check_bootloader(struct i2c_client *client,
>  				     unsigned int state)
>  {
>  	u8 val;
> +	int ret;
>  
>  recheck:
> -	if (i2c_master_recv(client, &val, 1) != 1) {
> -		dev_err(&client->dev, "%s: i2c recv failed\n", __func__);
> -		return -EIO;
> -	}
> +	ret = mxt_i2c_recv(client, &val, 1);
> +	if (ret)
> +		return ret;
>  
>  	switch (state) {
>  	case MXT_WAITING_BOOTLOAD_CMD:
> @@ -363,23 +400,13 @@ static int mxt_unlock_bootloader(struct i2c_client *client)
>  	buf[0] = MXT_UNLOCK_CMD_LSB;
>  	buf[1] = MXT_UNLOCK_CMD_MSB;
>  
> -	if (i2c_master_send(client, buf, 2) != 2) {
> -		dev_err(&client->dev, "%s: i2c send failed\n", __func__);
> -		return -EIO;
> -	}
> -
> -	return 0;
> +	return mxt_i2c_send(client, buf, 2);
>  }
>  
>  static int mxt_fw_write(struct i2c_client *client,
>  			     const u8 *data, unsigned int frame_size)
>  {
> -	if (i2c_master_send(client, data, frame_size) != frame_size) {
> -		dev_err(&client->dev, "%s: i2c send failed\n", __func__);
> -		return -EIO;
> -	}
> -
> -	return 0;
> +	return mxt_i2c_send(client, data, frame_size);
>  }
>  
>  static int __mxt_read_reg(struct i2c_client *client,
> @@ -387,7 +414,6 @@ static int __mxt_read_reg(struct i2c_client *client,
>  {
>  	struct i2c_msg xfer[2];
>  	u8 buf[2];
> -	int ret;
>  
>  	buf[0] = reg & 0xff;
>  	buf[1] = (reg >> 8) & 0xff;
> @@ -404,17 +430,7 @@ static int __mxt_read_reg(struct i2c_client *client,
>  	xfer[1].len = len;
>  	xfer[1].buf = val;
>  
> -	ret = i2c_transfer(client->adapter, xfer, 2);
> -	if (ret == 2) {
> -		ret = 0;
> -	} else {
> -		if (ret >= 0)
> -			ret = -EIO;
> -		dev_err(&client->dev, "%s: i2c transfer failed (%d)\n",
> -			__func__, ret);
> -	}
> -
> -	return ret;
> +	return mxt_i2c_transfer(client, xfer, 2);
>  }
>  
>  static int mxt_read_reg(struct i2c_client *client, u16 reg, u8 *val)
> @@ -438,16 +454,7 @@ static int __mxt_write_reg(struct i2c_client *client, u16 reg, u16 len,
>  	buf[1] = (reg >> 8) & 0xff;
>  	memcpy(&buf[2], val, len);
>  
> -	ret = i2c_master_send(client, buf, count);
> -	if (ret == count) {
> -		ret = 0;
> -	} else {
> -		if (ret >= 0)
> -			ret = -EIO;
> -		dev_err(&client->dev, "%s: i2c send failed (%d)\n",
> -			__func__, ret);
> -	}
> -
> +	ret = mxt_i2c_send(client, buf, count);
>  	kfree(buf);
>  	return ret;
>  }
> -- 
> 1.8.1
> 

    Reviewed-by: Henrik Rydberg <rydberg@euromail.se>

Thanks,
Henrik

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2013-02-20 20:36 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-02-15 14:55 [PATCH v2] Input: atmel_mxt_ts - refactor i2c error handling Daniel Kurtz
2013-02-20 20:37 ` Henrik Rydberg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).