iPod Classic: rework on I2C driver

- Some rewrite with the intent to get ride of these random errors
  appearing on some builds/devices (not much noticeable on RB but
  can ruin bootloader builds).
- Error handling (ACK).
- IIC clock increased to be the same as in OF.

Change-Id: Idf8cfa3c230a0a61ec9c879bf6f0ea8b061a4607

firmware/export/s5l8702.h

@@ -196,6 +196,20 @@
 #define I2CCLKGATE(i)   ((i) == 1 ? CLOCKGATE_I2C1 : \
                                     CLOCKGATE_I2C0)
 
+/*  s5l8702 I2C controller is similar to s5l8700, known differences are:
+
+    * IICCON[5] is not used in s5l8702.
+    * IICCON[13:8] are used to enable interrupts.
+      IICSTA2[13:8] are used to read the status and write-clear interrupts.
+      Known interrupts:
+       [13] STOP on bus (TBC)
+       [12] START on bus (TBC)
+       [8] byte transmited or received in Master mode (not tested in Slave)
+    * IICCON[4] does not clear interrupts, it is enabled when a byte is
+      transmited or received, in Master mode the tx/rx of the next byte
+      starts when it is written as "1".
+*/
+
 #define IICCON(bus)     (*((uint32_t volatile*)(0x3C600000 + 0x300000 * (bus))))
 #define IICSTAT(bus)    (*((uint32_t volatile*)(0x3C600004 + 0x300000 * (bus))))
 #define IICADD(bus)     (*((uint32_t volatile*)(0x3C600008 + 0x300000 * (bus))))
@@ -203,6 +217,7 @@
 #define IICUNK10(bus)   (*((uint32_t volatile*)(0x3C600010 + 0x300000 * (bus))))
 #define IICUNK14(bus)   (*((uint32_t volatile*)(0x3C600014 + 0x300000 * (bus))))
 #define IICUNK18(bus)   (*((uint32_t volatile*)(0x3C600018 + 0x300000 * (bus))))
+#define IICSTA2(bus)    (*((uint32_t volatile*)(0x3C600020 + 0x300000 * (bus))))
 
 
 /////INTERRUPT CONTROLLERS/////

firmware/target/arm/s5l8702/debug-s5l8702.c

@@ -144,8 +144,6 @@ bool dbg_hw_info(void)
             _DEBUG_PRINTF("USB D+: %d mV", adc_read_usbdata_voltage(true));
             _DEBUG_PRINTF("USB D-: %d mV", adc_read_usbdata_voltage(false));
             line++;
-            extern unsigned long i2c_rd_err, i2c_wr_err;
-            _DEBUG_PRINTF("i2c rd/wr errors: %lu/%lu", i2c_rd_err, i2c_wr_err);
         }
 #ifdef UC870X_DEBUG
         else if(state==(max_states-1))

firmware/target/arm/s5l8702/i2c-s5l8702.c

@@ -25,173 +25,174 @@
 #include "i2c-s5l8702.h"
 #include "clocking-s5l8702.h"
 
-/*  Driver for the s5l8700 built-in I2C controller in master mode
+/*  Driver for the s5l8702 built-in I2C controller in master mode
 
     Both the i2c_read and i2c_write function take the following arguments:
     * slave, the address of the i2c slave device to read from / write to
     * address, optional sub-address in the i2c slave (unused if -1)
     * len, number of bytes to be transfered
     * data, pointer to data to be transfered
-    A return value < 0 indicates an error.
+    A return value > 0 indicates an error.
 
     Note:
-    * blocks the calling thread for the entire duraton of the i2c transfer but
+    * blocks the calling thread for the entire duraton of the i2c transfer.
-      uses wakeup_wait/wakeup_signal to allow other threads to run.
-    * ACK from slave is not checked, so functions never return an error
-
-    Fixme:
-    * actually there is no STOP + i2c_off() on error
-    * very rare random errors when reading and/or(?) writing registers on some
-      builds/devices, hard to trace, not a 'delay' issue, it seems related
-      with alignment of STRs and/or(?) LDRs, code cache lines, pipelines...
-      The new code tries to mix STRs and LDRs at some points but ATM it is
-      unknown if it might solve or mitigate the problem. Probably it could be
-      really fixed using wait_rdy() before accessing any register, as OF does.
-*/
-
-/*  s5l8702 I2C controller is similar to s5l8700, known differences are:
-
-    * IICCON[5] is not used in s5l8702.
-    * IICCON[13:8] are used to enable interrupts.
-      IICUNK20[13:8] are used to read the status and write-clear interrupts.
-      Known interrupts:
-       [13] STOP on bus (TBC)
-       [12] START on bus (TBC)
-       [8] byte transmited or received in Master mode (not tested in Slave)
-    * IICCON[4] does not clear interrupts, it is enabled when a byte is
-      transmited or received, in Master mode the tx/rx of the next byte
-      starts when it is written as "1".
 */
 
 static struct mutex i2c_mtx[2];
 
-static void i2c_on(int bus)
-{
-    /* enable I2C clock */
-    clockgate_enable(I2CCLKGATE(bus), true);
-
-    IICCON(bus) = (0 << 8) | /* INT_EN = disabled */
-                  (1 << 7) | /* ACK_GEN */
-                  (0 << 6) | /* CLKSEL = PCLK/16 */
-                  (7 << 0);  /* CK_REG */
-
-    /* serial output on */
-    IICSTAT(bus) = (1 << 4);
-}
-
-static void i2c_off(int bus)
-{
-    /* serial output off */
-    IICSTAT(bus) = 0;
-
-    /* disable I2C clock */
-    clockgate_enable(I2CCLKGATE(bus), false);
-}
-
 void i2c_init()
 {
     mutex_init(&i2c_mtx[0]);
     mutex_init(&i2c_mtx[1]);
 }
 
+static void wait_rdy(int bus)
+{
+    while (IICUNK10(bus));
+}
+
+static void i2c_on(int bus)
+{
+    /* enable I2C clock */
+    clockgate_enable(I2CCLKGATE(bus), true);
+}
+
+static void i2c_off(int bus)
+{
+    /* serial output off */
+    wait_rdy(bus);
+    IICSTAT(bus) = 0;
+    /* disable I2C clock */
+    wait_rdy(bus);
+    clockgate_enable(I2CCLKGATE(bus), false);
+}
+
+/* wait for bus not busy, or tx/rx byte (should return once
+   8 data + 1 ack clocks are generated), or STOP. */
+static void i2c_wait_io(int bus)
+{
+    while (((IICSTAT(bus) & (1 << 5)) != 0) &&
+            ((IICSTA2(bus) & ((1 << 8)|(1 << 13))) == 0)) {
+        wait_rdy(bus);
+    }
+    IICSTA2(bus) |= (1 << 8)|(1 << 13);
+}
+
+static int i2c_start(int bus, unsigned char slave, bool rd)
+{
+    /* configure port */
+    wait_rdy(bus);
+    IICCON(bus) = (0 << 8) | /* INT_EN = disabled */
+                  (1 << 7) | /* ACK_GEN */
+                  (0 << 6) | /* CLKSEL = PCLK/32 (TBC) */
+                  (4 << 0);  /* CK_REG */
+
+    /* START */
+    wait_rdy(bus);
+    IICDS(bus) = slave | rd;
+    wait_rdy(bus);
+    IICSTAT(bus) = rd ? 0xB0 : 0xF0;
+
+    i2c_wait_io(bus);
+
+    /* check ACK */
+    if (IICSTAT(bus) & 1)
+        return 1;
+
+    return 0;
+}
+
+static void i2c_stop(int bus)
+{
+    /* STOP */
+    wait_rdy(bus);
+    IICSTAT(bus) &= ~0x20;
+    wait_rdy(bus);
+    IICCON(bus) = 0x10;
+    i2c_wait_io(bus);
+}
+
+static int i2c_wr_internal(int bus, unsigned char slave,
+                    int address, int len, const unsigned char *data)
+{
+    int rc = 0;
+
+    if (i2c_start(bus, slave, false) == 0)
+    {
+        /* write address + data */
+        const unsigned char *ptr = data;
+        const unsigned char addr = address;
+        if (address >= 0) {
+            ptr = &addr;
+            len++;
+        }
+        while (len--) {
+            wait_rdy(bus);
+            IICDS(bus) = *ptr;
+            udelay(5);
+            wait_rdy(bus);
+            IICCON(bus) = IICCON(bus);
+            i2c_wait_io(bus);
+            /* check ACK */
+            if (IICSTAT(bus) & 1) {
+                rc = 2;
+                break;
+            }
+            if (ptr == &addr) ptr = data;
+            else ptr++;
+        }
+    }
+    else
+        rc = 1;
+
+    i2c_stop(bus);
+    return rc;
+}
+
+static int i2c_rd_internal(int bus, unsigned char slave, int len, unsigned char *data)
+{
+    int rc = 0;
+
+    if (i2c_start(bus, slave, true) == 0)
+    {
+        while (len--) {
+            wait_rdy(bus);
+            IICCON(bus) &= ~(len ? 0 : 0x80); /* ACK or NAK */
+            i2c_wait_io(bus);
+            *data++ = IICDS(bus);
+        }
+    }
+    else
+        rc = 3;
+
+    i2c_stop(bus);
+    return rc;
+}
+
 int i2c_wr(int bus, unsigned char slave, int address, int len, const unsigned char *data)
 {
     i2c_on(bus);
-    long timeout = USEC_TIMER + 20000;
+    int rc = i2c_wr_internal(bus, slave, address, len, data);
-
-    /* START */
-    IICDS(bus) = slave & ~1;
-    IICSTAT(bus) = 0xF0;
-    while ((IICCON(bus) & 0x10) == 0)
-        if (TIME_AFTER(USEC_TIMER, timeout))
-            return 1;
-
-    if (address >= 0) {
-        /* write address */
-        IICDS(bus) = address;
-        IICCON(bus) = IICCON(bus);
-        while ((IICCON(bus) & 0x10) == 0)
-            if (TIME_AFTER(USEC_TIMER, timeout))
-                return 2;
-    }
-
-    /* write data */
-    while (len--) {
-        IICDS(bus) = *data++;
-        IICCON(bus) = IICCON(bus);
-        while ((IICCON(bus) & 0x10) == 0)
-            if (TIME_AFTER(USEC_TIMER, timeout))
-                return 4;
-    }
-
-    /* STOP */
-    IICSTAT(bus) = 0xD0;
-    IICCON(bus) = IICCON(bus);
-    while ((IICSTAT(bus) & (1 << 5)) != 0)
-        if (TIME_AFTER(USEC_TIMER, timeout))
-            return 5;
-
     i2c_off(bus);
-    return 0;
+    return rc;
 }
 
 int i2c_rd(int bus, unsigned char slave, int address, int len, unsigned char *data)
 {
     i2c_on(bus);
-    long timeout = USEC_TIMER + 20000;
+    int rc = i2c_wr_internal(bus, slave, address, 0, NULL);
-
+    if (rc == 0)
-    if (address >= 0) {
+        rc = i2c_rd_internal(bus, slave, len, data);
-        /* START */
-        IICDS(bus) = slave & ~1;
-        IICSTAT(bus) = 0xF0;
-        while ((IICCON(bus) & 0x10) == 0)
-            if (TIME_AFTER(USEC_TIMER, timeout))
-                return 1;
-
-        /* write address */
-        IICDS(bus) = address;
-        IICCON(bus) = IICCON(bus);
-        while ((IICCON(bus) & 0x10) == 0)
-            if (TIME_AFTER(USEC_TIMER, timeout))
-                return 2;
-    }
-
-    /* (repeated) START */
-    IICDS(bus) = slave | 1;
-    IICSTAT(bus) = 0xB0;
-    IICCON(bus) = IICCON(bus);
-    while ((IICCON(bus) & 0x10) == 0)
-        if (TIME_AFTER(USEC_TIMER, timeout))
-            return 3;
-
-    while (len--) {
-        IICCON(bus) &= ~(len ? 0 : 0x80); /* ACK or NAK */
-        while ((IICCON(bus) & 0x10) == 0)
-            if (TIME_AFTER(USEC_TIMER, timeout))
-                return 4;
-        *data++ = IICDS(bus);
-    }
-
-    /* STOP */
-    IICSTAT(bus) = 0x90;
-    IICCON(bus) = IICCON(bus);
-    while ((IICSTAT(bus) & (1 << 5)) != 0)
-        if (TIME_AFTER(USEC_TIMER, timeout))
-            return 5;
-
     i2c_off(bus);
-    return 0;
+    return rc;
 }
 
-unsigned long i2c_rd_err, i2c_wr_err;
-
 int i2c_write(int bus, unsigned char slave, int address, int len, const unsigned char *data)
 {
     int ret;
     mutex_lock(&i2c_mtx[bus]);
     ret = i2c_wr(bus, slave, address, len, data);
     mutex_unlock(&i2c_mtx[bus]);
-    if (ret) i2c_wr_err++;
     return ret;
 }
 
@@ -201,18 +202,14 @@ int i2c_read(int bus, unsigned char slave, int address, int len, unsigned char *
     mutex_lock(&i2c_mtx[bus]);
     ret = i2c_rd(bus, slave, address, len, data);
     mutex_unlock(&i2c_mtx[bus]);
-    if (ret) i2c_rd_err++;
     return ret;
 }
 
-static void wait_rdy(int bus)
-{
-    while (IICUNK10(bus));
-}
-
 void i2c_preinit(int bus)
 {
-    clockgate_enable(I2CCLKGATE(bus), true);
+    if (bus == 0) PCON3 = (PCON3 & ~0x00000ff0) | 0x00000220;
+    /* TBC: else if(bus == 1) PCON6 = (PCON6 & ~0x0ff00000) | 0x02200000; */
+    i2c_on(bus);
     wait_rdy(bus);
     IICADD(bus) = 0x40;   /* own slave address */
     wait_rdy(bus);
@@ -220,11 +217,10 @@ void i2c_preinit(int bus)
     wait_rdy(bus);
     IICUNK18(bus) = 0;
     wait_rdy(bus);
-    IICSTAT(bus) = 0x80;  /* master Rx mode, Tx/Rx off */
+    IICSTAT(bus) = 0x80;  /* master Rx mode, serial output off */
     wait_rdy(bus);
     IICCON(bus) = 0;
     wait_rdy(bus);
-    IICSTAT(bus) = 0;     /* slave Rx mode, Tx/Rx off */
+    IICSTA2(bus) = 0x3f00;
-    wait_rdy(bus);
+    i2c_off(bus);
-    clockgate_enable(I2CCLKGATE(bus), false);
 }