Re: [PATCH v4] i2c: rcar: add support for I2C_M_RECV_LEN

[Eugeniu Rosca <erosca@de.adit-jv.com>]

Hello Wolfram,

On Di, Apr 05, 2022 at 12:07:56 +0200, Wolfram Sang wrote:
> With this feature added, SMBus Block reads and Proc calls are now
> supported. This patch is the best of two independent developments by
> Wolfram and Bhuvanesh + Andrew, refactored again by Wolfram.
> 
> Signed-off-by: Bhuvanesh Surachari <bhuvanesh_surachari@mentor.com>
> Signed-off-by: Andrew Gabbasov <andrew_gabbasov@mentor.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> For testing, I wired a Lager board (R-Car H2) and a Salvator-XS (R-Car
> H3 ES2.0) together. The Lager board ran the testunit and provided SMBus
> Proc Calls. The Salvator-XS board was requesting the data.
> 
> Compared to my previous version: sending 1 byte works now, sending with
> DMA as well. Invalid sizes are detected, too. This is as much as I can
> test, I'd think.
> 
> Compared to Bhuvanesh + Andrew's last version: less intrusive and more
> self contained (no goto), Proc Calls are covered as well
> 
> I tried some other refactoring as well (like one single place where
> rcar_i2c_dma() is called) but IMHO this is the most readable solution.
> 
> Thank you everyone for working on this. I am very interested in your
> comments and test results!
> 
>  drivers/i2c/busses/i2c-rcar.c | 31 +++++++++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)

I am not an i2c/SMBus expert, but I genuinely tried to attack the patch
from multiple angles, including static analysis (smatch, cppcheck, sparse,
PVS Studio, coccicheck, make W=123), code review, dynamic testing
(KASAN, UBSAN and friends enabled) and couldn't spot any misbehavior or
any obvious opportunities for optimization.

We've tested this patch on vanilla and on 4.14 using reference and
non-reference boards and the behavior matched our expectations.

I've also briefly glanced at the i2c fault injection possibilities, as
described in https://elinux.org/Tests:I2C-fault-injection, but soon
realized this will require HW/board modifications, which are not
straightforward in my personal environment.

Looking forward to any other review comments.
Thank you for your friendly support and cooperation.

Reviewed-by: Eugeniu Rosca <erosca@de.adit-jv.com>
Tested-by: Eugeniu Rosca <erosca@de.adit-jv.com>

Best regards,
Eugeniu