Re: [PATCH 57/98] sony-laptop: Add support for extra keyboard events

Matthias Welwarsky
2009-03-30T06:20:30Z

On Monday 30 March 2009 05:52:39 Mattia Dongili wrote:
> On Sun, Mar 29, 2009 at 05:41:07PM +0100, Matthew Garrett wrote:
> > On Sun, Mar 29, 2009 at 06:38:14PM +0200, Matthias Welwarsky wrote:
> > > Whoops, just noticed that there was a bug in my patch. No wonder it
> > > confused you. Should untranslated events >= 0x90 still generate ACPI
> > > events? In that case, the following patch would fix it. If not, the
> > > acpi event reporting must be moved into the else path, too.
> >
> > I don't think it's likely to be useful - if we ever implement support
> > for them then we'll generate different events and generate user
> > confusion.
>
> yes, but up to 2.6.29 they are being reported for all the models
> supported by the former SNC initialization code. I'd rather keep them
> until /proc/acpi/event goes away.

Hm, but if you keep them, there would be the possibility of ambiguous events.
Events >= 0x90 are being translated and possibly (mostly) become < 0x90, so
there's something like a namespace clash here. I think there should be a
marker for translated events.

> > > --- sony-laptop.c.orig 2009-03-29 12:41:44.000000000 +0200
> > > +++ sony-laptop.c 2009-03-29 18:36:31.000000000 +0200
> > > @@ -865,7 +865,7 @@
> > > static struct sony_nc_event sony_100_events[] = {
> > > { 0x90, SONYPI_EVENT_PKEY_P1 },
> > > { 0x10, SONYPI_EVENT_ANYBUTTON_RELEASED },
> > > - { 0x91, SONYPI_EVENT_PKEY_P1 },
> > > + { 0x91, SONYPI_EVENT_PKEY_P2 },
> > > { 0x11, SONYPI_EVENT_ANYBUTTON_RELEASED },
> > > { 0x81, SONYPI_EVENT_FNKEY_F1 },
> > > { 0x01, SONYPI_EVENT_FNKEY_RELEASED },
> > > @@ -929,7 +929,7 @@
> > > if (sony_find_snc_handle(0x127) == ev)
> > > key_handle = 0x127;
> > >
> > > - if (handle) {
> > > + if (key_handle) {
> > > struct sony_nc_event *key_event;
> > >
> > > if (sony_call_snc_handle(key_handle, 0x200, &result))
> > > @@ -955,15 +955,16 @@
> > > printk(KERN_INFO DRV_PFX
> > > "Unknown event: 0x%x 0x%x\n", key_handle,
> > > ev);
> > > - }
> > > + } else
> > > + sony_laptop_report_input_event(ev);
> > > } else if (sony_find_snc_handle(0x124) == ev) {
> > > sony_nc_rfkill_update();
> > > return;
> > > }
> > > - }
> > > + } else
> > > + sony_laptop_report_input_event(ev);
> > >
> > > dprintk("sony_acpi_notify, event: 0x%.2x\n", ev);
> > > - sony_laptop_report_input_event(ev);
> > > acpi_bus_generate_proc_event(sony_nc_acpi_device, 1, ev);
> > > }
> >
> > Yes, that looks good, just move the acpi_bus_generate_proc_event into
> > the same blocks as sony_laptop_report_input_event.
> >
> > Acked-by: Matthew Garrett <mjg@redhat.com>
>
> I'll take this version of the patch.

What's about rfkill then? Should rfkill hardware switch activations also send
an acpi event in addition? Currently they don't.

> Len, this patch isn't upstream yet, would you prefer me to resend
> only this one with the fixes from Matthias merged in or do you want it
> as a separate fix?
>
> Also, Matthias can we have your signoff line?

I'll resend the patch with a proper sign-off once we're clear about these
questions, ok?

regards,
matthias