2017-04-24 21:29:35

by Maarten Maathuis

[permalink] [raw]
Subject: [PATCH] platform/x86/intel-vbtn: add volume up and down

Tested on HP Elite X2 1012 G1.
Matches event report of Lenovo Helix 2
(https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).

V2: Fix indent and add sign-off

Signed-off-by: Maarten Maathuis <[email protected]>
---
drivers/platform/x86/intel-vbtn.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
index 554e82ebe83c..1616cb9c4ae5 100644
--- a/drivers/platform/x86/intel-vbtn.c
+++ b/drivers/platform/x86/intel-vbtn.c
@@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
static const struct key_entry intel_vbtn_keymap[] = {
{ KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
{ KE_KEY, 0xC1, { KEY_POWER } }, /* power key release */
+ { KE_KEY, 0xC4, { KEY_VOLUMEUP} }, /* volume-up key press */
+ { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } }, /* volume-up key release */
+ { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } }, /* volume-down key press */
+ { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } }, /* volume-down key release */
{ KE_END },
};

--
2.12.2


2017-04-24 21:38:01

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis <[email protected]> wrote:
> Tested on HP Elite X2 1012 G1.
> Matches event report of Lenovo Helix 2
> (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).
>

Much better!

> V2: Fix indent and add sign-off

Usually this line goes after --- (body delimiter).
No need to resend this time. I would wait a bit for actual
author/driver maintainer to comment. Otherwise patch looks good enough
to me.

>
> Signed-off-by: Maarten Maathuis <[email protected]>
> ---
> drivers/platform/x86/intel-vbtn.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
> index 554e82ebe83c..1616cb9c4ae5 100644
> --- a/drivers/platform/x86/intel-vbtn.c
> +++ b/drivers/platform/x86/intel-vbtn.c
> @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
> static const struct key_entry intel_vbtn_keymap[] = {
> { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
> { KE_KEY, 0xC1, { KEY_POWER } }, /* power key release */
> + { KE_KEY, 0xC4, { KEY_VOLUMEUP} }, /* volume-up key press */
> + { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } }, /* volume-up key release */
> + { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } }, /* volume-down key press */
> + { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } }, /* volume-down key release */
> { KE_END },
> };
>
> --
> 2.12.2
>



--
With Best Regards,
Andy Shevchenko

2017-04-24 21:42:03

by Maarten Maathuis

[permalink] [raw]
Subject: Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
<[email protected]> wrote:
> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis <[email protected]> wrote:
>> Tested on HP Elite X2 1012 G1.
>> Matches event report of Lenovo Helix 2
>> (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).
>>
>
> Much better!
>
>> V2: Fix indent and add sign-off
>
> Usually this line goes after --- (body delimiter).
> No need to resend this time. I would wait a bit for actual
> author/driver maintainer to comment. Otherwise patch looks good enough
> to me.

The intent is not have this in the commit message?
I'll keep an eye out if i can place it below "---" next time.
Although i suspect the message would end in the actual code diff,
which seems odd.

>
>>
>> Signed-off-by: Maarten Maathuis <[email protected]>
>> ---
>> drivers/platform/x86/intel-vbtn.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
>> index 554e82ebe83c..1616cb9c4ae5 100644
>> --- a/drivers/platform/x86/intel-vbtn.c
>> +++ b/drivers/platform/x86/intel-vbtn.c
>> @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
>> static const struct key_entry intel_vbtn_keymap[] = {
>> { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
>> { KE_KEY, 0xC1, { KEY_POWER } }, /* power key release */
>> + { KE_KEY, 0xC4, { KEY_VOLUMEUP} }, /* volume-up key press */
>> + { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } }, /* volume-up key release */
>> + { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } }, /* volume-down key press */
>> + { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } }, /* volume-down key release */
>> { KE_END },
>> };
>>
>> --
>> 2.12.2
>>
>
>
>
> --
> With Best Regards,
> Andy Shevchenko



--
Far away from the primal instinct, the song seems to fade away, the
river get wider between your thoughts and the things we do and say.

2017-04-25 02:43:43

by AceLan Kao

[permalink] [raw]
Subject: Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

According the spec. I have, the values are correct.
Please merge it, thanks.

2017-04-25 5:41 GMT+08:00 Maarten Maathuis <[email protected]>:
> On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
> <[email protected]> wrote:
>> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis <[email protected]> wrote:
>>> Tested on HP Elite X2 1012 G1.
>>> Matches event report of Lenovo Helix 2
>>> (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).
>>>
>>
>> Much better!
>>
>>> V2: Fix indent and add sign-off
>>
>> Usually this line goes after --- (body delimiter).
>> No need to resend this time. I would wait a bit for actual
>> author/driver maintainer to comment. Otherwise patch looks good enough
>> to me.
>
> The intent is not have this in the commit message?
> I'll keep an eye out if i can place it below "---" next time.
> Although i suspect the message would end in the actual code diff,
> which seems odd.
>
>>
>>>
>>> Signed-off-by: Maarten Maathuis <[email protected]>
>>> ---
>>> drivers/platform/x86/intel-vbtn.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
>>> index 554e82ebe83c..1616cb9c4ae5 100644
>>> --- a/drivers/platform/x86/intel-vbtn.c
>>> +++ b/drivers/platform/x86/intel-vbtn.c
>>> @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
>>> static const struct key_entry intel_vbtn_keymap[] = {
>>> { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
>>> { KE_KEY, 0xC1, { KEY_POWER } }, /* power key release */
>>> + { KE_KEY, 0xC4, { KEY_VOLUMEUP} }, /* volume-up key press */
>>> + { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } }, /* volume-up key release */
>>> + { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } }, /* volume-down key press */
>>> + { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } }, /* volume-down key release */
>>> { KE_END },
>>> };
>>>
>>> --
>>> 2.12.2
>>>
>>
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>
>
>
> --
> Far away from the primal instinct, the song seems to fade away, the
> river get wider between your thoughts and the things we do and say.

2017-04-25 05:01:21

by Maarten Maathuis

[permalink] [raw]
Subject: Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

On Tue, Apr 25, 2017 at 4:43 AM, AceLan Kao <[email protected]> wrote:
> According the spec. I have, the values are correct.
> Please merge it, thanks.
>

Is there a reason the whole spec isn't implemented?
Is it under NDA?

> 2017-04-25 5:41 GMT+08:00 Maarten Maathuis <[email protected]>:
>> On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
>> <[email protected]> wrote:
>>> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis <[email protected]> wrote:
>>>> Tested on HP Elite X2 1012 G1.
>>>> Matches event report of Lenovo Helix 2
>>>> (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).
>>>>
>>>
>>> Much better!
>>>
>>>> V2: Fix indent and add sign-off
>>>
>>> Usually this line goes after --- (body delimiter).
>>> No need to resend this time. I would wait a bit for actual
>>> author/driver maintainer to comment. Otherwise patch looks good enough
>>> to me.
>>
>> The intent is not have this in the commit message?
>> I'll keep an eye out if i can place it below "---" next time.
>> Although i suspect the message would end in the actual code diff,
>> which seems odd.
>>
>>>
>>>>
>>>> Signed-off-by: Maarten Maathuis <[email protected]>
>>>> ---
>>>> drivers/platform/x86/intel-vbtn.c | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
>>>> index 554e82ebe83c..1616cb9c4ae5 100644
>>>> --- a/drivers/platform/x86/intel-vbtn.c
>>>> +++ b/drivers/platform/x86/intel-vbtn.c
>>>> @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
>>>> static const struct key_entry intel_vbtn_keymap[] = {
>>>> { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
>>>> { KE_KEY, 0xC1, { KEY_POWER } }, /* power key release */
>>>> + { KE_KEY, 0xC4, { KEY_VOLUMEUP} }, /* volume-up key press */
>>>> + { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } }, /* volume-up key release */
>>>> + { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } }, /* volume-down key press */
>>>> + { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } }, /* volume-down key release */
>>>> { KE_END },
>>>> };
>>>>
>>>> --
>>>> 2.12.2
>>>>
>>>
>>>
>>>
>>> --
>>> With Best Regards,
>>> Andy Shevchenko
>>
>>
>>
>> --
>> Far away from the primal instinct, the song seems to fade away, the
>> river get wider between your thoughts and the things we do and say.



--
Far away from the primal instinct, the song seems to fade away, the
river get wider between your thoughts and the things we do and say.

2017-04-25 06:15:28

by AceLan Kao

[permalink] [raw]
Subject: Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

In the beginning, we just need the power button function, so I didn't
implement all the keys to the driver.
And we didn't get any further requirement from the following projects
we were working on,
so I'm not aware that there are machines other than Dell using this driver.

I'll try filling up all events on the spec later.

2017-04-25 13:01 GMT+08:00 Maarten Maathuis <[email protected]>:
> On Tue, Apr 25, 2017 at 4:43 AM, AceLan Kao <[email protected]> wrote:
>> According the spec. I have, the values are correct.
>> Please merge it, thanks.
>>
>
> Is there a reason the whole spec isn't implemented?
> Is it under NDA?
>
>> 2017-04-25 5:41 GMT+08:00 Maarten Maathuis <[email protected]>:
>>> On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
>>> <[email protected]> wrote:
>>>> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis <[email protected]> wrote:
>>>>> Tested on HP Elite X2 1012 G1.
>>>>> Matches event report of Lenovo Helix 2
>>>>> (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).
>>>>>
>>>>
>>>> Much better!
>>>>
>>>>> V2: Fix indent and add sign-off
>>>>
>>>> Usually this line goes after --- (body delimiter).
>>>> No need to resend this time. I would wait a bit for actual
>>>> author/driver maintainer to comment. Otherwise patch looks good enough
>>>> to me.
>>>
>>> The intent is not have this in the commit message?
>>> I'll keep an eye out if i can place it below "---" next time.
>>> Although i suspect the message would end in the actual code diff,
>>> which seems odd.
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Maarten Maathuis <[email protected]>
>>>>> ---
>>>>> drivers/platform/x86/intel-vbtn.c | 4 ++++
>>>>> 1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c
>>>>> index 554e82ebe83c..1616cb9c4ae5 100644
>>>>> --- a/drivers/platform/x86/intel-vbtn.c
>>>>> +++ b/drivers/platform/x86/intel-vbtn.c
>>>>> @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
>>>>> static const struct key_entry intel_vbtn_keymap[] = {
>>>>> { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
>>>>> { KE_KEY, 0xC1, { KEY_POWER } }, /* power key release */
>>>>> + { KE_KEY, 0xC4, { KEY_VOLUMEUP} }, /* volume-up key press */
>>>>> + { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } }, /* volume-up key release */
>>>>> + { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } }, /* volume-down key press */
>>>>> + { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } }, /* volume-down key release */
>>>>> { KE_END },
>>>>> };
>>>>>
>>>>> --
>>>>> 2.12.2
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> With Best Regards,
>>>> Andy Shevchenko
>>>
>>>
>>>
>>> --
>>> Far away from the primal instinct, the song seems to fade away, the
>>> river get wider between your thoughts and the things we do and say.
>
>
>
> --
> Far away from the primal instinct, the song seems to fade away, the
> river get wider between your thoughts and the things we do and say.



--
Chia-Lin Kao(AceLan)
http://blog.acelan.idv.tw/
E-Mail: acelan.kaoATcanonical.com (s/AT/@/)