| Message ID | 20190103232556.17965-2-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | e9eb788f9442d1b5d93efdb30c3be071ce8a22b1 |
| Headers | show |
| Series | [1/2] Input: soc_button_array - Add usage-page 0x01 usage-id 0xca mapping | expand |
On Fri, Jan 4, 2019 at 12:26 AM Hans de Goede <hdegoede@redhat.com> wrote: > > The Microsoft documenation for the PNP0C40 device aka the > "Windows-compatible button array" describes the 5th GpioInt listed in > the resources as: '5. Interrupt corresponding to the "Rotation Lock" > button, if supported'. > > Notice this describes the 5th entry as a button while we sofar have been > mapping it to EV_SW, SW_ROTATE_LOCK. On my Point of View TAB P1006W-232 > which actually comes with a rotation-lock button, the button indeed is a > button and not a slider/switch. An image search for other Windows tablets > has found 2 more models with a rotation-lock button and on both of those > it too is a push-button and not a slider/switch. > > Further evidence can be found in the HUT extension HUTRR52 from Microsoft > which adds rotation lock support to the HUT, which describes 2 different > usages: "0xC9 System Display Rotation Lock Button" and > "0xCA System Display Rotation Lock Slider Switch" note that switch is seen > as a separate thing here and the non switch wording is an exact match for > the "Windows-compatible button array" spec wording. > > TL;DR: our current mapping of the 5th GPIO to SW_ROTATE_LOCK is wrong > because the 5th GPIO is for a push-button not a switch. > > This commit fixes this by maping the 5th GPIO to KEY_ROTATE_LOCK_TOGGLE. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > The referenced documents can currently be found here: > https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/other-acpi-namespace-objects > https://www.usb.org/sites/default/files/hutrr52_system_display_rotation_lock_controls_0.pdf We should probably add those links to the commit description. They sometimes disappear, but that will help for regressions in the short term if there are any. Both patches are: Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Cheers, Benjamin > --- > drivers/input/misc/soc_button_array.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c > index f53923b1593b..bb458beecb43 100644 > --- a/drivers/input/misc/soc_button_array.c > +++ b/drivers/input/misc/soc_button_array.c > @@ -377,7 +377,7 @@ static struct soc_button_info soc_button_PNP0C40[] = { > { "home", 1, EV_KEY, KEY_LEFTMETA, false, true }, > { "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false }, > { "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false }, > - { "rotation_lock", 4, EV_SW, SW_ROTATE_LOCK, false, false }, > + { "rotation_lock", 4, EV_KEY, KEY_ROTATE_LOCK_TOGGLE, false, false }, > { } > }; > > -- > 2.20.1 >
Hi, On 07-01-19 10:58, Benjamin Tissoires wrote: > On Fri, Jan 4, 2019 at 12:26 AM Hans de Goede <hdegoede@redhat.com> wrote: >> >> The Microsoft documenation for the PNP0C40 device aka the >> "Windows-compatible button array" describes the 5th GpioInt listed in >> the resources as: '5. Interrupt corresponding to the "Rotation Lock" >> button, if supported'. >> >> Notice this describes the 5th entry as a button while we sofar have been >> mapping it to EV_SW, SW_ROTATE_LOCK. On my Point of View TAB P1006W-232 >> which actually comes with a rotation-lock button, the button indeed is a >> button and not a slider/switch. An image search for other Windows tablets >> has found 2 more models with a rotation-lock button and on both of those >> it too is a push-button and not a slider/switch. >> >> Further evidence can be found in the HUT extension HUTRR52 from Microsoft >> which adds rotation lock support to the HUT, which describes 2 different >> usages: "0xC9 System Display Rotation Lock Button" and >> "0xCA System Display Rotation Lock Slider Switch" note that switch is seen >> as a separate thing here and the non switch wording is an exact match for >> the "Windows-compatible button array" spec wording. >> >> TL;DR: our current mapping of the 5th GPIO to SW_ROTATE_LOCK is wrong >> because the 5th GPIO is for a push-button not a switch. >> >> This commit fixes this by maping the 5th GPIO to KEY_ROTATE_LOCK_TOGGLE. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> The referenced documents can currently be found here: >> https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/other-acpi-namespace-objects >> https://www.usb.org/sites/default/files/hutrr52_system_display_rotation_lock_controls_0.pdf > > We should probably add those links to the commit description. > They sometimes disappear, but that will help for regressions in the > short term if there are any. That is fine with me, I put them here because some subsystems don't want URLs in the commit message since they go stale over time. Do you want me to do a resend with these added to the commit message? > Both patches are: > Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Thanks & Regards, Hans >> --- >> drivers/input/misc/soc_button_array.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c >> index f53923b1593b..bb458beecb43 100644 >> --- a/drivers/input/misc/soc_button_array.c >> +++ b/drivers/input/misc/soc_button_array.c >> @@ -377,7 +377,7 @@ static struct soc_button_info soc_button_PNP0C40[] = { >> { "home", 1, EV_KEY, KEY_LEFTMETA, false, true }, >> { "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false }, >> { "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false }, >> - { "rotation_lock", 4, EV_SW, SW_ROTATE_LOCK, false, false }, >> + { "rotation_lock", 4, EV_KEY, KEY_ROTATE_LOCK_TOGGLE, false, false }, >> { } >> }; >> >> -- >> 2.20.1 >>
On Mon, Jan 7, 2019 at 11:03 AM Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 07-01-19 10:58, Benjamin Tissoires wrote: > > On Fri, Jan 4, 2019 at 12:26 AM Hans de Goede <hdegoede@redhat.com> wrote: > >> > >> The Microsoft documenation for the PNP0C40 device aka the > >> "Windows-compatible button array" describes the 5th GpioInt listed in > >> the resources as: '5. Interrupt corresponding to the "Rotation Lock" > >> button, if supported'. > >> > >> Notice this describes the 5th entry as a button while we sofar have been > >> mapping it to EV_SW, SW_ROTATE_LOCK. On my Point of View TAB P1006W-232 > >> which actually comes with a rotation-lock button, the button indeed is a > >> button and not a slider/switch. An image search for other Windows tablets > >> has found 2 more models with a rotation-lock button and on both of those > >> it too is a push-button and not a slider/switch. > >> > >> Further evidence can be found in the HUT extension HUTRR52 from Microsoft > >> which adds rotation lock support to the HUT, which describes 2 different > >> usages: "0xC9 System Display Rotation Lock Button" and > >> "0xCA System Display Rotation Lock Slider Switch" note that switch is seen > >> as a separate thing here and the non switch wording is an exact match for > >> the "Windows-compatible button array" spec wording. > >> > >> TL;DR: our current mapping of the 5th GPIO to SW_ROTATE_LOCK is wrong > >> because the 5th GPIO is for a push-button not a switch. > >> > >> This commit fixes this by maping the 5th GPIO to KEY_ROTATE_LOCK_TOGGLE. > >> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> --- > >> The referenced documents can currently be found here: > >> https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/other-acpi-namespace-objects > >> https://www.usb.org/sites/default/files/hutrr52_system_display_rotation_lock_controls_0.pdf > > > > We should probably add those links to the commit description. > > They sometimes disappear, but that will help for regressions in the > > short term if there are any. > > That is fine with me, I put them here because some subsystems don't > want URLs in the commit message since they go stale over time. That is a policy Dmitry needs to make then :) But I often prefer having those for the reasons above. At least we know the docs were available even if the link is broken :) > > Do you want me to do a resend with these added to the commit message? Up to Dmitry again. But if it doesn't bother you, maybe a quick v2 would be fine. Cheers, Benjamin > > > Both patches are: > > Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > Thanks & Regards, > > Hans > > > >> --- > >> drivers/input/misc/soc_button_array.c | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c > >> index f53923b1593b..bb458beecb43 100644 > >> --- a/drivers/input/misc/soc_button_array.c > >> +++ b/drivers/input/misc/soc_button_array.c > >> @@ -377,7 +377,7 @@ static struct soc_button_info soc_button_PNP0C40[] = { > >> { "home", 1, EV_KEY, KEY_LEFTMETA, false, true }, > >> { "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false }, > >> { "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false }, > >> - { "rotation_lock", 4, EV_SW, SW_ROTATE_LOCK, false, false }, > >> + { "rotation_lock", 4, EV_KEY, KEY_ROTATE_LOCK_TOGGLE, false, false }, > >> { } > >> }; > >> > >> -- > >> 2.20.1 > >>
On Mon, Jan 07, 2019 at 11:05:23AM +0100, Benjamin Tissoires wrote: > On Mon, Jan 7, 2019 at 11:03 AM Hans de Goede <hdegoede@redhat.com> wrote: > > > > Hi, > > > > On 07-01-19 10:58, Benjamin Tissoires wrote: > > > On Fri, Jan 4, 2019 at 12:26 AM Hans de Goede <hdegoede@redhat.com> wrote: > > >> > > >> The Microsoft documenation for the PNP0C40 device aka the > > >> "Windows-compatible button array" describes the 5th GpioInt listed in > > >> the resources as: '5. Interrupt corresponding to the "Rotation Lock" > > >> button, if supported'. > > >> > > >> Notice this describes the 5th entry as a button while we sofar have been > > >> mapping it to EV_SW, SW_ROTATE_LOCK. On my Point of View TAB P1006W-232 > > >> which actually comes with a rotation-lock button, the button indeed is a > > >> button and not a slider/switch. An image search for other Windows tablets > > >> has found 2 more models with a rotation-lock button and on both of those > > >> it too is a push-button and not a slider/switch. > > >> > > >> Further evidence can be found in the HUT extension HUTRR52 from Microsoft > > >> which adds rotation lock support to the HUT, which describes 2 different > > >> usages: "0xC9 System Display Rotation Lock Button" and > > >> "0xCA System Display Rotation Lock Slider Switch" note that switch is seen > > >> as a separate thing here and the non switch wording is an exact match for > > >> the "Windows-compatible button array" spec wording. > > >> > > >> TL;DR: our current mapping of the 5th GPIO to SW_ROTATE_LOCK is wrong > > >> because the 5th GPIO is for a push-button not a switch. > > >> > > >> This commit fixes this by maping the 5th GPIO to KEY_ROTATE_LOCK_TOGGLE. > > >> > > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > >> --- > > >> The referenced documents can currently be found here: > > >> https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/other-acpi-namespace-objects > > >> https://www.usb.org/sites/default/files/hutrr52_system_display_rotation_lock_controls_0.pdf > > > > > > We should probably add those links to the commit description. > > > They sometimes disappear, but that will help for regressions in the > > > short term if there are any. > > > > That is fine with me, I put them here because some subsystems don't > > want URLs in the commit message since they go stale over time. > > That is a policy Dmitry needs to make then :) > But I often prefer having those for the reasons above. At least we > know the docs were available even if the link is broken :) "'Tis better to have loved and lost, / Than never to have loved at all". Even if links go stale there sometimes enough breadcrumbs to locate the document somewhere on a mirror. > > > > > Do you want me to do a resend with these added to the commit message? > > Up to Dmitry again. But if it doesn't bother you, maybe a quick v2 > would be fine. Sorry, I think I already pushed the patches out to my next branch. Thanks.
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c index f53923b1593b..bb458beecb43 100644 --- a/drivers/input/misc/soc_button_array.c +++ b/drivers/input/misc/soc_button_array.c @@ -377,7 +377,7 @@ static struct soc_button_info soc_button_PNP0C40[] = { { "home", 1, EV_KEY, KEY_LEFTMETA, false, true }, { "volume_up", 2, EV_KEY, KEY_VOLUMEUP, true, false }, { "volume_down", 3, EV_KEY, KEY_VOLUMEDOWN, true, false }, - { "rotation_lock", 4, EV_SW, SW_ROTATE_LOCK, false, false }, + { "rotation_lock", 4, EV_KEY, KEY_ROTATE_LOCK_TOGGLE, false, false }, { } };
The Microsoft documenation for the PNP0C40 device aka the "Windows-compatible button array" describes the 5th GpioInt listed in the resources as: '5. Interrupt corresponding to the "Rotation Lock" button, if supported'. Notice this describes the 5th entry as a button while we sofar have been mapping it to EV_SW, SW_ROTATE_LOCK. On my Point of View TAB P1006W-232 which actually comes with a rotation-lock button, the button indeed is a button and not a slider/switch. An image search for other Windows tablets has found 2 more models with a rotation-lock button and on both of those it too is a push-button and not a slider/switch. Further evidence can be found in the HUT extension HUTRR52 from Microsoft which adds rotation lock support to the HUT, which describes 2 different usages: "0xC9 System Display Rotation Lock Button" and "0xCA System Display Rotation Lock Slider Switch" note that switch is seen as a separate thing here and the non switch wording is an exact match for the "Windows-compatible button array" spec wording. TL;DR: our current mapping of the 5th GPIO to SW_ROTATE_LOCK is wrong because the 5th GPIO is for a push-button not a switch. This commit fixes this by maping the 5th GPIO to KEY_ROTATE_LOCK_TOGGLE. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- The referenced documents can currently be found here: https://docs.microsoft.com/en-us/windows-hardware/drivers/bringup/other-acpi-namespace-objects https://www.usb.org/sites/default/files/hutrr52_system_display_rotation_lock_controls_0.pdf --- drivers/input/misc/soc_button_array.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)