I don't know how valuable same-company reviewed-by's are in the end,
but the patches look good to me with the small change in SMBalert, so you
could add:
Reviewed-by: Jan Glauber <jglau...@cavium.com>
--Jan
On Tue, Feb 27, 2018 at 01:26:18PM +0000, George Cherian wrote:
> I2C bus enters the STOP condition after the DATA_DONE interrupt is raised.
> Essentially the driver should be checking the bus state before sending
> any transaction. In case a transaction is initiated while the
> bus is busy, the prior transaction's stop condition is not achieved.
> Add the check to make sure the bus is not busy before every transaction.
>
> Signed-off-by: George Cherian <george.cher...@cavium.com>
> ---
> drivers/i2c/busses/i2c-xlp9xx.c | 32 ++++++++++++++++++++++++++++++++
> 1 file changed, 32 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
> index 1f6d780..42dd1fa 100644
> --- a/drivers/i2c/busses/i2c-xlp9xx.c
> +++ b/drivers/i2c/busses/i2c-xlp9xx.c
> @@ -16,6 +16,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
> +#include <linux/delay.h>
>
> #define XLP9XX_I2C_DIV 0x0
> #define XLP9XX_I2C_CTRL 0x1
> @@ -36,6 +37,8 @@
> #define XLP9XX_I2C_TIMEOUT 0X10
> #define XLP9XX_I2C_GENCALLADDR 0x11
>
> +#define XLP9XX_I2C_STATUS_BUSY BIT(0)
> +
> #define XLP9XX_I2C_CMD_START BIT(7)
> #define XLP9XX_I2C_CMD_STOP BIT(6)
> #define XLP9XX_I2C_CMD_READ BIT(5)
> @@ -71,6 +74,7 @@
> #define XLP9XX_I2C_HIGH_FREQ 400000
> #define XLP9XX_I2C_FIFO_SIZE 0x80U
> #define XLP9XX_I2C_TIMEOUT_MS 1000
> +#define XLP9XX_I2C_BUSY_TIMEOUT 50
>
> #define XLP9XX_I2C_FIFO_WCNT_MASK 0xff
> #define XLP9XX_I2C_STATUS_ERRMASK (XLP9XX_I2C_INTEN_ARLOST | \
> @@ -241,6 +245,26 @@ static irqreturn_t xlp9xx_i2c_isr(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static int xlp9xx_i2c_check_bus_status(struct xlp9xx_i2c_dev *priv)
> +{
> + u32 status;
> + u32 busy_timeout = XLP9XX_I2C_BUSY_TIMEOUT;
> +
> + while (busy_timeout) {
> + status = xlp9xx_read_i2c_reg(priv, XLP9XX_I2C_STATUS);
> + if ((status & XLP9XX_I2C_STATUS_BUSY) == 0)
> + break;
> +
> + busy_timeout--;
> + usleep_range(1000, 1100);
> + }
> +
> + if (!busy_timeout)
> + return -EIO;
> +
> + return 0;
> +}
> +
> static int xlp9xx_i2c_init(struct xlp9xx_i2c_dev *priv)
> {
> u32 prescale;
> @@ -363,6 +387,14 @@ static int xlp9xx_i2c_xfer(struct i2c_adapter *adap,
> struct i2c_msg *msgs,
> int i, ret;
> struct xlp9xx_i2c_dev *priv = i2c_get_adapdata(adap);
>
> + ret = xlp9xx_i2c_check_bus_status(priv);
> + if (ret) {
> + xlp9xx_i2c_init(priv);
> + ret = xlp9xx_i2c_check_bus_status(priv);
> + if (ret)
> + return ret;
> + }
> +
> for (i = 0; i < num; i++) {
> ret = xlp9xx_i2c_xfer_msg(priv, &msgs[i], i == num - 1);
> if (ret != 0)
> --
> 2.1.4
