Re: mistake the i2c_bitbang direction

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

kiyohara%kk.iij4u.or.jp@localhost wrote:

> And, I will not understand the specification of i2c.

http://www.nxp.com/acrobat_download/literature/9398/39340011.pdf

> I think that the following correction is necessary for gpiic of 405Gp
> of imb4xx if his correction is correct.

It looks OPB I2C hardware is implemented properly and
it doesn't require DIR() ops at all, but it might still be
better to have compatibility with other implementation
which will hold SDA output value set during DIR() is INPUT.
(I'm not sure how the DIR() op was defined and designed though)

---

Index: arch/powerpc/ibm4xx/dev/gpiic_opb.c
===================================================================
RCS file: /cvsroot/src/sys/arch/powerpc/ibm4xx/dev/gpiic_opb.c,v
retrieving revision 1.5
diff -u -r1.5 gpiic_opb.c
--- arch/powerpc/ibm4xx/dev/gpiic_opb.c 6 Dec 2007 17:00:33 -0000       1.5
+++ arch/powerpc/ibm4xx/dev/gpiic_opb.c 8 Jul 2008 17:55:52 -0000
@@ -55,6 +55,7 @@
        bus_space_tag_t sc_bust;
        bus_space_handle_t sc_bush;
        uint8_t sc_txen;
+       uint8_t sc_tx;
        struct i2c_controller sc_i2c;
        struct i2c_bitbang_ops sc_bops;
        kmutex_t sc_buslock;
@@ -105,6 +106,7 @@
        mutex_init(&sc->sc_buslock, MUTEX_DEFAULT, IPL_NONE);
 
        sc->sc_txen = 0;
+       sc->sc_tx = IIC_DIRECTCNTL_SCC | IIC_DIRECTCNTL_SDAC;
        sc->sc_i2c.ic_cookie = sc;
        sc->sc_i2c.ic_acquire_bus = gpiic_acquire_bus;
        sc->sc_i2c.ic_release_bus = gpiic_release_bus;
@@ -204,8 +206,18 @@
 gpiic_set_dir(void *arg, uint32_t bits)
 {
        struct gpiic_softc *sc = arg;
+       uint8_t tx, txen;
 
-       sc->sc_txen = (uint8_t)bits;
+       txen = (uint8_t)bits;
+       if (sc->sc_txen == txen)
+               return;
+
+       sc->sc_txen = txen;
+
+       tx = sc->sc_tx;
+       if (sc->sc_txen == 0)
+               tx |= IIC_DIRECTCNTL_SDAC;
+       bus_space_write_1(sc->sc_bust, sc->sc_bush, IIC_DIRECTCNTL, tx);
 }
 
 static void
@@ -213,10 +225,9 @@
 {
        struct gpiic_softc *sc = arg;
 
-       if (sc->sc_txen == 0 && (bits & IIC_DIRECTCNTL_SDAC) == 0) {
-               printf("gpiic_set_bits: SDA low with no output enable\n");
+       sc->sc_tx = (uint8_t)bits;
+       if (sc->sc_txen == 0)
                bits |= IIC_DIRECTCNTL_SDAC;
-       }
 
        bus_space_write_1(sc->sc_bust, sc->sc_bush, IIC_DIRECTCNTL, bits);
 }
@@ -227,9 +238,6 @@
        struct gpiic_softc *sc = arg;
        uint8_t rv;
 
-       if (sc->sc_txen != 0)
-               printf("gpiic_read_bits: Read in output mode\n");
-
        rv = bus_space_read_1(sc->sc_bust, sc->sc_bush, IIC_DIRECTCNTL) << 2;
        rv &= (IIC_DIRECTCNTL_SCC | IIC_DIRECTCNTL_SDAC);
 
---
Izumi Tsutsui