[src/trunk]: src/sys/dev/i2c - Don't use I2C_F_POLL.

[thorpej <thorpej%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/3ee20f30a083
branches:  trunk
changeset: 967847:3ee20f30a083
user:      thorpej <thorpej%NetBSD.org@localhost>
date:      Mon Dec 23 20:38:08 2019 +0000

description:
- Don't use I2C_F_POLL.
- Don't access the i2c from hard interrupt context.  Instead, schedule a
  soft interrupt to do the real work.  (No need to mask the interrupt
  source, since this device has an edge-sensitive interrupt per the DT
  info; will need to be revisited if there's ever a flavor that uses a
  level-sensitive interrupt).
- Split out the i2c bus acquire / release from the register read / write
  functions, allowing us to batch several i2c transactions under a single
  acquire / release cycle.

diffstat:

 sys/dev/i2c/tcakp.c |  104 ++++++++++++++++++++++++++++++++++++++++++---------
 1 files changed, 85 insertions(+), 19 deletions(-)

diffs (230 lines):

diff -r 92e2620fcbee -r 3ee20f30a083 sys/dev/i2c/tcakp.c
--- a/sys/dev/i2c/tcakp.c       Mon Dec 23 20:17:33 2019 +0000
+++ b/sys/dev/i2c/tcakp.c       Mon Dec 23 20:38:08 2019 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: tcakp.c,v 1.10 2018/10/17 16:56:40 jmcneill Exp $ */
+/* $NetBSD: tcakp.c,v 1.11 2019/12/23 20:38:08 thorpej Exp $ */
 
 /*-
  * Copyright (c) 2017 Jared McNeill <jmcneill%invisible.ca@localhost>
@@ -29,7 +29,7 @@
 #include "opt_fdt.h"
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: tcakp.c,v 1.10 2018/10/17 16:56:40 jmcneill Exp $");
+__KERNEL_RCSID(0, "$NetBSD: tcakp.c,v 1.11 2019/12/23 20:38:08 thorpej Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -99,6 +99,7 @@
        uint16_t        sc_keymap[128];
 
        void            *sc_ih;
+       void            *sc_sih;
 
        int             sc_enabled;
        device_t        sc_wskbddev;
@@ -107,6 +108,9 @@
 static int     tcakp_match(device_t, cfdata_t, void *);
 static void    tcakp_attach(device_t, device_t, void *);
 
+static int     tcakp_i2c_lock(struct tcakp_softc *);
+static void    tcakp_i2c_unlock(struct tcakp_softc *);
+
 static int     tcakp_read(struct tcakp_softc *, uint8_t, uint8_t *);
 static int     tcakp_write(struct tcakp_softc *, uint8_t, uint8_t);
 
@@ -137,8 +141,28 @@
 tcakp_intr(void *priv)
 {
        struct tcakp_softc * const sc = priv;
+
+       /*
+        * Schedule our soft interrupt handler.  We can't access the i2c
+        * from hard interrupt context, so just go ahead and claim the
+        * interrupt.
+        *
+        * XXX If we ever end up with an instance that uses
+        * level-sensitive interrupts, we will need to mask
+        * the interrupt source.
+        */
+       softint_schedule(sc->sc_sih);
+       return 1;
+}
+
+static void
+tcakp_softintr(void *priv)
+{
+       struct tcakp_softc * const sc = priv;
        uint8_t stat, ev;
-       int ret = 0;
+
+       if (tcakp_i2c_lock(sc) != 0)
+               return;
 
        tcakp_read(sc, TCA_INT_STAT, &stat);
        if (stat & INT_STAT_K_INT) {
@@ -147,6 +171,8 @@
                        const bool pressed = __SHIFTOUT(ev, TCA_EVENT_STATE);
                        const uint8_t code = __SHIFTOUT(ev, TCA_EVENT_CODE);
 
+                       tcakp_i2c_unlock(sc);
+
                        /* Translate raw code to keymap index */
                        const u_int index = tcakp_decode(sc, code);
 
@@ -157,13 +183,13 @@
                        if (sc->sc_wskbddev)
                                wskbd_input(sc->sc_wskbddev, type, key);
 
+                       if (tcakp_i2c_lock(sc) != 0)
+                               return;
                        tcakp_read(sc, TCA_EVENT('A'), &ev);
                }
-               ret = 1;
        }
        tcakp_write(sc, TCA_INT_STAT, stat);
-
-       return ret;
+       tcakp_i2c_unlock(sc);
 }
 
 static int
@@ -171,6 +197,7 @@
 {
        uint32_t mask;
        uint8_t val;
+       int error;
 
        if (sc->sc_rows == 0 || sc->sc_cols == 0) {
                aprint_error_dev(sc->sc_dev, "not configured\n");
@@ -180,6 +207,10 @@
        mask = __BITS(sc->sc_rows - 1, 0);
        mask += __BITS(sc->sc_cols - 1, 0) << 8;
 
+       error = tcakp_i2c_lock(sc);
+       if (error)
+               return error;
+
        /* Lock the keyboard */
        tcakp_write(sc, TCA_KEY_LCK_EC, KEY_LCK_EC_K_LCK_EN);
        /* Select keyboard mode */
@@ -196,6 +227,8 @@
        tcakp_read(sc, TCA_INT_STAT, &val);
        tcakp_write(sc, TCA_INT_STAT, val);
 
+       tcakp_i2c_unlock(sc);
+
        return 0;
 }
 
@@ -234,6 +267,11 @@
 tcakp_enable(void *v, int on)
 {
        struct tcakp_softc * const sc = v;
+       int error;
+
+       error = tcakp_i2c_lock(sc);
+       if (error)
+               return error;
 
        if (on) {
                /* Disable key lock */
@@ -243,6 +281,7 @@
                tcakp_write(sc, TCA_KEY_LCK_EC, KEY_LCK_EC_K_LCK_EN);
        }
 
+       tcakp_i2c_unlock(sc);
        return 0;
 }
 
@@ -276,6 +315,8 @@
        struct tcakp_softc * const sc = v;
        uint8_t ev = 0;
 
+       /* XXX i2c bus acquire */
+
        do {
                tcakp_read(sc, TCA_EVENT('A'), &ev);
        } while (ev == 0);
@@ -287,6 +328,8 @@
        *type = pressed ? WSCONS_EVENT_KEY_DOWN :
                          WSCONS_EVENT_KEY_UP;
        *data = sc->sc_keymap[index];
+
+       /* XXX i2c bus release */
 }
 
 static void
@@ -345,6 +388,21 @@
 #ifdef FDT
        sc->sc_ih = fdtbus_intr_establish(sc->sc_phandle, 0, IPL_VM, 0,
            tcakp_intr, sc);
+       /*
+        * XXX This is an edge-sensitive interrupt, but we'd like to
+        * be able to check at run-time just to be sure.
+        */
+       if (sc->sc_ih == NULL) {
+               aprint_error_dev(sc->sc_dev, "unable to establish interrupt\n");
+               return;
+       }
+
+       sc->sc_sih = softint_establish(SOFTINT_SERIAL, tcakp_softintr, sc);
+       if (sc->sc_sih == NULL) {
+               aprint_error_dev(sc->sc_dev,
+                   "unable to establish soft interupt\n");
+               return;
+       }
 
        tcakp_configure_fdt(sc);
 #endif
@@ -362,28 +420,36 @@
 }
 
 static int
-tcakp_read(struct tcakp_softc *sc, uint8_t reg, uint8_t *val)
+tcakp_i2c_lock(struct tcakp_softc *sc)
 {
        int error;
 
-       iic_acquire_bus(sc->sc_i2c, I2C_F_POLL);
-       error = iic_exec(sc->sc_i2c, I2C_OP_READ_WITH_STOP, sc->sc_addr,
-           &reg, 1, val, 1, I2C_F_POLL);
-       iic_release_bus(sc->sc_i2c, I2C_F_POLL);
+       error = iic_acquire_bus(sc->sc_i2c, 0);
+       if (error) {
+               aprint_error_dev(sc->sc_dev,
+                   "unable to acquire bus lock (%d)\n", error);
+       }
+       return error;
+}
 
-       return error;
+static void
+tcakp_i2c_unlock(struct tcakp_softc *sc)
+{
+       iic_release_bus(sc->sc_i2c, 0);
+}
+
+static int
+tcakp_read(struct tcakp_softc *sc, uint8_t reg, uint8_t *val)
+{
+       return iic_exec(sc->sc_i2c, I2C_OP_READ_WITH_STOP, sc->sc_addr,
+           &reg, 1, val, 1, 0);
 }
 
 static int
 tcakp_write(struct tcakp_softc *sc, uint8_t reg, uint8_t val)
 {
        uint8_t buf[2] = { reg, val };
-       int error;
 
-       iic_acquire_bus(sc->sc_i2c, I2C_F_POLL);
-       error = iic_exec(sc->sc_i2c, I2C_OP_WRITE_WITH_STOP, sc->sc_addr,
-           NULL, 0, buf, 2, I2C_F_POLL);
-       iic_release_bus(sc->sc_i2c, I2C_F_POLL);
-
-       return error;
+       return iic_exec(sc->sc_i2c, I2C_OP_WRITE_WITH_STOP,
+           sc->sc_addr, NULL, 0, buf, 2, 0);
 }