[PATCH v2 2/7] i2c: omap: reorder exit path of omap_i2c_xfer_msg()

2012-10-25 Thread Felipe Balbi 
just a cleanup patch trying to make exit path
more straightforward. No changes otherwise.

Signed-off-by: Felipe Balbi ba...@ti.com
---
 drivers/i2c/busses/i2c-omap.c | 26 +-
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index c07d9c4..bea0277 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -505,6 +505,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
 {
struct omap_i2c_dev *dev = i2c_get_adapdata(adap);
unsigned long timeout;
+   int ret;
u16 w;
 
dev_dbg(dev-dev, addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n,
@@ -582,31 +583,38 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
dev-buf_len = 0;
if (timeout == 0) {
dev_err(dev-dev, controller timed out\n);
-   omap_i2c_init(dev);
-   return -ETIMEDOUT;
+   ret = -ETIMEDOUT;
+   goto err_i2c_init;
}
 
-   if (likely(!dev-cmd_err))
-   return 0;
-
/* We have an error */
if (dev-cmd_err  (OMAP_I2C_STAT_AL | OMAP_I2C_STAT_ROVR |
OMAP_I2C_STAT_XUDF)) {
-   omap_i2c_init(dev);
-   return -EIO;
+   ret = -EIO;
+   goto err_i2c_init;
}
 
if (dev-cmd_err  OMAP_I2C_STAT_NACK) {
if (msg-flags  I2C_M_IGNORE_NAK)
return 0;
+
if (stop) {
w = omap_i2c_read_reg(dev, OMAP_I2C_CON_REG);
w |= OMAP_I2C_CON_STP;
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, w);
}
-   return -EREMOTEIO;
+
+   ret = -EREMOTEIO;
+   goto err;
}
-   return -EIO;
+
+   return 0;
+
+err_i2c_init:
+   omap_i2c_init(dev);
+
+err:
+   return ret;
 }
 
 
-- 
1.8.0

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

Re: [PATCH v2 2/7] i2c: omap: reorder exit path of omap_i2c_xfer_msg()

2012-10-25 Thread Santosh Shilimkar 

On Thursday 25 October 2012 05:55 PM, Felipe Balbi wrote:

just a cleanup patch trying to make exit path
more straightforward. No changes otherwise.

Signed-off-by: Felipe Balbi ba...@ti.com
---
  drivers/i2c/busses/i2c-omap.c | 26 +-
  1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
index c07d9c4..bea0277 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -505,6 +505,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
  {
struct omap_i2c_dev *dev = i2c_get_adapdata(adap);
unsigned long timeout;
+   int ret;

You might want to initialize the error value to avoid return 0. Compiler
might be already cribbing for it


u16 w;

dev_dbg(dev-dev, addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n,
@@ -582,31 +583,38 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
dev-buf_len = 0;
if (timeout == 0) {
dev_err(dev-dev, controller timed out\n);
-   omap_i2c_init(dev);
-   return -ETIMEDOUT;
+   ret = -ETIMEDOUT;
+   goto err_i2c_init;
}

-   if (likely(!dev-cmd_err))
-   return 0;
-

Have you have drooped this check completely ?


/* We have an error */
if (dev-cmd_err  (OMAP_I2C_STAT_AL | OMAP_I2C_STAT_ROVR |
OMAP_I2C_STAT_XUDF)) {
-   omap_i2c_init(dev);
-   return -EIO;
+   ret = -EIO;
+   goto err_i2c_init;
}

if (dev-cmd_err  OMAP_I2C_STAT_NACK) {
if (msg-flags  I2C_M_IGNORE_NAK)
return 0;
+
if (stop) {
w = omap_i2c_read_reg(dev, OMAP_I2C_CON_REG);
w |= OMAP_I2C_CON_STP;
omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, w);
}
-   return -EREMOTEIO;
+
+   ret = -EREMOTEIO;
+   goto err;
}
-   return -EIO;
+
+   return 0;

With initialized value you can use
return ret;

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

Re: [PATCH v2 2/7] i2c: omap: reorder exit path of omap_i2c_xfer_msg()

2012-10-25 Thread Felipe Balbi 
Hi,

On Thu, Oct 25, 2012 at 06:12:00PM +0530, Santosh Shilimkar wrote:
 On Thursday 25 October 2012 05:55 PM, Felipe Balbi wrote:
 just a cleanup patch trying to make exit path
 more straightforward. No changes otherwise.
 
 Signed-off-by: Felipe Balbi ba...@ti.com
 ---
   drivers/i2c/busses/i2c-omap.c | 26 +-
   1 file changed, 17 insertions(+), 9 deletions(-)
 
 diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
 index c07d9c4..bea0277 100644
 --- a/drivers/i2c/busses/i2c-omap.c
 +++ b/drivers/i2c/busses/i2c-omap.c
 @@ -505,6 +505,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
   {
  struct omap_i2c_dev *dev = i2c_get_adapdata(adap);
  unsigned long timeout;
 +int ret;
 You might want to initialize the error value to avoid return 0. Compiler
 might be already cribbing for it
 
  u16 w;
 
  dev_dbg(dev-dev, addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n,
 @@ -582,31 +583,38 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
  dev-buf_len = 0;
  if (timeout == 0) {
  dev_err(dev-dev, controller timed out\n);
 -omap_i2c_init(dev);
 -return -ETIMEDOUT;
 +ret = -ETIMEDOUT;
 +goto err_i2c_init;
  }
 
 -if (likely(!dev-cmd_err))
 -return 0;
 -
 Have you have drooped this check completely ?

yes, the idea is to have a single exit point, so all checks below would
be executed.

  /* We have an error */
  if (dev-cmd_err  (OMAP_I2C_STAT_AL | OMAP_I2C_STAT_ROVR |
  OMAP_I2C_STAT_XUDF)) {
 -omap_i2c_init(dev);
 -return -EIO;
 +ret = -EIO;
 +goto err_i2c_init;
  }
 
  if (dev-cmd_err  OMAP_I2C_STAT_NACK) {
  if (msg-flags  I2C_M_IGNORE_NAK)
  return 0;
 +
  if (stop) {
  w = omap_i2c_read_reg(dev, OMAP_I2C_CON_REG);
  w |= OMAP_I2C_CON_STP;
  omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, w);
  }
 -return -EREMOTEIO;
 +
 +ret = -EREMOTEIO;
 +goto err;
  }
 -return -EIO;
 +
 +return 0;
 With initialized value you can use
 return ret;

hmm, I guess I did that on a follow up patch where I grouped the stop
handling.

-- 
balbi


signature.asc
Description: Digital signature

Re: [PATCH v2 2/7] i2c: omap: reorder exit path of omap_i2c_xfer_msg()

2012-10-25 Thread Lothar Waßmann 
Hi,

Santosh Shilimkar writes:
 On Thursday 25 October 2012 05:55 PM, Felipe Balbi wrote:
  just a cleanup patch trying to make exit path
  more straightforward. No changes otherwise.
 
  Signed-off-by: Felipe Balbi ba...@ti.com
  ---
drivers/i2c/busses/i2c-omap.c | 26 +-
1 file changed, 17 insertions(+), 9 deletions(-)
 
  diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c
  index c07d9c4..bea0277 100644
  --- a/drivers/i2c/busses/i2c-omap.c
  +++ b/drivers/i2c/busses/i2c-omap.c
  @@ -505,6 +505,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap,
{
  struct omap_i2c_dev *dev = i2c_get_adapdata(adap);
  unsigned long timeout;
  +   int ret;
[...]
  +   ret = -EREMOTEIO;
  +   goto err;
  }
  -   return -EIO;
  +
  +   return 0;
 With initialized value you can use
 return ret;
 
Doing it this way has the advantage, that if an additional error exit
is added it will generate an 'uninitialized variable' warning, if it
fails to set the return value.



Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html