| 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
| ||
|
Date: Wed, 23 Mar 2022 22:52:29 +0100
From: Eugeniu Rosca <erosca@...adit-jv.com>
To: Wolfram Sang <wsa+renesas@...g-engineering.com>,
Andrew Gabbasov <andrew_gabbasov@...tor.com>,
Geert Uytterhoeven <geert+renesas@...der.be>
CC: Eugeniu Rosca <erosca@...adit-jv.com>,
<linux-renesas-soc@...r.kernel.org>, <linux-i2c@...r.kernel.org>,
<linux-kernel@...r.kernel.org>,
Bhuvanesh Surachari <bhuvanesh_surachari@...tor.com>,
Eugeniu Rosca <roscaeugeniu@...il.com>
Subject: Re: [PATCH v2] i2c: rcar: add SMBus block read support
Dear Wolfram,
Dear Andrew,
Dear Geert,
A couple of questions and test results below.
On Thu, Feb 17, 2022 at 08:44:51PM +0100, Wolfram Sang wrote:
> 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 am obviously not an SMBus expert, but I wonder if simply testing the
PCA9654 I/O Expander with SMBus support on the H3-Salvator-X target
could be acceptable as a test procedure? See some test results below.
>
> 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;
>
############################################################
################# PATCH-INDEPENDENT OUTPUT #################
############################################################
## .config: https://gist.github.com/erosca/690c3e6065b55546e511f9ef8ba59625
## i2c-tools: https://git.kernel.org/pub/scm/utils/i2c-tools/i2c-tools.git/commit/?id=cf3541b8a7
root@...r-gen3:# uname -r
5.17.0+
root@...r-gen3:# cat /sys/firmware/devicetree/base/model
Renesas Salvator-X board based on r8a77951
root@...r-gen3:# i2cdetect -l
i2c-7 i2c e60b0000.i2c I2C adapter
i2c-2 i2c e6510000.i2c I2C adapter
i2c-4 i2c e66d8000.i2c I2C adapter
^
` i2c-4 is the PCA9654 I/O Expander with SMBus protocol support
root@...r-gen3:# i2cdetect -y -r 4
0 1 2 3 4 5 6 7 8 9 a b c d e f
00: -- -- -- -- -- -- -- --
10: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
20: UU -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
30: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
40: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
50: -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- --
60: UU UU UU UU UU UU -- -- 68 -- UU -- -- -- -- --
70: UU UU UU UU UU UU -- --
############################################################
###################### VANILLA v5.17 #######################
############################################################
root@...r-gen3:# i2cdetect -F 4
Functionalities implemented by /dev/i2c-4:
I2C yes
SMBus Quick Command no
SMBus Send Byte yes
SMBus Receive Byte yes
SMBus Write Byte yes
SMBus Read Byte yes
SMBus Write Word yes
SMBus Read Word yes
SMBus Process Call yes
SMBus Block Write yes
SMBus Block Read no <<<--- We aim to enable this
SMBus Block Process Call no
SMBus PEC yes
I2C Block Write yes
I2C Block Read yes
root@...r-gen3:# i2cget -y 4 0x68 0 i 8
0x08 0xff 0x06 0xff 0x5f 0xff 0x11 0x08
root@...r-gen3:# i2cget -y 4 0x68 0 s
Error: Adapter does not have SMBus block read capability
############################################################
#################### ANDREW'S V2 PATCH #####################
############################################################
root@...r-gen3:# i2cdetect -F 4
Functionalities implemented by /dev/i2c-4:
I2C yes
SMBus Quick Command no
SMBus Send Byte yes
SMBus Receive Byte yes
SMBus Write Byte yes
SMBus Read Byte yes
SMBus Write Word yes
SMBus Read Word yes
SMBus Process Call yes
SMBus Block Write yes
SMBus Block Read yes <<<--- Enabled (tested below)
SMBus Block Process Call no
SMBus PEC yes
I2C Block Write yes
I2C Block Read yes
root@...r-gen3:# i2cget -y 4 0x68 0 i 8
0x08 0xff 0x06 0xff 0x5f 0xff 0x11 0x08
root@...r-gen3:# i2cget -y 4 0x68 0 s
0x08 0xff 0x06 0xff 0x5f 0xff 0x11 0x08
############################################################
##################### WOLFRAM'S PATCH ######################
############################################################
root@...r-gen3:# i2cdetect -F 4
Functionalities implemented by /dev/i2c-4:
I2C yes
SMBus Quick Command no
SMBus Send Byte yes
SMBus Receive Byte yes
SMBus Write Byte yes
SMBus Read Byte yes
SMBus Write Word yes
SMBus Read Word yes
SMBus Process Call yes
SMBus Block Write yes
SMBus Block Read yes <<<--- Enabled (tested)
SMBus Block Process Call yes <<<--- Enabled (not tested)
SMBus PEC yes
I2C Block Write yes
I2C Block Read yes
root@...r-gen3:# i2cget -y 4 0x68 0 i 8
0x08 0xff 0x06 0xff 0x5f 0xff 0x11 0x08
root@...r-gen3:# i2cget -y 4 0x68 0 s
0xff 0x06 0xff 0x5f 0xff 0x11 0x08 0x08
############################################################
Any comments?
Best regards,
Eugeniu
Powered by blists - more mailing lists