Hi, On 04/22/2010 05:02 PM, Takashi Iwai wrote: > At Thu, 22 Apr 2010 04:37:04 -0400, > Hans de Goede wrote: >> >> While working on the sound suspend / resume problems with my laptop >> I noticed that the hardware volume handling code in essence just detects >> key presses, and then does some hardcoded modification of the master volume >> based on which key is pressed. >> >> This made me think that clearly the right thing to do here is just report >> these keypresses to userspace as keypresses using an input device and let >> userspace decide what to with them. >> >> This patch does this, getting rid of the ugly direct ac97 writes from >> the tasklet, the ac97lock and the need for using a tasklet in general. >> >> As an added bonus the keys now work identical to volume keys on a (usb) >> keyboard with multimedia keys, providing visual feedback of the volume >> level change, and a better range of the volume control (with a properly >> configured desktop environment). > > I like the basic idea. However, two points to be fixed: > > - We need a kconfig to keep the old behavior. We are not allowed to > give any regression in general. > Yes, I already thought about this, and sort of expected this comment, but I opted to start with the straight forward patch. > - CONFIG_INPUT is tristate. It can be a module, thus the dependency > is a bit messy. Imagine you want to build snd-maestro3 into kernel > while CONFIG_INPUT=m is given. > Ugh, I would say just don't do that, but I understand that that is not an acceptable answer, and I see you've already provided a solution, thanks. > A hack to avoid to avoid the dependency is to create a bool Kconfig to > specify whether input feature is added or not; this is anyway needed > for the first reason above. Suppose it CONFIG_SND_MAESTRO3_INPUT. > > Then it looks like: > > config SND_MAESTRO3_INPUT > bool "enable input device for maestro3 volume switches" > depends on SND_MAESTRO3 > depends on INPUT=y || INPUT=SND_MAESTRO3 > help > .... > > Then you can use "#ifdef CONFIG_SND_MAESTRO3_INPUT" to determine > compile condition. If this isn't defined, keep the old behavior, > i.e. controlling ac97 registers directly. Ok, I'll respin this patches taking the above comments into account, I'll do this either tomorrow or sometime next week. Regards, Hans _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
