Correct shortcomings in imxiic(4) that occurred during the initial

development in 2013.  Registers are to be accessed byte-wise.  The
command buffer, used to transfer the register index we want to access
on the chip, is not only used on write operations, but also on read
operations.  Thus it has to be pulled into the main i2c exec method.
Since these are two transfers, we have to set "repeat start" and wait
for busy to be set.  Some machines have a clock-frequency attribute on
the controller node which controls the speed (e.g. 400 kHz instead of
100 kHz).

ok kettenis@

1 files changed, 66 insertions, 64 deletions

diff --git a/sys/dev/fdt/imxiic.c b/sys/dev/fdt/imxiic.c
index e2dd4528775..80e92b1a672 100644
--- a/sys/dev/fdt/imxiic.c
+++ b/sys/dev/fdt/imxiic.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: imxiic.c,v 1.1 2018/03/30 20:02:28 patrick Exp $ */
+/* $OpenBSD: imxiic.c,v 1.2 2018/05/16 10:36:28 patrick Exp $ */
 /*
  * Copyright (c) 2013 Patrick Wildt <patrick@blueri.se>
  *
@@ -54,6 +54,7 @@ struct imxiic_softc {
 	bus_size_t		sc_ios;
 	void			*sc_ih;
 	int			sc_node;
+	int			sc_bitrate;
 
 	struct rwlock		sc_buslock;
 	struct i2c_controller	i2c_tag;
@@ -71,22 +72,21 @@ 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_read(struct imxiic_softc *, int, void *, int);
-int imxiic_write(struct imxiic_softc *, int, const void *, int,
-    const void *, int);
+int imxiic_write(struct imxiic_softc *, int, const void *, int);
 
 int imxiic_i2c_acquire_bus(void *, int);
 void imxiic_i2c_release_bus(void *, int);
 int imxiic_i2c_exec(void *, i2c_op_t, i2c_addr_t, const void *, size_t,
     void *, size_t, int);
 
-#define HREAD2(sc, reg)							\
-	(bus_space_read_2((sc)->sc_iot, (sc)->sc_ioh, (reg)))
-#define HWRITE2(sc, reg, val)						\
-	bus_space_write_2((sc)->sc_iot, (sc)->sc_ioh, (reg), (val))
-#define HSET2(sc, reg, bits)						\
-	HWRITE2((sc), (reg), HREAD2((sc), (reg)) | (bits))
-#define HCLR2(sc, reg, bits)						\
-	HWRITE2((sc), (reg), HREAD2((sc), (reg)) & ~(bits))
+#define HREAD1(sc, reg)							\
+	(bus_space_read_1((sc)->sc_iot, (sc)->sc_ioh, (reg)))
+#define HWRITE1(sc, reg, val)						\
+	bus_space_write_1((sc)->sc_iot, (sc)->sc_ioh, (reg), (val))
+#define HSET1(sc, reg, bits)						\
+	HWRITE1((sc), (reg), HREAD1((sc), (reg)) | (bits))
+#define HCLR1(sc, reg, bits)						\
+	HWRITE1((sc), (reg), HREAD1((sc), (reg)) & ~(bits))
 
 void imxiic_scan(struct device *, struct i2cbus_attach_args *, void *);
 
@@ -133,12 +133,14 @@ imxiic_attach(struct device *parent, struct device *self, void *aux)
 	/* set iomux pins */
 	pinctrl_byname(faa->fa_node, "default");
 
-	/* set speed to 100kHz */
-	imxiic_setspeed(sc, 100);
+	/* set speed */
+	sc->sc_bitrate = OF_getpropint(sc->sc_node,
+	    "clock-frequency", 100000) / 1000;
+	imxiic_setspeed(sc, sc->sc_bitrate);
 
 	/* reset */
-	HWRITE2(sc, I2C_I2CR, 0);
-	HWRITE2(sc, I2C_I2SR, 0);
+	HWRITE1(sc, I2C_I2CR, 0);
+	HWRITE1(sc, I2C_I2SR, 0);
 
 	sc->stopped = 1;
 	rw_init(&sc->sc_buslock, sc->sc_dev.dv_xname);
@@ -178,7 +180,7 @@ imxiic_setspeed(struct imxiic_softc *sc, u_int speed)
 		sc->frequency = imxiic_clk_div[i][1];
 	}
 
-	HWRITE2(sc, I2C_IFDR, sc->frequency);
+	HWRITE1(sc, I2C_IFDR, sc->frequency);
 }
 
 #if 0
@@ -188,12 +190,12 @@ imxiic_intr(void *arg)
 	struct imxiic_softc *sc = arg;
 	u_int16_t status;
 
-	status = HREAD2(sc, I2C_I2SR);
+	status = HREAD1(sc, I2C_I2SR);
 
 	if (ISSET(status, I2C_I2SR_IIF)) {
 		/* acknowledge the interrupts */
-		HWRITE2(sc, I2C_I2SR,
-		    HREAD2(sc, I2C_I2SR) & ~I2C_I2SR_IIF);
+		HWRITE1(sc, I2C_I2SR,
+		    HREAD1(sc, I2C_I2SR) & ~I2C_I2SR_IIF);
 
 		sc->intr_status |= status;
 		wakeup(&sc->intr_status);
@@ -231,9 +233,9 @@ imxiic_wait_state(struct imxiic_softc *sc, uint32_t mask, uint32_t value)
 {
 	uint32_t state;
 	int timeout;
-	state = HREAD2(sc, I2C_I2SR);
+	state = HREAD1(sc, I2C_I2SR);
 	for (timeout = 1000; timeout > 0; timeout--) {
-		if (((state = HREAD2(sc, I2C_I2SR)) & mask) == value)
+		if (((state = HREAD1(sc, I2C_I2SR)) & mask) == value)
 			return 0;
 		delay(10);
 	}
@@ -245,68 +247,58 @@ imxiic_read(struct imxiic_softc *sc, int addr, void *data, int len)
 {
 	int i;
 
-	HWRITE2(sc, I2C_I2DR, (addr << 1) | 1);
+	HWRITE1(sc, I2C_I2DR, (addr << 1) | 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)
+	HCLR1(sc, I2C_I2SR, I2C_I2SR_IIF);
+	if (HREAD1(sc, I2C_I2SR) & I2C_I2SR_RXAK)
 		return (EIO);
 
-	HCLR2(sc, I2C_I2CR, I2C_I2CR_MTX);
+	HCLR1(sc, I2C_I2CR, I2C_I2CR_MTX);
 	if (len - 1)
-		HCLR2(sc, I2C_I2CR, I2C_I2CR_TXAK);
+		HCLR1(sc, I2C_I2CR, I2C_I2CR_TXAK);
 
 	/* dummy read */
-	HREAD2(sc, I2C_I2DR);
+	HREAD1(sc, I2C_I2DR);
 
 	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);
+		HCLR1(sc, I2C_I2SR, I2C_I2SR_IIF);
 
 		if (i == (len - 1)) {
-			HCLR2(sc, I2C_I2CR, I2C_I2CR_MSTA | I2C_I2CR_MTX);
+			HCLR1(sc, I2C_I2CR, I2C_I2CR_MSTA | I2C_I2CR_MTX);
 			imxiic_wait_state(sc, I2C_I2SR_IBB, 0);
 			sc->stopped = 1;
 		} else if (i == (len - 2)) {
-			HSET2(sc, I2C_I2CR, I2C_I2CR_TXAK);
+			HSET1(sc, I2C_I2CR, I2C_I2CR_TXAK);
 		}
-		((uint8_t*)data)[i] = HREAD2(sc, I2C_I2DR);
+		((uint8_t*)data)[i] = HREAD1(sc, I2C_I2DR);
 	}
 
 	return 0;
 }
 
 int
-imxiic_write(struct imxiic_softc *sc, int addr, const void *cmd, int cmdlen,
-    const void *data, int len)
+imxiic_write(struct imxiic_softc *sc, int addr, const void *data, int len)
 {
 	int i;
 
-	HWRITE2(sc, I2C_I2DR, addr << 1);
+	HWRITE1(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)
+	HCLR1(sc, I2C_I2SR, I2C_I2SR_IIF);
+	if (HREAD1(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]);
+		HWRITE1(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)
+		HCLR1(sc, I2C_I2SR, I2C_I2SR_IIF);
+		if (HREAD1(sc, I2C_I2SR) & I2C_I2SR_RXAK)
 			return (EIO);
 	}
 	return 0;
@@ -320,14 +312,14 @@ imxiic_i2c_acquire_bus(void *cookie, int flags)
 	rw_enter(&sc->sc_buslock, RW_WRITE);
 
 	/* clock gating */
-	clock_enable(sc->sc_node, NULL);
+	clock_enable_all(sc->sc_node);
 
-	/* set speed to 100kHz */
-	imxiic_setspeed(sc, 100);
+	/* set speed */
+	imxiic_setspeed(sc, sc->sc_bitrate);
 
 	/* enable the controller */
-	HWRITE2(sc, I2C_I2SR, 0);
-	HWRITE2(sc, I2C_I2CR, I2C_I2CR_IEN);
+	HWRITE1(sc, I2C_I2SR, 0);
+	HWRITE1(sc, I2C_I2CR, I2C_I2CR_IEN);
 
 	/* wait for it to be stable */
 	delay(50);
@@ -340,7 +332,7 @@ imxiic_i2c_release_bus(void *cookie, int flags)
 {
 	struct imxiic_softc *sc = cookie;
 
-	HWRITE2(sc, I2C_I2CR, 0);
+	HWRITE1(sc, I2C_I2CR, 0);
 
 	rw_exit(&sc->sc_buslock);
 }
@@ -354,11 +346,9 @@ imxiic_i2c_exec(void *cookie, i2c_op_t op, i2c_addr_t addr,
 
 	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);
+	HSET1(sc, I2C_I2CR, I2C_I2CR_MSTA);
 
 	if (imxiic_wait_state(sc, I2C_I2SR_IBB, I2C_I2SR_IBB)) {
 		ret = EIO;
@@ -367,17 +357,29 @@ imxiic_i2c_exec(void *cookie, i2c_op_t op, i2c_addr_t addr,
 
 	sc->stopped = 0;
 
-	HSET2(sc, I2C_I2CR, I2C_I2CR_IIEN | I2C_I2CR_MTX | I2C_I2CR_TXAK);
+	HSET1(sc, I2C_I2CR, I2C_I2CR_IIEN | I2C_I2CR_MTX | I2C_I2CR_TXAK);
+
+	if (cmdbuf) {
+		ret = imxiic_write(sc, addr, cmdbuf, cmdlen);
+		if (ret)
+			goto fail;
+
+		HSET1(sc, I2C_I2CR, I2C_I2CR_RSTA);
+		if (imxiic_wait_state(sc, I2C_I2SR_IBB, I2C_I2SR_IBB)) {
+			ret = EIO;
+			goto fail;
+		}
+	}
 
 	if (I2C_OP_READ_P(op)) {
 		ret = imxiic_read(sc, addr, buf, len);
 	} else {
-		ret = imxiic_write(sc, addr, cmdbuf, cmdlen, buf, len);
+		ret = imxiic_write(sc, addr, buf, len);
 	}
 
 fail:
 	if (!sc->stopped) {
-		HCLR2(sc, I2C_I2CR, I2C_I2CR_MSTA | I2C_I2CR_MTX);
+		HCLR1(sc, I2C_I2CR, I2C_I2CR_MSTA | I2C_I2CR_MTX);
 		imxiic_wait_state(sc, I2C_I2SR_IBB, 0);
 		sc->stopped = 1;
 	}
@@ -390,10 +392,10 @@ imxiic_detach(struct device *self, int flags)
 {
 	struct imxiic_softc *sc = (struct imxiic_softc *)self;
 
-	HWRITE2(sc, I2C_IADR, 0);
-	HWRITE2(sc, I2C_IFDR, 0);
-	HWRITE2(sc, I2C_I2CR, 0);
-	HWRITE2(sc, I2C_I2SR, 0);
+	HWRITE1(sc, I2C_IADR, 0);
+	HWRITE1(sc, I2C_IFDR, 0);
+	HWRITE1(sc, I2C_I2CR, 0);
+	HWRITE1(sc, I2C_I2SR, 0);
 
 	bus_space_unmap(sc->sc_iot, sc->sc_ioh, sc->sc_ios);
 	return 0;