Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/i2c The hid-over-i2c spec specifies that compliant d...



details:   https://anonhg.NetBSD.org/src/rev/5a3e205704d0
branches:  trunk
changeset: 1005824:5a3e205704d0
user:      thorpej <thorpej%NetBSD.org@localhost>
date:      Sun Dec 22 16:44:35 2019 +0000

description:
The hid-over-i2c spec specifies that compliant devices use level-sensitive
interrupts.  However, it's not safe to do i2c bus access in hard interrupt
context, and we must read the event data off the device in order to clear
the interrupt condition.

Address this by using acpi_intr_mask() to mask off the interrupt source
while a softint is pending to service the events, re-enabling it once
servicing is completed.

While here, re-factor the interrupt setup / tear-down code a bit to
eventually once day simplify supporting the FDT bindings for hid-over-i2c.

diffstat:

 sys/dev/i2c/ihidev.c |  156 ++++++++++++++++++++++++++++++++++++++------------
 sys/dev/i2c/ihidev.h |    4 +-
 2 files changed, 120 insertions(+), 40 deletions(-)

diffs (265 lines):

diff -r dc16d812db2b -r 5a3e205704d0 sys/dev/i2c/ihidev.c
--- a/sys/dev/i2c/ihidev.c      Sun Dec 22 16:37:36 2019 +0000
+++ b/sys/dev/i2c/ihidev.c      Sun Dec 22 16:44:35 2019 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ihidev.c,v 1.9 2019/10/01 18:00:08 chs Exp $ */
+/* $NetBSD: ihidev.c,v 1.10 2019/12/22 16:44:35 thorpej Exp $ */
 /* $OpenBSD ihidev.c,v 1.13 2017/04/08 02:57:23 deraadt Exp $ */
 
 /*-
@@ -54,7 +54,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ihidev.c,v 1.9 2019/10/01 18:00:08 chs Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ihidev.c,v 1.10 2019/12/22 16:44:35 thorpej Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -71,6 +71,7 @@
 #  include "acpica.h"
 #endif
 #if NACPICA > 0
+#include <dev/acpi/acpivar.h>
 #include <dev/acpi/acpi_intr.h>
 #endif
 
@@ -109,10 +110,14 @@
 CFATTACH_DECL_NEW(ihidev, sizeof(struct ihidev_softc),
     ihidev_match, ihidev_attach, ihidev_detach, NULL);
 
+static bool    ihiddev_intr_init(struct ihidev_softc *);
+static void    ihiddev_intr_fini(struct ihidev_softc *);
+
 static bool    ihidev_suspend(device_t, const pmf_qual_t *);
 static bool    ihidev_resume(device_t, const pmf_qual_t *);
 static int     ihidev_hid_command(struct ihidev_softc *, int, void *, bool);
 static int     ihidev_intr(void *);
+static void    ihidev_softintr(void *);
 static int     ihidev_reset(struct ihidev_softc *, bool);
 static int     ihidev_hid_desc_parse(struct ihidev_softc *);
 
@@ -200,20 +205,9 @@
                    repsz));
        }
        sc->sc_ibuf = kmem_zalloc(sc->sc_isize, KM_SLEEP);
-#if NACPICA > 0
-       {
-               char buf[100];
-
-               sc->sc_ih = acpi_intr_establish(self, sc->sc_phandle, IPL_TTY,
-                   false, ihidev_intr, sc, device_xname(self));
-               if (sc->sc_ih == NULL) {
-                       aprint_error_dev(self, "can't establish interrupt\n");
-                       return;
-               }
-               aprint_normal_dev(self, "interrupting at %s\n",
-                   acpi_intr_string(sc->sc_ih, buf, sizeof(buf)));
+       if (! ihiddev_intr_init(sc)) {
+               return;
        }
-#endif
 
        iha.iaa = ia;
        iha.parent = sc;
@@ -260,10 +254,7 @@
        struct ihidev_softc *sc = device_private(self);
 
        mutex_enter(&sc->sc_intr_lock);
-#if NACPICA > 0
-       if (sc->sc_ih != NULL)
-               acpi_intr_disestablish(sc->sc_ih);
-#endif
+       ihiddev_intr_fini(sc);
        if (ihidev_hid_command(sc, I2C_HID_CMD_SET_POWER,
            &I2C_HID_POWER_OFF, true))
        aprint_error_dev(sc->sc_dev, "failed to power down\n");
@@ -649,31 +640,110 @@
        return (0);
 }
 
+static bool
+ihiddev_intr_init(struct ihidev_softc *sc)
+{
+#if NACPICA > 0
+       ACPI_HANDLE hdl = (void *)(uintptr_t)sc->sc_phandle;
+       struct acpi_resources res;
+       ACPI_STATUS rv;
+       char buf[100];
+
+       rv = acpi_resource_parse(sc->sc_dev, hdl, "_CRS", &res,
+           &acpi_resource_parse_ops_quiet);
+       if (ACPI_FAILURE(rv)) {
+               aprint_error_dev(sc->sc_dev, "can't parse '_CRS'\n");
+               return false;
+       }
+
+       const struct acpi_irq * const irq = acpi_res_irq(&res, 0);
+       if (irq == NULL) {
+               aprint_error_dev(sc->sc_dev, "no IRQ resource\n");
+               acpi_resource_cleanup(&res);
+               return false;
+       }
+
+       sc->sc_intr_type =
+           irq->ar_type == ACPI_EDGE_SENSITIVE ? IST_EDGE : IST_LEVEL;
+
+       acpi_resource_cleanup(&res);
+
+       sc->sc_ih = acpi_intr_establish(sc->sc_dev, sc->sc_phandle, IPL_TTY,
+           false, ihidev_intr, sc, device_xname(sc->sc_dev));
+       if (sc->sc_ih == NULL) {
+               aprint_error_dev(sc->sc_dev, "can't establish interrupt\n");
+               return false;
+       }
+       aprint_normal_dev(sc->sc_dev, "interrupting at %s\n",
+           acpi_intr_string(sc->sc_ih, buf, sizeof(buf)));
+
+       sc->sc_sih = softint_establish(SOFTINT_SERIAL, ihidev_softintr, sc);
+       if (sc->sc_sih == NULL) {
+               aprint_error_dev(sc->sc_dev,
+                   "can't establish soft interrupt\n");
+               return false;
+       }
+
+       return true;
+#else
+       aprint_error_dev(sc->sc_dev, "can't establish interrupt\n");
+       return false;
+#endif
+}
+
+static void
+ihiddev_intr_fini(struct ihidev_softc *sc)
+{
+#if NACPICA > 0
+       if (sc->sc_ih != NULL) {
+               acpi_intr_disestablish(sc->sc_ih);
+       }
+       if (sc->sc_sih != NULL) {
+               softint_disestablish(sc->sc_sih);
+       }
+#endif
+}
+
 static int
 ihidev_intr(void *arg)
 {
-       struct ihidev_softc *sc = arg;
+       struct ihidev_softc * const sc = arg;
+
+       mutex_enter(&sc->sc_intr_lock);
+
+       /*
+        * Schedule our soft interrupt handler.  If we're using a level-
+        * triggered interrupt, we have to mask it off while we wait
+        * for service.
+        */
+       softint_schedule(sc->sc_sih);
+       if (sc->sc_intr_type == IST_LEVEL) {
+#if NACPICA > 0
+               acpi_intr_mask(sc->sc_ih);
+#endif
+       }
+
+       mutex_exit(&sc->sc_intr_lock);
+
+       return 1;
+}
+
+static void
+ihidev_softintr(void *arg)
+{
+       struct ihidev_softc * const sc = arg;
        struct ihidev *scd;
        u_int psize;
        int res, i;
        u_char *p;
        u_int rep = 0;
 
-       /*
-        * XXX: force I2C_F_POLL for now to avoid dwiic interrupting
-        * while we are interrupting
-        */
-
-       mutex_enter(&sc->sc_intr_lock);
-       iic_acquire_bus(sc->sc_tag, I2C_F_POLL);
-
+       iic_acquire_bus(sc->sc_tag, 0);
        res = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP, sc->sc_addr, NULL, 0,
-           sc->sc_ibuf, sc->sc_isize, I2C_F_POLL);
-
-       iic_release_bus(sc->sc_tag, I2C_F_POLL);
-       mutex_exit(&sc->sc_intr_lock);
+           sc->sc_ibuf, sc->sc_isize, 0);
+       iic_release_bus(sc->sc_tag, 0);
        if (res != 0)
-               return 1;
+               goto out;
 
        /*
         * 6.1.1 - First two bytes are the packet length, which must be less
@@ -683,7 +753,7 @@
        if (!psize || psize > sc->sc_isize) {
                DPRINTF(("%s: %s: invalid packet size (%d vs. %d)\n",
                    sc->sc_dev.dv_xname, __func__, psize, sc->sc_isize));
-               return (1);
+               goto out;
        }
 
        /* 3rd byte is the report id */
@@ -695,22 +765,30 @@
        if (rep >= sc->sc_nrepid) {
                aprint_error_dev(sc->sc_dev, "%s: bad report id %d\n",
                    __func__, rep);
-               return (1);
+               goto out;
        }
 
-       DPRINTF(("%s: ihidev_intr: hid input (rep %d):", sc->sc_dev.dv_xname,
-           rep));
+       DPRINTF(("%s: %s: hid input (rep %d):", sc->sc_dev.dv_xname,
+           __func__, rep));
        for (i = 0; i < sc->sc_isize; i++)
                DPRINTF((" %.2x", sc->sc_ibuf[i]));
        DPRINTF(("\n"));
 
        scd = sc->sc_subdevs[rep];
        if (scd == NULL || !(scd->sc_state & IHIDEV_OPEN))
-               return (1);
+               goto out;
 
        scd->sc_intr(scd, p, psize);
 
-       return 1;
+ out:
+       /*
+        * If our interrupt is level-triggered, re-enable it now.
+        */
+       if (sc->sc_intr_type == IST_LEVEL) {
+#if NACPICA > 0
+               acpi_intr_unmask(sc->sc_ih);
+#endif
+       }
 }
 
 static int
diff -r dc16d812db2b -r 5a3e205704d0 sys/dev/i2c/ihidev.h
--- a/sys/dev/i2c/ihidev.h      Sun Dec 22 16:37:36 2019 +0000
+++ b/sys/dev/i2c/ihidev.h      Sun Dec 22 16:44:35 2019 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ihidev.h,v 1.1 2017/12/10 17:05:54 bouyer Exp $ */
+/* $NetBSD: ihidev.h,v 1.2 2019/12/22 16:44:35 thorpej Exp $ */
 /* $OpenBSD ihidev.h,v 1.4 2016/01/31 18:24:35 jcs Exp $ */
 
 /*-
@@ -100,7 +100,9 @@
        uint64_t        sc_phandle;
 
        void *          sc_ih;
+       void *          sc_sih;
        kmutex_t        sc_intr_lock;
+       int             sc_intr_type;
 
        u_int           sc_hid_desc_addr;
        union {



Home | Main Index | Thread Index | Old Index