All of lore.kernel.org
 help / color / mirror / Atom feed
From: moinejf@free.fr (Jean-Francois Moine)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v4 02/24] drm/i2c: tda998x: check more I/O errors
Date: Sat, 25 Jan 2014 18:14:45 +0100	[thread overview]
Message-ID: <20140125181445.38f8b30b@armhf> (raw)
In-Reply-To: <cover.1390646891.git.moinejf@free.fr>

This patch adds more error checking inn I2C I/O functions.
In case of I/O error, this permits to avoid writing in bad controller
pages, a bad chipset detection or looping when getting the EDID.

Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 57 +++++++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 26dd299..11f0607 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -362,7 +362,7 @@ fail:
 	return 0;
 }
 
-static void
+static int
 set_page(struct tda998x_priv *priv, uint16_t reg)
 {
 	if (REG2PAGE(reg) != priv->current_page) {
@@ -371,11 +371,14 @@ set_page(struct tda998x_priv *priv, uint16_t reg)
 				REG_CURPAGE, REG2PAGE(reg)
 		};
 		int ret = i2c_master_send(client, buf, sizeof(buf));
-		if (ret < 0)
+		if (ret < 0) {
 			dev_err(&client->dev, "Error %d writing to REG_CURPAGE\n", ret);
+			return ret;
+		}
 
 		priv->current_page = REG2PAGE(reg);
 	}
+	return 0;
 }
 
 static int
@@ -385,7 +388,9 @@ reg_read_range(struct tda998x_priv *priv, uint16_t reg, char *buf, int cnt)
 	uint8_t addr = REG2ADDR(reg);
 	int ret;
 
-	set_page(priv, reg);
+	ret = set_page(priv, reg);
+	if (ret < 0)
+		return ret;
 
 	ret = i2c_master_send(client, &addr, sizeof(addr));
 	if (ret < 0)
@@ -412,18 +417,24 @@ reg_write_range(struct tda998x_priv *priv, uint16_t reg, uint8_t *p, int cnt)
 	buf[0] = REG2ADDR(reg);
 	memcpy(&buf[1], p, cnt);
 
-	set_page(priv, reg);
+	ret = set_page(priv, reg);
+	if (ret < 0)
+		return;
 
 	ret = i2c_master_send(client, buf, cnt + 1);
 	if (ret < 0)
 		dev_err(&client->dev, "Error %d writing to 0x%x\n", ret, reg);
 }
 
-static uint8_t
+static int
 reg_read(struct tda998x_priv *priv, uint16_t reg)
 {
 	uint8_t val = 0;
-	reg_read_range(priv, reg, &val, sizeof(val));
+	int ret;
+
+	ret = reg_read_range(priv, reg, &val, sizeof(val));
+	if (ret < 0)
+		return ret;
 	return val;
 }
 
@@ -434,7 +445,9 @@ reg_write(struct tda998x_priv *priv, uint16_t reg, uint8_t val)
 	uint8_t buf[] = {REG2ADDR(reg), val};
 	int ret;
 
-	set_page(priv, reg);
+	ret = set_page(priv, reg);
+	if (ret < 0)
+		return;
 
 	ret = i2c_master_send(client, buf, ARRAY_SIZE(buf));
 	if (ret < 0)
@@ -448,7 +461,9 @@ reg_write16(struct tda998x_priv *priv, uint16_t reg, uint16_t val)
 	uint8_t buf[] = {REG2ADDR(reg), val >> 8, val};
 	int ret;
 
-	set_page(priv, reg);
+	ret = set_page(priv, reg);
+	if (ret < 0)
+		return;
 
 	ret = i2c_master_send(client, buf, ARRAY_SIZE(buf));
 	if (ret < 0)
@@ -458,13 +473,21 @@ reg_write16(struct tda998x_priv *priv, uint16_t reg, uint16_t val)
 static void
 reg_set(struct tda998x_priv *priv, uint16_t reg, uint8_t val)
 {
-	reg_write(priv, reg, reg_read(priv, reg) | val);
+	int old_val;
+
+	old_val = reg_read(priv, reg);
+	if (old_val >= 0)
+		reg_write(priv, reg, old_val | val);
 }
 
 static void
 reg_clear(struct tda998x_priv *priv, uint16_t reg, uint8_t val)
 {
-	reg_write(priv, reg, reg_read(priv, reg) & ~val);
+	int old_val;
+
+	old_val = reg_read(priv, reg);
+	if (old_val >= 0)
+		reg_write(priv, reg, old_val & ~val);
 }
 
 static void
@@ -970,8 +993,10 @@ read_edid_block(struct drm_encoder *encoder, uint8_t *buf, int blk)
 
 	/* wait for block read to complete: */
 	for (i = 100; i > 0; i--) {
-		uint8_t val = reg_read(priv, REG_INT_FLAGS_2);
-		if (val & INT_FLAGS_2_EDID_BLK_RD)
+		ret = reg_read(priv, REG_INT_FLAGS_2);
+		if (ret < 0)
+			return ret;
+		if (ret & INT_FLAGS_2_EDID_BLK_RD)
 			break;
 		msleep(1);
 	}
@@ -1134,6 +1159,7 @@ tda998x_encoder_init(struct i2c_client *client,
 		    struct drm_encoder_slave *encoder_slave)
 {
 	struct tda998x_priv *priv;
+	int ret;
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -1158,8 +1184,11 @@ tda998x_encoder_init(struct i2c_client *client,
 	tda998x_reset(priv);
 
 	/* read version: */
-	priv->rev = reg_read(priv, REG_VERSION_LSB) |
-			reg_read(priv, REG_VERSION_MSB) << 8;
+	ret = reg_read(priv, REG_VERSION_LSB) |
+		(reg_read(priv, REG_VERSION_MSB) << 8);
+	if (ret < 0)
+		goto fail;
+	priv->rev = ret;
 
 	/* mask off feature bits: */
 	priv->rev &= ~0x30; /* not-hdcp and not-scalar bit */
-- 
1.8.5.3

WARNING: multiple messages have this Message-ID (diff)
From: Jean-Francois Moine <moinejf@free.fr>
To: dri-devel@lists.freedesktop.org
Cc: Dave Airlie <airlied@gmail.com>, Rob Clark <robdclark@gmail.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	Russell King - ARM Linux <linux@arm.linux.org.uk>
Subject: [PATCH v4 02/24] drm/i2c: tda998x: check more I/O errors
Date: Sat, 25 Jan 2014 18:14:45 +0100	[thread overview]
Message-ID: <20140125181445.38f8b30b@armhf> (raw)
In-Reply-To: <cover.1390646891.git.moinejf@free.fr>

This patch adds more error checking inn I2C I/O functions.
In case of I/O error, this permits to avoid writing in bad controller
pages, a bad chipset detection or looping when getting the EDID.

Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>
Tested-by: Russell King <rmk+kernel@arm.linux.org.uk>
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
 drivers/gpu/drm/i2c/tda998x_drv.c | 57 +++++++++++++++++++++++++++++----------
 1 file changed, 43 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 26dd299..11f0607 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -362,7 +362,7 @@ fail:
 	return 0;
 }
 
-static void
+static int
 set_page(struct tda998x_priv *priv, uint16_t reg)
 {
 	if (REG2PAGE(reg) != priv->current_page) {
@@ -371,11 +371,14 @@ set_page(struct tda998x_priv *priv, uint16_t reg)
 				REG_CURPAGE, REG2PAGE(reg)
 		};
 		int ret = i2c_master_send(client, buf, sizeof(buf));
-		if (ret < 0)
+		if (ret < 0) {
 			dev_err(&client->dev, "Error %d writing to REG_CURPAGE\n", ret);
+			return ret;
+		}
 
 		priv->current_page = REG2PAGE(reg);
 	}
+	return 0;
 }
 
 static int
@@ -385,7 +388,9 @@ reg_read_range(struct tda998x_priv *priv, uint16_t reg, char *buf, int cnt)
 	uint8_t addr = REG2ADDR(reg);
 	int ret;
 
-	set_page(priv, reg);
+	ret = set_page(priv, reg);
+	if (ret < 0)
+		return ret;
 
 	ret = i2c_master_send(client, &addr, sizeof(addr));
 	if (ret < 0)
@@ -412,18 +417,24 @@ reg_write_range(struct tda998x_priv *priv, uint16_t reg, uint8_t *p, int cnt)
 	buf[0] = REG2ADDR(reg);
 	memcpy(&buf[1], p, cnt);
 
-	set_page(priv, reg);
+	ret = set_page(priv, reg);
+	if (ret < 0)
+		return;
 
 	ret = i2c_master_send(client, buf, cnt + 1);
 	if (ret < 0)
 		dev_err(&client->dev, "Error %d writing to 0x%x\n", ret, reg);
 }
 
-static uint8_t
+static int
 reg_read(struct tda998x_priv *priv, uint16_t reg)
 {
 	uint8_t val = 0;
-	reg_read_range(priv, reg, &val, sizeof(val));
+	int ret;
+
+	ret = reg_read_range(priv, reg, &val, sizeof(val));
+	if (ret < 0)
+		return ret;
 	return val;
 }
 
@@ -434,7 +445,9 @@ reg_write(struct tda998x_priv *priv, uint16_t reg, uint8_t val)
 	uint8_t buf[] = {REG2ADDR(reg), val};
 	int ret;
 
-	set_page(priv, reg);
+	ret = set_page(priv, reg);
+	if (ret < 0)
+		return;
 
 	ret = i2c_master_send(client, buf, ARRAY_SIZE(buf));
 	if (ret < 0)
@@ -448,7 +461,9 @@ reg_write16(struct tda998x_priv *priv, uint16_t reg, uint16_t val)
 	uint8_t buf[] = {REG2ADDR(reg), val >> 8, val};
 	int ret;
 
-	set_page(priv, reg);
+	ret = set_page(priv, reg);
+	if (ret < 0)
+		return;
 
 	ret = i2c_master_send(client, buf, ARRAY_SIZE(buf));
 	if (ret < 0)
@@ -458,13 +473,21 @@ reg_write16(struct tda998x_priv *priv, uint16_t reg, uint16_t val)
 static void
 reg_set(struct tda998x_priv *priv, uint16_t reg, uint8_t val)
 {
-	reg_write(priv, reg, reg_read(priv, reg) | val);
+	int old_val;
+
+	old_val = reg_read(priv, reg);
+	if (old_val >= 0)
+		reg_write(priv, reg, old_val | val);
 }
 
 static void
 reg_clear(struct tda998x_priv *priv, uint16_t reg, uint8_t val)
 {
-	reg_write(priv, reg, reg_read(priv, reg) & ~val);
+	int old_val;
+
+	old_val = reg_read(priv, reg);
+	if (old_val >= 0)
+		reg_write(priv, reg, old_val & ~val);
 }
 
 static void
@@ -970,8 +993,10 @@ read_edid_block(struct drm_encoder *encoder, uint8_t *buf, int blk)
 
 	/* wait for block read to complete: */
 	for (i = 100; i > 0; i--) {
-		uint8_t val = reg_read(priv, REG_INT_FLAGS_2);
-		if (val & INT_FLAGS_2_EDID_BLK_RD)
+		ret = reg_read(priv, REG_INT_FLAGS_2);
+		if (ret < 0)
+			return ret;
+		if (ret & INT_FLAGS_2_EDID_BLK_RD)
 			break;
 		msleep(1);
 	}
@@ -1134,6 +1159,7 @@ tda998x_encoder_init(struct i2c_client *client,
 		    struct drm_encoder_slave *encoder_slave)
 {
 	struct tda998x_priv *priv;
+	int ret;
 
 	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -1158,8 +1184,11 @@ tda998x_encoder_init(struct i2c_client *client,
 	tda998x_reset(priv);
 
 	/* read version: */
-	priv->rev = reg_read(priv, REG_VERSION_LSB) |
-			reg_read(priv, REG_VERSION_MSB) << 8;
+	ret = reg_read(priv, REG_VERSION_LSB) |
+		(reg_read(priv, REG_VERSION_MSB) << 8);
+	if (ret < 0)
+		goto fail;
+	priv->rev = ret;
 
 	/* mask off feature bits: */
 	priv->rev &= ~0x30; /* not-hdcp and not-scalar bit */
-- 
1.8.5.3

  parent reply	other threads:[~2014-01-25 17:14 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1390646891.git.moinejf@free.fr>
2014-01-25 17:14 ` [PATCH v4 18/24] drm/i2c: tda998x: set the PLL division factor in range 0..3 Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 24/24] drm/i2c: tda998x: adjust the audio clock divider for S/PDIF Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-27 20:12   ` Russell King - ARM Linux
2014-01-27 20:12     ` Russell King - ARM Linux
2014-01-25 17:14 ` [PATCH v4 17/24] drm/i2c: tda998x: fix the ENABLE_SPACE register Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 16/24] drm/i2c: tda998x: add DT documentation Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 15/24] drm/i2c: tda998x: use irq for connection status and EDID read Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-27 20:20   ` Russell King - ARM Linux
2014-01-27 20:20     ` Russell King - ARM Linux
2014-01-25 17:14 ` [PATCH v4 14/24] drm/i2c: tda998x: always enable EDID read IRQ Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 23/24] drm/i2c: tda998x: code optimization Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 22/24] drm/i2c: tda998x: change the frequence in the audio channel Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 19/24] drm/i2c: tda998x: make the audio code more readable Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 20/24] drm/i2c: tda998x: remove the unused variable ca_i2s Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 21/24] drm/i2c: tda998x: add the active aspect in HDMI AVI frame Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 13/24] drm/i2c: tda998x: add DT support Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 09/24] drm/i2c: tda998x: use HDMI constants Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 10/24] drm/i2c: tda998x: don't read write-only registers Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 11/24] drm/i2c: tda998x: free the CEC device on encoder_destroy Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 07/24] drm/i2c: tda998x: set the video mode from the adjusted value Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-27 19:59   ` Russell King - ARM Linux
2014-01-27 19:59     ` Russell King - ARM Linux
2014-01-25 17:14 ` [PATCH v4 12/24] drm/i2c: tda998x: check the CEC device creation Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-27 20:01   ` Russell King - ARM Linux
2014-01-27 20:01     ` Russell King - ARM Linux
2014-01-25 17:14 ` [PATCH v4 08/24] drm/i2c: tda998x: fix bad value in the AIF Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 05/24] drm/i2c: tda998x: don't freeze the system at audio startup time Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` Jean-Francois Moine [this message]
2014-01-25 17:14   ` [PATCH v4 02/24] drm/i2c: tda998x: check more I/O errors Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 06/24] drm/i2c: tda998x: force the page register at startup time Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 04/24] drm/i2c: tda998x: change probe message origin Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 03/24] drm/i2c: tda998x: code cleanup Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14 ` [PATCH v4 01/24] drm/i2c: tda998x: simplify the i2c read/write functions Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine
2014-01-25 17:14   ` Jean-Francois Moine

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=20140125181445.38f8b30b@armhf \
    --to=moinejf@free.fr \
    --cc=linux-arm-kernel@lists.infradead.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.