linux-kernel - Re: [PATCH] platform/x86: ideapad-laptop: Add has_touchpad

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <063eb02d-a699-3f6c-fd1b-721e9d195e82@flygoat.com>
Date:   Sat, 2 Jan 2021 10:36:02 +0800
From:   Jiaxun Yang <jiaxun.yang@...goat.com>
To:     Barnabás Pőcze <pobrn@...tonmail.com>
Cc:     "platform-driver-x86@...r.kernel.org" 
        <platform-driver-x86@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        Ike Panhc <ike.pan@...onical.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Mark Gross <mgross@...ux.intel.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] platform/x86: ideapad-laptop: Add has_touchpad_switch

在 2021/1/2 上午1:09, Barnabás Pőcze 写道:
> Hi
>
>
> 2021. január 1., péntek 17:08 keltezéssel, Jiaxun Yang írta:
>
>> [...]
>>>> @@ -1006,6 +1018,10 @@ static int ideapad_acpi_add(struct platform_device *pdev)
>>>>   	if (!priv->has_hw_rfkill_switch)
>>>>   		write_ec_cmd(priv->adev->handle, VPCCMD_W_RF, 1);
>>>>
>>>> +	/* The same for Touchpad */
>>>> +	if (!priv->has_touchpad_switch)
>>>> +		write_ec_cmd(priv->adev->handle, VPCCMD_W_TOUCHPAD, 1);
>>>> +
>>> Shouldn't it be the other way around: `if (priv->has_touchpad_switch)`?
>> It is to prevent accidentally disable touchpad on machines that do have EC switch,
>> so it's intentional.
>> [...]
> Sorry, but the explanation not fully clear to me. The commit message seems to
> indicate that some models "do not use EC to switch touchpad", and I take that
> means that reading from VPCCMD_R_TOUCHPAD will not reflect the actual state of the
> touchpad and writing to VPCCMD_W_TOUCHPAD will not change the state of the touchpad.

I'm just trying to prevent removing functionality on machines that 
touchpad can be controlled
by EC but also equipped I2C HID touchpad. At least users will have a 
functional touchpad
after that.

>
> But then why do you still write to VPCCMD_W_TOUCHPAD on devices where supposedly
> this does not have any effect (at least not the desired one)? And the part of the
> code I made my comment about only runs on machines on which the touchpad supposedly
> cannot be controlled by the EC. What am I missing?
>
> And there is the other problem: on some machines, this patch removes working
> functionality.
Yeah that's a problem. I just don't want to repeat the story of rfkill 
whitelist, it ends up with
countless machine to be added.

Maybe I should specify HID of touchpad as well. Two machines that known 
to be problematic
all have ELAN0634 touchpad.

Thanks.

- Jiaxun

>
>
> Regards,
> Barnabás Pőcze