* [PATCH 2/6] hid-prodikeys: Delete an unnecessary variable initialisation in pk_probe()
From: SF Markus Elfring @ 2018-02-03 17:38 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina; +Cc: LKML, kernel-janitors
In-Reply-To: <c388b4d8-fa43-1bf2-e27d-1a3a8d636118@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 3 Feb 2018 14:45:24 +0100
The local variable "pm" will eventually be set to an appropriate pointer
a bit later. Thus omit the explicit initialisation at the beginning.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/hid-prodikeys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-prodikeys.c b/drivers/hid/hid-prodikeys.c
index 9cad4973a67b..2e79dfc92162 100644
--- a/drivers/hid/hid-prodikeys.c
+++ b/drivers/hid/hid-prodikeys.c
@@ -798,6 +798,6 @@ static int pk_probe(struct hid_device *hdev, const struct hid_device_id *id)
unsigned short ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
unsigned long quirks = id->driver_data;
struct pk_device *pk;
- struct pcmidi_snd *pm = NULL;
+ struct pcmidi_snd *pm;
pk = kzalloc(sizeof(*pk), GFP_KERNEL);
--
2.16.1
^ permalink raw reply related
* [PATCH 1/6] hid-prodikeys: Delete two error messages for a failed memory allocation in pk_probe()
From: SF Markus Elfring @ 2018-02-03 17:37 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina; +Cc: LKML, kernel-janitors
In-Reply-To: <c388b4d8-fa43-1bf2-e27d-1a3a8d636118@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 3 Feb 2018 14:36:20 +0100
Omit extra messages for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/hid-prodikeys.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/hid/hid-prodikeys.c b/drivers/hid/hid-prodikeys.c
index 87eda34ea2f8..9cad4973a67b 100644
--- a/drivers/hid/hid-prodikeys.c
+++ b/drivers/hid/hid-prodikeys.c
@@ -802,15 +802,12 @@ static int pk_probe(struct hid_device *hdev, const struct hid_device_id *id)
pk = kzalloc(sizeof(*pk), GFP_KERNEL);
- if (pk == NULL) {
- hid_err(hdev, "can't alloc descriptor\n");
+ if (!pk)
return -ENOMEM;
- }
pk->hdev = hdev;
pm = kzalloc(sizeof(*pm), GFP_KERNEL);
if (pm == NULL) {
- hid_err(hdev, "can't alloc descriptor\n");
ret = -ENOMEM;
goto err_free_pk;
}
--
2.16.1
^ permalink raw reply related
* [PATCH 0/6] HID-Prodikeys: Adjustments for two function implementations
From: SF Markus Elfring @ 2018-02-03 17:36 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 3 Feb 2018 18:21:23 +0100
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (6):
Delete two error messages for a failed memory allocation in pk_probe()
Delete an unnecessary variable initialisation in pk_probe()
Move an assignment in pk_probe()
Delete an unnecessary return statement in pcmidi_send_note()
Combine two condition checks into one statement in pcmidi_send_note()
Rename a jump label in pcmidi_send_note()
drivers/hid/hid-prodikeys.c | 21 +++++++--------------
1 file changed, 7 insertions(+), 14 deletions(-)
--
2.16.1
^ permalink raw reply
* Re: [PATCH] Input: matrix_keypad - fix keypad does not response
From: Zhang Bo @ 2018-02-03 16:31 UTC (permalink / raw)
To: Marcus Folkesson
Cc: dmitry.torokhov, DRivshin, robh, linux-input, linux-kernel,
zhang.bo19
In-Reply-To: <20180203134450.GC707@gmail.com>
At 2018-02-03 21:44:50, "Marcus Folkesson" <marcus.folkesson@gmail.com> wrote:
>> diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
>> index 1f316d66e6f7..13fe51824637 100644
>> --- a/drivers/input/keyboard/matrix_keypad.c
>> +++ b/drivers/input/keyboard/matrix_keypad.c
>> @@ -169,7 +169,8 @@ static void matrix_keypad_scan(struct work_struct *work)
>> /* Enable IRQs again */
>> spin_lock_irq(&keypad->lock);
>> keypad->scan_pending = false;
>> - enable_row_irqs(keypad);
>> + if (keypad->stopped == false)
>> + enable_row_irqs(keypad);
>> spin_unlock_irq(&keypad->lock);
>> }
>>
>> @@ -202,14 +203,16 @@ static int matrix_keypad_start(struct input_dev *dev)
>> {
>> struct matrix_keypad *keypad = input_get_drvdata(dev);
>>
>> + spin_lock_irq(&keypad->lock);
>> keypad->stopped = false;
>> - mb();
>>
>> /*
>> * Schedule an immediate key scan to capture current key state;
>> * columns will be activated and IRQs be enabled after the scan.
>> */
>> - schedule_delayed_work(&keypad->work, 0);
>> + if (keypad->scan_pending == false)
>> + schedule_delayed_work(&keypad->work, 0);
>> + spin_unlock_irq(&keypad->lock);
>>
>> return 0;
>> }
>> @@ -218,14 +221,17 @@ static void matrix_keypad_stop(struct input_dev *dev)
>> {
>> struct matrix_keypad *keypad = input_get_drvdata(dev);
>>
>> + spin_lock_irq(&keypad->lock);
>> keypad->stopped = true;
>> - mb();
>> - flush_work(&keypad->work.work);
>> /*
>> * matrix_keypad_scan() will leave IRQs enabled;
>> * we should disable them now.
>> */
>> - disable_row_irqs(keypad);
>> + if (keypad->scan_pending == false)
>> + disable_row_irqs(keypad);
>> + spin_unlock_irq(&keypad->lock);
>> +
>> + flush_work(&keypad->work.work);
>> }
>
>
>Hum, I think we should use spin_lock_irqsave/spin_lock_irqrestore
>instead to be on the safe side.
>I don't see how we could guarantee that irqs is allways enabled when
>calling spin_lock_irq().
spin_lock_irq and spin_unlock_irq are called in matrix_keypad_stop and matrix_keypad_start
which are in suspend and resume, they run in process context, matrix_keypad_scan runs in
worker which is in process context also.
^ permalink raw reply
* Re:Re: [PATCH] Input: matrix_keypad - fix keypad does not response
From: Zhang Bo @ 2018-02-03 16:24 UTC (permalink / raw)
To: Marcus Folkesson
Cc: dmitry.torokhov, DRivshin, robh, linux-input, linux-kernel,
zhang.bo19
In-Reply-To: <20180203134450.GC707@gmail.com>
At 2018-02-03 21:44:50, "Marcus Folkesson" <marcus.folkesson@gmail.com> wrote:
>> diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
>> index 1f316d66e6f7..13fe51824637 100644
>> --- a/drivers/input/keyboard/matrix_keypad.c
>> +++ b/drivers/input/keyboard/matrix_keypad.c
>> @@ -169,7 +169,8 @@ static void matrix_keypad_scan(struct work_struct *work)
>> /* Enable IRQs again */
>> spin_lock_irq(&keypad->lock);
>> keypad->scan_pending = false;
>> - enable_row_irqs(keypad);
>> + if (keypad->stopped == false)
>> + enable_row_irqs(keypad);
>> spin_unlock_irq(&keypad->lock);
>> }
>>
>> @@ -202,14 +203,16 @@ static int matrix_keypad_start(struct input_dev *dev)
>> {
>> struct matrix_keypad *keypad = input_get_drvdata(dev);
>>
>> + spin_lock_irq(&keypad->lock);
>> keypad->stopped = false;
>> - mb();
>>
>> /*
>> * Schedule an immediate key scan to capture current key state;
>> * columns will be activated and IRQs be enabled after the scan.
>> */
>> - schedule_delayed_work(&keypad->work, 0);
>> + if (keypad->scan_pending == false)
>> + schedule_delayed_work(&keypad->work, 0);
>> + spin_unlock_irq(&keypad->lock);
>>
>> return 0;
>> }
>> @@ -218,14 +221,17 @@ static void matrix_keypad_stop(struct input_dev *dev)
>> {
>> struct matrix_keypad *keypad = input_get_drvdata(dev);
>>
>> + spin_lock_irq(&keypad->lock);
>> keypad->stopped = true;
>> - mb();
>> - flush_work(&keypad->work.work);
>> /*
>> * matrix_keypad_scan() will leave IRQs enabled;
>> * we should disable them now.
>> */
>> - disable_row_irqs(keypad);
>> + if (keypad->scan_pending == false)
>> + disable_row_irqs(keypad);
>> + spin_unlock_irq(&keypad->lock);
>> +
>> + flush_work(&keypad->work.work);
>> }
>
>
>Hum, I think we should use spin_lock_irqsave/spin_lock_irqrestore
>instead to be on the safe side.
>I don't see how we could guarantee that irqs is allways enabled when
>calling spin_lock_irq().
spin_lock_irq and spin_unlock_irq are called in matrix_keypad_stop and matrix_keypad_start
which are in suspend and resume, they run in process context, matrix_keypad_scan runs in
worker which is in process context also.
^ permalink raw reply
* [PATCH v2] Input: matrix_keypad - fix keypad does not response
From: Zhang Bo @ 2018-02-03 15:58 UTC (permalink / raw)
To: dmitry.torokhov
Cc: robh, DRivshin, linux-input, linux-kernel, zhang.bo19,
andy.shevchenko, zbsdta
If matrix_keypad_stop() is calling and the keypad interrupt is triggered,
disable_row_irqs() is called by both matrix_keypad_interrupt() and
matrix_keypad_stop() at the same time. then disable_row_irqs() is called
twice, and the device enter suspend state before keypad->work is executed.
At this condition the device will start keypad and enable irq once after
resume. and then irqs are disabled yet because irqs are disabled twice and
only enable once.
Take lock around keypad->stopped and queue delayed work in
matrix_keypad_start() and matrix_keypad_stop() to ensure irqs operation and
scheduling scan work are in atomic operation.
Signed-off-by: Zhang Bo <zbsdta@126.com>
---
Changes in v2:
- Change commit message and full name in the signed-off-by tag.
drivers/input/keyboard/matrix_keypad.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 1f316d66e6f7..13fe51824637 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -169,7 +169,8 @@ static void matrix_keypad_scan(struct work_struct *work)
/* Enable IRQs again */
spin_lock_irq(&keypad->lock);
keypad->scan_pending = false;
- enable_row_irqs(keypad);
+ if (keypad->stopped == false)
+ enable_row_irqs(keypad);
spin_unlock_irq(&keypad->lock);
}
@@ -202,14 +203,16 @@ static int matrix_keypad_start(struct input_dev *dev)
{
struct matrix_keypad *keypad = input_get_drvdata(dev);
+ spin_lock_irq(&keypad->lock);
keypad->stopped = false;
- mb();
/*
* Schedule an immediate key scan to capture current key state;
* columns will be activated and IRQs be enabled after the scan.
*/
- schedule_delayed_work(&keypad->work, 0);
+ if (keypad->scan_pending == false)
+ schedule_delayed_work(&keypad->work, 0);
+ spin_unlock_irq(&keypad->lock);
return 0;
}
@@ -218,14 +221,17 @@ static void matrix_keypad_stop(struct input_dev *dev)
{
struct matrix_keypad *keypad = input_get_drvdata(dev);
+ spin_lock_irq(&keypad->lock);
keypad->stopped = true;
- mb();
- flush_work(&keypad->work.work);
/*
* matrix_keypad_scan() will leave IRQs enabled;
* we should disable them now.
*/
- disable_row_irqs(keypad);
+ if (keypad->scan_pending == false)
+ disable_row_irqs(keypad);
+ spin_unlock_irq(&keypad->lock);
+
+ flush_work(&keypad->work.work);
}
#ifdef CONFIG_PM_SLEEP
--
2.14.3
^ permalink raw reply related
* [PATCH] HID: Fix hid_report_len usage
From: Aaron Ma @ 2018-02-03 15:57 UTC (permalink / raw)
To: aaron.ma, jikos, benjamin.tissoires, linux-input, linux-kernel
In-Reply-To: <20180108024141.10590-2-aaron.ma@canonical.com>
Follow the change of return type u32 of hid_report_len,
fix all the types of variables those get the return value of
hid_report_len to u32, and all other code already uses u32.
Cc: stable@vger.kernel.org
Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
---
drivers/hid/hid-input.c | 3 ++-
drivers/hid/hid-multitouch.c | 5 +++--
drivers/hid/hid-rmi.c | 4 ++--
drivers/hid/wacom_sys.c | 4 ++--
4 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 04d01b57d94c..d86398755b0d 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1368,7 +1368,8 @@ static void hidinput_led_worker(struct work_struct *work)
led_work);
struct hid_field *field;
struct hid_report *report;
- int len, ret;
+ int ret;
+ u32 len;
__u8 *buf;
field = hidinput_get_led_field(hid);
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 3b4739bde05d..2e1736ba2444 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -370,7 +370,8 @@ static const struct attribute_group mt_attribute_group = {
static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
{
struct mt_device *td = hid_get_drvdata(hdev);
- int ret, size = hid_report_len(report);
+ int ret;
+ u32 size = hid_report_len(report);
u8 *buf;
/*
@@ -1183,7 +1184,7 @@ static void mt_set_input_mode(struct hid_device *hdev)
struct hid_report_enum *re;
struct mt_class *cls = &td->mtclass;
char *buf;
- int report_len;
+ u32 report_len;
if (td->inputmode < 0)
return;
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index c6c05df3e8d2..9c9362149641 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -89,8 +89,8 @@ struct rmi_data {
u8 *writeReport;
u8 *readReport;
- int input_report_size;
- int output_report_size;
+ u32 input_report_size;
+ u32 output_report_size;
unsigned long flags;
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 409543160af7..b54ef1ffcbec 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -219,7 +219,7 @@ static void wacom_feature_mapping(struct hid_device *hdev,
unsigned int equivalent_usage = wacom_equivalent_usage(usage->hid);
u8 *data;
int ret;
- int n;
+ u32 n;
switch (equivalent_usage) {
case HID_DG_CONTACTMAX:
@@ -519,7 +519,7 @@ static int wacom_set_device_mode(struct hid_device *hdev,
u8 *rep_data;
struct hid_report *r;
struct hid_report_enum *re;
- int length;
+ u32 length;
int error = -ENOMEM, limit = 0;
if (wacom_wac->mode_report < 0)
--
2.14.3
^ permalink raw reply related
* Re: [PATCH v2 2/2] HID: core: Fix size as type u32
From: Aaron Ma @ 2018-02-03 14:28 UTC (permalink / raw)
To: Marcus Folkesson; +Cc: linux-kernel, linux-input, jikos, benjamin.tissoires
In-Reply-To: <20180203075531.GA707@gmail.com>
On 02/03/2018 03:55 PM, Marcus Folkesson wrote:
> Hi Aaron,
>
> On Mon, Jan 08, 2018 at 10:41:41AM +0800, Aaron Ma wrote:
>> When size is negative, calling memset will make segment fault.
>> Declare the size as type u32 to keep memset safe.
>>
>> size in struct hid_report is unsigned, fix return type of
>> hid_report_len to u32.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
>> ---
>> drivers/hid/hid-core.c | 10 +++++-----
>> include/linux/hid.h | 6 +++---
>> 2 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 0c3f608131cf..cf81c53e3b98 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -1390,7 +1390,7 @@ u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags)
>> * of implement() working on 8 byte chunks
>> */
>>
>> - int len = hid_report_len(report) + 7;
>> + u32 len = hid_report_len(report) + 7;
>>
>> return kmalloc(len, flags);
>> }
>> @@ -1455,7 +1455,7 @@ void __hid_request(struct hid_device *hid, struct hid_report *report,
>> {
>> char *buf;
>> int ret;
>> - int len;
>> + u32 len;
>>
>> buf = hid_alloc_report_buf(report, GFP_KERNEL);
>> if (!buf)
>> @@ -1481,14 +1481,14 @@ void __hid_request(struct hid_device *hid, struct hid_report *report,
>> }
>> EXPORT_SYMBOL_GPL(__hid_request);
>>
>> -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
>> +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
>> int interrupt)
>> {
>> struct hid_report_enum *report_enum = hid->report_enum + type;
>> struct hid_report *report;
>> struct hid_driver *hdrv;
>> unsigned int a;
>> - int rsize, csize = size;
>> + u32 rsize, csize = size;
>> u8 *cdata = data;
>> int ret = 0;
>>
>> @@ -1546,7 +1546,7 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event);
>> *
>> * This is data entry for lower layers.
>> */
>> -int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int interrupt)
>> +int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int interrupt)
>> {
>> struct hid_report_enum *report_enum;
>> struct hid_driver *hdrv;
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index d491027a7c22..9bc296eebc98 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -841,7 +841,7 @@ extern int hidinput_connect(struct hid_device *hid, unsigned int force);
>> extern void hidinput_disconnect(struct hid_device *);
>>
>> int hid_set_field(struct hid_field *, unsigned, __s32);
>> -int hid_input_report(struct hid_device *, int type, u8 *, int, int);
>> +int hid_input_report(struct hid_device *, int type, u8 *, u32, int);
>> int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field);
>> struct hid_field *hidinput_get_led_field(struct hid_device *hid);
>> unsigned int hidinput_count_leds(struct hid_device *hid);
>> @@ -1088,13 +1088,13 @@ static inline void hid_hw_wait(struct hid_device *hdev)
>> *
>> * @report: the report we want to know the length
>> */
>> -static inline int hid_report_len(struct hid_report *report)
>> +static inline u32 hid_report_len(struct hid_report *report)
> hid_report_len() is used in several files.
> If we think it is a good idea to change the return type, we should fix
> these files as well.
>
> [08:47:56]marcus@little:~/git/linux$ git grep -l hid_report_len
> drivers/hid/hid-core.c
> drivers/hid/hid-input.c
> drivers/hid/hid-multitouch.c
> drivers/hid/hid-rmi.c
> drivers/hid/usbhid/hid-core.c
> drivers/hid/wacom_sys.c
> drivers/staging/greybus/hid.c
> include/linux/hid.h
Sure, I will fix these return type in a new patch.
Thanks,
Aaron
>
>> {
>> /* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */
>> return ((report->size - 1) >> 3) + 1 + (report->id > 0);
>> }
>>
>> -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
>> +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
>> int interrupt);
>>
>> /* HID quirks API */
>> --
>> 2.14.3
> Best regards
> Marcus Folkesson
^ permalink raw reply
* Re: [PATCH 1/2] HID: core: i2c-hid: fix size check and type usage
From: Aaron Ma @ 2018-02-03 14:17 UTC (permalink / raw)
To: Marcus Folkesson; +Cc: linux-kernel, linux-input, jikos, benjamin.tissoires
In-Reply-To: <20180203083651.GB707@gmail.com>
Hi Marcus:
This patch is replaced by v2 patches you just reviewed.
Thanks,
Aaron
^ permalink raw reply
* [PATCH] Input: matrix_keypad - fix keypad does not response
From: Zhang Bo @ 2018-02-03 14:03 UTC (permalink / raw)
To: dmitry.torokhov
Cc: robh, DRivshin, linux-input, linux-kernel, zhang.bo19, zbsdta
If matrix_keypad_stop() is calling and the keypad interrupt is triggered,
disable_row_irqs() is called by both matrix_keypad_interrupt() and
matrix_keypad_stop() at the same time. then disable_row_irqs() is called
twice, and the device enter suspend state before keypad->work is executed.
At this condition the device will start keypad and enable irq once after
resume. and then irqs are disabled yet because irqs are disabled twice and
only enable once.
Take lock around keypad->stopped and queue delayed work in
matrix_keypad_start() and matrix_keypad_stop() to ensure irqs operation and
scheduling scan work are in atomic operation.
Signed-off-by: Zhang Bo <zbsdta@126.com>
---
drivers/input/keyboard/matrix_keypad.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 1f316d66e6f7..13fe51824637 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -169,7 +169,8 @@ static void matrix_keypad_scan(struct work_struct *work)
/* Enable IRQs again */
spin_lock_irq(&keypad->lock);
keypad->scan_pending = false;
- enable_row_irqs(keypad);
+ if (keypad->stopped == false)
+ enable_row_irqs(keypad);
spin_unlock_irq(&keypad->lock);
}
@@ -202,14 +203,16 @@ static int matrix_keypad_start(struct input_dev *dev)
{
struct matrix_keypad *keypad = input_get_drvdata(dev);
+ spin_lock_irq(&keypad->lock);
keypad->stopped = false;
- mb();
/*
* Schedule an immediate key scan to capture current key state;
* columns will be activated and IRQs be enabled after the scan.
*/
- schedule_delayed_work(&keypad->work, 0);
+ if (keypad->scan_pending == false)
+ schedule_delayed_work(&keypad->work, 0);
+ spin_unlock_irq(&keypad->lock);
return 0;
}
@@ -218,14 +221,17 @@ static void matrix_keypad_stop(struct input_dev *dev)
{
struct matrix_keypad *keypad = input_get_drvdata(dev);
+ spin_lock_irq(&keypad->lock);
keypad->stopped = true;
- mb();
- flush_work(&keypad->work.work);
/*
* matrix_keypad_scan() will leave IRQs enabled;
* we should disable them now.
*/
- disable_row_irqs(keypad);
+ if (keypad->scan_pending == false)
+ disable_row_irqs(keypad);
+ spin_unlock_irq(&keypad->lock);
+
+ flush_work(&keypad->work.work);
}
#ifdef CONFIG_PM_SLEEP
--
2.14.3
^ permalink raw reply related
* Re: [PATCH] Input: matrix_keypad - fix keypad does not response
From: Marcus Folkesson @ 2018-02-03 13:44 UTC (permalink / raw)
To: Zhang Bo
Cc: dmitry.torokhov, DRivshin, robh, linux-input, linux-kernel,
zhang.bo19
In-Reply-To: <20180203120046.10988-1-zbsdta@126.com>
[-- Attachment #1: Type: text/plain, Size: 2810 bytes --]
Hi!
On Sat, Feb 03, 2018 at 08:00:46PM +0800, Zhang Bo wrote:
> From: zhangbo <zbsdta@126.com>
>
> If matrix_keypad_stop() is calling and the keypad interrupt is triggered,
> disable_row_irqs() is called by both matrix_keypad_interrupt() and
> matrix_keypad_stop() at the same time. then disable_row_irqs() is called
> twice, and the device enter suspend state before keypad->work is executed.
> At this condition the device will start keypad and enable irq once after
> resume. and then irqs are disabled yet because irqs are disabled twice and
> only enable once.
>
> Signed-off-by: zhangbo <zbsdta@126.com>
Please use your full real name in the signed-off-by tag.
> ---
> drivers/input/keyboard/matrix_keypad.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
> index 1f316d66e6f7..13fe51824637 100644
> --- a/drivers/input/keyboard/matrix_keypad.c
> +++ b/drivers/input/keyboard/matrix_keypad.c
> @@ -169,7 +169,8 @@ static void matrix_keypad_scan(struct work_struct *work)
> /* Enable IRQs again */
> spin_lock_irq(&keypad->lock);
> keypad->scan_pending = false;
> - enable_row_irqs(keypad);
> + if (keypad->stopped == false)
> + enable_row_irqs(keypad);
> spin_unlock_irq(&keypad->lock);
> }
>
> @@ -202,14 +203,16 @@ static int matrix_keypad_start(struct input_dev *dev)
> {
> struct matrix_keypad *keypad = input_get_drvdata(dev);
>
> + spin_lock_irq(&keypad->lock);
> keypad->stopped = false;
> - mb();
>
> /*
> * Schedule an immediate key scan to capture current key state;
> * columns will be activated and IRQs be enabled after the scan.
> */
> - schedule_delayed_work(&keypad->work, 0);
> + if (keypad->scan_pending == false)
> + schedule_delayed_work(&keypad->work, 0);
> + spin_unlock_irq(&keypad->lock);
>
> return 0;
> }
> @@ -218,14 +221,17 @@ static void matrix_keypad_stop(struct input_dev *dev)
> {
> struct matrix_keypad *keypad = input_get_drvdata(dev);
>
> + spin_lock_irq(&keypad->lock);
> keypad->stopped = true;
> - mb();
> - flush_work(&keypad->work.work);
> /*
> * matrix_keypad_scan() will leave IRQs enabled;
> * we should disable them now.
> */
> - disable_row_irqs(keypad);
> + if (keypad->scan_pending == false)
> + disable_row_irqs(keypad);
> + spin_unlock_irq(&keypad->lock);
> +
> + flush_work(&keypad->work.work);
> }
Hum, I think we should use spin_lock_irqsave/spin_lock_irqrestore
instead to be on the safe side.
I don't see how we could guarantee that irqs is allways enabled when
calling spin_lock_irq().
>
> #ifdef CONFIG_PM_SLEEP
> --
> 2.14.3
Best regards
Marcus Folkesson
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] hid-rmi: Delete an error message for a failed memory allocation in rmi_probe()
From: SF Markus Elfring @ 2018-02-03 12:48 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina
Cc: LKML, kernel-janitors, Andrew Duggan
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 3 Feb 2018 13:42:08 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/hid-rmi.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 0f43c4292685..d92a17954b67 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -678,10 +678,8 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
alloc_size = data->output_report_size + data->input_report_size;
data->writeReport = devm_kzalloc(&hdev->dev, alloc_size, GFP_KERNEL);
- if (!data->writeReport) {
- hid_err(hdev, "failed to allocate buffer for HID reports\n");
+ if (!data->writeReport)
return -ENOMEM;
- }
data->readReport = data->writeReport + data->output_report_size;
--
2.16.1
^ permalink raw reply related
* [PATCH] Input: matrix_keypad - fix keypad does not response
From: Zhang Bo @ 2018-02-03 12:00 UTC (permalink / raw)
To: dmitry.torokhov
Cc: DRivshin, robh, linux-input, linux-kernel, zbsdta, zhang.bo19
From: zhangbo <zbsdta@126.com>
If matrix_keypad_stop() is calling and the keypad interrupt is triggered,
disable_row_irqs() is called by both matrix_keypad_interrupt() and
matrix_keypad_stop() at the same time. then disable_row_irqs() is called
twice, and the device enter suspend state before keypad->work is executed.
At this condition the device will start keypad and enable irq once after
resume. and then irqs are disabled yet because irqs are disabled twice and
only enable once.
Signed-off-by: zhangbo <zbsdta@126.com>
---
drivers/input/keyboard/matrix_keypad.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 1f316d66e6f7..13fe51824637 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -169,7 +169,8 @@ static void matrix_keypad_scan(struct work_struct *work)
/* Enable IRQs again */
spin_lock_irq(&keypad->lock);
keypad->scan_pending = false;
- enable_row_irqs(keypad);
+ if (keypad->stopped == false)
+ enable_row_irqs(keypad);
spin_unlock_irq(&keypad->lock);
}
@@ -202,14 +203,16 @@ static int matrix_keypad_start(struct input_dev *dev)
{
struct matrix_keypad *keypad = input_get_drvdata(dev);
+ spin_lock_irq(&keypad->lock);
keypad->stopped = false;
- mb();
/*
* Schedule an immediate key scan to capture current key state;
* columns will be activated and IRQs be enabled after the scan.
*/
- schedule_delayed_work(&keypad->work, 0);
+ if (keypad->scan_pending == false)
+ schedule_delayed_work(&keypad->work, 0);
+ spin_unlock_irq(&keypad->lock);
return 0;
}
@@ -218,14 +221,17 @@ static void matrix_keypad_stop(struct input_dev *dev)
{
struct matrix_keypad *keypad = input_get_drvdata(dev);
+ spin_lock_irq(&keypad->lock);
keypad->stopped = true;
- mb();
- flush_work(&keypad->work.work);
/*
* matrix_keypad_scan() will leave IRQs enabled;
* we should disable them now.
*/
- disable_row_irqs(keypad);
+ if (keypad->scan_pending == false)
+ disable_row_irqs(keypad);
+ spin_unlock_irq(&keypad->lock);
+
+ flush_work(&keypad->work.work);
}
#ifdef CONFIG_PM_SLEEP
--
2.14.3
^ permalink raw reply related
* Re: i8042 AUX port [serio1] suspend takes a second on Dell XPS 13 9360 and TUXEDO Book 1406
From: Paul Menzel @ 2018-02-03 11:09 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, it+linux-input
In-Reply-To: <a7c19320-4942-e41d-5323-0ac3693e7012@molgen.mpg.de>
Dear Dmitry,
Am 30.01.2018 um 19:39 schrieb Paul Menzel:
> On 01/30/18 19:07, Dmitry Torokhov wrote:
>> On Tue, Jan 30, 2018 at 09:52:45AM -0800, Dmitry Torokhov wrote:
>>> On Tue, Jan 30, 2018 at 06:36:34PM +0100, Paul Menzel wrote:
>>>> I do not know, when it started, but with Linux 4.14-rc8 and 4.15,
>>>> benchmarking suspend and resume time with `sleepgraph.py` [1][2],
>>>> there is a
>>>> regression, that i8042 AUX port [serio1] suspend takes a second on
>>>> Dell XPS
>>>> 13 9360 and TUXEDO Book 1406.
>>>
>>> It would be really helpful to know when the regression started.
>>
>> So the reason it takes longer is because the touchpad does not want to
>> talk to us for some reason and we wait until commands time out:
>>
>> [ 94.591636] calling serio1+ @ 2299, parent: i8042
>> [ 94.794292] psmouse serio1: Failed to disable mouse on isa0060/serio1
>> [ 95.593303] call serio1+ returned 0 after 974280 usecs
>>
>> but it is not clear why it happens, I do not think we changed anything
>> in that path for a while, so it might be some other change affecting
>> things indirectly. I'm afraid you'll have to narrow the scope, and
>> ideally bisect.
>
> Thank you for looking into this. Just to note, that it worked with Linux
> 4.14.1 from Ubuntu 17.10, so it’s probably something in 4.15. Bisecting
> will take a bit, as the FTRACE stuff broke in Linux 4.15-rc1 and was
> only fixed in 4.15-rc9. Hopefully, it’s just one commit I need to
> backport. I’ll get back to you hopefully at the end of the week.
Unfortunately, I am unable to reproduce it currently. Also not with the
original Linux kernel versions I tried. I’ll report back, when I see it
the next time.
Kind regards,
Paul
^ permalink raw reply
* [PATCH] HID: Roccat: Delete an error message for a failed memory allocation in nine functions
From: SF Markus Elfring @ 2018-02-03 10:50 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina, Stefan Achatz
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 3 Feb 2018 11:45:15 +0100
Omit an extra message for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/hid-roccat-arvo.c | 5 ++---
drivers/hid/hid-roccat-isku.c | 5 ++---
drivers/hid/hid-roccat-koneplus.c | 5 ++---
drivers/hid/hid-roccat-konepure.c | 5 ++---
drivers/hid/hid-roccat-kovaplus.c | 5 ++---
drivers/hid/hid-roccat-lua.c | 5 ++---
drivers/hid/hid-roccat-pyra.c | 5 ++---
drivers/hid/hid-roccat-ryos.c | 5 ++---
drivers/hid/hid-roccat-savu.c | 5 ++---
9 files changed, 18 insertions(+), 27 deletions(-)
diff --git a/drivers/hid/hid-roccat-arvo.c b/drivers/hid/hid-roccat-arvo.c
index 329c5d1270f9..8d1eccd4fffd 100644
--- a/drivers/hid/hid-roccat-arvo.c
+++ b/drivers/hid/hid-roccat-arvo.c
@@ -300,10 +300,9 @@ static int arvo_init_specials(struct hid_device *hdev)
}
arvo = kzalloc(sizeof(*arvo), GFP_KERNEL);
- if (!arvo) {
- hid_err(hdev, "can't alloc device descriptor\n");
+ if (!arvo)
return -ENOMEM;
- }
+
hid_set_drvdata(hdev, arvo);
retval = arvo_init_arvo_device_struct(usb_dev, arvo);
diff --git a/drivers/hid/hid-roccat-isku.c b/drivers/hid/hid-roccat-isku.c
index 02db537f8f3e..6f18925c72db 100644
--- a/drivers/hid/hid-roccat-isku.c
+++ b/drivers/hid/hid-roccat-isku.c
@@ -280,10 +280,9 @@ static int isku_init_specials(struct hid_device *hdev)
}
isku = kzalloc(sizeof(*isku), GFP_KERNEL);
- if (!isku) {
- hid_err(hdev, "can't alloc device descriptor\n");
+ if (!isku)
return -ENOMEM;
- }
+
hid_set_drvdata(hdev, isku);
retval = isku_init_isku_device_struct(usb_dev, isku);
diff --git a/drivers/hid/hid-roccat-koneplus.c b/drivers/hid/hid-roccat-koneplus.c
index 09e8fc72aa1d..3981553d02a6 100644
--- a/drivers/hid/hid-roccat-koneplus.c
+++ b/drivers/hid/hid-roccat-koneplus.c
@@ -385,10 +385,9 @@ static int koneplus_init_specials(struct hid_device *hdev)
== USB_INTERFACE_PROTOCOL_MOUSE) {
koneplus = kzalloc(sizeof(*koneplus), GFP_KERNEL);
- if (!koneplus) {
- hid_err(hdev, "can't alloc device descriptor\n");
+ if (!koneplus)
return -ENOMEM;
- }
+
hid_set_drvdata(hdev, koneplus);
retval = koneplus_init_koneplus_device_struct(usb_dev, koneplus);
diff --git a/drivers/hid/hid-roccat-konepure.c b/drivers/hid/hid-roccat-konepure.c
index 07de2f9014c6..529e6024e73f 100644
--- a/drivers/hid/hid-roccat-konepure.c
+++ b/drivers/hid/hid-roccat-konepure.c
@@ -89,10 +89,9 @@ static int konepure_init_specials(struct hid_device *hdev)
}
konepure = kzalloc(sizeof(*konepure), GFP_KERNEL);
- if (!konepure) {
- hid_err(hdev, "can't alloc device descriptor\n");
+ if (!konepure)
return -ENOMEM;
- }
+
hid_set_drvdata(hdev, konepure);
retval = roccat_common2_device_init_struct(usb_dev, konepure);
diff --git a/drivers/hid/hid-roccat-kovaplus.c b/drivers/hid/hid-roccat-kovaplus.c
index 43617fb28b87..da9c090542c6 100644
--- a/drivers/hid/hid-roccat-kovaplus.c
+++ b/drivers/hid/hid-roccat-kovaplus.c
@@ -452,10 +452,9 @@ static int kovaplus_init_specials(struct hid_device *hdev)
== USB_INTERFACE_PROTOCOL_MOUSE) {
kovaplus = kzalloc(sizeof(*kovaplus), GFP_KERNEL);
- if (!kovaplus) {
- hid_err(hdev, "can't alloc device descriptor\n");
+ if (!kovaplus)
return -ENOMEM;
- }
+
hid_set_drvdata(hdev, kovaplus);
retval = kovaplus_init_kovaplus_device_struct(usb_dev, kovaplus);
diff --git a/drivers/hid/hid-roccat-lua.c b/drivers/hid/hid-roccat-lua.c
index ac1a7313e259..0ccaa345de27 100644
--- a/drivers/hid/hid-roccat-lua.c
+++ b/drivers/hid/hid-roccat-lua.c
@@ -123,10 +123,9 @@ static int lua_init_specials(struct hid_device *hdev)
int retval;
lua = kzalloc(sizeof(*lua), GFP_KERNEL);
- if (!lua) {
- hid_err(hdev, "can't alloc device descriptor\n");
+ if (!lua)
return -ENOMEM;
- }
+
hid_set_drvdata(hdev, lua);
retval = lua_init_lua_device_struct(usb_dev, lua);
diff --git a/drivers/hid/hid-roccat-pyra.c b/drivers/hid/hid-roccat-pyra.c
index b30aa7b82bf8..c22cc42dadb6 100644
--- a/drivers/hid/hid-roccat-pyra.c
+++ b/drivers/hid/hid-roccat-pyra.c
@@ -404,10 +404,9 @@ static int pyra_init_specials(struct hid_device *hdev)
== USB_INTERFACE_PROTOCOL_MOUSE) {
pyra = kzalloc(sizeof(*pyra), GFP_KERNEL);
- if (!pyra) {
- hid_err(hdev, "can't alloc device descriptor\n");
+ if (!pyra)
return -ENOMEM;
- }
+
hid_set_drvdata(hdev, pyra);
retval = pyra_init_pyra_device_struct(usb_dev, pyra);
diff --git a/drivers/hid/hid-roccat-ryos.c b/drivers/hid/hid-roccat-ryos.c
index 47cc8f30ff6d..1153e07ae415 100644
--- a/drivers/hid/hid-roccat-ryos.c
+++ b/drivers/hid/hid-roccat-ryos.c
@@ -97,10 +97,9 @@ static int ryos_init_specials(struct hid_device *hdev)
}
ryos = kzalloc(sizeof(*ryos), GFP_KERNEL);
- if (!ryos) {
- hid_err(hdev, "can't alloc device descriptor\n");
+ if (!ryos)
return -ENOMEM;
- }
+
hid_set_drvdata(hdev, ryos);
retval = roccat_common2_device_init_struct(usb_dev, ryos);
diff --git a/drivers/hid/hid-roccat-savu.c b/drivers/hid/hid-roccat-savu.c
index 6dbf6e04dce7..3b39f6556708 100644
--- a/drivers/hid/hid-roccat-savu.c
+++ b/drivers/hid/hid-roccat-savu.c
@@ -69,10 +69,9 @@ static int savu_init_specials(struct hid_device *hdev)
}
savu = kzalloc(sizeof(*savu), GFP_KERNEL);
- if (!savu) {
- hid_err(hdev, "can't alloc device descriptor\n");
+ if (!savu)
return -ENOMEM;
- }
+
hid_set_drvdata(hdev, savu);
retval = roccat_common2_device_init_struct(usb_dev, savu);
--
2.16.1
^ permalink raw reply related
* Re: [PATCH 1/2] HID: core: i2c-hid: fix size check and type usage
From: Marcus Folkesson @ 2018-02-03 8:36 UTC (permalink / raw)
To: Aaron Ma; +Cc: linux-kernel, linux-input, jikos, benjamin.tissoires
In-Reply-To: <20180102173006.506-1-aaron.ma@canonical.com>
[-- Attachment #1: Type: text/plain, Size: 2918 bytes --]
Hi Aaron,
On Wed, Jan 03, 2018 at 01:30:05AM +0800, Aaron Ma wrote:
> When convert char array with signed int, if the inbuf[x] is negative then
> upper bits will be set to 1. Fix this by using u8 instead of char.
>
> ret_size has to be at least 3, hid_input_report use it after minus 2 bytes.
At least 2?
hid_input_report() checks (!size)
>
> size should be more than 0 to keep memset safe.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> drivers/hid/hid-core.c | 4 ++--
> drivers/hid/i2c-hid/i2c-hid.c | 10 +++++-----
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 0c3f608131cf..992547771d96 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1506,7 +1506,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
> if (rsize > HID_MAX_BUFFER_SIZE)
> rsize = HID_MAX_BUFFER_SIZE;
>
> - if (csize < rsize) {
> + if ((csize < rsize) && (csize > 0)) {
> dbg_hid("report %d is too short, (%d < %d)\n", report->id,
> csize, rsize);
> memset(cdata + csize, 0, rsize - csize);
> @@ -1566,7 +1566,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i
> report_enum = hid->report_enum + type;
> hdrv = hid->driver;
>
> - if (!size) {
> + if (size <= 0) {
All code that is using hid_input_report() seems to check that no
negative size is provided.
If we want this check, maybe we should consider making size unsigned
instead?
> dbg_hid("empty report\n");
> ret = -1;
> goto unlock;
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index e054ee43c1e2..09404ffdb08b 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -144,10 +144,10 @@ struct i2c_hid {
> * register of the HID
> * descriptor. */
> unsigned int bufsize; /* i2c buffer size */
> - char *inbuf; /* Input buffer */
> - char *rawbuf; /* Raw Input buffer */
> - char *cmdbuf; /* Command buffer */
> - char *argsbuf; /* Command arguments buffer */
> + u8 *inbuf; /* Input buffer */
> + u8 *rawbuf; /* Raw Input buffer */
> + u8 *cmdbuf; /* Command buffer */
> + u8 *argsbuf; /* Command arguments buffer */
Looks good
>
> unsigned long flags; /* device flags */
> unsigned long quirks; /* Various quirks */
> @@ -473,7 +473,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
>
> ret_size = ihid->inbuf[0] | ihid->inbuf[1] << 8;
>
> - if (!ret_size) {
> + if (ret_size <= 2) {
if (ret_size < 2) ?
> /* host or device initiated RESET completed */
> if (test_and_clear_bit(I2C_HID_RESET_PENDING, &ihid->flags))
> wake_up(&ihid->wait);
> --
> 2.14.3
Best regards
Marcus Folkesson
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] hid-saitek: Delete an error message for a failed memory allocation in saitek_probe()
From: SF Markus Elfring @ 2018-02-03 8:19 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 3 Feb 2018 09:12:24 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/hid-saitek.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/hid-saitek.c b/drivers/hid/hid-saitek.c
index 39e642686ff0..49355ceddd85 100644
--- a/drivers/hid/hid-saitek.c
+++ b/drivers/hid/hid-saitek.c
@@ -45,10 +45,8 @@ static int saitek_probe(struct hid_device *hdev,
int ret;
ssc = devm_kzalloc(&hdev->dev, sizeof(*ssc), GFP_KERNEL);
- if (ssc == NULL) {
- hid_err(hdev, "can't alloc saitek descriptor\n");
+ if (!ssc)
return -ENOMEM;
- }
ssc->quirks = quirks;
ssc->mode = -1;
--
2.16.1
^ permalink raw reply related
* Re: [PATCH v2 2/2] HID: core: Fix size as type u32
From: Marcus Folkesson @ 2018-02-03 7:55 UTC (permalink / raw)
To: Aaron Ma; +Cc: linux-kernel, linux-input, jikos, benjamin.tissoires
In-Reply-To: <20180108024141.10590-2-aaron.ma@canonical.com>
[-- Attachment #1: Type: text/plain, Size: 4086 bytes --]
Hi Aaron,
On Mon, Jan 08, 2018 at 10:41:41AM +0800, Aaron Ma wrote:
> When size is negative, calling memset will make segment fault.
> Declare the size as type u32 to keep memset safe.
>
> size in struct hid_report is unsigned, fix return type of
> hid_report_len to u32.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> ---
> drivers/hid/hid-core.c | 10 +++++-----
> include/linux/hid.h | 6 +++---
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 0c3f608131cf..cf81c53e3b98 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1390,7 +1390,7 @@ u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t flags)
> * of implement() working on 8 byte chunks
> */
>
> - int len = hid_report_len(report) + 7;
> + u32 len = hid_report_len(report) + 7;
>
> return kmalloc(len, flags);
> }
> @@ -1455,7 +1455,7 @@ void __hid_request(struct hid_device *hid, struct hid_report *report,
> {
> char *buf;
> int ret;
> - int len;
> + u32 len;
>
> buf = hid_alloc_report_buf(report, GFP_KERNEL);
> if (!buf)
> @@ -1481,14 +1481,14 @@ void __hid_request(struct hid_device *hid, struct hid_report *report,
> }
> EXPORT_SYMBOL_GPL(__hid_request);
>
> -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
> +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
> int interrupt)
> {
> struct hid_report_enum *report_enum = hid->report_enum + type;
> struct hid_report *report;
> struct hid_driver *hdrv;
> unsigned int a;
> - int rsize, csize = size;
> + u32 rsize, csize = size;
> u8 *cdata = data;
> int ret = 0;
>
> @@ -1546,7 +1546,7 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event);
> *
> * This is data entry for lower layers.
> */
> -int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int interrupt)
> +int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int interrupt)
> {
> struct hid_report_enum *report_enum;
> struct hid_driver *hdrv;
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index d491027a7c22..9bc296eebc98 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -841,7 +841,7 @@ extern int hidinput_connect(struct hid_device *hid, unsigned int force);
> extern void hidinput_disconnect(struct hid_device *);
>
> int hid_set_field(struct hid_field *, unsigned, __s32);
> -int hid_input_report(struct hid_device *, int type, u8 *, int, int);
> +int hid_input_report(struct hid_device *, int type, u8 *, u32, int);
> int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned int code, struct hid_field **field);
> struct hid_field *hidinput_get_led_field(struct hid_device *hid);
> unsigned int hidinput_count_leds(struct hid_device *hid);
> @@ -1088,13 +1088,13 @@ static inline void hid_hw_wait(struct hid_device *hdev)
> *
> * @report: the report we want to know the length
> */
> -static inline int hid_report_len(struct hid_report *report)
> +static inline u32 hid_report_len(struct hid_report *report)
hid_report_len() is used in several files.
If we think it is a good idea to change the return type, we should fix
these files as well.
[08:47:56]marcus@little:~/git/linux$ git grep -l hid_report_len
drivers/hid/hid-core.c
drivers/hid/hid-input.c
drivers/hid/hid-multitouch.c
drivers/hid/hid-rmi.c
drivers/hid/usbhid/hid-core.c
drivers/hid/wacom_sys.c
drivers/staging/greybus/hid.c
include/linux/hid.h
> {
> /* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */
> return ((report->size - 1) >> 3) + 1 + (report->id > 0);
> }
>
> -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
> +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
> int interrupt);
>
> /* HID quirks API */
> --
> 2.14.3
Best regards
Marcus Folkesson
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device
From: Aaron Ma @ 2018-02-03 3:59 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: lkml, linux-input, Jiri Kosina
In-Reply-To: <7ddae302-a44f-b729-c826-082bdcb38d70@canonical.com>
Hi
Could anyone review an apply this single patch?
The 2nd patch had been sent as v2.
Regards,
Aaron
^ permalink raw reply
* Re: [PATCH v2 2/2] HID: core: Fix size as type u32
From: Aaron Ma @ 2018-02-03 3:57 UTC (permalink / raw)
To: linux-kernel, linux-input, jikos, benjamin.tissoires
In-Reply-To: <20180108024141.10590-2-aaron.ma@canonical.com>
Hi:
Could anyone review and apply these 2 patch?
Regards,
Aaron
^ permalink raw reply
* [PATCH] hid-sony: Delete an error message for a failed memory allocation in two functions
From: SF Markus Elfring @ 2018-02-02 21:12 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 2 Feb 2018 22:08:31 +0100
Omit an extra message for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/hid-sony.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index b9dc3ac4d4aa..dc22f40c2ea2 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1930,7 +1930,6 @@ static int sony_leds_init(struct sony_sc *sc)
led = kzalloc(sizeof(struct led_classdev) + name_sz, GFP_KERNEL);
if (!led) {
- hid_err(hdev, "Couldn't allocate memory for LED %d\n", n);
ret = -ENOMEM;
goto error_leds;
}
@@ -2722,10 +2721,8 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
quirks |= FUTUREMAX_DANCE_MAT;
sc = devm_kzalloc(&hdev->dev, sizeof(*sc), GFP_KERNEL);
- if (sc == NULL) {
- hid_err(hdev, "can't alloc sony descriptor\n");
+ if (!sc)
return -ENOMEM;
- }
spin_lock_init(&sc->lock);
--
2.16.1
^ permalink raw reply related
* Re: [PATCH] Input: synaptics - Lenovo Carbon X1 Gen5 (2017) devices should use RMI
From: Dmitry Torokhov @ 2018-02-02 20:50 UTC (permalink / raw)
To: Damjan Georgievski; +Cc: linux-input
In-Reply-To: <CAEk1YH7Ni04Hs4RHtDJyE368aA4-DSW5aGx+v8kpbhLF2OR2Sg@mail.gmail.com>
On Fri, Aug 25, 2017 at 12:55:29PM +0200, Damjan Georgievski wrote:
> On 21 August 2017 at 18:25, Damjan Georgievski <gdamjan@gmail.com> wrote:
> >> I wonder if we have different configs...
> >>
> >>>
> >>> Second issues is, trackpoint is reported as "ImPS/2 Generic Wheel
> >>> Mouse". Middle button scrolling is not enabled by default.
> >>
> >> I think this will be fixed by:
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/patch/?id=ec667683c532c93fb41e100e5d61a518971060e2
> >>
> >
> > Interesting, with this patch, the trackpoint appeared sooner, but
> > still late (at the 44 second mark):
>
> but that seems sporadic. on second boot, it didn't show up until I
> reloaded the psmouse module
>
> any ideas how to debug this issue further? possibly before the change
> goes upstream.
Damjan,
Since my other patches seem to have fixed the trackpoint detection
issue, mind giving your "tested-by" to this patch so I can get it in?
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] hid-zydacron: Delete an error message for a failed memory allocation in zc_probe()
From: SF Markus Elfring @ 2018-02-02 20:46 UTC (permalink / raw)
To: linux-input, Benjamin Tissoires, Jiri Kosina; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 2 Feb 2018 21:41:37 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/hid/hid-zydacron.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/hid/hid-zydacron.c b/drivers/hid/hid-zydacron.c
index 1a660bd97ab2..3e8afaea2ab3 100644
--- a/drivers/hid/hid-zydacron.c
+++ b/drivers/hid/hid-zydacron.c
@@ -170,10 +170,8 @@ static int zc_probe(struct hid_device *hdev, const struct hid_device_id *id)
struct zc_device *zc;
zc = devm_kzalloc(&hdev->dev, sizeof(*zc), GFP_KERNEL);
- if (zc == NULL) {
- hid_err(hdev, "can't alloc descriptor\n");
+ if (!zc)
return -ENOMEM;
- }
hid_set_drvdata(hdev, zc);
--
2.16.1
^ permalink raw reply related
* Re: Support for touch controller ILI2511: Integrate into ili210x or new driver ?
From: Dmitry Torokhov @ 2018-02-02 20:06 UTC (permalink / raw)
To: Stefan Roese; +Cc: linux-input, Henrik Rydberg, Olivier Sobrie
In-Reply-To: <92137959-654f-79f9-e15f-d9bed2185518@denx.de>
Hi Stefan,
On Fri, Feb 02, 2018 at 05:03:10PM +0100, Stefan Roese wrote:
> Hi,
>
> I'm currently working on supporting the Ilitek ILI2511 touch controller
> in mainline. The 2 choices I have are:
>
> a) Try to integrate the ILI2511 support into the existing ili210x
> driver
>
> or
>
> b) Create a new driver for ILI2511 which might be suitable for other /
> future Ilitek controller
>
> Looking at alternative a), ili210x seems to be pretty unused and in not
> real good shape (only supporting platform-data, not using devm_ calls,
> not using threaded interrupts etc...). I could enhance this driver but
> my main concern here is, that I have no chance to test this driver on
> the currently supported chips.
>
> Alternative b) sounds more promising to me. Especially since ILI2511
> supports touch protocol v3.x (Ilitek naming), which is quite different
> from the currently supported protocol.
>
> What's your recommendation here? Is it okay to go with adding a new
> driver, perhaps named something like ili2xxx?
It all depends on how much code can be shared. If protocol is
sufficiently different I do not see why it can't be a separate driver.
OTOH we do not have any users of ili210x in mainline, so I would not
worry too much if we break it - interested parties should have been
working on upstreaming the rest of their code if it is important to
them. I'll gladly take patches converting to devm, dropping platform
data support, using generic device properties, etc.
Thanks.
--
Dmitry
^ permalink raw reply
* Support for touch controller ILI2511: Integrate into ili210x or new driver ?
From: Stefan Roese @ 2018-02-02 16:03 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Henrik Rydberg, Olivier Sobrie
Hi,
I'm currently working on supporting the Ilitek ILI2511 touch controller
in mainline. The 2 choices I have are:
a) Try to integrate the ILI2511 support into the existing ili210x
driver
or
b) Create a new driver for ILI2511 which might be suitable for other /
future Ilitek controller
Looking at alternative a), ili210x seems to be pretty unused and in not
real good shape (only supporting platform-data, not using devm_ calls,
not using threaded interrupts etc...). I could enhance this driver but
my main concern here is, that I have no chance to test this driver on
the currently supported chips.
Alternative b) sounds more promising to me. Especially since ILI2511
supports touch protocol v3.x (Ilitek naming), which is quite different
from the currently supported protocol.
What's your recommendation here? Is it okay to go with adding a new
driver, perhaps named something like ili2xxx?
Thanks,
Stefan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox