Fix coldfire PCF50606 I2C driver, and iriver FM radio I2C driver. Both drivers had wrong timing, discovered while experimenting with buffered writes.

git-svn-id: svn://svn.rockbox.org/rockbox/trunk@15122 a1c6a512-1295-4272-9138-f99709370657

2 files changed, 60 insertions, 63 deletions

diff --git a/firmware/drivers/fmradio_i2c.c b/firmware/drivers/fmradio_i2c.c
index 0a1d0c1fd7..9f890ca34a 100644
--- a/firmware/drivers/fmradio_i2c.c
+++ b/firmware/drivers/fmradio_i2c.c
@@ -49,11 +49,10 @@ int fmradio_i2c_read(unsigned char address, unsigned char* buf, int count)
 #ifdef IRIVER_H300_SERIES
 
 /* SDA is GPIO57 */
-#define SDA_LO     and_l(~0x02000000, &GPIO1_OUT)
-#define SDA_HI      or_l( 0x02000000, &GPIO1_OUT)
-#define SDA_INPUT  and_l(~0x02000000, &GPIO1_ENABLE)
-#define SDA_OUTPUT  or_l( 0x02000000, &GPIO1_ENABLE)
-#define SDA             ( 0x02000000 & GPIO1_READ)
+#define SDA_OUTINIT and_l(~0x02000000, &GPIO1_OUT)
+#define SDA_HI_IN   and_l(~0x02000000, &GPIO1_ENABLE)
+#define SDA_LO_OUT   or_l( 0x02000000, &GPIO1_ENABLE)
+#define SDA              ( 0x02000000 & GPIO1_READ)
 
 /* SCL is GPIO56 */
 #define SCL_INPUT  and_l(~0x01000000, &GPIO1_ENABLE)
@@ -65,11 +64,10 @@ int fmradio_i2c_read(unsigned char address, unsigned char* buf, int count)
 #else
 
 /* SDA is GPIO55 */
-#define SDA_LO     and_l(~0x00800000, &GPIO1_OUT)
-#define SDA_HI      or_l( 0x00800000, &GPIO1_OUT)
-#define SDA_INPUT  and_l(~0x00800000, &GPIO1_ENABLE)
-#define SDA_OUTPUT  or_l( 0x00800000, &GPIO1_ENABLE)
-#define SDA             ( 0x00800000 & GPIO1_READ)
+#define SDA_OUTINIT and_l(~0x00800000, &GPIO1_OUT)
+#define SDA_HI_IN   and_l(~0x00800000, &GPIO1_ENABLE)
+#define SDA_LO_OUT   or_l( 0x00800000, &GPIO1_ENABLE)
+#define SDA              ( 0x00800000 & GPIO1_READ)
 
 /* SCL is GPIO3 */
 #define SCL_INPUT  and_l(~0x00000008, &GPIO_ENABLE)
@@ -94,22 +92,23 @@ int fmradio_i2c_read(unsigned char address, unsigned char* buf, int count)
 
 static void fmradio_i2c_start(void)
 {
-    SDA_OUTPUT;
-    SCL_OUTPUT;
-    SDA_HI;
     SCL_HI;
+    SCL_OUTPUT;
+    SDA_HI_IN;
+    SDA_OUTINIT;
     DELAY;
-    SDA_LO;
+    SDA_LO_OUT;
     DELAY;
     SCL_LO;
 }
 
 static void fmradio_i2c_stop(void)
 {
-   SDA_LO;
+   SDA_LO_OUT;
+   DELAY;
    SCL_HI;
    DELAY;
-   SDA_HI;
+   SDA_HI_IN;
 }
 
 /* Generate ACK or NACK */
@@ -123,23 +122,21 @@ static void fmradio_i2c_ack(bool nack)
        In their infinite wisdom, iriver didn't pull up the SCL line, so
        we have to drive the SCL high repeatedly to simulate a pullup. */
     
-    SCL_LO;      /* Set the clock low */
-
     if (nack)
-        SDA_HI;
+        SDA_HI_IN;
     else
-        SDA_LO;
-    
-    SCL_INPUT;   /* Set the clock to input */
-    while(!SCL)  /* and wait for the slave to release it */
+        SDA_LO_OUT;
+    DELAY;
+
+    SCL_HI;
+    do
     {
-        SCL_HI;
         SCL_OUTPUT;  /* Set the clock to output */
         SCL_INPUT;   /* Set the clock to input */
         DELAY;
     }
+    while(!SCL);  /* and wait for the slave to release it */
 
-    DELAY;
     SCL_OUTPUT;
     SCL_LO;
 }
@@ -156,24 +153,23 @@ static int fmradio_i2c_getack(void)
        In their infinite wisdom, iriver didn't pull up the SCL line, so
        we have to drive the SCL high repeatedly to simulate a pullup. */
 
-    SDA_INPUT;   /* And set to input */
-    SCL_INPUT;   /* Set the clock to input */
-    while(!SCL)  /* and wait for the slave to release it */
+    SDA_HI_IN;
+    DELAY;
+
+    SCL_HI;          /* set clock to high */
+    do
     {
-        SCL_HI;
         SCL_OUTPUT;  /* Set the clock to output */
         SCL_INPUT;   /* Set the clock to input */
         DELAY;
     }
+    while(!SCL);     /* and wait for the slave to release it */
 
     if (SDA)
-        /* ack failed */
-        ret = 0;
+        ret = 0;    /* ack failed */
     
     SCL_OUTPUT;
     SCL_LO;
-    SDA_HI;
-    SDA_OUTPUT;
 
     return ret;
 }
@@ -185,20 +181,14 @@ static void fmradio_i2c_outb(unsigned char byte)
    /* clock out each bit, MSB first */
    for ( i=0x80; i; i>>=1 ) {
       if ( i & byte )
-      {
-         SDA_HI;
-      }
+         SDA_HI_IN;
       else
-      {
-         SDA_LO;
-      }
+         SDA_LO_OUT;
       DELAY;
       SCL_HI;
       DELAY;
       SCL_LO;
    }
-
-   SDA_HI;
 }
 
 static unsigned char fmradio_i2c_inb(void)
@@ -206,18 +196,15 @@ static unsigned char fmradio_i2c_inb(void)
    int i;
    unsigned char byte = 0;
 
+   SDA_HI_IN;
    /* clock in each bit, MSB first */
    for ( i=0x80; i; i>>=1 ) {
-       SDA_INPUT;   /* And set to input */
        DELAY;
+       SCL_HI;
        DELAY;
        if ( SDA )
            byte |= i;
-       SCL_HI;
-       DELAY;
        SCL_LO;
-       DELAY;
-       SDA_OUTPUT;
    }
 
    return byte;
@@ -278,14 +265,14 @@ int fmradio_i2c_read(int address, unsigned char* buf, int count)
 #define SDA_HI     or_b(0x10, &PBDRL)
 #define SDA_INPUT  and_b(~0x10, &PBIORL)
 #define SDA_OUTPUT or_b(0x10, &PBIORL)
-#define SDA     (PBDR & 0x0010)
+#define SDA        (PBDR & 0x0010)
 
 /* SCL is PB1 */
 #define SCL_INPUT  and_b(~0x02, &PBIORL)
 #define SCL_OUTPUT or_b(0x02, &PBIORL)
 #define SCL_LO     and_b(~0x02, &PBDRL)
 #define SCL_HI     or_b(0x02, &PBDRL)
-#define SCL     (PBDR & 0x0002)
+#define SCL        (PBDR & 0x0002)
 
 /* arbitrary delay loop */
 #define DELAY   do { int _x; for(_x=0;_x<20;_x++);} while (0)
diff --git a/firmware/target/coldfire/pcf50606-coldfire.c b/firmware/target/coldfire/pcf50606-coldfire.c
index d777b41cb1..d57b249f0d 100644
--- a/firmware/target/coldfire/pcf50606-coldfire.c
+++ b/firmware/target/coldfire/pcf50606-coldfire.c
@@ -140,6 +140,11 @@ inline void pcf50606_i2c_stop(void)
     asm (
         "or.l    %[sdab],(8,%[sdard]) \n" /* SDA_LO_OUT */
 
+        "move.l  %[dly],%%d0          \n" /* DELAY */
+    "1:                               \n"
+        "subq.l  #1,%%d0              \n"
+        "bhi.s   1b                   \n"
+
         "not.l   %[sclb]              \n" /* SCL_HI_IN */
         "and.l   %[sclb],(8,%[sclrd]) \n"
         "not.l   %[sclb]              \n"
@@ -169,6 +174,7 @@ inline void pcf50606_i2c_stop(void)
     );
 #else
     SDA_LO_OUT;
+    DELAY;
     SCL_HI_IN;
     DELAY;
     SDA_HI_IN;
@@ -189,6 +195,11 @@ inline void pcf50606_i2c_ack(bool ack)
     "1:                               \n"
         "or.l    %[sdab],(8,%[sdard]) \n" /*   SDA_LO_OUT */
 
+        "move.l  %[dly],%%d0          \n" /* DELAY */
+    "1:                               \n"
+        "subq.l  #1,%%d0              \n"
+        "bhi.s   1b                   \n"
+
         "not.l   %[sclb]              \n" /* SCL_HI_IN */
         "and.l   %[sclb],(8,%[sclrd]) \n"
         "not.l   %[sclb]              \n"
@@ -220,9 +231,8 @@ inline void pcf50606_i2c_ack(bool ack)
         SDA_LO_OUT;
     else
         SDA_HI_IN;
-
+    DELAY;
     SCL_HI_IN;
-
     DELAY;
     SCL_LO_OUT;
 #endif
@@ -251,16 +261,16 @@ inline bool pcf50606_i2c_getack(void)
         "btst.l  %[sclbnum],%%d0      \n"
         "beq.s   1b                   \n"
 
+        "move.l  %[dly],%%d0          \n" /* DELAY */
+    "1:                               \n"
+        "subq.l  #1,%%d0              \n"
+        "bhi.s   1b                   \n"
+
         "move.l  (%[sdard]),%%d0      \n" /* ret = !SDA */
         "btst.l  %[sdabnum],%%d0      \n"
         "seq.b   %[ret]               \n"
 
         "or.l    %[sclb],(8,%[sclrd]) \n" /* SCL_LO_OUT */
-
-        "move.l  %[dly],%%d0          \n" /* DELAY */
-    "1:                               \n"
-        "subq.l  #1,%%d0              \n"
-        "bhi.s   1b                   \n"
         : /* outputs */
         [ret]   "=&d"(ret)
         : /* inputs */
@@ -278,11 +288,11 @@ inline bool pcf50606_i2c_getack(void)
     SDA_HI_IN;
     DELAY;
     SCL_HI_IN;
+    DELAY;
 
     ret = !SDA;
-
+               
     SCL_LO_OUT;
-    DELAY;
 #endif
     return ret;
 }
@@ -372,6 +382,11 @@ unsigned char pcf50606_i2c_inb(bool ack)
         "clr.l   %[byte]              \n" /* byte = 0 */
 
     "2:                               \n" /* do */
+        "move.l  %[dly],%%d0          \n" /* DELAY */
+    "1:                               \n"
+        "subq.l  #1,%%d0              \n"
+        "bhi.s   1b                   \n"
+
         "not.l   %[sclb]              \n" /* SCL_HI_IN */
         "and.l   %[sclb],(8,%[sclrd]) \n"
         "not.l   %[sclb]              \n"
@@ -394,11 +409,6 @@ unsigned char pcf50606_i2c_inb(bool ack)
 
         "or.l    %[sclb],(8,%[sclrd]) \n" /* SCL_LO_OUT */
 
-        "move.l  %[dly],%%d0          \n" /* DELAY */
-    "1:                               \n"
-        "subq.l  #1,%%d0              \n"
-        "bhi.s   1b                   \n"
-
         "subq.l  #1,%%d1              \n" /* i-- */
         "bne.s   2b                   \n" /* while (i != 0) */
         : /* outputs */
@@ -421,12 +431,12 @@ unsigned char pcf50606_i2c_inb(bool ack)
     SDA_HI_IN;
     for ( i=0x80; i; i>>=1 )
     {
+        DELAY;
         SCL_HI_IN;
         DELAY;
         if ( SDA )
             byte |= i;
         SCL_LO_OUT;
-        DELAY;
     }
 #endif