diff mbox

[2/2] i2c: omap: make reset a seperate function

Message ID 1351160586-12306-3-git-send-email-shubhrajyoti@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Shubhrajyoti Datta Oct. 25, 2012, 10:23 a.m. UTC
Implement reset as a seperate function.
This will enable us to make sure that we don't do the
calculation again on every transfer.
Also at probe the reset is not added as the hwmod is doing that
for us.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
 drivers/i2c/busses/i2c-omap.c |   28 ++++++++++++++++++----------
 1 files changed, 18 insertions(+), 10 deletions(-)

Comments

Felipe Balbi Oct. 25, 2012, 10:55 a.m. UTC | #1
Hi,

On Thu, Oct 25, 2012 at 03:53:06PM +0530, Shubhrajyoti D wrote:
> Implement reset as a seperate function.
> This will enable us to make sure that we don't do the
> calculation again on every transfer.
> Also at probe the reset is not added as the hwmod is doing that
> for us.

since you're touching registers which supposedly only hwmod should
touch, you ought to Cc Benoit to make sure he knows what's you're doing
here. I'm adding him to Cc

> @@ -592,7 +597,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
>  	if (timeout == 0) {
>  		dev_err(dev->dev, "controller timed out\n");
>  		ret = -ETIMEDOUT;
> -		omap_i2c_init(dev);
> +		omap_i2c_reset(dev);
> +		__omap_i2c_init(dev);
>  		goto out;
>  	}
>  
> @@ -603,7 +609,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
>  			|| (dev->cmd_err & OMAP_I2C_STAT_ROVR)
>  			|| (dev->cmd_err & OMAP_I2C_STAT_XUDF)) {
>  		ret = -EIO;
> -		omap_i2c_init(dev);
> +		omap_i2c_reset(dev);
> +		__omap_i2c_init(dev);
>  		goto out;
>  	}
>  
> @@ -621,7 +628,8 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
>  			return 0;
>  
>  		ret = -EREMOTEIO;
> -		omap_i2c_init(dev);
> +		omap_i2c_reset(dev);
> +		__omap_i2c_init(dev);

eventually we need to try to forcefully trigger these errors above
(nack, overflow, underflow and arbitration lost) and try to make sure if
actually need to reset the controller all the time. I find it really odd
that we're always resetting the IP in every error condition without
actually trying to figure out what's wrong with the driver (if there is
something wrong with the driver, of course).
Shubhrajyoti Datta Oct. 25, 2012, 1:47 p.m. UTC | #2
On Thursday 25 October 2012 04:25 PM, Felipe Balbi wrote:

>  overflow, underflow
these are data errors personally feel may be removed.

>  and arbitration lost) 
will investigate.
> and try to make sure if
> actually need to reset the controller all the time. I find it really odd
> that we're always resetting the IP in every error condition without
> actually trying to figure out what's wrong with the driver (if there is
> something wrong with the driver, of course).

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

Patch

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index 3d400b1..5a87ff9 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -306,15 +306,9 @@  static void __omap_i2c_init(struct omap_i2c_dev *dev)
 		omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, dev->iestate);
 }
 
-static int omap_i2c_init(struct omap_i2c_dev *dev)
+static int omap_i2c_reset(struct omap_i2c_dev *dev)
 {
-	u16 psc = 0, scll = 0, sclh = 0;
-	u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
-	unsigned long fclk_rate = 12000000;
 	unsigned long timeout;
-	unsigned long internal_clk = 0;
-	struct clk *fclk;
-
 	if (dev->rev >= OMAP_I2C_OMAP1_REV_2) {
 		/* Disable I2C controller before soft reset */
 		omap_i2c_write_reg(dev, OMAP_I2C_CON_REG,
@@ -360,6 +354,17 @@  static int omap_i2c_init(struct omap_i2c_dev *dev)
 			dev->westate = OMAP_I2C_WE_ALL;
 		}
 	}
+	return 0;
+}
+
+static int omap_i2c_init(struct omap_i2c_dev *dev)
+{
+	u16 psc = 0, scll = 0, sclh = 0;
+	u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0;
+	unsigned long fclk_rate = 12000000;
+	unsigned long internal_clk = 0;
+	struct clk *fclk;
+
 
 	if (dev->flags & OMAP_I2C_FLAG_ALWAYS_ARMXOR_CLK) {
 		/*
@@ -592,7 +597,8 @@  static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 	if (timeout == 0) {
 		dev_err(dev->dev, "controller timed out\n");
 		ret = -ETIMEDOUT;
-		omap_i2c_init(dev);
+		omap_i2c_reset(dev);
+		__omap_i2c_init(dev);
 		goto out;
 	}
 
@@ -603,7 +609,8 @@  static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 			|| (dev->cmd_err & OMAP_I2C_STAT_ROVR)
 			|| (dev->cmd_err & OMAP_I2C_STAT_XUDF)) {
 		ret = -EIO;
-		omap_i2c_init(dev);
+		omap_i2c_reset(dev);
+		__omap_i2c_init(dev);
 		goto out;
 	}
 
@@ -621,7 +628,8 @@  static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 			return 0;
 
 		ret = -EREMOTEIO;
-		omap_i2c_init(dev);
+		omap_i2c_reset(dev);
+		__omap_i2c_init(dev);
 	}
 
 out: