* Re: [PATCH RESEND] HID: fix error message in hid_open_report()
From: Jiri Kosina @ 2019-08-20 14:55 UTC (permalink / raw)
To: Michał Mirosław; +Cc: linux-input, Benjamin Tissoires, linux-kernel
In-Reply-To: <1230b0c19fd21fdc4d0eb30f3e32e67fff86fef9.1563818405.git.mirq-linux@rere.qmqm.pl>
On Mon, 22 Jul 2019, Michał Mirosław wrote:
> On HID report descriptor parsing error the code displays bogus
> pointer instead of error offset (subtracts start=NULL from end).
> Make the message more useful by displaying correct error offset
> and include total buffer size for reference.
>
> This was carried over from ancient times - "Fixed" commit just
> promoted the message from DEBUG to ERROR.
>
> Cc: stable@vger.kernel.org
> Fixes: 8c3d52fc393b ("HID: make parser more verbose about parsing errors by default")
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
[ ... snip ... ]
> @@ -1230,7 +1232,8 @@ int hid_open_report(struct hid_device *device)
> }
> }
>
> - hid_err(device, "item fetching failed at offset %d\n", (int)(end - start));
> + hid_err(device, "item fetching failed at offset %zu/%zu\n",
> + size - (end - start), size);
Hi Michal,
thanks for the fix.
This causes:
drivers/hid/hid-core.c: In function ‘hid_open_report’:
drivers/hid/hid-core.c:1235:2: warning: format ‘%zu’ expects argument of type ‘size_t’, but argument 4 has type ‘unsigned int’ [-Wformat=]
hid_err(device, "item fetching failed at offset %zu/%zu\n",
^
could you please fix that up and resubmit?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 2/2] drivers: input: mouse: alps: drop unneeded likely() call around IS_ERR()
From: Pali Rohár @ 2019-08-20 14:22 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: Enrico Weigelt, metux IT consult, linux-kernel, dmitry.torokhov,
linux-input, linux-ntfs-dev
In-Reply-To: <02f5b546-5c30-4998-19b2-76b816a35371@metux.net>
On Tuesday 20 August 2019 14:21:33 Enrico Weigelt, metux IT consult wrote:
> On 20.08.19 13:17, Pali Rohár wrote:
> > On Tuesday 20 August 2019 12:56:12 Enrico Weigelt, metux IT consult wrote:
> > > From: Enrico Weigelt <info@metux.net>
> > >
> > > IS_ERR() already calls unlikely(), so this extra unlikely() call
> > > around IS_ERR() is not needed.
> > >
> > > Signed-off-by: Enrico Weigelt <info@metux.net>
> > > ---
> > > drivers/input/mouse/alps.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> > > index 34700ed..ed16614 100644
> > > --- a/drivers/input/mouse/alps.c
> > > +++ b/drivers/input/mouse/alps.c
> > > @@ -1476,7 +1476,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
> > > /* On V2 devices the DualPoint Stick reports bare packets */
> > > dev = priv->dev2;
> > > dev2 = psmouse->dev;
> > > - } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
> > > + } else if (IS_ERR_OR_NULL(priv->dev3)) {
> > > /* Register dev3 mouse if we received PS/2 packet first time */
> > > if (!IS_ERR(priv->dev3))
> > > psmouse_queue_work(psmouse, &priv->dev3_register_work,
> >
> > Hello! Two months ago I already saw this patch. See discussion:
> > https://patchwork.kernel.org/patch/10977099/
>
> phuh, that's long chain of links to folow ;-)
>
> So, your primary argument is just *documenting* that this supposed to
> be a rare condition ?
Yes. Also Dmitry said that prefer explicit unlikely.
> In that case, wouldn't a comment be more suitable for that ?
And why to add comment if current state of code is more-readable and
does not need it?
I do not see anything wrong on this code path.
People normally add comments to code which is problematic to understand
or is somehow tricky, no so obvious or document how should code behave.
> It seems that this issue is coming up again and again ... when people
> run cocci scans (like I didn't), I'd suspect this to happen even more
> in the future.
People first need to understand that static code analysis cannot work
100% reliably and always is needed to properly review changes.
And if the only reason for this change is to satisfy some static code
analysis program, would not it better to teach this program to no
generate such false-positive results?
--
Pali Rohár
pali.rohar@gmail.com
^ permalink raw reply
* [PATCH v2] (submitted) input: misc: soc_button_array: use platform_device_register_resndata()
From: Enrico Weigelt, metux IT consult @ 2019-08-20 12:25 UTC (permalink / raw)
To: linux-kernel; +Cc: dmitry.torokhov, linux-input
From: Enrico Weigelt <info@metux.net>
The registration of gpio-keys device can be written much shorter
by using the platform_device_register_resndata() helper.
v2:
* pass &pdev->dev to platform_device_register_resndata()
* fixed errval on failed platform_device_register_resndata()
Signed-off-by: Enrico Weigelt <info@metux.net>
---
drivers/input/misc/soc_button_array.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 5e59f8e5..27550f9 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -110,25 +110,24 @@ static int soc_button_lookup_gpio(struct device *dev, int acpi_index)
gpio_keys_pdata->nbuttons = n_buttons;
gpio_keys_pdata->rep = autorepeat;
- pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
- if (!pd) {
- error = -ENOMEM;
+ pd = platform_device_register_resndata(
+ &pdev->dev,
+ "gpio-keys",
+ PLATFORM_DEVID_AUTO,
+ NULL,
+ 0,
+ gpio_keys_pdata,
+ sizeof(*gpio_keys_pdata));
+
+ error = PTR_ERR_OR_ZERO(pd);
+
+ if (IS_ERR(pd)) {
+ dev_err(&pdev->dev, "failed registering gpio-keys: %ld\n", PTR_ERR(pd));
goto err_free_mem;
}
- error = platform_device_add_data(pd, gpio_keys_pdata,
- sizeof(*gpio_keys_pdata));
- if (error)
- goto err_free_pdev;
-
- error = platform_device_add(pd);
- if (error)
- goto err_free_pdev;
-
return pd;
-err_free_pdev:
- platform_device_put(pd);
err_free_mem:
devm_kfree(&pdev->dev, gpio_keys_pdata);
return ERR_PTR(error);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH 2/2] drivers: input: mouse: alps: drop unneeded likely() call around IS_ERR()
From: Enrico Weigelt, metux IT consult @ 2019-08-20 12:21 UTC (permalink / raw)
To: Pali Rohár, Enrico Weigelt, metux IT consult
Cc: linux-kernel, dmitry.torokhov, linux-input, linux-ntfs-dev
In-Reply-To: <20190820111719.7blyk5jstgwde2ae@pali>
On 20.08.19 13:17, Pali Rohár wrote:
> On Tuesday 20 August 2019 12:56:12 Enrico Weigelt, metux IT consult wrote:
>> From: Enrico Weigelt <info@metux.net>
>>
>> IS_ERR() already calls unlikely(), so this extra unlikely() call
>> around IS_ERR() is not needed.
>>
>> Signed-off-by: Enrico Weigelt <info@metux.net>
>> ---
>> drivers/input/mouse/alps.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
>> index 34700ed..ed16614 100644
>> --- a/drivers/input/mouse/alps.c
>> +++ b/drivers/input/mouse/alps.c
>> @@ -1476,7 +1476,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
>> /* On V2 devices the DualPoint Stick reports bare packets */
>> dev = priv->dev2;
>> dev2 = psmouse->dev;
>> - } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
>> + } else if (IS_ERR_OR_NULL(priv->dev3)) {
>> /* Register dev3 mouse if we received PS/2 packet first time */
>> if (!IS_ERR(priv->dev3))
>> psmouse_queue_work(psmouse, &priv->dev3_register_work,
>
> Hello! Two months ago I already saw this patch. See discussion:
> https://patchwork.kernel.org/patch/10977099/
phuh, that's long chain of links to folow ;-)
So, your primary argument is just *documenting* that this supposed to
be a rare condition ?
In that case, wouldn't a comment be more suitable for that ?
It seems that this issue is coming up again and again ... when people
run cocci scans (like I didn't), I'd suspect this to happen even more
in the future.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* Re: [PATCH] Input: add support for polling to input devices
From: Michal Vokáč @ 2019-08-20 11:56 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, open list:HID CORE LAYER, lkml, Rob Herring
In-Reply-To: <CAO-hwJKfHCwLkEDWrzJHejjaB+vY=0RsfY-=xfdRUSQPpeUVAg@mail.gmail.com>
On 13. 08. 19 16:04, Benjamin Tissoires wrote:
> On Mon, Aug 12, 2019 at 7:11 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
>>
>> Hi Benjamin,
>>
>> On Mon, Aug 12, 2019 at 06:50:38PM +0200, Benjamin Tissoires wrote:
>>> Hi Dmitry,
>>>
>>> On Fri, Aug 9, 2019 at 7:35 PM Dmitry Torokhov
>>> <dmitry.torokhov@gmail.com> wrote:
>>>>
>>>> Separating "normal" and "polled" input devices was a mistake, as often we
>>>> want to allow the very same device work on both interrupt-driven and
>>>> polled mode, depending on the board on which the device is used.
>>>>
>>>> This introduces new APIs:
>>>>
>>>> - input_setup_polling
>>>> - input_set_poll_interval
>>>> - input_set_min_poll_interval
>>>> - input_set_max_poll_interval
>>>>
>>>> These new APIs allow switching an input device into polled mode with sysfs
>>>> attributes matching drivers using input_polled_dev APIs that will be
>>>> eventually removed.
>>>
>>> Are you sure that using sysfs is the correct way here?
>>> I would think using generic properties that can be overwritten by the
>>> DSDT or by the device tree would make things easier from a driver
>>> point of view.
>>
>> Couple of points: I wanted it to be compatible with input-polldev.c so
>> the sysfs attributes must match (so that we can convert existing drivers
>> and zap input-polldev).
>
> Oh, I missed that. Good point.
>
>> I also am not sure if polling parameters are
>> property of board, or it is either fundamental hardware limitation or
>> simply desired system behavior.
>
> I think it's a combination of everything: sometimes the board misses
> the capability to not do IRQs for that device, and using properties
> would be better here: you can define them where you need (board,
> platform or device level), and have a working platfrom from the kernel
> description entirely.
> However, it doesn't solve the issue of input-polldev, so maybe
> properties should be added on top of this sysfs.
>
>> If Rob is OK with adding device
>> properties I'd be fine adding them as a followup, otherwise we can have
>> udev adjust the behavior as needed for given box shortly after boot.
>
> Fair enough.
>
>>
>>>
>>> Also, checkpatch complains about a few octal permissions that are
>>> preferred over symbolic ones, and there is a possible 'out of memory'
>>> nessage at drivers/input/input-poller.c:75.
>>
>> Yes, there is. It is there so we would know what device we were trying
>> to set up when OOM happened. You can probable decipher the driver from
>> the stack trace, but figuring particular device instance is harder.
>
> Could you add a comment there explaining this choice? I have a feeling
> you'll have to refuse a few patches of people running coccinelle
> scripts and be too happy to send a kernel patch.
>
> Other than that:
> Acked-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
Hi Dmitry,
what is the status of this patch? Are we still waiting for Rob to
comment on the device properties or is this ready to land?
Little bit OT question: what tree/branch do you use to apply patches?
According to the mailing list you recently applied some patches but
I can not find them here [1].
Thank you,
Michal
[1] https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/
>>>>
>>>> Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
>>>> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>>> ---
>>>> drivers/input/Makefile | 2 +-
>>>> drivers/input/input-poller.c | 208 +++++++++++++++++++++++++++++++++++
>>>> drivers/input/input-poller.h | 18 +++
>>>> drivers/input/input.c | 36 ++++--
>>>> include/linux/input.h | 12 ++
>>>> 5 files changed, 268 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/input/Makefile b/drivers/input/Makefile
>>>> index 40de6a7be641..e35650930371 100644
>>>> --- a/drivers/input/Makefile
>>>> +++ b/drivers/input/Makefile
>>>> @@ -6,7 +6,7 @@
>>>> # Each configuration option enables a list of files.
>>>>
>>>> obj-$(CONFIG_INPUT) += input-core.o
>>>> -input-core-y := input.o input-compat.o input-mt.o ff-core.o
>>>> +input-core-y := input.o input-compat.o input-mt.o input-poller.o ff-core.o
>>>>
>>>> obj-$(CONFIG_INPUT_FF_MEMLESS) += ff-memless.o
>>>> obj-$(CONFIG_INPUT_POLLDEV) += input-polldev.o
>>>> diff --git a/drivers/input/input-poller.c b/drivers/input/input-poller.c
>>>> new file mode 100644
>>>> index 000000000000..e041adb04f5a
>>>> --- /dev/null
>>>> +++ b/drivers/input/input-poller.c
>>>> @@ -0,0 +1,208 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-only
>>>> +/*
>>>> + * Support for polling mode for input devices.
>>>> + */
>>>> +
>>>> +#include <linux/device.h>
>>>> +#include <linux/input.h>
>>>> +#include <linux/jiffies.h>
>>>> +#include <linux/mutex.h>
>>>> +#include <linux/slab.h>
>>>> +#include <linux/types.h>
>>>> +#include <linux/workqueue.h>
>>>> +#include "input-poller.h"
>>>> +
>>>> +struct input_dev_poller {
>>>> + void (*poll)(struct input_dev *dev);
>>>> +
>>>> + unsigned int poll_interval; /* msec */
>>>> + unsigned int poll_interval_max; /* msec */
>>>> + unsigned int poll_interval_min; /* msec */
>>>> +
>>>> + struct input_dev *input;
>>>> + struct delayed_work work;
>>>> +};
>>>> +
>>>> +static void input_dev_poller_queue_work(struct input_dev_poller *poller)
>>>> +{
>>>> + unsigned long delay;
>>>> +
>>>> + delay = msecs_to_jiffies(poller->poll_interval);
>>>> + if (delay >= HZ)
>>>> + delay = round_jiffies_relative(delay);
>>>> +
>>>> + queue_delayed_work(system_freezable_wq, &poller->work, delay);
>>>> +}
>>>> +
>>>> +static void input_dev_poller_work(struct work_struct *work)
>>>> +{
>>>> + struct input_dev_poller *poller =
>>>> + container_of(work, struct input_dev_poller, work.work);
>>>> +
>>>> + poller->poll(poller->input);
>>>> + input_dev_poller_queue_work(poller);
>>>> +}
>>>> +
>>>> +void input_dev_poller_finalize(struct input_dev_poller *poller)
>>>> +{
>>>> + if (!poller->poll_interval)
>>>> + poller->poll_interval = 500;
>>>> + if (!poller->poll_interval_max)
>>>> + poller->poll_interval_max = poller->poll_interval;
>>>> +}
>>>> +
>>>> +void input_dev_poller_start(struct input_dev_poller *poller)
>>>> +{
>>>> + /* Only start polling if polling is enabled */
>>>> + if (poller->poll_interval > 0) {
>>>> + poller->poll(poller->input);
>>>> + input_dev_poller_queue_work(poller);
>>>> + }
>>>> +}
>>>> +
>>>> +void input_dev_poller_stop(struct input_dev_poller *poller)
>>>> +{
>>>> + cancel_delayed_work_sync(&poller->work);
>>>> +}
>>>> +
>>>> +int input_setup_polling(struct input_dev *dev,
>>>> + void (*poll_fn)(struct input_dev *dev))
>>>> +{
>>>> + struct input_dev_poller *poller;
>>>> +
>>>> + poller = kzalloc(sizeof(*poller), GFP_KERNEL);
>>>> + if (!poller) {
>>>> + dev_err(dev->dev.parent ?: &dev->dev,
>>>> + "%s: unable to allocate poller structure\n", __func__);
>>>> + return -ENOMEM;
>>>> + }
>>>> +
>>>> + INIT_DELAYED_WORK(&poller->work, input_dev_poller_work);
>>>> + poller->input = dev;
>>>> + poller->poll = poll_fn;
>>>> +
>>>> + dev->poller = poller;
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(input_setup_polling);
>>>> +
>>>> +static bool input_dev_ensure_poller(struct input_dev *dev)
>>>> +{
>>>> + if (!dev->poller) {
>>>> + dev_err(dev->dev.parent ?: &dev->dev,
>>>> + "poller structure has not been set up\n");
>>>> + return false;
>>>> + }
>>>> +
>>>> + return true;
>>>> +}
>>>> +
>>>> +void input_set_poll_interval(struct input_dev *dev, unsigned int interval)
>>>> +{
>>>> + if (input_dev_ensure_poller(dev))
>>>> + dev->poller->poll_interval = interval;
>>>> +}
>>>> +EXPORT_SYMBOL(input_set_poll_interval);
>>>> +
>>>> +void input_set_min_poll_interval(struct input_dev *dev, unsigned int interval)
>>>> +{
>>>> + if (input_dev_ensure_poller(dev))
>>>> + dev->poller->poll_interval_min = interval;
>>>> +}
>>>> +EXPORT_SYMBOL(input_set_min_poll_interval);
>>>> +
>>>> +void input_set_max_poll_interval(struct input_dev *dev, unsigned int interval)
>>>> +{
>>>> + if (input_dev_ensure_poller(dev))
>>>> + dev->poller->poll_interval_max = interval;
>>>> +}
>>>> +EXPORT_SYMBOL(input_set_max_poll_interval);
>>>> +
>>>> +/* SYSFS interface */
>>>> +
>>>> +static ssize_t input_dev_get_poll_interval(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + char *buf)
>>>> +{
>>>> + struct input_dev *input = to_input_dev(dev);
>>>> +
>>>> + return sprintf(buf, "%d\n", input->poller->poll_interval);
>>>> +}
>>>> +
>>>> +static ssize_t input_dev_set_poll_interval(struct device *dev,
>>>> + struct device_attribute *attr,
>>>> + const char *buf, size_t count)
>>>> +{
>>>> + struct input_dev *input = to_input_dev(dev);
>>>> + struct input_dev_poller *poller = input->poller;
>>>> + unsigned int interval;
>>>> + int err;
>>>> +
>>>> + err = kstrtouint(buf, 0, &interval);
>>>> + if (err)
>>>> + return err;
>>>> +
>>>> + if (interval < poller->poll_interval_min)
>>>> + return -EINVAL;
>>>> +
>>>> + if (interval > poller->poll_interval_max)
>>>> + return -EINVAL;
>>>> +
>>>> + mutex_lock(&input->mutex);
>>>> +
>>>> + poller->poll_interval = interval;
>>>> +
>>>> + if (input->users) {
>>>> + cancel_delayed_work_sync(&poller->work);
>>>> + if (poller->poll_interval > 0)
>>>> + input_dev_poller_queue_work(poller);
>>>> + }
>>>> +
>>>> + mutex_unlock(&input->mutex);
>>>> +
>>>> + return count;
>>>> +}
>>>> +
>>>> +static DEVICE_ATTR(poll, S_IRUGO | S_IWUSR,
>>>> + input_dev_get_poll_interval, input_dev_set_poll_interval);
>>>> +
>>>> +static ssize_t input_dev_get_poll_max(struct device *dev,
>>>> + struct device_attribute *attr, char *buf)
>>>> +{
>>>> + struct input_dev *input = to_input_dev(dev);
>>>> +
>>>> + return sprintf(buf, "%d\n", input->poller->poll_interval_max);
>>>> +}
>>>> +
>>>> +static DEVICE_ATTR(max, S_IRUGO, input_dev_get_poll_max, NULL);
>>>> +
>>>> +static ssize_t input_dev_get_poll_min(struct device *dev,
>>>> + struct device_attribute *attr, char *buf)
>>>> +{
>>>> + struct input_dev *input = to_input_dev(dev);
>>>> +
>>>> + return sprintf(buf, "%d\n", input->poller->poll_interval_min);
>>>> +}
>>>> +
>>>> +static DEVICE_ATTR(min, S_IRUGO, input_dev_get_poll_min, NULL);
>>>> +
>>>> +static umode_t input_poller_attrs_visible(struct kobject *kobj,
>>>> + struct attribute *attr, int n)
>>>> +{
>>>> + struct device *dev = kobj_to_dev(kobj);
>>>> + struct input_dev *input = to_input_dev(dev);
>>>> +
>>>> + return input->poller ? attr->mode : 0;
>>>> +}
>>>> +
>>>> +static struct attribute *input_poller_attrs[] = {
>>>> + &dev_attr_poll.attr,
>>>> + &dev_attr_max.attr,
>>>> + &dev_attr_min.attr,
>>>> + NULL
>>>> +};
>>>> +
>>>> +struct attribute_group input_poller_attribute_group = {
>>>> + .is_visible = input_poller_attrs_visible,
>>>> + .attrs = input_poller_attrs,
>>>> +};
>>>> diff --git a/drivers/input/input-poller.h b/drivers/input/input-poller.h
>>>> new file mode 100644
>>>> index 000000000000..e3fca0be1d32
>>>> --- /dev/null
>>>> +++ b/drivers/input/input-poller.h
>>>> @@ -0,0 +1,18 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>>> +#ifndef _INPUT_POLLER_H
>>>> +#define _INPUT_POLLER_H
>>>> +
>>>> +/*
>>>> + * Support for polling mode for input devices.
>>>> + */
>>>> +#include <linux/sysfs.h>
>>>> +
>>>> +struct input_dev_poller;
>>>> +
>>>> +void input_dev_poller_finalize(struct input_dev_poller *poller);
>>>> +void input_dev_poller_start(struct input_dev_poller *poller);
>>>> +void input_dev_poller_stop(struct input_dev_poller *poller);
>>>> +
>>>> +extern struct attribute_group input_poller_attribute_group;
>>>> +
>>>> +#endif /* _INPUT_POLLER_H */
>>>> diff --git a/drivers/input/input.c b/drivers/input/input.c
>>>> index 7494a0dede79..c08aa3596144 100644
>>>> --- a/drivers/input/input.c
>>>> +++ b/drivers/input/input.c
>>>> @@ -24,6 +24,7 @@
>>>> #include <linux/mutex.h>
>>>> #include <linux/rcupdate.h>
>>>> #include "input-compat.h"
>>>> +#include "input-poller.h"
>>>>
>>>> MODULE_AUTHOR("Vojtech Pavlik <vojtech@suse.cz>");
>>>> MODULE_DESCRIPTION("Input core");
>>>> @@ -603,20 +604,31 @@ int input_open_device(struct input_handle *handle)
>>>>
>>>> handle->open++;
>>>>
>>>> - if (!dev->users++ && dev->open)
>>>> - retval = dev->open(dev);
>>>> + if (dev->users++) {
>>>> + /*
>>>> + * Device is already opened, so we can exit immediately and
>>>> + * report success.
>>>> + */
>>>> + goto out;
>>>> + }
>>>>
>>>> - if (retval) {
>>>> - dev->users--;
>>>> - if (!--handle->open) {
>>>> + if (dev->open) {
>>>> + retval = dev->open(dev);
>>>> + if (retval) {
>>>> + dev->users--;
>>>> + handle->open--;
>>>> /*
>>>> * Make sure we are not delivering any more events
>>>> * through this handle
>>>> */
>>>> synchronize_rcu();
>>>> + goto out;
>>>> }
>>>> }
>>>>
>>>> + if (dev->poller)
>>>> + input_dev_poller_start(dev->poller);
>>>> +
>>>> out:
>>>> mutex_unlock(&dev->mutex);
>>>> return retval;
>>>> @@ -655,8 +667,13 @@ void input_close_device(struct input_handle *handle)
>>>>
>>>> __input_release_device(handle);
>>>>
>>>> - if (!--dev->users && dev->close)
>>>> - dev->close(dev);
>>>> + if (!--dev->users) {
>>>> + if (dev->poller)
>>>> + input_dev_poller_stop(dev->poller);
>>>> +
>>>> + if (dev->close)
>>>> + dev->close(dev);
>>>> + }
>>>>
>>>> if (!--handle->open) {
>>>> /*
>>>> @@ -1502,6 +1519,7 @@ static const struct attribute_group *input_dev_attr_groups[] = {
>>>> &input_dev_attr_group,
>>>> &input_dev_id_attr_group,
>>>> &input_dev_caps_attr_group,
>>>> + &input_poller_attribute_group,
>>>> NULL
>>>> };
>>>>
>>>> @@ -1511,6 +1529,7 @@ static void input_dev_release(struct device *device)
>>>>
>>>> input_ff_destroy(dev);
>>>> input_mt_destroy_slots(dev);
>>>> + kfree(dev->poller);
>>>> kfree(dev->absinfo);
>>>> kfree(dev->vals);
>>>> kfree(dev);
>>>> @@ -2175,6 +2194,9 @@ int input_register_device(struct input_dev *dev)
>>>> if (!dev->setkeycode)
>>>> dev->setkeycode = input_default_setkeycode;
>>>>
>>>> + if (dev->poller)
>>>> + input_dev_poller_finalize(dev->poller);
>>>> +
>>>> error = device_add(&dev->dev);
>>>> if (error)
>>>> goto err_free_vals;
>>>> diff --git a/include/linux/input.h b/include/linux/input.h
>>>> index e95a439d8bd5..94f277cd806a 100644
>>>> --- a/include/linux/input.h
>>>> +++ b/include/linux/input.h
>>>> @@ -21,6 +21,8 @@
>>>> #include <linux/timer.h>
>>>> #include <linux/mod_devicetable.h>
>>>>
>>>> +struct input_dev_poller;
>>>> +
>>>> /**
>>>> * struct input_value - input value representation
>>>> * @type: type of value (EV_KEY, EV_ABS, etc)
>>>> @@ -71,6 +73,8 @@ enum input_clock_type {
>>>> * not sleep
>>>> * @ff: force feedback structure associated with the device if device
>>>> * supports force feedback effects
>>>> + * @poller: poller structure associated with the device if device is
>>>> + * set up to use polling mode
>>>> * @repeat_key: stores key code of the last key pressed; used to implement
>>>> * software autorepeat
>>>> * @timer: timer for software autorepeat
>>>> @@ -156,6 +160,8 @@ struct input_dev {
>>>>
>>>> struct ff_device *ff;
>>>>
>>>> + struct input_dev_poller *poller;
>>>> +
>>>> unsigned int repeat_key;
>>>> struct timer_list timer;
>>>>
>>>> @@ -372,6 +378,12 @@ void input_unregister_device(struct input_dev *);
>>>>
>>>> void input_reset_device(struct input_dev *);
>>>>
>>>> +int input_setup_polling(struct input_dev *dev,
>>>> + void (*poll_fn)(struct input_dev *dev));
>>>> +void input_set_poll_interval(struct input_dev *dev, unsigned int interval);
>>>> +void input_set_min_poll_interval(struct input_dev *dev, unsigned int interval);
>>>> +void input_set_max_poll_interval(struct input_dev *dev, unsigned int interval);
>>>> +
>>>> int __must_check input_register_handler(struct input_handler *);
>>>> void input_unregister_handler(struct input_handler *);
>>>>
>>>> --
>>>> 2.23.0.rc1.153.gdeed80330f-goog
>>>>
>>>>
>>>> --
>>>> Dmitry
>>
>> --
>> Dmitry
^ permalink raw reply
* Re: [PATCH 2/2] drivers: input: mouse: alps: drop unneeded likely() call around IS_ERR()
From: Pali Rohár @ 2019-08-20 11:17 UTC (permalink / raw)
To: Enrico Weigelt, metux IT consult
Cc: linux-kernel, dmitry.torokhov, linux-input, linux-ntfs-dev
In-Reply-To: <1566298572-12409-2-git-send-email-info@metux.net>
On Tuesday 20 August 2019 12:56:12 Enrico Weigelt, metux IT consult wrote:
> From: Enrico Weigelt <info@metux.net>
>
> IS_ERR() already calls unlikely(), so this extra unlikely() call
> around IS_ERR() is not needed.
>
> Signed-off-by: Enrico Weigelt <info@metux.net>
> ---
> drivers/input/mouse/alps.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 34700ed..ed16614 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -1476,7 +1476,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
> /* On V2 devices the DualPoint Stick reports bare packets */
> dev = priv->dev2;
> dev2 = psmouse->dev;
> - } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
> + } else if (IS_ERR_OR_NULL(priv->dev3)) {
> /* Register dev3 mouse if we received PS/2 packet first time */
> if (!IS_ERR(priv->dev3))
> psmouse_queue_work(psmouse, &priv->dev3_register_work,
Hello! Two months ago I already saw this patch. See discussion:
https://patchwork.kernel.org/patch/10977099/
--
Pali Rohár
pali.rohar@gmail.com
^ permalink raw reply
* [PATCH v2] input: misc: soc_button_array: use platform_device_register_resndata()
From: Enrico Weigelt, metux IT consult @ 2019-08-20 11:16 UTC (permalink / raw)
To: linux-kernel; +Cc: dmitry.torokhov, linux-input
From: Enrico Weigelt <info@metux.net>
The registration of gpio-keys device can be written much shorter
by using the platform_device_register_resndata() helper.
v2:
* pass &pdev->dev to platform_device_register_resndata()
* fixed errval on failed platform_device_register_resndata()
Signed-off-by: Enrico Weigelt <info@metux.net>
---
drivers/input/misc/soc_button_array.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/input/misc/soc_button_array.c b/drivers/input/misc/soc_button_array.c
index 5e59f8e5..27550f9 100644
--- a/drivers/input/misc/soc_button_array.c
+++ b/drivers/input/misc/soc_button_array.c
@@ -110,25 +110,24 @@ static int soc_button_lookup_gpio(struct device *dev, int acpi_index)
gpio_keys_pdata->nbuttons = n_buttons;
gpio_keys_pdata->rep = autorepeat;
- pd = platform_device_alloc("gpio-keys", PLATFORM_DEVID_AUTO);
- if (!pd) {
- error = -ENOMEM;
+ pd = platform_device_register_resndata(
+ &pdev->dev,
+ "gpio-keys",
+ PLATFORM_DEVID_AUTO,
+ NULL,
+ 0,
+ gpio_keys_pdata,
+ sizeof(*gpio_keys_pdata));
+
+ error = PTR_ERR_OR_ZERO(pd);
+
+ if (IS_ERR(pd)) {
+ dev_err(&pdev->dev, "failed registering gpio-keys: %ld\n", PTR_ERR(pd));
goto err_free_mem;
}
- error = platform_device_add_data(pd, gpio_keys_pdata,
- sizeof(*gpio_keys_pdata));
- if (error)
- goto err_free_pdev;
-
- error = platform_device_add(pd);
- if (error)
- goto err_free_pdev;
-
return pd;
-err_free_pdev:
- platform_device_put(pd);
err_free_mem:
devm_kfree(&pdev->dev, gpio_keys_pdata);
return ERR_PTR(error);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] input: misc: soc_button_array: use platform_device_register_resndata()
From: Enrico Weigelt, metux IT consult @ 2019-08-20 11:16 UTC (permalink / raw)
To: Dmitry Torokhov, Enrico Weigelt, metux IT consult
Cc: linux-kernel, linux-input
In-Reply-To: <20190729172313.GA755@penguin>
On 29.07.19 19:23, Dmitry Torokhov wrote:
Hi,
> I wonder if we should pass &pdev->dev instead of NULL here to form
> proper device hierarchy, now that we have this option.
good point, thanks, fixed in v2.
>> + "gpio-keys",
>> + PLATFORM_DEVID_AUTO,
>> + NULL,
>> + 0,
>> + gpio_keys_pdata,
>> + sizeof(*gpio_keys_pdata));
>> +
>> + if (IS_ERR(pd)) {
>> + dev_err(&pdev->dev, "failed registering gpio-keys: %ld\n", PTR_ERR(pd));
>> goto err_free_mem;
>
> Since you did not assign 'error' value here this goto will result in the
> function returning 0 even if platform_device_register_resndata() failed.
Uh, thanks. IMHO it's even worse: 'error' could be uninitialized.
I'm sending v2 separately.
Thanks for your review.
--mtx
--
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287
^ permalink raw reply
* [PATCH 2/2] drivers: input: mouse: alps: drop unneeded likely() call around IS_ERR()
From: Enrico Weigelt, metux IT consult @ 2019-08-20 10:56 UTC (permalink / raw)
To: linux-kernel; +Cc: pali.rohar, dmitry.torokhov, linux-input, linux-ntfs-dev
In-Reply-To: <1566298572-12409-1-git-send-email-info@metux.net>
From: Enrico Weigelt <info@metux.net>
IS_ERR() already calls unlikely(), so this extra unlikely() call
around IS_ERR() is not needed.
Signed-off-by: Enrico Weigelt <info@metux.net>
---
drivers/input/mouse/alps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 34700ed..ed16614 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -1476,7 +1476,7 @@ static void alps_report_bare_ps2_packet(struct psmouse *psmouse,
/* On V2 devices the DualPoint Stick reports bare packets */
dev = priv->dev2;
dev2 = psmouse->dev;
- } else if (unlikely(IS_ERR_OR_NULL(priv->dev3))) {
+ } else if (IS_ERR_OR_NULL(priv->dev3)) {
/* Register dev3 mouse if we received PS/2 packet first time */
if (!IS_ERR(priv->dev3))
psmouse_queue_work(psmouse, &priv->dev3_register_work,
--
1.9.1
^ permalink raw reply related
* [PATCH 1/2] fs: ntfs: drop unneeded likely() call around IS_ERR()
From: Enrico Weigelt, metux IT consult @ 2019-08-20 10:56 UTC (permalink / raw)
To: linux-kernel; +Cc: pali.rohar, dmitry.torokhov, linux-input, linux-ntfs-dev
From: Enrico Weigelt <info@metux.net>
IS_ERR() already calls unlikely(), so this extra likely() call
around the !IS_ERR() is not needed.
Signed-off-by: Enrico Weigelt <info@metux.net>
---
fs/ntfs/mft.c | 8 ++++----
fs/ntfs/namei.c | 2 +-
fs/ntfs/runlist.c | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/fs/ntfs/mft.c b/fs/ntfs/mft.c
index 20c841a..06f706b 100644
--- a/fs/ntfs/mft.c
+++ b/fs/ntfs/mft.c
@@ -71,7 +71,7 @@ static inline MFT_RECORD *map_mft_record_page(ntfs_inode *ni)
}
/* Read, map, and pin the page. */
page = ntfs_map_page(mft_vi->i_mapping, index);
- if (likely(!IS_ERR(page))) {
+ if (!IS_ERR(page)) {
/* Catch multi sector transfer fixup errors. */
if (likely(ntfs_is_mft_recordp((le32*)(page_address(page) +
ofs)))) {
@@ -154,7 +154,7 @@ MFT_RECORD *map_mft_record(ntfs_inode *ni)
mutex_lock(&ni->mrec_lock);
m = map_mft_record_page(ni);
- if (likely(!IS_ERR(m)))
+ if (!IS_ERR(m))
return m;
mutex_unlock(&ni->mrec_lock);
@@ -271,7 +271,7 @@ MFT_RECORD *map_extent_mft_record(ntfs_inode *base_ni, MFT_REF mref,
m = map_mft_record(ni);
/* map_mft_record() has incremented this on success. */
atomic_dec(&ni->count);
- if (likely(!IS_ERR(m))) {
+ if (!IS_ERR(m)) {
/* Verify the sequence number. */
if (likely(le16_to_cpu(m->sequence_number) == seq_no)) {
ntfs_debug("Done 1.");
@@ -1776,7 +1776,7 @@ static int ntfs_mft_data_extend_allocation_nolock(ntfs_volume *vol)
do {
rl2 = ntfs_cluster_alloc(vol, old_last_vcn, nr, lcn, MFT_ZONE,
true);
- if (likely(!IS_ERR(rl2)))
+ if (!IS_ERR(rl2))
break;
if (PTR_ERR(rl2) != -ENOSPC || nr == min_nr) {
ntfs_error(vol->sb, "Failed to allocate the minimal "
diff --git a/fs/ntfs/namei.c b/fs/ntfs/namei.c
index 2d3cc9e..4e6a44b 100644
--- a/fs/ntfs/namei.c
+++ b/fs/ntfs/namei.c
@@ -115,7 +115,7 @@ static struct dentry *ntfs_lookup(struct inode *dir_ino, struct dentry *dent,
dent_ino = MREF(mref);
ntfs_debug("Found inode 0x%lx. Calling ntfs_iget.", dent_ino);
dent_inode = ntfs_iget(vol->sb, dent_ino);
- if (likely(!IS_ERR(dent_inode))) {
+ if (!IS_ERR(dent_inode)) {
/* Consistency check. */
if (is_bad_inode(dent_inode) || MSEQNO(mref) ==
NTFS_I(dent_inode)->seq_no ||
diff --git a/fs/ntfs/runlist.c b/fs/ntfs/runlist.c
index 508744a..97932fb 100644
--- a/fs/ntfs/runlist.c
+++ b/fs/ntfs/runlist.c
@@ -951,7 +951,7 @@ runlist_element *ntfs_mapping_pairs_decompress(const ntfs_volume *vol,
}
/* Now combine the new and old runlists checking for overlaps. */
old_rl = ntfs_runlists_merge(old_rl, rl);
- if (likely(!IS_ERR(old_rl)))
+ if (!IS_ERR(old_rl))
return old_rl;
ntfs_free(rl);
ntfs_error(vol->sb, "Failed to merge runlists.");
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] HID: wacom: correct misreported EKR ring values
From: Jiri Kosina @ 2019-08-20 8:43 UTC (permalink / raw)
To: Aaron Armstrong Skomra
Cc: linux-input, benjamin.tissoires, ping.cheng, jason.gerecke,
Aaron Armstrong Skomra, # v4 . 3+
In-Reply-To: <1565982054-29236-1-git-send-email-aaron.skomra@wacom.com>
On Fri, 16 Aug 2019, Aaron Armstrong Skomra wrote:
> The EKR ring claims a range of 0 to 71 but actually reports
> values 1 to 72. The ring is used in relative mode so this
> change should not affect users.
>
> Signed-off-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
> Fixes: 72b236d60218f ("HID: wacom: Add support for Express Key Remote.")
> Cc: <stable@vger.kernel.org> # v4.3+
> Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
> Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
Queued for 5.3, thanks Aaron.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v5 15/17] mfd: ioc3: Add driver for SGI IOC3 chip
From: Thomas Bogendoerfer @ 2019-08-20 8:17 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Greg Kroah-Hartman, Jiri Slaby,
Evgeniy Polyakov, linux-doc, linux-kernel, linux-mips,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190820062308.GK3545@piout.net>
On Tue, 20 Aug 2019 08:23:08 +0200
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> On 19/08/2019 18:31:38+0200, Thomas Bogendoerfer wrote:
> > diff --git a/drivers/mfd/ioc3.c b/drivers/mfd/ioc3.c
> > new file mode 100644
> > index 000000000000..5bcb3461a189
> > --- /dev/null
> > +++ b/drivers/mfd/ioc3.c
> > @@ -0,0 +1,586 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * SGI IOC3 multifunction device driver
> > + *
> > + * Copyright (C) 2018, 2019 Thomas Bogendoerfer <tbogendoerfer@suse.de>
> > + *
> > + * Based on work by:
> > + * Stanislaw Skowronek <skylark@unaligned.org>
> > + * Joshua Kinard <kumba@gentoo.org>
> > + * Brent Casavant <bcasavan@sgi.com> - IOC4 master driver
> > + * Pat Gefre <pfg@sgi.com> - IOC3 serial port IRQ demuxer
> > + */
> > +
> > +#include <linux/delay.h>
> > +#include <linux/errno.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/mfd/core.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/platform_data/sgi-w1.h>
> > +#include <linux/rtc/ds1685.h>
> I don't think this include is necessary.
you are right. I'll move it to the patch where IP30 systemboard gets added.
Thanks,
Thomas.
--
SUSE Linux GmbH
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah
HRB 21284 (AG Nürnberg)
^ permalink raw reply
* Re: [PATCH v5 15/17] mfd: ioc3: Add driver for SGI IOC3 chip
From: Alexandre Belloni @ 2019-08-20 6:23 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Greg Kroah-Hartman, Jiri Slaby,
Evgeniy Polyakov, linux-doc, linux-kernel, linux-mips,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190819163144.3478-16-tbogendoerfer@suse.de>
Hi,
On 19/08/2019 18:31:38+0200, Thomas Bogendoerfer wrote:
> diff --git a/drivers/mfd/ioc3.c b/drivers/mfd/ioc3.c
> new file mode 100644
> index 000000000000..5bcb3461a189
> --- /dev/null
> +++ b/drivers/mfd/ioc3.c
> @@ -0,0 +1,586 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * SGI IOC3 multifunction device driver
> + *
> + * Copyright (C) 2018, 2019 Thomas Bogendoerfer <tbogendoerfer@suse.de>
> + *
> + * Based on work by:
> + * Stanislaw Skowronek <skylark@unaligned.org>
> + * Joshua Kinard <kumba@gentoo.org>
> + * Brent Casavant <bcasavan@sgi.com> - IOC4 master driver
> + * Pat Gefre <pfg@sgi.com> - IOC3 serial port IRQ demuxer
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/errno.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/core.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/platform_data/sgi-w1.h>
> +#include <linux/rtc/ds1685.h>
I don't think this include is necessary.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH] Input: hyperv-keyboard: Use in-place iterator API in the channel callback
From: dmitry.torokhov @ 2019-08-20 3:18 UTC (permalink / raw)
To: Dexuan Cui
Cc: linux-input@vger.kernel.org, linux-hyperv@vger.kernel.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley, gregkh@linuxfoundation.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1566270066-27546-1-git-send-email-decui@microsoft.com>
On Tue, Aug 20, 2019 at 03:01:23AM +0000, Dexuan Cui wrote:
> Simplify the ring buffer handling with the in-place API.
>
> Also avoid the dynamic allocation and the memory leak in the channel
> callback function.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
>
> Hi Dmitry, can this patch go through Sasha's hyperv tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git
>
> This is a purely Hyper-V specific change.
Sure, no problem.
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>
> drivers/input/serio/hyperv-keyboard.c | 35 ++++++-----------------------------
> 1 file changed, 6 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c
> index 88ae7c2..e486a8a 100644
> --- a/drivers/input/serio/hyperv-keyboard.c
> +++ b/drivers/input/serio/hyperv-keyboard.c
> @@ -237,40 +237,17 @@ static void hv_kbd_handle_received_packet(struct hv_device *hv_dev,
>
> static void hv_kbd_on_channel_callback(void *context)
> {
> + struct vmpacket_descriptor *desc;
> struct hv_device *hv_dev = context;
> - void *buffer;
> - int bufferlen = 0x100; /* Start with sensible size */
> u32 bytes_recvd;
> u64 req_id;
> - int error;
>
> - buffer = kmalloc(bufferlen, GFP_ATOMIC);
> - if (!buffer)
> - return;
> -
> - while (1) {
> - error = vmbus_recvpacket_raw(hv_dev->channel, buffer, bufferlen,
> - &bytes_recvd, &req_id);
> - switch (error) {
> - case 0:
> - if (bytes_recvd == 0) {
> - kfree(buffer);
> - return;
> - }
> -
> - hv_kbd_handle_received_packet(hv_dev, buffer,
> - bytes_recvd, req_id);
> - break;
> + foreach_vmbus_pkt(desc, hv_dev->channel) {
> + bytes_recvd = desc->len8 * 8;
> + req_id = desc->trans_id;
>
> - case -ENOBUFS:
> - kfree(buffer);
> - /* Handle large packet */
> - bufferlen = bytes_recvd;
> - buffer = kmalloc(bytes_recvd, GFP_ATOMIC);
> - if (!buffer)
> - return;
> - break;
> - }
> + hv_kbd_handle_received_packet(hv_dev, desc, bytes_recvd,
> + req_id);
> }
> }
>
> --
> 1.8.3.1
>
--
Dmitry
^ permalink raw reply
* [PATCH] Input: hyperv-keyboard: Use in-place iterator API in the channel callback
From: Dexuan Cui @ 2019-08-20 3:01 UTC (permalink / raw)
To: dmitry.torokhov@gmail.com, linux-input@vger.kernel.org,
linux-hyperv@vger.kernel.org, Stephen Hemminger, Sasha Levin,
sashal@kernel.org, Haiyang Zhang, KY Srinivasan, Michael Kelley
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
Dexuan Cui
Simplify the ring buffer handling with the in-place API.
Also avoid the dynamic allocation and the memory leak in the channel
callback function.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
Hi Dmitry, can this patch go through Sasha's hyperv tree:
https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git
This is a purely Hyper-V specific change.
drivers/input/serio/hyperv-keyboard.c | 35 ++++++-----------------------------
1 file changed, 6 insertions(+), 29 deletions(-)
diff --git a/drivers/input/serio/hyperv-keyboard.c b/drivers/input/serio/hyperv-keyboard.c
index 88ae7c2..e486a8a 100644
--- a/drivers/input/serio/hyperv-keyboard.c
+++ b/drivers/input/serio/hyperv-keyboard.c
@@ -237,40 +237,17 @@ static void hv_kbd_handle_received_packet(struct hv_device *hv_dev,
static void hv_kbd_on_channel_callback(void *context)
{
+ struct vmpacket_descriptor *desc;
struct hv_device *hv_dev = context;
- void *buffer;
- int bufferlen = 0x100; /* Start with sensible size */
u32 bytes_recvd;
u64 req_id;
- int error;
- buffer = kmalloc(bufferlen, GFP_ATOMIC);
- if (!buffer)
- return;
-
- while (1) {
- error = vmbus_recvpacket_raw(hv_dev->channel, buffer, bufferlen,
- &bytes_recvd, &req_id);
- switch (error) {
- case 0:
- if (bytes_recvd == 0) {
- kfree(buffer);
- return;
- }
-
- hv_kbd_handle_received_packet(hv_dev, buffer,
- bytes_recvd, req_id);
- break;
+ foreach_vmbus_pkt(desc, hv_dev->channel) {
+ bytes_recvd = desc->len8 * 8;
+ req_id = desc->trans_id;
- case -ENOBUFS:
- kfree(buffer);
- /* Handle large packet */
- bufferlen = bytes_recvd;
- buffer = kmalloc(bytes_recvd, GFP_ATOMIC);
- if (!buffer)
- return;
- break;
- }
+ hv_kbd_handle_received_packet(hv_dev, desc, bytes_recvd,
+ req_id);
}
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH] HID: hyperv: Use in-place iterator API in the channel callback
From: Dexuan Cui @ 2019-08-20 2:56 UTC (permalink / raw)
To: jikos@kernel.org, benjamin.tissoires@redhat.com,
linux-input@vger.kernel.org, linux-hyperv@vger.kernel.org,
Stephen Hemminger, Sasha Levin, sashal@kernel.org, Haiyang Zhang,
KY Srinivasan, Michael Kelley
Cc: gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
Dexuan Cui
Simplify the ring buffer handling with the in-place API.
Also avoid the dynamic allocation and the memory leak in the channel
callback function.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
Hi Jiri, Benjamin, can this patch go through Sasha's hyperv tree:
https://git.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git
This is a purely Hyper-V specific change.
drivers/hid/hid-hyperv.c | 56 +++++++++---------------------------------------
1 file changed, 10 insertions(+), 46 deletions(-)
diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
index 7795831..f363163 100644
--- a/drivers/hid/hid-hyperv.c
+++ b/drivers/hid/hid-hyperv.c
@@ -314,60 +314,24 @@ static void mousevsc_on_receive(struct hv_device *device,
static void mousevsc_on_channel_callback(void *context)
{
- const int packet_size = 0x100;
- int ret;
struct hv_device *device = context;
- u32 bytes_recvd;
- u64 req_id;
struct vmpacket_descriptor *desc;
- unsigned char *buffer;
- int bufferlen = packet_size;
-
- buffer = kmalloc(bufferlen, GFP_ATOMIC);
- if (!buffer)
- return;
-
- do {
- ret = vmbus_recvpacket_raw(device->channel, buffer,
- bufferlen, &bytes_recvd, &req_id);
-
- switch (ret) {
- case 0:
- if (bytes_recvd <= 0) {
- kfree(buffer);
- return;
- }
- desc = (struct vmpacket_descriptor *)buffer;
-
- switch (desc->type) {
- case VM_PKT_COMP:
- break;
-
- case VM_PKT_DATA_INBAND:
- mousevsc_on_receive(device, desc);
- break;
-
- default:
- pr_err("unhandled packet type %d, tid %llx len %d\n",
- desc->type, req_id, bytes_recvd);
- break;
- }
+ foreach_vmbus_pkt(desc, device->channel) {
+ switch (desc->type) {
+ case VM_PKT_COMP:
break;
- case -ENOBUFS:
- kfree(buffer);
- /* Handle large packet */
- bufferlen = bytes_recvd;
- buffer = kmalloc(bytes_recvd, GFP_ATOMIC);
-
- if (!buffer)
- return;
+ case VM_PKT_DATA_INBAND:
+ mousevsc_on_receive(device, desc);
+ break;
+ default:
+ pr_err("Unhandled packet type %d, tid %llx len %d\n",
+ desc->type, desc->trans_id, desc->len8 * 8);
break;
}
- } while (1);
-
+ }
}
static int mousevsc_connect_to_vsp(struct hv_device *device)
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH v5 12/17] net: sgi: ioc3-eth: use dma-direct for dma allocations
From: Jakub Kicinski @ 2019-08-20 0:07 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
Jiri Slaby, Evgeniy Polyakov, linux-doc, linux-kernel, linux-mips,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190819163144.3478-13-tbogendoerfer@suse.de>
On Mon, 19 Aug 2019 18:31:35 +0200, Thomas Bogendoerfer wrote:
> @@ -1386,18 +1427,24 @@ static netdev_tx_t ioc3_start_xmit(struct sk_buff *skb, struct net_device *dev)
> unsigned long b2 = (data | 0x3fffUL) + 1UL;
> unsigned long s1 = b2 - data;
> unsigned long s2 = data + len - b2;
> + dma_addr_t d;
>
> desc->cmd = cpu_to_be32(len | ETXD_INTWHENDONE |
> ETXD_B1V | ETXD_B2V | w0);
> desc->bufcnt = cpu_to_be32((s1 << ETXD_B1CNT_SHIFT) |
> (s2 << ETXD_B2CNT_SHIFT));
> - desc->p1 = cpu_to_be64(ioc3_map(skb->data, 1));
> - desc->p2 = cpu_to_be64(ioc3_map((void *)b2, 1));
> + d = dma_map_single(ip->dma_dev, skb->data, s1, DMA_TO_DEVICE);
You'll need to check the DMA address with dma_mapping_error(dev, addr),
otherwise static checkers will get upset.
> + desc->p1 = cpu_to_be64(ioc3_map(d, PCI64_ATTR_PREF));
> + d = dma_map_single(ip->dma_dev, (void *)b2, s1, DMA_TO_DEVICE);
> + desc->p2 = cpu_to_be64(ioc3_map(d, PCI64_ATTR_PREF));
^ permalink raw reply
* Re: [PATCH v5 11/17] net: sgi: ioc3-eth: no need to stop queue set_multicast_list
From: Jakub Kicinski @ 2019-08-20 0:04 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
Jiri Slaby, Evgeniy Polyakov, linux-doc, linux-kernel, linux-mips,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190819163144.3478-12-tbogendoerfer@suse.de>
On Mon, 19 Aug 2019 18:31:34 +0200, Thomas Bogendoerfer wrote:
> netif_stop_queue()/netif_wake_qeue() aren't needed for changing
> multicast filters. Use spinlocks instead for proper protection
> of private struct.
>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
> drivers/net/ethernet/sgi/ioc3-eth.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
> index d862f28887f9..7f85a3bfef14 100644
> --- a/drivers/net/ethernet/sgi/ioc3-eth.c
> +++ b/drivers/net/ethernet/sgi/ioc3-eth.c
> @@ -1542,8 +1542,7 @@ static void ioc3_set_multicast_list(struct net_device *dev)
> struct netdev_hw_addr *ha;
> u64 ehar = 0;
>
> - netif_stop_queue(dev); /* Lock out others. */
> -
> + spin_lock_irq(&ip->ioc3_lock);
What does this lock protect? 🤔 No question that stopping TX queues
makes little sense, but this function is only called from
ndo_set_rx_mode(), so with rtnl_lock held.
I thought it may protect ip->emcr, but that one is accessed with no
locking from the ioc3_timer() -> ioc3_setup_duplex() path..
> if (dev->flags & IFF_PROMISC) { /* Set promiscuous. */
> ip->emcr |= EMCR_PROMISC;
> writel(ip->emcr, ®s->emcr);
> @@ -1572,7 +1571,7 @@ static void ioc3_set_multicast_list(struct net_device *dev)
> writel(ip->ehar_l, ®s->ehar_l);
> }
>
> - netif_wake_queue(dev); /* Let us get going again. */
> + spin_unlock_irq(&ip->ioc3_lock);
> }
>
> module_pci_driver(ioc3_driver);
^ permalink raw reply
* Re: [PATCH v5 10/17] net: sgi: ioc3-eth: rework skb rx handling
From: Jakub Kicinski @ 2019-08-19 23:55 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
Jiri Slaby, Evgeniy Polyakov, linux-doc, linux-kernel, linux-mips,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190819163144.3478-11-tbogendoerfer@suse.de>
On Mon, 19 Aug 2019 18:31:33 +0200, Thomas Bogendoerfer wrote:
> Buffers alloacted by alloc_skb() are already cache aligned so there
> is no need for an extra align done by ioc3_alloc_skb. And instead
> of skb_put/skb_trim simply use one skb_put after frame size is known
> during receive.
>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
> drivers/net/ethernet/sgi/ioc3-eth.c | 50 ++++++++-----------------------------
> 1 file changed, 11 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
> index c875640926d6..d862f28887f9 100644
> --- a/drivers/net/ethernet/sgi/ioc3-eth.c
> +++ b/drivers/net/ethernet/sgi/ioc3-eth.c
> @@ -11,7 +11,6 @@
> *
> * To do:
> *
> - * o Handle allocation failures in ioc3_alloc_skb() more gracefully.
> * o Handle allocation failures in ioc3_init_rings().
> * o Use prefetching for large packets. What is a good lower limit for
> * prefetching?
> @@ -72,6 +71,12 @@
> #define TX_RING_ENTRIES 128
> #define TX_RING_MASK (TX_RING_ENTRIES - 1)
>
> +/* BEWARE: The IOC3 documentation documents the size of rx buffers as
> + * 1644 while it's actually 1664. This one was nasty to track down...
> + */
> +#define RX_OFFSET 10
> +#define RX_BUF_SIZE 1664
> +
> #define ETCSR_FD ((17 << ETCSR_IPGR2_SHIFT) | (11 << ETCSR_IPGR1_SHIFT) | 21)
> #define ETCSR_HD ((21 << ETCSR_IPGR2_SHIFT) | (21 << ETCSR_IPGR1_SHIFT) | 21)
>
> @@ -111,31 +116,6 @@ static void ioc3_init(struct net_device *dev);
> static const char ioc3_str[] = "IOC3 Ethernet";
> static const struct ethtool_ops ioc3_ethtool_ops;
>
> -/* We use this to acquire receive skb's that we can DMA directly into. */
> -
> -#define IOC3_CACHELINE 128UL
Is the cache line on the platform this driver works on 128B?
This looks like a DMA engine alignment requirement, more than an
optimization.
The comment in __alloc_skb() says:
/* We do our best to align skb_shared_info on a separate cache
* line. It usually works because kmalloc(X > SMP_CACHE_BYTES) gives
* aligned memory blocks, unless SLUB/SLAB debug is enabled.
* Both skb->head and skb_shared_info are cache line aligned.
*/
note the "unless".
> -static inline unsigned long aligned_rx_skb_addr(unsigned long addr)
> -{
> - return (~addr + 1) & (IOC3_CACHELINE - 1UL);
> -}
> -
> -static inline struct sk_buff *ioc3_alloc_skb(unsigned long length,
> - unsigned int gfp_mask)
> -{
> - struct sk_buff *skb;
> -
> - skb = alloc_skb(length + IOC3_CACHELINE - 1, gfp_mask);
> - if (likely(skb)) {
> - int offset = aligned_rx_skb_addr((unsigned long)skb->data);
> -
> - if (offset)
> - skb_reserve(skb, offset);
> - }
> -
> - return skb;
> -}
> -
> static inline unsigned long ioc3_map(void *ptr, unsigned long vdev)
> {
> #ifdef CONFIG_SGI_IP27
> @@ -148,12 +128,6 @@ static inline unsigned long ioc3_map(void *ptr, unsigned long vdev)
> #endif
> }
>
> -/* BEWARE: The IOC3 documentation documents the size of rx buffers as
> - * 1644 while it's actually 1664. This one was nasty to track down ...
> - */
> -#define RX_OFFSET 10
> -#define RX_BUF_ALLOC_SIZE (1664 + RX_OFFSET + IOC3_CACHELINE)
> -
> #define IOC3_SIZE 0x100000
>
> static inline u32 mcr_pack(u32 pulse, u32 sample)
> @@ -534,10 +508,10 @@ static inline void ioc3_rx(struct net_device *dev)
> err = be32_to_cpu(rxb->err); /* It's valid ... */
> if (err & ERXBUF_GOODPKT) {
> len = ((w0 >> ERXBUF_BYTECNT_SHIFT) & 0x7ff) - 4;
> - skb_trim(skb, len);
> + skb_put(skb, len);
> skb->protocol = eth_type_trans(skb, dev);
>
> - new_skb = ioc3_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
> + new_skb = alloc_skb(RX_BUF_SIZE, GFP_ATOMIC);
> if (!new_skb) {
> /* Ouch, drop packet and just recycle packet
> * to keep the ring filled.
> @@ -546,6 +520,7 @@ static inline void ioc3_rx(struct net_device *dev)
> new_skb = skb;
> goto next;
> }
> + new_skb->dev = dev;
Assigning dev pointer seems unrelated to the rest of the patch?
> if (likely(dev->features & NETIF_F_RXCSUM))
> ioc3_tcpudp_checksum(skb,
> @@ -556,8 +531,6 @@ static inline void ioc3_rx(struct net_device *dev)
>
> ip->rx_skbs[rx_entry] = NULL; /* Poison */
>
> - /* Because we reserve afterwards. */
> - skb_put(new_skb, (1664 + RX_OFFSET));
> rxb = (struct ioc3_erxbuf *)new_skb->data;
> skb_reserve(new_skb, RX_OFFSET);
>
> @@ -846,16 +819,15 @@ static void ioc3_alloc_rings(struct net_device *dev)
> for (i = 0; i < RX_BUFFS; i++) {
> struct sk_buff *skb;
>
> - skb = ioc3_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
> + skb = alloc_skb(RX_BUF_SIZE, GFP_ATOMIC);
> if (!skb) {
> show_free_areas(0, NULL);
> continue;
> }
> + skb->dev = dev;
>
> ip->rx_skbs[i] = skb;
>
> - /* Because we reserve afterwards. */
> - skb_put(skb, (1664 + RX_OFFSET));
> rxb = (struct ioc3_erxbuf *)skb->data;
> rxr[i] = cpu_to_be64(ioc3_map(rxb, 1));
> skb_reserve(skb, RX_OFFSET);
^ permalink raw reply
* Re: [PATCH v5 09/17] net: sgi: ioc3-eth: use defines for constants dealing with desc rings
From: Jakub Kicinski @ 2019-08-19 23:53 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
Jiri Slaby, Evgeniy Polyakov, linux-doc, linux-kernel, linux-mips,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190819163144.3478-10-tbogendoerfer@suse.de>
On Mon, 19 Aug 2019 18:31:32 +0200, Thomas Bogendoerfer wrote:
> Descriptor ring sizes of the IOC3 are more or less fixed size. To
> make clearer where there is a relation to ring sizes use defines.
>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply
* Re: [PATCH v5 00/17] Use MFD framework for SGI IOC3 drivers
From: Jakub Kicinski @ 2019-08-19 23:51 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
Jiri Slaby, Evgeniy Polyakov, linux-doc, linux-kernel, linux-mips,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190819163144.3478-1-tbogendoerfer@suse.de>
On Mon, 19 Aug 2019 18:31:23 +0200, Thomas Bogendoerfer wrote:
> - requested by Jakub I've splitted ioc3 ethernet driver changes into
> more steps to make the transition more visible;
Thanks a lot for doing that!
^ permalink raw reply
* Re: [PATCH] HID: wacom: correct misreported EKR ring values
From: Sasha Levin @ 2019-08-19 19:03 UTC (permalink / raw)
To: Aaron Armstrong Skomra
Cc: open list:HID CORE LAYER, Jiri Kosina, benjamin.tissoires,
Cheng, Ping, Jason Gerecke, Aaron Armstrong Skomra, # v4 . 3+
In-Reply-To: <CAEoswT3mvABD7T_0WkwrAOe1PHO62jZ63tUb2UBQLm_Tqu-guw@mail.gmail.com>
On Mon, Aug 19, 2019 at 10:13:31AM -0700, Aaron Armstrong Skomra wrote:
>On Mon, Aug 19, 2019 at 10:02 AM Sasha Levin <sashal@kernel.org> wrote:
>
>> On Mon, Aug 19, 2019 at 09:41:54AM -0700, Aaron Armstrong Skomra wrote:
>> >The EKR ring claims a range of 0 to 71 but actually reports
>> >values 1 to 72. The ring is used in relative mode so this
>> >change should not affect users.
>> >
>> >Signed-off-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
>> >Fixes: 72b236d60218f ("HID: wacom: Add support for Express Key Remote.")
>> >Cc: <stable@vger.kernel.org> # v4.3+
>> >Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
>> >Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
>> >---
>> >Patch specifically targeted to v4.9.189
>>
>> Is this not a problem upstream as well? Why not?
>>
>> If it is, this patch will need to go upstream first, and then it'll get
>> to stable branches from there.
>>
>> Hi Sasha,
>I neglected my "--in-reply-to" in git send-email, I will resend. My
>apologies.
Ah, I see what happened. Looks good now, thanks!
^ permalink raw reply
* [PATCH] HID: wacom: correct misreported EKR ring values
From: Aaron Armstrong Skomra @ 2019-08-19 17:20 UTC (permalink / raw)
To: sashal, linux-input, jikos, benjamin.tissoires, ping.cheng,
jason.gerecke
Cc: Aaron Armstrong Skomra, # v4 . 3+
In-Reply-To: <20190817181109.33F1B2173B@mail.kernel.org>
The EKR ring claims a range of 0 to 71 but actually reports
values 1 to 72. The ring is used in relative mode so this
change should not affect users.
Signed-off-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
Fixes: 72b236d60218f ("HID: wacom: Add support for Express Key Remote.")
Cc: <stable@vger.kernel.org> # v4.3+
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
---
My first attempt at sending this
patch specifically targeted to v4.9.189
neglected the "in-reply-to" in git send-email. My apologies.
drivers/hid/wacom_wac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 6c3bf8846b52..949761dd29ca 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -819,7 +819,7 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
input_report_key(input, BTN_BASE2, (data[11] & 0x02));
if (data[12] & 0x80)
- input_report_abs(input, ABS_WHEEL, (data[12] & 0x7f));
+ input_report_abs(input, ABS_WHEEL, (data[12] & 0x7f) - 1);
else
input_report_abs(input, ABS_WHEEL, 0);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] HID: wacom: correct misreported EKR ring values
From: Sasha Levin @ 2019-08-19 17:02 UTC (permalink / raw)
To: Aaron Armstrong Skomra
Cc: linux-input, jikos, benjamin.tissoires, ping.cheng, jason.gerecke,
Aaron Armstrong Skomra, # v4 . 3+
In-Reply-To: <1566232914-9919-1-git-send-email-aaron.skomra@wacom.com>
On Mon, Aug 19, 2019 at 09:41:54AM -0700, Aaron Armstrong Skomra wrote:
>The EKR ring claims a range of 0 to 71 but actually reports
>values 1 to 72. The ring is used in relative mode so this
>change should not affect users.
>
>Signed-off-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
>Fixes: 72b236d60218f ("HID: wacom: Add support for Express Key Remote.")
>Cc: <stable@vger.kernel.org> # v4.3+
>Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
>Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
>---
>Patch specifically targeted to v4.9.189
Is this not a problem upstream as well? Why not?
If it is, this patch will need to go upstream first, and then it'll get
to stable branches from there.
^ permalink raw reply
* [PATCH] HID: wacom: correct misreported EKR ring values
From: Aaron Armstrong Skomra @ 2019-08-19 16:41 UTC (permalink / raw)
To: sashal, linux-input, jikos, benjamin.tissoires, ping.cheng,
jason.gerecke
Cc: Aaron Armstrong Skomra, # v4 . 3+
The EKR ring claims a range of 0 to 71 but actually reports
values 1 to 72. The ring is used in relative mode so this
change should not affect users.
Signed-off-by: Aaron Armstrong Skomra <aaron.skomra@wacom.com>
Fixes: 72b236d60218f ("HID: wacom: Add support for Express Key Remote.")
Cc: <stable@vger.kernel.org> # v4.3+
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
---
Patch specifically targeted to v4.9.189
drivers/hid/wacom_wac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 6c3bf8846b52..949761dd29ca 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -819,7 +819,7 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
input_report_key(input, BTN_BASE2, (data[11] & 0x02));
if (data[12] & 0x80)
- input_report_abs(input, ABS_WHEEL, (data[12] & 0x7f));
+ input_report_abs(input, ABS_WHEEL, (data[12] & 0x7f) - 1);
else
input_report_abs(input, ABS_WHEEL, 0);
--
2.17.1
^ permalink raw reply related
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