| 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
| ||
|
Date: Mon, 25 Mar 2019 10:23:47 +0100
From: Benjamin Tissoires <benjamin.tissoires@...hat.com>
To: Vladislav Dalechyn <vlad.dalechin@...il.com>
Cc: Jiri Kosina <jikos@...nel.org>,
Kai Heng Feng <kai.heng.feng@...onical.com>,
Stephen Boyd <swboyd@...omium.org>, bigeasy@...utronix.de,
Dmitry Torokhov <dtor@...omium.org>,
"open list:HID CORE LAYER" <linux-input@...r.kernel.org>,
lkml <linux-kernel@...r.kernel.org>,
h0tw4t3r <hotwater438@...anota.com>
Subject: Re: [PATCH] ELAN touchpad i2c_hid bugs fix
Hi Vladislav,
we are almost there.
When submitting/applying a patch, we do run ./script/checkpatch.pl on
the final patch.
This gives us here 2 warnings (inlined below)
Also, can you versionize your submissions (use `-v` in git
format-patch: `git format-patch -v3 HEAD`).
On Sun, Mar 24, 2019 at 8:10 PM Vladislav Dalechyn
<vlad.dalechin@...il.com> wrote:
>
> From: h0tw4t3r <hotwater438@...anota.com>
checkpatch.pl complains here:
WARNING: Missing Signed-off-by: line by nominal patch author 'h0tw4t3r
<hotwater438@...anota.com>'
Either:
- change your git settings and reset the author (git commit --amend
--reset-author)
- just drop this "From:" from the patch from git format-patch
>
> Description: The ELAN1200:04F3:303E touchpad exposes several issues, all
> caused by an error setting the correct IRQ_TRIGGER flag:
> - i2c_hid incoplete error flood in journalctl;
> - Five finger tap kill's module so you have to restart it;
> - Two finger scoll is working incorrect and sometimes even when you
> raised one of two finger still thinks that you are scrolling
>
> Fix all of these with a new quirk that corrects the trigger flag
> announced by the ACPI tables. (edge-falling).
>
> Reason behind moving i2c_hid_init_irq section described below:
> i2c_hid_init_irq now checks for a quirk, so we must setup the quirks
> before we init the irq, and we cannot setup the quirk earlier, so we must
> init the irq later.
>
> Signed-off-by: Vladislav Dalechyn <hotwater438@...anota.com>
IIRC Andy said Co-developped-by: Hans de Goede <...> here. Does that stand Hans?
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/i2c-hid/i2c-hid-core.c | 25 +++++++++++++++----------
> 2 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index b6d93f4ad037..660b4e0e912e 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -389,6 +389,7 @@
> #define USB_DEVICE_ID_TOSHIBA_CLICK_L9W 0x0401
> #define USB_DEVICE_ID_HP_X2 0x074d
> #define USB_DEVICE_ID_HP_X2_10_COVER 0x0755
> +#define I2C_DEVICE_ID_ELAN_TOUCHPAD 0x303e
>
> #define USB_VENDOR_ID_ELECOM 0x056e
> #define USB_DEVICE_ID_ELECOM_BM084 0x0061
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 90164fed08d3..9b417914411f 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -51,6 +51,7 @@
> #define I2C_HID_QUIRK_NO_RUNTIME_PM BIT(2)
> #define I2C_HID_QUIRK_DELAY_AFTER_SLEEP BIT(3)
> #define I2C_HID_QUIRK_BOGUS_IRQ BIT(4)
> +#define I2C_HID_QUIRK_FORCE_TRIGGER_FALLING BIT(5)
>
> /* flags */
> #define I2C_HID_STARTED 0
> @@ -182,8 +183,10 @@ static const struct i2c_hid_quirks {
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> { I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,
> I2C_HID_QUIRK_NO_RUNTIME_PM },
> + { USB_VENDOR_ID_ELAN, I2C_DEVICE_ID_ELAN_TOUCHPAD,
> + I2C_HID_QUIRK_BOGUS_IRQ | I2C_HID_QUIRK_FORCE_TRIGGER_FALLING },
> { USB_VENDOR_ID_ELAN, HID_ANY_ID,
> - I2C_HID_QUIRK_BOGUS_IRQ },
> + I2C_HID_QUIRK_BOGUS_IRQ },
> { 0, 0 }
> };
>
> @@ -854,6 +857,8 @@ static int i2c_hid_init_irq(struct i2c_client *client)
>
> if (!irq_get_trigger_type(client->irq))
> irqflags = IRQF_TRIGGER_LOW;
> + if (ihid->quirks & I2C_HID_QUIRK_FORCE_TRIGGER_FALLING)
> + irqflags = IRQF_TRIGGER_FALLING;
>
> ret = request_threaded_irq(client->irq, NULL, i2c_hid_irq,
> irqflags | IRQF_ONESHOT, client->name, ihid);
> @@ -1123,14 +1128,10 @@ static int i2c_hid_probe(struct i2c_client *client,
> if (ret < 0)
> goto err_pm;
>
> - ret = i2c_hid_init_irq(client);
> - if (ret < 0)
> - goto err_pm;
> -
> hid = hid_allocate_device();
> if (IS_ERR(hid)) {
> ret = PTR_ERR(hid);
> - goto err_irq;
> + goto err_pm;
> }
>
> ihid->hid = hid;
> @@ -1149,11 +1150,15 @@ static int i2c_hid_probe(struct i2c_client *client,
>
> ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
>
> + ret = i2c_hid_init_irq(client);
> + if (ret < 0)
> + goto err_mem_free;
> +
> ret = hid_add_device(hid);
> if (ret) {
> if (ret != -ENODEV)
> hid_err(client, "can't add hid device: %d\n", ret);
> - goto err_mem_free;
> + goto err_irq;
> }
>
> if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
> @@ -1161,12 +1166,12 @@ static int i2c_hid_probe(struct i2c_client *client,
>
> return 0;
>
> +err_irq:
> + free_irq(client->irq, ihid);
> +
WARNING: please, no spaces at the start of a line
#112: FILE: drivers/hid/i2c-hid/i2c-hid-core.c:1170:
+ free_irq(client->irq, ihid);$
Thanks for fixing these 3 minor issues, and we will be able to merge it :)
Cheers,
Benjamin
> err_mem_free:
> hid_destroy_device(hid);
>
> -err_irq:
> - free_irq(client->irq, ihid);
> -
> err_pm:
> pm_runtime_put_noidle(&client->dev);
> pm_runtime_disable(&client->dev);
> --
> 2.20.1
>
Powered by blists - more mailing lists