On Wed, Apr 21, 2010 at 11:04:09AM -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 it. The maestro2 code is nearly identical. Any chance you'd
give it the same treatment? I should be able to dig up a few laptops to
test both drivers if necessary.
<snip>
> @@ -2524,6 +2494,42 @@ static int m3_resume(struct pci_dev *pci)
> }
> #endif /* CONFIG_PM */
>
> +#ifdef CONFIG_INPUT
> +static int __devinit snd_m3_input_register(struct snd_m3 *chip)
> +{
> + struct input_dev *input_dev;
> + int err;
> +
> + input_dev = input_allocate_device();
> + if (!input_dev)
> + return -ENOMEM;
> +
> + snprintf(chip->phys, sizeof(chip->phys), "pci-%s/input0",
> + pci_name(chip->pci));
What's the proper format of phys? I see gameport stuff uses
pci%s/gameport0, ir stuff uses pci-%s/ir0. I can't immediately find any
other pci input things.
> +
> + input_dev->name = chip->card->driver;
> + input_dev->phys = chip->phys;
> + input_dev->id.bustype = BUS_PCI;
> + input_dev->id.vendor = chip->pci->vendor;
> + input_dev->id.product = chip->pci->device;
> + input_dev->dev.parent = &chip->pci->dev;
> +
> + input_dev->evbit[0] = BIT_MASK(EV_KEY);
> + input_dev->keybit[BIT_WORD(KEY_MUTE)] |= BIT_MASK(KEY_MUTE);
> + input_dev->keybit[BIT_WORD(KEY_VOLUMEDOWN)] |= BIT_MASK(KEY_VOLUMEDOWN);
> + input_dev->keybit[BIT_WORD(KEY_VOLUMEUP)] |= BIT_MASK(KEY_VOLUMEUP);
__set_bit() perhaps
> +
> + err = input_register_device(input_dev);
> + if (err) {
> + input_dev->dev.parent = NULL;
Is this nullification needed?
> + input_free_device(input_dev);
> + } else {
> + chip->input_dev = input_dev;
> + }
> +
> + return err;
> +}
> +#endif /* CONFIG_INPUT */
>
> /*
> */
--
Ville Syrjälä
syrjala@xxxxxx
http://www.sci.fi/~syrjala/
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
