Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1314015 > unrolled thread

Re: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell Vostro V131

Started byPali Rohár <pali.rohar@gmail.com>
First post2016-01-21 10:10 +0100
Last post2016-01-22 12:10 +0100
Articles 6 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell  Vostro V131 Pali Rohár <pali.rohar@gmail.com> - 2016-01-21 10:10 +0100
    Re: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell  Vostro V131 Michał Kępień <kernel@kempniu.pl> - 2016-01-21 12:00 +0100
      Re: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell  Vostro V131 Pali Rohár <pali.rohar@gmail.com> - 2016-01-21 14:50 +0100
        Re: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell  Vostro V131 Michał Kępień <kernel@kempniu.pl> - 2016-01-21 16:00 +0100
          Re: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell  Vostro V131 Pali Rohár <pali.rohar@gmail.com> - 2016-01-21 16:50 +0100
            Re: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell  Vostro V131 Michał Kępień <kernel@kempniu.pl> - 2016-01-22 12:10 +0100

#1314015 — Re: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell Vostro V131

FromPali Rohár <pali.rohar@gmail.com>
Date2016-01-21 10:10 +0100
SubjectRe: [PATCH v2] dell-wmi: process Dell Instant Launch hotkey on Dell Vostro V131
Message-ID<qTjpE-39M-21@gated-at.bofh.it>
Michał, can you prepare new (v3) version of this patch? Now required
acpi video changes are included and so dell-wmi changes should go to...
To finally fix this keypress bug on Dell Vostro V131 machine.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [next] | [standalone]


#1314094

FromMichał Kępień <kernel@kempniu.pl>
Date2016-01-21 12:00 +0100
Message-ID<qTl85-45V-1@gated-at.bofh.it>
In reply to#1314015
> Michał, can you prepare new (v3) version of this patch? Now required
> acpi video changes are included and so dell-wmi changes should go to...
> To finally fix this keypress bug on Dell Vostro V131 machine.

I keep this on my to-do list, but the updated patch will depend on the
final version of the SMBIOS API rework, so I guess there is little point
in posting it now as that API is subject to change.  But rest assured
that as soon as the final version of the API rework series (which I have
yet to prepare, of course) gets applied by Darren, I will post a v3 of
this patch - after all, it is the very reason I am working on the API
rework.

-- 
Best regards,
Michał Kępień

[toc] | [prev] | [next] | [standalone]


#1314201

FromPali Rohár <pali.rohar@gmail.com>
Date2016-01-21 14:50 +0100
Message-ID<qTnMC-5Vj-1@gated-at.bofh.it>
In reply to#1314094
On Thursday 21 January 2016 11:52:03 Michał Kępień wrote:
> > Michał, can you prepare new (v3) version of this patch? Now required
> > acpi video changes are included and so dell-wmi changes should go to...
> > To finally fix this keypress bug on Dell Vostro V131 machine.
> 
> I keep this on my to-do list, but the updated patch will depend on the
> final version of the SMBIOS API rework, so I guess there is little point
> in posting it now as that API is subject to change.  But rest assured
> that as soon as the final version of the API rework series (which I have
> yet to prepare, of course) gets applied by Darren, I will post a v3 of
> this patch - after all, it is the very reason I am working on the API
> rework.

There is still need to patch dell-wmi.c? And this change does not depend
on another SMBIOS change (in dell-laptop), right?

I still have in my mind some changes for dell-wmi.c code but I'm waiting
until all other changes for dell-wmi.c will be merged to prevent
conflicts and needed rebase.

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]


#1314244

FromMichał Kępień <kernel@kempniu.pl>
Date2016-01-21 16:00 +0100
Message-ID<qToSm-6CC-23@gated-at.bofh.it>
In reply to#1314201
> > > Michał, can you prepare new (v3) version of this patch? Now required
> > > acpi video changes are included and so dell-wmi changes should go to...
> > > To finally fix this keypress bug on Dell Vostro V131 machine.
> > 
> > I keep this on my to-do list, but the updated patch will depend on the
> > final version of the SMBIOS API rework, so I guess there is little point
> > in posting it now as that API is subject to change.  But rest assured
> > that as soon as the final version of the API rework series (which I have
> > yet to prepare, of course) gets applied by Darren, I will post a v3 of
> > this patch - after all, it is the very reason I am working on the API
> > rework.
> 
> There is still need to patch dell-wmi.c? And this change does not depend
> on another SMBIOS change (in dell-laptop), right?

Well, back in December, you wrote [1]:

> This patch is not enough for enabling 0xe025 key on that Vostro machine.
> Some extra SMBIOS call is needed, without them ACPI will not send WMI
> keypress event.
> 
> (...)
> 
> Maybe now it could make sense to unify Dell SMBIOS API in kernel and
> move common functions to one place and let drivers to use just common
> functions. According to older Dell ACPI WMI documentation in DMI is bit
> which specify if BIOS support SMBIOS via WMI or not.
> 
> At least I think this one patch should not be included into kernel until
> there will be full support for 0xe025 key (adding that SMBIOS call).

From the above I understood that first you want to unify the Dell SMBIOS
API used throughout the kernel (that's currently in progress), so that
it can then be used in dell-wmi as well to perform the SMBIOS call
needed on the Vostro V131.

If you want me to just rework the patch so that it doesn't introduce a
quirk structure, I recalled another reason to use it after all: there
are other Dell laptops which require the special SMI for enabling WMI
events, but report the Dell Instant Launch Hotkey using a different WMI
event code [2].  So I'd say that two separate issues should be fixed
using DMI matching:

  * whether the special SMBIOS call is required,
  * whether the 0x0e25 event should be translated into a keypress.

A quirk structure looks like an elegant way to deal with this.

Could you please advise how you would like me to proceed with this?

[1] http://www.spinics.net/lists/platform-driver-x86/msg07845.html
[2] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1205791/comments/12

-- 
Best regards,
Michał Kępień

[toc] | [prev] | [next] | [standalone]


#1314290

FromPali Rohár <pali.rohar@gmail.com>
Date2016-01-21 16:50 +0100
Message-ID<qTpEL-7eF-41@gated-at.bofh.it>
In reply to#1314244
On Thursday 21 January 2016 15:56:12 Michał Kępień wrote:
> > > > Michał, can you prepare new (v3) version of this patch? Now required
> > > > acpi video changes are included and so dell-wmi changes should go to...
> > > > To finally fix this keypress bug on Dell Vostro V131 machine.
> > > 
> > > I keep this on my to-do list, but the updated patch will depend on the
> > > final version of the SMBIOS API rework, so I guess there is little point
> > > in posting it now as that API is subject to change.  But rest assured
> > > that as soon as the final version of the API rework series (which I have
> > > yet to prepare, of course) gets applied by Darren, I will post a v3 of
> > > this patch - after all, it is the very reason I am working on the API
> > > rework.
> > 
> > There is still need to patch dell-wmi.c? And this change does not depend
> > on another SMBIOS change (in dell-laptop), right?
> 
> Well, back in December, you wrote [1]:
> 
> > This patch is not enough for enabling 0xe025 key on that Vostro machine.
> > Some extra SMBIOS call is needed, without them ACPI will not send WMI
> > keypress event.
> > 
> > (...)
> > 
> > Maybe now it could make sense to unify Dell SMBIOS API in kernel and
> > move common functions to one place and let drivers to use just common
> > functions. According to older Dell ACPI WMI documentation in DMI is bit
> > which specify if BIOS support SMBIOS via WMI or not.
> > 
> > At least I think this one patch should not be included into kernel until
> > there will be full support for 0xe025 key (adding that SMBIOS call).
> 
> From the above I understood that first you want to unify the Dell SMBIOS
> API used throughout the kernel (that's currently in progress), so that
> it can then be used in dell-wmi as well to perform the SMBIOS call
> needed on the Vostro V131.
> 
> If you want me to just rework the patch so that it doesn't introduce a
> quirk structure, I recalled another reason to use it after all: there
> are other Dell laptops which require the special SMI for enabling WMI
> events, but report the Dell Instant Launch Hotkey using a different WMI
> event code [2].  So I'd say that two separate issues should be fixed
> using DMI matching:
> 
>   * whether the special SMBIOS call is required,
>   * whether the 0x0e25 event should be translated into a keypress.
> 
> A quirk structure looks like an elegant way to deal with this.
> 
> Could you please advise how you would like me to proceed with this?
> 
> [1] http://www.spinics.net/lists/platform-driver-x86/msg07845.html
> [2] https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1205791/comments/12
> 

Hm...

For full support we need:

1) invoke special SMBIOS call for some machine

2) patch dell-wmi.c to do not drop some events for some machines

Part 2) needs to touch only dell-wmi.c code, this is obvious. But I
thought that part 1) will be done in dell-laptop.c code where are all
others SMBIOS calls. Reason is just because dell-wmi.c is doing WMI
stuff. Do you want to include this is part 1) also to dell-wmi.c file?

-- 
Pali Rohár
pali.rohar@gmail.com

[toc] | [prev] | [next] | [standalone]


#1314903

FromMichał Kępień <kernel@kempniu.pl>
Date2016-01-22 12:10 +0100
Message-ID<qTHLl-3dT-53@gated-at.bofh.it>
In reply to#1314290
> For full support we need:
> 
> 1) invoke special SMBIOS call for some machine
> 
> 2) patch dell-wmi.c to do not drop some events for some machines
> 
> Part 2) needs to touch only dell-wmi.c code, this is obvious. But I
> thought that part 1) will be done in dell-laptop.c code where are all
> others SMBIOS calls. Reason is just because dell-wmi.c is doing WMI
> stuff. Do you want to include this is part 1) also to dell-wmi.c file?

Yes, that was my idea.  That SMBIOS call is basically WMI initialization
code.  Consider the case of Vostro V131 - there is no WMI on that
machine without that SMBIOS call and there is no need to issue that
SMBIOS call if you don't want to use WMI.  Given that dell-wmi does not
depend on dell-laptop, consider the extreme case where the user selects
CONFIG_DELL_LAPTOP=n and CONFIG_DELL_WMI=m, expecting WMI to work.  If
we put the WMI-enabling SMBIOS call in dell-laptop, it won't.  Also,
once my Dell SMBIOS API rework series gets applied, adding the relevant
call to dell-wmi will be easy.

-- 
Best regards,
Michał Kępień

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web