[PATCHv2 1/3] i2c: xlp9xx: Check for Bus state before every transfer

[PATCHv2 1/3] i2c: xlp9xx: Check for Bus state before every transfer

[George Cherian]

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.cherian@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

[PATCH 2/3] i2c: xlp9xx: Handle NACK on DATA properly

[George Cherian]

In case we receive NACK on DATA we shouldn't be resetting the controller,
rather we should issue STOP command. This will terminate the current
transaction and -EIO is returned.

While at that handle the SMBus Quick Command properly.
We shouldn't be setting the XLP9XX_I2C_CMD_READ/WRITE for such
transactions.

Signed-off-by: George Cherian <george.cherian@cavium.com>
---
 drivers/i2c/busses/i2c-xlp9xx.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index 42dd1fa..eb8913e 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -352,7 +352,9 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, struct i2c_msg *msg,
 
 	/* set cmd reg */
 	cmd = XLP9XX_I2C_CMD_START;
-	cmd |= (priv->msg_read ? XLP9XX_I2C_CMD_READ : XLP9XX_I2C_CMD_WRITE);
+	if (msg->len)
+		cmd |= (priv->msg_read ?
+			XLP9XX_I2C_CMD_READ : XLP9XX_I2C_CMD_WRITE);
 	if (last_msg)
 		cmd |= XLP9XX_I2C_CMD_STOP;
 
@@ -361,12 +363,12 @@ static int xlp9xx_i2c_xfer_msg(struct xlp9xx_i2c_dev *priv, struct i2c_msg *msg,
 	timeleft = msecs_to_jiffies(XLP9XX_I2C_TIMEOUT_MS);
 	timeleft = wait_for_completion_timeout(&priv->msg_complete, timeleft);
 
-	if (priv->msg_err) {
+	if (priv->msg_err & XLP9XX_I2C_INTEN_BUSERR) {
 		dev_dbg(priv->dev, "transfer error %x!\n", priv->msg_err);
-		if (priv->msg_err & XLP9XX_I2C_INTEN_BUSERR)
-			xlp9xx_i2c_init(priv);
-		return (priv->msg_err & XLP9XX_I2C_INTEN_NACKADDR) ?
-			-ENXIO : -EIO;
+		xlp9xx_write_i2c_reg(priv, XLP9XX_I2C_CMD, XLP9XX_I2C_CMD_STOP);
+		return -EIO;
+	} else if (priv->msg_err & XLP9XX_I2C_INTEN_NACKADDR) {
+		return -ENXIO;
 	}
 
 	if (timeleft == 0) {
-- 
2.1.4

[PATCH 3/3] i2c: xlp9xx: Add support for SMBAlert

[George Cherian]

Add support for SMBus alert mechanism to i2c-xlp9xx driver.
The second interrupt is parsed to use for SMBus alert.
The first interrupt is the i2c controller main interrupt.

Signed-off-by: Kamlakant Patel <kamlakant.patel@cavium.com>
Signed-off-by: George Cherian <george.cherian@cavium.com>
---
 drivers/i2c/busses/i2c-xlp9xx.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
index eb8913e..9462eab 100644
--- a/drivers/i2c/busses/i2c-xlp9xx.c
+++ b/drivers/i2c/busses/i2c-xlp9xx.c
@@ -10,6 +10,7 @@
 #include <linux/clk.h>
 #include <linux/completion.h>
 #include <linux/i2c.h>
+#include <linux/i2c-smbus.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -84,6 +85,8 @@ struct xlp9xx_i2c_dev {
 	struct device *dev;
 	struct i2c_adapter adapter;
 	struct completion msg_complete;
+	struct i2c_smbus_alert_setup alert_data;
+	struct i2c_client *ara;
 	int irq;
 	bool msg_read;
 	bool len_recv;
@@ -447,6 +450,21 @@ static int xlp9xx_i2c_get_frequency(struct platform_device *pdev,
 	return 0;
 }
 
+static int xlp9xx_i2c_smbus_setup(struct xlp9xx_i2c_dev *priv,
+				  struct platform_device *pdev)
+{
+	if (!priv->alert_data.irq)
+		return -EINVAL;
+
+	priv->alert_data.alert_edge_triggered = 0;
+
+	priv->ara = i2c_setup_smbus_alert(&priv->adapter, &priv->alert_data);
+	if (!priv->ara)
+		return -ENODEV;
+
+	return 0;
+}
+
 static int xlp9xx_i2c_probe(struct platform_device *pdev)
 {
 	struct xlp9xx_i2c_dev *priv;
@@ -467,6 +485,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
 		dev_err(&pdev->dev, "invalid irq!\n");
 		return priv->irq;
 	}
+	/* SMBAlert irq */
+	priv->alert_data.irq = platform_get_irq(pdev, 1);
+	if (priv->alert_data.irq <= 0)
+		priv->alert_data.irq = 0;
 
 	xlp9xx_i2c_get_frequency(pdev, priv);
 	xlp9xx_i2c_init(priv);
@@ -493,6 +515,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
 	if (err)
 		return err;
 
+	err = xlp9xx_i2c_smbus_setup(priv, pdev);
+	if (err)
+		dev_info(&pdev->dev, "No active SMBus alert %d\n", err);
+
 	platform_set_drvdata(pdev, priv);
 	dev_dbg(&pdev->dev, "I2C bus:%d added\n", priv->adapter.nr);
 
-- 
2.1.4

Re: [PATCH 3/3] i2c: xlp9xx: Add support for SMBAlert

[Jan Glauber]

On Tue, Feb 27, 2018 at 01:26:20PM +0000, George Cherian wrote:
> Add support for SMBus alert mechanism to i2c-xlp9xx driver.
> The second interrupt is parsed to use for SMBus alert.
> The first interrupt is the i2c controller main interrupt.
> 
> Signed-off-by: Kamlakant Patel <kamlakant.patel@cavium.com>
> Signed-off-by: George Cherian <george.cherian@cavium.com>
> ---
>  drivers/i2c/busses/i2c-xlp9xx.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
> index eb8913e..9462eab 100644
> --- a/drivers/i2c/busses/i2c-xlp9xx.c
> +++ b/drivers/i2c/busses/i2c-xlp9xx.c
> @@ -10,6 +10,7 @@
>  #include <linux/clk.h>
>  #include <linux/completion.h>
>  #include <linux/i2c.h>
> +#include <linux/i2c-smbus.h>
>  #include <linux/init.h>
>  #include <linux/interrupt.h>
>  #include <linux/io.h>
> @@ -84,6 +85,8 @@ struct xlp9xx_i2c_dev {
>  	struct device *dev;
>  	struct i2c_adapter adapter;
>  	struct completion msg_complete;
> +	struct i2c_smbus_alert_setup alert_data;
> +	struct i2c_client *ara;
>  	int irq;
>  	bool msg_read;
>  	bool len_recv;
> @@ -447,6 +450,21 @@ static int xlp9xx_i2c_get_frequency(struct platform_device *pdev,
>  	return 0;
>  }
>  
> +static int xlp9xx_i2c_smbus_setup(struct xlp9xx_i2c_dev *priv,
> +				  struct platform_device *pdev)
> +{
> +	if (!priv->alert_data.irq)
> +		return -EINVAL;
> +
> +	priv->alert_data.alert_edge_triggered = 0;

Hi George,

I think this is not needed anymore, see:
9b9f2b8bc2ac i2c: i2c-smbus: Use threaded irq for smbalert

--Jan

> +
> +	priv->ara = i2c_setup_smbus_alert(&priv->adapter, &priv->alert_data);
> +	if (!priv->ara)
> +		return -ENODEV;
> +
> +	return 0;
> +}
> +
>  static int xlp9xx_i2c_probe(struct platform_device *pdev)
>  {
>  	struct xlp9xx_i2c_dev *priv;
> @@ -467,6 +485,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
>  		dev_err(&pdev->dev, "invalid irq!\n");
>  		return priv->irq;
>  	}
> +	/* SMBAlert irq */
> +	priv->alert_data.irq = platform_get_irq(pdev, 1);
> +	if (priv->alert_data.irq <= 0)
> +		priv->alert_data.irq = 0;
>  
>  	xlp9xx_i2c_get_frequency(pdev, priv);
>  	xlp9xx_i2c_init(priv);
> @@ -493,6 +515,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
>  	if (err)
>  		return err;
>  
> +	err = xlp9xx_i2c_smbus_setup(priv, pdev);
> +	if (err)
> +		dev_info(&pdev->dev, "No active SMBus alert %d\n", err);
> +
>  	platform_set_drvdata(pdev, priv);
>  	dev_dbg(&pdev->dev, "I2C bus:%d added\n", priv->adapter.nr);
>  
> -- 
> 2.1.4

Re: [PATCH 3/3] i2c: xlp9xx: Add support for SMBAlert

[Phil Reid]

On 6/03/2018 16:36, Jan Glauber wrote:
> On Tue, Feb 27, 2018 at 01:26:20PM +0000, George Cherian wrote:
>> Add support for SMBus alert mechanism to i2c-xlp9xx driver.
>> The second interrupt is parsed to use for SMBus alert.
>> The first interrupt is the i2c controller main interrupt.
>>
>> Signed-off-by: Kamlakant Patel <kamlakant.patel@cavium.com>
>> Signed-off-by: George Cherian <george.cherian@cavium.com>
>> ---
>>   drivers/i2c/busses/i2c-xlp9xx.c | 26 ++++++++++++++++++++++++++
>>   1 file changed, 26 insertions(+)
>>
>> diff --git a/drivers/i2c/busses/i2c-xlp9xx.c b/drivers/i2c/busses/i2c-xlp9xx.c
>> index eb8913e..9462eab 100644
>> --- a/drivers/i2c/busses/i2c-xlp9xx.c
>> +++ b/drivers/i2c/busses/i2c-xlp9xx.c
>> @@ -10,6 +10,7 @@
>>   #include <linux/clk.h>
>>   #include <linux/completion.h>
>>   #include <linux/i2c.h>
>> +#include <linux/i2c-smbus.h>
>>   #include <linux/init.h>
>>   #include <linux/interrupt.h>
>>   #include <linux/io.h>
>> @@ -84,6 +85,8 @@ struct xlp9xx_i2c_dev {
>>   	struct device *dev;
>>   	struct i2c_adapter adapter;
>>   	struct completion msg_complete;
>> +	struct i2c_smbus_alert_setup alert_data;
>> +	struct i2c_client *ara;
>>   	int irq;
>>   	bool msg_read;
>>   	bool len_recv;
>> @@ -447,6 +450,21 @@ static int xlp9xx_i2c_get_frequency(struct platform_device *pdev,
>>   	return 0;
>>   }
>>   
>> +static int xlp9xx_i2c_smbus_setup(struct xlp9xx_i2c_dev *priv,
>> +				  struct platform_device *pdev)
>> +{
>> +	if (!priv->alert_data.irq)
>> +		return -EINVAL;
>> +
>> +	priv->alert_data.alert_edge_triggered = 0;
> 
> Hi George,
> 
> I think this is not needed anymore, see:
> 9b9f2b8bc2ac i2c: i2c-smbus: Use threaded irq for smbalert
> 
> --Jan

Yes.

And also all of this is not needed if named interrupts.
- interrupt-names
	"irq", "wakeup" and "smbus_alert" names are recognized by I2C core,
	other names are	left to individual drivers.

presence of named irq smbus_alert should result in alert handler being
created for that bus by the core

> 
>> +
>> +	priv->ara = i2c_setup_smbus_alert(&priv->adapter, &priv->alert_data);
>> +	if (!priv->ara)
>> +		return -ENODEV;
>> +
>> +	return 0;
>> +}
>> +
>>   static int xlp9xx_i2c_probe(struct platform_device *pdev)
>>   {
>>   	struct xlp9xx_i2c_dev *priv;
>> @@ -467,6 +485,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
>>   		dev_err(&pdev->dev, "invalid irq!\n");
>>   		return priv->irq;
>>   	}
>> +	/* SMBAlert irq */
>> +	priv->alert_data.irq = platform_get_irq(pdev, 1);
>> +	if (priv->alert_data.irq <= 0)
>> +		priv->alert_data.irq = 0;
>>   
>>   	xlp9xx_i2c_get_frequency(pdev, priv);
>>   	xlp9xx_i2c_init(priv);
>> @@ -493,6 +515,10 @@ static int xlp9xx_i2c_probe(struct platform_device *pdev)
>>   	if (err)
>>   		return err;
>>   
>> +	err = xlp9xx_i2c_smbus_setup(priv, pdev);
>> +	if (err)
>> +		dev_info(&pdev->dev, "No active SMBus alert %d\n", err);
>> +
>>   	platform_set_drvdata(pdev, priv);
>>   	dev_dbg(&pdev->dev, "I2C bus:%d added\n", priv->adapter.nr);
>>   
>> -- 
>> 2.1.4
> 
> 


-- 
Regards
Phil Reid

Re: [PATCH 3/3] i2c: xlp9xx: Add support for SMBAlert

[George Cherian]

On 03/06/2018 02:48 PM, Phil Reid wrote:
> On 6/03/2018 16:36, Jan Glauber wrote:
>> On Tue, Feb 27, 2018 at 01:26:20PM +0000, George Cherian wrote:
>>> Add support for SMBus alert mechanism to i2c-xlp9xx driver.
>>> The second interrupt is parsed to use for SMBus alert.
>>> The first interrupt is the i2c controller main interrupt.
>>>
>>> Signed-off-by: Kamlakant Patel <kamlakant.patel@cavium.com>
>>> Signed-off-by: George Cherian <george.cherian@cavium.com>
>>> ---
>>>   drivers/i2c/busses/i2c-xlp9xx.c | 26 ++++++++++++++++++++++++++
>>>   1 file changed, 26 insertions(+)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-xlp9xx.c 
>>> b/drivers/i2c/busses/i2c-xlp9xx.c
>>> index eb8913e..9462eab 100644
>>> --- a/drivers/i2c/busses/i2c-xlp9xx.c
>>> +++ b/drivers/i2c/busses/i2c-xlp9xx.c
>>> @@ -10,6 +10,7 @@
>>>   #include <linux/clk.h>
>>>   #include <linux/completion.h>
>>>   #include <linux/i2c.h>
>>> +#include <linux/i2c-smbus.h>
>>>   #include <linux/init.h>
>>>   #include <linux/interrupt.h>
>>>   #include <linux/io.h>
>>> @@ -84,6 +85,8 @@ struct xlp9xx_i2c_dev {
>>>       struct device *dev;
>>>       struct i2c_adapter adapter;
>>>       struct completion msg_complete;
>>> +    struct i2c_smbus_alert_setup alert_data;
>>> +    struct i2c_client *ara;
>>>       int irq;
>>>       bool msg_read;
>>>       bool len_recv;
>>> @@ -447,6 +450,21 @@ static int xlp9xx_i2c_get_frequency(struct 
>>> platform_device *pdev,
>>>       return 0;
>>>   }
>>> +static int xlp9xx_i2c_smbus_setup(struct xlp9xx_i2c_dev *priv,
>>> +                  struct platform_device *pdev)
>>> +{
>>> +    if (!priv->alert_data.irq)
>>> +        return -EINVAL;
>>> +
>>> +    priv->alert_data.alert_edge_triggered = 0;
>>
>> Hi George,
>>
>> I think this is not needed anymore, see:
>> 9b9f2b8bc2ac i2c: i2c-smbus: Use threaded irq for smbalert
>>
>> --Jan
> 
> Yes.
> 
> And also all of this is not needed if named interrupts.
> - interrupt-names
>      "irq", "wakeup" and "smbus_alert" names are recognized by I2C core,
>      other names are    left to individual drivers.
> 
> presence of named irq smbus_alert should result in alert handler being
> created for that bus by the core
> 

Thanks for the comments I will send out a v2 for this patch.
>>
>>> +
>>> +    priv->ara = i2c_setup_smbus_alert(&priv->adapter, 
>>> &priv->alert_data);
>>> +    if (!priv->ara)
>>> +        return -ENODEV;
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   static int xlp9xx_i2c_probe(struct platform_device *pdev)
>>>   {
>>>       struct xlp9xx_i2c_dev *priv;
>>> @@ -467,6 +485,10 @@ static int xlp9xx_i2c_probe(struct 
>>> platform_device *pdev)
>>>           dev_err(&pdev->dev, "invalid irq!\n");
>>>           return priv->irq;
>>>       }
>>> +    /* SMBAlert irq */
>>> +    priv->alert_data.irq = platform_get_irq(pdev, 1);
>>> +    if (priv->alert_data.irq <= 0)
>>> +        priv->alert_data.irq = 0;
>>>       xlp9xx_i2c_get_frequency(pdev, priv);
>>>       xlp9xx_i2c_init(priv);
>>> @@ -493,6 +515,10 @@ static int xlp9xx_i2c_probe(struct 
>>> platform_device *pdev)
>>>       if (err)
>>>           return err;
>>> +    err = xlp9xx_i2c_smbus_setup(priv, pdev);
>>> +    if (err)
>>> +        dev_info(&pdev->dev, "No active SMBus alert %d\n", err);
>>> +
>>>       platform_set_drvdata(pdev, priv);
>>>       dev_dbg(&pdev->dev, "I2C bus:%d added\n", priv->adapter.nr);
>>> -- 
>>> 2.1.4
>>
>>
> 
> 

Re: [PATCHv2 1/3] i2c: xlp9xx: Check for Bus state before every transfer

[Jan Glauber]

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 <jglauber@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.cherian@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

Re: [PATCHv2 1/3] i2c: xlp9xx: Check for Bus state before every transfer

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 283 bytes --]


> I don't know how valuable same-company reviewed-by's are in the end, 

Case by case. And in your case, I trust you.

> but the patches look good to me with the small change in SMBalert, so you
> could add:
> 
> Reviewed-by: Jan Glauber <jglauber@cavium.com>

Thanks.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]