linux-kernel - Re: [PATCH v2] i2c: rcar: add SMBus block read support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yg6ls0zyTDe7LQbK@kunai>
Date:   Thu, 17 Feb 2022 20:44:51 +0100
From:   Wolfram Sang <wsa+renesas@...g-engineering.com>
To:     Andrew Gabbasov <andrew_gabbasov@...tor.com>
Cc:     linux-renesas-soc@...r.kernel.org, linux-i2c@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Geert Uytterhoeven <geert+renesas@...der.be>,
        Bhuvanesh Surachari <bhuvanesh_surachari@...tor.com>
Subject: Re: [PATCH v2] i2c: rcar: add SMBus block read support

Hi Andrew,

first sorry that it took so long. The reason here is that my original
plan was to add 256-byte support to RECV_LEN in the I2C core and enable
it on R-Car afterwards. Sadly, I never found the time to drive this
forward. So, all RECV_LEN things got stuck for a while :(

> This patch (adapted) was tested with v4.14, but due to lack of real
> hardware with SMBus block read operations support, using "simulation",
> that is manual analysis of data, read from plain I2C devices with
> SMBus block read request.

You could wire up two R-Car I2C instances, set up one as an I2C slave
handled by the I2C testunit and then use the other instance with
SMBUS_BLOCK_PROC_CALL which also needs RECV_LEN. Check
Documentation/i2c/slave-testunit-backend.rst for details.

I wonder a bit about the complexity of your patch. In my WIP-branch for
256-byte transfers, I have the following patch. It is only missing the
range check for the received byte, but that it easy to add. Do you see
anything else missing? If not, I prefer this simpler version because it
is less intrusive and the state machine is a bit fragile (due to HW
issues with old HW).

From: Wolfram Sang <wsa+renesas@...g-engineering.com>
Date: Sun, 2 Aug 2020 00:24:52 +0200
Subject: [PATCH] i2c: rcar: add support for I2C_M_RECV_LEN

Signed-off-by: Wolfram Sang <wsa+renesas@...g-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 217def2d7cb4..e473f5c0a708 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -528,6 +528,7 @@ static void rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 static void rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 {
 	struct i2c_msg *msg = priv->msg;
+	bool recv_len_init = priv->pos == 0 && msg->flags & I2C_M_RECV_LEN;
 
 	/* FIXME: sometimes, unknown interrupt happened. Do nothing */
 	if (!(msr & MDR))
@@ -542,11 +543,13 @@ static void rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 	} else if (priv->pos < msg->len) {
 		/* get received data */
 		msg->buf[priv->pos] = rcar_i2c_read(priv, ICRXTX);
+		if (recv_len_init)
+			msg->len += msg->buf[0];
 		priv->pos++;
 	}
 
 	/* If next received data is the _LAST_, go to new phase. */
-	if (priv->pos + 1 == msg->len) {
+	if (priv->pos + 1 == msg->len && !recv_len_init) {
 		if (priv->flags & ID_LAST_MSG) {
 			rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
 		} else {
@@ -889,7 +892,7 @@ static u32 rcar_i2c_func(struct i2c_adapter *adap)
 	 * I2C_M_IGNORE_NAK (automatically sends STOP after NAK)
 	 */
 	u32 func = I2C_FUNC_I2C | I2C_FUNC_SLAVE |
-		   (I2C_FUNC_SMBUS_EMUL & ~I2C_FUNC_SMBUS_QUICK);
+		   (I2C_FUNC_SMBUS_EMUL_ALL & ~I2C_FUNC_SMBUS_QUICK);
 
 	if (priv->flags & ID_P_HOST_NOTIFY)
 		func |= I2C_FUNC_SMBUS_HOST_NOTIFY;

Happy hacking,

   Wolfram


Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)