On Tue, 12 Jul 2011 17:51:36 -0400, Matthew Garrett <mjg@xxxxxxxxxx> wrote:
> - keycode = KEY_SWITCHVIDEOMODE;
> + if (!acpi_notifier_call_chain(device, event, 0))
> + keycode = KEY_SWITCHVIDEOMODE;
Right, acpi_notify_call_chain returns -EINVAL when the underlying
function call returns NOTIFY_BAD.
> - acpi_notifier_call_chain(device, event, 0);
> + if (event != ACPI_VIDEO_NOTIFY_SWITCH)
> + acpi_notifier_call_chain(device, event, 0);
Not ideal to have two calls to this function only one of which will be
used. Could look like
bool check_call_chain = false;
...
case ACPI_VIDEO_NOTIFY_SWITCH:
acpi_bus_generate_proc_event(device, event, 0);
keycode = KEY_SWITCHVIDEOMODE;
check_call_chain = true
break;
...
if (acpi_notifier_call_chain(device, event, 0) < 0)
if (check_call_chain)
keycode = 0;
Feel free to just call me for bikeshedding.
> + if (strcmp(event->device_class, ACPI_VIDEO_CLASS))
> + return NOTIFY_DONE;
I'm not entirely clear on accepted coding style here, but I'd much
rather this look like:
if (strcmp(event->device_class, ACPI_VIDEO_CLASS) != 0)
I must have read this four times before I figured out that you weren't
early returning on all "video" devices...
> +
> + if (event->type == 0x80 && !(acpi->cevt & 0x1))
> + ret = NOTIFY_BAD;
> +
Seriously? It's some kind of magic non-switch switch event? And you can
tell by checking magic bits within the event?
How can I test this and know if it works?
--
keith.packard@xxxxxxxxx
Attachment:
pgpFT3SxsmULV.pgp
Description: PGP signature
