Patchwork [U-Boot] I2C: OMAP: detect more devices when probing an i2c bus

login
register
mail settings
Submitter Nick Thompson
Date April 11, 2011, 9:28 a.m.
Message ID <4DA2C9C0.2000206@ge.com>
Download mbox | patch
Permalink /patch/90578/
State Superseded
Delegated to: Heiko Schocher
Headers show

Comments

Nick Thompson - April 11, 2011, 9:28 a.m.
The omap24xx driver only seems to support devices that have a single subaddress
byte. With these types of devices, the first access in a bus transaction is
usually a write (writes the subaddress) followed by either a read or write to
access the devices registers.

Many such devices will respond to a read as the first access, but there are at
least some that will NACK such a read. (e.g. ADV7180.)

The probe function attempts to detect a devices ACK to a read access only and
fails to find devices that NACK a read.

This commit modifies the probe function to start a write instead. This detects
devices that respond to reads (since they must also respond to writes) as well
as those that only respond to writes. The bus is immediately set to idle after a
(N)ACK avoiding actually writing anything to the device.

Signed-off-by: Nick Thompson <nick.thompson@ge.com>
---
Tested on OMAP3530 with an ADV7180 video ADC.

Detection of a device takes the same time as failing to find a device, so the
probe is slightly faster.

 drivers/i2c/omap24xx_i2c.c |   42 +++++++++++-------------------------------
 1 files changed, 11 insertions(+), 31 deletions(-)
Heiko Schocher - April 12, 2011, 6:50 a.m.
Hello Nick,

Nick Thompson wrote:
> The omap24xx driver only seems to support devices that have a single subaddress
> byte. With these types of devices, the first access in a bus transaction is
> usually a write (writes the subaddress) followed by either a read or write to
> access the devices registers.
> 
> Many such devices will respond to a read as the first access, but there are at
> least some that will NACK such a read. (e.g. ADV7180.)
> 
> The probe function attempts to detect a devices ACK to a read access only and
> fails to find devices that NACK a read.
> 
> This commit modifies the probe function to start a write instead. This detects
> devices that respond to reads (since they must also respond to writes) as well
> as those that only respond to writes. The bus is immediately set to idle after a
> (N)ACK avoiding actually writing anything to the device.
> 
> Signed-off-by: Nick Thompson <nick.thompson@ge.com>
> ---
> Tested on OMAP3530 with an ADV7180 video ADC.
> 
> Detection of a device takes the same time as failing to find a device, so the
> probe is slightly faster.
> 
>  drivers/i2c/omap24xx_i2c.c |   42 +++++++++++-------------------------------
>  1 files changed, 11 insertions(+), 31 deletions(-)

Please check your patch with checkpatch, as it shows:

total: 2 errors, 8 warnings, 54 lines checked

NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
      scripts/cleanfile

[PATCH] I2C: OMAP: detect more devices when probing an i2c bus.eml has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Beside of this, your patch looks ok to me.

bye,
Heiko

Patch

diff --git a/drivers/i2c/omap24xx_i2c.c b/drivers/i2c/omap24xx_i2c.c
index 215be34..bc637bc 100644
--- a/drivers/i2c/omap24xx_i2c.c
+++ b/drivers/i2c/omap24xx_i2c.c
@@ -321,43 +321,23 @@  int i2c_probe (uchar chip)
     /* wait until bus not busy */
     wait_for_bb ();
 
-    /* try to read one byte */
+    /* try to write one byte */
     writew (1, &i2c_base->cnt);
     /* set slave address */
     writew (chip, &i2c_base->sa);
     /* stop bit needed here */
-    writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_STP, &i2c_base->con);
+    writew (I2C_CON_EN | I2C_CON_MST | I2C_CON_STT | I2C_CON_TRX |
+        I2C_CON_STP, &i2c_base->con);
 
-    while (1) {
-        status = wait_for_pin();
-        if (status == 0 || status & I2C_STAT_AL) {
-            res = 1;
-            goto probe_exit;
-        }
-        if (status & I2C_STAT_NACK) {
-            res = 1;
-            writew(0xff, &i2c_base->stat);
-            writew (readw (&i2c_base->con) | I2C_CON_STP, &i2c_base->con);
-            wait_for_bb ();
-            break;
-        }
-        if (status & I2C_STAT_ARDY) {
-            writew(I2C_STAT_ARDY, &i2c_base->stat);
-            break;
-        }
-        if (status & I2C_STAT_RRDY) {
-            res = 0;
-#if defined(CONFIG_OMAP243X) || defined(CONFIG_OMAP34XX) || \
-    defined(CONFIG_OMAP44XX)
-            readb(&i2c_base->data);
-#else
-            readw(&i2c_base->data);
-#endif
-            writew(I2C_STAT_RRDY, &i2c_base->stat);
-        }
-    }
+    status = wait_for_pin();
+
+    /* check for ACK (!NAK) */
+    if (!(status & I2C_STAT_NACK))
+        res = 0;
+
+    /* abort transfer (force idle state) */
+    writew (0, &i2c_base->con);
 
-probe_exit:
     flush_fifo();
     writew (0, &i2c_base->cnt); /* don't allow any more data in...we don't want it.*/
     writew(0xFFFF, &i2c_base->stat);