diff mbox

[v2] i2c: davinci: Avoid sending to own address

Message ID 55534B7A.8070105@nokia.com
State Changes Requested
Headers show

Commit Message

Alexander A Sverdlin May 13, 2015, 1:02 p.m. UTC
Sending a message to own address locks the controller up in very bizarre state,
it behaves as slave even if MDR register clearly states master. The controller
remains in this state until reset. To avoid unnecessary timeouts simply avoid
sending to own address. The controller cannot do this any way. Also, do not
enable AAS IRQ, as the slave mode is not supported by the driver and the only
possibility to trigger this IRQ is to send to own address.

Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
---
Changes in v2:
- rebased on 110bc76729d4 of Linus's tree;

 drivers/i2c/busses/i2c-davinci.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

grygorii.strashko@linaro.org May 14, 2015, 11:03 a.m. UTC | #1
On 05/13/2015 04:02 PM, Alexander Sverdlin wrote:
> Sending a message to own address locks the controller up in very bizarre state,
> it behaves as slave even if MDR register clearly states master. The controller
> remains in this state until reset. To avoid unnecessary timeouts simply avoid
> sending to own address. The controller cannot do this any way. Also, do not
> enable AAS IRQ, as the slave mode is not supported by the driver and the only
> possibility to trigger this IRQ is to send to own address.
>

Reviewed-by: Grygorii Strashko <grygorii.strashko@linaro.org>

> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@nokia.com>
> ---
> Changes in v2:
> - rebased on 110bc76729d4 of Linus's tree;
>
>   drivers/i2c/busses/i2c-davinci.c |    9 ++++++---
>   1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
> index 4788a32..df96ab6 100644
> --- a/drivers/i2c/busses/i2c-davinci.c
> +++ b/drivers/i2c/busses/i2c-davinci.c
> @@ -41,8 +41,8 @@
>
>   #define DAVINCI_I2C_TIMEOUT	(1*HZ)
>   #define DAVINCI_I2C_MAX_TRIES	2
> -#define I2C_DAVINCI_INTR_ALL    (DAVINCI_I2C_IMR_AAS | \
> -				 DAVINCI_I2C_IMR_SCD | \
> +#define DAVINCI_I2C_OWN_ADDRESS	0x08
> +#define I2C_DAVINCI_INTR_ALL    (DAVINCI_I2C_IMR_SCD | \
>   				 DAVINCI_I2C_IMR_ARDY | \
>   				 DAVINCI_I2C_IMR_NACK | \
>   				 DAVINCI_I2C_IMR_AL)
> @@ -233,7 +233,7 @@ static int i2c_davinci_init(struct davinci_i2c_dev *dev)
>   	/* Respond at reserved "SMBus Host" slave address" (and zero);
>   	 * we seem to have no option to not respond...
>   	 */
> -	davinci_i2c_write_reg(dev, DAVINCI_I2C_OAR_REG, 0x08);
> +	davinci_i2c_write_reg(dev, DAVINCI_I2C_OAR_REG, DAVINCI_I2C_OWN_ADDRESS);
>
>   	dev_dbg(dev->dev, "PSC  = %d\n",
>   		davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));
> @@ -390,6 +390,9 @@ i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
>   	u16 w;
>   	unsigned long time_left;
>
> +	if (msg->addr == DAVINCI_I2C_OWN_ADDRESS)
> +		return -EIO;
> +
>   	/* Introduce a delay, required for some boards (e.g Davinci EVM) */
>   	if (pdata->bus_delay)
>   		udelay(pdata->bus_delay);
>
Wolfram Sang June 2, 2015, 3:48 p.m. UTC | #2
On Wed, May 13, 2015 at 03:02:50PM +0200, Alexander Sverdlin wrote:
> Sending a message to own address locks the controller up in very bizarre state,
> it behaves as slave even if MDR register clearly states master. The controller

Eeks...

>  
> +	if (msg->addr == DAVINCI_I2C_OWN_ADDRESS)
> +		return -EIO;

I think we should return at least -EADDRNOTAVAIL here. Otherwise the
user might wonder if the slave is broken giving always IO errors. Maybe
we should also add a warning message, I leave that decision to you.
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-davinci.c b/drivers/i2c/busses/i2c-davinci.c
index 4788a32..df96ab6 100644
--- a/drivers/i2c/busses/i2c-davinci.c
+++ b/drivers/i2c/busses/i2c-davinci.c
@@ -41,8 +41,8 @@ 
 
 #define DAVINCI_I2C_TIMEOUT	(1*HZ)
 #define DAVINCI_I2C_MAX_TRIES	2
-#define I2C_DAVINCI_INTR_ALL    (DAVINCI_I2C_IMR_AAS | \
-				 DAVINCI_I2C_IMR_SCD | \
+#define DAVINCI_I2C_OWN_ADDRESS	0x08
+#define I2C_DAVINCI_INTR_ALL    (DAVINCI_I2C_IMR_SCD | \
 				 DAVINCI_I2C_IMR_ARDY | \
 				 DAVINCI_I2C_IMR_NACK | \
 				 DAVINCI_I2C_IMR_AL)
@@ -233,7 +233,7 @@  static int i2c_davinci_init(struct davinci_i2c_dev *dev)
 	/* Respond at reserved "SMBus Host" slave address" (and zero);
 	 * we seem to have no option to not respond...
 	 */
-	davinci_i2c_write_reg(dev, DAVINCI_I2C_OAR_REG, 0x08);
+	davinci_i2c_write_reg(dev, DAVINCI_I2C_OAR_REG, DAVINCI_I2C_OWN_ADDRESS);
 
 	dev_dbg(dev->dev, "PSC  = %d\n",
 		davinci_i2c_read_reg(dev, DAVINCI_I2C_PSC_REG));
@@ -390,6 +390,9 @@  i2c_davinci_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg, int stop)
 	u16 w;
 	unsigned long time_left;
 
+	if (msg->addr == DAVINCI_I2C_OWN_ADDRESS)
+		return -EIO;
+
 	/* Introduce a delay, required for some boards (e.g Davinci EVM) */
 	if (pdata->bus_delay)
 		udelay(pdata->bus_delay);