Re: [PATCH net v3] net: sched: fix packet stuck problem for lockless qdisc
On Tue, 13 Apr 2021, Juergen Gross wrote: > > what Jiri said about "I am still planning to have Yunsheng Lin's > > (CCing) fix [1] tested in the coming days." is that Juergen has > > done the test and provide a "Tested-by" tag. > > Correct. And I did this after Jiri asking me to do so. Exactly, Juergen's setup is the one where this issue is being reproduced reliably for us, and Juergen verified that your patch fixes that issue. Seeing that you now also addressed the STATE_DEACTIVATED issue (which we don't have reproducer for though), I think your series should be good to go if the STATE_DEACTIVATED fix has been verified independently. Thanks! -- Jiri Kosina SUSE Labs
[GIT PULL] HID fixes
Linus, please pull from git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-linus to receive HID fixes. I wanted to send them earlier than for the last -rc, but then unfortunately many things interfered. The changes are all device/driver specific fixes. = - EV_KEY and EV_ABS regression fix for Wacom from Ping Cheng - BIOS-specific quirk to fix some of the AMD_SFH-based systems, from Hans de Goede - other small error handling fixes and device ID additions = Thanks. Douglas Gilbert (1): HID cp2112: fix support for multiple gpiochips Hans de Goede (3): AMD_SFH: Removed unused activecontrolstatus member from the amd_mp2_dev struct AMD_SFH: Add sensor_mask module parameter AMD_SFH: Add DMI quirk table for BIOS-es which don't set the activestatus bits Jia-Ju Bai (1): HID: alps: fix error return code in alps_input_configured() Jiapeng Zhong (1): HID: wacom: Assign boolean values to a bool variable Luke D Jones (1): HID: asus: Add support for 2021 ASUS N-Key keyboard Ping Cheng (1): HID: wacom: set EV_KEY and EV_ABS only for non-HID_GENERIC type of devices Shou-Chieh Hsu (1): HID: google: add don USB id drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 40 +++--- drivers/hid/amd-sfh-hid/amd_sfh_pcie.h | 1 - drivers/hid/hid-alps.c | 1 + drivers/hid/hid-asus.c | 3 +++ drivers/hid/hid-cp2112.c | 22 +-- drivers/hid/hid-google-hammer.c| 2 ++ drivers/hid/hid-ids.h | 2 ++ drivers/hid/wacom_wac.c| 8 +++ 8 files changed, 59 insertions(+), 20 deletions(-) -- Jiri Kosina SUSE Labs
Re: [PATCH 1/2] zram: fix crashes due to use of cpu hotplug multistate
On Thu, 8 Apr 2021, Greg KH wrote: > > If there is a driver/subsystem code that can't handle the reverse > > operation to modprobe, it clearly can't handle error handling during > > modprobe (which, one would hope, is supported), and should be fixed. > > Huh? No, that's not the issue here, it's the issue of different > userspace code paths into the module at the same time that it is trying > to be unloaded. That has nothing to do with loading the module the > first time as userspace is not touching those apis yet. So do you claim that once the first (out of possibly many) userspace-visible sysfs entry has been created during module insertion and made available to userspace, there is never going to be rollback happening that'd be removing that first sysfs entry again? Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH 1/2] zram: fix crashes due to use of cpu hotplug multistate
On Thu, 8 Apr 2021, Greg KH wrote: > Removing a module from a system has always been "let's try it and see!" > type of operation for a very long time. Which part of it? If there is a driver/subsystem code that can't handle the reverse operation to modprobe, it clearly can't handle error handling during modprobe (which, one would hope, is supported), and should be fixed. If there is a particular issue in kernel dynamic linker that causes crash on module removal, we'd better off fixing it. Is there one such that makes you claim module removal unsupported? Thanks, -- Jiri Kosina SUSE Labs
Re: Packet gets stuck in NOLOCK pfifo_fast qdisc
On Sat, 3 Apr 2021, Hillf Danton wrote: > >>> Sure. Seems they crept in over time. I had some plans to write a > >>> lockless HTB implementation. But with fq+EDT with BPF it seems that > >>> it is no longer needed, we have a more generic/better solution. So > >>> I dropped it. Also most folks should really be using fq, fq_codel, > >>> etc. by default anyways. Using pfifo_fast alone is not ideal IMO. > >> > >> Half a year later, we still have the NOLOCK implementation > >> present, and pfifo_fast still does set the TCQ_F_NOLOCK flag on itself. > >> > >> And we've just been bitten by this very same race which appears to be > >> still unfixed, with single packet being stuck in pfifo_fast qdisc > >> basically indefinitely due to this very race that this whole thread began > >> with back in 2019. > >> > >> Unless there are > >> > >>(a) any nice ideas how to solve this in an elegant way without > >>(re-)introducing extra spinlock (Cong's fix) or > >> > >>(b) any objections to revert as per the argumentation above > >> > >> I'll be happy to send a revert of the whole NOLOCK implementation next > >> week. > >> > >Jiri > > > > Feel free to revert it as the scorch wont end without a deluge. I am still planning to have Yunsheng Lin's (CCing) fix [1] tested in the coming days. If it works, then we can consider proceeding with it, otherwise I am all for reverting the whole NOLOCK stuff. [1] https://lore.kernel.org/linux-can/1616641991-14847-1-git-send-email-linyunsh...@huawei.com/T/#u -- Jiri Kosina SUSE Labs
Re: Packet gets stuck in NOLOCK pfifo_fast qdisc
On Thu, 3 Sep 2020, John Fastabend wrote: > > > At this point I fear we could consider reverting the NOLOCK stuff. > > > I personally would hate doing so, but it looks like NOLOCK benefits are > > > outweighed by its issues. > > > > I agree, NOLOCK brings more pains than gains. There are many race > > conditions hidden in generic qdisc layer, another one is enqueue vs. > > reset which is being discussed in another thread. > > Sure. Seems they crept in over time. I had some plans to write a > lockless HTB implementation. But with fq+EDT with BPF it seems that > it is no longer needed, we have a more generic/better solution. So > I dropped it. Also most folks should really be using fq, fq_codel, > etc. by default anyways. Using pfifo_fast alone is not ideal IMO. Half a year later, we still have the NOLOCK implementation present, and pfifo_fast still does set the TCQ_F_NOLOCK flag on itself. And we've just been bitten by this very same race which appears to be still unfixed, with single packet being stuck in pfifo_fast qdisc basically indefinitely due to this very race that this whole thread began with back in 2019. Unless there are (a) any nice ideas how to solve this in an elegant way without (re-)introducing extra spinlock (Cong's fix) or (b) any objections to revert as per the argumentation above I'll be happy to send a revert of the whole NOLOCK implementation next week. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v2] livepatch: Replace the fake signal sending with TIF_NOTIFY_SIGNAL infrastructure
On Mon, 29 Mar 2021, Miroslav Benes wrote:
> Livepatch sends a fake signal to all remaining blocking tasks of a
> running transition after a set period of time. It uses TIF_SIGPENDING
> flag for the purpose. Commit 12db8b690010 ("entry: Add support for
> TIF_NOTIFY_SIGNAL") added a generic infrastructure to achieve the same.
> Replace our bespoke solution with the generic one.
>
> Reviewed-by: Jens Axboe
> Reviewed-by: Petr Mladek
> Signed-off-by: Miroslav Benes
Pushed out to livepatching#for-5.13/signal.
--
Jiri Kosina
SUSE Labs
Re: [PATCH 0/2] HID: Add support for Surface Aggregator Module HID transport
On Wed, 10 Mar 2021, Maximilian Luz wrote: > This series adds support for the Surface System Aggregator Module (SSAM) > HID transport subsystem. > > The SSAM is an embedded controller, found on 5th- and later generation > Microsoft Surface devices. On some of these devices (specifically > Surface Laptops 1, 2, and 3, as well as Surface Book 3), built-in input > devices are connected via the SSAM. These devices communicate (mostly) > via normal HID reports, so adding support for them is (mostly) just a > matter of implementing an HID transport driver. > > SSAM actually has two different HID interfaces: One (legacy) interface > used on Surface Laptops 1 and 2, and a newer interface for the rest. The > newer interface allows for multiple HID devices to be addressed and is > implemented in the first patch. The older interface only allows a single > HID device to be connected and, furthermore, only allows a single output > report, specifically one for the caps lock LED. This is implemented in > the second patch. > > See the commit messages of the respective patches for more details. Now queued in hid.git#for-5.13/surface-system-aggregator-intergration Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()
On Tue, 23 Mar 2021, Sedat Dilek wrote: > > > > > > > > Johannes is telling me that he merged this patch internally, > > > > > > > > but I have no > > > > > > > > idea what is happening to it ... ? > > > > > > > > > > > > > > > > The reported splat is a clear bug, so it should be fixed one > > > > > > > > way or the > > > > > > > > other. > > > > > > > > > > > > > > Should I take this to wireless-drivers? > > > > > > > > > > > > I can't speak for the maintainers, but as far as I am concerned, it > > > > > > definitely is a 5.12 material, as it fixes real scheduling bug. > > > > > > > > > > Yes, please take this to w-d. We have a similar patch internally, but > > > > > there's a backlog and it will take me some time to get to it. I'll > > > > > resolve eventual conflicts when time comes. > > > > > > > > Ok, can I have your ack for patchwork? > > > > > > Sorry, forgot that. > > > > > > Acked-by: Luca Coelho > > > > Sorry for sounding like broken record :) but this fix is still not in any > > tree as far as I can tell. And it's fixing real scheduling in atomic bug. > > > > Thanks, > > > > [ CC Chris Murphy ] > > A week ago Chris sent an email to linux-wireless with pointing to: > > https://bugzilla.kernel.org/show_bug.cgi?id=212297 > > AFAICS, that is the same bug. Indeed, it is. Chris, if you want to provide your Tested-by:, the (yet unmerged) patch to test can be found here: https://patchwork.kernel.org/project/linux-wireless/patch/nycvar.yfh.7.76.2103021125430.12...@cbobk.fhfr.pm/ -- Jiri Kosina SUSE Labs
Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()
On Sat, 13 Mar 2021, Luca Coelho wrote: > > > > > > > > It's possible for iwl_pcie_enqueue_hcmd() to be called with > > > > > > > > hard IRQs > > > > > > > > disabled (e.g. from LED core). We can't enable BHs in such a > > > > > > > > situation. > > > > > > > > > > > > > > > > Turn the unconditional BH-enable/BH-disable code into > > > > > > > > hardirq-disable/conditional-enable. > > > > > > > > > > > > > > > > This fixes the warning below. > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > friendly ping on this one ... > > > > > > > > > > > > Luca, > > > > > > > > > > > > Johannes is telling me that he merged this patch internally, but I > > > > > > have no > > > > > > idea what is happening to it ... ? > > > > > > > > > > > > The reported splat is a clear bug, so it should be fixed one way or > > > > > > the > > > > > > other. > > > > > > > > > > Should I take this to wireless-drivers? > > > > > > > > I can't speak for the maintainers, but as far as I am concerned, it > > > > definitely is a 5.12 material, as it fixes real scheduling bug. > > > > > > Yes, please take this to w-d. We have a similar patch internally, but > > > there's a backlog and it will take me some time to get to it. I'll > > > resolve eventual conflicts when time comes. > > > > Ok, can I have your ack for patchwork? > > Sorry, forgot that. > > Acked-by: Luca Coelho Sorry for sounding like broken record :) but this fix is still not in any tree as far as I can tell. And it's fixing real scheduling in atomic bug. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH 5.11 13/31] net: bonding: fix error return code of bond_neigh_init()
On Fri, 19 Mar 2021, Greg Kroah-Hartman wrote:
> > > diff --git a/drivers/net/bonding/bond_main.c
> > > b/drivers/net/bonding/bond_main.c
> > > index 5fe5232cc3f3..fba6b6d1b430 100644
> > > --- a/drivers/net/bonding/bond_main.c
> > > +++ b/drivers/net/bonding/bond_main.c
> > > @@ -3917,11 +3917,15 @@ static int bond_neigh_init(struct neighbour *n)
> > >
> > > rcu_read_lock();
> > > slave = bond_first_slave_rcu(bond);
> > > - if (!slave)
> > > + if (!slave) {
> > > + ret = -EINVAL;
> > > goto out;
> > > + }
> > > slave_ops = slave->dev->netdev_ops;
> > > - if (!slave_ops->ndo_neigh_setup)
> > > + if (!slave_ops->ndo_neigh_setup) {
> > > + ret = -EINVAL;
> > > goto out;
> > > + }
> >
> > This patch is completely broken and breaks bonding functionality
> > altogether for me.
>
> Is Linus's tree also broken for you? This showed up in 5.12-rc3.
Yes, it is.
--
Jiri Kosina
SUSE Labs
Re: [PATCH 5.11 13/31] net: bonding: fix error return code of bond_neigh_init()
On Fri, 19 Mar 2021, Jiri Kosina wrote:
> > [ Upstream commit 2055a99da8a253a357bdfd359b3338ef3375a26c ]
> >
> > When slave is NULL or slave_ops->ndo_neigh_setup is NULL, no error
> > return code of bond_neigh_init() is assigned.
> > To fix this bug, ret is assigned with -EINVAL in these cases.
> >
> > Fixes: 9e99bfefdbce ("bonding: fix bond_neigh_init()")
> > Reported-by: TOTE Robot
> > Signed-off-by: Jia-Ju Bai
> > Signed-off-by: David S. Miller
> > Signed-off-by: Sasha Levin
> > ---
> > drivers/net/bonding/bond_main.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/bonding/bond_main.c
> > b/drivers/net/bonding/bond_main.c
> > index 5fe5232cc3f3..fba6b6d1b430 100644
> > --- a/drivers/net/bonding/bond_main.c
> > +++ b/drivers/net/bonding/bond_main.c
> > @@ -3917,11 +3917,15 @@ static int bond_neigh_init(struct neighbour *n)
> >
> > rcu_read_lock();
> > slave = bond_first_slave_rcu(bond);
> > - if (!slave)
> > + if (!slave) {
> > + ret = -EINVAL;
> > goto out;
> > + }
> > slave_ops = slave->dev->netdev_ops;
> > - if (!slave_ops->ndo_neigh_setup)
> > + if (!slave_ops->ndo_neigh_setup) {
> > + ret = -EINVAL;
> > goto out;
> > + }
>
> This patch is completely broken and breaks bonding functionality
> altogether for me.
... and I just found out that revert is already queued in netdev.git. So
please drop it from stable queue as well.
--
Jiri Kosina
SUSE Labs
Re: [PATCH 5.11 13/31] net: bonding: fix error return code of bond_neigh_init()
On Fri, 19 Mar 2021, Greg Kroah-Hartman wrote:
> From: Jia-Ju Bai
>
> [ Upstream commit 2055a99da8a253a357bdfd359b3338ef3375a26c ]
>
> When slave is NULL or slave_ops->ndo_neigh_setup is NULL, no error
> return code of bond_neigh_init() is assigned.
> To fix this bug, ret is assigned with -EINVAL in these cases.
>
> Fixes: 9e99bfefdbce ("bonding: fix bond_neigh_init()")
> Reported-by: TOTE Robot
> Signed-off-by: Jia-Ju Bai
> Signed-off-by: David S. Miller
> Signed-off-by: Sasha Levin
> ---
> drivers/net/bonding/bond_main.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 5fe5232cc3f3..fba6b6d1b430 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -3917,11 +3917,15 @@ static int bond_neigh_init(struct neighbour *n)
>
> rcu_read_lock();
> slave = bond_first_slave_rcu(bond);
> - if (!slave)
> + if (!slave) {
> + ret = -EINVAL;
> goto out;
> + }
> slave_ops = slave->dev->netdev_ops;
> - if (!slave_ops->ndo_neigh_setup)
> + if (!slave_ops->ndo_neigh_setup) {
> + ret = -EINVAL;
> goto out;
> + }
This patch is completely broken and breaks bonding functionality
altogether for me.
--
Jiri Kosina
SUSE Labs
Re: [PATCH v2] HID: intel_ish-hid: HBM: Use connected standby state bit during suspend/resume
On Tue, 16 Mar 2021, Srinivas Pandruvada wrote: > From: Ye Xiang > > The individual sensor drivers implemented in the ISH firmware needs > capability to take special actions when there is a change in the system > standby state. The ISH core firmware passes this notification to > individual sensor drivers in response to the OS request via connected > standby bit in the SYSTEM_STATE_STATUS command. > > This change sets CONNECTED_STANDBY_STATE_BIT bit to 1 during suspend > callback and clears during resume callback. > > Signed-off-by: Ye Xiang > [srinivas.pandruv...@linux.intel.com: changelog rewrite] > Acked-by: Srinivas Pandruvada > --- > v2: > changed changelog to be more clear > Changed the name in the signed-off to match "From" > > drivers/hid/intel-ish-hid/ishtp/hbm.c | 6 +++--- > drivers/hid/intel-ish-hid/ishtp/hbm.h | 1 + > 2 files changed, 4 insertions(+), 3 deletions(-) Applied, thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: ft260: fix an error message in ft260_i2c_write_read()
On Thu, 18 Mar 2021, Dan Carpenter wrote:
> The "len" variable is uninitialize.
>
> Fixes: 6a82582d9fa4 ("HID: ft260: add usb hid to i2c host bridge driver")
> Signed-off-by: Dan Carpenter
> ---
> drivers/hid/hid-ft260.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 047aa85a7c83..a5751607ce24 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -512,7 +512,8 @@ static int ft260_i2c_write_read(struct ft260_device *dev,
> struct i2c_msg *msgs)
> struct hid_device *hdev = dev->hdev;
>
> if (msgs[0].len > 2) {
> - hid_err(hdev, "%s: unsupported wr len: %d\n", __func__, len);
> + hid_err(hdev, "%s: unsupported wr len: %d\n", __func__,
> + msgs[0].len);
> return -EOPNOTSUPP;
> }
Applied, thanks Dan.
--
Jiri Kosina
SUSE Labs
Re: [PATCH] Bluetooth: avoid deadlock between hci_dev->lock and socket lock
On Tue, 16 Mar 2021, Marcel Holtmann wrote:
> > Fixes: eab2404ba798 ("Bluetooth: Add BT_PHY socket option")
> > Signed-off-by: Jiri Kosina
> > ---
> > net/bluetooth/hci_conn.c | 4
> > 1 file changed, 4 deletions(-)
>
> patch has been applied to bluetooth-next tree.
Thanks; could it be pushed for 5.12-rc still though? It fixes an actual
possibility of deadlock.
--
Jiri Kosina
SUSE Labs
[PATCH] Bluetooth: avoid deadlock between hci_dev->lock and socket lock
From: Jiri Kosina
Commit eab2404ba798 ("Bluetooth: Add BT_PHY socket option") added a
dependency between socket lock and hci_dev->lock that could lead to
deadlock.
It turns out that hci_conn_get_phy() is not in any way relying on hdev
being immutable during the runtime of this function, neither does it even
look at any of the members of hdev, and as such there is no need to hold
that lock.
This fixes the lockdep splat below:
==
WARNING: possible circular locking dependency detected
5.12.0-rc1-00026-g73d464503354 #10 Not tainted
--
bluetoothd/1118 is trying to acquire lock:
8f078383c078 (>lock){+.+.}-{3:3}, at: hci_conn_get_phy+0x1c/0x150
[bluetooth]
but task is already holding lock:
8f07e831d920 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at:
l2cap_sock_getsockopt+0x8b/0x610
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #3 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}:
lock_sock_nested+0x72/0xa0
l2cap_sock_ready_cb+0x18/0x70 [bluetooth]
l2cap_config_rsp+0x27a/0x520 [bluetooth]
l2cap_sig_channel+0x658/0x1330 [bluetooth]
l2cap_recv_frame+0x1ba/0x310 [bluetooth]
hci_rx_work+0x1cc/0x640 [bluetooth]
process_one_work+0x244/0x5f0
worker_thread+0x3c/0x380
kthread+0x13e/0x160
ret_from_fork+0x22/0x30
-> #2 (>lock#2/1){+.+.}-{3:3}:
__mutex_lock+0xa3/0xa10
l2cap_chan_connect+0x33a/0x940 [bluetooth]
l2cap_sock_connect+0x141/0x2a0 [bluetooth]
__sys_connect+0x9b/0xc0
__x64_sys_connect+0x16/0x20
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
-> #1 (>chan_lock){+.+.}-{3:3}:
__mutex_lock+0xa3/0xa10
l2cap_chan_connect+0x322/0x940 [bluetooth]
l2cap_sock_connect+0x141/0x2a0 [bluetooth]
__sys_connect+0x9b/0xc0
__x64_sys_connect+0x16/0x20
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
-> #0 (>lock){+.+.}-{3:3}:
__lock_acquire+0x147a/0x1a50
lock_acquire+0x277/0x3d0
__mutex_lock+0xa3/0xa10
hci_conn_get_phy+0x1c/0x150 [bluetooth]
l2cap_sock_getsockopt+0x5a9/0x610 [bluetooth]
__sys_getsockopt+0xcc/0x200
__x64_sys_getsockopt+0x20/0x30
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
other info that might help us debug this:
Chain exists of:
>lock --> >lock#2/1 --> sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP
Possible unsafe locking scenario:
CPU0CPU1
lock(sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP);
lock(>lock#2/1);
lock(sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP);
lock(>lock);
*** DEADLOCK ***
1 lock held by bluetoothd/1118:
#0: 8f07e831d920 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at:
l2cap_sock_getsockopt+0x8b/0x610 [bluetooth]
stack backtrace:
CPU: 3 PID: 1118 Comm: bluetoothd Not tainted 5.12.0-rc1-00026-g73d464503354
#10
Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
Call Trace:
dump_stack+0x7f/0xa1
check_noncircular+0x105/0x120
? __lock_acquire+0x147a/0x1a50
__lock_acquire+0x147a/0x1a50
lock_acquire+0x277/0x3d0
? hci_conn_get_phy+0x1c/0x150 [bluetooth]
? __lock_acquire+0x2e1/0x1a50
? lock_is_held_type+0xb4/0x120
? hci_conn_get_phy+0x1c/0x150 [bluetooth]
__mutex_lock+0xa3/0xa10
? hci_conn_get_phy+0x1c/0x150 [bluetooth]
? lock_acquire+0x277/0x3d0
? mark_held_locks+0x49/0x70
? mark_held_locks+0x49/0x70
? hci_conn_get_phy+0x1c/0x150 [bluetooth]
hci_conn_get_phy+0x1c/0x150 [bluetooth]
l2cap_sock_getsockopt+0x5a9/0x610 [bluetooth]
__sys_getsockopt+0xcc/0x200
__x64_sys_getsockopt+0x20/0x30
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7fb73df33eee
Code: 48 8b 0d 85 0f 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00
00 00 00 90 f3 0f 1e fa 49 89 ca b8 37 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01
c3 48 8b 0d 52 0f 0c 00 f7 d8 64 89 01 48
RSP: 002b:7fffcfbbbf08 EFLAGS: 0203 ORIG_RAX: 0037
RAX: ffda RBX: 0019 RCX: 7fb73df33eee
RDX: 000e RSI: 0112 RDI: 0018
RBP: R08: 7fffcfbbbf44 R09:
R10: 7fffcfbbbf3c R11: 0203 R12:
R13: 0018 R14: R15: 556fcefc70d0
Fixes: eab2404ba798 ("Bluetooth: Add BT_PHY socket option")
Signed-off-by: Jiri Kosina
---
net/bluetooth/hci_conn.c | 4
1 file changed, 4 deletions(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 6ffa89e3ba0a..f726466905
Re: Lockdep report for hci_conn_get_phy()
On Thu, 4 Mar 2021, Jiri Kosina wrote:
> ==
> WARNING: possible circular locking dependency detected
> 5.12.0-rc1-00026-g73d464503354 #10 Not tainted
> --
> bluetoothd/1118 is trying to acquire lock:
> 8f078383c078 (>lock){+.+.}-{3:3}, at: hci_conn_get_phy+0x1c/0x150
> [bluetooth]
>
> but task is already holding lock:
> 8f07e831d920 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at:
> l2cap_sock_getsockopt+0x8b/0x610
>
>
> which lock already depends on the new lock.
>
>
> the existing dependency chain (in reverse order) is:
>
> -> #3 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}:
> lock_sock_nested+0x72/0xa0
> l2cap_sock_ready_cb+0x18/0x70 [bluetooth]
> l2cap_config_rsp+0x27a/0x520 [bluetooth]
> l2cap_sig_channel+0x658/0x1330 [bluetooth]
> l2cap_recv_frame+0x1ba/0x310 [bluetooth]
> hci_rx_work+0x1cc/0x640 [bluetooth]
> process_one_work+0x244/0x5f0
> worker_thread+0x3c/0x380
> kthread+0x13e/0x160
> ret_from_fork+0x22/0x30
>
> -> #2 (>lock#2/1){+.+.}-{3:3}:
> __mutex_lock+0xa3/0xa10
> l2cap_chan_connect+0x33a/0x940 [bluetooth]
> l2cap_sock_connect+0x141/0x2a0 [bluetooth]
> __sys_connect+0x9b/0xc0
> __x64_sys_connect+0x16/0x20
> do_syscall_64+0x33/0x80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> -> #1 (>chan_lock){+.+.}-{3:3}:
> __mutex_lock+0xa3/0xa10
> l2cap_chan_connect+0x322/0x940 [bluetooth]
> l2cap_sock_connect+0x141/0x2a0 [bluetooth]
> __sys_connect+0x9b/0xc0
> __x64_sys_connect+0x16/0x20
> do_syscall_64+0x33/0x80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> -> #0 (>lock){+.+.}-{3:3}:
> __lock_acquire+0x147a/0x1a50
> lock_acquire+0x277/0x3d0
> __mutex_lock+0xa3/0xa10
> hci_conn_get_phy+0x1c/0x150 [bluetooth]
> l2cap_sock_getsockopt+0x5a9/0x610 [bluetooth]
> __sys_getsockopt+0xcc/0x200
> __x64_sys_getsockopt+0x20/0x30
> do_syscall_64+0x33/0x80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
So looking at the code and digging a bit in the history, it seems like the
above dependency chain has been there since ever ...
> other info that might help us debug this:
>
> Chain exists of:
>>lock --> >lock#2/1 --> sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP
> Possible unsafe locking scenario:
>
> CPU0CPU1
>
>lock(sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP);
> lock(>lock#2/1);
> lock(sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP);
>lock(>lock);
>
> *** DEADLOCK ***
>
> 1 lock held by bluetoothd/1118:
> #0: 8f07e831d920 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at:
> l2cap_sock_getsockopt+0x8b/0x610 [bluetooth]
>
> stack backtrace:
> CPU: 3 PID: 1118 Comm: bluetoothd Not tainted 5.12.0-rc1-00026-g73d464503354
> #10
> Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
> Call Trace:
> dump_stack+0x7f/0xa1
> check_noncircular+0x105/0x120
> ? __lock_acquire+0x147a/0x1a50
> __lock_acquire+0x147a/0x1a50
> lock_acquire+0x277/0x3d0
> ? hci_conn_get_phy+0x1c/0x150 [bluetooth]
> ? __lock_acquire+0x2e1/0x1a50
> ? lock_is_held_type+0xb4/0x120
> ? hci_conn_get_phy+0x1c/0x150 [bluetooth]
> __mutex_lock+0xa3/0xa10
> ? hci_conn_get_phy+0x1c/0x150 [bluetooth]
> ? lock_acquire+0x277/0x3d0
> ? mark_held_locks+0x49/0x70
> ? mark_held_locks+0x49/0x70
> ? hci_conn_get_phy+0x1c/0x150 [bluetooth]
> hci_conn_get_phy+0x1c/0x150 [bluetooth]
> l2cap_sock_getsockopt+0x5a9/0x610 [bluetooth]
> __sys_getsockopt+0xcc/0x200
> __x64_sys_getsockopt+0x20/0x30
> do_syscall_64+0x33/0x80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
... but the sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP -> conn->hdev dependency
has been added only in eab2404ba798 ("Bluetooth: Add BT_PHY socket
option") and I've started to see this splat only now as I've probably
recently acquired userspace that excercises this getsockopt(BT_PHY).
--
Jiri Kosina
SUSE Labs
Re: [PATCHv2 1/1] HID: ft260: add usb hid to i2c host bridge driver
On Thu, 4 Mar 2021, Michael Zaidman wrote: > > > The FTDI FT260 chip implements USB to I2C/UART bridges through two > > > USB HID class interfaces. The first - for I2C, and the second for UART. > > > Each interface is independent, and the kernel detects it as a separate > > > USB hidraw device. > > > > > > This commit adds I2C host adapter support. > > > > > > Signed-off-by:Michael Zaidman > > > > I've applied the patch, ran some tests with a UMFT260EV1A evaluation board, > > and found no issues with the functionality it provides. > > > > Tested-by: Aaron Jones (FTDI-UK) > > Aaron, thanks for giving it a try - much appreciated! I have now queued the driver in hid.git#for-5.13/ft260. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()
On Sat, 13 Mar 2021, Kalle Valo wrote: > >> > From: Jiri Kosina > >> > > >> > It's possible for iwl_pcie_enqueue_hcmd() to be called with hard IRQs > >> > disabled (e.g. from LED core). We can't enable BHs in such a situation. > >> > > >> > Turn the unconditional BH-enable/BH-disable code into > >> > hardirq-disable/conditional-enable. > >> > > >> > This fixes the warning below. > >> > >> Hi, > >> > >> friendly ping on this one ... > > > > Luca, > > > > Johannes is telling me that he merged this patch internally, but I have no > > idea what is happening to it ... ? > > > > The reported splat is a clear bug, so it should be fixed one way or the > > other. > > Should I take this to wireless-drivers? I can't speak for the maintainers, but as far as I am concerned, it definitely is a 5.12 material, as it fixes real scheduling bug. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()
On Mon, 8 Mar 2021, Jiri Kosina wrote: > > From: Jiri Kosina > > > > It's possible for iwl_pcie_enqueue_hcmd() to be called with hard IRQs > > disabled (e.g. from LED core). We can't enable BHs in such a situation. > > > > Turn the unconditional BH-enable/BH-disable code into > > hardirq-disable/conditional-enable. > > > > This fixes the warning below. > > Hi, > > friendly ping on this one ... Luca, Johannes is telling me that he merged this patch internally, but I have no idea what is happening to it ... ? The reported splat is a clear bug, so it should be fixed one way or the other. Thanks. > > Thanks! > > > > > WARNING: CPU: 1 PID: 1139 at kernel/softirq.c:178 > > __local_bh_enable_ip+0xa5/0xf0 > > CPU: 1 PID: 1139 Comm: NetworkManager Not tainted > > 5.12.0-rc1-4-gb4ded168af79 #7 > > Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) > > 05/31/2017 > > RIP: 0010:__local_bh_enable_ip+0xa5/0xf0 > > Code: f7 69 e8 ee 23 14 00 fb 66 0f 1f 44 00 00 65 8b 05 f0 f4 f7 69 85 c0 > > 74 3f 48 83 c4 08 5b c3 65 8b 05 9b fe f7 69 85 c0 75 8e <0f> 0b eb 8a 48 > > 89 3c 24 e8 4e 20 14 00 48 8b 3c 24 eb 91 e8 13 4e > > RSP: 0018:afd580b13298 EFLAGS: 00010046 > > RAX: RBX: 0201 RCX: > > RDX: 0003 RSI: 0201 RDI: c1272389 > > RBP: 96517ae4c018 R08: 0001 R09: > > R10: afd580b13178 R11: 0001 R12: 96517b06 > > R13: R14: 8000 R15: 0001 > > FS: 7fc604ebefc0() GS:96526748() > > knlGS: > > CS: 0010 DS: ES: CR0: 80050033 > > CR2: 55fb3fef13b2 CR3: 000109112004 CR4: 003706e0 > > Call Trace: > > ? _raw_spin_unlock_bh+0x1f/0x30 > > iwl_pcie_enqueue_hcmd+0x5d9/0xa00 [iwlwifi] > > iwl_trans_txq_send_hcmd+0x6c/0x430 [iwlwifi] > > iwl_trans_send_cmd+0x88/0x170 [iwlwifi] > > ? lock_acquire+0x277/0x3d0 > > iwl_mvm_send_cmd+0x32/0x80 [iwlmvm] > > iwl_mvm_led_set+0xc2/0xe0 [iwlmvm] > > ? led_trigger_event+0x46/0x70 > > led_trigger_event+0x46/0x70 > > ieee80211_do_open+0x5c5/0xa20 [mac80211] > > ieee80211_open+0x67/0x90 [mac80211] > > __dev_open+0xd4/0x150 > > __dev_change_flags+0x19e/0x1f0 > > dev_change_flags+0x23/0x60 > > do_setlink+0x30d/0x1230 > > ? lock_is_held_type+0xb4/0x120 > > ? __nla_validate_parse.part.7+0x57/0xcb0 > > ? __lock_acquire+0x2e1/0x1a50 > > __rtnl_newlink+0x560/0x910 > > ? __lock_acquire+0x2e1/0x1a50 > > ? __lock_acquire+0x2e1/0x1a50 > > ? lock_acquire+0x277/0x3d0 > > ? sock_def_readable+0x5/0x290 > > ? lock_is_held_type+0xb4/0x120 > > ? find_held_lock+0x2d/0x90 > > ? sock_def_readable+0xb3/0x290 > > ? lock_release+0x166/0x2a0 > > ? lock_is_held_type+0x90/0x120 > > rtnl_newlink+0x47/0x70 > > rtnetlink_rcv_msg+0x25c/0x470 > > ? netlink_deliver_tap+0x97/0x3e0 > > ? validate_linkmsg+0x350/0x350 > > netlink_rcv_skb+0x50/0x100 > > netlink_unicast+0x1b2/0x280 > > netlink_sendmsg+0x336/0x450 > > sock_sendmsg+0x5b/0x60 > > sys_sendmsg+0x1ed/0x250 > > ? copy_msghdr_from_user+0x5c/0x90 > > ___sys_sendmsg+0x88/0xd0 > > ? lock_is_held_type+0xb4/0x120 > > ? find_held_lock+0x2d/0x90 > > ? lock_release+0x166/0x2a0 > > ? __fget_files+0xfe/0x1d0 > > ? __sys_sendmsg+0x5e/0xa0 > > __sys_sendmsg+0x5e/0xa0 > > ? lockdep_hardirqs_on_prepare+0xd9/0x170 > > do_syscall_64+0x33/0x80 > > entry_SYSCALL_64_after_hwframe+0x44/0xae > > RIP: 0033:0x7fc605c9572d > > Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 da ee ff ff 8b 54 24 1c > > 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff > > ff 77 33 44 89 c7 48 89 44 24 08 e8 2e ef ff ff 48 > > RSP: 002b:7fffc83789f0 EFLAGS: 0293 ORIG_RAX: 002e > > RAX: ffda RBX: 55ef468570c0 RCX: 7fc605c9572d > > RDX: RSI: 7fffc8378a30 RDI: 000c > > RBP: 0010 R08: R09: 0000 > > R10: R11: 0293 R12: > > R13: 7fffc8378b80 R14: 7fffc8378b7c R15: > > irq event stamp: 170785 > > hardirqs last enabled at (170783): [] > > __local_bh_enable_ip+0x82/0xf0 > > hardirqs last disabled at (170784): [] > >
Re: [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition
On Mon, 8 Mar 2021, Srinivas Pandruvada wrote: > > > A remove callback is only ever called for a bound device. So there > > > is no > > > need to check for device or driver being NULL. > > > > Srinivas, any objections to this patchset? The cleanups look good to > > me. > Sorry, I missed this series. > No objection for taking these patches. Thanks. Applied with your Acked-by: If you disagree with that interpretation of your statement above, please holler :) Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: wacom: Assign boolean values to a bool variable
On Wed, 20 Jan 2021, Jiapeng Zhong wrote:
> Fix the following coccicheck warnings:
>
> ./drivers/hid/wacom_wac.c:2536:2-6: WARNING: Assignment of
> 0/1 to bool variable.
>
> Reported-by: Abaci Robot
> Signed-off-by: Jiapeng Zhong
> ---
> 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 1bd0eb7..62b0f71 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -2533,7 +2533,7 @@ static void wacom_wac_finger_slot(struct wacom_wac
> *wacom_wac,
> !wacom_wac->shared->is_touch_on) {
> if (!wacom_wac->shared->touch_down)
> return;
> - prox = 0;
> + prox = false;
> }
Applied, thanks.
--
Jiri Kosina
SUSE Labs
Re: [PATCH] hid: hid-alps: fix error return code in alps_input_configured()
On Thu, 4 Mar 2021, Jia-Ju Bai wrote:
> When input_register_device() fails, no error return code is assigned.
> To fix this bug, ret is assigned with -ENOENT as error return code.
>
> Reported-by: TOTE Robot
> Signed-off-by: Jia-Ju Bai
> ---
> drivers/hid/hid-alps.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hid/hid-alps.c b/drivers/hid/hid-alps.c
> index 3feaece13ade..6b665931147d 100644
> --- a/drivers/hid/hid-alps.c
> +++ b/drivers/hid/hid-alps.c
> @@ -761,6 +761,7 @@ static int alps_input_configured(struct hid_device *hdev,
> struct hid_input *hi)
>
> if (input_register_device(data->input2)) {
> input_free_device(input2);
> + ret = -ENOENT;
> goto exit;
> }
> }
Applied, thanks.
--
Jiri Kosina
SUSE Labs
Re: [PATCH] HID: intel_ish-hid: HBM: Use connected standby state bit during suspend/resume
On Wed, 3 Mar 2021, Ye Xiang wrote: > ISH firmware uses connected standby state bit (CONNECTED_STANDBY_STATE_BIT > bit 1) > to notify current power state to sensors instead of suspend state bit (bit 0). > So send both SUSPEND_STATE_BIT and CONNECTED_STANDBY_STATE_BIT to firmware > to be compatible with the previous version. Could you please make the changelog more verbose -- namely what user-visible issue this is fixing? Thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH 0/4] Stylus-on-touchscreen device support
On Wed, 17 Feb 2021, наб wrote: > This patchset adds support for stylus-on-touchscreen devices as found on > the OneMix 3 Pro and Dell Inspiron 15 7000 2-in-1 (7591), among others; > with it, they properly behave like a drawing tablet. > > Patches 2 and 4 funxionally depend on patch 1. > Patch 4 needs patch 3 to apply. > > The output of this patchset and the need for a kernel, rather than > userspace, patch was previously discussed here: > > https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/558#note_792834 > > Ahelenia Ziemiańska (4): > HID: multitouch: require Finger field to mark Win8 reports as MT > HID: multitouch: set Stylus suffix for Stylus-application devices, too > HID: input: replace outdated HID numbers+comments with macros > HID: input: work around Win8 stylus-on-touchscreen reporting > > drivers/hid/hid-input.c | 47 +--- > drivers/hid/hid-multitouch.c | 18 -- > 2 files changed, 55 insertions(+), 10 deletions(-) Benjamin, this patchset looks good to me; do you have any objections on queuing it for 5.13? Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: plantronics: Workaround for double volume key presses
On Sun, 7 Feb 2021, Maxim Mikityanskiy wrote:
> Plantronics Blackwire 3220 Series (047f:c056) sends HID reports twice
> for each volume key press. This patch adds a quirk to hid-plantronics
> for this product ID, which will ignore the second volume key press if
> it happens within 5 ms from the last one that was handled.
>
> The patch was tested on the mentioned model only, it shouldn't affect
> other models, however, this quirk might be needed for them too.
> Auto-repeat (when a key is held pressed) is not affected, because the
> rate is about 3 times per second, which is far less frequent than once
> in 5 ms.
>
> Fixes: 81bb773faed7 ("HID: plantronics: Update to map volume up/down
> controls")
> Signed-off-by: Maxim Mikityanskiy
> ---
> People from Plantronics, maybe you could advise on a better fix than
> filtering duplicate events on driver level? Do you happen to know why
> they occur in the first place? Are any other headsets affected?
Makes one wonder how the Windows driver is dealing with this indeed.
Anyway, as there doesn't seem to be better cure available for now, I have
applied the patch. Thanks,
--
Jiri Kosina
SUSE Labs
Re: [PATCH v1 1/3] HID: intel-ish-hid: Drop if block with an always false condition
On Sat, 6 Feb 2021, Uwe Kleine-König wrote: > A remove callback is only ever called for a bound device. So there is no > need to check for device or driver being NULL. Srinivas, any objections to this patchset? The cleanups look good to me. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: google: add don USB id
On Tue, 2 Mar 2021, Shou-Chieh Hsu wrote:
> Add 1 additional hammer-like device.
>
> Signed-off-by: Shou-Chieh Hsu
> ---
>
> drivers/hid/hid-google-hammer.c | 2 ++
> drivers/hid/hid-ids.h | 1 +
> 2 files changed, 3 insertions(+)
>
> diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
> index d9319622da44..e60c31dd05ff 100644
> --- a/drivers/hid/hid-google-hammer.c
> +++ b/drivers/hid/hid-google-hammer.c
> @@ -573,6 +573,8 @@ static void hammer_remove(struct hid_device *hdev)
> }
>
> static const struct hid_device_id hammer_devices[] = {
> + { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
> + USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_DON) },
> { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
>USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_HAMMER) },
> { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index e42aaae3138f..0b4929258478 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -493,6 +493,7 @@
> #define USB_DEVICE_ID_GOOGLE_MASTERBALL 0x503c
> #define USB_DEVICE_ID_GOOGLE_MAGNEMITE 0x503d
> #define USB_DEVICE_ID_GOOGLE_MOONBALL0x5044
> +#define USB_DEVICE_ID_GOOGLE_DON 0x5050
>
> #define USB_VENDOR_ID_GOTOP 0x08f2
> #define USB_DEVICE_ID_SUPER_Q2 0x007f
Applied, thanks.
--
Jiri Kosina
SUSE Labs
Re: [PATCH v2 4/4] HID: i2c-hid: acpi: Drop redundant ACPI_PTR()
On Mon, 1 Mar 2021, Andy Shevchenko wrote: > > > > The driver depends on ACPI, ACPI_PTR() resolution is always the same. > > > > Otherwise a compiler may produce a warning. > > > > > > > > That said, the rule of thumb either ugly ifdeffery with ACPI_PTR or > > > > none should be used in a driver. > > > > > > > > Signed-off-by: Andy Shevchenko > > > > > > Thanks a lot for the series. This indeed cleans things up. > > > > Indeed, thanks. > > > > > For the series: > > > Acked-by: Benjamin Tissoires > > > > > > Jiri, I wonder where we want to land this one. This is not strictly > > > bug fixes, but we could definitively sneak this one in 5.12-rc1. > > > Well, I should probably run the series on an acpi laptop here before > > > merging, but I'd like to know if delaying to 5.13 is OK or if we need > > > this in 5.12. > > > > I'd like to do it the standard way and have it bake in for-next to see if > > it really doesn't break anything, so unless there are convicing arguments > > for 5.12-rcX, I'd rathre queue this for 5.13. > > For the record, I'm not in hurry with this, up to you how to proceed. > Thanks! Queued in for-5.13/i2c-hid. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()
On Tue, 2 Mar 2021, Jiri Kosina wrote: > From: Jiri Kosina > > It's possible for iwl_pcie_enqueue_hcmd() to be called with hard IRQs > disabled (e.g. from LED core). We can't enable BHs in such a situation. > > Turn the unconditional BH-enable/BH-disable code into > hardirq-disable/conditional-enable. > > This fixes the warning below. Hi, friendly ping on this one ... Thanks! > > WARNING: CPU: 1 PID: 1139 at kernel/softirq.c:178 > __local_bh_enable_ip+0xa5/0xf0 > CPU: 1 PID: 1139 Comm: NetworkManager Not tainted > 5.12.0-rc1-4-gb4ded168af79 #7 > Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017 > RIP: 0010:__local_bh_enable_ip+0xa5/0xf0 > Code: f7 69 e8 ee 23 14 00 fb 66 0f 1f 44 00 00 65 8b 05 f0 f4 f7 69 85 c0 > 74 3f 48 83 c4 08 5b c3 65 8b 05 9b fe f7 69 85 c0 75 8e <0f> 0b eb 8a 48 89 > 3c 24 e8 4e 20 14 00 48 8b 3c 24 eb 91 e8 13 4e > RSP: 0018:afd580b13298 EFLAGS: 00010046 > RAX: RBX: 0201 RCX: > RDX: 0003 RSI: 0201 RDI: c1272389 > RBP: 96517ae4c018 R08: 0001 R09: > R10: afd580b13178 R11: 0001 R12: 96517b06 > R13: R14: 8000 R15: 0001 > FS: 7fc604ebefc0() GS:96526748() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 55fb3fef13b2 CR3: 000109112004 CR4: 003706e0 > Call Trace: > ? _raw_spin_unlock_bh+0x1f/0x30 > iwl_pcie_enqueue_hcmd+0x5d9/0xa00 [iwlwifi] > iwl_trans_txq_send_hcmd+0x6c/0x430 [iwlwifi] > iwl_trans_send_cmd+0x88/0x170 [iwlwifi] > ? lock_acquire+0x277/0x3d0 > iwl_mvm_send_cmd+0x32/0x80 [iwlmvm] > iwl_mvm_led_set+0xc2/0xe0 [iwlmvm] > ? led_trigger_event+0x46/0x70 > led_trigger_event+0x46/0x70 > ieee80211_do_open+0x5c5/0xa20 [mac80211] > ieee80211_open+0x67/0x90 [mac80211] > __dev_open+0xd4/0x150 > __dev_change_flags+0x19e/0x1f0 > dev_change_flags+0x23/0x60 > do_setlink+0x30d/0x1230 > ? lock_is_held_type+0xb4/0x120 > ? __nla_validate_parse.part.7+0x57/0xcb0 > ? __lock_acquire+0x2e1/0x1a50 > __rtnl_newlink+0x560/0x910 > ? __lock_acquire+0x2e1/0x1a50 > ? __lock_acquire+0x2e1/0x1a50 > ? lock_acquire+0x277/0x3d0 > ? sock_def_readable+0x5/0x290 > ? lock_is_held_type+0xb4/0x120 > ? find_held_lock+0x2d/0x90 > ? sock_def_readable+0xb3/0x290 > ? lock_release+0x166/0x2a0 > ? lock_is_held_type+0x90/0x120 > rtnl_newlink+0x47/0x70 > rtnetlink_rcv_msg+0x25c/0x470 > ? netlink_deliver_tap+0x97/0x3e0 > ? validate_linkmsg+0x350/0x350 > netlink_rcv_skb+0x50/0x100 > netlink_unicast+0x1b2/0x280 > netlink_sendmsg+0x336/0x450 > sock_sendmsg+0x5b/0x60 > sys_sendmsg+0x1ed/0x250 > ? copy_msghdr_from_user+0x5c/0x90 > ___sys_sendmsg+0x88/0xd0 > ? lock_is_held_type+0xb4/0x120 > ? find_held_lock+0x2d/0x90 > ? lock_release+0x166/0x2a0 > ? __fget_files+0xfe/0x1d0 > ? __sys_sendmsg+0x5e/0xa0 > __sys_sendmsg+0x5e/0xa0 > ? lockdep_hardirqs_on_prepare+0xd9/0x170 > do_syscall_64+0x33/0x80 > entry_SYSCALL_64_after_hwframe+0x44/0xae > RIP: 0033:0x7fc605c9572d > Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 da ee ff ff 8b 54 24 1c > 48 8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff > 77 33 44 89 c7 48 89 44 24 08 e8 2e ef ff ff 48 > RSP: 002b:7fffc83789f0 EFLAGS: 0293 ORIG_RAX: 002e > RAX: ffda RBX: 55ef468570c0 RCX: 7fc605c9572d > RDX: RSI: 7fffc8378a30 RDI: 000c > RBP: 0010 R08: R09: > R10: R11: 0293 R12: > R13: 7fffc8378b80 R14: 7fffc8378b7c R15: > irq event stamp: 170785 > hardirqs last enabled at (170783): [] > __local_bh_enable_ip+0x82/0xf0 > hardirqs last disabled at (170784): [] > _raw_read_lock_irqsave+0x8d/0x90 > softirqs last enabled at (170782): [] > iwl_pcie_enqueue_hcmd+0x5d9/0xa00 [iwlwifi] > softirqs last disabled at (170785): [] > iwl_pcie_enqueue_hcmd+0x116/0xa00 [iwlwifi] > > Signed-off-by: Jiri Kosina > --- > drivers/net/wireless/intel/iwlwifi/pcie/tx.c | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/tx.c > b/drivers/net/wireless/intel/iwlwifi/pcie/tx.c > index fb3d2f6299aa..0b35bf258fad 100644 > --- a/drivers/net/wireless/intel/iwlwifi/pcie/tx.c > +++ b/drivers/net/wireless/intel/iwlwifi/pcie/tx.c > @@ -928,6 +928,7 @@ int iwl_pcie_enqueue_hcmd(struct iwl
Lockdep report for hci_conn_get_phy()
Hi,
I am getting the lockdep splat below with current Linus' tree (5.12-rc1).
I haven't yet analyzed the dependency chain and the code, but sending out
early in case someone has seen this before and analyzed / fixed it
already.
Thanks.
==
WARNING: possible circular locking dependency detected
5.12.0-rc1-00026-g73d464503354 #10 Not tainted
--
bluetoothd/1118 is trying to acquire lock:
8f078383c078 (>lock){+.+.}-{3:3}, at: hci_conn_get_phy+0x1c/0x150
[bluetooth]
but task is already holding lock:
8f07e831d920 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at:
l2cap_sock_getsockopt+0x8b/0x610
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #3 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}:
lock_sock_nested+0x72/0xa0
l2cap_sock_ready_cb+0x18/0x70 [bluetooth]
l2cap_config_rsp+0x27a/0x520 [bluetooth]
l2cap_sig_channel+0x658/0x1330 [bluetooth]
l2cap_recv_frame+0x1ba/0x310 [bluetooth]
hci_rx_work+0x1cc/0x640 [bluetooth]
process_one_work+0x244/0x5f0
worker_thread+0x3c/0x380
kthread+0x13e/0x160
ret_from_fork+0x22/0x30
-> #2 (>lock#2/1){+.+.}-{3:3}:
__mutex_lock+0xa3/0xa10
l2cap_chan_connect+0x33a/0x940 [bluetooth]
l2cap_sock_connect+0x141/0x2a0 [bluetooth]
__sys_connect+0x9b/0xc0
__x64_sys_connect+0x16/0x20
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
-> #1 (>chan_lock){+.+.}-{3:3}:
__mutex_lock+0xa3/0xa10
l2cap_chan_connect+0x322/0x940 [bluetooth]
l2cap_sock_connect+0x141/0x2a0 [bluetooth]
__sys_connect+0x9b/0xc0
__x64_sys_connect+0x16/0x20
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
-> #0 (>lock){+.+.}-{3:3}:
__lock_acquire+0x147a/0x1a50
lock_acquire+0x277/0x3d0
__mutex_lock+0xa3/0xa10
hci_conn_get_phy+0x1c/0x150 [bluetooth]
l2cap_sock_getsockopt+0x5a9/0x610 [bluetooth]
__sys_getsockopt+0xcc/0x200
__x64_sys_getsockopt+0x20/0x30
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
other info that might help us debug this:
Chain exists of:
>lock --> >lock#2/1 --> sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP
Possible unsafe locking scenario:
CPU0CPU1
lock(sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP);
lock(>lock#2/1);
lock(sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP);
lock(>lock);
*** DEADLOCK ***
1 lock held by bluetoothd/1118:
#0: 8f07e831d920 (sk_lock-AF_BLUETOOTH-BTPROTO_L2CAP){+.+.}-{0:0}, at:
l2cap_sock_getsockopt+0x8b/0x610 [bluetooth]
stack backtrace:
CPU: 3 PID: 1118 Comm: bluetoothd Not tainted 5.12.0-rc1-00026-g73d464503354
#10
Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
Call Trace:
dump_stack+0x7f/0xa1
check_noncircular+0x105/0x120
? __lock_acquire+0x147a/0x1a50
__lock_acquire+0x147a/0x1a50
lock_acquire+0x277/0x3d0
? hci_conn_get_phy+0x1c/0x150 [bluetooth]
? __lock_acquire+0x2e1/0x1a50
? lock_is_held_type+0xb4/0x120
? hci_conn_get_phy+0x1c/0x150 [bluetooth]
__mutex_lock+0xa3/0xa10
? hci_conn_get_phy+0x1c/0x150 [bluetooth]
? lock_acquire+0x277/0x3d0
? mark_held_locks+0x49/0x70
? mark_held_locks+0x49/0x70
? hci_conn_get_phy+0x1c/0x150 [bluetooth]
hci_conn_get_phy+0x1c/0x150 [bluetooth]
l2cap_sock_getsockopt+0x5a9/0x610 [bluetooth]
__sys_getsockopt+0xcc/0x200
__x64_sys_getsockopt+0x20/0x30
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7fb73df33eee
Code: 48 8b 0d 85 0f 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00
00 00 00 90 f3 0f 1e fa 49 89 ca b8 37 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01
c3 48 8b 0d 52 0f 0c 00 f7 d8 64 89 01 48
RSP: 002b:7fffcfbbbf08 EFLAGS: 0203 ORIG_RAX: 0037
RAX: ffda RBX: 0019 RCX: 7fb73df33eee
RDX: 000e RSI: 0112 RDI: 0018
RBP: R08: 7fffcfbbbf44 R09:
R10: 7fffcfbbbf3c R11: 0203 R12:
R13: 0018 R14: 0000 R15: 556fcefc70d0
--
Jiri Kosina
SUSE Labs
Re: [PATCH v2] iwlwifi: don't call netif_napi_add() with rxq->lock held (was Re: Lockdep warning in iwl_pcie_rx_handle())
On Wed, 3 Mar 2021, Kalle Valo wrote: > > ... i believe you want to drop the "(was ...") part from the patch > > subject. > > Too late now, it's already applied and pull request sent. Why was it > there in the first place? Yeah, it was, but I don't think it's a big issue :) So let it be. BTW, how about the other fix I sent? It's also fixing a real functional issue, so it IMHO is a -rc material https://lore.kernel.org/linux-wireless/nycvar.yfh.7.76.2103021125430.12...@cbobk.fhfr.pm/ Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v2] iwlwifi: don't call netif_napi_add() with rxq->lock held (was Re: Lockdep warning in iwl_pcie_rx_handle())
On Wed, 3 Mar 2021, Kalle Valo wrote: > Patch applied to wireless-drivers.git, thanks. Thanks, but ... > 295d4cd82b01 iwlwifi: don't call netif_napi_add() with rxq->lock held (was > Re: Lockdep warning in iwl_pcie_rx_handle()) ... i believe you want to drop the "(was ...") part from the patch subject. -- Jiri Kosina SUSE Labs
Re: [PATCH v2] iwlwifi: don't call netif_napi_add() with rxq->lock held (was Re: Lockdep warning in iwl_pcie_rx_handle())
On Tue, 2 Mar 2021, Kalle Valo wrote: > > Thanks, Jiri! Let's take your patch since you already sent it out. > > > > Kalle, can you please take this directly to wireless-drivers.git? > > > > Acked-by: Luca Coelho > > Ok but I don't see this either in patchwork or lore, hopefully it shows > up later. Not sure about patchwork, but vger had hiccup (again) earlier today, everything depending on the ML traffic is probably slower. lore has it now though: https://lore.kernel.org/lkml/nycvar.yfh.7.76.2103021134060.12...@cbobk.fhfr.pm/ Thanks, -- Jiri Kosina SUSE Labs
[PATCH v2] iwlwifi: don't call netif_napi_add() with rxq->lock held (was Re: Lockdep warning in iwl_pcie_rx_handle())
From: Jiri Kosina
We can't call netif_napi_add() with rxq-lock held, as there is a potential
for deadlock as spotted by lockdep (see below). rxq->lock is not
protecting anything over the netif_napi_add() codepath anyway, so let's
drop it just before calling into NAPI.
WARNING: possible irq lock inversion dependency detected
5.12.0-rc1-2-gbada49429032 #5 Not tainted
irq/136-iwlwifi/565 just changed the state of lock:
89f28433b0b0 (>lock){+.-.}-{2:2}, at: iwl_pcie_rx_handle+0x7f/0x960
[iwlwifi]
but this lock took another, SOFTIRQ-unsafe lock in the past:
(napi_hash_lock){+.+.}-{2:2}
and interrupts could create inverse lock ordering between them.
other info that might help us debug this:
Possible interrupt unsafe locking scenario:
CPU0CPU1
lock(napi_hash_lock);
local_irq_disable();
lock(>lock);
lock(napi_hash_lock);
lock(>lock);
*** DEADLOCK ***
1 lock held by irq/136-iwlwifi/565:
#0: 89f2b1440170 (sync_cmd_lockdep_map){+.+.}-{0:0}, at:
iwl_pcie_irq_handler+0x5/0xb30
the shortest dependencies between 2nd lock and 1st lock:
-> (napi_hash_lock){+.+.}-{2:2} {
HARDIRQ-ON-W at:
lock_acquire+0x277/0x3d0
_raw_spin_lock+0x2c/0x40
netif_napi_add+0x14b/0x270
e1000_probe+0x2fe/0xee0 [e1000e]
local_pci_probe+0x42/0x90
pci_device_probe+0x10b/0x1c0
really_probe+0xef/0x4b0
driver_probe_device+0xde/0x150
device_driver_attach+0x4f/0x60
__driver_attach+0x9c/0x140
bus_for_each_dev+0x79/0xc0
bus_add_driver+0x18d/0x220
driver_register+0x5b/0xf0
do_one_initcall+0x5b/0x300
do_init_module+0x5b/0x21c
load_module+0x1dae/0x22c0
__do_sys_finit_module+0xad/0x110
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
SOFTIRQ-ON-W at:
lock_acquire+0x277/0x3d0
_raw_spin_lock+0x2c/0x40
netif_napi_add+0x14b/0x270
e1000_probe+0x2fe/0xee0 [e1000e]
local_pci_probe+0x42/0x90
pci_device_probe+0x10b/0x1c0
really_probe+0xef/0x4b0
driver_probe_device+0xde/0x150
device_driver_attach+0x4f/0x60
__driver_attach+0x9c/0x140
bus_for_each_dev+0x79/0xc0
bus_add_driver+0x18d/0x220
driver_register+0x5b/0xf0
do_one_initcall+0x5b/0x300
do_init_module+0x5b/0x21c
load_module+0x1dae/0x22c0
__do_sys_finit_module+0xad/0x110
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
INITIAL USE at:
lock_acquire+0x277/0x3d0
_raw_spin_lock+0x2c/0x40
netif_napi_add+0x14b/0x270
e1000_probe+0x2fe/0xee0 [e1000e]
local_pci_probe+0x42/0x90
pci_device_probe+0x10b/0x1c0
really_probe+0xef/0x4b0
driver_probe_device+0xde/0x150
device_driver_attach+0x4f/0x60
__driver_attach+0x9c/0x140
bus_for_each_dev+0x79/0xc0
bus_add_driver+0x18d/0x220
driver_register+0x5b/0xf0
do_one_initcall+0x5b/0x300
do_init_module+0x5b/0x21c
load_module+0x1dae/0x22c0
__do_sys_finit_module+0xad/0x110
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
}
... key at: [] napi_hash_lock+0x18/0x40
... acquired at:
_raw_spin_lock+0x2c/0x40
netif_napi_add+0x14b/0x270
_iwl_pcie_rx_init+0x1f4/0x710 [iwlwifi]
iwl_pcie_rx_init+0x1b/0x3b0 [iwlwifi]
iwl_trans_pcie_start_fw+0x2ac/0x6a0 [iwlwifi]
iwl_mvm_load_ucode_wait_alive+0x116/0x460 [iwlmvm]
iwl_run_init_mvm_ucode+0xa4/0x3a0 [iwlmvm]
iwl_op_mode_mvm_start+0x9ed/0xbf0 [iwlmvm]
_iwl_op_mode_start.isra.4+0x42/0x80 [iwlwifi]
iwl_opmode_register+0x71/0xe0 [iwlwifi]
iwl_mvm_init+0x34/0x1000 [iwlmvm]
do_one_initcall+0x5b/0x300
do_init_module+0x5b/0x21c
loa
[PATCH] iwlwifi: Fix softirq/hardirq disabling in iwl_pcie_enqueue_hcmd()
From: Jiri Kosina
It's possible for iwl_pcie_enqueue_hcmd() to be called with hard IRQs
disabled (e.g. from LED core). We can't enable BHs in such a situation.
Turn the unconditional BH-enable/BH-disable code into
hardirq-disable/conditional-enable.
This fixes the warning below.
WARNING: CPU: 1 PID: 1139 at kernel/softirq.c:178
__local_bh_enable_ip+0xa5/0xf0
CPU: 1 PID: 1139 Comm: NetworkManager Not tainted
5.12.0-rc1-4-gb4ded168af79 #7
Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16 ) 05/31/2017
RIP: 0010:__local_bh_enable_ip+0xa5/0xf0
Code: f7 69 e8 ee 23 14 00 fb 66 0f 1f 44 00 00 65 8b 05 f0 f4 f7 69 85 c0 74
3f 48 83 c4 08 5b c3 65 8b 05 9b fe f7 69 85 c0 75 8e <0f> 0b eb 8a 48 89 3c 24
e8 4e 20 14 00 48 8b 3c 24 eb 91 e8 13 4e
RSP: 0018:afd580b13298 EFLAGS: 00010046
RAX: RBX: 0201 RCX:
RDX: 0003 RSI: 0201 RDI: c1272389
RBP: 96517ae4c018 R08: 0001 R09:
R10: afd580b13178 R11: 0001 R12: 96517b06
R13: R14: 8000 R15: 0001
FS: 7fc604ebefc0() GS:96526748() knlGS:
CS: 0010 DS: ES: CR0: 80050033
CR2: 55fb3fef13b2 CR3: 000109112004 CR4: 003706e0
Call Trace:
? _raw_spin_unlock_bh+0x1f/0x30
iwl_pcie_enqueue_hcmd+0x5d9/0xa00 [iwlwifi]
iwl_trans_txq_send_hcmd+0x6c/0x430 [iwlwifi]
iwl_trans_send_cmd+0x88/0x170 [iwlwifi]
? lock_acquire+0x277/0x3d0
iwl_mvm_send_cmd+0x32/0x80 [iwlmvm]
iwl_mvm_led_set+0xc2/0xe0 [iwlmvm]
? led_trigger_event+0x46/0x70
led_trigger_event+0x46/0x70
ieee80211_do_open+0x5c5/0xa20 [mac80211]
ieee80211_open+0x67/0x90 [mac80211]
__dev_open+0xd4/0x150
__dev_change_flags+0x19e/0x1f0
dev_change_flags+0x23/0x60
do_setlink+0x30d/0x1230
? lock_is_held_type+0xb4/0x120
? __nla_validate_parse.part.7+0x57/0xcb0
? __lock_acquire+0x2e1/0x1a50
__rtnl_newlink+0x560/0x910
? __lock_acquire+0x2e1/0x1a50
? __lock_acquire+0x2e1/0x1a50
? lock_acquire+0x277/0x3d0
? sock_def_readable+0x5/0x290
? lock_is_held_type+0xb4/0x120
? find_held_lock+0x2d/0x90
? sock_def_readable+0xb3/0x290
? lock_release+0x166/0x2a0
? lock_is_held_type+0x90/0x120
rtnl_newlink+0x47/0x70
rtnetlink_rcv_msg+0x25c/0x470
? netlink_deliver_tap+0x97/0x3e0
? validate_linkmsg+0x350/0x350
netlink_rcv_skb+0x50/0x100
netlink_unicast+0x1b2/0x280
netlink_sendmsg+0x336/0x450
sock_sendmsg+0x5b/0x60
sys_sendmsg+0x1ed/0x250
? copy_msghdr_from_user+0x5c/0x90
___sys_sendmsg+0x88/0xd0
? lock_is_held_type+0xb4/0x120
? find_held_lock+0x2d/0x90
? lock_release+0x166/0x2a0
? __fget_files+0xfe/0x1d0
? __sys_sendmsg+0x5e/0xa0
__sys_sendmsg+0x5e/0xa0
? lockdep_hardirqs_on_prepare+0xd9/0x170
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x7fc605c9572d
Code: 28 89 54 24 1c 48 89 74 24 10 89 7c 24 08 e8 da ee ff ff 8b 54 24 1c 48
8b 74 24 10 41 89 c0 8b 7c 24 08 b8 2e 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 33
44 89 c7 48 89 44 24 08 e8 2e ef ff ff 48
RSP: 002b:7fffc83789f0 EFLAGS: 0293 ORIG_RAX: 002e
RAX: ffda RBX: 55ef468570c0 RCX: 7fc605c9572d
RDX: RSI: 7fffc8378a30 RDI: 000c
RBP: 0010 R08: R09:
R10: R11: 0293 R12:
R13: 7fffc8378b80 R14: 7fffc8378b7c R15:
irq event stamp: 170785
hardirqs last enabled at (170783): []
__local_bh_enable_ip+0x82/0xf0
hardirqs last disabled at (170784): []
_raw_read_lock_irqsave+0x8d/0x90
softirqs last enabled at (170782): []
iwl_pcie_enqueue_hcmd+0x5d9/0xa00 [iwlwifi]
softirqs last disabled at (170785): []
iwl_pcie_enqueue_hcmd+0x116/0xa00 [iwlwifi]
Signed-off-by: Jiri Kosina
---
drivers/net/wireless/intel/iwlwifi/pcie/tx.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/pcie/tx.c
b/drivers/net/wireless/intel/iwlwifi/pcie/tx.c
index fb3d2f6299aa..0b35bf258fad 100644
--- a/drivers/net/wireless/intel/iwlwifi/pcie/tx.c
+++ b/drivers/net/wireless/intel/iwlwifi/pcie/tx.c
@@ -928,6 +928,7 @@ int iwl_pcie_enqueue_hcmd(struct iwl_trans *trans,
u32 cmd_pos;
const u8 *cmddata[IWL_MAX_CMD_TBS_PER_TFD];
u16 cmdlen[IWL_MAX_CMD_TBS_PER_TFD];
+ unsigned long flags;
if (WARN(!trans->wide_cmd_header &&
group_id > IWL_ALWAYS_LONG_GROUP,
@@ -1011,10 +1012,10 @@ int iwl_pcie_enqueue_hcmd(struct iwl_trans *trans,
goto free_dup_buf;
}
- spin_lock_bh(>lock);
+ spin_lock_irqsave(>lock, flags);
if (iwl_txq_space(trans, txq) < ((cmd->flags & CMD_ASYNC) ? 2 : 1)) {
- spin_unlock_bh(>lock);
+
[PATCH] iwlwifi: don't call netif_napi_add() with rxq->lock held (was Re: Lockdep warning in iwl_pcie_rx_handle())
On Mon, 1 Mar 2021, Johannes Berg wrote:
> > I am getting the splat below with Linus' tree as of today (5.11-rc1,
> > fe07bfda2fb). I haven't started to look into the code yet, but apparently
> > this has been already reported by Heiner here:
> >
> > https://www.spinics.net/lists/linux-wireless/msg208353.html
> >
> > so before I start digging deep into it (the previous kernel this
> > particular machine had is 5.9, so I'd rather avoid lenghty bisect for now
> > in case someone has already looked into it and has ideas where the problem
> > is), I thought I'd ask whether this has been root-caused elsewhere
> > already.
>
> Yeah, I'm pretty sure we have a fix for this, though I'm not sure right
> now where it is in the pipeline.
>
> It's called "iwlwifi: pcie: don't add NAPI under rxq->lock" but right
> now I can't find it in any of the public archives.
I was not able to find that patch anywhere indeed, but in the meantime I
fixed it by the patch below. Please consider merging either of the fixes.
Also I am still seeing
WARNING: CPU: 2 PID: 1138 at kernel/softirq.c:178
__local_bh_enable_ip+0xa5/0xf0
on the
[ 21.902173] iwl_pcie_enqueue_hcmd+0x5d9/0xa00 [iwlwifi]
[ 21.906445] iwl_trans_txq_send_hcmd+0x6c/0x430 [iwlwifi]
[ 21.910757] iwl_trans_send_cmd+0x88/0x170 [iwlwifi]
[ 21.915074] ? lock_acquire+0x277/0x3d0
[ 21.919327] iwl_mvm_send_cmd+0x32/0x80 [iwlmvm]
[ 21.923616] iwl_mvm_led_set+0xc2/0xe0 [iwlmvm]
codepath, but I'll look into it separately.
From: Jiri Kosina
Subject: [PATCH] iwlwifi: don't call netif_napi_add() with rxq->lock held
We can't call netif_napi_add() with rxq-lock held, as there is a potential
for deadlock as spotted by lockdep (see below). rxq->lock is not
protecting anything over the netif_napi_add() codepath anyway, so let's
drop it just before calling into NAPI.
WARNING: possible irq lock inversion dependency detected
5.12.0-rc1-2-gbada49429032 #5 Not tainted
irq/136-iwlwifi/565 just changed the state of lock:
89f28433b0b0 (>lock){+.-.}-{2:2}, at: iwl_pcie_rx_handle+0x7f/0x960
[iwlwifi]
but this lock took another, SOFTIRQ-unsafe lock in the past:
(napi_hash_lock){+.+.}-{2:2}
and interrupts could create inverse lock ordering between them.
other info that might help us debug this:
Possible interrupt unsafe locking scenario:
CPU0CPU1
lock(napi_hash_lock);
local_irq_disable();
lock(>lock);
lock(napi_hash_lock);
lock(>lock);
*** DEADLOCK ***
1 lock held by irq/136-iwlwifi/565:
#0: 89f2b1440170 (sync_cmd_lockdep_map){+.+.}-{0:0}, at:
iwl_pcie_irq_handler+0x5/0xb30
the shortest dependencies between 2nd lock and 1st lock:
-> (napi_hash_lock){+.+.}-{2:2} {
HARDIRQ-ON-W at:
lock_acquire+0x277/0x3d0
_raw_spin_lock+0x2c/0x40
netif_napi_add+0x14b/0x270
e1000_probe+0x2fe/0xee0 [e1000e]
local_pci_probe+0x42/0x90
pci_device_probe+0x10b/0x1c0
really_probe+0xef/0x4b0
driver_probe_device+0xde/0x150
device_driver_attach+0x4f/0x60
__driver_attach+0x9c/0x140
bus_for_each_dev+0x79/0xc0
bus_add_driver+0x18d/0x220
driver_register+0x5b/0xf0
do_one_initcall+0x5b/0x300
do_init_module+0x5b/0x21c
load_module+0x1dae/0x22c0
__do_sys_finit_module+0xad/0x110
do_syscall_64+0x33/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae
SOFTIRQ-ON-W at:
lock_acquire+0x277/0x3d0
_raw_spin_lock+0x2c/0x40
netif_napi_add+0x14b/0x270
e1000_probe+0x2fe/0xee0 [e1000e]
local_pci_probe+0x42/0x90
pci_device_probe+0x10b/0x1c0
really_probe+0xef/0x4b0
driver_probe_device+0xde/0x150
device_driver_attach+0x4f/0x60
__driver_attach+0x9c/0x140
bus_for_each_dev+0x79/0xc0
bus_add_driver+0x18d/0x220
driver_register+0x5b/0xf0
do_one_initcall+0x5b/0x300
do_init_module+0x5b/0x21c
load_module+0
Re: Lockdep warning in iwl_pcie_rx_handle()
On Mon, 1 Mar 2021, Jiri Kosina wrote:
> I am getting the splat below with Linus' tree as of today (5.11-rc1,
> fe07bfda2fb). I haven't started to look into the code yet, but apparently
> this has been already reported by Heiner here:
>
> https://www.spinics.net/lists/linux-wireless/msg208353.html
>
> so before I start digging deep into it (the previous kernel this
> particular machine had is 5.9, so I'd rather avoid lenghty bisect for now
> in case someone has already looked into it and has ideas where the problem
> is), I thought I'd ask whether this has been root-caused elsewhere
> already.
>
> Thanks.
After reverting 25edc8f259c7106 ("iwlwifi: pcie: properly implement
NAPI"), I don't see the lockdep warning any more (*), so it seems to be
culprit (or at least related). CCing Johannes.
Leaving the original mail below for reference.
(*) I see the warning below instead; but let's focus on fixing the lockdep
reported issue in the first place, I guess. It might be that I'd need
to revert more things in order to get to a fully consistent state.
[ 23.488194] WARNING: CPU: 1 PID: 1139 at kernel/softirq.c:178
__local_bh_enable_ip+0xa5/0xf0
[ 23.488203] Modules linked in: tun ip6table_mangle ip6table_filter
ip6_tables iptable_mangle xt_DSCP xt_tcpudp xt_conntrack nf_conntrack
nf_defrag_ipv6 libcrc32c nf_defrag_ipv4 iptable_filter ip_tables x_tables
algif_skcipher af_alg bnep dm_crypt hid_generic snd_usb_audio snd_usbmidi_lib
snd_rawmidi uvcvideo videobuf2_vmalloc btusb videobuf2_memops btrtl btbcm
videobuf2_v4l2 btintel videodev videobuf2_common bluetooth ecdh_generic ecc
iwlmvm mac80211 snd_hda_codec_hdmi libarc4 intel_rapl_msr snd_hda_codec_realtek
snd_soc_skl iTCO_wdt iTCO_vendor_support snd_soc_sst_ipc snd_hda_codec_generic
snd_soc_sst_dsp snd_hda_ext_core snd_soc_acpi_intel_match snd_soc_acpi wmi_bmof
snd_soc_core intel_rapl_common x86_pkg_temp_thermal usbhid intel_powerclamp
snd_compress coretemp snd_hda_intel snd_intel_dspcfg kvm_intel snd_hda_codec
iwlwifi snd_hwdep e1000e kvm snd_hda_core ptp irqbypass joydev pcspkr snd_pcm
i2c_i801 pps_core cfg80211 i2c_smbus snd_timer mei_me mei thermal
intel_pch_thermal
[ 23.488334] thinkpad_acpi wmi battery ledtrig_audio platform_profile snd ac
soundcore rfkill tpm_crb tpm_tis tpm_tis_core tpm acpi_pad button nls_iso8859_1
nls_cp437 vfat fat dm_mod fuse rtsx_pci_sdmmc mmc_core crct10dif_pclmul
crc32_pclmul crc32c_intel ghash_clmulni_intel aesni_intel crypto_simd cryptd
i915 i2c_algo_bit drm_kms_helper syscopyarea xhci_pci serio_raw sysfillrect
sysimgblt fb_sys_fops xhci_hcd rtsx_pci usbcore drm video sg msr efivarfs
[ 23.488404] CPU: 1 PID: 1139 Comm: NetworkManager Not tainted
5.12.0-rc1-4-gb4ded168af79 #7
[ 23.488408] Hardware name: LENOVO 20K5S22R00/20K5S22R00, BIOS R0IET38W (1.16
) 05/31/2017
[ 23.488411] RIP: 0010:__local_bh_enable_ip+0xa5/0xf0
[ 23.488416] Code: f7 69 e8 ee 23 14 00 fb 66 0f 1f 44 00 00 65 8b 05 f0 f4
f7 69 85 c0 74 3f 48 83 c4 08 5b c3 65 8b 05 9b fe f7 69 85 c0 75 8e <0f> 0b eb
8a 48 89 3c 24 e8 4e 20 14 00 48 8b 3c 24 eb 91 e8 13 4e
[ 23.488420] RSP: 0018:afd580b13298 EFLAGS: 00010046
[ 23.488424] RAX: RBX: 0201 RCX:
[ 23.488426] RDX: 0003 RSI: 0201 RDI: c1272389
[ 23.488429] RBP: 96517ae4c018 R08: 0001 R09:
[ 23.488431] R10: afd580b13178 R11: 0001 R12: 96517b06
[ 23.488433] R13: R14: 8000 R15: 0001
[ 23.488436] FS: 7fc604ebefc0() GS:96526748()
knlGS:
[ 23.488439] CS: 0010 DS: ES: CR0: 80050033
[ 23.488442] CR2: 55fb3fef13b2 CR3: 000109112004 CR4: 003706e0
[ 23.488444] Call Trace:
[ 23.488447] ? _raw_spin_unlock_bh+0x1f/0x30
[ 23.488453] iwl_pcie_enqueue_hcmd+0x5d9/0xa00 [iwlwifi]
[ 23.488499] iwl_trans_txq_send_hcmd+0x6c/0x430 [iwlwifi]
[ 23.488536] iwl_trans_send_cmd+0x88/0x170 [iwlwifi]
[ 23.488556] ? lock_acquire+0x277/0x3d0
[ 23.488564] iwl_mvm_send_cmd+0x32/0x80 [iwlmvm]
[ 23.488593] iwl_mvm_led_set+0xc2/0xe0 [iwlmvm]
[ 23.488629] ? led_trigger_event+0x46/0x70
[ 23.488633] led_trigger_event+0x46/0x70
[ 23.488643] ieee80211_do_open+0x5c5/0xa20 [mac80211]
[ 23.488719] ieee80211_open+0x67/0x90 [mac80211]
[ 23.488777] __dev_open+0xd4/0x150
[ 23.488790] __dev_change_flags+0x19e/0x1f0
[ 23.488805] dev_change_flags+0x23/0x60
[ 23.488813] do_setlink+0x30d/0x1230
[ 23.488829] ? lock_is_held_type+0xb4/0x120
[ 23.488841] ? __nla_validate_parse.part.7+0x57/0xcb0
[ 23.488855] ? __lock_acquire+0x2e1/0x1a50
[ 23.488870] __rtnl_newlink+0x560/0x910
[ 23.488890] ? __lock_acquire+0x2e1/0x1a50
[ 23.488893] ? __lock_acquire+0x2e1/0x1a50
[ 23.488915] ? lock_acquire+0x277/0x3d0
[
Lockdep warning in iwl_pcie_rx_handle()
Hi,
I am getting the splat below with Linus' tree as of today (5.11-rc1,
fe07bfda2fb). I haven't started to look into the code yet, but apparently
this has been already reported by Heiner here:
https://www.spinics.net/lists/linux-wireless/msg208353.html
so before I start digging deep into it (the previous kernel this
particular machine had is 5.9, so I'd rather avoid lenghty bisect for now
in case someone has already looked into it and has ideas where the problem
is), I thought I'd ask whether this has been root-caused elsewhere
already.
Thanks.
[9.970414]
[9.973157] WARNING: possible irq lock inversion dependency detected
[9.976186] 5.12.0-rc1-2-gbada49429032 #5 Not tainted
[9.979239]
[9.982223] irq/136-iwlwifi/565 just changed the state of lock:
[9.984904] 89f28433b0b0 (>lock){+.-.}-{2:2}, at:
iwl_pcie_rx_handle+0x7f/0x960 [iwlwifi]
[9.987932] but this lock took another, SOFTIRQ-unsafe lock in the past:
[9.990980] (napi_hash_lock){+.+.}-{2:2}
[9.990986]
and interrupts could create inverse lock ordering between them.
[ 10.003955]
other info that might help us debug this:
[ 10.008989] Possible interrupt unsafe locking scenario:
[ 10.013966]CPU0CPU1
[ 10.016404]
[ 10.018976] lock(napi_hash_lock);
[ 10.021529]local_irq_disable();
[ 10.023985]lock(>lock);
[ 10.026340]lock(napi_hash_lock);
[ 10.028644]
[ 10.030934] lock(>lock);
[ 10.033119]
*** DEADLOCK ***
[ 10.039456] 1 lock held by irq/136-iwlwifi/565:
[ 10.041549] #0: 89f2b1440170 (sync_cmd_lockdep_map){+.+.}-{0:0}, at:
iwl_pcie_irq_handler+0x5/0xb30 [iwlwifi]
[ 10.043728]
the shortest dependencies between 2nd lock and 1st lock:
[ 10.047836] -> (napi_hash_lock){+.+.}-{2:2} {
[ 10.049845] HARDIRQ-ON-W at:
[ 10.051860] lock_acquire+0x277/0x3d0
[ 10.053881] _raw_spin_lock+0x2c/0x40
[ 10.056170] netif_napi_add+0x14b/0x270
[ 10.058349] e1000_probe+0x2fe/0xee0 [e1000e]
[ 10.060539] local_pci_probe+0x42/0x90
[ 10.062609] pci_device_probe+0x10b/0x1c0
[ 10.064579] really_probe+0xef/0x4b0
[ 10.066578] driver_probe_device+0xde/0x150
[ 10.068765] device_driver_attach+0x4f/0x60
[ 10.070890] __driver_attach+0x9c/0x140
[ 10.073115] bus_for_each_dev+0x79/0xc0
[ 10.075304] bus_add_driver+0x18d/0x220
[ 10.077464] driver_register+0x5b/0xf0
[ 10.079615] do_one_initcall+0x5b/0x300
[ 10.081686] do_init_module+0x5b/0x21c
[ 10.083730] load_module+0x1dae/0x22c0
[ 10.085751] __do_sys_finit_module+0xad/0x110
[ 10.087799] do_syscall_64+0x33/0x80
[ 10.089758] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 10.091747] SOFTIRQ-ON-W at:
[ 10.093634] lock_acquire+0x277/0x3d0
[ 10.095514] _raw_spin_lock+0x2c/0x40
[ 10.097353] netif_napi_add+0x14b/0x270
[ 10.099208] e1000_probe+0x2fe/0xee0 [e1000e]
[ 10.101032] local_pci_probe+0x42/0x90
[ 10.102835] pci_device_probe+0x10b/0x1c0
[ 10.104648] really_probe+0xef/0x4b0
[ 10.106439] driver_probe_device+0xde/0x150
[ 10.106443] device_driver_attach+0x4f/0x60
[ 10.106446] __driver_attach+0x9c/0x140
[ 10.111451] bus_for_each_dev+0x79/0xc0
[ 10.111454] bus_add_driver+0x18d/0x220
[ 10.111456] driver_register+0x5b/0xf0
[ 10.111459] do_one_initcall+0x5b/0x300
[ 10.111462] do_init_module+0x5b/0x21c
[ 10.111467] load_module+0x1dae/0x22c0
[ 10.111469] __do_sys_finit_module+0xad/0x110
[ 10.122761] do_syscall_64+0x33/0x80
[ 10.122768] entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 10.122772] INITIAL USE at:
[ 10.122775] lock_acquire+0x277/0x3d0
[ 10.128672] _raw_spin_lock+0x2c/0x40
[ 10.130160] netif_napi_add+0x14b/0x270
[ 10.131725] e1000_probe+0x2fe/0xee0 [e1000e]
[ 10.133268]
Re: [PATCH v2 4/4] HID: i2c-hid: acpi: Drop redundant ACPI_PTR()
On Mon, 1 Mar 2021, Benjamin Tissoires wrote: > Hi, > > On Fri, Feb 26, 2021 at 8:34 PM Andy Shevchenko > wrote: > > > > The driver depends on ACPI, ACPI_PTR() resolution is always the same. > > Otherwise a compiler may produce a warning. > > > > That said, the rule of thumb either ugly ifdeffery with ACPI_PTR or > > none should be used in a driver. > > > > Signed-off-by: Andy Shevchenko > > Thanks a lot for the series. This indeed cleans things up. Indeed, thanks. > For the series: > Acked-by: Benjamin Tissoires > > Jiri, I wonder where we want to land this one. This is not strictly > bug fixes, but we could definitively sneak this one in 5.12-rc1. > Well, I should probably run the series on an acpi laptop here before > merging, but I'd like to know if delaying to 5.13 is OK or if we need > this in 5.12. I'd like to do it the standard way and have it bake in for-next to see if it really doesn't break anything, so unless there are convicing arguments for 5.12-rcX, I'd rathre queue this for 5.13. Thanks, -- Jiri Kosina SUSE Labs
[GIT PULL] HID for 5.12
|5 +- Documentation/hid/intel-ish-hid.rst| 78 +- Documentation/hid/uhid.rst | 34 +- MAINTAINERS|6 + arch/arm64/configs/defconfig |3 +- drivers/hid/Kconfig| 19 + drivers/hid/Makefile |3 +- drivers/hid/hid-chicony.c | 55 + drivers/hid/hid-core.c |9 +- drivers/hid/hid-google-hammer.c| 85 +- drivers/hid/hid-ids.h | 11 +- drivers/hid/hid-input.c| 12 + drivers/hid/hid-ite.c | 12 +- drivers/hid/hid-lg-g15.c |2 +- drivers/hid/hid-logitech-dj.c |8 +- drivers/hid/hid-logitech-hidpp.c | 246 +++- drivers/hid/hid-multitouch.c | 10 + drivers/hid/hid-playstation.c | 1351 drivers/hid/hid-quirks.c | 26 +- drivers/hid/hid-roccat-arvo.c |6 +- drivers/hid/hid-sony.c | 20 +- drivers/hid/hid-uclogic-core.c |2 + drivers/hid/hid-uclogic-params.c |2 + drivers/hid/i2c-hid/Kconfig| 47 +- drivers/hid/i2c-hid/Makefile |6 +- drivers/hid/i2c-hid/i2c-hid-acpi.c | 143 +++ drivers/hid/i2c-hid/i2c-hid-core.c | 254 +--- drivers/hid/i2c-hid/i2c-hid-of-goodix.c| 116 ++ drivers/hid/i2c-hid/i2c-hid-of.c | 143 +++ drivers/hid/i2c-hid/i2c-hid.h | 22 + drivers/hid/intel-ish-hid/ipc/hw-ish.h |2 + drivers/hid/intel-ish-hid/ipc/ipc.c| 27 + drivers/hid/intel-ish-hid/ipc/pci-ish.c| 55 +- drivers/hid/wacom_sys.c|2 +- drivers/hid/wacom_wac.c|7 +- include/linux/hid-sensor-hub.h |9 +- include/linux/hid.h| 15 +- include/linux/platform_data/i2c-hid.h | 41 - 44 files changed, 2606 insertions(+), 419 deletions(-) create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7375p.yaml create mode 100644 drivers/hid/hid-playstation.c create mode 100644 drivers/hid/i2c-hid/i2c-hid-acpi.c create mode 100644 drivers/hid/i2c-hid/i2c-hid-of-goodix.c create mode 100644 drivers/hid/i2c-hid/i2c-hid-of.c delete mode 100644 include/linux/platform_data/i2c-hid.h -- Jiri Kosina SUSE Labs
Re: [PATCH 00/11] HID: playstation: revert LED class exposure
On Wed, 17 Feb 2021, Benjamin Tissoires wrote: > [sending those patches on behalf of Roderick] > > There is a current thread on LED LKML which basically means that > we have to revert the LED class exposure until things are settled. > > I am sending here the full series that will end up in linux-next. > But with some git magic, the final PR to Linus will not have the > reverts in it, just the plain patches. > > I am queuing in for-5.12/playstation patches 1 to 6 immediately > (the reverts). > > I am also queuing in for-5.12/playstation-v2 patches 7 and 8 on > top of 51151098d7ab8 immediately. Those 2 patches have already > been reviewed the usual process. > > I am waiting 1 day for others to chime in regarding patches 9 to > 11 before applying them to for-5.12/playstation-v2. They are > basically the same patches that were already reviewed on the > linux-input LKML, but without the LED class bits. FWIW, as discussed elsewhere, I am fully in favor of this solution. Thanks! -- Jiri Kosina SUSE Labs
Re: [PATCH v2 1/3] iio: Add relative sensitivity support
On Fri, 12 Feb 2021, Jonathan Cameron wrote: > > Some hid sensors may use relative sensitivity such as als sensor. > > This patch adds relative sensitivity checking for all hid sensors. > > > > Signed-off-by: Ye Xiang > Hi, > > One totally trivial extra line below. I'll fix that whilst applying > unless you need to respin for some reason. > > I'm fine with the series, but looking for an Ack from Jiri > for the HID header changes. Acked-by: Jiri Kosina Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: Ignore battery for Elan touchscreen on HP Spectre X360 15-df0xxx
On Fri, 22 Jan 2021, Elia Devito wrote: > Battery status is reported for the HP Spectre X360 Convertible 15-df0xxx > even if it does not have a battery. Prevent it to always report the > battery as low. > > Signed-off-by: Elia Devito Applied, thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH v2 0/3] Add quirks to AMD Sensor Fusion Hub driver
On Wed, 27 Jan 2021, m...@richard-neumann.de wrote: > From: Richard Neumann > > This patch adds quirks to the upstream (v8) version of the > AMD Sensor Fusion Hub driver. > The quirks provide a function to detect the sensor mask for systems > that do not have it stored in the AMD_P2C_MSG3 register. > The information about the systems IDs and available sensors was > taken from: https://bugzilla.kernel.org/show_bug.cgi?id=199715 > > Changes since v1: > * Added missing object amd_sfh_quirks.o to amd_sfh-objs > * changed type of "system" in "amd_sfh_quirks_get_sensor_mask" > - struct dmi_system_id -> const struct dmi_system_id Nehal, Sandeep, can you please review this patchset? Thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: logitech-dj: add support for the new lightspeed connection iteration
On Sat, 23 Jan 2021, Filipe Laíns wrote: > From: Filipe Laíns > > This new connection type is the new iteration of the Lightspeed > connection and will probably be used in some of the newer gaming > devices. It is currently use in the G Pro X Superlight. > > This patch should be backported to older versions, as currently the > driver will panic when seing the unsupported connection. This isn't > an issue when using the receiver that came with the device, as Logitech > has been using different PIDs when they change the connection type, but > is an issue when using a generic receiver (well, generic Lightspeed > receiver), which is the case of the one in the Powerplay mat. Currently, > the only generic Ligthspeed receiver we support, and the only one that > exists AFAIK, is ther Powerplay. > > As it stands, the driver will panic when seeing a G Pro X Superlight > connected to the Powerplay receiver and won't send any input events to > userspace! The kernel will warn about this so the issue should be easy > to identify, but it is still very worrying how hard it will fail :( Applied to for-5.11/upstream-fixes, thanks Filipe. -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: intel-ish-hid: ipc: Add Tiger Lake H PCI device ID
On Mon, 8 Feb 2021, Srinivas Pandruvada wrote: > > Added Tiger Lake H PCI device ID to the supported device list. > > > > Signed-off-by: You-Sheng Yang > Acked-by: Srinivas Pandruvada Applied, thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH 1/2] HID: logitech-dj: add support for the new lightspeed receiver iteration
On Sat, 23 Jan 2021, Filipe Laíns wrote: > From: Filipe Laíns > > Tested with the G Pro X Superlight. libratbag sees the device, as > expected, and input events are passing trough. > > https://github.com/libratbag/libratbag/pull/1122 > > The receiver has a quirk where the moused interface doesn't have a > report ID, I am not sure why, perhaps they forgot. All other interfaces > have report IDs so I am left scratching my head. > Since this driver doesn't have a quirk system, I simply implemented it > as a different receiver type, which is true, it just wouldn't be the > prefered approach :P Benjamin, do you have any other idea how to accomplish this without this kind of spaghetti-style conditions? If not, I am tempted to apply the patch as is. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v2] HID: logitech-dj: add support for keyboard events in eQUAD step 4 Gaming
On Fri, 5 Feb 2021, Filipe Laíns wrote:
> From: Filipe Laíns
>
> In e400071a805d6229223a98899e9da8c6233704a1 I added support for the
> receiver that comes with the G602 device, but unfortunately I screwed up
> during testing and it seems the keyboard events were actually not being
> sent to userspace.
> This resulted in keyboard events being broken in userspace, please
> backport the fix.
>
> The receiver uses the normal 0x01 Logitech keyboard report descriptor,
> as expected, so it is just a matter of flagging it as supported.
>
> Reported in
> https://github.com/libratbag/libratbag/issues/1124
>
> Fixes: e400071a805d6 ("HID: logitech-dj: add the G602 receiver")
> Cc:
> Signed-off-by: Filipe Laíns
> ---
>
> Changes in v2:
> - added missing Fixes: anc Cc: tags
Applied, thanks Filipe.
--
Jiri Kosina
SUSE Labs
Re: [PATCH] HID: logitech-dj: add support for keyboard events in eQUAD step 4 Gaming
On Sat, 30 Jan 2021, Filipe Laíns wrote: > From: Filipe Laíns > > In e400071a805d6229223a98899e9da8c6233704a1 I added support for the > receiver that comes with the G602 device, but unfortunately I screwed up > during testing and it seems the keyboard events were actually not being > sent to userspace. > This resulted in keyboard events being broken in userspace, please > backport the fix. > > The receiver uses the normal 0x01 Logitech keyboard report descriptor, > as expected, so it is just a matter of flagging it as supported. > > Reported in > https://github.com/libratbag/libratbag/issues/1124 > > Signed-off-by: Filipe Laíns Given this is a regression, could you please add proper Fixes: and Cc: stable tags? Thank you, -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: displays: convert sysfs sprintf/snprintf family to sysfs_emit
On Tue, 2 Feb 2021, Jiapeng Chong wrote: > Fix the following coccicheck warning: > > ./drivers/hid/hid-roccat-arvo.c:45:8-16: WARNING: use scnprintf or > sprintf. > > ./drivers/hid/hid-roccat-arvo.c:95:8-16: WARNING: use scnprintf or > sprintf. > > ./drivers/hid/hid-roccat-arvo.c:149:8-16: WARNING: use scnprintf or > sprintf. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong Appplied. -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: wacom: convert sysfs sprintf/snprintf family to sysfs_emit
On Tue, 2 Feb 2021, Jiapeng Chong wrote: > Fix the following coccicheck warning: > > ./drivers/hid/wacom_sys.c:1828:8-16: WARNING: use scnprintf or sprintf. > > Reported-by: Abaci Robot > Signed-off-by: Jiapeng Chong > --- > drivers/hid/wacom_sys.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c > index aa9e488..8328ef1 100644 > --- a/drivers/hid/wacom_sys.c > +++ b/drivers/hid/wacom_sys.c > @@ -1825,7 +1825,7 @@ static ssize_t wacom_show_speed(struct device *dev, > struct hid_device *hdev = to_hid_device(dev); > struct wacom *wacom = hid_get_drvdata(hdev); > > - return snprintf(buf, PAGE_SIZE, "%i\n", wacom->wacom_wac.bt_high_speed); > + return sysfs_emit(buf, "%i\n", wacom->wacom_wac.bt_high_speed); > } Applied, thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: lg-g15: make a const array static, makes object smaller
On Thu, 4 Feb 2021, Colin King wrote: > From: Colin Ian King > > Don't populate the const array led_names on the stack but instead make > it static. Makes the object code smaller by 79 bytes: > > Before: >text data bss dec hex filename > 19686 7952 256 278946cf6 drivers/hid/hid-lg-g15.o > > After: >text data bss dec hex filename > 19543 8016 256 278156ca7 drivers/hid/hid-lg-g15.o > > (gcc version 10.2.0) > > Signed-off-by: Colin Ian King Applied. -- Jiri Kosina SUSE Labs
Re: [GIT PULL] Floppy patch for 5.12
On Thu, 4 Feb 2021, Kurt Garloff wrote:
> >> The following changes since commit
> >> 0d7389718c32ad6bb8bee7895c91e2418b6b26aa:
> >>
> >> Merge tag 'nvme-5.21-2020-02-02' of git://git.infradead.org/nvme into
> >> for-5.12/drivers (2021-02-02 07:11:47 -0700)
> >>
> >> are available in the Git repository at:
> >>
> >> https://github.com/evdenis/linux-floppy tags/floppy-for-5.12
> > Pulled, thanks.
>
> Great, thanks!
>
> Next is -stable then ... so all those cloud images using floppy to
> inject metadata work again, despite current libblkid. (Fortunately, most
> use cdrom these days.)
-stable we can do only after the commit lands in Linus' tree.
Once that happens, I believe we can just as the version we have in
openSUSE 15.2 kernel for now as-is:
https://github.com/openSUSE/kernel-source/commit/ab10a7db5f5b721bf2145e6eab9358a751dd0e5b
--
Jiri Kosina
SUSE Labs
Re: [PATCH] floppy: reintroduce O_NDELAY fix
On Tue, 26 Jan 2021, Denis Efremov wrote: > Applied. I'll send it to Jens soon with a couple of cleanup patches. > > https://github.com/evdenis/linux-floppy/commit/e32f6163c47efbdbad06258560aa00d1c7e5b699 Denis, I don't see this fix in Jens' tree yet. Is there any problem with the patch? Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v2] HID: google: Get HID report on probe to confirm tablet switch state
On Fri, 15 Jan 2021, Nicolas Boichat wrote: > This forces reading the base folded state anytime the device is > probed, to make sure it's in sync. > > This is useful after a reboot, if the device re-enumerates for > any reason (e.g. ESD shock), or if the driver is unbound/rebound > (debugging/testing). > > Without this, the tablet switch state is only synchronized after a > key is pressed (since the device would then send a report that > includes the switch state), leading to strange UX (e.g. UI > mode changes when a key is pressed after reboot). > > This is not a problem on detachable base attach, as the device, > by itself, sends a report after it is booted up. > > Signed-off-by: Nicolas Boichat Applied, thanks Nicolas. -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: hid-input: avoid splitting keyboard, system and consumer controls
On Wed, 13 Jan 2021, Dmitry Torokhov wrote: > A typical USB keyboard usually splits its keys into several reports: > > - one for the basic alphanumeric keys, modifier keys, F keys, six pack > keys and keypad. This report's application is normally listed as > GenericDesktop.Keyboard > - a GenericDesktop.SystemControl report for the system control keys, such > as power and sleep > - Consumer.ConsumerControl report for multimedia (forward, rewind, > play/pause, mute, etc) and other extended keys. > - additional output, vendor specific, and feature reports > > Splitting each report into a separate input device is wasteful and even > hurts userspace as it makes it harder to determine the true capabilities > (set of available keys) of a keyboard, so let's adjust application > matching to merge system control and consumer control reports with > keyboard report, if one has already been processed. > > Signed-off-by: Dmitry Torokhov > --- Queued in for-5.12/core. Thanks, -- Jiri Kosina SUSE Labs
[GIT PULL] HID fixes
Linus, please pull from git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-linus to receve the following fixes for HID subsystem: = - NULL pointer dereference regression fix for Wacom driver (Jason Gerecke) - functional regression fix for pam handling on some Elan and Synaptics touchpads (Kai-Heng Feng) = Thanks. Jason Gerecke (1): HID: wacom: Correct NULL dereference on AES pen proximity Kai-Heng Feng (1): HID: multitouch: Apply MT_QUIRK_CONFIDENCE quirk for multi-input devices drivers/hid/hid-multitouch.c | 3 ++- drivers/hid/wacom_sys.c | 7 --- drivers/hid/wacom_wac.h | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) -- Jiri Kosina SUSE Labs
Re: [PATCH 0/2 v2] HID: fix some kernel-doc notations
On Tue, 19 Jan 2021, Randy Dunlap wrote: > Clean up kernel-doc notation in 2 files > and in drivers/hid/hid-quirks.c. > > Cc: Jiri Kosina > Cc: Benjamin Tissoires > Cc: linux-in...@vger.kernel.org > > rebase & resend: > [PATCH 1/2 v2] HID: correct kernel-doc notation in > [PATCH 2/2 v2] HID: correct kernel-doc notation in hid-quirks.c Applied, thanks Randy. -- Jiri Kosina SUSE Labs
Re: [PATCH v6 0/2] Documentation: livepatch: Document reliable stacktrace and minor cleanup
On Fri, 22 Jan 2021, Jiri Kosina wrote: > > > This series adds a document, mainly written by Mark Rutland, which > > > makes explicit the requirements for implementing reliable stacktrace > > > in order to aid architectures adding this feature. It also updates > > > the other livepatching documents to use automatically generated tables > > > of contents following review comments on Mark's document. > > > > So...is this deemed ready and, if so, do you want it to go through the > > docs tree or via some other path? > > I am planning to take it through livepatching tree unless there are any > additional last-minutes comments. Now applied to for-5.12/doc branch. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH -next] hid/hid-sensor-custom: convert comma to semicolon
On Wed, 20 Jan 2021, Jonathan Cameron wrote: > > > > Replace a comma between expression statements by a semicolon. > > > > > > > > Signed-off-by: Zheng Yongjun > > > Acked-by: Srinivas Pandruvada > > Applied to the togreg branch of iio.git and pushed out as testing for > > the autobuilders to see if they can break it. > Hi Jiri, > > Just realised this is in HID rather than IIO. I hope you don't mind as > it's now deep in a tree I'd rather not rebase. > > Sorry about that. No worries, feel free to keep it in your tree. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v6 0/2] Documentation: livepatch: Document reliable stacktrace and minor cleanup
On Thu, 21 Jan 2021, Jonathan Corbet wrote: > > This series adds a document, mainly written by Mark Rutland, which > > makes explicit the requirements for implementing reliable stacktrace > > in order to aid architectures adding this feature. It also updates > > the other livepatching documents to use automatically generated tables > > of contents following review comments on Mark's document. > > So...is this deemed ready and, if so, do you want it to go through the > docs tree or via some other path? I am planning to take it through livepatching tree unless there are any additional last-minutes comments. Thanks, -- Jiri Kosina SUSE Labs
[PATCH] floppy: reintroduce O_NDELAY fix
From: Jiri Kosina
This issue was originally fixed in 09954bad4 ("floppy: refactor open()
flags handling").
The fix as a side-effect, however, introduce issue for open(O_ACCMODE)
that is being used for ioctl-only open. I wrote a fix for that, but
instead of it being merged, full revert of 09954bad4 was performed,
re-introducing the O_NDELAY / O_NONBLOCK issue, and it strikes again.
This is a forward-port of the original fix to current codebase; the
original submission had the changelog below:
Commit 09954bad4 ("floppy: refactor open() flags handling"), as a
side-effect, causes open(/dev/fdX, O_ACCMODE) to fail. It turns out that
this is being used setfdprm userspace for ioctl-only open().
Reintroduce back the original behavior wrt !(FMODE_READ|FMODE_WRITE)
modes, while still keeping the original O_NDELAY bug fixed.
Cc: sta...@vger.kernel.org
Reported-by: Wim Osterholt
Tested-by: Wim Osterholt
Reported-and-tested-by: Kurt Garloff
Fixes: 09954bad4 ("floppy: refactor open() flags handling")
Fixes: f2791e7ead ("Revert "floppy: refactor open() flags handling"")
Signed-off-by: Jiri Kosina
---
drivers/block/floppy.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index dfe1dfc901cc..0b71292d9d5a 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4121,23 +4121,23 @@ static int floppy_open(struct block_device *bdev,
fmode_t mode)
if (fdc_state[FDC(drive)].rawcmd == 1)
fdc_state[FDC(drive)].rawcmd = 2;
- if (!(mode & FMODE_NDELAY)) {
- if (mode & (FMODE_READ|FMODE_WRITE)) {
- drive_state[drive].last_checked = 0;
- clear_bit(FD_OPEN_SHOULD_FAIL_BIT,
- _state[drive].flags);
- if (bdev_check_media_change(bdev))
- floppy_revalidate(bdev->bd_disk);
- if (test_bit(FD_DISK_CHANGED_BIT,
_state[drive].flags))
- goto out;
- if (test_bit(FD_OPEN_SHOULD_FAIL_BIT,
_state[drive].flags))
- goto out;
- }
- res = -EROFS;
- if ((mode & FMODE_WRITE) &&
- !test_bit(FD_DISK_WRITABLE_BIT, _state[drive].flags))
+ if (mode & (FMODE_READ|FMODE_WRITE)) {
+ drive_state[drive].last_checked = 0;
+ clear_bit(FD_OPEN_SHOULD_FAIL_BIT, _state[drive].flags);
+ if (bdev_check_media_change(bdev))
+ floppy_revalidate(bdev->bd_disk);
+ if (test_bit(FD_DISK_CHANGED_BIT, _state[drive].flags))
+ goto out;
+ if (test_bit(FD_OPEN_SHOULD_FAIL_BIT,
_state[drive].flags))
goto out;
}
+
+ res = -EROFS;
+
+ if ((mode & FMODE_WRITE) &&
+ !test_bit(FD_DISK_WRITABLE_BIT,
_state[drive].flags))
+ goto out;
+
mutex_unlock(_lock);
mutex_unlock(_mutex);
return 0;
--
Jiri Kosina
SUSE Labs
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On Thu, 21 Jan 2021, Denis Efremov wrote:
> > From: Jiri Kosina
> > Subject: [PATCH v2] floppy: reintroduce O_NDELAY fix
> >
> > Originally fixed in 09954bad4 ("floppy: refactor open() flags handling")
> > then reverted for unknown reason in f2791e7eadf437 instead of taking
> > the open(O_ACCMODE) for ioctl-only open fix, which had the changelog below
> >
> >
> > Commit 09954bad4 ("floppy: refactor open() flags handling"), as a
> > side-effect, causes open(/dev/fdX, O_ACCMODE) to fail. It turns out that
> > this is being used setfdprm userspace for ioctl-only open().
> >
> > Reintroduce back the original behavior wrt !(FMODE_READ|FMODE_WRITE)
> > modes, while still keeping the original O_NDELAY bug fixed.
> >
> > Cc: sta...@vger.kernel.org # v4.5+
>
> Are you sure that it's not worth to backport it to LTS v4.4? Because
> f2791e7ead is just a revert and 09954bad4 is not presented in v4.4 I'm
> not sure what fixes tag is better to use in this case.
You are right; I'll drop the '4.5+' indicator and will backport it once/if
it hits Linus' tree.
> > + if (mode & (FMODE_READ|FMODE_WRITE)) {
> > + UDRS->last_checked = 0;
>
> UDRS will still break the compilation here.
Doh, forgot to refresh before sending, sorry for the noise.
I'll send the final version once I get confirmation from the reporter that
it's fixing the issue properly, add his Reported-by: etc.
Thanks,
--
Jiri Kosina
SUSE Labs
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On Thu, 21 Jan 2021, Jiri Kosina wrote:
> I am currently waiting for confirmation by the original reporter that the
> patch below fixes the issue.
... a now a patch that actually compiles :) (made a mistake when
forward-porting from the older kernel on which this has been reported).
From: Jiri Kosina
Subject: [PATCH v2] floppy: reintroduce O_NDELAY fix
Originally fixed in 09954bad4 ("floppy: refactor open() flags handling")
then reverted for unknown reason in f2791e7eadf437 instead of taking
the open(O_ACCMODE) for ioctl-only open fix, which had the changelog below
Commit 09954bad4 ("floppy: refactor open() flags handling"), as a
side-effect, causes open(/dev/fdX, O_ACCMODE) to fail. It turns out that
this is being used setfdprm userspace for ioctl-only open().
Reintroduce back the original behavior wrt !(FMODE_READ|FMODE_WRITE)
modes, while still keeping the original O_NDELAY bug fixed.
Cc: sta...@vger.kernel.org # v4.5+
Reported-by: Wim Osterholt
Tested-by: Wim Osterholt
Signed-off-by: Jiri Kosina
=
Fixes: 09954bad4 ("floppy: refactor open() flags handling")
Fixes: f2791e7ead ("Revert "floppy: refactor open() flags handling"")
Signed-off-by: Jiri Kosina
---
v1 -> v2: fix build issue due to bad forward-port
drivers/block/floppy.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index dfe1dfc901cc..f9e839c8c5aa 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4121,23 +4121,23 @@ static int floppy_open(struct block_device *bdev,
fmode_t mode)
if (fdc_state[FDC(drive)].rawcmd == 1)
fdc_state[FDC(drive)].rawcmd = 2;
- if (!(mode & FMODE_NDELAY)) {
- if (mode & (FMODE_READ|FMODE_WRITE)) {
- drive_state[drive].last_checked = 0;
- clear_bit(FD_OPEN_SHOULD_FAIL_BIT,
- _state[drive].flags);
- if (bdev_check_media_change(bdev))
- floppy_revalidate(bdev->bd_disk);
- if (test_bit(FD_DISK_CHANGED_BIT,
_state[drive].flags))
- goto out;
- if (test_bit(FD_OPEN_SHOULD_FAIL_BIT,
_state[drive].flags))
- goto out;
- }
- res = -EROFS;
- if ((mode & FMODE_WRITE) &&
- !test_bit(FD_DISK_WRITABLE_BIT, _state[drive].flags))
+ if (mode & (FMODE_READ|FMODE_WRITE)) {
+ UDRS->last_checked = 0;
+ clear_bit(FD_OPEN_SHOULD_FAIL_BIT, _state[drive].flags);
+ if (bdev_check_media_change(bdev))
+ floppy_revalidate(bdev->bd_disk);
+ if (test_bit(FD_DISK_CHANGED_BIT, _state[drive].flags))
+ goto out;
+ if (test_bit(FD_OPEN_SHOULD_FAIL_BIT,
_state[drive].flags))
goto out;
}
+
+ res = -EROFS;
+
+ if ((mode & FMODE_WRITE) &&
+ !test_bit(FD_DISK_WRITABLE_BIT,
_state[drive].flags))
+ goto out;
+
mutex_unlock(_lock);
mutex_unlock(_mutex);
return 0;
--
Jiri Kosina
SUSE Labs
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On Thu, 21 Jan 2021, Denis Efremov wrote:
> I think it's hard to recall the exact reasons after so many years.
Yeah, I guess so :)
> I'll send a patch today based on this one.
I am currently waiting for confirmation by the original reporter that the
patch below fixes the issue.
From: Jiri Kosina
Subject: [PATCH] floppy: reintroduce O_NDELAY fix
Originally fixed in 09954bad4 ("floppy: refactor open() flags handling")
then reverted for unknown reason in f2791e7eadf437 instead of taking
the open(O_ACCMODE) for ioctl-only open fix, which had the changelog below
Commit 09954bad4 ("floppy: refactor open() flags handling"), as a
side-effect, causes open(/dev/fdX, O_ACCMODE) to fail. It turns out that
this is being used setfdprm userspace for ioctl-only open().
Reintroduce back the original behavior wrt !(FMODE_READ|FMODE_WRITE)
modes, while still keeping the original O_NDELAY bug fixed.
Cc: sta...@vger.kernel.org # v4.5+
Reported-by: Wim Osterholt
Tested-by: Wim Osterholt
Signed-off-by: Jiri Kosina
=
Fixes: 09954bad4 ("floppy: refactor open() flags handling")
Fixes: f2791e7ead ("Revert "floppy: refactor open() flags handling"")
Signed-off-by: Jiri Kosina
---
drivers/block/floppy.c | 29 ++---
1 file changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index dfe1dfc901cc..bda9417aa0a8 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -4121,23 +4121,22 @@ static int floppy_open(struct block_device *bdev,
fmode_t mode)
if (fdc_state[FDC(drive)].rawcmd == 1)
fdc_state[FDC(drive)].rawcmd = 2;
- if (!(mode & FMODE_NDELAY)) {
- if (mode & (FMODE_READ|FMODE_WRITE)) {
- drive_state[drive].last_checked = 0;
- clear_bit(FD_OPEN_SHOULD_FAIL_BIT,
- _state[drive].flags);
- if (bdev_check_media_change(bdev))
- floppy_revalidate(bdev->bd_disk);
- if (test_bit(FD_DISK_CHANGED_BIT,
_state[drive].flags))
- goto out;
- if (test_bit(FD_OPEN_SHOULD_FAIL_BIT,
_state[drive].flags))
- goto out;
- }
- res = -EROFS;
- if ((mode & FMODE_WRITE) &&
- !test_bit(FD_DISK_WRITABLE_BIT, _state[drive].flags))
+ if (mode & (FMODE_READ|FMODE_WRITE)) {
+ UDRS->last_checked = 0;
+ clear_bit(FD_OPEN_SHOULD_FAIL_BIT, _state[drive].flags);
+ check_disk_change(bdev);
+ if (test_bit(FD_DISK_CHANGED_BIT, _state[drive].flags))
+ goto out;
+ if (test_bit(FD_OPEN_SHOULD_FAIL_BIT,
_state[drive].flags))
goto out;
}
+
+ res = -EROFS;
+
+ if ((mode & FMODE_WRITE) &&
+ !test_bit(FD_DISK_WRITABLE_BIT,
_state[drive].flags))
+ goto out;
+
mutex_unlock(_lock);
mutex_unlock(_mutex);
return 0;
--
Jiri Kosina
SUSE Labs
Re: [PATCH RESEND] floppy: fix open(O_ACCMODE) for ioctl-only open
On Mon, 25 Jul 2016, Jens Axboe wrote:
> > From: Jiri Kosina
> >
> > Commit 09954bad4 ("floppy: refactor open() flags handling"), as a
> > side-effect, causes open(/dev/fdX, O_ACCMODE) to fail. It turns out that
> > this is being used setfdprm userspace for ioctl-only open().
> >
> > Reintroduce back the original behavior wrt !(FMODE_READ|FMODE_WRITE)
> > modes, while still keeping the original O_NDELAY bug fixed.
> >
> > Cc: sta...@vger.kernel.org # v4.5+
> > Reported-by: Wim Osterholt
> > Tested-by: Wim Osterholt
> > Signed-off-by: Jiri Kosina
>
> Added for this series, thanks.
[ CCing Denis too ]
Let me revive this 4 years old thread.
I've just now noticed that instead of my patch above being merged, what
happened instead was
commit f2791e7eadf437633f30faa51b30878cf15650be
Author: Jens Axboe
Date: Thu Aug 25 08:56:51 2016 -0600
Revert "floppy: refactor open() flags handling"
This reverts commit 09954bad448791ef01202351d437abdd9497a804.
which was plain revert of 09954bad4 (without any further explanation),
which in turn reintroduced the O_NDELAY issue, and I've just been hit by
it again.
I am not able to find any e-mail thread that'd indicate why ultimately
revert happened, instead of mergin my fix.
Jens, do you have any idea?
Thanks,
--
Jiri Kosina
SUSE Labs
Re: [PATCH] HID: logitech-hidpp: add support for Unified Battery (1004) feature
On Fri, 8 Jan 2021, Filipe Laíns wrote: > Yeah :head_scratch: > There is a header comment at the start of each feature section, which I think > does a good job pointing this out. IMO the problem with the naming is more for > people who see its usage in other parts of the code, but I guess that is C for > you right? Names don't scale well with code quantity :P Alright ... thug life :) Let's just keep it the way it is. > > > > Could you please use standard kernel commenting style here? > > > > > > Oops, sorry. Will do :) I have adjusted the wrong comment and applied. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v2] HID: sony: Add support for tilt on guitar hero guitars
On Fri, 4 Dec 2020, sanjay.govi...@gmail.com wrote: > From: Sanjay Govind > > This commit adds support for tilt on Standard Guitar Hero PS3 Guitars, and > GH3 PC Guitars, mapping it to ABS_RY. > > Note that GH3 PC Guitars are identical, only they use different VID and PIDs. > Also note that vendor id 0x12ba is used by a variety of different rhythm > controllers on the ps3. > > Signed-off-by: Sanjay Govind > > Fix some incorrect constants after a refactor I have removed the sentence above and applied, thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH v1] HID: make arrays usage and value to be the same
On Sat, 5 Dec 2020, Will McVicker wrote: > The HID subsystem allows an "HID report field" to have a different > number of "values" and "usages" when it is allocated. When a field > struct is created, the size of the usage array is guaranteed to be at > least as large as the values array, but it may be larger. This leads to > a potential out-of-bounds write in > __hidinput_change_resolution_multipliers() and an out-of-bounds read in > hidinput_count_leds(). > > To fix this, let's make sure that both the usage and value arrays are > the same size. Now applied, sorry for the delay and thanks for the fix. -- Jiri Kosina SUSE Labs
[GIT PULL] HID fixes
Linus, please pull from git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-linus to receive fixes for HID subsystem; = - memory leak fix for Wacom driver (Ping Cheng) - various trivial small fixes, cleanups and device ID additions = Thanks. Arnd Bergmann (2): HID: sfh: fix address space confusion HID: sony: select CONFIG_CRC32 Filipe Laíns (1): HID: logitech-dj: add the G602 receiver Kai-Heng Feng (1): HID: multitouch: Enable multi-input for Synaptics pointstick/touchpad device Nicholas Miell (1): HID: logitech-hidpp: Add product ID for MX Ergo in Bluetooth mode Ping Cheng (1): HID: wacom: Fix memory leakage caused by kfifo_alloc Seth Miller (1): HID: Ignore battery for Elan touchscreen on ASUS UX550 Tom Rix (2): HID: uclogic: remove h from printk format specifier HID: wiimote: remove h from printk format specifier drivers/hid/Kconfig | 1 + drivers/hid/amd-sfh-hid/amd_sfh_client.c | 8 drivers/hid/amd-sfh-hid/amd_sfh_hid.h| 2 +- drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 2 +- drivers/hid/amd-sfh-hid/amd_sfh_pcie.h | 2 +- drivers/hid/hid-ids.h| 1 + drivers/hid/hid-input.c | 2 ++ drivers/hid/hid-logitech-dj.c| 4 drivers/hid/hid-logitech-hidpp.c | 2 ++ drivers/hid/hid-multitouch.c | 4 drivers/hid/hid-uclogic-params.c | 2 +- drivers/hid/hid-wiimote-core.c | 2 +- drivers/hid/wacom_sys.c | 35 +--- 13 files changed, 55 insertions(+), 12 deletions(-) -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: wacom: Fix memory leak in wacom_probe()
On Thu, 10 Dec 2020, Peilin Ye wrote: > wacom_probe() is leaking memory. Free `_wac->pen_fifo` when > hid_parse() or wacom_parse_and_register() fails. Thanks for the patch. It's however already been fixed in hid.git#for-5.11/upstream-fixes (37309f47e2f5) branch that will be going to Linus shortly. -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: google: Get HID report on probe to confirm tablet switch state
On Thu, 24 Dec 2020, Nicolas Boichat wrote: > This forces reading the base folded status anytime the device is > probed. Could you please provide a little bit more verbose changelog (namely what is the actual problem this patch is fixing)? Thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH v2] HID: Add Wireless Radio Control feature for Chicony devices
On Wed, 23 Dec 2020, Jian-Hong Pan wrote: > Some Chicony's keyboards support airplane mode hotkey (Fn+F2) with > "Wireless Radio Control" feature. For example, the wireless keyboard > [04f2:1236] shipped with ASUS all-in-one desktop. > > After consulting Chicony for this hotkey, learned the device will send > with 0x11 as the report ID and 0x1 as the value when the key is pressed > down. > > This patch maps the event as KEY_RFKILL. > > Signed-off-by: Jian-Hong Pan > --- > v2: Remove the duplicated key pressed check. Applied, thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: logitech-hidpp: add support for Unified Battery (1004) feature
On Fri, 8 Jan 2021, Filipe Laíns wrote: > > > -static int hidpp20_query_battery_info(struct hidpp_device *hidpp) > > > +static int hidpp20_query_battery_info_1000(struct hidpp_device *hidpp) > > > > That '_1000' suffix looks strange to me, as it's not completely obvious > > just from looking at the code what it actually means. Would it perhaps be > > more readable to call it something like hidpp20_query_battery_level(), and > > symmentrically change hidpp20_query_battery_info_1004() to e.g. > > hidpp20_query_battery_voltage() ? > > The problem here is that hidpp20_query_battery_info_1004() does not set the > battery voltage, it is also the battery level. The best alternative I can > think > of is replacing the 1000/1004 with slightly mangled HID++ feature names, like > we > do on the other feature function. The drawback here is that I think that could > get confusing quickly. > > hidpp20_batterylevel_query_battery_info() > hidpp20_unifiedbattery_query_battery_info() > > Note that this does not provide *that* much more information than the feature > number, though it is probably the best option. What do you think? Alright, what a mess :) Would it perhaps help if there is at least a short comment preceding the function definition, noting what the constants actually are? > > [ ... snip ... ] > > > + /* if the device supports state of charge (battery percentage) we > > > won't > > > + export the battery level information. there are 4 possible > > > battery > > > + levels and they all are optional, this means that the device > > > might > > > + not support any of them, we are just better off with the > > > battery > > > + percentage. */ > > > > Could you please use standard kernel commenting style here? > > Oops, sorry. Will do :) Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: logitech-dj: add the G602 receiver
On Mon, 4 Jan 2021, Filipe Laíns wrote: > Tested. The device gets correctly exported to userspace and I can see > mouse and keyboard events. > > Signed-off-by: Filipe Laíns Applied to hid.git#for-5.11/upstream-fixes. -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: core: detect and skip invalid inputs to snto32()
On Wed, 16 Dec 2020, Randy Dunlap wrote:
> Prevent invalid (0, 0) inputs to hid-core's snto32() function.
>
> Maybe it is just the dummy device here that is causing this, but
> there are hundreds of calls to snto32(0, 0). Having n (bits count)
> of 0 is causing the current UBSAN trap with a shift value of
> 0x (-1, or n - 1 in this function).
>
> Either of the value to shift being 0 or the bits count being 0 can be
> handled by just returning 0 to the caller, avoiding the following
> complex shift + OR operations:
>
> return value & (1 << (n - 1)) ? value | (~0U << n) : value;
>
> Fixes: dde5845a529f ("[PATCH] Generic HID layer - code split")
> Signed-off-by: Randy Dunlap
> Reported-by: syzbot+1e911ad71dd4ea72e...@syzkaller.appspotmail.com
> Cc: Jiri Kosina
> Cc: Benjamin Tissoires
> Cc: linux-in...@vger.kernel.org
> ---
> drivers/hid/hid-core.c |3 +++
> 1 file changed, 3 insertions(+)
>
> --- lnx-510.orig/drivers/hid/hid-core.c
> +++ lnx-510/drivers/hid/hid-core.c
> @@ -1307,6 +1307,9 @@ EXPORT_SYMBOL_GPL(hid_open_report);
>
> static s32 snto32(__u32 value, unsigned n)
> {
> + if (!value || !n)
> + return 0;
> +
Given the fact that this has been in the code basically since ever, we're
probably fine, but it's good to have this fixed nevertheless. Applied
conservatively to hid.git#for-5.12/core.
Thanks,
--
Jiri Kosina
SUSE Labs
Re: [PATCH] HID: logitech-hidpp: add support for Unified Battery (1004) feature
On Mon, 4 Jan 2021, la...@archlinux.org wrote:
> From: Filipe Laíns
>
> This new feature present in new devices replaces the old Battery Level
> Status (0x1000) feature. It keeps essentially the same information for
> levels (reporting critical, low, good and full) but makes these levels
> optional, the device exports a capability setting which describes which
> levels it supports. In addition to this, there is an optional
> state_of_charge paramenter that exports the battery percentage.
>
> This patch adds support for this new feature. There were some
> implementation choices, as described below and in the code.
>
> If the device supports the state_of_charge parameter, we will just
> export the battery percentage and not the levels, which the device might
> still support.
>
> Since this feature can co-exist with the Battery Voltage (0x1001)
> feature and we currently only support one battery feature, I changed the
> battery feature discovery to try to use 0x1000 and 0x1004 first and only
> then 0x1001, the battery voltage feature.
> In the future we could uncouple this and make the battery feature
> co-exists with 0x1000 and 0x1004, allowing the device to export voltage
> information in addition to the battery percentage or level.
>
> I tested this patch with a MX Anywhere 3, which supports the new
> feature. Since I don't have any device that doesn't support the
> state_of_charge parameter of this feature, I forced the MX Anywhere 3 to
> use the level information, instead of battery percentage, to test that
> part of the implementation.
> I also tested with a MX Master 3, which supports the Battery Level
> Status (0x1000) feature, and a G703 Hero, which supports the Battery
> Voltage (0x1001) feature, to make sure nothing broke there.
Thanks a lot for the patch, Filipe. Minor details:
> Signed-off-by: Filipe Laíns
> ---
> drivers/hid/hid-logitech-hidpp.c | 244 ++-
> 1 file changed, 237 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c
> b/drivers/hid/hid-logitech-hidpp.c
> index f85781464807..291c6b4d26b7 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -92,6 +92,8 @@ MODULE_PARM_DESC(disable_tap_to_click,
> #define HIDPP_CAPABILITY_BATTERY_MILEAGE BIT(2)
> #define HIDPP_CAPABILITY_BATTERY_LEVEL_STATUSBIT(3)
> #define HIDPP_CAPABILITY_BATTERY_VOLTAGE BIT(4)
> +#define HIDPP_CAPABILITY_BATTERY_PERCENTAGE BIT(5)
> +#define HIDPP_CAPABILITY_UNIFIED_BATTERY BIT(6)
>
> #define lg_map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max,
> EV_KEY, (c))
>
> @@ -152,6 +154,7 @@ struct hidpp_battery {
> int voltage;
> int charge_type;
> bool online;
> + u8 supported_levels_1004;
> };
>
> /**
> @@ -1171,7 +1174,7 @@ static int hidpp20_batterylevel_get_battery_info(struct
> hidpp_device *hidpp,
> return 0;
> }
>
> -static int hidpp20_query_battery_info(struct hidpp_device *hidpp)
> +static int hidpp20_query_battery_info_1000(struct hidpp_device *hidpp)
That '_1000' suffix looks strange to me, as it's not completely obvious
just from looking at the code what it actually means. Would it perhaps be
more readable to call it something like hidpp20_query_battery_level(), and
symmentrically change hidpp20_query_battery_info_1004() to e.g.
hidpp20_query_battery_voltage() ?
[ ... snip ... ]
> + /* if the device supports state of charge (battery percentage) we won't
> +export the battery level information. there are 4 possible battery
> +levels and they all are optional, this means that the device might
> +not support any of them, we are just better off with the battery
> +percentage. */
Could you please use standard kernel commenting style here?
Thanks,
--
Jiri Kosina
SUSE Labs
Re: [PATCH] Input: gtco - remove driver
On Thu, 17 Dec 2020, Dmitry Torokhov wrote:
> > > Note that our HID support has greatly improved over the last 10 years,
> > > we may also consider reverting 6f8d9e26e7de ("hid-core.c: Adds all GTCO
> > > CalComp Digitizers and InterWrite School Products to blacklist") and see
> > > if GTCO devices actually work with normal HID drivers.
> >
> > Sounds like a good plan to me. Perhaps you can do that in a series
> > together, and stage that for 5.12?
>
> Sorry, I already zapped the driver in 5.11.
>
> Unfortunately I do not have this hardware, so while we could remove
> these devices from the blacklist we will have to do that blindly. Please
> let me know if you still want to do that.
We can also wait for the first person to potentially complain (if ever)
about the driver going away -- that'd mean that the person does actually
still have that hardware, and we could ask him to test with hid-generic.
Thanks,
--
Jiri Kosina
SUSE Labs
Re: [PATCH v2] HID: Add Wireless Radio Control feature for Chicony devices
On Wed, 23 Dec 2020, Jian-Hong Pan wrote: > Some Chicony's keyboards support airplane mode hotkey (Fn+F2) with > "Wireless Radio Control" feature. For example, the wireless keyboard > [04f2:1236] shipped with ASUS all-in-one desktop. > > After consulting Chicony for this hotkey, learned the device will send > with 0x11 as the report ID and 0x1 as the value when the key is pressed > down. > > This patch maps the event as KEY_RFKILL. I don't know how exactly does the report descriptor of that device look like, but is this not doable from userspace via setkeycode() (udev/systemd is shipping a lot of such mappings already -- see evdev/keyboard definitions in hwdb). Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: uclogic: remove h from printk format specifier
On Tue, 15 Dec 2020, t...@redhat.com wrote: > From: Tom Rix > > See Documentation/core-api/printk-formats.rst. > h should no longer be used in the format specifier for printk. > > Signed-off-by: Tom Rix Applied, thanks. -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: wiimote: remove h from printk format specifier
On Tue, 15 Dec 2020, t...@redhat.com wrote: > From: Tom Rix > > See Documentation/core-api/printk-formats.rst. > h should no longer be used in the format specifier for printk. > > Signed-off-by: Tom Rix Applied this one too. Thanks Tom. -- Jiri Kosina SUSE Labs
Re: [PATCH 0/2] hid: intel-ish-hid: ipc: enable OOB support for EHL
On Wed, 16 Dec 2020, Zhang Lixu wrote: > The EHL (Elkhart Lake) based platforms provide a OOB (Out of band) > service, which allows wakup device when the system is in S5 (Soft-Off > state). This OOB service can be enabled/disabled from BIOS settings. > > These two patches is to enable this feature for EHL platform. > > We have tested these patches on both ISH platforms and EHL platforms, > it works fine. > > Zhang Lixu (2): > hid: intel-ish-hid: ipc: finish power flow for EHL OOB > hid: intel-ish-hid: ipc: Address EHL Sx resume issues > > drivers/hid/intel-ish-hid/ipc/hw-ish.h | 1 + > drivers/hid/intel-ish-hid/ipc/ipc.c | 27 + > drivers/hid/intel-ish-hid/ipc/pci-ish.c | 54 - > 3 files changed, 81 insertions(+), 1 deletion(-) Applied to hid.git#for-5.12/intel-ish. -- Jiri Kosina SUSE Labs
Re: [PATCH 0/2] hid: intel-ish-hid: ipc: enable OOB support for EHL
On Wed, 16 Dec 2020, Zhang Lixu wrote: > The EHL (Elkhart Lake) based platforms provide a OOB (Out of band) > service, which allows wakup device when the system is in S5 (Soft-Off > state). This OOB service can be enabled/disabled from BIOS settings. > > These two patches is to enable this feature for EHL platform. > > We have tested these patches on both ISH platforms and EHL platforms, > it works fine. > > Zhang Lixu (2): > hid: intel-ish-hid: ipc: finish power flow for EHL OOB > hid: intel-ish-hid: ipc: Address EHL Sx resume issues > > drivers/hid/intel-ish-hid/ipc/hw-ish.h | 1 + > drivers/hid/intel-ish-hid/ipc/ipc.c | 27 + > drivers/hid/intel-ish-hid/ipc/pci-ish.c | 54 - > 3 files changed, 81 insertions(+), 1 deletion(-) Srinivas, can I please get your Acked-by / Reviewed-by for this? Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] hid: sony: select CONFIG_CRC32
On Sun, 3 Jan 2021, Arnd Bergmann wrote: > From: Arnd Bergmann > > Without crc32 support, this driver fails to link: > > arm-linux-gnueabi-ld: drivers/hid/hid-sony.o: in function `sony_raw_event': > hid-sony.c:(.text+0x8f4): undefined reference to `crc32_le' > arm-linux-gnueabi-ld: hid-sony.c:(.text+0x900): undefined reference to > `crc32_le' > arm-linux-gnueabi-ld: drivers/hid/hid-sony.o:hid-sony.c:(.text+0x4408): more > undefined references to `crc32_le' follow > > Signed-off-by: Arnd Bergmann Applied, thanks Arnd. -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: sfh: fix address space confusion
On Sun, 3 Jan 2021, Arnd Bergmann wrote:
> From: Arnd Bergmann
>
> The new driver uses a phys_addr_t to store a DMA address,
> which does not work when the two are different size:
>
> drivers/hid/amd-sfh-hid/amd_sfh_client.c:157:11: error: incompatible pointer
> types passing 'phys_addr_t *' (aka 'unsigned int *') to parameter of type
> 'dma_addr_t *' (aka 'unsigned long long *')
> [-Werror,-Wincompatible-pointer-types]
>
> _data->sensor_phys_addr[i],
>
> ^
> include/linux/dma-mapping.h:393:15: note: passing argument to parameter
> 'dma_handle' here
> dma_addr_t *dma_handle, gfp_t gfp)
> ^
>
> Change both the type and the variable name to dma_addr for consistency.
>
> Fixes: 4b2c53d93a4b ("SFH:Transport Driver to add support of AMD Sensor
> Fusion Hub (SFH)")
> Signed-off-by: Arnd Bergmann
Good catch; queued for 5.11, thanks.
--
Jiri Kosina
SUSE Labs
Re: [PATCH] HID: multitouch: Enable multi-input for Synaptics pointstick/touchpad device
On Wed, 30 Dec 2020, Kai-Heng Feng wrote: > Pointstick and its left/right buttons on HP EliteBook 850 G7 need > multi-input quirk to work correctly. Applied to hid.git#for-5.11/upstream-fixes. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v2 0/8] Documentation: HID: edit/correct all files
On Mon, 28 Dec 2020, Randy Dunlap wrote: > Make editing corrections to all files in Documentation/hid/. > > Cc: Jiri Kosina > Cc: Benjamin Tissoires > Cc: linux-in...@vger.kernel.org > Cc: Jonathan Cameron > Cc: Srinivas Pandruvada > Cc: linux-...@vger.kernel.org > Cc: Jonathan Corbet > Cc: linux-...@vger.kernel.org > > [PATCH v2 1/8] Documentation: HID: hid-alps editing & corrections > [PATCH v2 2/8] Documentation: HID: amd-sfh-hid editing & corrections > [PATCH v2 3/8] Documentation: HID: hiddev editing & corrections > [PATCH v2 4/8] Documentation: HID: intel-ish-hid editing & corrections > [PATCH v2 5/8] Documentation: HID: hidraw editing & corrections > [PATCH v2 6/8] Documentation: HID: hid-sensor editing & corrections > [PATCH v2 7/8] Documentation: HID: hid-transport editing & corrections > [PATCH v2 8/8] Documentation: HID: uhid editing & corrections > > Documentation/hid/amd-sfh-hid.rst | 22 +++ > Documentation/hid/hid-alps.rst |4 - > Documentation/hid/hid-sensor.rst| 18 +++--- > Documentation/hid/hid-transport.rst | 12 ++-- > Documentation/hid/hiddev.rst| 10 +-- > Documentation/hid/hidraw.rst|5 + > Documentation/hid/intel-ish-hid.rst | 78 +- > Documentation/hid/uhid.rst | 34 +-- > 8 files changed, 93 insertions(+), 90 deletions(-) Queued in hid.git#for-5.12/doc. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v4 1/3] HID: hid-sensor-custom: Add custom sensor iio support
On Tue, 15 Dec 2020, Ye Xiang wrote: > Currently custom sensors properties are not decoded and it is up to > user space to interpret. > > Some manufacturers already standardized the meaning of some custom sensors. > They can be presented as a proper IIO sensor. We can identify these sensors > based on manufacturer and serial number property in the report. > > This change is identifying hinge sensor when the manufacturer is "INTEL". > This creates a platform device so that a sensor driver can be loaded to > process these sensors. > > Signed-off-by: Ye Xiang Acked-by: Jiri Kosina Jonathan, feel free to take this with the rest through your tree. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] Input: gtco - remove driver
On Sat, 5 Dec 2020, Dmitry Torokhov wrote:
> The driver has its own HID descriptor parsing code, that had and still
> has several issues discovered by syzbot and other tools. Ideally we
> should move the driver over to the HID subsystem, so that it uses proven
> parsing code. However the devices in question are EOL, and GTCO is not
> willing to extend resources for that, so let's simply remove the driver.
Acked-by: Jiri Kosina
>
> Note that our HID support has greatly improved over the last 10 years,
> we may also consider reverting 6f8d9e26e7de ("hid-core.c: Adds all GTCO
> CalComp Digitizers and InterWrite School Products to blacklist") and see
> if GTCO devices actually work with normal HID drivers.
Sounds like a good plan to me. Perhaps you can do that in a series
together, and stage that for 5.12?
Thanks,
--
Jiri Kosina
SUSE Labs
Re: [PATCH v1] HID: make arrays usage and value to be the same
On Mon, 14 Dec 2020, Will McVicker wrote:
> > The HID subsystem allows an "HID report field" to have a different
> > number of "values" and "usages" when it is allocated. When a field
> > struct is created, the size of the usage array is guaranteed to be at
> > least as large as the values array, but it may be larger. This leads to
> > a potential out-of-bounds write in
> > __hidinput_change_resolution_multipliers() and an out-of-bounds read in
> > hidinput_count_leds().
> >
> > To fix this, let's make sure that both the usage and value arrays are
> > the same size.
> >
> > Signed-off-by: Will McVicker
> > ---
> > drivers/hid/hid-core.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > index 56172fe6995c..8a8b2b982f83 100644
> > --- a/drivers/hid/hid-core.c
> > +++ b/drivers/hid/hid-core.c
> > @@ -90,7 +90,7 @@ EXPORT_SYMBOL_GPL(hid_register_report);
> > * Register a new field for this report.
> > */
> >
> > -static struct hid_field *hid_register_field(struct hid_report *report,
> > unsigned usages, unsigned values)
> > +static struct hid_field *hid_register_field(struct hid_report *report,
> > unsigned usages)
> > {
> > struct hid_field *field;
> >
> > @@ -101,7 +101,7 @@ static struct hid_field *hid_register_field(struct
> > hid_report *report, unsigned
> >
> > field = kzalloc((sizeof(struct hid_field) +
> > usages * sizeof(struct hid_usage) +
> > -values * sizeof(unsigned)), GFP_KERNEL);
> > +usages * sizeof(unsigned)), GFP_KERNEL);
> > if (!field)
> > return NULL;
> >
> > @@ -300,7 +300,7 @@ static int hid_add_field(struct hid_parser *parser,
> > unsigned report_type, unsign
> > usages = max_t(unsigned, parser->local.usage_index,
> > parser->global.report_count);
> >
> > - field = hid_register_field(report, usages, parser->global.report_count);
> > + field = hid_register_field(report, usages);
> > if (!field)
> > return 0;
> >
> > --
> > 2.29.2.576.ga3fc446d84-goog
> >
>
> Hi Jiri and Benjamin,
>
> This is a friendly reminder in case this got lost in your inbox.
Hi Will,
I am planning to merge it once the merge window is over.
--
Jiri Kosina
SUSE Labs
[GIT PULL] HID for 5.11
Linus, please pull from git://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-linus to receive 5.11 HID subsystem queue. Highlights: = - AMD SFH (Sensor Fusion Hub) support, from Sandeep Singh - increase of maximum HID report size to 16KB in order to support some of the modern devices, from Dean Camera - control interface support for hidraw, from Dean Camera - Sony DS4 power and firmware reporting fixes, from Roderick Colenbrander - support for ghlive PS3/WII u dongles, from Pascal Giard = Thanks. Coiby Xu (1): HID: i2c-hid: show the error when failing to fetch the HID descriptor Colin Ian King (1): SFH: fix error return check for -ERESTARTSYS Dean Camera (2): HID: Increase HID maximum report size to 16KB HID: hidraw: Add additional hidraw input/output report ioctls. Ethan Warth (1): HID: mf: add support for 0079:1846 Mayflash/Dragonrise USB Gamecube Adapter Gustavo A. R. Silva (2): HID: usbhid: Fix fall-through warnings for Clang HID: input: Fix fall-through warnings for Clang Hans de Goede (2): HID: logitech-hidpp: Add hid_device_id for V470 bluetooth mouse HID: ite: Add support for Acer S1002 keyboard-dock Jing Xiangfeng (1): HID: intel-ish-hid: Remove unnecessary assignment to variable rv Jiri Kosina (2): HID: SFH: Add documentation HID: elecom: drop stray comment Julian Sax (1): HID: i2c-hid: add Vero K147 to descriptor override Luke D Jones (1): HID: asus: Add support for ASUS N-Key keyboard Pascal Giard (1): HID: sony: support for ghlive ps3/wii u dongles Rikard Falkeborn (1): HID: wacom: Constify attribute_groups Roderick Colenbrander (3): HID: sony: Report more accurate DS4 power status. HID: sony: Don't use fw_version/hw_version for sysfs cleanup. HID: sony: Workaround for DS4 dongle hotplug kernel crash. Sandeep Singh (5): SFH: Add maintainers and documentation for AMD SFH based on HID framework SFH: PCIe driver to add support of AMD sensor fusion hub SFH:Transport Driver to add support of AMD Sensor Fusion Hub (SFH) SFH: Create HID report to Enable support of AMD sensor fusion Hub (SFH) AMD_SFH: Fix for incorrect Sensor index YOSHIOKA Takuma (2): HID: elecom: rewrite report based on model specific parameters HID: elecom: add support for EX-G M-XGL20DLBK wireless mouse dmitry.torok...@gmail.com (1): HID: hid-input: occasionally report stylus battery even if not changed Documentation/hid/amd-sfh-hid.rst | 145 + Documentation/hid/hidraw.rst | 45 +- Documentation/hid/index.rst| 1 + MAINTAINERS| 8 + drivers/hid/Kconfig| 3 + drivers/hid/Makefile | 2 + drivers/hid/amd-sfh-hid/Kconfig| 18 + drivers/hid/amd-sfh-hid/Makefile | 13 + drivers/hid/amd-sfh-hid/amd_sfh_client.c | 246 drivers/hid/amd-sfh-hid/amd_sfh_hid.c | 174 ++ drivers/hid/amd-sfh-hid/amd_sfh_hid.h | 67 +++ drivers/hid/amd-sfh-hid/amd_sfh_pcie.c | 152 + drivers/hid/amd-sfh-hid/amd_sfh_pcie.h | 79 +++ .../amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.c | 224 +++ .../amd-sfh-hid/hid_descriptor/amd_sfh_hid_desc.h | 107 .../hid_descriptor/amd_sfh_hid_report_desc.h | 645 + drivers/hid/hid-asus.c | 123 +++- drivers/hid/hid-elecom.c | 51 +- drivers/hid/hid-ids.h | 7 + drivers/hid/hid-input.c| 6 +- drivers/hid/hid-ite.c | 13 +- drivers/hid/hid-logitech-hidpp.c | 2 + drivers/hid/hid-mf.c | 2 + drivers/hid/hid-quirks.c | 3 + drivers/hid/hid-sony.c | 247 ++-- drivers/hid/hidraw.c | 24 +- drivers/hid/i2c-hid/i2c-hid-core.c | 5 +- drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 8 + drivers/hid/intel-ish-hid/ishtp-hid.c | 6 +- drivers/hid/usbhid/hid-core.c | 2 + drivers/hid/wacom_sys.c| 16 +- include/linux/hid.h| 3 +- include/uapi/linux/hidraw.h| 6 + samples/hidraw/hid-example.c | 2 +- 34 files changed, 2360 insertions(+), 95 deletions(-) create mode 100644 Documentation/hid/amd-sfh-hid.rst create mode 100644 drivers/hid/amd-sfh-hid/Kconfig create mode 100644 drivers/hid/amd-sfh-hid/Makefile create mode 100644 drivers/hid/amd-sfh-hid/amd_sfh_client.c create mode 100644
Re: [PATCH v3] HID: sony: support for ghlive ps3/wii u dongles
On Wed, 25 Nov 2020, Pascal Giard wrote: > This commit adds support for the Guitar Hero Live PS3 and Wii U dongles. Applied, thanks Pascal. -- Jiri Kosina SUSE Labs
Re: [PATCH 1/2] HID: elecom: rewrite report based on model specific parameters
On Wed, 25 Nov 2020, Takuma YOSHIOKA wrote: > > case USB_DEVICE_ID_ELECOM_M_DT1URBK: > > case USB_DEVICE_ID_ELECOM_M_DT1DRBK: > > case USB_DEVICE_ID_ELECOM_M_HT1URBK: > > case USB_DEVICE_ID_ELECOM_M_HT1DRBK: > > - mouse_button_fixup(hdev, rdesc, *rsize, 8); > > + /*mouse_button_fixup(hdev, rdesc, *rsize, 13, 15, 21, 31, 8);*/ > > I'm very very sorry, I noticed that I've forgotten to remove this > commented-out line... > This "13, 15, 21, 31" line should be removed completely. No worries, I've dropped that one already. Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH v2] HID: sony: support for ghlive ps3/wii u dongles
On Wed, 25 Nov 2020, pascal.gi...@etsmtl.ca wrote: > Thank you for reviewing this new version. > > You are right, we could totally do without GHL_GUITAR_CONTROLLER. > > This can be seen as premature generalization or an excess of optimism; > I'm assuming that the PS4 also needs magic control messages to behave > correctly, and that I will figure those sooner than later. But I may be > assuming too much and this will be trivial to add when the time comes. Yeah, let's extend this only when really needed. > Do you want me to submit a v3? Please do, thanks. I'll merge that one, I promise :) Sorry for not having catched this in v1 already. -- Jiri Kosina SUSE Labs
Re: [PATCH 1/2] HID: elecom: rewrite report based on model specific parameters
On Sun, 22 Nov 2020, YOSHIOKA Takuma wrote: > The report descriptor for EX-G wireless mouse (M-XGL20DLBK) is a bit > different from that for trackball mice such as DEFT. For such mouse, the > current `mouse_button_fixup` cannot be used as is, because it uses > hard-coded indices for a report descriptor. > > Add parameters to `mouse_button_fixup` function, in order to support > fixing report descriptors for more models. I have applied both patches, thank you. -- Jiri Kosina SUSE Labs
Re: [PATCH v2] HID: sony: support for ghlive ps3/wii u dongles
On Sat, 7 Nov 2020, Pascal Giard wrote: > This commit adds support for the Guitar Hero Live PS3 and Wii U dongles. > > These dongles require a "magic" USB control message [1] to be sent > approximately every 10 seconds otherwise the dongle will not report > events where the strumbar is hit while a fret is being held. > > Also, inspired by a patch sent on linux-input by Sanjay Govind [2], the > accelerometer is mapped to ABS_RY for tilt. > > Interestingly, the Wii U and PS3 dongles share the same VID and PID. > > [1] https://github.com/ghlre/GHLtarUtility/ > [2] https://marc.info/?l=linux-input=157242835928542=2 > > Signed-off-by: Pascal Giard > --- > differences from v1: > * Patches hid-sony instead of creating a new driver > * Changed memory allocation scheme in case of fail > --- > drivers/hid/Kconfig| 1 + > drivers/hid/hid-ids.h | 3 ++ > drivers/hid/hid-sony.c | 115 + > 3 files changed, 119 insertions(+) > > diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig > index 34f07371716d..e2df2ae112a5 100644 > --- a/drivers/hid/Kconfig > +++ b/drivers/hid/Kconfig > @@ -897,6 +897,7 @@ config HID_SONY > * Buzz controllers > * Sony PS3 Blue-ray Disk Remote Control (Bluetooth) > * Logitech Harmony adapter for Sony Playstation 3 (Bluetooth) > + * Guitar Hero Live PS3 and Wii U guitar dongles > > config SONY_FF > bool "Sony PS2/3/4 accessories force feedback support" > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h > index 1c71a1aa76b2..e3a3942079cf 100644 > --- a/drivers/hid/hid-ids.h > +++ b/drivers/hid/hid-ids.h > @@ -1060,6 +1060,9 @@ > #define USB_DEVICE_ID_SONY_BUZZ_CONTROLLER 0x0002 > #define USB_DEVICE_ID_SONY_WIRELESS_BUZZ_CONTROLLER 0x1000 > > +#define USB_VENDOR_ID_SONY_GHLIVE0x12ba > +#define USB_DEVICE_ID_SONY_PS3WIIU_GHLIVE_DONGLE 0x074b > + > #define USB_VENDOR_ID_SINO_LITE 0x1345 > #define USB_DEVICE_ID_SINO_LITE_CONTROLLER 0x3008 > > diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c > index 4c6ed6ef31f1..700bea6239f6 100644 > --- a/drivers/hid/hid-sony.c > +++ b/drivers/hid/hid-sony.c > @@ -11,6 +11,7 @@ > * Copyright (c) 2013 Colin Leitner > * Copyright (c) 2014-2016 Frank Praznik > * Copyright (c) 2018 Todd Kelner > + * Copyright (c) 2020 Pascal Giard > */ > > /* > @@ -35,6 +36,8 @@ > #include > #include > #include > +#include > +#include > #include > > #include "hid-ids.h" > @@ -56,6 +59,8 @@ > #define NSG_MR5U_REMOTE_BTBIT(14) > #define NSG_MR7U_REMOTE_BTBIT(15) > #define SHANWAN_GAMEPAD BIT(16) > +#define GHL_GUITAR_PS3WIIUBIT(17) > +#define GHL_GUITAR_CONTROLLER BIT(18) Hi Pascal, thanks for fixing the previous version. This one looks good to me, I just have one remaining question -- why do we need both quirks here? Given the particular VID/PID gets both of them set anyway (and only that VID/PID), and the code is shared, what is the point of consuming the extra bit? Thanks, -- Jiri Kosina SUSE Labs
Re: [PATCH] HID: wacom: Constify attribute_groups
On Wed, 25 Nov 2020, Rikard Falkeborn wrote: > These are never modified, so make them const to allow the compiler to put > them in read-only memory. It also allows the compiler to shrink the > resulting module with ~900 bytes, test-built with gcc 10.2 on x86_64. > >textdata bss dec hex filename > 204377 42832 576 247785 3c7e9 drivers/hid/wacom_old.ko > 204240 42064 576 246880 3c460 drivers/hid/wacom_new.ko Applied, thanks Rikard. -- Jiri Kosina SUSE Labs
Re: [PATCH 063/141] HID: input: Fix fall-through warnings for Clang
On Fri, 20 Nov 2020, Gustavo A. R. Silva wrote: > In preparation to enable -Wimplicit-fallthrough for Clang, fix a warning > by explicitly adding a goto statement instead of letting the code fall > through to the next case. > > Link: https://github.com/KSPP/linux/issues/115 > Signed-off-by: Gustavo A. R. Silva > --- > drivers/hid/hid-input.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c > index 9770db624bfa..37601b800a2e 100644 > --- a/drivers/hid/hid-input.c > +++ b/drivers/hid/hid-input.c > @@ -743,6 +743,7 @@ static void hidinput_configure_usage(struct hid_input > *hidinput, struct hid_fiel > field->flags |= HID_MAIN_ITEM_RELATIVE; > break; > } > + goto unknown; > > default: goto unknown; This makes my eyes hurt :) But adding the annotation would be ugly as well, so let me just take it as-is. Thanks, -- Jiri Kosina SUSE Labs
