Hi,
On 04/21/2010 06:05 PM, Ville Syrjälä wrote:
> 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?
Yes, I already noticed it was nearly identical, but I didn't work
on it as I have no hardware to test any changes ...
> I should be able to dig up a few laptops to
> test both drivers if necessary.
I'll do an identical patch for the meastro2, if you could test that that would
be great.
>
> <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.
>
I've no idea, I took the pci-%s/ part from other pci drivers registering
input devices and the input0 part is based on doing:
cat /sys/class/input/input?/phys
On my system which yields a string ending in input0 for almost all
input devices.
>> +
>> + 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
Ah yes, good one.
>
>> +
>> + err = input_register_device(input_dev);
>> + if (err) {
>> + input_dev->dev.parent = NULL;
>
> Is this nullification needed?
>
I copy pasted this from gspca's input handling code, as I'm
familiar with the gspca code, but looking at what
input_free_device actually does: no.
I'll respin the patch with these 2 issues fixed and repost it
+ an identical one for the es1968 driver.
>> + input_free_device(input_dev);
>> + } else {
>> + chip->input_dev = input_dev;
>> + }
>> +
>> + return err;
>> +}
>> +#endif /* CONFIG_INPUT */
>>
>> /*
>> */
>
Regards,
Hans
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
