On Mon, Jan 7, 2019 at 11:03 AM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > Hi, > > On 07-01-19 10:58, Benjamin Tissoires wrote: > > On Fri, Jan 4, 2019 at 12:26 AM Hans de Goede <hdegoede@xxxxxxxxxx> 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@xxxxxxxxxx> > >> --- > >> 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@xxxxxxxxxx> > > 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 > >>
