2018-08-16 16:28:20

by Esben Haabendal

[permalink] [raw]
Subject: [PATCH v4 2/3] i2c: imx: Simplify stopped state tracking

From: Esben Haabendal <[email protected]>

Always update the stopped state when busy status have been checked.
This is identical to what was done before, with the exception of error
handling.
Without this change, some errors cause the stopped state to be left in
incorrect state in i2c_imx_stop(), i2c_imx_dma_read(), i2c_imx_read() and
i2c_imx_xfer().

Signed-off-by: Esben Haabendal <[email protected]>
Acked-by: Uwe Kleine-König <[email protected]>
---
drivers/i2c/busses/i2c-imx.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index ad6adefb64da..c406700789e1 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -421,10 +421,14 @@ static int i2c_imx_bus_busy(struct imx_i2c_struct *i2c_imx, int for_busy)
return -EAGAIN;
}

- if (for_busy && (temp & I2SR_IBB))
+ if (for_busy && (temp & I2SR_IBB)) {
+ i2c_imx->stopped = 0;
break;
- if (!for_busy && !(temp & I2SR_IBB))
+ }
+ if (!for_busy && !(temp & I2SR_IBB)) {
+ i2c_imx->stopped = 1;
break;
+ }
if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
dev_dbg(&i2c_imx->adapter.dev,
"<%s> I2C bus is busy\n", __func__);
@@ -538,7 +542,6 @@ static int i2c_imx_start(struct imx_i2c_struct *i2c_imx)
result = i2c_imx_bus_busy(i2c_imx, 1);
if (result)
return result;
- i2c_imx->stopped = 0;

temp |= I2CR_IIEN | I2CR_MTX | I2CR_TXAK;
temp &= ~I2CR_DMAEN;
@@ -567,10 +570,8 @@ static void i2c_imx_stop(struct imx_i2c_struct *i2c_imx)
udelay(i2c_imx->disable_delay);
}

- if (!i2c_imx->stopped) {
+ if (!i2c_imx->stopped)
i2c_imx_bus_busy(i2c_imx, 0);
- i2c_imx->stopped = 1;
- }

/* Disable I2C controller */
temp = i2c_imx->hwdata->i2cr_ien_opcode ^ I2CR_IEN,
@@ -724,7 +725,6 @@ static int i2c_imx_dma_read(struct imx_i2c_struct *i2c_imx,
temp &= ~(I2CR_MSTA | I2CR_MTX);
imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
i2c_imx_bus_busy(i2c_imx, 0);
- i2c_imx->stopped = 1;
} else {
/*
* For i2c master receiver repeat restart operation like:
@@ -850,7 +850,6 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs, bo
temp &= ~(I2CR_MSTA | I2CR_MTX);
imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
i2c_imx_bus_busy(i2c_imx, 0);
- i2c_imx->stopped = 1;
} else {
/*
* For i2c master receiver repeat restart operation like:
--
2.18.0



2018-08-20 09:16:14

by Wolfram Sang

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] i2c: imx: Simplify stopped state tracking

On Thu, Aug 16, 2018 at 10:43:13AM +0200, Esben Haabendal wrote:
> From: Esben Haabendal <[email protected]>
>
> Always update the stopped state when busy status have been checked.
> This is identical to what was done before, with the exception of error
> handling.
> Without this change, some errors cause the stopped state to be left in
> incorrect state in i2c_imx_stop(), i2c_imx_dma_read(), i2c_imx_read() and
> i2c_imx_xfer().
>
> Signed-off-by: Esben Haabendal <[email protected]>
> Acked-by: Uwe Kleine-König <[email protected]>

Applied to for-next, thanks! I did not add stable because the cover
latter said minor issue? Not really sure here, though...


Attachments:
(No filename) (686.00 B)
signature.asc (849.00 B)
Download all attachments

2018-08-20 09:23:01

by Esben Haabendal

[permalink] [raw]
Subject: Re: [PATCH v4 2/3] i2c: imx: Simplify stopped state tracking

Wolfram Sang <[email protected]> writes:

> On Thu, Aug 16, 2018 at 10:43:13AM +0200, Esben Haabendal wrote:
>> From: Esben Haabendal <[email protected]>
>>
>> Always update the stopped state when busy status have been checked.
>> This is identical to what was done before, with the exception of error
>> handling.
>> Without this change, some errors cause the stopped state to be left in
>> incorrect state in i2c_imx_stop(), i2c_imx_dma_read(), i2c_imx_read() and
>> i2c_imx_xfer().
>>
>> Signed-off-by: Esben Haabendal <[email protected]>
>> Acked-by: Uwe Kleine-König <[email protected]>
>
> Applied to for-next, thanks! I did not add stable because the cover
> latter said minor issue? Not really sure here, though...

I agree, it does not need to go into stable.

/Esben