Re: asmc: restore keyboard backlight on resume

2017-02-11 Thread Theo de Raadt
>I'm not sure if you choose DVACT_WAKEUP on purpose, another way of doing
>it would be to do a task_add() in DVACT_RESUME.  The difference is that
>the resuming thread is not allowed to sleep in RESUME.  At some point we
>tried to keep the number of WAKEUP low, but I'm not sure if it is still
>relevant today.

Hmm, I don't recall it that way.  The problem with introducing the split
between RESUME and WAKEUP was a long learning curve for deveopers, and so we
cheated a little.  I still prefer the syncronous behaviour of a proper
WAKEUP since studying failure is easier.  Asyncronous event interactions are
highly undebuggable in this scenario.

So I prefer the obviousness of WAKEUP.

ok deraadt

>> Index: sys/dev/isa/asmc.c
>> ===
>> RCS file: /cvs/src/sys/dev/isa/asmc.c,v
>> retrieving revision 1.30
>> diff -u -p -u -p -r1.30 asmc.c
>> --- sys/dev/isa/asmc.c   22 Apr 2016 20:45:53 -  1.30
>> +++ sys/dev/isa/asmc.c   10 Feb 2017 00:40:15 -
>> @@ -92,6 +92,7 @@ void   asmc_update(void *);
>>  int asmc_match(struct device *, void *, void *);
>>  voidasmc_attach(struct device *, struct device *, void *);
>>  int asmc_detach(struct device *, int);
>> +int asmc_activate(struct device *, int);
>>  
>>  /* wskbd hook functions */
>>  voidasmc_backlight(void *);
>> @@ -101,7 +102,7 @@ extern int (*wskbd_get_backlight)(struct
>>  extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
>>  
>>  const struct cfattach asmc_ca = {
>> -sizeof(struct asmc_softc), asmc_match, asmc_attach
>> +sizeof(struct asmc_softc), asmc_match, asmc_attach, NULL, asmc_activate
>>  };
>>  
>>  struct cfdriver asmc_cd = {
>> @@ -355,6 +356,20 @@ asmc_detach(struct device *self, int fla
>>  
>>  task_del(systq, >sc_task_backlight);
>>  asmc_try(sc, ASMC_WRITE, "LKSB", buf, 2);
>> +return 0;
>> +}
>> +
>> +int
>> +asmc_activate(struct device *self, int act)
>> +{
>> +struct asmc_softc *sc = (struct asmc_softc *)self;
>> +
>> +switch (act) {
>> +case DVACT_WAKEUP:
>> +asmc_backlight(sc);
>> +break;
>> +}
>> +
>>  return 0;
>>  }
>>  
>> 
>
>



Re: asmc: restore keyboard backlight on resume

2017-02-10 Thread Mike Larkin
On Thu, Feb 09, 2017 at 06:41:38PM -0600, joshua stein wrote:
> After resume, the keyboard backlight is still off, so restore it
> (this was also helpful to figure out the machine was actually
> resuming).
> 

looks ok to me. ok mlarkin@

-ml

> 
> Index: sys/dev/isa/asmc.c
> ===
> RCS file: /cvs/src/sys/dev/isa/asmc.c,v
> retrieving revision 1.30
> diff -u -p -u -p -r1.30 asmc.c
> --- sys/dev/isa/asmc.c22 Apr 2016 20:45:53 -  1.30
> +++ sys/dev/isa/asmc.c10 Feb 2017 00:40:15 -
> @@ -92,6 +92,7 @@ voidasmc_update(void *);
>  int  asmc_match(struct device *, void *, void *);
>  void asmc_attach(struct device *, struct device *, void *);
>  int  asmc_detach(struct device *, int);
> +int  asmc_activate(struct device *, int);
>  
>  /* wskbd hook functions */
>  void asmc_backlight(void *);
> @@ -101,7 +102,7 @@ extern int (*wskbd_get_backlight)(struct
>  extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
>  
>  const struct cfattach asmc_ca = {
> - sizeof(struct asmc_softc), asmc_match, asmc_attach
> + sizeof(struct asmc_softc), asmc_match, asmc_attach, NULL, asmc_activate
>  };
>  
>  struct cfdriver asmc_cd = {
> @@ -355,6 +356,20 @@ asmc_detach(struct device *self, int fla
>  
>   task_del(systq, >sc_task_backlight);
>   asmc_try(sc, ASMC_WRITE, "LKSB", buf, 2);
> + return 0;
> +}
> +
> +int
> +asmc_activate(struct device *self, int act)
> +{
> + struct asmc_softc *sc = (struct asmc_softc *)self;
> +
> + switch (act) {
> + case DVACT_WAKEUP:
> + asmc_backlight(sc);
> + break;
> + }
> +
>   return 0;
>  }
>  
> 



Re: asmc: restore keyboard backlight on resume

2017-02-10 Thread Martin Pieuchot
On 09/02/17(Thu) 18:41, joshua stein wrote:
> After resume, the keyboard backlight is still off, so restore it
> (this was also helpful to figure out the machine was actually
> resuming).

ok mpi@

I'm not sure if you choose DVACT_WAKEUP on purpose, another way of doing
it would be to do a task_add() in DVACT_RESUME.  The difference is that
the resuming thread is not allowed to sleep in RESUME.  At some point we
tried to keep the number of WAKEUP low, but I'm not sure if it is still
relevant today.

> Index: sys/dev/isa/asmc.c
> ===
> RCS file: /cvs/src/sys/dev/isa/asmc.c,v
> retrieving revision 1.30
> diff -u -p -u -p -r1.30 asmc.c
> --- sys/dev/isa/asmc.c22 Apr 2016 20:45:53 -  1.30
> +++ sys/dev/isa/asmc.c10 Feb 2017 00:40:15 -
> @@ -92,6 +92,7 @@ voidasmc_update(void *);
>  int  asmc_match(struct device *, void *, void *);
>  void asmc_attach(struct device *, struct device *, void *);
>  int  asmc_detach(struct device *, int);
> +int  asmc_activate(struct device *, int);
>  
>  /* wskbd hook functions */
>  void asmc_backlight(void *);
> @@ -101,7 +102,7 @@ extern int (*wskbd_get_backlight)(struct
>  extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
>  
>  const struct cfattach asmc_ca = {
> - sizeof(struct asmc_softc), asmc_match, asmc_attach
> + sizeof(struct asmc_softc), asmc_match, asmc_attach, NULL, asmc_activate
>  };
>  
>  struct cfdriver asmc_cd = {
> @@ -355,6 +356,20 @@ asmc_detach(struct device *self, int fla
>  
>   task_del(systq, >sc_task_backlight);
>   asmc_try(sc, ASMC_WRITE, "LKSB", buf, 2);
> + return 0;
> +}
> +
> +int
> +asmc_activate(struct device *self, int act)
> +{
> + struct asmc_softc *sc = (struct asmc_softc *)self;
> +
> + switch (act) {
> + case DVACT_WAKEUP:
> + asmc_backlight(sc);
> + break;
> + }
> +
>   return 0;
>  }
>  
> 



asmc: restore keyboard backlight on resume

2017-02-09 Thread joshua stein
After resume, the keyboard backlight is still off, so restore it
(this was also helpful to figure out the machine was actually
resuming).


Index: sys/dev/isa/asmc.c
===
RCS file: /cvs/src/sys/dev/isa/asmc.c,v
retrieving revision 1.30
diff -u -p -u -p -r1.30 asmc.c
--- sys/dev/isa/asmc.c  22 Apr 2016 20:45:53 -  1.30
+++ sys/dev/isa/asmc.c  10 Feb 2017 00:40:15 -
@@ -92,6 +92,7 @@ void  asmc_update(void *);
 intasmc_match(struct device *, void *, void *);
 void   asmc_attach(struct device *, struct device *, void *);
 intasmc_detach(struct device *, int);
+intasmc_activate(struct device *, int);
 
 /* wskbd hook functions */
 void   asmc_backlight(void *);
@@ -101,7 +102,7 @@ extern int (*wskbd_get_backlight)(struct
 extern int (*wskbd_set_backlight)(struct wskbd_backlight *);
 
 const struct cfattach asmc_ca = {
-   sizeof(struct asmc_softc), asmc_match, asmc_attach
+   sizeof(struct asmc_softc), asmc_match, asmc_attach, NULL, asmc_activate
 };
 
 struct cfdriver asmc_cd = {
@@ -355,6 +356,20 @@ asmc_detach(struct device *self, int fla
 
task_del(systq, >sc_task_backlight);
asmc_try(sc, ASMC_WRITE, "LKSB", buf, 2);
+   return 0;
+}
+
+int
+asmc_activate(struct device *self, int act)
+{
+   struct asmc_softc *sc = (struct asmc_softc *)self;
+
+   switch (act) {
+   case DVACT_WAKEUP:
+   asmc_backlight(sc);
+   break;
+   }
+
return 0;
 }