RE: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
From: ååæ
Date: Mon Jan 21 2019 - 02:43:25 EST
-----Original Message-----
From: Kai-Heng Feng [mailto:kai.heng.feng@xxxxxxxxxxxxx]
Sent: Monday, January 21, 2019 12:28 PM
To: ååæ
Cc: Benjamin Tissoires; Dmitry Torokhov; open list:HID CORE LAYER; lkml
Subject: Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
> On Jan 18, 2019, at 17:29, ååæ <kt.liao@xxxxxxxxxx> wrote:
>
>
>
> -----Original Message-----
> From: Kai Heng Feng [mailto:kai.heng.feng@xxxxxxxxxxxxx]
> Sent: Friday, January 18, 2019 12:15 AM
> To: Benjamin Tissoires
> Cc: Dmitry Torokhov; ååæ; open list:HID CORE LAYER; lkml
> Subject: Re: [PATCH 1/1] Input: elantech: Use SMBus based on bus info
>
>
>
>> On Jan 17, 2019, at 10:42 PM, Benjamin Tissoires
> <benjamin.tissoires@xxxxxxxxxx> wrote:
>>
>> Hi Kai-Heng,
>>
>> On Thu, Jan 17, 2019 at 10:30 AM Kai-Heng Feng
>> <kai.heng.feng@xxxxxxxxxxxxx> wrote:
>>>
>>> There are some new HP laptops with Elantech touchpad don't support
>>> multitouch.
>>>
>>> Both ETP_BUS_SMB_HST_NTFY_ONLY and ETP_BUS_PS2_SMB_HST_NTFY devices
>>> can use SMBus to support 5 fingers touch, so we need to chech them too.
>>>
>>> So use elantech_use_host_notify() to do a more thouroughly check.
>>>
>>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
>>> ---
>>> drivers/input/mouse/elantech.c | 58
>>> +++++++++++++++++-----------------
>>> 1 file changed, 29 insertions(+), 29 deletions(-)
>>>
>>> diff --git a/drivers/input/mouse/elantech.c
>>> b/drivers/input/mouse/elantech.c index 9fe075c137dc..5bcf1c147eb1
>>> 100644
>>> --- a/drivers/input/mouse/elantech.c
>>> +++ b/drivers/input/mouse/elantech.c
>>> @@ -1799,6 +1799,34 @@ 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_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;
>>> +}
>>> +
>>> /**
>>> * elantech_setup_smbus - called once the PS/2 devices are enumerated
>>> * and decides to instantiate a SMBus InterTouch device.
>>> @@ -1818,7 +1846,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) ||
>>
>> That was my initial approach of the series, but I ended up being more
>> conservative as this would flip all of the existing elantech SMBUS
>> capable touchpads to use elan_i2c.
>> And I didn't want to deal with 4/5 year old laptops that suddenly broke.
>>
>> So I wonder if you can restrict this default change to the recent
>> laptops (let's say 2018+). Maybe by looking at their FW version or
>> something else in the DMI?
>
> It was KT who told me that I should use ETP_BUS_PS2_SMB_HST_NTFY.
>
> As for date, KT still knows better than me.
>
> KT,
> Can you name a year which is safe enough to enable SMBus?
>
> I have discussed it internally.
> The internal rule for FW's SMbus implementation is stable after 2018
> If you meet some special case, please let me know.
Thanks for the info. Iâll use this for the V2 patch.
>
> BTW, The SMbus supporting is requested by HP this time, and there are
> plenty of HP laptop use old IC which doesn't meet "
> ETP_NEW_IC_SMBUS_HOST_NOTIFYâ.
One more question, does ETP_BUS_SMB_HST_NTFY_ONLY means it should stick to SMBus, because it doesnât support PS/2?
Iâd like to merge all checks into elantech_use_host_notify() but ETP_BUS_SMB_HST_NTFY_ONLY caught my attention.
ETP_BUS_SMB_HST_NTFY_ONLY is for our previous planning but it never happen so far, and it won't happen in the future.
There are two cases for our released touchpad and they are " ETP_BUS_PS2_ONLY" and " ETP_BUS_PS2_SMB_HST_NTFY".
Thanks
KT
Kai-Heng
>
> Elan touchpad works well in PS/2 for HP, because it don't support
> TrackPoint.
> You may let old HP platform work as PS/2 for safety.
>
> Thanks
> KT
>
> Kai-Heng
>
>>
>> Cheers,
>> Benjamin
>>
>>> psmouse_matches_pnp_id(psmouse,
> i2c_blacklist_pnp_ids))
>>> return -ENXIO;
>>> }
>>> @@ -1838,34 +1866,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;
>>> --
>>> 2.17.1
>