| Message ID | 1464736867-6084-1-git-send-email-jprvita@endlessm.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
On 31 May 2016 at 19:21, João Paulo Rechi Vita <jprvita@gmail.com> wrote: > Check if disabling the i8042 AUX port has any effect on > VPCCMD_R_TOUCHPAD, and use that information to decide whether or not > ideapad_sync_touchpad_state() should do something. > Just put this together quickly, as a RFC for a runtime detection of whether or not the laptop has touchpad hardware control. I didn't think it thoroughly, tho. Does it make any sense? Regards, -- João Paulo Rechi Vita http://about.me/jprvita -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 31 May 2016 at 19:25, João Paulo Rechi Vita <jprvita@gmail.com> wrote: > On 31 May 2016 at 19:21, João Paulo Rechi Vita <jprvita@gmail.com> wrote: >> Check if disabling the i8042 AUX port has any effect on >> VPCCMD_R_TOUCHPAD, and use that information to decide whether or not >> ideapad_sync_touchpad_state() should do something. >> > > Just put this together quickly, as a RFC for a runtime detection of > whether or not the laptop has touchpad hardware control. I didn't > think it thoroughly, tho. Does it make any sense? > And of course, I tested it on the Yoga 900 and it works there. -- João Paulo Rechi Vita http://about.me/jprvita -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2016-06-01 2:21 GMT+03:00 João Paulo Rechi Vita <jprvita@gmail.com>: > Check if disabling the i8042 AUX port has any effect on > VPCCMD_R_TOUCHPAD, and use that information to decide whether or not > ideapad_sync_touchpad_state() should do something. > > Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com> > --- > drivers/platform/x86/ideapad-laptop.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c > index 06a837a..08dd487 100644 > --- a/drivers/platform/x86/ideapad-laptop.c > +++ b/drivers/platform/x86/ideapad-laptop.c > @@ -93,6 +93,7 @@ struct ideapad_private { > struct dentry *debug; > unsigned long cfg; > bool has_hw_rfkill_switch; > + bool has_hw_tp_ctrl; > }; > > static bool no_bt_rfkill; > @@ -773,6 +774,9 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv) > { > unsigned long value; > > + if (!priv->has_hw_tp_ctrl) > + return; > + > /* Without reading from EC touchpad LED doesn't switch state */ > if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value)) { > /* Some IdeaPads don't really turn off touchpad - they only > @@ -930,6 +934,18 @@ static const struct dmi_system_id no_hw_rfkill_list[] = { > {} > }; > > +static bool ideapad_has_touchpad_ctrl(struct ideapad_private *priv) > +{ > + unsigned char param; > + unsigned long value_disabled, value_enabled; > + > + i8042_command(¶m, I8042_CMD_AUX_DISABLE); > + read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value_disabled); > + i8042_command(¶m, I8042_CMD_AUX_ENABLE); > + read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value_enabled); This should be tested on a device that has touchpad control, but personally I don't think it will work in that way. As VPCCMD_R_TOUCHPAD returns different values independently of the actual touchpad state, but synchronously with the LED state changes, I think disabling the touchpad by I8042_CMD_AUX_DISABLE will not affect the value read with VPCCMD_R_TOUCHPAD. So if this code will for some reason work, we should test it in advance, merging it without prior testing has a very big chances to break touchpad switching. > + return (value_disabled != value_enabled); > +} > + > static int ideapad_acpi_add(struct platform_device *pdev) > { > int ret, i; > @@ -978,6 +994,7 @@ static int ideapad_acpi_add(struct platform_device *pdev) > ideapad_register_rfkill(priv, i); > > ideapad_sync_rfk_state(priv); > + priv->has_hw_tp_ctrl = ideapad_has_touchpad_ctrl(priv); > ideapad_sync_touchpad_state(priv); > > if (acpi_video_get_backlight_type() == acpi_backlight_vendor) { > -- > 2.5.0 > -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 1 June 2016 at 00:37, Maxim Mikityanskiy <maxtram95@gmail.com> wrote: > 2016-06-01 2:21 GMT+03:00 João Paulo Rechi Vita <jprvita@gmail.com>: >> Check if disabling the i8042 AUX port has any effect on >> VPCCMD_R_TOUCHPAD, and use that information to decide whether or not >> ideapad_sync_touchpad_state() should do something. >> >> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com> >> --- >> drivers/platform/x86/ideapad-laptop.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c >> index 06a837a..08dd487 100644 >> --- a/drivers/platform/x86/ideapad-laptop.c >> +++ b/drivers/platform/x86/ideapad-laptop.c >> @@ -93,6 +93,7 @@ struct ideapad_private { >> struct dentry *debug; >> unsigned long cfg; >> bool has_hw_rfkill_switch; >> + bool has_hw_tp_ctrl; >> }; >> >> static bool no_bt_rfkill; >> @@ -773,6 +774,9 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv) >> { >> unsigned long value; >> >> + if (!priv->has_hw_tp_ctrl) >> + return; >> + >> /* Without reading from EC touchpad LED doesn't switch state */ >> if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value)) { >> /* Some IdeaPads don't really turn off touchpad - they only >> @@ -930,6 +934,18 @@ static const struct dmi_system_id no_hw_rfkill_list[] = { >> {} >> }; >> >> +static bool ideapad_has_touchpad_ctrl(struct ideapad_private *priv) >> +{ >> + unsigned char param; >> + unsigned long value_disabled, value_enabled; >> + >> + i8042_command(¶m, I8042_CMD_AUX_DISABLE); >> + read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value_disabled); >> + i8042_command(¶m, I8042_CMD_AUX_ENABLE); >> + read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value_enabled); > > This should be tested on a device that has touchpad control, but > personally I don't think it will work in that way. As > VPCCMD_R_TOUCHPAD returns different values independently of the actual > touchpad state, but synchronously with the LED state changes, I think > disabling the touchpad by I8042_CMD_AUX_DISABLE will not affect the > value read with VPCCMD_R_TOUCHPAD. So if this code will for some > reason work, we should test it in advance, merging it without prior > testing has a very big chances to break touchpad switching. > From your previous explanation of ideapad_sync_touchpad_state() I understood that the LED state would reflect the touchpad state when you disabled the AUX port, but maybe I missed something on the way. I tried to reach out to colleagues and friends to see if someone has an Ideapad Z570, but I'm not very confident I'll find one. -- João Paulo Rechi Vita http://about.me/jprvita -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c index 06a837a..08dd487 100644 --- a/drivers/platform/x86/ideapad-laptop.c +++ b/drivers/platform/x86/ideapad-laptop.c @@ -93,6 +93,7 @@ struct ideapad_private { struct dentry *debug; unsigned long cfg; bool has_hw_rfkill_switch; + bool has_hw_tp_ctrl; }; static bool no_bt_rfkill; @@ -773,6 +774,9 @@ static void ideapad_sync_touchpad_state(struct ideapad_private *priv) { unsigned long value; + if (!priv->has_hw_tp_ctrl) + return; + /* Without reading from EC touchpad LED doesn't switch state */ if (!read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value)) { /* Some IdeaPads don't really turn off touchpad - they only @@ -930,6 +934,18 @@ static const struct dmi_system_id no_hw_rfkill_list[] = { {} }; +static bool ideapad_has_touchpad_ctrl(struct ideapad_private *priv) +{ + unsigned char param; + unsigned long value_disabled, value_enabled; + + i8042_command(¶m, I8042_CMD_AUX_DISABLE); + read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value_disabled); + i8042_command(¶m, I8042_CMD_AUX_ENABLE); + read_ec_data(priv->adev->handle, VPCCMD_R_TOUCHPAD, &value_enabled); + return (value_disabled != value_enabled); +} + static int ideapad_acpi_add(struct platform_device *pdev) { int ret, i; @@ -978,6 +994,7 @@ static int ideapad_acpi_add(struct platform_device *pdev) ideapad_register_rfkill(priv, i); ideapad_sync_rfk_state(priv); + priv->has_hw_tp_ctrl = ideapad_has_touchpad_ctrl(priv); ideapad_sync_touchpad_state(priv); if (acpi_video_get_backlight_type() == acpi_backlight_vendor) {
Check if disabling the i8042 AUX port has any effect on VPCCMD_R_TOUCHPAD, and use that information to decide whether or not ideapad_sync_touchpad_state() should do something. Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com> --- drivers/platform/x86/ideapad-laptop.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)