Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH] p54usb: Fix race between disconnect and firmware loading
From: Kalle Valo @ 2019-06-25  4:43 UTC (permalink / raw)
  To: Alan Stern
  Cc: Christian Lamparter, syzbot, davem, andreyknvl, syzkaller-bugs,
	Kernel development list, USB list, linux-wireless, netdev
In-Reply-To: <Pine.LNX.4.44L0.1905201042110.1498-100000@iolanthe.rowland.org>

Alan Stern <stern@rowland.harvard.edu> wrote:

> The syzbot fuzzer found a bug in the p54 USB wireless driver.  The
> issue involves a race between disconnect and the firmware-loader
> callback routine, and it has several aspects.
> 
> One big problem is that when the firmware can't be loaded, the
> callback routine tries to unbind the driver from the USB _device_ (by
> calling device_release_driver) instead of from the USB _interface_ to
> which it is actually bound (by calling usb_driver_release_interface).
> 
> The race involves access to the private data structure.  The driver's
> disconnect handler waits for a completion that is signalled by the
> firmware-loader callback routine.  As soon as the completion is
> signalled, you have to assume that the private data structure may have
> been deallocated by the disconnect handler -- even if the firmware was
> loaded without errors.  However, the callback routine does access the
> private data several times after that point.
> 
> Another problem is that, in order to ensure that the USB device
> structure hasn't been freed when the callback routine runs, the driver
> takes a reference to it.  This isn't good enough any more, because now
> that the callback routine calls usb_driver_release_interface, it has
> to ensure that the interface structure hasn't been freed.
> 
> Finally, the driver takes an unnecessary reference to the USB device
> structure in the probe function and drops the reference in the
> disconnect handler.  This extra reference doesn't accomplish anything,
> because the USB core already guarantees that a device structure won't
> be deallocated while a driver is still bound to any of its interfaces.
> 
> To fix these problems, this patch makes the following changes:
> 
> 	Call usb_driver_release_interface() rather than
> 	device_release_driver().
> 
> 	Don't signal the completion until after the important
> 	information has been copied out of the private data structure,
> 	and don't refer to the private data at all thereafter.
> 
> 	Lock udev (the interface's parent) before unbinding the driver
> 	instead of locking udev->parent.
> 
> 	During the firmware loading process, take a reference to the
> 	USB interface instead of the USB device.
> 
> 	Don't take an unnecessary reference to the device during probe
> 	(and then don't drop it during disconnect).
> 
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Reported-and-tested-by: syzbot+200d4bb11b23d929335f@syzkaller.appspotmail.com
> CC: <stable@vger.kernel.org>
> Acked-by: Christian Lamparter <chunkeey@gmail.com>

Patch applied to wireless-drivers-next.git, thanks.

6e41e2257f10 p54usb: Fix race between disconnect and firmware loading

-- 
https://patchwork.kernel.org/patch/10951527/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH 1/2] mwifiex: dispatch/rotate from reorder table atomically
From: Kalle Valo @ 2019-06-25  4:45 UTC (permalink / raw)
  To: Brian Norris
  Cc: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu,
	linux-kernel, linux-wireless, Doug Anderson, Brian Norris
In-Reply-To: <20190604205323.200361-2-briannorris@chromium.org>

Brian Norris <briannorris@chromium.org> wrote:

> mwifiex_11n_scan_and_dispatch() and
> mwifiex_11n_dispatch_pkt_until_start_win() share similar patterns, where
> they perform a few different actions on the same table, using the same
> lock, but non-atomically. There have been other attempts to clean up
> this sort of behavior, but they have had problems (incomplete;
> introducing new deadlocks).
> 
> We can improve these functions' atomicity by queueing up our RX packets
> in a list, to dispatch at the end of the function. This avoids problems
> of another operation modifying the table in between our dispatch and
> rotation operations.
> 
> This was inspired by investigations around this:
> 
>   http://lkml.kernel.org/linux-wireless/20181130175957.167031-1-briannorris@chromium.org
>   Subject: [4.20 PATCH] Revert "mwifiex: restructure rx_reorder_tbl_lock usage"
> 
> While the original (now-reverted) patch had good intentions in
> restructuring some of the locking patterns in this driver, it missed an
> important detail: we cannot defer to softirq contexts while already in
> an atomic context. We can help avoid this sort of problem by separating
> the two steps of:
> (1) iterating / clearing the mwifiex reordering table
> (2) dispatching received packets to upper layers
> 
> This makes it much harder to make lock recursion mistakes, as these
> two steps no longer need to hold the same locks.
> 
> Testing: I've played with a variety of stress tests, including download
> stress tests on the same APs which caught regressions with commit
> 5188d5453bc9 ("mwifiex: restructure rx_reorder_tbl_lock usage"). I've
> primarily tested on Marvell 8997 / PCIe, although I've given 8897 / SDIO
> a quick spin as well.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Acked-by: Ganapathi Bhat <gbhat@marvell.com>

New warning:

drivers/net/wireless/marvell/mwifiex/wmm.c: In function 'mwifiex_wmm_process_tx':
drivers/net/wireless/marvell/mwifiex/wmm.c:1438:4: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
    mwifiex_11n_aggregate_pkt(priv, ptr, ptr_index, flags);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/marvell/mwifiex/wmm.c:1406:16: note: 'flags' was declared here
  unsigned long flags;
                ^~~~~

2 patches set to Changes Requested.

10976083 [1/2] mwifiex: dispatch/rotate from reorder table atomically
10976087 [2/2] mwifiex: don't disable hardirqs; just softirqs

-- 
https://patchwork.kernel.org/patch/10976083/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH] mwifiex: drop 'set_consistent_dma_mask' log message
From: Kalle Valo @ 2019-06-25  4:46 UTC (permalink / raw)
  To: Brian Norris
  Cc: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu,
	linux-kernel, linux-wireless, Brian Norris
In-Reply-To: <20190604172858.107084-1-briannorris@chromium.org>

Brian Norris <briannorris@chromium.org> wrote:

> This message is pointless.
> 
> While we're at it, include the error code in the error message, which is
> not pointless.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Patch applied to wireless-drivers-next.git, thanks.

f7369179ad32 mwifiex: drop 'set_consistent_dma_mask' log message

-- 
https://patchwork.kernel.org/patch/10975823/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH] mwifiex: print PCI mmap with %pK
From: Kalle Valo @ 2019-06-25  4:46 UTC (permalink / raw)
  To: Brian Norris
  Cc: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu,
	linux-kernel, linux-wireless, Brian Norris
In-Reply-To: <20190604173144.109142-1-briannorris@chromium.org>

Brian Norris <briannorris@chromium.org> wrote:

> Unadorned '%p' has restrictive policies these days, such that it usually
> just prints garbage at early boot (see
> Documentation/core-api/printk-formats.rst, "kernel will print
> ``(ptrval)`` until it gathers enough entropy"). Annotating with %pK
> (for "kernel pointer") allows the kptr_restrict sysctl to control
> printing policy better.
> 
> We might just as well drop this message entirely, but this fix was easy
> enough for now.
> 
> Signed-off-by: Brian Norris <briannorris@chromium.org>

Patch applied to wireless-drivers-next.git, thanks.

2fc0aa454473 mwifiex: print PCI mmap with %pK

-- 
https://patchwork.kernel.org/patch/10975827/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH 09/11] mwifiex: update set_mac_address logic
From: Kalle Valo @ 2019-06-25  4:47 UTC (permalink / raw)
  To: Ganapathi Bhat
  Cc: linux-wireless, Cathy Luo, Zhiyuan Yang, James Cao, Rakesh Parmar,
	Sharvari Harisangam, Ganapathi Bhat
In-Reply-To: <1560352331-16898-1-git-send-email-gbhat@marvell.com>

Ganapathi Bhat <gbhat@marvell.com> wrote:

> From: Sharvari Harisangam <sharvari@marvell.com>
> 
> In set_mac_address, driver check for interfaces with same bss_type
> For first STA entry, this would return 3 interfaces since all priv's have
> bss_type as 0 due to kzalloc. Thus mac address gets changed for STA
> unexpected. This patch adds check for first STA and avoids mac address
> change. This patch also adds mac_address change for p2p based on bss_num
> type.
> 
> Signed-off-by: Sharvari Harisangam <sharvari@marvell.com>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>

Patch applied to wireless-drivers-next.git, thanks.

7afb94da3cd8 mwifiex: update set_mac_address logic

-- 
https://patchwork.kernel.org/patch/10990209/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie()
From: Kalle Valo @ 2019-06-25  4:51 UTC (permalink / raw)
  To: Brian Norris
  Cc: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu,
	linux-kernel, linux-wireless, Takashi Iwai, Guenter Roeck,
	Brian Norris
In-Reply-To: <20190615001321.241808-2-briannorris@chromium.org>

Brian Norris <briannorris@chromium.org> wrote:

> This is clearer than copy/pasting the magic number '+ 2' around, and it
> even saves the need for one existing comment.
> 
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Reviewed-by: Takashi Iwai <tiwai@suse.de>

This depends on:

63d7ef36103d mwifiex: Don't abort on small, spec-compliant vendor IEs

Patch set to Awaiting Upstream.

-- 
https://patchwork.kernel.org/patch/10996893/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH] mwifiex: ignore processing invalid command response
From: Kalle Valo @ 2019-06-25  4:53 UTC (permalink / raw)
  To: Ganapathi Bhat
  Cc: linux-wireless, Brian Norris, Cathy Luo, Zhiyuan Yang, James Cao,
	Rakesh Parmar, Swati Kushwaha, Ganapathi Bhat
In-Reply-To: <1561126484-7735-1-git-send-email-gbhat@marvell.com>

Ganapathi Bhat <gbhat@marvell.com> wrote:

> From: Swati Kushwaha <swatiuma@marvell.com>
> 
> Firmware can send invalid command response, the processing of
> which can attempt to modify unexpected context and cause issues.
> To fix this, driver should check that the command response ID is
> same as the one it downloaded, and ignore processing of invalid
> response.
> 
> Signed-off-by: Swati Kushwaha <swatiuma@marvell.com>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>

Patch applied to wireless-drivers-next.git, thanks.

74f202aaae0a mwifiex: ignore processing invalid command response

-- 
https://patchwork.kernel.org/patch/11010163/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH] rtlwifi: rtl8192cu: fix error handle when usb probe failed
From: Kalle Valo @ 2019-06-25  4:54 UTC (permalink / raw)
  To: pkshih; +Cc: linux-wireless, andreyknvl, Larry.Finger
In-Reply-To: <20190529065730.25951-1-pkshih@realtek.com>

<pkshih@realtek.com> wrote:

> From: Ping-Ke Shih <pkshih@realtek.com>
> 
> rtl_usb_probe() must do error handle rtl_deinit_core() only if
> rtl_init_core() is done, otherwise goto error_out2.
> 
> | usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
> | rtl_usb: reg 0xf0, usbctrl_vendorreq TimeOut! status:0xffffffb9 value=0x0
> | rtl8192cu: Chip version 0x10
> | rtl_usb: reg 0xa, usbctrl_vendorreq TimeOut! status:0xffffffb9 value=0x0
> | rtl_usb: Too few input end points found
> | INFO: trying to register non-static key.
> | the code is fine but needs lockdep annotation.
> | turning off the locking correctness validator.
> | CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.1.0-rc4-319354-g9a33b36 #3
> | Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> | Google 01/01/2011
> | Workqueue: usb_hub_wq hub_event
> | Call Trace:
> |   __dump_stack lib/dump_stack.c:77 [inline]
> |   dump_stack+0xe8/0x16e lib/dump_stack.c:113
> |   assign_lock_key kernel/locking/lockdep.c:786 [inline]
> |   register_lock_class+0x11b8/0x1250 kernel/locking/lockdep.c:1095
> |   __lock_acquire+0xfb/0x37c0 kernel/locking/lockdep.c:3582
> |   lock_acquire+0x10d/0x2f0 kernel/locking/lockdep.c:4211
> |   __raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
> |   _raw_spin_lock_irqsave+0x44/0x60 kernel/locking/spinlock.c:152
> |   rtl_c2hcmd_launcher+0xd1/0x390
> | drivers/net/wireless/realtek/rtlwifi/base.c:2344
> |   rtl_deinit_core+0x25/0x2d0 drivers/net/wireless/realtek/rtlwifi/base.c:574
> |   rtl_usb_probe.cold+0x861/0xa70
> | drivers/net/wireless/realtek/rtlwifi/usb.c:1093
> |   usb_probe_interface+0x31d/0x820 drivers/usb/core/driver.c:361
> |   really_probe+0x2da/0xb10 drivers/base/dd.c:509
> |   driver_probe_device+0x21d/0x350 drivers/base/dd.c:671
> |   __device_attach_driver+0x1d8/0x290 drivers/base/dd.c:778
> |   bus_for_each_drv+0x163/0x1e0 drivers/base/bus.c:454
> |   __device_attach+0x223/0x3a0 drivers/base/dd.c:844
> |   bus_probe_device+0x1f1/0x2a0 drivers/base/bus.c:514
> |   device_add+0xad2/0x16e0 drivers/base/core.c:2106
> |   usb_set_configuration+0xdf7/0x1740 drivers/usb/core/message.c:2021
> |   generic_probe+0xa2/0xda drivers/usb/core/generic.c:210
> |   usb_probe_device+0xc0/0x150 drivers/usb/core/driver.c:266
> |   really_probe+0x2da/0xb10 drivers/base/dd.c:509
> |   driver_probe_device+0x21d/0x350 drivers/base/dd.c:671
> |   __device_attach_driver+0x1d8/0x290 drivers/base/dd.c:778
> |   bus_for_each_drv+0x163/0x1e0 drivers/base/bus.c:454
> |   __device_attach+0x223/0x3a0 drivers/base/dd.c:844
> |   bus_probe_device+0x1f1/0x2a0 drivers/base/bus.c:514
> |   device_add+0xad2/0x16e0 drivers/base/core.c:2106
> |   usb_new_device.cold+0x537/0xccf drivers/usb/core/hub.c:2534
> |   hub_port_connect drivers/usb/core/hub.c:5089 [inline]
> |   hub_port_connect_change drivers/usb/core/hub.c:5204 [inline]
> |   port_event drivers/usb/core/hub.c:5350 [inline]
> |   hub_event+0x138e/0x3b00 drivers/usb/core/hub.c:5432
> |   process_one_work+0x90f/0x1580 kernel/workqueue.c:2269
> |   worker_thread+0x9b/0xe20 kernel/workqueue.c:2415
> |   kthread+0x313/0x420 kernel/kthread.c:253
> |   ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
> 
> Reported-by: syzbot+1fcc5ef45175fc774231@syzkaller.appspotmail.com
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
> Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Patch applied to wireless-drivers-next.git, thanks.

6c0ed66f1a5b rtlwifi: rtl8192cu: fix error handle when usb probe failed

-- 
https://patchwork.kernel.org/patch/10966133/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH] rtlwifi: remove redundant assignment to variable badworden
From: Kalle Valo @ 2019-06-25  4:59 UTC (permalink / raw)
  To: Colin King
  Cc: Ping-Ke Shih, David S . Miller, linux-wireless, netdev,
	kernel-janitors, linux-kernel
In-Reply-To: <20190530184044.8479-1-colin.king@canonical.com>

Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> The variable badworden is assigned with a value that is never read and
> it is re-assigned a new value immediately afterwards.  The assignment is
> redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Acked-by: Larry Finger <Larry.Finger@lwfinger.net>

Patch applied to wireless-drivers-next.git, thanks.

5315f9d40191 rtlwifi: remove redundant assignment to variable badworden

-- 
https://patchwork.kernel.org/patch/10969175/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH] rtlwifi: remove redundant assignment to variable k
From: Kalle Valo @ 2019-06-25  5:00 UTC (permalink / raw)
  To: Colin King
  Cc: Ping-Ke Shih, David S . Miller, linux-wireless, netdev,
	kernel-janitors, linux-kernel
In-Reply-To: <20190531141412.18632-1-colin.king@canonical.com>

Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> The assignment of 0 to variable k is never read once we break out of
> the loop, so the assignment is redundant and can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Patch applied to wireless-drivers-next.git, thanks.

f0822dfc5887 rtlwifi: remove redundant assignment to variable k

-- 
https://patchwork.kernel.org/patch/10970261/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH v2 1/2] mt7601u: do not schedule rx_tasklet when the device has been disconnected
From: Kalle Valo @ 2019-06-25  5:01 UTC (permalink / raw)
  To: Lorenzo Bianconi; +Cc: kubakici, linux-wireless, lorenzo.bianconi
In-Reply-To: <bfb533c51126ee82843e3d525e55ec5b08adb860.1559906499.git.lorenzo@kernel.org>

Lorenzo Bianconi <lorenzo@kernel.org> wrote:

> Do not schedule rx_tasklet when the usb dongle is disconnected.
> Moreover do not grub rx_lock in mt7601u_kill_rx since usb_poison_urb
> can run concurrently with urb completion and we can unlink urbs from rx
> ring in any order.
> This patch fixes the common kernel warning reported when
> the device is removed.
> 
> [   24.921354] usb 3-14: USB disconnect, device number 7
> [   24.921593] ------------[ cut here ]------------
> [   24.921594] RX urb mismatch
> [   24.921675] WARNING: CPU: 4 PID: 163 at drivers/net/wireless/mediatek/mt7601u/dma.c:200 mt7601u_complete_rx+0xcb/0xd0 [mt7601u]
> [   24.921769] CPU: 4 PID: 163 Comm: kworker/4:2 Tainted: G           OE     4.19.31-041931-generic #201903231635
> [   24.921770] Hardware name: To Be Filled By O.E.M. To Be Filled By O.E.M./Z97 Extreme4, BIOS P1.30 05/23/2014
> [   24.921782] Workqueue: usb_hub_wq hub_event
> [   24.921797] RIP: 0010:mt7601u_complete_rx+0xcb/0xd0 [mt7601u]
> [   24.921800] RSP: 0018:ffff9bd9cfd03d08 EFLAGS: 00010086
> [   24.921802] RAX: 0000000000000000 RBX: ffff9bd9bf043540 RCX: 0000000000000006
> [   24.921803] RDX: 0000000000000007 RSI: 0000000000000096 RDI: ffff9bd9cfd16420
> [   24.921804] RBP: ffff9bd9cfd03d28 R08: 0000000000000002 R09: 00000000000003a8
> [   24.921805] R10: 0000002f485fca34 R11: 0000000000000000 R12: ffff9bd9bf043c1c
> [   24.921806] R13: ffff9bd9c62fa3c0 R14: 0000000000000082 R15: 0000000000000000
> [   24.921807] FS:  0000000000000000(0000) GS:ffff9bd9cfd00000(0000) knlGS:0000000000000000
> [   24.921808] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   24.921808] CR2: 00007fb2648b0000 CR3: 0000000142c0a004 CR4: 00000000001606e0
> [   24.921809] Call Trace:
> [   24.921812]  <IRQ>
> [   24.921819]  __usb_hcd_giveback_urb+0x8b/0x140
> [   24.921821]  usb_hcd_giveback_urb+0xca/0xe0
> [   24.921828]  xhci_giveback_urb_in_irq.isra.42+0x82/0xf0
> [   24.921834]  handle_cmd_completion+0xe02/0x10d0
> [   24.921837]  xhci_irq+0x274/0x4a0
> [   24.921838]  xhci_msi_irq+0x11/0x20
> [   24.921851]  __handle_irq_event_percpu+0x44/0x190
> [   24.921856]  handle_irq_event_percpu+0x32/0x80
> [   24.921861]  handle_irq_event+0x3b/0x5a
> [   24.921867]  handle_edge_irq+0x80/0x190
> [   24.921874]  handle_irq+0x20/0x30
> [   24.921889]  do_IRQ+0x4e/0xe0
> [   24.921891]  common_interrupt+0xf/0xf
> [   24.921892]  </IRQ>
> [   24.921900] RIP: 0010:usb_hcd_flush_endpoint+0x78/0x180
> [   24.921354] usb 3-14: USB disconnect, device number 7
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

2 patches applied to wireless-drivers-next.git, thanks.

4079e8ccabc3 mt7601u: do not schedule rx_tasklet when the device has been disconnected
23377c200b2e mt7601u: fix possible memory leak when the device is disconnected

-- 
https://patchwork.kernel.org/patch/10981545/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH v4 0/7] Hexdump Enhancements
From: Joe Perches @ 2019-06-25  5:01 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
	Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
	Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
	Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
	James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
	Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
	netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
	linux-fsdevel
In-Reply-To: <20190625031726.12173-1-alastair@au1.ibm.com>

On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Apologies for the large CC list, it's a heads up for those responsible
> for subsystems where a prototype change in generic code causes a change
> in those subsystems.
[]
> The default behaviour of hexdump is unchanged, however, the prototype
> for hex_dump_to_buffer() has changed, and print_hex_dump() has been
> renamed to print_hex_dump_ext(), with a wrapper replacing it for
> compatibility with existing code, which would have been too invasive to
> change.

I believe this cover letter is misleading.

The point of the wrapper is to avoid unnecessary changes
in existing
code.



^ permalink raw reply

* Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Joe Perches @ 2019-06-25  5:01 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
	Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
	Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
	Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
	James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
	Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
	netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
	linux-fsdevel
In-Reply-To: <20190625031726.12173-5-alastair@au1.ibm.com>

On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> In order to support additional features, rename hex_dump_to_buffer to
> hex_dump_to_buffer_ext, and replace the ascii bool parameter with flags.
[]
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
[]
> @@ -1338,9 +1338,8 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len)
>  		}
>  
>  		WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
> -						rowsize, sizeof(u32),
> -						line, sizeof(line),
> -						false) >= sizeof(line));
> +						rowsize, sizeof(u32), line,
> +						sizeof(line)) >= sizeof(line));

Huh?  Why do this?

> diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c b/drivers/isdn/hardware/mISDN/mISDNisar.c
[]
> @@ -70,8 +70,9 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg, u8 len, u8 *msg)
>  			int l = 0;
>  
>  			while (l < (int)len) {
> -				hex_dump_to_buffer(msg + l, len - l, 32, 1,
> -						   isar->log, 256, 1);
> +				hex_dump_to_buffer_ext(msg + l, len - l, 32, 1,
> +						       isar->log, 256,
> +						       HEXDUMP_ASCII);

Again, why do any of these?

The point of the wrapper is to avoid changing these.



^ permalink raw reply

* Re: [PATCH][next] qtnfmac: Use struct_size() in kzalloc()
From: Kalle Valo @ 2019-06-25  5:01 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Igor Mitsyanko, Avinash Patil, Sergey Matyukevich,
	David S. Miller, linux-wireless, netdev, linux-kernel,
	Gustavo A. R. Silva
In-Reply-To: <20190607191745.GA19120@embeddedor>

"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:

> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
> 
> struct ieee80211_regdomain {
> 	...
>         struct ieee80211_reg_rule reg_rules[];
> };
> 
> instance = kzalloc(sizeof(*mac->rd) +
>                           sizeof(struct ieee80211_reg_rule) *
>                           count, GFP_KERNEL);
> 
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
> 
> instance = kzalloc(struct_size(instance, reg_rules, count), GFP_KERNEL);
> 
> This code was detected with the help of Coccinelle.
> 
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Reviewed-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>

Patch applied to wireless-drivers-next.git, thanks.

9a1ace64ca3b qtnfmac: Use struct_size() in kzalloc()

-- 
https://patchwork.kernel.org/patch/10982675/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH] rtlwifi: rtl8188ee: remove redundant assignment to rtstatus
From: Kalle Valo @ 2019-06-25  5:02 UTC (permalink / raw)
  To: Colin King
  Cc: Ping-Ke Shih, David S . Miller, linux-wireless, netdev,
	kernel-janitors, linux-kernel
In-Reply-To: <20190608105800.26571-1-colin.king@canonical.com>

Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> Variable rtstatus is being initialized with a value that is never read
> as rtstatus is being re-assigned a little later on. The assignment is
> redundant and hence can be removed.
> 
> Addresses-Coverity: ("Unused value")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Patch applied to wireless-drivers-next.git, thanks.

25a986e426b0 rtlwifi: rtl8188ee: remove redundant assignment to rtstatus

-- 
https://patchwork.kernel.org/patch/10983111/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Alastair D'Silva @ 2019-06-25  5:06 UTC (permalink / raw)
  To: Joe Perches
  Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
	Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
	Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
	Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
	James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
	Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
	netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
	linux-fsdevel
In-Reply-To: <3340b520a57e00a483eae170be97316c8d18c22c.camel@perches.com>

On Mon, 2019-06-24 at 22:01 -0700, Joe Perches wrote:
> On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> > From: Alastair D'Silva <alastair@d-silva.org>
> > 
> > In order to support additional features, rename hex_dump_to_buffer
> > to
> > hex_dump_to_buffer_ext, and replace the ascii bool parameter with
> > flags.
> []
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
> > b/drivers/gpu/drm/i915/intel_engine_cs.c
> []
> > @@ -1338,9 +1338,8 @@ static void hexdump(struct drm_printer *m,
> > const void *buf, size_t len)
> >  		}
> >  
> >  		WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
> > -						rowsize, sizeof(u32),
> > -						line, sizeof(line),
> > -						false) >=
> > sizeof(line));
> > +						rowsize, sizeof(u32),
> > line,
> > +						sizeof(line)) >=
> > sizeof(line));
> 
> Huh?  Why do this?
> 
> > diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c
> > b/drivers/isdn/hardware/mISDN/mISDNisar.c
> []
> > @@ -70,8 +70,9 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg,
> > u8 len, u8 *msg)
> >  			int l = 0;
> >  
> >  			while (l < (int)len) {
> > -				hex_dump_to_buffer(msg + l, len - l,
> > 32, 1,
> > -						   isar->log, 256, 1);
> > +				hex_dump_to_buffer_ext(msg + l, len -
> > l, 32, 1,
> > +						       isar->log, 256,
> > +						       HEXDUMP_ASCII);
> 
> Again, why do any of these?
> 
> The point of the wrapper is to avoid changing these.
> 
> 

The change actions Jani's suggestion:
https://lkml.org/lkml/2019/6/20/343


-- 
Alastair D'Silva           mob: 0423 762 819
skype: alastair_dsilva    
Twitter: @EvilDeece
blog: http://alastair.d-silva.org



^ permalink raw reply

* Re: [PATCH] ssb/gpio: Remove unnecessary WARN_ON from driver_gpio
From: Kalle Valo @ 2019-06-25  5:07 UTC (permalink / raw)
  To: Michael Büsch; +Cc: H Buus, Larry Finger, linux-wireless
In-Reply-To: <20190610204927.2de21c9a@wiggum>

Michael Büsch <m@bues.ch> writes:

> The WARN_ON triggers on older BCM4401-B0 100Base-TX ethernet controllers.
> The warning serves no purpose. So let's just remove it.
>
> Reported-by: H Buus <ubuntu@hbuus.com>
> Signed-off-by: Michael Büsch <m@bues.ch>

For some reason patchwork (or pwcli script) didn't like this patch so
manually applied to wireless-drivers-next:

e73e43246da6 ssb/gpio: Remove unnecessary WARN_ON from driver_gpio

I have a faint recollection that I had a similar problem with another
patch from Michael, did we ever conclude what was the issue?

-- 
Kalle Valo

^ permalink raw reply

* Re: [PATCH 1/5] iwlegacy: 3945: no need to check return value of debugfs_create functions
From: Kalle Valo @ 2019-06-25  5:08 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Johannes Berg, Greg Kroah-Hartman, Stanislaw Gruszka,
	David S. Miller, linux-wireless, netdev
In-Reply-To: <20190612142658.12792-1-gregkh@linuxfoundation.org>

Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> When calling debugfs functions, there is no need to ever check the
> return value.  This driver was saving the debugfs file away to be
> removed at a later time.  However, the 80211 core would delete the whole
> directory that the debugfs files are created in, after it asks the
> driver to do the deletion, so just rely on the 80211 core to do all of
> the cleanup for us, making us not need to keep a pointer to the dentries
> around at all.
> 
> This cleans up the structure of the driver data a bit and makes the code
> a tiny bit smaller.
> 
> Cc: Stanislaw Gruszka <sgruszka@redhat.com>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: linux-wireless@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

2 patches applied to wireless-drivers-next.git, thanks.

f503c7695343 iwlegacy: 3945: no need to check return value of debugfs_create functions
ffb92649f4d9 iwlegacy: 4965: no need to check return value of debugfs_create functions

-- 
https://patchwork.kernel.org/patch/10990125/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH v2 01/11] rtw88: add fast xmit support
From: Kalle Valo @ 2019-06-25  5:09 UTC (permalink / raw)
  To: yhchuang; +Cc: linux-wireless
In-Reply-To: <1560497055-17197-2-git-send-email-yhchuang@realtek.com>

<yhchuang@realtek.com> wrote:

> From: Yan-Hsuan Chuang <yhchuang@realtek.com>
> 
> With dynamic power save support, rtw88 is able to support fast tx
> path, claim it to mac80211.
> 
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>

11 patches applied to wireless-drivers-next.git, thanks.

e6fec313fa3f rtw88: add fast xmit support
44cc4c63a877 rtw88: add support for random mac scan
6fabdc4a34d0 rtw88: add beacon function setting
818d46e7715e rtw88: 8822c: add rf write protection when switching channel
f859e71f9615 rtw88: 8822c: update channel and bandwidth BB setting
e027446667b5 rtw88: 8822c: disable rx clock gating before counter reset
e1cc056c92f9 rtw88: 8822c: use more accurate ofdm fa counting
d41673b941f2 rtw88: power on again if it was already on
a11cddd42b67 rtw88: restore DACK results to save time
e9c87a3b744b rtw88: rsvd page should go though management queue
4a36de3996c7 rtw88: fix typo rtw_writ16_set

-- 
https://patchwork.kernel.org/patch/10994533/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH] p54: remove dead branch in op_conf_tx callback
From: Kalle Valo @ 2019-06-25  5:09 UTC (permalink / raw)
  To: Christian Lamparter; +Cc: linux-wireless
In-Reply-To: <20190615100009.14654-2-chunkeey@gmail.com>

Christian Lamparter <chunkeey@gmail.com> wrote:

> This patch removes the error branch for (queue > dev->queues).
> It is no longer needed anymore as the "queue" value is validated by
> cfg80211's parse_txq_params() before the driver code gets called.
> 
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>

Patch applied to wireless-drivers-next.git, thanks.

12e66ffbd534 p54: remove dead branch in op_conf_tx callback

-- 
https://patchwork.kernel.org/patch/10997025/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH v2 1/7] rt2x00: allow to specify watchdog interval
From: Kalle Valo @ 2019-06-25  5:10 UTC (permalink / raw)
  To: Stanislaw Gruszka
  Cc: linux-wireless, Tomislav Požega, Daniel Golle, Felix Fietkau,
	Mathias Kresin
In-Reply-To: <20190615100100.29800-2-sgruszka@redhat.com>

Stanislaw Gruszka <sgruszka@redhat.com> wrote:

> Allow subdriver to change watchdog interval by intialize
> link->watchdog_interval value before rt2x00link_register().
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>

7 patches applied to wireless-drivers-next.git, thanks.

9f3e3323e996 rt2x00: allow to specify watchdog interval
2034afe4db4a rt2800: add helpers for reading dma done index
759c5b599cf4 rt2800: initial watchdog implementation
09db3b000619 rt2800: add pre_reset_hw callback
710e6cc1595e rt2800: do not nullify initialization vector data
e403fa31ed71 rt2x00: add restart hw
0f47aeeada2a rt2800: do not enable watchdog by default

-- 
https://patchwork.kernel.org/patch/10997029/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH v2] airo: switch to skcipher interface
From: Kalle Valo @ 2019-06-25  5:12 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-wireless, linux-crypto, herbert, ebiggers, johannes, linux,
	Ard Biesheuvel
In-Reply-To: <20190617084338.24918-1-ard.biesheuvel@linaro.org>

Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> The AIRO driver applies a ctr(aes) on a buffer of considerable size
> (2400 bytes), and instead of invoking the crypto API to handle this
> in its entirety, it open codes the counter manipulation and invokes
> the AES block cipher directly.
> 
> Let's fix this, by switching to the sync skcipher API instead.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Patch applied to wireless-drivers-next.git, thanks.

e5db0ad7563c airo: switch to skcipher interface

-- 
https://patchwork.kernel.org/patch/10998553/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply

* Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Joe Perches @ 2019-06-25  5:17 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
	Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
	Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
	Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
	James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
	Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
	netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
	linux-fsdevel
In-Reply-To: <746098160c4ff6527d573d2af23c403b6d4e5b80.camel@d-silva.org>

On Tue, 2019-06-25 at 15:06 +1000, Alastair D'Silva wrote:
> On Mon, 2019-06-24 at 22:01 -0700, Joe Perches wrote:
> > On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> > > From: Alastair D'Silva <alastair@d-silva.org>
> > > 
> > > In order to support additional features, rename hex_dump_to_buffer
> > > to
> > > hex_dump_to_buffer_ext, and replace the ascii bool parameter with
> > > flags.
> > []
> > > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c
> > > b/drivers/gpu/drm/i915/intel_engine_cs.c
> > []
> > > @@ -1338,9 +1338,8 @@ static void hexdump(struct drm_printer *m,
> > > const void *buf, size_t len)
> > >  		}
> > >  
> > >  		WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
> > > -						rowsize, sizeof(u32),
> > > -						line, sizeof(line),
> > > -						false) >=
> > > sizeof(line));
> > > +						rowsize, sizeof(u32),
> > > line,
> > > +						sizeof(line)) >=
> > > sizeof(line));
> > 
> > Huh?  Why do this?
[]
> The change actions Jani's suggestion:
> https://lkml.org/lkml/2019/6/20/343

I think you need to read this change again.



^ permalink raw reply

* Re: [PATCH v4 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Joe Perches @ 2019-06-25  5:19 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
	Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
	Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
	Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
	James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
	Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
	netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
	linux-fsdevel
In-Reply-To: <746098160c4ff6527d573d2af23c403b6d4e5b80.camel@d-silva.org>

On Tue, 2019-06-25 at 15:06 +1000, Alastair D'Silva wrote:
> The change actions Jani's suggestion:
> https://lkml.org/lkml/2019/6/20/343

I suggest not changing any of the existing uses of
hex_dump_to_buffer and only use hex_dump_to_buffer_ext
when necessary for your extended use cases.




^ permalink raw reply

* Re: [PATCH v4 5/7] lib/hexdump.c: Allow multiple groups to be separated by lines '|'
From: Joe Perches @ 2019-06-25  5:37 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
	Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
	Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
	Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
	James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
	Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
	David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
	netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
	linux-fsdevel
In-Reply-To: <20190625031726.12173-6-alastair@au1.ibm.com>

On Tue, 2019-06-25 at 13:17 +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> With the wider display format, it can become hard to identify how many
> bytes into the line you are looking at.
> 
> The patch adds new flags to hex_dump_to_buffer() and print_hex_dump() to
> print vertical lines to separate every N groups of bytes.
> 
> eg.
> buf:00000000: 454d414e 43415053|4e495f45 00584544  NAMESPAC|E_INDEX.
> buf:00000010: 00000000 00000002|00000000 00000000  ........|........
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>  include/linux/printk.h |  3 +++
>  lib/hexdump.c          | 59 ++++++++++++++++++++++++++++++++++++------
>  2 files changed, 54 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/printk.h b/include/linux/printk.h
[]
> @@ -485,6 +485,9 @@ enum {
>  
>  #define HEXDUMP_ASCII			BIT(0)
>  #define HEXDUMP_SUPPRESS_REPEATED	BIT(1)
> +#define HEXDUMP_2_GRP_LINES		BIT(2)
> +#define HEXDUMP_4_GRP_LINES		BIT(3)
> +#define HEXDUMP_8_GRP_LINES		BIT(4)

These aren't really bits as only one value should be set
as 8 overrides 4 and 4 overrides 2.

I would also expect this to be a value of 2 in your above
example, rather than 8.  It's described as groups not bytes.

The example is showing a what would normally be a space ' '
separator as a vertical bar '|' every 2nd grouping.



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox