All of lore.kernel.org
 help / color / mirror / Atom feed
From: Federico Fuga <fuga@studiofuga.com>
To: unlisted-recipients:; (no To-header on input)
Cc: Federico Fuga <fuga@studiofuga.com>,
	Gregory CLEMENT <gregory.clement@bootlin.com>,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] i2c: mv64xxx: Implement I2C_M_RECV_LEN and I2C_FUNC_SMBUS_READ_BLOCK_DATA
Date: Sat, 18 Jan 2020 12:58:20 +0100	[thread overview]
Message-ID: <20200118115820.9080-1-fuga@studiofuga.com> (raw)

The i2c_mv64xxx driver doesn't implement the I2C_M_REC_LEN function
essential to allow blocks with variable length to be read from an i2c
 slave.
This is needed to implement the SMBus Read Block Data function.

This patch implements the function by changing the bytes_left and
msg len on the fly if the flag is specified.

It has been successfully tested on Allwinner A33 with a special
i2c chip that returns variable length blocks on reading.

Signed-off-by: Federico Fuga <fuga@studiofuga.com>
---
 drivers/i2c/busses/i2c-mv64xxx.c | 67 +++++++++++++++++++++++++-------
 1 file changed, 53 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index a5a95ea5b81a..cff9cb20bcc9 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -128,6 +128,7 @@ struct mv64xxx_i2c_data {
 	u32			addr1;
 	u32			addr2;
 	u32			bytes_left;
+	u32         effective_length;
 	u32			byte_posn;
 	u32			send_stop;
 	u32			block;
@@ -333,7 +334,18 @@ static void mv64xxx_i2c_send_start(struct mv64xxx_i2c_data *drv_data)
 {
 	drv_data->msg = drv_data->msgs;
 	drv_data->byte_posn = 0;
-	drv_data->bytes_left = drv_data->msg->len;
+
+    /* If we should retrieve the length from the buffer, make sure */
+	/* to read enough bytes to avoid sending the */
+	/* STOP bit after the read if the first byte */
+	if (drv_data->msg->flags & I2C_M_RECV_LEN) {
+		drv_data->effective_length = -1;
+		drv_data->bytes_left = 3;
+	} else {
+		drv_data->effective_length = drv_data->msg->len;
+		drv_data->bytes_left = drv_data->msg->len;
+	}
+
 	drv_data->aborting = 0;
 	drv_data->rc = 0;
 
@@ -342,6 +354,42 @@ static void mv64xxx_i2c_send_start(struct mv64xxx_i2c_data *drv_data)
 	       drv_data->reg_base + drv_data->reg_offsets.control);
 }
 
+static void
+mv64xxx_i2c_do_send_stop(struct mv64xxx_i2c_data *drv_data)
+{
+	drv_data->cntl_bits &= ~MV64XXX_I2C_REG_CONTROL_INTEN;
+	writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_STOP,
+		drv_data->reg_base + drv_data->reg_offsets.control);
+	drv_data->block = 0;
+	if (drv_data->errata_delay)
+		udelay(5);
+
+	wake_up(&drv_data->waitq);
+}
+
+static void
+mv64xxx_i2c_do_read_data(struct mv64xxx_i2c_data *drv_data)
+{
+	u8 data;
+
+	data = readl(drv_data->reg_base + drv_data->reg_offsets.data);
+	drv_data->msg->buf[drv_data->byte_posn++] = data;
+
+	if (drv_data->effective_length == -1) {
+		/* length=0 should not be allowed, but is indeed possible.
+		 * To avoid locking the chip, we keep reading at least 2 bytes
+		 */
+		if (data < 1)
+			data = 1;
+		drv_data->effective_length = data+1;
+		drv_data->bytes_left = data+1;
+		drv_data->msg->len = data+1;
+	}
+
+	writel(drv_data->cntl_bits,
+		drv_data->reg_base + drv_data->reg_offsets.control);
+}
+
 static void
 mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
 {
@@ -392,23 +440,13 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
 		break;
 
 	case MV64XXX_I2C_ACTION_RCV_DATA:
-		drv_data->msg->buf[drv_data->byte_posn++] =
-			readl(drv_data->reg_base + drv_data->reg_offsets.data);
-		writel(drv_data->cntl_bits,
-			drv_data->reg_base + drv_data->reg_offsets.control);
+	    mv64xxx_i2c_do_read_data(drv_data);
 		break;
 
 	case MV64XXX_I2C_ACTION_RCV_DATA_STOP:
 		drv_data->msg->buf[drv_data->byte_posn++] =
 			readl(drv_data->reg_base + drv_data->reg_offsets.data);
-		drv_data->cntl_bits &= ~MV64XXX_I2C_REG_CONTROL_INTEN;
-		writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_STOP,
-			drv_data->reg_base + drv_data->reg_offsets.control);
-		drv_data->block = 0;
-		if (drv_data->errata_delay)
-			udelay(5);
-
-		wake_up(&drv_data->waitq);
+	    mv64xxx_i2c_do_send_stop(drv_data);
 		break;
 
 	case MV64XXX_I2C_ACTION_INVALID:
@@ -706,7 +744,8 @@ mv64xxx_i2c_can_offload(struct mv64xxx_i2c_data *drv_data)
 static u32
 mv64xxx_i2c_functionality(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR | I2C_FUNC_SMBUS_EMUL;
+	return I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR |
+		I2C_FUNC_SMBUS_READ_BLOCK_DATA | I2C_FUNC_SMBUS_EMUL;
 }
 
 static int
-- 
2.17.1


WARNING: multiple messages have this Message-ID (diff)
From: Federico Fuga <fuga@studiofuga.com>
Cc: Federico Fuga <fuga@studiofuga.com>,
	Gregory CLEMENT <gregory.clement@bootlin.com>,
	linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] i2c: mv64xxx: Implement I2C_M_RECV_LEN and I2C_FUNC_SMBUS_READ_BLOCK_DATA
Date: Sat, 18 Jan 2020 12:58:20 +0100	[thread overview]
Message-ID: <20200118115820.9080-1-fuga@studiofuga.com> (raw)

The i2c_mv64xxx driver doesn't implement the I2C_M_REC_LEN function
essential to allow blocks with variable length to be read from an i2c
 slave.
This is needed to implement the SMBus Read Block Data function.

This patch implements the function by changing the bytes_left and
msg len on the fly if the flag is specified.

It has been successfully tested on Allwinner A33 with a special
i2c chip that returns variable length blocks on reading.

Signed-off-by: Federico Fuga <fuga@studiofuga.com>
---
 drivers/i2c/busses/i2c-mv64xxx.c | 67 +++++++++++++++++++++++++-------
 1 file changed, 53 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mv64xxx.c b/drivers/i2c/busses/i2c-mv64xxx.c
index a5a95ea5b81a..cff9cb20bcc9 100644
--- a/drivers/i2c/busses/i2c-mv64xxx.c
+++ b/drivers/i2c/busses/i2c-mv64xxx.c
@@ -128,6 +128,7 @@ struct mv64xxx_i2c_data {
 	u32			addr1;
 	u32			addr2;
 	u32			bytes_left;
+	u32         effective_length;
 	u32			byte_posn;
 	u32			send_stop;
 	u32			block;
@@ -333,7 +334,18 @@ static void mv64xxx_i2c_send_start(struct mv64xxx_i2c_data *drv_data)
 {
 	drv_data->msg = drv_data->msgs;
 	drv_data->byte_posn = 0;
-	drv_data->bytes_left = drv_data->msg->len;
+
+    /* If we should retrieve the length from the buffer, make sure */
+	/* to read enough bytes to avoid sending the */
+	/* STOP bit after the read if the first byte */
+	if (drv_data->msg->flags & I2C_M_RECV_LEN) {
+		drv_data->effective_length = -1;
+		drv_data->bytes_left = 3;
+	} else {
+		drv_data->effective_length = drv_data->msg->len;
+		drv_data->bytes_left = drv_data->msg->len;
+	}
+
 	drv_data->aborting = 0;
 	drv_data->rc = 0;
 
@@ -342,6 +354,42 @@ static void mv64xxx_i2c_send_start(struct mv64xxx_i2c_data *drv_data)
 	       drv_data->reg_base + drv_data->reg_offsets.control);
 }
 
+static void
+mv64xxx_i2c_do_send_stop(struct mv64xxx_i2c_data *drv_data)
+{
+	drv_data->cntl_bits &= ~MV64XXX_I2C_REG_CONTROL_INTEN;
+	writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_STOP,
+		drv_data->reg_base + drv_data->reg_offsets.control);
+	drv_data->block = 0;
+	if (drv_data->errata_delay)
+		udelay(5);
+
+	wake_up(&drv_data->waitq);
+}
+
+static void
+mv64xxx_i2c_do_read_data(struct mv64xxx_i2c_data *drv_data)
+{
+	u8 data;
+
+	data = readl(drv_data->reg_base + drv_data->reg_offsets.data);
+	drv_data->msg->buf[drv_data->byte_posn++] = data;
+
+	if (drv_data->effective_length == -1) {
+		/* length=0 should not be allowed, but is indeed possible.
+		 * To avoid locking the chip, we keep reading at least 2 bytes
+		 */
+		if (data < 1)
+			data = 1;
+		drv_data->effective_length = data+1;
+		drv_data->bytes_left = data+1;
+		drv_data->msg->len = data+1;
+	}
+
+	writel(drv_data->cntl_bits,
+		drv_data->reg_base + drv_data->reg_offsets.control);
+}
+
 static void
 mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
 {
@@ -392,23 +440,13 @@ mv64xxx_i2c_do_action(struct mv64xxx_i2c_data *drv_data)
 		break;
 
 	case MV64XXX_I2C_ACTION_RCV_DATA:
-		drv_data->msg->buf[drv_data->byte_posn++] =
-			readl(drv_data->reg_base + drv_data->reg_offsets.data);
-		writel(drv_data->cntl_bits,
-			drv_data->reg_base + drv_data->reg_offsets.control);
+	    mv64xxx_i2c_do_read_data(drv_data);
 		break;
 
 	case MV64XXX_I2C_ACTION_RCV_DATA_STOP:
 		drv_data->msg->buf[drv_data->byte_posn++] =
 			readl(drv_data->reg_base + drv_data->reg_offsets.data);
-		drv_data->cntl_bits &= ~MV64XXX_I2C_REG_CONTROL_INTEN;
-		writel(drv_data->cntl_bits | MV64XXX_I2C_REG_CONTROL_STOP,
-			drv_data->reg_base + drv_data->reg_offsets.control);
-		drv_data->block = 0;
-		if (drv_data->errata_delay)
-			udelay(5);
-
-		wake_up(&drv_data->waitq);
+	    mv64xxx_i2c_do_send_stop(drv_data);
 		break;
 
 	case MV64XXX_I2C_ACTION_INVALID:
@@ -706,7 +744,8 @@ mv64xxx_i2c_can_offload(struct mv64xxx_i2c_data *drv_data)
 static u32
 mv64xxx_i2c_functionality(struct i2c_adapter *adap)
 {
-	return I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR | I2C_FUNC_SMBUS_EMUL;
+	return I2C_FUNC_I2C | I2C_FUNC_10BIT_ADDR |
+		I2C_FUNC_SMBUS_READ_BLOCK_DATA | I2C_FUNC_SMBUS_EMUL;
 }
 
 static int
-- 
2.17.1

             reply	other threads:[~2020-01-18 11:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-18 11:58 Federico Fuga [this message]
2020-01-18 11:58 ` [PATCH] i2c: mv64xxx: Implement I2C_M_RECV_LEN and I2C_FUNC_SMBUS_READ_BLOCK_DATA Federico Fuga
2020-02-22 12:48 ` Wolfram Sang
2020-03-13 20:48   ` Gregory CLEMENT
2020-03-13 20:58     ` Federico Fuga
2020-03-20 14:55 ` Wolfram Sang

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=20200118115820.9080-1-fuga@studiofuga.com \
    --to=fuga@studiofuga.com \
    --cc=gregory.clement@bootlin.com \
    --cc=linux-i2c@vger.kernel.org \
    --cc=linux-kernel@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.