On Fri, 2007-08-24 at 02:32 +0100, Matthew Garrett wrote: > > +static int acpi_fuj02b1_get_volume_state(void *data) > > This probably wants to be exported as an ALSA mixer rather than a > special device in /proc. > > > +static int acpi_fuj02b1_get_brightness_state(void *data) > > And this certainly wants to use the backlight class. > > > +static int acpi_fuj02b1_get_pointer_state(void *data) > > Less sure about this one, but I'm not sure exactly what it's meant to > do - control whether the mouse pointer is enabled? > > > + acpi_bus_generate_event(device, event, (u32) fuj02b1->volume_level | (fuj02b1->mute_state * 0x1000000) | 0x10000000); > > ALSA is capable of generating events when the mixer volume changes, so I > don't think there's a need to explicitly generate an event here > (assuming you move the mixer over to ALSA) > > > + acpi_bus_generate_event(device, event, (u32) fuj02b1->brightness_level | 0x20000000); > > There's some disagreement over how to do this correctly. The currently > implemented way is for you to send KEY_BRIGHTNESSUP or > KEY_BRIGHTNESSDOWN and then let any userspace application check HAL to > determine whether it should do anything (we'd add an entry for Fujitsus > to indicate that the keys were merely notifications rather than > instructions), but there's also the argument that these keys should only > be used if they're acting as instructions to the software. > I have the same question for ACPI video driver. Take KEY_BRIGHTNESSUP/KEY_BRIGHTNESSDOWN for example, if ACPI video driver gets such a notification, it will change the brightness level first and then send an event to user space. But IMO, what the driver need to do is to provide a user space I/F first, /sys/class/backlight/acpi_videox/... in this case, and send a notification when keys are pressed. Any comments about this? I'll send a patch to make ACPI video driver do nothing but export the events to user space in the notify handler if you guys think it's okay. Thanks, Rui > > + acpi_bus_generate_event(device, event, (u32) fuj02b1->pointer_state | 0x30000000); > > What this one should do depends on what the pointer state actually means > :) > > The idea here is to present consistent interfaces whenever possible, > allowing software to make use of them without having to know anything > about the specifics of the Fujitsu hardware. The drivers currently in > the kernel are gradually being ported over to this. > - To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
