subject:"massage volume control task"

Re: massage volume control task

2015-01-18 Thread David Gwynne


 On 17 Jan 2015, at 11:06, Jim Smith j...@insec.sh wrote:
 
 On 2 Jan 2015, at 4:15 pm, David Gwynne da...@gwynne.id.au wrote:
 
 can someone test this?
 
 it allocates storage for the volume change details rather than cast
 arguments to a single global task.
 
 adds some safety while there if audio0 is a hotplug device.
 
 ok?
 
 The keyboard audio controls on my X120e following -current work
 same as before with this diff applied.

cool, thank you.

unless anyone objects im going to commit this tomorrow morning (tues 20th of 
jan around 11am, gmt+10).

cheers,
dlg

 
 
 Index: audio.c
 ===
 RCS file: /cvs/src/sys/dev/audio.c,v
 retrieving revision 1.125
 diff -u -p -r1.125 audio.c
 --- audio.c  19 Dec 2014 22:44:58 -  1.125
 +++ audio.c  2 Jan 2015 06:08:39 -
 @@ -465,11 +465,6 @@ audioattach(struct device *parent, struc
  }
  DPRINTF((audio_attach: inputs ports=0x%x, output ports=0x%x\n,
   sc-sc_inports.allports, sc-sc_outports.allports));
 -
 -#if NWSKBD  0
 -task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback, NULL,
 -NULL);
 -#endif /* NWSKBD  0 */
 }
 
 int
 @@ -3432,27 +3427,39 @@ filt_audiowrite(struct knote *kn, long h
 }
 
 #if NWSKBD  0
 +struct wskbd_vol_change {
 +struct task t;
 +long dir;
 +long out;
 +};
 +
 int
 wskbd_set_mixervolume(long dir, long out)
 {
  struct audio_softc *sc;
 +struct wskbd_vol_change *ch;
 
  if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
  DPRINTF((wskbd_set_mixervolume: audio_cd\n));
  return (ENXIO);
  }
 
 -task_del(systq, sc-sc_mixer_task);
 -task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback,
 -(void *)dir, (void *)out);
 -task_add(systq, sc-sc_mixer_task);
 +ch = malloc(sizeof(*ch), M_TEMP, M_NOWAIT);
 +if (ch == NULL)
 +return (ENOMEM);
 +
 +task_set(ch-t, wskbd_set_mixervolume_callback, ch, NULL);
 +ch-dir = dir;
 +ch-out = out;
 +task_add(systq, ch-t);
 
  return (0);
 }
 
 void
 -wskbd_set_mixervolume_callback(void *arg1, void *arg2)
 +wskbd_set_mixervolume_callback(void *xch, void *null)
 {
 +struct wskbd_vol_change *ch = xch;
  struct audio_softc *sc;
  struct au_mixer_ports *ports;
  mixer_devinfo_t mi;
 @@ -3461,19 +3468,19 @@ wskbd_set_mixervolume_callback(void *arg
  u_int gain;
  int error;
 
 -if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
 -DPRINTF((%s: audio_cd\n, __func__));
 -return;
 -}
 +dir = ch-dir;
 +out = ch-out;
 +free(ch, M_TEMP, sizeof(*ch));
 
 -dir = (long)arg1;
 -out = (long)arg2;
 +sc = (struct audio_softc *)device_lookup(audio_cd, 0);
 +if (sc == NULL)
 +return;
 
  ports = out ? sc-sc_outports : sc-sc_inports;
 
  if (ports-master == -1) {
  DPRINTF((%s: master == -1\n, __func__));
 -return;
 +goto done;
  }
 
  if (dir == 0) {
 @@ -3482,7 +3489,7 @@ wskbd_set_mixervolume_callback(void *arg
  error = au_get_mute(sc, ports, mute);
  if (error != 0) {
  DPRINTF((%s: au_get_mute: %d\n, __func__, error));
 -return;
 +goto done;
  }
 
  mute = !mute;
 @@ -3490,7 +3497,7 @@ wskbd_set_mixervolume_callback(void *arg
  error = au_set_mute(sc, ports, mute);
  if (error != 0) {
  DPRINTF((%s: au_set_mute: %d\n, __func__, error));
 -return;
 +goto done;
  }
  } else {
  /* Raise or lower volume */
 @@ -3499,7 +3506,7 @@ wskbd_set_mixervolume_callback(void *arg
  error = sc-hw_if-query_devinfo(sc-hw_hdl, mi);
  if (error != 0) {
  DPRINTF((%s: query_devinfo: %d\n, __func__, error));
 -return;
 +goto done;
  }
 
  au_get_gain(sc, ports, gain, balance);
 @@ -3512,8 +3519,11 @@ wskbd_set_mixervolume_callback(void *arg
  error = au_set_gain(sc, ports, gain, balance);
  if (error != 0) {
  DPRINTF((%s: au_set_gain: %d\n, __func__, error));
 -return;
 +goto done;
  }
  }
 +
 +done:
 +device_unref(sc-dev);
 }
 #endif /* NWSKBD  0 */

Re: massage volume control task

2015-01-16 Thread Jim Smith

On 2 Jan 2015, at 4:15 pm, David Gwynne da...@gwynne.id.au wrote:

 can someone test this?
 
 it allocates storage for the volume change details rather than cast
 arguments to a single global task.
 
 adds some safety while there if audio0 is a hotplug device.
 
 ok?

The keyboard audio controls on my X120e following -current work
same as before with this diff applied.

 
 Index: audio.c
 ===
 RCS file: /cvs/src/sys/dev/audio.c,v
 retrieving revision 1.125
 diff -u -p -r1.125 audio.c
 --- audio.c   19 Dec 2014 22:44:58 -  1.125
 +++ audio.c   2 Jan 2015 06:08:39 -
 @@ -465,11 +465,6 @@ audioattach(struct device *parent, struc
   }
   DPRINTF((audio_attach: inputs ports=0x%x, output ports=0x%x\n,
sc-sc_inports.allports, sc-sc_outports.allports));
 -
 -#if NWSKBD  0
 - task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback, NULL,
 - NULL);
 -#endif /* NWSKBD  0 */
 }
 
 int
 @@ -3432,27 +3427,39 @@ filt_audiowrite(struct knote *kn, long h
 }
 
 #if NWSKBD  0
 +struct wskbd_vol_change {
 + struct task t;
 + long dir;
 + long out;
 +};
 +
 int
 wskbd_set_mixervolume(long dir, long out)
 {
   struct audio_softc *sc;
 + struct wskbd_vol_change *ch;
 
   if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
   DPRINTF((wskbd_set_mixervolume: audio_cd\n));
   return (ENXIO);
   }
 
 - task_del(systq, sc-sc_mixer_task);
 - task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback,
 - (void *)dir, (void *)out);
 - task_add(systq, sc-sc_mixer_task);
 + ch = malloc(sizeof(*ch), M_TEMP, M_NOWAIT);
 + if (ch == NULL)
 + return (ENOMEM);
 +
 + task_set(ch-t, wskbd_set_mixervolume_callback, ch, NULL);
 + ch-dir = dir;
 + ch-out = out;
 + task_add(systq, ch-t);
 
   return (0);
 }
 
 void
 -wskbd_set_mixervolume_callback(void *arg1, void *arg2)
 +wskbd_set_mixervolume_callback(void *xch, void *null)
 {
 + struct wskbd_vol_change *ch = xch;
   struct audio_softc *sc;
   struct au_mixer_ports *ports;
   mixer_devinfo_t mi;
 @@ -3461,19 +3468,19 @@ wskbd_set_mixervolume_callback(void *arg
   u_int gain;
   int error;
 
 - if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
 - DPRINTF((%s: audio_cd\n, __func__));
 - return;
 - }
 + dir = ch-dir;
 + out = ch-out;
 + free(ch, M_TEMP, sizeof(*ch));
 
 - dir = (long)arg1;
 - out = (long)arg2;
 + sc = (struct audio_softc *)device_lookup(audio_cd, 0);
 + if (sc == NULL)
 + return;
 
   ports = out ? sc-sc_outports : sc-sc_inports;
 
   if (ports-master == -1) {
   DPRINTF((%s: master == -1\n, __func__));
 - return;
 + goto done;
   }
 
   if (dir == 0) {
 @@ -3482,7 +3489,7 @@ wskbd_set_mixervolume_callback(void *arg
   error = au_get_mute(sc, ports, mute);
   if (error != 0) {
   DPRINTF((%s: au_get_mute: %d\n, __func__, error));
 - return;
 + goto done;
   }
 
   mute = !mute;
 @@ -3490,7 +3497,7 @@ wskbd_set_mixervolume_callback(void *arg
   error = au_set_mute(sc, ports, mute);
   if (error != 0) {
   DPRINTF((%s: au_set_mute: %d\n, __func__, error));
 - return;
 + goto done;
   }
   } else {
   /* Raise or lower volume */
 @@ -3499,7 +3506,7 @@ wskbd_set_mixervolume_callback(void *arg
   error = sc-hw_if-query_devinfo(sc-hw_hdl, mi);
   if (error != 0) {
   DPRINTF((%s: query_devinfo: %d\n, __func__, error));
 - return;
 + goto done;
   }
 
   au_get_gain(sc, ports, gain, balance);
 @@ -3512,8 +3519,11 @@ wskbd_set_mixervolume_callback(void *arg
   error = au_set_gain(sc, ports, gain, balance);
   if (error != 0) {
   DPRINTF((%s: au_set_gain: %d\n, __func__, error));
 - return;
 + goto done;
   }
   }
 +
 +done:
 + device_unref(sc-dev);
 }
 #endif /* NWSKBD  0 */

Re: massage volume control task

2015-01-11 Thread David Gwynne


 On 3 Jan 2015, at 9:35 am, David Gwynne da...@gwynne.id.au wrote:
 
 
 On 2 Jan 2015, at 9:55 pm, Mark Kettenis mark.kette...@xs4all.nl wrote:
 
 Date: Fri, 2 Jan 2015 16:15:07 +1000
 From: David Gwynne da...@gwynne.id.au
 
 can someone test this?
 
 it allocates storage for the volume change details rather than cast
 arguments to a single global task.
 
 adds some safety while there if audio0 is a hotplug device.
 
 ok?
 
 The problem with this approach is that things silently fail when
 memory allocation fails.  Exactly the problem that workq_add_task()
 had.  Perhaps the best thing is to store the event in the softc,
 although then you'd probably need some locking to make sure the task
 sees a consistent dir/out pair.
 
 is silently fail a pun here?
 
 you can lose events either way. if you get more than one keypress before the 
 task runs, you'll lose. if you are in a situation where you cant allocate 
 memory, you lose.
 
 i think the code generally is bogus so im trying to do as little as possible 
 to it. the more i look at it the more i want to just remove it.
 
 its been unreliable in all incarnations.

did anyone try this diff?

dlg

 
 
 Index: audio.c
 ===
 RCS file: /cvs/src/sys/dev/audio.c,v
 retrieving revision 1.125
 diff -u -p -r1.125 audio.c
 --- audio.c 19 Dec 2014 22:44:58 -  1.125
 +++ audio.c 2 Jan 2015 06:08:39 -
 @@ -465,11 +465,6 @@ audioattach(struct device *parent, struc
 }
 DPRINTF((audio_attach: inputs ports=0x%x, output ports=0x%x\n,
  sc-sc_inports.allports, sc-sc_outports.allports));
 -
 -#if NWSKBD  0
 -   task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback, NULL,
 -   NULL);
 -#endif /* NWSKBD  0 */
 }
 
 int
 @@ -3432,27 +3427,39 @@ filt_audiowrite(struct knote *kn, long h
 }
 
 #if NWSKBD  0
 +struct wskbd_vol_change {
 +   struct task t;
 +   long dir;
 +   long out;
 +};
 +
 int
 wskbd_set_mixervolume(long dir, long out)
 {
 struct audio_softc *sc;
 +   struct wskbd_vol_change *ch;
 
 if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
 DPRINTF((wskbd_set_mixervolume: audio_cd\n));
 return (ENXIO);
 }
 
 -   task_del(systq, sc-sc_mixer_task);
 -   task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback,
 -   (void *)dir, (void *)out);
 -   task_add(systq, sc-sc_mixer_task);
 +   ch = malloc(sizeof(*ch), M_TEMP, M_NOWAIT);
 +   if (ch == NULL)
 +   return (ENOMEM);
 +
 +   task_set(ch-t, wskbd_set_mixervolume_callback, ch, NULL);
 +   ch-dir = dir;
 +   ch-out = out;
 +   task_add(systq, ch-t);
 
 return (0);
 }
 
 void
 -wskbd_set_mixervolume_callback(void *arg1, void *arg2)
 +wskbd_set_mixervolume_callback(void *xch, void *null)
 {
 +   struct wskbd_vol_change *ch = xch;
 struct audio_softc *sc;
 struct au_mixer_ports *ports;
 mixer_devinfo_t mi;
 @@ -3461,19 +3468,19 @@ wskbd_set_mixervolume_callback(void *arg
 u_int gain;
 int error;
 
 -   if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
 -   DPRINTF((%s: audio_cd\n, __func__));
 -   return;
 -   }
 +   dir = ch-dir;
 +   out = ch-out;
 +   free(ch, M_TEMP, sizeof(*ch));
 
 -   dir = (long)arg1;
 -   out = (long)arg2;
 +   sc = (struct audio_softc *)device_lookup(audio_cd, 0);
 +   if (sc == NULL)
 +   return;
 
 ports = out ? sc-sc_outports : sc-sc_inports;
 
 if (ports-master == -1) {
 DPRINTF((%s: master == -1\n, __func__));
 -   return;
 +   goto done;
 }
 
 if (dir == 0) {
 @@ -3482,7 +3489,7 @@ wskbd_set_mixervolume_callback(void *arg
 error = au_get_mute(sc, ports, mute);
 if (error != 0) {
 DPRINTF((%s: au_get_mute: %d\n, __func__, error));
 -   return;
 +   goto done;
 }
 
 mute = !mute;
 @@ -3490,7 +3497,7 @@ wskbd_set_mixervolume_callback(void *arg
 error = au_set_mute(sc, ports, mute);
 if (error != 0) {
 DPRINTF((%s: au_set_mute: %d\n, __func__, error));
 -   return;
 +   goto done;
 }
 } else {
 /* Raise or lower volume */
 @@ -3499,7 +3506,7 @@ wskbd_set_mixervolume_callback(void *arg
 error = sc-hw_if-query_devinfo(sc-hw_hdl, mi);
 if (error != 0) {
 DPRINTF((%s: query_devinfo: %d\n, __func__, error));
 -   return;
 +   goto done;
 }
 
 au_get_gain(sc, ports, gain, balance);
 @@ -3512,8 +3519,11 @@ wskbd_set_mixervolume_callback(void *arg
 error = au_set_gain(sc, ports, gain, balance);
 if (error != 0) {
 DPRINTF((%s: au_set_gain: %d\n, __func__, error));
 -   return;
 +   goto done;
 }
 }
 +
 +done:
 +   device_unref(sc-dev);
 }

Re: massage volume control task

2015-01-02 Thread Mark Kettenis

 Date: Fri, 2 Jan 2015 16:15:07 +1000
 From: David Gwynne da...@gwynne.id.au
 
 can someone test this?
 
 it allocates storage for the volume change details rather than cast
 arguments to a single global task.
 
 adds some safety while there if audio0 is a hotplug device.
 
 ok?

The problem with this approach is that things silently fail when
memory allocation fails.  Exactly the problem that workq_add_task()
had.  Perhaps the best thing is to store the event in the softc,
although then you'd probably need some locking to make sure the task
sees a consistent dir/out pair.

 Index: audio.c
 ===
 RCS file: /cvs/src/sys/dev/audio.c,v
 retrieving revision 1.125
 diff -u -p -r1.125 audio.c
 --- audio.c   19 Dec 2014 22:44:58 -  1.125
 +++ audio.c   2 Jan 2015 06:08:39 -
 @@ -465,11 +465,6 @@ audioattach(struct device *parent, struc
   }
   DPRINTF((audio_attach: inputs ports=0x%x, output ports=0x%x\n,
sc-sc_inports.allports, sc-sc_outports.allports));
 -
 -#if NWSKBD  0
 - task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback, NULL,
 - NULL);
 -#endif /* NWSKBD  0 */
  }
  
  int
 @@ -3432,27 +3427,39 @@ filt_audiowrite(struct knote *kn, long h
  }
  
  #if NWSKBD  0
 +struct wskbd_vol_change {
 + struct task t;
 + long dir;
 + long out;
 +};
 +
  int
  wskbd_set_mixervolume(long dir, long out)
  {
   struct audio_softc *sc;
 + struct wskbd_vol_change *ch;
  
   if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
   DPRINTF((wskbd_set_mixervolume: audio_cd\n));
   return (ENXIO);
   }
  
 - task_del(systq, sc-sc_mixer_task);
 - task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback,
 - (void *)dir, (void *)out);
 - task_add(systq, sc-sc_mixer_task);
 + ch = malloc(sizeof(*ch), M_TEMP, M_NOWAIT);
 + if (ch == NULL)
 + return (ENOMEM);
 +
 + task_set(ch-t, wskbd_set_mixervolume_callback, ch, NULL);
 + ch-dir = dir;
 + ch-out = out;
 + task_add(systq, ch-t);
  
   return (0);
  }
  
  void
 -wskbd_set_mixervolume_callback(void *arg1, void *arg2)
 +wskbd_set_mixervolume_callback(void *xch, void *null)
  {
 + struct wskbd_vol_change *ch = xch;
   struct audio_softc *sc;
   struct au_mixer_ports *ports;
   mixer_devinfo_t mi;
 @@ -3461,19 +3468,19 @@ wskbd_set_mixervolume_callback(void *arg
   u_int gain;
   int error;
  
 - if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
 - DPRINTF((%s: audio_cd\n, __func__));
 - return;
 - }
 + dir = ch-dir;
 + out = ch-out;
 + free(ch, M_TEMP, sizeof(*ch));
  
 - dir = (long)arg1;
 - out = (long)arg2;
 + sc = (struct audio_softc *)device_lookup(audio_cd, 0);
 + if (sc == NULL)
 + return;
  
   ports = out ? sc-sc_outports : sc-sc_inports;
  
   if (ports-master == -1) {
   DPRINTF((%s: master == -1\n, __func__));
 - return;
 + goto done;
   }
  
   if (dir == 0) {
 @@ -3482,7 +3489,7 @@ wskbd_set_mixervolume_callback(void *arg
   error = au_get_mute(sc, ports, mute);
   if (error != 0) {
   DPRINTF((%s: au_get_mute: %d\n, __func__, error));
 - return;
 + goto done;
   }
  
   mute = !mute;
 @@ -3490,7 +3497,7 @@ wskbd_set_mixervolume_callback(void *arg
   error = au_set_mute(sc, ports, mute);
   if (error != 0) {
   DPRINTF((%s: au_set_mute: %d\n, __func__, error));
 - return;
 + goto done;
   }
   } else {
   /* Raise or lower volume */
 @@ -3499,7 +3506,7 @@ wskbd_set_mixervolume_callback(void *arg
   error = sc-hw_if-query_devinfo(sc-hw_hdl, mi);
   if (error != 0) {
   DPRINTF((%s: query_devinfo: %d\n, __func__, error));
 - return;
 + goto done;
   }
  
   au_get_gain(sc, ports, gain, balance);
 @@ -3512,8 +3519,11 @@ wskbd_set_mixervolume_callback(void *arg
   error = au_set_gain(sc, ports, gain, balance);
   if (error != 0) {
   DPRINTF((%s: au_set_gain: %d\n, __func__, error));
 - return;
 + goto done;
   }
   }
 +
 +done:
 + device_unref(sc-dev);
  }
  #endif /* NWSKBD  0 */

Re: massage volume control task

2015-01-02 Thread David Gwynne


 On 2 Jan 2015, at 9:55 pm, Mark Kettenis mark.kette...@xs4all.nl wrote:
 
 Date: Fri, 2 Jan 2015 16:15:07 +1000
 From: David Gwynne da...@gwynne.id.au
 
 can someone test this?
 
 it allocates storage for the volume change details rather than cast
 arguments to a single global task.
 
 adds some safety while there if audio0 is a hotplug device.
 
 ok?
 
 The problem with this approach is that things silently fail when
 memory allocation fails.  Exactly the problem that workq_add_task()
 had.  Perhaps the best thing is to store the event in the softc,
 although then you'd probably need some locking to make sure the task
 sees a consistent dir/out pair.

is silently fail a pun here?

you can lose events either way. if you get more than one keypress before the 
task runs, you'll lose. if you are in a situation where you cant allocate 
memory, you lose.

i think the code generally is bogus so im trying to do as little as possible to 
it. the more i look at it the more i want to just remove it.

its been unreliable in all incarnations.

 
 Index: audio.c
 ===
 RCS file: /cvs/src/sys/dev/audio.c,v
 retrieving revision 1.125
 diff -u -p -r1.125 audio.c
 --- audio.c  19 Dec 2014 22:44:58 -  1.125
 +++ audio.c  2 Jan 2015 06:08:39 -
 @@ -465,11 +465,6 @@ audioattach(struct device *parent, struc
  }
  DPRINTF((audio_attach: inputs ports=0x%x, output ports=0x%x\n,
   sc-sc_inports.allports, sc-sc_outports.allports));
 -
 -#if NWSKBD  0
 -task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback, NULL,
 -NULL);
 -#endif /* NWSKBD  0 */
 }
 
 int
 @@ -3432,27 +3427,39 @@ filt_audiowrite(struct knote *kn, long h
 }
 
 #if NWSKBD  0
 +struct wskbd_vol_change {
 +struct task t;
 +long dir;
 +long out;
 +};
 +
 int
 wskbd_set_mixervolume(long dir, long out)
 {
  struct audio_softc *sc;
 +struct wskbd_vol_change *ch;
 
  if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
  DPRINTF((wskbd_set_mixervolume: audio_cd\n));
  return (ENXIO);
  }
 
 -task_del(systq, sc-sc_mixer_task);
 -task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback,
 -(void *)dir, (void *)out);
 -task_add(systq, sc-sc_mixer_task);
 +ch = malloc(sizeof(*ch), M_TEMP, M_NOWAIT);
 +if (ch == NULL)
 +return (ENOMEM);
 +
 +task_set(ch-t, wskbd_set_mixervolume_callback, ch, NULL);
 +ch-dir = dir;
 +ch-out = out;
 +task_add(systq, ch-t);
 
  return (0);
 }
 
 void
 -wskbd_set_mixervolume_callback(void *arg1, void *arg2)
 +wskbd_set_mixervolume_callback(void *xch, void *null)
 {
 +struct wskbd_vol_change *ch = xch;
  struct audio_softc *sc;
  struct au_mixer_ports *ports;
  mixer_devinfo_t mi;
 @@ -3461,19 +3468,19 @@ wskbd_set_mixervolume_callback(void *arg
  u_int gain;
  int error;
 
 -if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
 -DPRINTF((%s: audio_cd\n, __func__));
 -return;
 -}
 +dir = ch-dir;
 +out = ch-out;
 +free(ch, M_TEMP, sizeof(*ch));
 
 -dir = (long)arg1;
 -out = (long)arg2;
 +sc = (struct audio_softc *)device_lookup(audio_cd, 0);
 +if (sc == NULL)
 +return;
 
  ports = out ? sc-sc_outports : sc-sc_inports;
 
  if (ports-master == -1) {
  DPRINTF((%s: master == -1\n, __func__));
 -return;
 +goto done;
  }
 
  if (dir == 0) {
 @@ -3482,7 +3489,7 @@ wskbd_set_mixervolume_callback(void *arg
  error = au_get_mute(sc, ports, mute);
  if (error != 0) {
  DPRINTF((%s: au_get_mute: %d\n, __func__, error));
 -return;
 +goto done;
  }
 
  mute = !mute;
 @@ -3490,7 +3497,7 @@ wskbd_set_mixervolume_callback(void *arg
  error = au_set_mute(sc, ports, mute);
  if (error != 0) {
  DPRINTF((%s: au_set_mute: %d\n, __func__, error));
 -return;
 +goto done;
  }
  } else {
  /* Raise or lower volume */
 @@ -3499,7 +3506,7 @@ wskbd_set_mixervolume_callback(void *arg
  error = sc-hw_if-query_devinfo(sc-hw_hdl, mi);
  if (error != 0) {
  DPRINTF((%s: query_devinfo: %d\n, __func__, error));
 -return;
 +goto done;
  }
 
  au_get_gain(sc, ports, gain, balance);
 @@ -3512,8 +3519,11 @@ wskbd_set_mixervolume_callback(void *arg
  error = au_set_gain(sc, ports, gain, balance);
  if (error != 0) {
  DPRINTF((%s: au_set_gain: %d\n, __func__, error));
 -return;
 +goto done;
  }
  }
 +
 +done:
 +device_unref(sc-dev);
 }
 #endif /* NWSKBD  0 */

massage volume control task

2015-01-01 Thread David Gwynne

can someone test this?

it allocates storage for the volume change details rather than cast
arguments to a single global task.

adds some safety while there if audio0 is a hotplug device.

ok?

Index: audio.c
===
RCS file: /cvs/src/sys/dev/audio.c,v
retrieving revision 1.125
diff -u -p -r1.125 audio.c
--- audio.c 19 Dec 2014 22:44:58 -  1.125
+++ audio.c 2 Jan 2015 06:08:39 -
@@ -465,11 +465,6 @@ audioattach(struct device *parent, struc
}
DPRINTF((audio_attach: inputs ports=0x%x, output ports=0x%x\n,
 sc-sc_inports.allports, sc-sc_outports.allports));
-
-#if NWSKBD  0
-   task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback, NULL,
-   NULL);
-#endif /* NWSKBD  0 */
 }
 
 int
@@ -3432,27 +3427,39 @@ filt_audiowrite(struct knote *kn, long h
 }
 
 #if NWSKBD  0
+struct wskbd_vol_change {
+   struct task t;
+   long dir;
+   long out;
+};
+
 int
 wskbd_set_mixervolume(long dir, long out)
 {
struct audio_softc *sc;
+   struct wskbd_vol_change *ch;
 
if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
DPRINTF((wskbd_set_mixervolume: audio_cd\n));
return (ENXIO);
}
 
-   task_del(systq, sc-sc_mixer_task);
-   task_set(sc-sc_mixer_task, wskbd_set_mixervolume_callback,
-   (void *)dir, (void *)out);
-   task_add(systq, sc-sc_mixer_task);
+   ch = malloc(sizeof(*ch), M_TEMP, M_NOWAIT);
+   if (ch == NULL)
+   return (ENOMEM);
+
+   task_set(ch-t, wskbd_set_mixervolume_callback, ch, NULL);
+   ch-dir = dir;
+   ch-out = out;
+   task_add(systq, ch-t);
 
return (0);
 }
 
 void
-wskbd_set_mixervolume_callback(void *arg1, void *arg2)
+wskbd_set_mixervolume_callback(void *xch, void *null)
 {
+   struct wskbd_vol_change *ch = xch;
struct audio_softc *sc;
struct au_mixer_ports *ports;
mixer_devinfo_t mi;
@@ -3461,19 +3468,19 @@ wskbd_set_mixervolume_callback(void *arg
u_int gain;
int error;
 
-   if (audio_cd.cd_ndevs == 0 || (sc = audio_cd.cd_devs[0]) == NULL) {
-   DPRINTF((%s: audio_cd\n, __func__));
-   return;
-   }
+   dir = ch-dir;
+   out = ch-out;
+   free(ch, M_TEMP, sizeof(*ch));
 
-   dir = (long)arg1;
-   out = (long)arg2;
+   sc = (struct audio_softc *)device_lookup(audio_cd, 0);
+   if (sc == NULL)
+   return;
 
ports = out ? sc-sc_outports : sc-sc_inports;
 
if (ports-master == -1) {
DPRINTF((%s: master == -1\n, __func__));
-   return;
+   goto done;
}
 
if (dir == 0) {
@@ -3482,7 +3489,7 @@ wskbd_set_mixervolume_callback(void *arg
error = au_get_mute(sc, ports, mute);
if (error != 0) {
DPRINTF((%s: au_get_mute: %d\n, __func__, error));
-   return;
+   goto done;
}
 
mute = !mute;
@@ -3490,7 +3497,7 @@ wskbd_set_mixervolume_callback(void *arg
error = au_set_mute(sc, ports, mute);
if (error != 0) {
DPRINTF((%s: au_set_mute: %d\n, __func__, error));
-   return;
+   goto done;
}
} else {
/* Raise or lower volume */
@@ -3499,7 +3506,7 @@ wskbd_set_mixervolume_callback(void *arg
error = sc-hw_if-query_devinfo(sc-hw_hdl, mi);
if (error != 0) {
DPRINTF((%s: query_devinfo: %d\n, __func__, error));
-   return;
+   goto done;
}
 
au_get_gain(sc, ports, gain, balance);
@@ -3512,8 +3519,11 @@ wskbd_set_mixervolume_callback(void *arg
error = au_set_gain(sc, ports, gain, balance);
if (error != 0) {
DPRINTF((%s: au_set_gain: %d\n, __func__, error));
-   return;
+   goto done;
}
}
+
+done:
+   device_unref(sc-dev);
 }
 #endif /* NWSKBD  0 */

Re: massage volume control task

Re: massage volume control task

Re: massage volume control task

Re: massage volume control task

Re: massage volume control task

massage volume control task

6 matches

Site Navigation

Mail list logo

Footer information