* Re: [PATCH v2] HID: hiddev: fix potential Spectre v1
From: Jiri Kosina @ 2018-10-26 15:07 UTC (permalink / raw)
To: Breno Leitao; +Cc: linux-usb, linux-input, gustavo, stable
In-Reply-To: <1539979293-16102-1-git-send-email-leitao@debian.org>
On Fri, 19 Oct 2018, Breno Leitao wrote:
> uref->usage_index can be indirectly controlled by userspace, hence leading
> to a potential exploitation of the Spectre variant 1 vulnerability.
>
> This field is used as an array index by the hiddev_ioctl_usage() function,
> when 'cmd' is either HIDIOCGCOLLECTIONINDEX, HIDIOCGUSAGES or
> HIDIOCSUSAGES.
>
> For cmd == HIDIOCGCOLLECTIONINDEX case, uref->usage_index is compared to
> field->maxusage and then used as an index to dereference field->usage
> array. The same thing happens to the cmd == HIDIOC{G,S}USAGES cases, where
> uref->usage_index is checked against an array maximum value and then it is
> used as an index in an array.
>
> This is a summary of the HIDIOCGCOLLECTIONINDEX case, which matches the
> traditional Spectre V1 first load:
>
> copy_from_user(uref, user_arg, sizeof(*uref))
> if (uref->usage_index >= field->maxusage)
> goto inval;
> i = field->usage[uref->usage_index].collection_index;
> return i;
>
> This patch fixes this by sanitizing field uref->usage_index before using it
> to index field->usage (HIDIOCGCOLLECTIONINDEX) or field->value in
> HIDIOC{G,S}USAGES arrays, thus, avoiding speculation in the first load.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> Cc: <stable@vger.kernel.org>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: i2c-hid: Add a small delay after sleep command for Raydium touchpanel
From: Jiri Kosina @ 2018-10-26 15:09 UTC (permalink / raw)
To: Kai-Heng Feng; +Cc: benjamin.tissoires, hdegoede, linux-input, linux-kernel
In-Reply-To: <20181005044629.26286-1-kai.heng.feng@canonical.com>
On Fri, 5 Oct 2018, Kai-Heng Feng wrote:
> Raydium touchpanel (2386:4B33) sometimes does not work in desktop session
> although it works in display manager.
>
> During user logging, the display manager exits, close the HID device,
> then the device gets runtime suspended and powered off. The desktop
> session begins shortly after, opens the HID device, then the device gets
> runtime resumed and powered on.
>
> If the trasition from display manager to desktop sesesion is fast, the
> touchpanel cannot switch from powered off to powered on in short
> timeframe. So add a small delay to workaround the issue.
>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v2:
> - Use quirk to only match affected touchpanel
> - Only delay the next power on if the time hasn't elapsed
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] Revert "HID: add NOGET quirk for Eaton Ellipse MAX UPS"
From: Jiri Kosina @ 2018-10-26 15:11 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Laurent Bigonville, linux-input, linux-kernel
In-Reply-To: <20181017070153.465-1-benjamin.tissoires@redhat.com>
On Wed, 17 Oct 2018, Benjamin Tissoires wrote:
> This reverts commit 67ddbb3e6568fb1820b2cc45b00c50702b114801.
>
> 67ddbb3e656 ("HID: add NOGET quirk for Eaton Ellipse MAX UPS") was reported
> by Laurent Bigonville. It turns out that a later model Laurent got
> doesn't need the quirk after all.
>
> My take is that Eaton upgraded their firmwares, so we don't need it
> anymore.
>
> The old model was from 2012, so better make sure the new line works
> properly by removing the quirk. This allows upower to actually fetch
> the current data.
>
> Reported-by: Laurent Bigonville <bigon@bigon.be>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Ok, fingers crossed. Applied,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH 0/3] HID: debug: fix the ring buffer implementation
From: Jiri Kosina @ 2018-10-26 15:25 UTC (permalink / raw)
To: Vladis Dronov; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20181003171936.11271-1-vdronov@redhat.com>
On Wed, 3 Oct 2018, Vladis Dronov wrote:
> This patchset is fixing some aspects of the ring buffer implementation in
> drivers/hid/hid-debug.c. This implementation has certain problem points:
>
> - it may stuck in an infinite loop
> - it may return corrupted data
> - a reader and a writer are not protected by spinlocks, which can lead to
> the corrupted data
>
> The suggested patchset is a new ring buffer implementation which overwrites
> the oldest data in case of an overflow. One can verify the suggested ring
> buffer implementation by fuzzing it with modified kernel and fuzzer-reader
> at: https://gist.github.com/nefigtut/33d56e3870b67493cc867344aed2a062
Vladis,
thanks for cleaning it up. I actually like your rewrite quite a lot.
Quick question -- how well was it tested in which scenarios?
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: alps: allow incoming reports when only the trackstick is opened
From: Jiri Kosina @ 2018-10-26 15:27 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Masaki Ota, linux-input, linux-kernel
In-Reply-To: <20181012140525.24799-1-benjamin.tissoires@redhat.com>
On Fri, 12 Oct 2018, Benjamin Tissoires wrote:
> If userspace only reads the trackstick node, and no one is listening to
> the touchpad nor the hidraw node then, the device is not powered on.
>
> Add open/close callbacks to allow users to disable the touchpad in Gnome
> while keeping the trackstick active.
>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1559632
> Link: https://gitlab.gnome.org/GNOME/mutter/issues/128
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v4 4/8] regulator: stpmic1: add stpmic1 regulator driver
From: Mark Brown @ 2018-10-26 20:33 UTC (permalink / raw)
To: Pascal PAILLET-LME
Cc: dmitry.torokhov@gmail.com, robh+dt@kernel.org,
mark.rutland@arm.com, lee.jones@linaro.org, lgirdwood@gmail.com,
wim@linux-watchdog.org, linux@roeck-us.net,
linux-input@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-watchdog@vger.kernel.org,
benjamin.gaignard@linaro.org, eballetbo@gmail.com
In-Reply-To: <5BD1C3BB.6060500@st.com>
[-- Attachment #1: Type: text/plain, Size: 573 bytes --]
On Thu, Oct 25, 2018 at 01:23:08PM +0000, Pascal PAILLET-LME wrote:
> I have reworked the code so that we don't touch any more to the init_data.
> the new loop to register the regulators is below:
>
> for (i = 0; i < ARRAY_SIZE(stpmic1_regulator_cfgs); i++) {
> ret = stpmic1_regulator_register(pdev, i, &stpmic1_matches[i],
> regul);
> if (ret < 0)
> return ret;
> regul++;
> }
>
> Each regulator is registered, even is it is not described in the
> device-tree.
Looks good.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [git pull] Input updates for v4.20-rc0
From: Dmitry Torokhov @ 2018-10-26 23:49 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-kernel, linux-input
Hi Linus,
Please pull from:
git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
to receive updates for the input subsystem. Just random driver fixups,
nothing exiting.
Changelog:
---------
Brian Masney (1):
Input: pwm-vibrator - correct pwms in DT binding example
Dmitry Torokhov (1):
Input: synaptics - avoid using uninitialized variable when probing
George G. Davis (1):
Input: atmel_mxt_ts - fix multiple <linux/property.h> includes
Gustavo A. R. Silva (3):
Input: cyapa - mark expected switch fall-throughs
Input: atmel_mxt_ts - mark expected switch fall-through
Input: xen-kbdfront - mark expected switch fall-through
Hans de Goede (1):
Input: of_touchscreen - add support for touchscreen-min-x|y
Julian Sax (1):
Input: silead - try firmware reload after unsuccessful resume
Linus Walleij (1):
Input: Fix DIR-685 touchkeys MAINTAINERS entry
Martin Kepplinger (1):
Input: st1232 - set INPUT_PROP_DIRECT property
Randy Dunlap (1):
Input: wm97xx-ts - fix exit path
Rob Herring (2):
Input: sun4i-lradc - convert to using %pOFn instead of device_node.name
Input: xilinx_ps2 - convert to using %pOFn instead of device_node.name
Stephen Boyd (1):
Input: elants_i2c - use DMA safe i2c when possible
Diffstat:
--------
.../devicetree/bindings/input/pwm-vibrator.txt | 4 +--
.../bindings/input/touchscreen/touchscreen.txt | 6 ++--
MAINTAINERS | 2 +-
drivers/input/keyboard/sun4i-lradc-keys.c | 6 ++--
drivers/input/misc/xen-kbdfront.c | 2 +-
drivers/input/mouse/cyapa_gen3.c | 4 +--
drivers/input/mouse/synaptics.c | 4 +--
drivers/input/serio/xilinx_ps2.c | 2 +-
drivers/input/touchscreen/atmel_mxt_ts.c | 3 +-
drivers/input/touchscreen/elants_i2c.c | 7 +++--
drivers/input/touchscreen/of_touchscreen.c | 36 +++++++++++++++++-----
drivers/input/touchscreen/silead.c | 13 ++++++++
drivers/input/touchscreen/st1232.c | 1 +
drivers/input/touchscreen/wm97xx-core.c | 3 +-
14 files changed, 64 insertions(+), 29 deletions(-)
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] HID: moving to group maintainership model
From: Jiri Kosina @ 2018-10-27 12:19 UTC (permalink / raw)
To: linux-input, linux-kernel; +Cc: Benjamin Tissoires, Stephen Rothwell
From: Jiri Kosina <jkosina@suse.cz>
Benjamin and myself will from now on be sharing maintainership
responsibilities for hid.git.
Update maintainers to reflect that change, and also move a git repository
to shared space at kernel.org.
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
Stephen, could you please update the hid.git URL for linux-next as below?
Thanks.
MAINTAINERS | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index c0c4c5003998..6cdca65bb0a6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6588,9 +6588,9 @@ F: arch/*/include/asm/suspend*.h
HID CORE LAYER
M: Jiri Kosina <jikos@kernel.org>
-R: Benjamin Tissoires <benjamin.tissoires@redhat.com>
+M: Benjamin Tissoires <benjamin.tissoires@redhat.com>
L: linux-input@vger.kernel.org
-T: git git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
S: Maintained
F: drivers/hid/
F: include/linux/hid*
--
Jiri Kosina
SUSE Labs
^ permalink raw reply related
* [PATCH v2] HID: moving to group maintainership model
From: Jiri Kosina @ 2018-10-27 12:25 UTC (permalink / raw)
To: linux-input, linux-kernel; +Cc: Benjamin Tissoires, Stephen Rothwell
In-Reply-To: <nycvar.YFH.7.76.1810271417230.23511@cbobk.fhfr.pm>
From: Jiri Kosina <jkosina@suse.cz>
Date: Sat, 27 Oct 2018 14:16:13 +0200
Subject: [PATCH] HID: moving to group maintainership model
Benjamin and myself will from now on be sharing maintainership
responsibilities for hid.git.
Update maintainers to reflect that change, and also move a git
repository to shared space at kernel.org.
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
Stephen, could you please update the hid.git URL for linux-next as below?
Thanks.
v1->v2: Update all occurences in MAINTAINERS
MAINTAINERS | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index c0c4c5003998..4715cb8d9584 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6588,9 +6588,9 @@ F: arch/*/include/asm/suspend*.h
HID CORE LAYER
M: Jiri Kosina <jikos@kernel.org>
-R: Benjamin Tissoires <benjamin.tissoires@redhat.com>
+M: Benjamin Tissoires <benjamin.tissoires@redhat.com>
L: linux-input@vger.kernel.org
-T: git git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
S: Maintained
F: drivers/hid/
F: include/linux/hid*
@@ -15254,9 +15254,9 @@ F: include/linux/usb/gadget*
USB HID/HIDBP DRIVERS (USB KEYBOARDS, MICE, REMOTE CONTROLS, ...)
M: Jiri Kosina <jikos@kernel.org>
-R: Benjamin Tissoires <benjamin.tissoires@redhat.com>
+M: Benjamin Tissoires <benjamin.tissoires@redhat.com>
L: linux-usb@vger.kernel.org
-T: git git://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git
+T: git git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
S: Maintained
F: Documentation/hid/hiddev.txt
F: drivers/hid/usbhid/
--
Jiri Kosina
SUSE Labs
^ permalink raw reply related
* Oops in current tree in i2c
From: Linus Torvalds @ 2018-10-27 16:08 UTC (permalink / raw)
To: Jiri Kosina, Julian Sax, Benjamin Tissoires, Hans de Goede
Cc: linux-input, Linux Kernel Mailing List
Julian, Jiri,
On my laptop I'm getting a kernel page fault with the current git
tree, and I'm tentatively blaming commit
9ee3e06610fd ("HID: i2c-hid: override HID descriptors for certain devices")
but that's simply because it's the only thing that seems to touch this
particular area in this merge window.
The oops looks like this:
BUG: unable to handle kernel paging request at 000000007a25d598
PGD 0 P4D 0
Oops: 0000 [#1] SMP PTI
CPU: 1 PID: 888 Comm: systemd-udevd Not tainted 4.19.0-07715-g345671ea0f92 #4
Hardware name: Dell Inc. XPS 13 9350/09JHRY, BIOS 1.7.0 01/16/2018
RIP: 0010:strstr+0x19/0x70
where the code disassembly (and the register contents) shows that the
wild pointer is the first argument to "strstr()", which just has a
bogus value that is not a valid kernel pointer (RDI: 000000007a25d598
- which is obviously also the address of the page fault)
The call trace is:
dmi_matches+0x55/0xc0
dmi_first_match+0x26/0x40
i2c_hid_get_dmi_i2c_hid_desc_override+0x16/0x40 [i2c_hid]
i2c_hid_probe+0x28c/0x760 [i2c_hid]
i2c_device_probe+0x1e7/0x260
really_probe+0xf8/0x3e0
driver_probe_device+0x10f/0x120
bus_for_each_drv+0x66/0xb0
__device_attach+0xd9/0x150
bus_probe_device+0x8a/0xa0
device_add+0x48e/0x660
i2c_new_device+0x162/0x350
which is why I suspect that new i2c_hid_get_dmi_hid_report_desc_override() code.
I *think* the problem is that the i2c_hid_dmi_desc_override_table[]
isn't terminated by a NULL entry, and I will test that next.
What makes me *very* unhappy about this is that if I'm right, I think
it means that code was literally not tested at all by anybody who
didn't have one of the entries in that list.
Linus
^ permalink raw reply
* Re: Oops in current tree in i2c
From: Linus Torvalds @ 2018-10-27 16:19 UTC (permalink / raw)
To: Jiri Kosina, Julian Sax, Benjamin Tissoires, Hans de Goede
Cc: linux-input, Linux Kernel Mailing List
In-Reply-To: <CAHk-=wiVH6SdC2YutcDJ8kiAYUkO-ei3Wdjp2TXcbWas7LJrSg@mail.gmail.com>
On Sat, Oct 27, 2018 at 9:08 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> I *think* the problem is that the i2c_hid_dmi_desc_override_table[]
> isn't terminated by a NULL entry, and I will test that next.
Confirmed. That makes my laptop boot cleanly.
See commit b59dfdaef173 ("i2c-hid: properly terminate
i2c_hid_dmi_desc_override_table[] array")
Linus
^ permalink raw reply
* Re: Oops in current tree in i2c
From: Jiri Kosina @ 2018-10-27 16:20 UTC (permalink / raw)
To: Linus Torvalds
Cc: Julian Sax, Benjamin Tissoires, Hans de Goede, linux-input,
Linux Kernel Mailing List
In-Reply-To: <CAHk-=wiVH6SdC2YutcDJ8kiAYUkO-ei3Wdjp2TXcbWas7LJrSg@mail.gmail.com>
On Sat, 27 Oct 2018, Linus Torvalds wrote:
> I *think* the problem is that the i2c_hid_dmi_desc_override_table[]
> isn't terminated by a NULL entry, and I will test that next.
Hm, that almost certainly is indeed the issue, thanks a lot for reporting
it.
> What makes me *very* unhappy about this is that if I'm right, I think it
> means that code was literally not tested at all by anybody who didn't
> have one of the entries in that list.
Honestly, my autotesting of HID tree is running on HW that doesn't have
i2c transport at all, only USB a Bluetooth. Something to improve I guess;
will fix that next week.
Benjamin, do you have something for that in place already?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [git pull] Input updates for v4.20-rc0
From: Linus Torvalds @ 2018-10-27 17:21 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: Linux Kernel Mailing List, linux-input
In-Reply-To: <20181026234917.GA196749@dtor-ws>
On Fri, Oct 26, 2018 at 4:49 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> updates for the input subsystem. Just random driver fixups,
> nothing exiting.
Pulled,
Linus
^ permalink raw reply
* Another HID problem this merge window..
From: Linus Torvalds @ 2018-10-27 18:13 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires; +Cc: linux-input, Linux Kernel Mailing List
Ok, so this is a much smaller issue than the i2c one that cause boot
problems, but it's annoying.
We do *not* enable new random drivers by default. And we most
*definitely* don't do it when they are odd-ball ones that most people
have never heard of.
Yet the new "BigBen Interactive" driver that was added this merge
window did exactly that.
Just don't do it.
Yes, yes, every developer always thinks that _their_ driver is so
special and so magically important that it should be enabled by
default. But no. When we have thousands of drivers, we don't randomly
pick one new driver to be enabled by default just because some
developer thinks it is special. It's not.
So the
default !EXPERT
was completely wrong in commit 256a90ed9e46 ("HID: hid-bigbenff:
driver for BigBen Interactive PS3OFMINIPAD gamepad"). Please don't do
things like this.
Linus
^ permalink raw reply
* Re: Another HID problem this merge window..
From: Joe Perches @ 2018-10-27 19:36 UTC (permalink / raw)
To: Linus Torvalds, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, Linux Kernel Mailing List
In-Reply-To: <CAHk-=wiZ24JuVehJ5sEC0UG1Gk2nvB363wO02RRsR1oEht6R9Q@mail.gmail.com>
On Sat, 2018-10-27 at 11:13 -0700, Linus Torvalds wrote:
> Ok, so this is a much smaller issue than the i2c one that cause boot
> problems, but it's annoying.
>
> We do *not* enable new random drivers by default. And we most
> *definitely* don't do it when they are odd-ball ones that most people
> have never heard of.
>
> Yet the new "BigBen Interactive" driver that was added this merge
> window did exactly that.
In fairness, it seems many of the HID drivers do exactly that
and this could have been a "copy from example" addition.
$ git grep -P -i -B2 'default\s+\!EXPERT' -- drivers/hid/Kconfig
drivers/hid/Kconfig- tristate "A4 tech mice"
drivers/hid/Kconfig- depends on HID
drivers/hid/Kconfig: default !EXPERT
--
drivers/hid/Kconfig- tristate "Apple {i,Power,Mac}Books"
drivers/hid/Kconfig- depends on HID
drivers/hid/Kconfig: default !EXPERT
--
drivers/hid/Kconfig- tristate "Belkin Flip KVM and Wireless keyboard"
drivers/hid/Kconfig- depends on HID
drivers/hid/Kconfig: default !EXPERT
--
drivers/hid/Kconfig- tristate "Cherry Cymotion keyboard"
drivers/hid/Kconfig- depends on HID
drivers/hid/Kconfig: default !EXPERT
--
drivers/hid/Kconfig- tristate "Chicony devices"
drivers/hid/Kconfig- depends on HID
drivers/hid/Kconfig: default !EXPERT
--
drivers/hid/Kconfig- tristate "Cypress mouse and barcode readers"
drivers/hid/Kconfig- depends on HID
drivers/hid/Kconfig: default !EXPERT
--
drivers/hid/Kconfig- tristate "Ezkey BTC 8193 keyboard"
drivers/hid/Kconfig- depends on HID
drivers/hid/Kconfig: default !EXPERT
--
drivers/hid/Kconfig- tristate "ITE devices"
drivers/hid/Kconfig- depends on HID
drivers/hid/Kconfig: default !EXPERT
--
drivers/hid/Kconfig- tristate "Kensington Slimblade Trackball"
drivers/hid/Kconfig- depends on HID
drivers/hid/Kconfig: default !EXPERT
--
drivers/hid/Kconfig- tristate "Logitech devices"
drivers/hid/Kconfig- depends on HID
drivers/hid/Kconfig: default !EXPERT
^ permalink raw reply
* Re: Another HID problem this merge window..
From: Linus Torvalds @ 2018-10-27 20:17 UTC (permalink / raw)
To: joe; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input,
Linux Kernel Mailing List
In-Reply-To: <c6b7e5b26883c53db35454e9b0ffb44898517228.camel@perches.com>
On Sat, Oct 27, 2018 at 12:36 PM Joe Perches <joe@perches.com> wrote:
>
> In fairness, it seems many of the HID drivers do exactly that
> and this could have been a "copy from example" addition.
Interesting, and I think you're right.
I wonder why I haven't noticed this before. Some of those might be
hidden by other dependencies - what I do is to just check how "make
oldconfig" changes my normal fairly minimal configuration, and then
complain when I notice that somebody enables some odd new hardware.
Anyway, those other "!EXPERT" defaults look pretty questionable too.
HID itself beind default y and the HID_GENERIC driver defaulting on
seems eminently sane. And HID_LOGITECH might fall under both "very
common" and "enables other config options".
But even then, the !EXPERT seems an odd choice. Why not just 'y' for
the truly common cases?
I wonder if there is some truly old historical legacy there, ie the
old PC keyboard support would have been configurable out only for
expert users to avoid errors, and maybe the HID Kconfig file started
getting ideas from that...
Linus
^ permalink raw reply
* Re: Another HID problem this merge window..
From: Jiri Kosina @ 2018-10-27 21:45 UTC (permalink / raw)
To: Linus Torvalds
Cc: joe, Benjamin Tissoires, linux-input, Linux Kernel Mailing List
In-Reply-To: <CAHk-=wgTMd_orbgEmjtKdpPJdnFcho_-66_R3PBr7joxFOxqsw@mail.gmail.com>
On Sat, 27 Oct 2018, Linus Torvalds wrote:
> I wonder if there is some truly old historical legacy there, ie the old
> PC keyboard support would have been configurable out only for expert
> users to avoid errors, and maybe the HID Kconfig file started getting
> ideas from that...
This really goes waaay back to times when we extracted all the quirks from
the generic driver (which became unmaintainable exactly because quirks
being sprinkled left and right) into specialized drivers, but didn't want
to cause too many user surprises that all of a sudden their configuration
regressed when it comes to hardware support.
We've had exactly this discussion multiple times before, see for example
https://lkml.org/lkml/2010/5/20/227
So I guess there is no need for replaying it, I think we're in complete
agreement.
That being said, benff Kconfig setting definitely escaped attention. That
should never ever have been set to default y, I take blame for not
noticing that while applying the patch.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: moving to group maintainership model
From: Stephen Rothwell @ 2018-10-28 1:15 UTC (permalink / raw)
To: Jiri Kosina; +Cc: linux-input, linux-kernel, Benjamin Tissoires
In-Reply-To: <nycvar.YFH.7.76.1810271417230.23511@cbobk.fhfr.pm>
[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]
Hi Jiri,
On Sat, 27 Oct 2018 14:19:02 +0200 (CEST) Jiri Kosina <jikos@kernel.org> wrote:
>
> Stephen, could you please update the hid.git URL for linux-next as below?
> Thanks.
Done, also added Benjamin as a contact.
[for Benjamin's benefit ...]
Thanks for adding your subsystem tree as a participant of linux-next. As
you may know, this is not a judgement of your code. The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window.
You will need to ensure that the patches/commits in your tree/series have
been:
* submitted under GPL v2 (or later) and include the Contributor's
Signed-off-by,
* posted to the relevant mailing list,
* reviewed by you (or another maintainer of your subsystem tree),
* successfully unit tested, and
* destined for the current or next Linux merge window.
Basically, this should be just what you would send to Linus (or ask him
to fetch). It is allowed to be rebased if you deem it necessary.
--
Cheers,
Stephen Rothwell
sfr@canb.auug.org.au
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: Oops in current tree in i2c
From: Hans de Goede @ 2018-10-28 10:44 UTC (permalink / raw)
To: Linus Torvalds, Jiri Kosina, Julian Sax, Benjamin Tissoires
Cc: linux-input, Linux Kernel Mailing List
In-Reply-To: <CAHk-=wiVH6SdC2YutcDJ8kiAYUkO-ei3Wdjp2TXcbWas7LJrSg@mail.gmail.com>
Hi Linus,
On 27-10-18 18:08, Linus Torvalds wrote:
> Julian, Jiri,
> On my laptop I'm getting a kernel page fault with the current git
> tree, and I'm tentatively blaming commit
>
> 9ee3e06610fd ("HID: i2c-hid: override HID descriptors for certain devices")
>
> but that's simply because it's the only thing that seems to touch this
> particular area in this merge window.
>
> The oops looks like this:
>
> BUG: unable to handle kernel paging request at 000000007a25d598
> PGD 0 P4D 0
> Oops: 0000 [#1] SMP PTI
> CPU: 1 PID: 888 Comm: systemd-udevd Not tainted 4.19.0-07715-g345671ea0f92 #4
> Hardware name: Dell Inc. XPS 13 9350/09JHRY, BIOS 1.7.0 01/16/2018
> RIP: 0010:strstr+0x19/0x70
>
> where the code disassembly (and the register contents) shows that the
> wild pointer is the first argument to "strstr()", which just has a
> bogus value that is not a valid kernel pointer (RDI: 000000007a25d598
> - which is obviously also the address of the page fault)
>
> The call trace is:
>
> dmi_matches+0x55/0xc0
> dmi_first_match+0x26/0x40
> i2c_hid_get_dmi_i2c_hid_desc_override+0x16/0x40 [i2c_hid]
> i2c_hid_probe+0x28c/0x760 [i2c_hid]
> i2c_device_probe+0x1e7/0x260
> really_probe+0xf8/0x3e0
> driver_probe_device+0x10f/0x120
> bus_for_each_drv+0x66/0xb0
> __device_attach+0xd9/0x150
> bus_probe_device+0x8a/0xa0
> device_add+0x48e/0x660
> i2c_new_device+0x162/0x350
>
> which is why I suspect that new i2c_hid_get_dmi_hid_report_desc_override() code.
>
> I *think* the problem is that the i2c_hid_dmi_desc_override_table[]
> isn't terminated by a NULL entry, and I will test that next.
Yes that likely is the problem. I already had a bug report from one of the
Manjaro maintainers who was cherry picking this into the Manjaro kernel.
So I ran some tests on a laptop of mine which does use i2c-hid but I
failed to reproduce the issue, so we both (me and the Manjaro maintainer)
both assumed something went wrong with the backport.
Both of us seem to have overlooked the missing terminating entry,
as well as other people involved in the patch.
> What makes me *very* unhappy about this is that if I'm right, I think
> it means that code was literally not tested at all by anybody who
> didn't have one of the entries in that list.
That is not true, I've hit one of these unterminated dmi lists
issues before and it depends on what get put in mem directly
after the list by the linker, bugs caused by this do not always
reproduce unfortunately.
And as mentioned I have tested the patch on a machine with an i2c-hid
touchpad, which is not on the list and I did not hit this problem.
Anyways this is fixed now, thank you for catching this.
Regards,
Hans
^ permalink raw reply
* Re: [regression, bisected] Keyboard not responding after resuming from suspend/hibernate
From: Numan Demirdöğen @ 2018-10-28 19:06 UTC (permalink / raw)
To: Pavel Machek
Cc: jason.low2, Waiman.Long, paulmck, tglx, dmitry.torokhov, peterz,
mingo, linux-kernel, linux-input
In-Reply-To: <20181025074903.GA9323@amd>
[-- Attachment #1: Type: text/plain, Size: 9804 bytes --]
Thu, 25 Oct 2018 09:49:03 +0200 tarihinde
Pavel Machek <pavel@ucw.cz> yazdı:
> Hi!
>
> Here's problem bisected down to:
>
> commit 9d659ae14b545c4296e812c70493bfdc999b5c1c
> Author: Peter Zijlstra <peterz@infradead.org>
> Date: Tue Aug 23 14:40:16 2016 +0200
>
> locking/mutex: Add lock handoff to avoid starvation
>
> Implement lock handoff to avoid lock starvation.
>
> Numan, I assume revert of that patch on the 4.18 kernel still makes it
> work?
>
Unfortunately, I could not revert
9d659ae14b545c4296e812c70493bfdc999b5c1c on kernels from 4.18.16 to
4.10-rc1 because there were too much conflicts, which I could not solve
as an "average Joe". I tried a3ea3d9b865c2a8f7fe455c7fa26db4b6fd066e3
which is parent of 9d659ae14b545c4296e812c70493bfdc999b5c1c and
succeeded to compile kernel.
git checkout a3ea3d9b865c2a8f7fe455c7fa26db4b6fd066e3
Then, I compiled kernel and rebooted with it. I tried a couples of
times suspending and resuming, all of the time keyboard worked as
expected.
uname -a
Linux ubuntu 4.9.0-rc2reverted+ #1 SMP Sun Oct 28 20:29:39 +03 2018
x86_64 x86_64 x86_64 GNU/Linux
awk -f ver_linux
If some fields are empty or look unusual you may have an old version.
Compare to the current minimal requirements in Documentation/Changes.
Linux ubuntu 4.9.0-rc2reverted+ #1 SMP Sun Oct 28 20:29:39 +03 2018
x86_64 x86_64 x86_64 GNU/Linux
GNU Make 4.1
Binutils 2.30
Util-linux 2.31.1
Mount 2.31.1
Module-init-tools 24
E2fsprogs 1.44.1
Pcmciautils 018
Linux C Library 2.27
Dynamic linker (ldd) 2.27
Linux C++ Library 6.0.25
Procps 3.3.12
Kbd 2.0.4
Console-tools 2.0.4
Sh-utils 8.28
Udev 237
Wireless-tools 30
Modules Loaded ahci arc4 ath ath9k ath9k_common ath9k_hw
autofs4 binfmt_misc ccm cfg80211 coretemp crc32_pclmul crct10dif_pclmul
cryptd drm drm_kms_helper fb_sys_fops fjes ghash_clmulni_intel hid
hid_generic i2c_algo_bit i915 input_leds intel_cstate intel_powerclamp
intel_rapl intel_rapl_perf ip6table_filter ip6_tables ip6t_REJECT
ip6t_rt iptable_filter ip_tables ipt_REJECT irqbypass kvm kvm_intel
libahci lpc_ich mac80211 mac_hid mei mei_me nf_conntrack
nf_conntrack_broadcast nf_conntrack_ftp nf_conntrack_ipv4
nf_conntrack_ipv6 nf_conntrack_netbios_ns nf_defrag_ipv4 nf_defrag_ipv6
nf_log_common nf_log_ipv4 nf_log_ipv6 nf_nat nf_nat_ftp nf_reject_ipv4
nf_reject_ipv6 psmouse sch_fq_codel serio_raw shpchp snd snd_hda_codec
snd_hda_codec_conexant snd_hda_codec_generic snd_hda_codec_hdmi
snd_hda_core snd_hda_intel snd_hwdep snd_pcm snd_rawmidi snd_seq
snd_seq_device snd_seq_midi snd_seq_midi_event snd_timer sony_laptop
soundcore syscopyarea sysfillrect sysimgblt usbhid video
x86_pkg_temp_thermal x_tables xt_addrtype xt_conntrack xt_hl xt_limit
xt_LOG xt_tcpudp
dmesg | grep i8042
[ 2.986695] i8042: PNP: PS/2 Controller [PNP0303:PS2K,PNP0f13:PS2M]
at 0x60,0x64 irq 1,12 [ 2.989921] serio: i8042 KBD port at
0x60,0x64 irq 1 [ 2.989925] serio: i8042 AUX port at 0x60,0x64 irq
12 [ 3.014179] input: AT Translated Set 2 keyboard
as /devices/platform/i8042/serio0/input/input3 [ 20.297566] input:
AlpsPS/2 ALPS GlidePoint as /devices/platform/i8042/serio1/input/input8
> Peter, any ideas?
>
> Pavel
>
> On Fri 2018-10-19 10:20:31, Numan Demirdöğen wrote:
> > On Fri, 31 Aug 2018 21:53:11 +0300
> > Numan Demirdöğen <if.gnu.linux@gmail.com> wrote:
> >
> > >If I put laptop to suspend or hibernate by closing lid, power
> > >manager or any other method and then I resume/wake up laptop,
> > >keyboard is not responding. My laptop is a Sony Vaio VPCEH2F1E.
> > >
> > >Steps to produce bug:
> > >1. Boot
> > >2. Put laptop to sleep
> > >3. Resume
> > >
> > >What I expect to happen: Keyboard responds to key press.
> > >What happens: Keyboard does not respond but mouse and trackball are
> > >working.
> > >
> > >git bisect point 9d659ae14b545c4296e812c70493bfdc999b5c1c as the
> > >first bad commit.
> > >
> > >Bad commit link:
> > >https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=9d659ae14b545c4296e812c70493bfdc999b5c1c
> > >
> > > Link to actual bug report:
> > >https://bugzilla.kernel.org/show_bug.cgi?id=195471
> > >
> > >awk -f ver_linux
> > >Linux korsan 4.18.5-arch1-1-ARCH #1 SMP PREEMPT Fri Aug 24 12:48:58
> > >UTC 2018 x86_64 GNU/Linux GNU C 8.2.0
> > >GNU Make 4.2.1
> > >Binutils 2.31.1
> > >Util-linux 2.32.1
> > >Mount 2.32.1
> > >Module-init-tools 25
> > >E2fsprogs 1.44.4
> > >Jfsutils 1.1.15
> > >Reiserfsprogs 3.6.27
> > >Xfsprogs 4.17.0
> > >Pcmciautils 018
> > >Linux C Library 2.28
> > >Dynamic linker (ldd) 2.28
> > >Linux C++ Library 6.0.25
> > >Procps 3.3.15
> > >Kbd 2.0.4
> > >Console-tools 2.0.4
> > >Sh-utils 8.29
> > >Udev 239
> > >Wireless-tools 30
> > >Modules Loaded ac agpgart ahci arc4 ath ath3k ath9k
> > >ath9k_common ath9k_hw atkbd battery bluetooth bpfilter ccm cdrom
> > >cfg80211 coretemp crc16 crc32c_generic crc32c_intel crc32_pclmul
> > >crct10dif_pclmul cryptd drm drm_kms_helper ecdh_generic ehci_hcd
> > >ehci_pci evdev ext4 fb_sys_fops fscrypto fuse ghash_clmulni_intel
> > >gpio_ich hid hid_generic i2c_algo_bit i2c_i801 i8042 i915
> > >input_leds intel_cstate intel_gtt intel_powerclamp intel_rapl
> > >intel_rapl_perf intel_uncore iptable_filter iptable_mangle
> > >iptable_nat ip_tables irqbypass iTCO_vendor_support iTCO_wdt jbd2
> > >kvm kvm_intel led_class libahci libata libcrc32c libps2 lpc_ich
> > >mac80211 mac_hid mbcache mei mei_me mousedev msr nf_conntrack
> > >nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat nf_nat_ipv4 pcc_cpufreq
> > >psmouse rfkill rtc_cmos scsi_mod sd_mod serio serio_raw snd
> > >snd_hda_codec snd_hda_codec_conexant snd_hda_codec_generic
> > >snd_hda_codec_hdmi snd_hda_core snd_hda_intel snd_hwdep snd_pcm
> > >snd_timer sony_laptop soundcore sr_mod syscopyarea sysfillrect
> > >sysimgblt usb_common usbcore usbhid x86_pkg_temp_thermal x_tables
> > >
> > >dmesg | grep i8042
> > >[ 0.574078] i8042: PNP: PS/2 Controller
> > >[PNP0303:PS2K,PNP0f13:PS2M] at 0x60,0x64 irq 1,12 [ 0.575936]
> > >serio: i8042 KBD port at 0x60,0x64 irq 1 [ 0.576143] serio:
> > >i8042 AUX port at 0x60,0x64 irq 12 [ 0.618880] input: AT
> > >Translated Set 2 keyboard
> > >as /devices/platform/i8042/serio0/input/input3 [ 11.248435]
> > >input: AlpsPS/2 ALPS GlidePoint
> > >as /devices/platform/i8042/serio1/input/input5
> > >
> > >cat /proc/interrupts
> > > CPU0 CPU1 CPU2 CPU3
> > > 0: 8 0 0 0 IO-APIC
> > > 2-edge timer 1: 0 0 0 10286
> > > IO-APIC 1-edge i8042 8: 0 0
> > > 0 1 IO-APIC 8-edge rtc0 9: 0
> > > 9479 0 0 IO-APIC 9-fasteoi acpi 12:
> > > 0 0 370114 0 IO-APIC 12-edge i8042
> > > 16: 197457 0 0 0 IO-APIC
> > > 16-fasteoi ehci_hcd:usb1, ath9k 19: 0 0
> > > 0 0 IO-APIC 19-fasteoi i801_smbus 23:
> > > 0 0 71055 0 IO-APIC 23-fasteoi
> > > ehci_hcd:usb2 24: 0 74549 0 0
> > > PCI-MSI 512000-edge ahci[0000:00:1f.2] 25: 0
> > > 14 0 0 PCI-MSI 360448-edge mei_me
> > > 26: 0 0 138870 0 PCI-MSI
> > > 32768-edge i915 27: 0 0 0
> > > 330 PCI-MSI 442368-edge snd_hda_intel:card0
> > >NMI: 24 22 25 21 Non-maskable
> > >interrupts LOC: 534734 446934 603337 440320 Local
> > >timer interrupts SPU: 0 0 0 0
> > >Spurious interrupts PMI: 24 22 25
> > >21 Performance monitoring interrupts IWI: 26 12
> > >43115 22 IRQ work interrupts RTR: 0
> > >0 0 0 APIC ICR read retries RES: 95282
> > >76970 60242 46409 Rescheduling interrupts CAL:
> > >66746 75300 70554 69119 Function call interrupts
> > >TLB: 50929 54528 51413 48017 TLB shootdowns
> > >TRM: 0 0 0 0 Thermal event
> > >interrupts THR: 0 0 0 0
> > >Threshold APIC interrupts DFR: 0 0
> > >0 0 Deferred Error APIC interrupts MCE: 0
> > >0 0 0 Machine check exceptions MCP:
> > >18 19 19 19 Machine check polls
> > >HYP: 0 0 0 0 Hypervisor
> > >callback interrupts HRE: 0 0 0
> > >0 Hyper-V reenlightenment interrupts HVS: 0 0
> > >0 0 Hyper-V stimer0 interrupts ERR: 0
> > >MIS: 0 PIN: 0 0 0 0
> > >Posted-interrupt notification event NPI: 0
> > >0 0 0 Nested posted-interrupt event PIW:
> > >0 0 0 0 Posted-interrupt wakeup event
> > >
> > >P.S: My native language is not English.
> > >
> >
> > Hi,
> >
> > I am forwarding this e-mail which describes a problem related to
> > suspend/hibernate at the advise of Pavel Machek.
> >
[-- Attachment #2: Dijital OpenPGP imzası --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Logitech high-resolution scrolling..
From: Linus Torvalds @ 2018-10-28 19:13 UTC (permalink / raw)
To: Harry Cutts, Benjamin Tissoires, Jiri Kosina
Cc: linux-input, Linux Kernel Mailing List
So I use a Logitech MX Anywhere 2S mouse, and really like it. I have
the scroll-wheel unlocked, because I like flicking once to scroll a
lot.
However, the new high-res scroll code means that the scroll wheel
action is now much too sensitive. It's not even stable - it will
scroll back-and-forth a bit occasionally, and in fact it sometimes
seems to scroll not because I touch the scroll-wheel, but because the
movement of the mouse itself causes the scroll wheel to move slightly
and wiggle the scroll action.
So the recent change to enable the high-res scrolling really seems a
bit *too* extreme.
Is there some middle ground that turns the mouse from "look at it
sideways and it starts scrolling" to something slightly more
reasonable?
Linus
^ permalink raw reply
* Re: Logitech high-resolution scrolling..
From: Linus Torvalds @ 2018-10-28 21:08 UTC (permalink / raw)
To: Harry Cutts, Benjamin Tissoires, Jiri Kosina
Cc: linux-input, Linux Kernel Mailing List
In-Reply-To: <CAHk-=wjGKhoVaa60i5XZODEtzTUYVV3fouyYL4k=JsPY4K_xjQ@mail.gmail.com>
On Sun, Oct 28, 2018 at 12:13 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> So the recent change to enable the high-res scrolling really seems a
> bit *too* extreme.
>
> Is there some middle ground that turns the mouse from "look at it
> sideways and it starts scrolling" to something slightly more
> reasonable?
Actually, I think the bug may be in the generic HID high-resolution
scrolling code, and I only notice because the Logitech support means
that now I see it.
In particular, if you look at hid_scroll_counter_handle_scroll(),
you'll notice that it tries to turn a high-res scroll event into a
regular wheel event by using the resolution_multiplier.
But that code looks really broken. It tries to react to a "half
multiplier" thing:
int threshold = counter->resolution_multiplier / 2;
..
counter->remainder += hi_res_value;
if (abs(counter->remainder) >= threshold) {
and that's absolutely and entirely wrong.
Imagine that the high-res wheel counter has just moved a bit up (by
one high-res) tick, so now it's at the half-way mark to the
resolution_multiplier, and we scroll up by one:
low_res_scroll_amount =
counter->remainder / counter->resolution_multiplier
+ (hi_res_value > 0 ? 1 : -1);
input_report_rel(counter->dev, REL_WHEEL,
low_res_scroll_amount);
and then correct for it:
counter->remainder -=
low_res_scroll_amount * counter->resolution_multiplier;
now we went from "half resolution multiplier positive" to "half negative".
Which means that next time that the high-res event happens by even
just one high-resolution tick in the other direction, we'll now
generate a low-resolution scroll event in the other direction.
In other words, that function results in unstable behavior. Tiny tiny
movements back-and-forth in the high-res wheel events (which could be
just because either the sensor is unstable, or the wheel is wiggling
imperceptibly) can result in visible movement in the low-res
("regular") wheel reporting.
There is no "damping" function, in other words. Noise in the high
resolution reading can result in noise in the regular wheel reporting.
So that threshold handling needs to be fixed, I feel. Either get rid
of it entirely (you need to scroll a *full* resolution_multiplier to
get a regular wheel event), or the counter->remainder needs to be
*cleared* when a wheel event has been sent so that you don't get into
the whole "back-and-forth" mode.
Or some other damping model. I suspect there are people who have
researched what the right answer is, but I guarantee that the current
code is not the right answer.
I suspect this also explains why I *sometimes* see that "just moving
the mouse sends wheel events", and at other times don't. It needs to
get close to that "half a resolution multiplier" stage to get into the
bad cases, but then tiny tiny perturbations can cause unstable
behavior.
I can't be the only person seeing this, but I guess the Logitech mouse
is right now the only one that uses the new generic HID code, and I
guess not a lot of people have been *using* it.
Harry?
Linus
^ permalink raw reply
* Re: Oops in current tree in i2c
From: Benjamin Tissoires @ 2018-10-29 8:36 UTC (permalink / raw)
To: Hans de Goede
Cc: Linus Torvalds, jkosina, Julian Sax, open list:HID CORE LAYER,
lkml
In-Reply-To: <8d8e3771-0b33-2489-4e27-d3d63162df11@redhat.com>
Hi,
On Sun, Oct 28, 2018 at 11:45 AM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Hi Linus,
>
> On 27-10-18 18:08, Linus Torvalds wrote:
> > Julian, Jiri,
> > On my laptop I'm getting a kernel page fault with the current git
> > tree, and I'm tentatively blaming commit
> >
> > 9ee3e06610fd ("HID: i2c-hid: override HID descriptors for certain devices")
> >
> > but that's simply because it's the only thing that seems to touch this
> > particular area in this merge window.
> >
> > The oops looks like this:
> >
> > BUG: unable to handle kernel paging request at 000000007a25d598
> > PGD 0 P4D 0
> > Oops: 0000 [#1] SMP PTI
> > CPU: 1 PID: 888 Comm: systemd-udevd Not tainted 4.19.0-07715-g345671ea0f92 #4
> > Hardware name: Dell Inc. XPS 13 9350/09JHRY, BIOS 1.7.0 01/16/2018
> > RIP: 0010:strstr+0x19/0x70
> >
> > where the code disassembly (and the register contents) shows that the
> > wild pointer is the first argument to "strstr()", which just has a
> > bogus value that is not a valid kernel pointer (RDI: 000000007a25d598
> > - which is obviously also the address of the page fault)
> >
> > The call trace is:
> >
> > dmi_matches+0x55/0xc0
> > dmi_first_match+0x26/0x40
> > i2c_hid_get_dmi_i2c_hid_desc_override+0x16/0x40 [i2c_hid]
> > i2c_hid_probe+0x28c/0x760 [i2c_hid]
> > i2c_device_probe+0x1e7/0x260
> > really_probe+0xf8/0x3e0
> > driver_probe_device+0x10f/0x120
> > bus_for_each_drv+0x66/0xb0
> > __device_attach+0xd9/0x150
> > bus_probe_device+0x8a/0xa0
> > device_add+0x48e/0x660
> > i2c_new_device+0x162/0x350
> >
> > which is why I suspect that new i2c_hid_get_dmi_hid_report_desc_override() code.
> >
> > I *think* the problem is that the i2c_hid_dmi_desc_override_table[]
> > isn't terminated by a NULL entry, and I will test that next.
Thanks for fixing that in master. I should have spotted it while
reviewing the series. My bad.
>
> Yes that likely is the problem. I already had a bug report from one of the
> Manjaro maintainers who was cherry picking this into the Manjaro kernel.
>
> So I ran some tests on a laptop of mine which does use i2c-hid but I
> failed to reproduce the issue, so we both (me and the Manjaro maintainer)
> both assumed something went wrong with the backport.
>
> Both of us seem to have overlooked the missing terminating entry,
> as well as other people involved in the patch.
>
> > What makes me *very* unhappy about this is that if I'm right, I think
> > it means that code was literally not tested at all by anybody who
> > didn't have one of the entries in that list.
>
> That is not true, I've hit one of these unterminated dmi lists
> issues before and it depends on what get put in mem directly
> after the list by the linker, bugs caused by this do not always
> reproduce unfortunately.
>
> And as mentioned I have tested the patch on a machine with an i2c-hid
> touchpad, which is not on the list and I did not hit this problem.
Oh. This explains why I was also sure I tested this on a i2c-hid
laptop without having the oops.
I'll try to setup some automate testing for those also.
Cheers,
Benjamin
>
> Anyways this is fixed now, thank you for catching this.
>
> Regards,
>
> Hans
^ permalink raw reply
* Re: Another HID problem this merge window..
From: Benjamin Tissoires @ 2018-10-29 8:49 UTC (permalink / raw)
To: Jiri Kosina
Cc: Linus Torvalds, joe, open list:HID CORE LAYER, lkml,
Hans de Goede
In-Reply-To: <nycvar.YFH.7.76.1810272338590.23511@cbobk.fhfr.pm>
Hi,
On Sat, Oct 27, 2018 at 11:45 PM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Sat, 27 Oct 2018, Linus Torvalds wrote:
>
> > I wonder if there is some truly old historical legacy there, ie the old
> > PC keyboard support would have been configurable out only for expert
> > users to avoid errors, and maybe the HID Kconfig file started getting
> > ideas from that...
>
> This really goes waaay back to times when we extracted all the quirks from
> the generic driver (which became unmaintainable exactly because quirks
> being sprinkled left and right) into specialized drivers, but didn't want
> to cause too many user surprises that all of a sudden their configuration
> regressed when it comes to hardware support.
>
> We've had exactly this discussion multiple times before, see for example
>
> https://lkml.org/lkml/2010/5/20/227
>
> So I guess there is no need for replaying it, I think we're in complete
> agreement.
On the things I have on my plate, I'll try to remove all of the tiny
HID drivers that does nothing but some small remapping. This will
probably need help from userspace ("firmware" or bpf loading), and I
have not settled my plans yet.
I also think we should probably clean up the Kconfig now that
hid-generic can unbind itself if there is a special driver coming in.
And that means that we should probably start removing the blacklist of
devices that have special modules, to let them be taken by hid-generic
on boot until their driver is loaded.
>
> That being said, benff Kconfig setting definitely escaped attention. That
> should never ever have been set to default y, I take blame for not
> noticing that while applying the patch.
Not sure I have actually reviewed this one (I don't think so), but
I'll keep this in my head for next drivers.
Cheers,
Benjamin
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
^ permalink raw reply
* Re: Logitech high-resolution scrolling..
From: Jiri Kosina @ 2018-10-29 13:18 UTC (permalink / raw)
To: Linus Torvalds
Cc: Harry Cutts, Benjamin Tissoires, linux-input,
Linux Kernel Mailing List, Peter Hutterer
In-Reply-To: <CAHk-=wjGKhoVaa60i5XZODEtzTUYVV3fouyYL4k=JsPY4K_xjQ@mail.gmail.com>
On Sun, 28 Oct 2018, Linus Torvalds wrote:
> So I use a Logitech MX Anywhere 2S mouse, and really like it. I have
> the scroll-wheel unlocked, because I like flicking once to scroll a
> lot.
>
> However, the new high-res scroll code means that the scroll wheel
> action is now much too sensitive. It's not even stable - it will
> scroll back-and-forth a bit occasionally, and in fact it sometimes
> seems to scroll not because I touch the scroll-wheel, but because the
> movement of the mouse itself causes the scroll wheel to move slightly
> and wiggle the scroll action.
>
> So the recent change to enable the high-res scrolling really seems a
> bit *too* extreme.
>
> Is there some middle ground that turns the mouse from "look at it
> sideways and it starts scrolling" to something slightly more
> reasonable?
Benjamin indicated that Peter probably has found the issue in the code
(failure to properly reset on direction change) that might be causing
this. Adding to CC.
Peter?
Thanks,
--
Jiri Kosina
SUSE Labs
^ 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