Make this actually work. This makes the following changes:

- Move most of the bus setup stuff to imxiic_i2c_acquire_bus()
- Move the teardown to imxiic_i2c_release_bus()
- Always clear the IIF flag
- Allow cmd and data for write operations.

Some i2c operations are still rejected, but this works well enough to use
pcfrtc(4).

ok patrick@

1 files changed, 51 insertions, 41 deletions

diff --git a/sys/arch/armv7/imx/imxiic.c b/sys/arch/armv7/imx/imxiic.c
index ab5fd06f3ce..0976e1a0b37 100644
--- a/sys/arch/armv7/imx/imxiic.c
+++ b/sys/arch/armv7/imx/imxiic.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: imxiic.c,v 1.2 2013/11/06 19:03:07 syl Exp $ */
+/* $OpenBSD: imxiic.c,v 1.3 2016/05/16 21:38:35 kettenis Exp $ */
 /*
  * Copyright (c) 2013 Patrick Wildt <patrick@blueri.se>
  *
@@ -67,9 +67,9 @@ void imxiic_setspeed(struct imxiic_softc *, u_int);
 int imxiic_intr(void *);
 int imxiic_wait_intr(struct imxiic_softc *, int, int);
 int imxiic_wait_state(struct imxiic_softc *, uint32_t, uint32_t);
-int imxiic_start(struct imxiic_softc *, int, int, void *, int);
-int imxiic_read(struct imxiic_softc *, int, int, void *, int);
-int imxiic_write(struct imxiic_softc *, int, int, const void *, int);
+int imxiic_read(struct imxiic_softc *, int, void *, int);
+int imxiic_write(struct imxiic_softc *, int, const void *, int,
+    const void *, int);
 
 int imxiic_i2c_acquire_bus(void *, int);
 void imxiic_i2c_release_bus(void *, int);
@@ -223,20 +223,20 @@ imxiic_wait_state(struct imxiic_softc *sc, uint32_t mask, uint32_t value)
 }
 
 int
-imxiic_read(struct imxiic_softc *sc, int addr, int subaddr, void *data, int len)
+imxiic_read(struct imxiic_softc *sc, int addr, void *data, int len)
 {
 	int i;
 
-	HWRITE2(sc, I2C_I2DR, addr | 1);
+	HWRITE2(sc, I2C_I2DR, (addr << 1) | 1);
 
 	if (imxiic_wait_state(sc, I2C_I2SR_IIF, I2C_I2SR_IIF))
 		return (EIO);
-	while(!(HREAD2(sc, I2C_I2SR) & I2C_I2SR_IIF));
+	HCLR2(sc, I2C_I2SR, I2C_I2SR_IIF);
 	if (HREAD2(sc, I2C_I2SR) & I2C_I2SR_RXAK)
 		return (EIO);
 
 	HCLR2(sc, I2C_I2CR, I2C_I2CR_MTX);
-	if (len)
+	if (len - 1)
 		HCLR2(sc, I2C_I2CR, I2C_I2CR_TXAK);
 
 	/* dummy read */
@@ -245,6 +245,8 @@ imxiic_read(struct imxiic_softc *sc, int addr, int subaddr, void *data, int len)
 	for (i = 0; i < len; i++) {
 		if (imxiic_wait_state(sc, I2C_I2SR_IIF, I2C_I2SR_IIF))
 			return (EIO);
+		HCLR2(sc, I2C_I2SR, I2C_I2SR_IIF);
+
 		if (i == (len - 1)) {
 			HCLR2(sc, I2C_I2CR, I2C_I2CR_MSTA | I2C_I2CR_MTX);
 			imxiic_wait_state(sc, I2C_I2SR_IBB, 0);
@@ -259,21 +261,33 @@ imxiic_read(struct imxiic_softc *sc, int addr, int subaddr, void *data, int len)
 }
 
 int
-imxiic_write(struct imxiic_softc *sc, int addr, int subaddr, const void *data, int len)
+imxiic_write(struct imxiic_softc *sc, int addr, const void *cmd, int cmdlen,
+    const void *data, int len)
 {
 	int i;
 
-	HWRITE2(sc, I2C_I2DR, addr);
+	HWRITE2(sc, I2C_I2DR, addr << 1);
 
 	if (imxiic_wait_state(sc, I2C_I2SR_IIF, I2C_I2SR_IIF))
 		return (EIO);
+	HCLR2(sc, I2C_I2SR, I2C_I2SR_IIF);
 	if (HREAD2(sc, I2C_I2SR) & I2C_I2SR_RXAK)
 		return (EIO);
 
+	for (i = 0; i < cmdlen; i++) {
+		HWRITE2(sc, I2C_I2DR, ((uint8_t*)cmd)[i]);
+		if (imxiic_wait_state(sc, I2C_I2SR_IIF, I2C_I2SR_IIF))
+			return (EIO);
+		HCLR2(sc, I2C_I2SR, I2C_I2SR_IIF);
+		if (HREAD2(sc, I2C_I2SR) & I2C_I2SR_RXAK)
+			return (EIO);
+	}
+
 	for (i = 0; i < len; i++) {
 		HWRITE2(sc, I2C_I2DR, ((uint8_t*)data)[i]);
 		if (imxiic_wait_state(sc, I2C_I2SR_IIF, I2C_I2SR_IIF))
 			return (EIO);
+		HCLR2(sc, I2C_I2SR, I2C_I2SR_IIF);
 		if (HREAD2(sc, I2C_I2SR) & I2C_I2SR_RXAK)
 			return (EIO);
 	}
@@ -285,7 +299,22 @@ imxiic_i2c_acquire_bus(void *cookie, int flags)
 {
 	struct imxiic_softc *sc = cookie;
 
-	return (rw_enter(&sc->sc_buslock, RW_WRITE));
+	rw_enter(&sc->sc_buslock, RW_WRITE);
+
+	/* clock gating */
+	imxccm_enable_i2c(sc->unit);
+
+	/* set speed to 100kHz */
+	imxiic_setspeed(sc, 100);
+
+	/* enable the controller */
+	HWRITE2(sc, I2C_I2SR, 0);
+	HWRITE2(sc, I2C_I2CR, I2C_I2CR_IEN);
+
+	/* wait for it to be stable */
+	delay(50);
+
+	return 0;
 }
 
 void
@@ -293,7 +322,9 @@ imxiic_i2c_release_bus(void *cookie, int flags)
 {
 	struct imxiic_softc *sc = cookie;
 
-	(void) rw_exit(&sc->sc_buslock);
+	HWRITE2(sc, I2C_I2CR, 0);
+
+	rw_exit(&sc->sc_buslock);
 }
 
 int
@@ -301,35 +332,18 @@ imxiic_i2c_exec(void *cookie, i2c_op_t op, i2c_addr_t addr,
     const void *cmdbuf, size_t cmdlen, void *buf, size_t len, int flags)
 {
 	struct imxiic_softc *sc = cookie;
-	uint32_t ret = 0;
-	u_int8_t cmd = 0;
-
-	if (!I2C_OP_STOP_P(op) || cmdlen > 1)
-		return (EINVAL);
-
-	if (cmdlen > 0)
-		cmd = *(u_int8_t *)cmdbuf;
-
-	addr &= 0x7f;
-
-	/* clock gating */
-	imxccm_enable_i2c(sc->unit);
-
-	/* set speed to 100kHz */
-	imxiic_setspeed(sc, 100);
-
-	/* enable the controller */
-	HWRITE2(sc, I2C_I2SR, 0);
-	HWRITE2(sc, I2C_I2CR, I2C_I2CR_IEN);
+	int ret = 0;
 
-	/* wait for it to be stable */
-	delay(50);
+	if (!I2C_OP_STOP_P(op))
+		return EINVAL;
+	if (I2C_OP_READ_P(op) && cmdlen > 0)
+		return EINVAL;
 
 	/* start transaction */
 	HSET2(sc, I2C_I2CR, I2C_I2CR_MSTA);
 
 	if (imxiic_wait_state(sc, I2C_I2SR_IBB, I2C_I2SR_IBB)) {
-		ret = (EIO);
+		ret = EIO;
 		goto fail;
 	}
 
@@ -338,11 +352,9 @@ imxiic_i2c_exec(void *cookie, i2c_op_t op, i2c_addr_t addr,
 	HSET2(sc, I2C_I2CR, I2C_I2CR_IIEN | I2C_I2CR_MTX | I2C_I2CR_TXAK);
 
 	if (I2C_OP_READ_P(op)) {
-		if (imxiic_read(sc, (addr << 1), cmd, buf, len) != 0)
-			ret = (EIO);
+		ret = imxiic_read(sc, addr, buf, len);
 	} else {
-		if (imxiic_write(sc, (addr << 1), cmd, buf, len) != 0)
-			ret = (EIO);
+		ret = imxiic_write(sc, addr, cmdbuf, cmdlen, buf, len);
 	}
 
 fail:
@@ -352,8 +364,6 @@ fail:
 		sc->stopped = 1;
 	}
 
-	HWRITE2(sc, I2C_I2CR, 0);
-
 	return ret;
 }