ihidev: always register interrupt, stop polling if it fires

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

ihidev: always register interrupt, stop polling if it fires

joshua stein-3
The fast polling of ihidev may cause a problem during suspend/resume
because dwiic may be in an unknown state, so add a DVACT_QUIESCE
handler to properly shut it down.

Even if polling is requested, register the interrupt handler too.  
If it ever fires, stop polling and just use the interrupt.  This is
what led me to discover that ihidev's interrupt starts firing after
a suspend/resume cycle (but I still have no idea why it doesn't work
before that).



Index: dev/acpi/dwiic_acpi.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/dwiic_acpi.c,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 dwiic_acpi.c
--- dev/acpi/dwiic_acpi.c 16 Jul 2019 19:12:32 -0000 1.9
+++ dev/acpi/dwiic_acpi.c 19 Jul 2019 19:11:21 -0000
@@ -457,8 +457,9 @@ dwiic_acpi_found_ihidev(struct dwiic_sof
 
  aml_freevalue(&res);
 
- if (!sc->sc_poll_ihidev &&
-    !(crs.irq_int == 0 && crs.gpio_int_node == NULL))
+ if (sc->sc_poll_ihidev)
+ ia.ia_poll = 1;
+ if (!(crs.irq_int == 0 && crs.gpio_int_node == NULL))
  ia.ia_intr = &crs;
 
  if (config_found(sc->sc_iic, &ia, dwiic_i2c_print)) {
Index: dev/i2c/i2cvar.h
===================================================================
RCS file: /cvs/src/sys/dev/i2c/i2cvar.h,v
retrieving revision 1.16
diff -u -p -u -p -r1.16 i2cvar.h
--- dev/i2c/i2cvar.h 23 Apr 2016 09:40:28 -0000 1.16
+++ dev/i2c/i2cvar.h 19 Jul 2019 19:11:21 -0000
@@ -113,6 +113,7 @@ struct i2c_attach_args {
  char *ia_name; /* chip name */
  void *ia_cookie; /* pass extra info from bus to dev */
  void *ia_intr; /* interrupt info */
+ int ia_poll; /* to force polling */
 };
 
 /*
Index: dev/i2c/ihidev.c
===================================================================
RCS file: /cvs/src/sys/dev/i2c/ihidev.c,v
retrieving revision 1.19
diff -u -p -u -p -r1.19 ihidev.c
--- dev/i2c/ihidev.c 8 Apr 2019 17:50:45 -0000 1.19
+++ dev/i2c/ihidev.c 19 Jul 2019 19:11:21 -0000
@@ -63,6 +63,7 @@ static int I2C_HID_POWER_OFF = 0x1;
 int ihidev_match(struct device *, void *, void *);
 void ihidev_attach(struct device *, struct device *, void *);
 int ihidev_detach(struct device *, int);
+int ihidev_activate(struct device *, int);
 
 int ihidev_hid_command(struct ihidev_softc *, int, void *);
 int ihidev_intr(void *);
@@ -80,7 +81,7 @@ struct cfattach ihidev_ca = {
  ihidev_match,
  ihidev_attach,
  ihidev_detach,
- NULL
+ ihidev_activate,
 };
 
 struct cfdriver ihidev_cd = {
@@ -128,7 +129,7 @@ ihidev_attach(struct device *parent, str
  printf(", can't establish interrupt");
  }
 
- if (sc->sc_ih == NULL) {
+ if (ia->ia_poll) {
  printf(" (polling)");
  sc->sc_poll = 1;
  sc->sc_fastpoll = 1;
@@ -227,6 +228,39 @@ ihidev_detach(struct device *self, int f
  return (0);
 }
 
+int
+ihidev_activate(struct device *self, int act)
+{
+ struct ihidev_softc *sc = (struct ihidev_softc *)self;
+
+ DPRINTF(("%s(%d)\n", __func__, act));
+
+ switch (act) {
+ case DVACT_QUIESCE:
+ sc->sc_dying = 1;
+ if (sc->sc_poll && timeout_initialized(&sc->sc_timer)) {
+ DPRINTF(("%s: canceling polling\n",
+    sc->sc_dev.dv_xname));
+ timeout_del_barrier(&sc->sc_timer);
+ }
+ if (ihidev_hid_command(sc, I2C_HID_CMD_SET_POWER,
+    &I2C_HID_POWER_OFF))
+ printf("%s: failed to power down\n",
+    sc->sc_dev.dv_xname);
+ break;
+ case DVACT_WAKEUP:
+ ihidev_reset(sc);
+ sc->sc_dying = 0;
+ if (sc->sc_poll && timeout_initialized(&sc->sc_timer))
+ timeout_add(&sc->sc_timer, 2000);
+ break;
+ }
+
+ config_activate_children(self, act);
+
+ return 0;
+}
+
 void
 ihidev_sleep(struct ihidev_softc *sc, int ms)
 {
@@ -580,6 +614,16 @@ ihidev_hid_desc_parse(struct ihidev_soft
  return (0);
 }
 
+void
+ihidev_poll(void *arg)
+{
+ struct ihidev_softc *sc = arg;
+
+ sc->sc_frompoll = 1;
+ ihidev_intr(sc);
+ sc->sc_frompoll = 0;
+}
+
 int
 ihidev_intr(void *arg)
 {
@@ -589,6 +633,16 @@ ihidev_intr(void *arg)
  u_char *p;
  u_int rep = 0;
 
+ if (sc->sc_dying)
+ return 1;
+
+ if (sc->sc_poll && !sc->sc_frompoll) {
+ printf("%s: received interrupt while polling, disabling "
+    "polling\n", sc->sc_dev.dv_xname);
+ sc->sc_poll = 0;
+ timeout_del_barrier(&sc->sc_timer);
+ }
+
  /*
  * XXX: force I2C_F_POLL for now to avoid dwiic interrupting
  * while we are interrupting
@@ -660,7 +714,7 @@ ihidev_intr(void *arg)
 
  scd->sc_intr(scd, p, psize);
 
- if (sc->sc_poll && fast != sc->sc_fastpoll) {
+ if (sc->sc_poll && (fast != sc->sc_fastpoll)) {
  DPRINTF(("%s: %s->%s polling\n", sc->sc_dev.dv_xname,
     sc->sc_fastpoll ? "fast" : "slow",
     fast ? "fast" : "slow"));
@@ -668,7 +722,8 @@ ihidev_intr(void *arg)
  }
 
 more_polling:
- if (sc->sc_poll && sc->sc_refcnt && !timeout_pending(&sc->sc_timer))
+ if (sc->sc_poll && sc->sc_refcnt && !sc->sc_dying &&
+    !timeout_pending(&sc->sc_timer))
  timeout_add_msec(&sc->sc_timer,
     sc->sc_fastpoll ? FAST_POLL_MS : SLOW_POLL_MS);
 
@@ -740,7 +795,7 @@ ihidev_open(struct ihidev *scd)
 
  if (sc->sc_poll) {
  if (!timeout_initialized(&sc->sc_timer))
- timeout_set(&sc->sc_timer, (void *)ihidev_intr, sc);
+ timeout_set(&sc->sc_timer, (void *)ihidev_poll, sc);
  if (!timeout_pending(&sc->sc_timer))
  timeout_add(&sc->sc_timer, FAST_POLL_MS);
  }
@@ -766,7 +821,7 @@ ihidev_close(struct ihidev *scd)
 
  /* no sub-devices open, conserve power */
 
- if (sc->sc_poll)
+ if (sc->sc_poll && timeout_pending(&sc->sc_timer))
  timeout_del(&sc->sc_timer);
 
  if (ihidev_hid_command(sc, I2C_HID_CMD_SET_POWER, &I2C_HID_POWER_OFF))
Index: dev/i2c/ihidev.h
===================================================================
RCS file: /cvs/src/sys/dev/i2c/ihidev.h,v
retrieving revision 1.6
diff -u -p -u -p -r1.6 ihidev.h
--- dev/i2c/ihidev.h 25 Aug 2018 18:32:05 -0000 1.6
+++ dev/i2c/ihidev.h 19 Jul 2019 19:11:21 -0000
@@ -89,8 +89,10 @@ struct ihidev_softc {
  int sc_refcnt;
 
  int sc_poll;
+ int sc_frompoll;
  int sc_fastpoll;
  struct timeout sc_timer;
+ int sc_dying;
 };
 
 struct ihidev {

Reply | Threaded
Open this post in threaded view
|

Re: ihidev: always register interrupt, stop polling if it fires

Mike Larkin-2
On Fri, Jul 19, 2019 at 02:16:18PM -0500, joshua stein wrote:

> The fast polling of ihidev may cause a problem during suspend/resume
> because dwiic may be in an unknown state, so add a DVACT_QUIESCE
> handler to properly shut it down.
>
> Even if polling is requested, register the interrupt handler too.  
> If it ever fires, stop polling and just use the interrupt.  This is
> what led me to discover that ihidev's interrupt starts firing after
> a suspend/resume cycle (but I still have no idea why it doesn't work
> before that).
>
>

Seems to be a sensible workaround, but you might want to remove that
printf in the interrupt handler unless you're absolutely sure it's safe
there...

-ml

> Index: dev/acpi/dwiic_acpi.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/acpi/dwiic_acpi.c,v
> retrieving revision 1.9
> diff -u -p -u -p -r1.9 dwiic_acpi.c
> --- dev/acpi/dwiic_acpi.c 16 Jul 2019 19:12:32 -0000 1.9
> +++ dev/acpi/dwiic_acpi.c 19 Jul 2019 19:11:21 -0000
> @@ -457,8 +457,9 @@ dwiic_acpi_found_ihidev(struct dwiic_sof
>  
>   aml_freevalue(&res);
>  
> - if (!sc->sc_poll_ihidev &&
> -    !(crs.irq_int == 0 && crs.gpio_int_node == NULL))
> + if (sc->sc_poll_ihidev)
> + ia.ia_poll = 1;
> + if (!(crs.irq_int == 0 && crs.gpio_int_node == NULL))
>   ia.ia_intr = &crs;
>  
>   if (config_found(sc->sc_iic, &ia, dwiic_i2c_print)) {
> Index: dev/i2c/i2cvar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/i2c/i2cvar.h,v
> retrieving revision 1.16
> diff -u -p -u -p -r1.16 i2cvar.h
> --- dev/i2c/i2cvar.h 23 Apr 2016 09:40:28 -0000 1.16
> +++ dev/i2c/i2cvar.h 19 Jul 2019 19:11:21 -0000
> @@ -113,6 +113,7 @@ struct i2c_attach_args {
>   char *ia_name; /* chip name */
>   void *ia_cookie; /* pass extra info from bus to dev */
>   void *ia_intr; /* interrupt info */
> + int ia_poll; /* to force polling */
>  };
>  
>  /*
> Index: dev/i2c/ihidev.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/i2c/ihidev.c,v
> retrieving revision 1.19
> diff -u -p -u -p -r1.19 ihidev.c
> --- dev/i2c/ihidev.c 8 Apr 2019 17:50:45 -0000 1.19
> +++ dev/i2c/ihidev.c 19 Jul 2019 19:11:21 -0000
> @@ -63,6 +63,7 @@ static int I2C_HID_POWER_OFF = 0x1;
>  int ihidev_match(struct device *, void *, void *);
>  void ihidev_attach(struct device *, struct device *, void *);
>  int ihidev_detach(struct device *, int);
> +int ihidev_activate(struct device *, int);
>  
>  int ihidev_hid_command(struct ihidev_softc *, int, void *);
>  int ihidev_intr(void *);
> @@ -80,7 +81,7 @@ struct cfattach ihidev_ca = {
>   ihidev_match,
>   ihidev_attach,
>   ihidev_detach,
> - NULL
> + ihidev_activate,
>  };
>  
>  struct cfdriver ihidev_cd = {
> @@ -128,7 +129,7 @@ ihidev_attach(struct device *parent, str
>   printf(", can't establish interrupt");
>   }
>  
> - if (sc->sc_ih == NULL) {
> + if (ia->ia_poll) {
>   printf(" (polling)");
>   sc->sc_poll = 1;
>   sc->sc_fastpoll = 1;
> @@ -227,6 +228,39 @@ ihidev_detach(struct device *self, int f
>   return (0);
>  }
>  
> +int
> +ihidev_activate(struct device *self, int act)
> +{
> + struct ihidev_softc *sc = (struct ihidev_softc *)self;
> +
> + DPRINTF(("%s(%d)\n", __func__, act));
> +
> + switch (act) {
> + case DVACT_QUIESCE:
> + sc->sc_dying = 1;
> + if (sc->sc_poll && timeout_initialized(&sc->sc_timer)) {
> + DPRINTF(("%s: canceling polling\n",
> +    sc->sc_dev.dv_xname));
> + timeout_del_barrier(&sc->sc_timer);
> + }
> + if (ihidev_hid_command(sc, I2C_HID_CMD_SET_POWER,
> +    &I2C_HID_POWER_OFF))
> + printf("%s: failed to power down\n",
> +    sc->sc_dev.dv_xname);
> + break;
> + case DVACT_WAKEUP:
> + ihidev_reset(sc);
> + sc->sc_dying = 0;
> + if (sc->sc_poll && timeout_initialized(&sc->sc_timer))
> + timeout_add(&sc->sc_timer, 2000);
> + break;
> + }
> +
> + config_activate_children(self, act);
> +
> + return 0;
> +}
> +
>  void
>  ihidev_sleep(struct ihidev_softc *sc, int ms)
>  {
> @@ -580,6 +614,16 @@ ihidev_hid_desc_parse(struct ihidev_soft
>   return (0);
>  }
>  
> +void
> +ihidev_poll(void *arg)
> +{
> + struct ihidev_softc *sc = arg;
> +
> + sc->sc_frompoll = 1;
> + ihidev_intr(sc);
> + sc->sc_frompoll = 0;
> +}
> +
>  int
>  ihidev_intr(void *arg)
>  {
> @@ -589,6 +633,16 @@ ihidev_intr(void *arg)
>   u_char *p;
>   u_int rep = 0;
>  
> + if (sc->sc_dying)
> + return 1;
> +
> + if (sc->sc_poll && !sc->sc_frompoll) {
> + printf("%s: received interrupt while polling, disabling "
> +    "polling\n", sc->sc_dev.dv_xname);
> + sc->sc_poll = 0;
> + timeout_del_barrier(&sc->sc_timer);
> + }
> +
>   /*
>   * XXX: force I2C_F_POLL for now to avoid dwiic interrupting
>   * while we are interrupting
> @@ -660,7 +714,7 @@ ihidev_intr(void *arg)
>  
>   scd->sc_intr(scd, p, psize);
>  
> - if (sc->sc_poll && fast != sc->sc_fastpoll) {
> + if (sc->sc_poll && (fast != sc->sc_fastpoll)) {
>   DPRINTF(("%s: %s->%s polling\n", sc->sc_dev.dv_xname,
>      sc->sc_fastpoll ? "fast" : "slow",
>      fast ? "fast" : "slow"));
> @@ -668,7 +722,8 @@ ihidev_intr(void *arg)
>   }
>  
>  more_polling:
> - if (sc->sc_poll && sc->sc_refcnt && !timeout_pending(&sc->sc_timer))
> + if (sc->sc_poll && sc->sc_refcnt && !sc->sc_dying &&
> +    !timeout_pending(&sc->sc_timer))
>   timeout_add_msec(&sc->sc_timer,
>      sc->sc_fastpoll ? FAST_POLL_MS : SLOW_POLL_MS);
>  
> @@ -740,7 +795,7 @@ ihidev_open(struct ihidev *scd)
>  
>   if (sc->sc_poll) {
>   if (!timeout_initialized(&sc->sc_timer))
> - timeout_set(&sc->sc_timer, (void *)ihidev_intr, sc);
> + timeout_set(&sc->sc_timer, (void *)ihidev_poll, sc);
>   if (!timeout_pending(&sc->sc_timer))
>   timeout_add(&sc->sc_timer, FAST_POLL_MS);
>   }
> @@ -766,7 +821,7 @@ ihidev_close(struct ihidev *scd)
>  
>   /* no sub-devices open, conserve power */
>  
> - if (sc->sc_poll)
> + if (sc->sc_poll && timeout_pending(&sc->sc_timer))
>   timeout_del(&sc->sc_timer);
>  
>   if (ihidev_hid_command(sc, I2C_HID_CMD_SET_POWER, &I2C_HID_POWER_OFF))
> Index: dev/i2c/ihidev.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/i2c/ihidev.h,v
> retrieving revision 1.6
> diff -u -p -u -p -r1.6 ihidev.h
> --- dev/i2c/ihidev.h 25 Aug 2018 18:32:05 -0000 1.6
> +++ dev/i2c/ihidev.h 19 Jul 2019 19:11:21 -0000
> @@ -89,8 +89,10 @@ struct ihidev_softc {
>   int sc_refcnt;
>  
>   int sc_poll;
> + int sc_frompoll;
>   int sc_fastpoll;
>   struct timeout sc_timer;
> + int sc_dying;
>  };
>  
>  struct ihidev {
>