Re: [PATCH 2/2] i2c: imx-lpi2c: don't treat a NACK in SMBus block read as an error

[Carlos Song]

> An SMBus device might indicate that a value isn't ready yet by asserting
> a NACK.
> This will lead to the following error message which will flood the
> kernel log for some device like an IPMI SSIF BMC:
> 
>    [    4.277005] i2c i2c-5: SMBus read count failed -110
> 
> Stop the polling if NDF is set and return -EIO, just like everywhere
> else. While at it, drop the MSR_RDF_ASSERTED() macro, it's just one bit,
> no need for FIELD_GET.
> 
> Fixes: efdc383d1cc2 ("i2c: imx-lpi2c: fix SMBus block read NACK after byte count")
> Signed-off-by: Michael Walle <mwalle@xxxxxxxxxx>
> ---
>  drivers/i2c/busses/i2c-imx-lpi2c.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
> index f57e800ac3b5..583155802e8a 100644
> --- a/drivers/i2c/busses/i2c-imx-lpi2c.c
> +++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
> @@ -92,7 +92,6 @@
>  #define MRDR_RXEMPTY	BIT(14)
>  #define MDER_TDDE	BIT(0)
>  #define MDER_RDDE	BIT(1)
> -#define MSR_RDF_ASSERTED(x) FIELD_GET(MSR_RDF, (x))
>  
>  #define SCR_SEN		BIT(0)
>  #define SCR_RST		BIT(1)
> @@ -594,7 +593,10 @@ static int lpi2c_imx_read_init(struct lpi2c_imx_struct *lpi2c_imx,
>  		writel((RECV_DATA << 8) | 0x01, lpi2c_imx->base + LPI2C_MTDR);
>  
>  		ret = readl_poll_timeout(lpi2c_imx->base + LPI2C_MSR, val,
> -					 MSR_RDF_ASSERTED(val), 1, 1000);
> +					 (val & (MSR_RDF | MSR_NDF)), 1, 1000);
> +		if (val & MSR_NDF)
> +			return -EIO;
> +
>  		if (ret) {
>  			dev_err(&lpi2c_imx->adapter.dev, "SMBus read count failed %d\n", ret);
>  			return ret;

LGTM,
Acked-by: Carlos Song <carlos.song@xxxxxxx>
-- 
Best Regards,
Carlos