All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Hans de Goede <hdegoede@redhat.com>,
	Bastien Nocera <hadess@hadess.net>,
	linux-input@vger.kernel.org
Subject: [PATCH v2 4/6] Input: goodix - Push error logging up into i2c_read and i2c_write helpers
Date: Mon, 20 Sep 2021 17:06:41 +0200	[thread overview]
Message-ID: <20210920150643.155872-5-hdegoede@redhat.com> (raw)
In-Reply-To: <20210920150643.155872-1-hdegoede@redhat.com>

Make the goodix_i2c_read() and goodix_i2c_write*() helpers log errors
themselves. This allows removing all the error logging from their callers.

This already results in a nice cleanup with the current code and it also
helps to make the upcoming support for controllers without flash cleaner.

Reviewed-by: Bastien Nocera <hadess@hadess.net>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/input/touchscreen/goodix.c | 68 ++++++++++++------------------
 1 file changed, 28 insertions(+), 40 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 7ab4a19fb2bf..2205ebb9325e 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -172,7 +172,13 @@ int goodix_i2c_read(struct i2c_client *client, u16 reg, u8 *buf, int len)
 	msgs[1].buf   = buf;
 
 	ret = i2c_transfer(client->adapter, msgs, 2);
-	return ret < 0 ? ret : (ret != ARRAY_SIZE(msgs) ? -EIO : 0);
+	if (ret >= 0)
+		ret = (ret == ARRAY_SIZE(msgs) ? 0 : -EIO);
+
+	if (ret)
+		dev_err(&client->dev, "Error reading %d bytes from 0x%04x: %d\n",
+			len, reg, ret);
+	return ret;
 }
 
 /**
@@ -203,8 +209,15 @@ int goodix_i2c_write(struct i2c_client *client, u16 reg, const u8 *buf, int len)
 	msg.len = len + 2;
 
 	ret = i2c_transfer(client->adapter, &msg, 1);
+	if (ret >= 0)
+		ret = (ret == 1 ? 0 : -EIO);
+
 	kfree(addr_buf);
-	return ret < 0 ? ret : (ret != 1 ? -EIO : 0);
+
+	if (ret)
+		dev_err(&client->dev, "Error writing %d bytes to 0x%04x: %d\n",
+			len, reg, ret);
+	return ret;
 }
 
 int goodix_i2c_write_u8(struct i2c_client *client, u16 reg, u8 value)
@@ -244,13 +257,9 @@ static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
 	 */
 	max_timeout = jiffies + msecs_to_jiffies(GOODIX_BUFFER_STATUS_TIMEOUT);
 	do {
-		error = goodix_i2c_read(ts->client, addr, data,
-					header_contact_keycode_size);
-		if (error) {
-			dev_err(&ts->client->dev, "I2C transfer error: %d\n",
-					error);
+		error = goodix_i2c_read(ts->client, addr, data, header_contact_keycode_size);
+		if (error)
 			return error;
-		}
 
 		if (data[0] & GOODIX_BUFFER_STATUS_READY) {
 			touch_num = data[0] & 0x0f;
@@ -260,10 +269,8 @@ static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data)
 			if (touch_num > 1) {
 				addr += header_contact_keycode_size;
 				data += header_contact_keycode_size;
-				error = goodix_i2c_read(ts->client,
-						addr, data,
-						ts->contact_size *
-							(touch_num - 1));
+				error = goodix_i2c_read(ts->client, addr, data,
+							ts->contact_size * (touch_num - 1));
 				if (error)
 					return error;
 			}
@@ -373,9 +380,7 @@ static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id)
 	struct goodix_ts_data *ts = dev_id;
 
 	goodix_process_events(ts);
-
-	if (goodix_i2c_write_u8(ts->client, GOODIX_READ_COOR_ADDR, 0) < 0)
-		dev_err(&ts->client->dev, "I2C write end_cmd error\n");
+	goodix_i2c_write_u8(ts->client, GOODIX_READ_COOR_ADDR, 0);
 
 	return IRQ_HANDLED;
 }
@@ -500,11 +505,9 @@ int goodix_send_cfg(struct goodix_ts_data *ts, const u8 *cfg, int len)
 		return error;
 
 	error = goodix_i2c_write(ts->client, ts->chip->config_addr, cfg, len);
-	if (error) {
-		dev_err(&ts->client->dev, "Failed to write config data: %d",
-			error);
+	if (error)
 		return error;
-	}
+
 	dev_dbg(&ts->client->dev, "Config sent successfully.");
 
 	/* Let the firmware reconfigure itself, so sleep for 10ms */
@@ -890,8 +893,6 @@ static void goodix_read_config(struct goodix_ts_data *ts)
 	error = goodix_i2c_read(ts->client, ts->chip->config_addr,
 				ts->config, ts->chip->config_len);
 	if (error) {
-		dev_warn(&ts->client->dev, "Error reading config: %d\n",
-			 error);
 		ts->int_trigger_type = GOODIX_INT_TRIGGER;
 		ts->max_touch_num = GOODIX_MAX_CONTACTS;
 		return;
@@ -922,10 +923,8 @@ static int goodix_read_version(struct goodix_ts_data *ts)
 	char id_str[GOODIX_ID_MAX_LEN + 1];
 
 	error = goodix_i2c_read(ts->client, GOODIX_REG_ID, buf, sizeof(buf));
-	if (error) {
-		dev_err(&ts->client->dev, "read version failed: %d\n", error);
+	if (error)
 		return error;
-	}
 
 	memcpy(id_str, buf, GOODIX_ID_MAX_LEN);
 	id_str[GOODIX_ID_MAX_LEN] = 0;
@@ -951,13 +950,10 @@ static int goodix_i2c_test(struct i2c_client *client)
 	u8 test;
 
 	while (retry++ < 2) {
-		error = goodix_i2c_read(client, GOODIX_REG_ID,
-					&test, 1);
+		error = goodix_i2c_read(client, GOODIX_REG_ID, &test, 1);
 		if (!error)
 			return 0;
 
-		dev_err(&client->dev, "i2c test failed attempt %d: %d\n",
-			retry, error);
 		msleep(20);
 	}
 
@@ -1178,10 +1174,8 @@ static int goodix_ts_probe(struct i2c_client *client,
 	}
 
 	error = goodix_read_version(ts);
-	if (error) {
-		dev_err(&client->dev, "Read version failed.\n");
+	if (error)
 		return error;
-	}
 
 	ts->chip = goodix_get_chip_data(ts->id);
 
@@ -1249,10 +1243,8 @@ static int __maybe_unused goodix_suspend(struct device *dev)
 
 	usleep_range(5000, 6000);
 
-	error = goodix_i2c_write_u8(ts->client, GOODIX_REG_COMMAND,
-				    GOODIX_CMD_SCREEN_OFF);
+	error = goodix_i2c_write_u8(ts->client, GOODIX_REG_COMMAND, GOODIX_CMD_SCREEN_OFF);
 	if (error) {
-		dev_err(&ts->client->dev, "Screen off command failed\n");
 		goodix_irq_direction_input(ts);
 		goodix_request_irq(ts);
 		return -EAGAIN;
@@ -1293,12 +1285,8 @@ static int __maybe_unused goodix_resume(struct device *dev)
 	if (error)
 		return error;
 
-	error = goodix_i2c_read(ts->client, ts->chip->config_addr,
-				&config_ver, 1);
-	if (error)
-		dev_warn(dev, "Error reading config version: %d, resetting controller\n",
-			 error);
-	else if (config_ver != ts->config[0])
+	error = goodix_i2c_read(ts->client, ts->chip->config_addr, &config_ver, 1);
+	if (!error && config_ver != ts->config[0])
 		dev_info(dev, "Config version mismatch %d != %d, resetting controller\n",
 			 config_ver, ts->config[0]);
 
-- 
2.31.1


  parent reply	other threads:[~2021-09-20 15:06 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-20 15:06 [PATCH v2 0/6] Input: goodix - Add support for controllers without flash Hans de Goede
2021-09-20 15:06 ` [PATCH v2 1/6] Input: goodix - Change goodix_i2c_write() len parameter type to int Hans de Goede
2021-09-20 15:06 ` [PATCH v2 2/6] Input: goodix - Add a goodix.h header file Hans de Goede
2021-09-20 15:06 ` [PATCH v2 3/6] Input: goodix - Refactor reset handling Hans de Goede
2021-09-20 15:06 ` Hans de Goede [this message]
2021-09-20 15:06 ` [PATCH v2 5/6] Input: goodix - Allow specifying the config filename through a "goodix,config-name" device-property Hans de Goede
2021-09-20 15:06 ` [PATCH v2 6/6] Input: goodix - Add support for controllers without flash Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210920150643.155872-5-hdegoede@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=hadess@hadess.net \
    --cc=linux-input@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.