Hi,
On 7/30/07, Richard Hughes <hughsient@xxxxxxxxx> wrote:
> On 30/07/07, Matthew Garrett <mjg59@xxxxxxxxxxxxx> wrote:
> > On Sun, Jul 29, 2007 at 12:40:16PM -0400, Luming Yu wrote:
> > > switch (event) {
> > > case ACPI_VIDEO_NOTIFY_SWITCH: /* User requested a switch,
> > > * most likely via hotkey. */
> > > acpi_bus_generate_event(device, event, 0);
> > > + keycode = KEY_UNKNOWN;
> >
> > KEY_SWITCHVIDEOMODE ?
>
> Yes, please use this. This is what other ACPI drivers are using.
>
> > What's wrong with the existing KEY_BRIGHTNESSDOWN and KEY_BRIGHTNESSUP?
>
> Agree, this matches other drivers like thinkpad_acpi and sony-laptop.
>
> > > +#define KEY_BRIGHTNESS_ZERO 0x1fd
> > > +#define KEY_BRIGHTNESS_OFF 0x1fe
> >
> > You almost certainly want to go via linux-input if you're adding new
> > keycodes.
>
> Yup - do we have to distinguish between zero and off? Or is off
> "switch panel backlight off" and zero is "switch to lowest brightness
> level"?
>
> Thanks for doing this, this is another part or the puzzle to make keys
> just work in userspace and is very appreciated.
>
I have some concerns over this patch. Some of the events reported are
not the regular key presses that something needs to process and
perform an action. Consider ACPI_VIDEO_NOTIFY_SWITCH is "Used to
notify OSPM whenever the state of one of the output devices attached
to the VGA controller has been switched or toggled." As far as I
understand the event (whatever it was) was already processed by
firmware and we getting a notification event of system change. So I am
not sure that this event should be sent as a key event.
The brightness events on the other hand seem to be perfectly suited
for an input device (unless are are also already processed by the
firmware and action has already been taken).
--
Dmitry
-
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
