[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <9AFB67C8-12BE-4794-BA82-BF7D1AED22E8@canonical.com>
Date: Tue, 23 Jul 2019 14:45:51 +0800
From: Kai-Heng Feng <kai.heng.feng@...onical.com>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc: benjamin.tissoires@...hat.com, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org, Your Name <you@...mple.com>
Subject: Re: [PATCH v3] Input: elantech: Enable SMBus on new (2018+) systems
Hi Dmitry,
at 16:17, Dmitry Torokhov <dmitry.torokhov@...il.com> wrote:
> Hi Kai-Heng,
>
> On Mon, Jul 22, 2019 at 03:40:55PM +0800, Kai-Heng Feng wrote:
>> There are some new HP laptops with Elantech touchpad don't support
>> multitouch.
>>
>> Currently we use ETP_NEW_IC_SMBUS_HOST_NOTIFY() to check if SMBus is
>> supported, but in addition to firmware version, the bus type also
>> informs us if the IC can support SMBus, so also check that.
>>
>> In case of breaking old ICs, only enables SMBus on systems manufactured
>> after 2018, alongsides aforementioned checks.
>>
>> Lastly, consolidats all check into elantech_use_host_notify() and use it
>> to determine whether to use PS/2 or SMBus.
>>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@...onical.com>
>> Signed-off-by: Your Name <you@...mple.com>
>
> I do not think "Your Name" should be signing DCO here :)
Yes I don’t know how I messed that up...
>
>> +static bool elantech_use_host_notify(struct psmouse *psmouse,
>> + struct elantech_device_info *info)
>> +{
>> + bool host_notify = false;
>> +
>> + if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
>> + host_notify = true;
>> + else {
>> + switch (info->bus) {
>> + case ETP_BUS_PS2_ONLY:
>> + /* expected case */
>> + break;
>> + case ETP_BUS_SMB_ALERT_ONLY:
>> + /* fall-through */
>> + case ETP_BUS_PS2_SMB_ALERT:
>> + psmouse_dbg(psmouse, "Ignoring SMBus provider
>> through alert protocol.\n");
>> + break;
>> + case ETP_BUS_SMB_HST_NTFY_ONLY:
>> + /* fall-through */
>> + case ETP_BUS_PS2_SMB_HST_NTFY:
>> + /* SMbus implementation is stable since 2018 */
>> + if (dmi_get_bios_year() >= 2018)
>> + host_notify = true;
>> + break;
>> + default:
>> + psmouse_dbg(psmouse,
>> + "Ignoring SMBus bus provider %d.\n",
>> + info->bus);
>> + }
>> + }
>
> I think this is way too verbose. How about a bit more condensed form:
The one revised by you is more succinct. Please use yours instead :)
Thanks!
Kai-Heng
>
>
> Input: elantech - enable SMBus on new (2018+) systems
>
> From: Kai-Heng Feng <kai.heng.feng@...onical.com>
>
> There are some new HP laptops with Elantech touchpad that don't support
> multitouch.
>
> Currently we use ETP_NEW_IC_SMBUS_HOST_NOTIFY() to check if SMBus is
> supported,
> but in addition to firmware version, the bus type also informs us whether
> the IC
> can support SMBus. To avoid breaking old ICs, we will only enable SMbus
> support
> based the bus type on systems manufactured after 2018.
>
> Lastly, let's consolidate all checks into elantech_use_host_notify() and
> use it
> to determine whether to use PS/2 or SMBus.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@...onical.com>
> Acked-by: Benjamin Tissoires <benjamin.tissoires@...hat.com>
> Cc: stable@...r.kernel.org
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@...il.com>
> ---
> drivers/input/mouse/elantech.c | 54 +++++++++++++++++++---------------------
> 1 file changed, 25 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/input/mouse/elantech.c
> b/drivers/input/mouse/elantech.c
> index 2d8434b7b623..73544776a9ed 100644
> --- a/drivers/input/mouse/elantech.c
> +++ b/drivers/input/mouse/elantech.c
> @@ -1827,6 +1827,30 @@ static int elantech_create_smbus(struct psmouse
> *psmouse,
> leave_breadcrumbs);
> }
>
> +static bool elantech_use_host_notify(struct psmouse *psmouse,
> + struct elantech_device_info *info)
> +{
> + if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
> + return true;
> +
> + switch (info->bus) {
> + case ETP_BUS_PS2_ONLY:
> + /* expected case */
> + break;
> + case ETP_BUS_SMB_HST_NTFY_ONLY:
> + case ETP_BUS_PS2_SMB_HST_NTFY:
> + /* SMbus implementation is stable since 2018 */
> + if (dmi_get_bios_year() >= 2018)
> + return true;
> + default:
> + psmouse_dbg(psmouse,
> + "Ignoring SMBus bus provider %d\n", info->bus);
> + break;
> + }
> +
> + return false;
> +}
> +
> /**
> * elantech_setup_smbus - called once the PS/2 devices are enumerated
> * and decides to instantiate a SMBus InterTouch device.
> @@ -1846,7 +1870,7 @@ static int elantech_setup_smbus(struct psmouse
> *psmouse,
> * i2c_blacklist_pnp_ids.
> * Old ICs are up to the user to decide.
> */
> - if (!ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version) ||
> + if (!elantech_use_host_notify(psmouse, info) ||
> psmouse_matches_pnp_id(psmouse, i2c_blacklist_pnp_ids))
> return -ENXIO;
> }
> @@ -1866,34 +1890,6 @@ static int elantech_setup_smbus(struct psmouse
> *psmouse,
> return 0;
> }
>
> -static bool elantech_use_host_notify(struct psmouse *psmouse,
> - struct elantech_device_info *info)
> -{
> - if (ETP_NEW_IC_SMBUS_HOST_NOTIFY(info->fw_version))
> - return true;
> -
> - switch (info->bus) {
> - case ETP_BUS_PS2_ONLY:
> - /* expected case */
> - break;
> - case ETP_BUS_SMB_ALERT_ONLY:
> - /* fall-through */
> - case ETP_BUS_PS2_SMB_ALERT:
> - psmouse_dbg(psmouse, "Ignoring SMBus provider through alert
> protocol.\n");
> - break;
> - case ETP_BUS_SMB_HST_NTFY_ONLY:
> - /* fall-through */
> - case ETP_BUS_PS2_SMB_HST_NTFY:
> - return true;
> - default:
> - psmouse_dbg(psmouse,
> - "Ignoring SMBus bus provider %d.\n",
> - info->bus);
> - }
> -
> - return false;
> -}
> -
> int elantech_init_smbus(struct psmouse *psmouse)
> {
> struct elantech_device_info info;
>
> Thanks.
>
> --
> Dmitry
Powered by blists - more mailing lists