* Re: [PATCH v2] dma-debug: suppress cacheline overlap warning when arch has no DMA alignment requirement
From: Marek Szyprowski @ 2026-03-30 7:44 UTC (permalink / raw)
To: Mikhail Gavrilov, robin.murphy
Cc: iommu, linux-kernel, linux-usb, linux-mm, harry, vbabka, akpm,
stern, linux, andy.shevchenko, hch, Jeff.kirsher, catalin.marinas
In-Reply-To: <20260327124156.24820-1-mikhail.v.gavrilov@gmail.com>
On 27.03.2026 13:41, Mikhail Gavrilov wrote:
> When CONFIG_DMA_API_DEBUG is enabled, the DMA debug infrastructure
> tracks active mappings per cacheline and warns if two different DMA
> mappings share the same cacheline ("cacheline tracking EEXIST,
> overlapping mappings aren't supported").
>
> On x86_64, ARCH_KMALLOC_MINALIGN defaults to 8, so small kmalloc
> allocations (e.g. the 8-byte hub->buffer and hub->status in the USB
> hub driver) frequently land in the same 64-byte cacheline. When both
> are DMA-mapped, this triggers a false positive warning.
>
> This has been reported repeatedly since v5.14 (when the EEXIST check
> was added) across various USB host controllers and devices including
> xhci_hcd with USB hubs, USB audio devices, and USB ethernet adapters.
>
> The cacheline overlap is only a real concern on architectures that
> require DMA buffer alignment to cacheline boundaries (i.e. where
> ARCH_DMA_MINALIGN >= L1_CACHE_BYTES). On architectures like x86_64
> where dma_get_cache_alignment() returns 1, the hardware is
> cache-coherent and overlapping cacheline mappings are harmless.
>
> Suppress the EEXIST warning when dma_get_cache_alignment() is less
> than L1_CACHE_BYTES, indicating the architecture does not require
> cacheline-aligned DMA buffers.
>
> Verified with a kernel module reproducer that performs two kmalloc(8)
> allocations back-to-back and DMA-maps both:
>
> Before: allocations share a cacheline, EEXIST fires within ~50 pairs
> After: same cacheline pair found, but no warning emitted
>
> Fixes: 2b4bbc6231d7 ("dma-debug: report -EEXIST errors in add_dma_entry")
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=215740
> Suggested-by: Harry Yoo <harry@kernel.org>
> Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
> Signed-off-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Applied to dma-mapping-fixes. Thanks!
> ---
>
> v1 -> v2:
> - Moved fix from include/linux/slab.h (ARCH_KMALLOC_MINALIGN)
> to kernel/dma/debug.c per Harry Yoo's suggestion.
> - Instead of forcing cacheline-aligned allocations, suppress
> the warning when the architecture has no DMA alignment
> requirement (dma_get_cache_alignment() < L1_CACHE_BYTES).
>
> v1: https://lore.kernel.org/all/20260327055846.248829-1-mikhail.v.gavrilov@gmail.com/
>
> Reproducer module that triggers the bug reliably:
> https://bugzilla.kernel.org/attachment.cgi?id=309769
>
> kernel/dma/debug.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/dma/debug.c b/kernel/dma/debug.c
> index 0677918f06a8..1a725edbbbf6 100644
> --- a/kernel/dma/debug.c
> +++ b/kernel/dma/debug.c
> @@ -615,6 +615,7 @@ static void add_dma_entry(struct dma_debug_entry *entry, unsigned long attrs)
> } else if (rc == -EEXIST &&
> !(attrs & DMA_ATTR_SKIP_CPU_SYNC) &&
> !(entry->is_cache_clean && overlap_cache_clean) &&
> + dma_get_cache_alignment() >= L1_CACHE_BYTES &&
> !(IS_ENABLED(CONFIG_DMA_BOUNCE_UNALIGNED_KMALLOC) &&
> is_swiotlb_active(entry->dev))) {
> err_printk(entry->dev, entry,
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH v1 0/4] usb: serial: mxuport: extend MXU50U support and runtime controls
From: Crescent Hsieh @ 2026-03-30 7:24 UTC (permalink / raw)
To: Greg Kroah-Hartman, Johan Hovold; +Cc: linux-usb, linux-kernel
In-Reply-To: <20260324035041.352190-1-crescentcy.hsieh@moxa.com>
Hi,
Gentle ping on this series.
Please let me know if I should revise or split anything.
Related firmware patch:
https://lore.kernel.org/all/20260324024635.349522-1-crescentcy.hsieh@moxa.com/
Thanks,
Crescent Hsieh
^ permalink raw reply
* Re: [PATCH 1/2] usb: chipidea: core: fix device mode not work in non-lpm
From: Peter Chen (CIX) @ 2026-03-30 7:14 UTC (permalink / raw)
To: Xu Yang; +Cc: gregkh, jun.li, linux-usb, linux-kernel, imx
In-Reply-To: <7n6s3luk32sk36y47u6axedhysnrrin3radc7mgxh2pjzwdzv6@v3yu3tg4moiq>
On 26-03-27 18:54:45, Xu Yang wrote:
> On Wed, Mar 25, 2026 at 03:08:49PM +0800, Peter Chen (CIX) wrote:
> > On 26-03-19 17:57:15, Xu Yang wrote:
> > > In current design, we expect 2 ci_irq() to handle ID and VBUS events in
> > > usb role switch, like what ci_extcon_wakeup_int() does. Now we only call
> > > ci_irq() once. However, this won't bring any issues in low power mode,
> > > because ci_irq() just take the device out of low power mode, and then
> > > ci_extcon_wakeup_int() will call ci_irq() twice. If the device is not in
> > > suspend state, the device mode will not work properly because VBUS event
> > > won'tbe handled (ID event has higher priority) at all.
> >
> > %s/won'tbe/won't be
>
> OK.
>
> >
> > Is it possible change ci_irq_handler and handle both events?
>
> Yes, we can change ci_irq_handler() and let it set both ci->id_event and
> ci->b_sess_valid_event flags, then queue a ci_otg_work() to handle them
> later.
>
> I think this just unnecessarily call ci_irq_handler() to handle lpm/non-lpm
> case as the final path is ci_otg_work() and it will handle lpm/non-lpm case
> by naturally calling pm_runtime_get/put_sync(), otherwise it relies on
> ci_extcon_wakeup_int() to achieve the same purpose.
>
> Both methods work for me, may I know which one do you prefer? :)
My main concerns are below code, and it may confuse the reader.
ci->id_event = true;
ci->b_sess_valid_event = true;
I may prefer the first option, it is easy to understand.
Peter
>
> >
> > >
> > > Although 2 consecutive ci_irq() can work around the issue, do not do it
> > > because ci_usb_role_switch_set() may or not be in low power context which
> > > make the ci_irq() purpose not unique here. Because the final processing
> > > is in ci_otg_work(), just directly queue an otg work. This also refine
> > > the logic for more clarity and not set changed flag.
> > >
> > > Fixes: e1b5d2bed67c ("usb: chipidea: core: handle usb role switch in a common way")
> > > Cc: stable@vger.kernel.org
> > > Reviewed-by: Jun Li <jun.li@nxp.com>
> > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> > > ---
> > > drivers/usb/chipidea/core.c | 30 +++++++++++-------------------
> > > 1 file changed, 11 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
> > > index fac11f20cf0a..1bd231a852a1 100644
> > > --- a/drivers/usb/chipidea/core.c
> > > +++ b/drivers/usb/chipidea/core.c
> > > @@ -618,30 +618,22 @@ static int ci_usb_role_switch_set(struct usb_role_switch *sw,
> > > struct ci_hdrc *ci = usb_role_switch_get_drvdata(sw);
> > > struct ci_hdrc_cable *cable;
> > >
> > > - if (role == USB_ROLE_HOST) {
> > > - cable = &ci->platdata->id_extcon;
> > > - cable->changed = true;
> > > + cable = &ci->platdata->id_extcon;
> > > + if (role == USB_ROLE_HOST)
> > > cable->connected = true;
> > > - cable = &ci->platdata->vbus_extcon;
> > > - cable->changed = true;
> > > - cable->connected = false;
> > > - } else if (role == USB_ROLE_DEVICE) {
> > > - cable = &ci->platdata->id_extcon;
> > > - cable->changed = true;
> > > + else
> > > cable->connected = false;
> > > - cable = &ci->platdata->vbus_extcon;
> > > - cable->changed = true;
> > > +
> > > + cable = &ci->platdata->vbus_extcon;
> > > + if (role == USB_ROLE_DEVICE)
> > > cable->connected = true;
> > > - } else {
> > > - cable = &ci->platdata->id_extcon;
> > > - cable->changed = true;
> > > - cable->connected = false;
> > > - cable = &ci->platdata->vbus_extcon;
> > > - cable->changed = true;
> > > + else
> > > cable->connected = false;
> > > - }
> > >
> > > - ci_irq(ci);
> > > + ci->id_event = true;
> > > + ci->b_sess_valid_event = true;
> >
> > Why both ID and VBUS event are set as true unconditionally?
>
> The main purpose is to simplify the handling of the various situations.
>
> The usb role include below types:
> - host
> - device
> - none.
>
> 1. host <--> none
> Above change means ID change occur.
>
> 2. device <--> none
> Above change means VBUS change occur.
>
> 3. host <--> device
> Above change means ID and VBUS change occur.
>
> By setting both of them as true, the logic can be simplified here and
> ID and VBUS otg work will check if a real state change happen by comparing
> old state and current OTGSC_ID/OTGSC_BSV bit.
>
--
Best regards,
Peter
^ permalink raw reply
* [PATCH v2] dt-bindings: connector: add pd-disable dependency
From: Xu Yang @ 2026-03-30 6:35 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, gregkh, badhri, amitsd, kyletso
Cc: devicetree, linux-kernel, imx, linux-usb
When Power Delivery is not supported, the source is unable to obtain the
current capability from the Source PDO. As a result, typec-power-opmode
needs to be added to advertise such capability.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Fixes: 7a4440bc0d86 ("dt-bindings: connector: Add pd-disable property")
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
Changes in v2:
- add acked by tag
- DTS patch has been picked by Frank
---
Documentation/devicetree/bindings/connector/usb-connector.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index a00b239960a3..8ca0292490a2 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -335,6 +335,7 @@ properties:
maximum: 100000
dependencies:
+ pd-disable: [typec-power-opmode]
sink-vdos-v1: [ sink-vdos ]
sink-vdos: [ sink-vdos-v1 ]
--
2.34.1
^ permalink raw reply related
* Re: [PATCH] usb: cdns3: gadget: fix resource leak on gadget init failure
From: Peter Chen (CIX) @ 2026-03-30 5:52 UTC (permalink / raw)
To: Yongchao Wu
Cc: Pawel Laszczak, Roger Quadros, Greg Kroah-Hartman, linux-usb,
linux-kernel
In-Reply-To: <20260328151551.61228-1-yongchao.wu@autochips.com>
On 26-03-28 23:15:50, Yongchao Wu wrote:
> When cdns3_gadget_start() fails, the DRD gadget is left in an
> initialized state, causing a resource leak. When switching to host
> mode, the stale state triggers a synchronous external abort in
> xhci_gen_setup(), leading to kernel panic:
>
> [ 1301.377716][ T5000][E] Internal error: synchronous external abort: 96000010 [#1] PREEMPT SMP
> [ 1301.382485][ T5000][I] pc : xhci_gen_setup+0xa4/0x408
> [ 1301.393391][ T5000][I] backtrace:
> ...
> xhci_gen_setup+0xa4/0x408 <-- CRASH
> xhci_plat_setup+0x44/0x58
> usb_add_hcd+0x284/0x678
> ...
> cdns_role_set+0x9c/0xbc <-- Role switch
>
> Fix by calling cdns_drd_gadget_off() in the error path to properly
> clean up the DRD gadget state. This prevents both the resource leak
> and the kernel panic during role switching.
>
Good catch.
I have two questions:
- What exactly resource is leaked?
- Do you call cdns_drd_host_on before setup host?
Besides, you may add Fixed-by tag and Cc to stable tree.
Peter
> Signed-off-by: Yongchao Wu <yongchao.wu@autochips.com>
> ---
> drivers/usb/cdns3/cdns3-gadget.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
> index d59a60a16..4cc3f2ffb 100644
> --- a/drivers/usb/cdns3/cdns3-gadget.c
> +++ b/drivers/usb/cdns3/cdns3-gadget.c
> @@ -3428,6 +3428,7 @@ static int __cdns3_gadget_init(struct cdns *cdns)
> ret = cdns3_gadget_start(cdns);
> if (ret) {
> pm_runtime_put_sync(cdns->dev);
> + cdns_drd_gadget_off(cdns);
> return ret;
> }
>
> --
> 2.43.0
>
>
--
Best regards,
Peter
^ permalink raw reply
* Re: [PATCH v2] usbip: tools: Add usbip host driver availability check
From: Zongmin Zhou @ 2026-03-30 3:10 UTC (permalink / raw)
To: Shuah Khan
Cc: i, linux-kernel, linux-usb, valentina.manea.m, Zongmin Zhou,
Greg KH
In-Reply-To: <3b0aad9f-b424-41b0-9efe-60a1252153f6@linuxfoundation.org>
On 2026/3/28 02:29, Shuah Khan wrote:
> On 3/27/26 11:51, Shuah Khan wrote:
>> On 3/27/26 02:39, Zongmin Zhou wrote:
>>>
>>> On 2026/3/27 02:43, Shuah Khan wrote:
>>>> On 3/26/26 02:43, Greg KH wrote:
>>
>> [removed text]
>>
>>>>
>>>> The problem Zongmin is trying fix ish when usbipd starts, it looks for
>>>> exported if any - if it doesn't find any it assumes there aren't any
>>>> exported and doesn't detect that usbip_host driver isn't loaded.
>>>>
>>>> refresh_exported_devices() doesn't have the logic and it shouldn't
>>>> include that logic because this hook is called periodically to
>>>> refresh the list of exported devices. Starting usbipd and loading
>>>> the driver are distinct steps and without any dependencies.
>>>>
>>>> This patch he is trying to add that detection so a message can be
>>>> printed
>>>> to say "load the driver".
>>>>
>>>> A message can be added easily to cover two cases:
>>>>
>>>> 1. usbip_host driver isn't loaded
>>>> 2. There are no exported devices.
>>>>
>>>> refresh_exported_devices() will not find any devices in both
>>>> of the above scenarios. It isn't an error if it can't find
>>>> any exported devices.
>>>>
>>>> An informational message when refresh_exported_devices()
>>>> when it doesn't find any devices could help users.
>>>>
>>>> Zongmin,
>>>>
>>>> Would adding a message that says
>>>> "Check if usbip_host driver is loaded or export devices"
>>>> solve the problem of hard to debug problem you are addressing here?
>>>>
>>> Shuah,
>>>
>>> Your suggestion makes sense.
>>> Adding an informational message when no devices are found would be a
>>> simple
>>> and clean solution that helps users debug without being intrusive.
>>>
>>> However, I plan to add the info() message in
>>> usbip_generic_driver_open() instead of
>>> refresh_exported_devices(), because:
>>> - usbip_generic_driver_open() is called only once at initialization.
>>> - refresh_exported_devices() is called periodically to refresh the
>>> exported device list.
>>
>> refresh_exported_devices() isn't called periodically - it is called
>> from usbip_host_driver op: refresh_device_list and it will be called
>> whenever usbipd (host side) calls it whenever it receives a request from
>> user-space via process_request()
>>
>> For example running "usbip list -l" command will trigger a run of
>> refresh_exported_devices() via usbip_host_driver op: refresh_device_list
>>
>> I don't think it will that noisy to add a message to
>> refresh_exported_devices()
>> when device list is zero. Currently the logic doesn't detect device
>> list zero.
>> You have add that condition to print informational message.
>>
>>
>>> - When the server has no exported devices, having zero devices
>>> is normal and not worth frequent info messages.
>>
>> That is correct. Zero exported devices isn't an error and this could
>> persist until devices are exported.
>>
>>>
>>> Theoretically, we only need to prompt once at startup. Is my
>>> understanding correct?> > I'll also remove the existing error
>>> messages like below,
>>> since they cannot accurately determine whether the driver is loaded:
>>>
>>> if (ret)
>>> err("please load " USBIP_CORE_MOD_NAME ".ko and "
>>> USBIP_HOST_DRV_NAME ".ko!");
>>
>> Leave this one alone, because it gets called from a couple of places.
In usbip_generic_driver_open(), the only path that triggers this message
is a failure of udev_new().
This function fails due to system-level issues like memory exhaustion,
not because usbip driver module is missing.
Furthermore, since refresh_exported_devices() doesn't practically return
an error here,
the message is never seen during actual driver loading failures.
So I think it’s better to remove this inaccurate hint to avoid confusing
the users.
This is the reason why I previously wanted to remove it.
>>
>
> Better yet, why not change the usbip instead - usbip_list for example.
> This is the one that prints the device list and the change can be made
> there when the list is zero to say, "Check if driver is loaded and
> exported devices"
I think placing the check/message in refresh_exported_devices() would be
more effective.
This function covers all scenarios where the device list is refreshed,
including:
usbipd startup, usbip list -r, and usbip attach operations.
This way, users receive helpful feedback automatically without needing
to explicitly run usbip list -r.
>
> thanks,
> -- Shuah
^ permalink raw reply
* Re: correctly handling EPROTO
From: Alan Stern @ 2026-03-30 1:32 UTC (permalink / raw)
To: Michal Pecio; +Cc: Thinh Nguyen, Oliver Neukum, Bjørn Mork, USB list
In-Reply-To: <20260329184611.0afa92c7.michal.pecio@gmail.com>
On Sun, Mar 29, 2026 at 06:46:11PM +0200, Michal Pecio wrote:
> On Sat, 28 Mar 2026 21:52:37 -0400, Alan Stern wrote:
> > Storage is not a good example. It's so obviously critical that a
> > huge amount of effort has gone into making it exceptionally robust.
> > Very few of the other drivers do as good a job of error recovery.
>
> I though it's a good example because all the effort is in vain if
> xhci runs subsequent URBs before the driver even knows about it.
While I can't speak about the uas driver, usb-storage submits only one
URB at a time for command and status. For data transfers it uses the
scatter-gather library in core/message.c, which does queue up a whole
list of URBs. But the library is very careful, when any error occurs,
to have the completion handler immediately unlink all the outstanding
URBs in the queue (see sg_complete()).
uas has to work with USB streams, which no doubt makes everything even
more complicated.
> > Other drivers that might be affected include things like HID
>
> HID doesn't submit multiple URBs so there is nothing to restart after
> an error, but it's affected by the unilateral toggle reset issue.
>
> And by the way, is there any chance that ohci-hcd also clears host
> toggle on errors? I'm doing simple test with a keyboard (low speed
> interrupt in): disconnect D+ which causes some ETIME errors, connect
> it back, press and hold 'u'. Half the time uuuuu shows up, half the
> time only when I later add shift does UUUUU show up. Looks like the
> first packet is dropped due to toggle. I see it on xhci (known bug)
> and also on ohci, but never on ehci with a TT hub.
OHCI maintains its data toggle values in data structures that are stored
in the host computer's memory and accessed/updated via coherent DMA.
The data toggles are not updated when an error occurs -- but I don't
remember all the details of what the driver does, so something funky
could be happening.
It's hard to say what's going on with your hardware tests. You'd have
to use a bus analyzer to be certain.
> > various serial protocols
>
> If you mean usbserial, they are pretty boring. On completion they just
> submit the next URB, except for EPIPE, ENODEV etc. There is data loss
> on OUT endpoints, there can be more data loss due to toggle mismatch,
> and throwing out already queued URBs would probably add more loss.
>
> It seems usbserial won't care whether we restart instantly or not.
I was also thinking of things like cdc-acm and cdc-wdm. They do
maintain queues of URBs, and I don't know how they handle errors.
> > network drivers, video & audio
>
> And I suspect they are similar, because higher layers can tolerate loss
> so why bother recovering from USB errors. I may play with them later.
>
> BTW, I think audio only runs over isochronous. But I should have a bulk
> video device somethere.
There may be a few oddball devices doing things like that. A lot of
video devices use the uvcvideo driver, which (as far as I know) relies
entirely on isochronous transfers for its streaming data.
Alan Stern
^ permalink raw reply
* Re: xhci_hcd: AMD Raphael/Granite Ridge USB 2.0 xHCI [1022:15b8] dies on resume from suspend
From: Michal Pecio @ 2026-03-30 0:07 UTC (permalink / raw)
To: martinalderson; +Cc: linux-usb
In-Reply-To: <CA+_z3hRjPnQOrEp7uXsNsu6wzqQp9O6FT18i=XcSC7k74Xd2QA@mail.gmail.com>
On Sun, 29 Mar 2026 17:52:39 -0400, martinalderson@gmail.com wrote:
> [BUG] xhci_hcd 0000:0f:00.0: controller declared dead on resume from
> suspend
>
> Hardware:
> CPU: AMD Ryzen 9 7900 12-Core Processor
> Board: ASUS PRIME B650-PLUS
> Controller: AMD Raphael/Granite Ridge USB 2.0 xHCI [1022:15b8]
> Subsystem: ASUSTeK Computer Inc. [1043:8877]
> PCI: 0000:0f:00.0 (IOMMU group 30)
>
> Software:
> Kernel: 7.0.0-rc5 (commit be762d8b, built 2026-03-28)
> Distro: Fedora 43 (Workstation)
> Desktop: GNOME on Wayland
>
> Description:
> On the first suspend/resume cycle after boot, the xHCI controller at
> 0000:0f:00.0 (AMD Raphael/Granite Ridge USB 2.0) fails to resume and
> is declared dead. A Logitech Unifying Receiver (046d:c52b) on this
> controller is disconnected and the mouse (Logitech M720 Triathlon)
> stops functioning.
>
> A second xHCI controller on the same system (0000:0c:00.0, AMD 600
> Series Chipset USB 3.2 [1022:43f7]) also errors on resume (USBSTS
> 0x401) but successfully recovers via reinit. The 0f:00.0 controller
> does not recover.
>
> Regression from rc4: suspend/resume worked correctly on 7.0-rc4 and
> earlier kernels on the same hardware.
That's interesting because there were no USB subsystem changes
between 7.0-rc4 and 7.0-rc5.
Any chance you could git-bisect this?
Are both kernels built with the same .config?
> Reproduce:
> 1. Boot with USB device attached to a port on the 0000:0f:00.0
> controller
> 2. Suspend (systemd suspend)
> 3. Resume
By the way, are you using this affected controller to resume
(with a keyboard or something like that)?
> dmesg on resume:
> xhci_hcd 0000:0f:00.0: xHCI host not responding to stop endpoint command
> xhci_hcd 0000:0f:00.0: xHCI host controller not responding, assume dead
> xhci_hcd 0000:0f:00.0: HC died; cleaning up
> xhci_hcd 0000:0c:00.0: xHC error in resume, USBSTS 0x401, Reinit
> usb usb1: root hub lost power or was reset
> usb usb2: root hub lost power or was reset
> usb 1-7: WARN: invalid context state for evaluate context command.
> usb 1-10: WARN: invalid context state for evaluate context command.
> usb 7-1: USB disconnect, device number 2
>
> Workaround:
> PCI remove + rescan recovers the controller:
> echo 1 > /sys/bus/pci/devices/0000:0f:00.0/remove
> echo 1 > /sys/bus/pci/rescan
>
> A simple PCI device reset (echo 1 > .../reset) was insufficient -- the
> controller came back but did not re-enumerate the attached device.
What about the unbind/bind procedure described here?
https://bugzilla.kernel.org/show_bug.cgi?id=221073
> Notes:
> - The 0f:00.0 controller is USB 2.0 only (USB3 root hub has no ports)
> - hci version 0x120, hcc params 0x0110ffc5, quirks 0x0000000200000010
^ permalink raw reply
* xhci_hcd: AMD Raphael/Granite Ridge USB 2.0 xHCI [1022:15b8] dies on resume from suspend
From: martinalderson @ 2026-03-29 21:52 UTC (permalink / raw)
To: linux-usb
[BUG] xhci_hcd 0000:0f:00.0: controller declared dead on resume from suspend
Hardware:
CPU: AMD Ryzen 9 7900 12-Core Processor
Board: ASUS PRIME B650-PLUS
Controller: AMD Raphael/Granite Ridge USB 2.0 xHCI [1022:15b8]
Subsystem: ASUSTeK Computer Inc. [1043:8877]
PCI: 0000:0f:00.0 (IOMMU group 30)
Software:
Kernel: 7.0.0-rc5 (commit be762d8b, built 2026-03-28)
Distro: Fedora 43 (Workstation)
Desktop: GNOME on Wayland
Description:
On the first suspend/resume cycle after boot, the xHCI controller at
0000:0f:00.0 (AMD Raphael/Granite Ridge USB 2.0) fails to resume and
is declared dead. A Logitech Unifying Receiver (046d:c52b) on this
controller is disconnected and the mouse (Logitech M720 Triathlon)
stops functioning.
A second xHCI controller on the same system (0000:0c:00.0, AMD 600
Series Chipset USB 3.2 [1022:43f7]) also errors on resume (USBSTS
0x401) but successfully recovers via reinit. The 0f:00.0 controller
does not recover.
Regression from rc4: suspend/resume worked correctly on 7.0-rc4 and
earlier kernels on the same hardware.
Reproduce:
1. Boot with USB device attached to a port on the 0000:0f:00.0 controller
2. Suspend (systemd suspend)
3. Resume
dmesg on resume:
xhci_hcd 0000:0f:00.0: xHCI host not responding to stop endpoint command
xhci_hcd 0000:0f:00.0: xHCI host controller not responding, assume dead
xhci_hcd 0000:0f:00.0: HC died; cleaning up
xhci_hcd 0000:0c:00.0: xHC error in resume, USBSTS 0x401, Reinit
usb usb1: root hub lost power or was reset
usb usb2: root hub lost power or was reset
usb 1-7: WARN: invalid context state for evaluate context command.
usb 1-10: WARN: invalid context state for evaluate context command.
usb 7-1: USB disconnect, device number 2
Workaround:
PCI remove + rescan recovers the controller:
echo 1 > /sys/bus/pci/devices/0000:0f:00.0/remove
echo 1 > /sys/bus/pci/rescan
A simple PCI device reset (echo 1 > .../reset) was insufficient -- the
controller came back but did not re-enumerate the attached device.
Notes:
- The 0f:00.0 controller is USB 2.0 only (USB3 root hub has no ports)
- hci version 0x120, hcc params 0x0110ffc5, quirks 0x0000000200000010
^ permalink raw reply
* Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also
From: Linus Torvalds @ 2026-03-29 20:01 UTC (permalink / raw)
To: Jakub Kicinski
Cc: David Howells, netdev, Marc Dionne, David S. Miller, Eric Dumazet,
Paolo Abeni, linux-afs, linux-kernel, Mathieu Desnoyers,
John Johansen, Minas Harutyunyan, Simon Horman, apparmor,
linux-usb, stable
In-Reply-To: <20260329121208.6092419d@kernel.org>
On Sun, 29 Mar 2026 at 12:12, Jakub Kicinski <kuba@kernel.org> wrote:
>
> Could someone with sufficient weight to their name ack this?
I don't particularly like it. I think the name is too generic, and
it's all wrong anyway. Whether something is on a list or not ends up
being much too specific to the use-case, and I do *not* see a huge
advantage to a helper function that just wraps "list_empty()" with
another name that is actually *less* descriptive.
So no. NAK.
As you mention, the RCU version at least does something else, but
honestly, it looks pretty damn questionable too. And no, it does not
work for non-RCU lists in any kind of generic sense, since iut's
perfectly valid to remove list entries without poisoning them.,
For example, some places that want a simple "am I on a list" will use
__list_del_clearprev(), which does *not* poison the prev pointer, but
just clears it instead. And then the "am I on a list" is just checking
prev for NULL or not.
In other words: all of this is wrong. Whether you are on a list or not
is simply not a generic operation. It depends on the user.
The name is also *MUCH* too generic anyway.
Linus
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] r8152: Add support for the RTL8157 hardware
From: Jakub Kicinski @ 2026-03-29 19:34 UTC (permalink / raw)
To: mail
Cc: Jakub Kicinski, andrew+netdev, davem, edumazet, pabeni, linux-usb,
netdev, linux-kernel
In-Reply-To: <20260324-rtl8157_next-v4-2-034312b12de5@birger-koblitz.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8152: Add support for the RTL8157 hardware
This commit adds support for the RTL8157 chip, including its new 16-byte
descriptor format, SRAM/ADV interfaces, and PHY setup. It reuses existing
initialization functions where possible while branching for chip-specific
logic.
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index bab8e10e5f99..010126e6dae4 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -1629,6 +1862,28 @@ static u16 sram_read(struct r8152 *tp, u16 addr)
> return ocp_reg_read(tp, OCP_SRAM_DATA);
> }
>
> +static void sram_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
> +{
> + u16 data = sram_read(tp, addr);
> +
> + data = (data & ~clear) | set;
> + ocp_reg_write(tp, OCP_SRAM_DATA, data);
> +}
Does this sequence write the modified data back to the wrong address?
Indirect SRAM access ports typically auto-increment their internal address
pointer upon read. Since sram_read() reads OCP_SRAM_DATA, the address
pointer likely advances, causing the subsequent ocp_reg_write() to corrupt
the adjacent register instead of the intended one. Should this use
sram_write(tp, addr, data) to reset the address before writing?
> +static u16 sram2_read(struct r8152 *tp, u16 addr)
> +{
> + ocp_reg_write(tp, OCP_SRAM2_ADDR, addr);
> + return ocp_reg_read(tp, OCP_SRAM2_DATA);
> +}
> +
> +static void sram2_write_w0w1(struct r8152 *tp, u16 addr, u16 clear, u16 set)
> +{
> + u16 data = sram2_read(tp, addr);
> +
> + data = (data & ~clear) | set;
> + ocp_reg_write(tp, OCP_SRAM2_DATA, data);
> +}
Is the same auto-increment issue present here with OCP_SRAM2_DATA?
[ ... ]
> @@ -4080,6 +4473,14 @@ static void rtl8156_runtime_enable(struct r8152 *tp, bool enable)
> }
> }
>
> +static void rtl8157_runtime_enable(struct r8152 *tp, bool enable)
> +{
> + r8153_queue_wake(tp, enable);
> + r8153b_u1u2en(tp, false);
> + r8157_u2p3en(tp, false);
> + rtl_runtime_suspend_enable(tp, enable);
> +}
Does this function break power management upon the first suspend and resume
cycle? Unlike rtl8156_runtime_enable() which checks the enable parameter,
this implementation unconditionally calls r8153b_u1u2en(tp, false) and
r8157_u2p3en(tp, false) even when resuming.
[ ... ]
> @@ -6693,7 +7052,7 @@ static void rtl8156_up(struct r8152 *tp)
> r8153_aldps_en(tp, true);
> r8153_u2p3en(tp, true);
>
> - if (tp->udev->speed >= USB_SPEED_SUPER)
> + if (tp->version != RTL_VER_16 && tp->udev->speed >= USB_SPEED_SUPER)
> r8153b_u1u2en(tp, true);
> }
Because r8153_u2p3en(tp, true) is called unconditionally just before the
modified lines, will RTL8157 incorrectly write to the legacy USB_U2P3_CTRL
register instead of using its specific r8157_u2p3en() function?
[ ... ]
> @@ -6706,13 +7065,13 @@ static void rtl8156_down(struct r8152 *tp)
> return;
> }
>
> - ocp_data = ocp_read_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3);
> - ocp_data |= PLA_MCU_SPDWN_EN;
> - ocp_write_word(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, ocp_data);
> + ocp_word_set_bits(tp, MCU_TYPE_PLA, PLA_MAC_PWR_CTRL3, PLA_MCU_SPDWN_EN);
>
> r8153b_u1u2en(tp, false);
> - r8153_u2p3en(tp, false);
> - r8153b_power_cut_en(tp, false);
> + if (tp->version != RTL_VER_16) {
> + r8153_u2p3en(tp, false);
> + r8153b_power_cut_en(tp, false);
> + }
> r8153_aldps_en(tp, false);
While the legacy registers are correctly avoided for RTL_VER_16 here, are we
missing calls to r8157_u2p3en(tp, false) and r8157_power_cut_en(tp, false)?
If omitted, these power states might remain permanently enabled when the
interface goes down.
[ ... ]
> @@ -8362,10 +8825,21 @@ static void r8156b_init(struct r8152 *tp)
>
> set_bit(GREEN_ETHERNET, &tp->flags);
>
> - /* rx aggregation */
> - ocp_data = ocp_read_word(tp, MCU_TYPE_USB, USB_USB_CTRL);
> - ocp_data &= ~(RX_AGG_DISABLE | RX_ZERO_EN);
> - ocp_write_word(tp, MCU_TYPE_USB, USB_USB_CTRL, ocp_data);
> + /* rx aggregation / 16 bytes Rx descriptor */
> + if (tp->version == RTL_VER_16)
> + ocp_word_clr_bits(tp, MCU_TYPE_USB, USB_USB_CTRL, RX_AGG_DISABLE | RX_DESC_16B);
Is it intended to clear the RX_DESC_16B bit for RTL_VER_16? The driver uses
sizeof(struct rx_desc_v2) which is a 16-byte descriptor for RTL8157.
Clearing this bit might configure the hardware to send 8-byte descriptors
instead, leading to packet parsing corruption in rx_bottom().
[ ... ]
> @@ -9647,6 +10158,29 @@ static int rtl_ops_init(struct r8152 *tp)
> tp->rx_buf_sz = 32 * 1024;
> tp->eee_en = true;
> tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
> + r8152_desc_init(tp);
> + break;
> +
> + case RTL_VER_16:
> + tp->eee_en = true;
> + tp->eee_adv = MDIO_EEE_1000T | MDIO_EEE_100TX;
> + tp->eee_adv2 = MDIO_EEE_2_5GT | MDIO_EEE_5GT;
> + ops->init = r8156_init;
> + ops->enable = rtl8156_enable;
> + ops->disable = rtl8153_disable;
> + ops->up = rtl8156_up;
> + ops->down = rtl8156_down;
> + ops->unload = rtl8153_unload;
Will using rtl8153_unload for RTL_VER_16 result in incorrect power cut
teardown? rtl8153_unload calls the legacy r8153_power_cut_en(tp, false).
RTL8157 seems to require r8157_power_cut_en(tp, false) to properly clear
USB_MISC_2 bit 1 and PCUT_STATUS on module unload.
^ permalink raw reply
* Re: [PATCH net-next v4 1/2] r8152: Add support for 5Gbit Link Speeds and EEE
From: Jakub Kicinski @ 2026-03-29 19:34 UTC (permalink / raw)
To: mail
Cc: Jakub Kicinski, andrew+netdev, davem, edumazet, pabeni, linux-usb,
netdev, linux-kernel
In-Reply-To: <20260324-rtl8157_next-v4-1-034312b12de5@birger-koblitz.de>
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
r8152: Add support for 5Gbit Link Speeds and EEE
This commit adds support for 5GBit link speeds and EEE to the RTL8157
driver. It updates the speed and duplex settings, ethtool integration,
and EEE advertisement parameters to handle the new 5G capabilities.
> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
> index 3b6d4252d34c..bab8e10e5f99 100644
> --- a/drivers/net/usb/r8152.c
> +++ b/drivers/net/usb/r8152.c
> @@ -604,6 +604,7 @@ enum spd_duplex {
> FORCE_100M_FULL,
> FORCE_1000M_FULL,
> NWAY_2500M_FULL,
> + NWAY_5000M_FULL,
> };
Are there missing switch cases for NWAY_5000M_FULL in the hardware UPS
flag configurations, such as r8156_ups_flags?
It looks like if a 5Gbps link is established, it might fall through to the
default case and leave the speed bits as 0. Since ups_flags_speed(0) maps
to FORCE_10M_HALF, this could cause the device's clock scaling and power
management features to be misconfigured for 10M Half-Duplex instead of the
5Gbps link.
[ ... ]
> @@ -725,6 +726,7 @@ enum spd_duplex {
> #define BP4_SUPER_ONLY 0x1578 /* RTL_VER_04 only */
>
> enum rtl_register_content {
> + _5000bps = BIT(12),
> _2500bps = BIT(10),
Is it intentional that the MAC idle speed down feature (IDLE_SPDWN_EN) is
disabled for _2500bps but remains enabled for _5000bps?
In functions like rtl8156_enable, the code checks if the speed is _2500bps
to disable the idle speed down feature, but the new _5000bps constant is
not included in those checks. This might cause latency spikes or instability
at 5Gbps if the power-saving feature is enabled.
[ ... ]
> @@ -6343,15 +6356,20 @@ static int rtl8152_set_speed(struct r8152 *tp, u8 autoneg, u32 speed, u8 duplex,
> r8152_mdio_write(tp, MII_CTRL1000, new1);
> }
>
> - if (tp->support_2500full) {
> + if (tp->support_2500full || tp->support_5000full) {
> orig = ocp_reg_read(tp, OCP_10GBT_CTRL);
> - new1 = orig & ~MDIO_AN_10GBT_CTRL_ADV2_5G;
> + new1 = orig & ~(MDIO_AN_10GBT_CTRL_ADV2_5G | MDIO_AN_10GBT_CTRL_ADV5G);
>
> if (advertising & RTL_ADVERTISED_2500_FULL) {
> new1 |= MDIO_AN_10GBT_CTRL_ADV2_5G;
> tp->ups_info.speed_duplex = NWAY_2500M_FULL;
> }
>
> + if (advertising & RTL_ADVERTISED_5000_FULL) {
> + new1 |= MDIO_AN_10GBT_CTRL_ADV5G;
> + tp->ups_info.speed_duplex = NWAY_5000M_FULL;
> + }
> +
Could this unintentionally advertise 5GbE on a 2.5GbE-only adapter if
userspace requests it?
The earlier check in this function:
if (!(advertising & support))
return -EINVAL;
only ensures that at least one supported speed is requested, but it does
not strip unsupported speeds from the advertising mask. Since this block
is now entered when support_2500full is true, checking the advertising mask
for RTL_ADVERTISED_5000_FULL without verifying tp->support_5000full could
erroneously write the 5G bit to the PHY and misconfigure the hardware
state.
[ ... ]
> @@ -8793,6 +8814,19 @@ int rtl8152_get_link_ksettings(struct net_device *netdev,
> cmd->base.speed = SPEED_2500;
> }
>
> + if (tp->support_5000full) {
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
> + cmd->link_modes.advertising,
> + ocp_reg_read(tp, OCP_10GBT_CTRL) & MDIO_AN_10GBT_CTRL_ADV5G);
> +
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
> + cmd->link_modes.lp_advertising,
> + ocp_reg_read(tp, OCP_10GBT_STAT) & MDIO_AN_10GBT_STAT_LP5G);
> +
> + if (is_speed_5000(rtl8152_get_speed(tp)))
> + cmd->base.speed = SPEED_5000;
> + }
Since 5G adapters like the RTL8157 will have both support_2500full and
support_5000full set, will this result in duplicate blocking USB control
transfers?
The ocp_reg_read calls for OCP_10GBT_CTRL and OCP_10GBT_STAT, along with
rtl8152_get_speed, are also executed in the preceding support_2500full
block. Reading these into local variables before the capability checks
would avoid the overhead of duplicate hardware accesses.
[ ... ]
> @@ -8994,6 +9032,13 @@ static int r8153_get_eee(struct r8152 *tp, struct ethtool_keee *eee)
> linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT, common);
> }
>
> + if (tp->support_5000full) {
> + linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, eee->supported);
> +
> + if (speed & _5000bps)
> + linkmode_set_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT, common);
> + }
In r8153_get_eee, the reading of OCP_EEE_ADV2 and OCP_EEE_LPABLE2 is
performed inside the if (tp->support_2500full) block that precedes this new
code.
Since those registers contain the EEE capability bits for both 2.5G
and 5G speeds, what happens if a device has support_5000full enabled but
support_2500full disabled? It looks like the registers would never be read,
and the 5Gbps EEE capabilities would fail to populate correctly.
--
pw-bot: cr
^ permalink raw reply
* Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also
From: Jakub Kicinski @ 2026-03-29 19:14 UTC (permalink / raw)
To: David Howells
Cc: netdev, Marc Dionne, David S. Miller, Eric Dumazet, Paolo Abeni,
linux-afs, linux-kernel, Mathieu Desnoyers, John Johansen,
Minas Harutyunyan, Simon Horman, apparmor, linux-usb, stable
In-Reply-To: <20260326131838.634095-5-dhowells@redhat.com>
On Thu, 26 Mar 2026 13:18:29 +0000 David Howells wrote:
> Unfortunately, list_empty() is not usable with an entry that has been
> removed from a list with list_del_rcu() as ->next must be left pointing at
> the following entry so as not to break traversal under RCU.
This seems to break build for jffs.
Someone already marked this as rejected in PW.
Whoever it was PLEASE STOP.
Quoting documentation:
Updating patch status
~~~~~~~~~~~~~~~~~~~~~
Contributors and reviewers do not have the permissions to update patch
state directly in patchwork. Patchwork doesn't expose much information
about the history of the state of patches, therefore having multiple
people update the state leads to confusion.
Instead of delegating patchwork permissions netdev uses a simple mail
bot which looks for special commands/lines within the emails sent to
the mailing list. For example to mark a series as Changes Requested
one needs to send the following line anywhere in the email thread::
pw-bot: changes-requested
As a result the bot will set the entire series to Changes Requested.
This may be useful when author discovers a bug in their own series
and wants to prevent it from getting applied.
The use of the bot is entirely optional, if in doubt ignore its
existence completely. Maintainers will classify and update the state
of the patches themselves. No email should ever be sent to the list
with the main purpose of communicating with the bot, the bot commands
should be seen as metadata.
The use of the bot is restricted to authors of the patches (the
``From:`` header on patch submission and command must match!),
maintainers of the modified code according to the MAINTAINERS file
(again, ``From:`` must match the MAINTAINERS entry) and a handful of
senior reviewers.
Bot records its activity here:
https://netdev.bots.linux.dev/pw-bot.html
See:
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html#updating-patch-status
^ permalink raw reply
* Re: [PATCH net v3 04/11] list: Move on_list_rcu() to list.h and add on_list() also
From: Jakub Kicinski @ 2026-03-29 19:12 UTC (permalink / raw)
To: David Howells
Cc: netdev, Marc Dionne, David S. Miller, Eric Dumazet, Paolo Abeni,
linux-afs, linux-kernel, Mathieu Desnoyers, John Johansen,
Minas Harutyunyan, Simon Horman, apparmor, linux-usb, stable,
Linus Torvalds
In-Reply-To: <20260326131838.634095-5-dhowells@redhat.com>
On Thu, 26 Mar 2026 13:18:29 +0000 David Howells wrote:
> diff --git a/include/linux/list.h b/include/linux/list.h
> index 00ea8e5fb88b..d224e7210d1b 100644
> --- a/include/linux/list.h
> +++ b/include/linux/list.h
> @@ -381,6 +381,32 @@ static inline int list_empty(const struct list_head *head)
> return READ_ONCE(head->next) == head;
> }
>
> +/**
> + * on_list - Test whether an entry is on a list.
> + * @entry: The entry to check
> + *
> + * Test whether an entry is on a list. Safe to use on an entry initialised
> + * with INIT_LIST_HEAD() or LIST_HEAD() or removed with things like
> + * list_del_init(). Not safe for use with list_del() or list_del_rcu().
> + */
> +static inline bool on_list(const struct list_head *entry)
> +{
> + return !list_empty(entry);
> +}
> +
> +/**
> + * on_list_rcu - Test whether an entry is on a list (RCU-del safe).
> + * @entry: The entry to check
> + *
> + * Test whether an entry is on a list. Safe to use on an entry initialised
> + * with INIT_LIST_HEAD() or LIST_HEAD() or removed with things like
> + * list_del_init(). Also safe for use with list_del() or list_del_rcu().
> + */
> +static inline bool on_list_rcu(const struct list_head *entry)
> +{
> + return !list_empty(entry) && entry->prev != LIST_POISON2;
> +}
Could someone with sufficient weight to their name ack this?
The non-RCU version of on_list() does not sit well with me.
It provides no additional semantics above list_empty() and
the uninit / poison-related gotchas more obvious when typing
!list_empty(&entry->list).
I can believe the RCU version is more useful. It could probably
be used on both RCU and non-RCU entries?
Last minor nit - the list API consistently uses list_ as a prefix.
I have no better name to suggest but it's sad that on_list_rcu()
breaks that.
I think you're missing a READ_ONCE(), BTW.
^ permalink raw reply
* Re: correctly handling EPROTO
From: Michal Pecio @ 2026-03-29 16:46 UTC (permalink / raw)
To: Alan Stern; +Cc: Thinh Nguyen, Oliver Neukum, Bjørn Mork, USB list
In-Reply-To: <22c70ca7-57dc-4328-a5cc-d46c4f73556f@rowland.harvard.edu>
On Sat, 28 Mar 2026 21:52:37 -0400, Alan Stern wrote:
> Storage is not a good example. It's so obviously critical that a
> huge amount of effort has gone into making it exceptionally robust.
> Very few of the other drivers do as good a job of error recovery.
I though it's a good example because all the effort is in vain if
xhci runs subsequent URBs before the driver even knows about it.
> Other drivers that might be affected include things like HID
HID doesn't submit multiple URBs so there is nothing to restart after
an error, but it's affected by the unilateral toggle reset issue.
And by the way, is there any chance that ohci-hcd also clears host
toggle on errors? I'm doing simple test with a keyboard (low speed
interrupt in): disconnect D+ which causes some ETIME errors, connect
it back, press and hold 'u'. Half the time uuuuu shows up, half the
time only when I later add shift does UUUUU show up. Looks like the
first packet is dropped due to toggle. I see it on xhci (known bug)
and also on ohci, but never on ehci with a TT hub.
> various serial protocols
If you mean usbserial, they are pretty boring. On completion they just
submit the next URB, except for EPIPE, ENODEV etc. There is data loss
on OUT endpoints, there can be more data loss due to toggle mismatch,
and throwing out already queued URBs would probably add more loss.
It seems usbserial won't care whether we restart instantly or not.
> network drivers, video & audio
And I suspect they are similar, because higher layers can tolerate loss
so why bother recovering from USB errors. I may play with them later.
BTW, I think audio only runs over isochronous. But I should have a bulk
video device somethere.
Regards,
Michal
^ permalink raw reply
* Re: [PATCH 0/3] USB/UVC: Add quirks to prevent Razer Kiyo Pro xHCI cascade failure
From: Michal Pecio @ 2026-03-29 15:40 UTC (permalink / raw)
To: Jeffrey Hein
Cc: Laurent Pinchart, Hans de Goede, Greg Kroah-Hartman, linux-media,
linux-usb, stable
In-Reply-To: <CAD5VvzDWF7SO0Aytp3K_uXV6ZYoqEqN1dhfv7VtMAHSpHP+qTA@mail.gmail.com>
On Sun, 29 Mar 2026 08:03:42 -0700, Jeffrey Hein wrote:
> I have now tested on 6.17.0-19-generic (Ubuntu 25.04) with dynamic
> debug enabled for xhci_hcd and usbcore, and without any of my proposed
> patches or workarounds applied. No udev quirks, no LPM disable, no
> control throttle -- completely stock kernel.
>
> Results: the stress test passes 50/50 rounds with 0ms delay. On
> 6.8.0-106-generic the same test crashed consistently around round 25.
Thanks for the update, that's good to hear.
Hopefully it will still work without dynamic debug too.
If you would want to fix the old 6.8 kernel you will need to talk with
Ubuntu about it, because that version is no longer supported upstream.
Regards,
Michal
^ permalink raw reply
* Re: [PATCH 0/3] USB/UVC: Add quirks to prevent Razer Kiyo Pro xHCI cascade failure
From: Jeffrey Hein @ 2026-03-29 15:03 UTC (permalink / raw)
To: Michal Pecio
Cc: Laurent Pinchart, Hans de Goede, Greg Kroah-Hartman, linux-media,
linux-usb, stable
In-Reply-To: <20260323085845.6bf57b3b.michal.pecio@gmail.com>
Hi Michal,
I have now tested on 6.17.0-19-generic (Ubuntu 25.04) with dynamic
debug enabled for xhci_hcd and usbcore, and without any of my proposed
patches or workarounds applied. No udev quirks, no LPM disable, no
control throttle -- completely stock kernel.
Results: the stress test passes 50/50 rounds with 0ms delay. On
6.8.0-106-generic the same test crashed consistently around round 25.
The xHCI error handling changes between 6.8 and 6.17 appear to have
resolved the cascade failure. The controller no longer escalates to
hc_died() when the device firmware stalls.
The UVC probe control EPIPE (-32) still occurs at device init:
uvcvideo 2-3.4:1.1: Failed to set UVC probe control : -32 (exp. 26).
And with dynamic debug enabled, the kernel logs show it dynamically
disabling U1 LPM when the device responds slowly:
usb 2-3.4: Hub-initiated U1 disabled due to long timeout 16800us
So the kernel is now handling both the LPM issues and the error
recovery gracefully without needing the quirks I proposed.
I have not tested on non-Intel hardware. The firmware is already at
the latest version (1.5.0.1) per Razer's standalone updater.
Full debug log from the stress test is available at:
https://github.com/jphein/kiyo-xhci-fix
Given these results, it seems like the patch series may no longer be
needed for current kernels. Happy to provide any additional testing or
logs.
JP
On Mon, Mar 23, 2026 at 12:58 AM Michal Pecio <michal.pecio@gmail.com> wrote:
>
> On Sun, 22 Mar 2026 15:10:28 -0700, Jeffrey Hein wrote:
> > Both failure modes are in the device firmware (version 8.21), not the
> > kernel, so they exist on any kernel version. On 6.8.0-106-generic
> > (where I tested), the TRB_STOP_RING case in
> > xhci_handle_command_timeout() goes straight to xhci_halt() +
> > xhci_hc_died() without attempting per-device recovery.
>
> Command timeout is a failure of the xHCI controller, not the device,
> and as Alan said, it's generally not supposed to happen so we are
> curious how it happens and if it can be prevented in xhci-hcd.
>
> Device behavior may be a contributing factor, as can be a kernel bug
> or controller HW bug. It would be helpful if somebody tried this on
> non-Intel hardware and on current kernels, because there were various
> changes to xHCI error handling over the last two years.
>
> > The stress test script is in the series repository:
> >
> > https://github.com/jphein/kiyo-xhci-fix
> >
> > stress-test-kiyo.sh exercises UVC controls via v4l2-ctl at maximum
> > rate -- brightness, contrast, saturation, white balance, exposure,
> > focus, pan/tilt/zoom -- cycling through their full ranges each round.
> > With 0ms delay between controls, the crash consistently occurs around
> > round 25 of 50 (~5-10 seconds of sustained rapid SET_CUR).
>
> OK, I will see if it does anything interesting on my hardware, but it
> may be nothing because I don't have this camera.
>
> Did you try it on a different camera in the same USB port?
>
> > That said, the firmware lockup itself is controller-independent -- the
> > device stops responding to USB control transfers regardless of the
> > host controller. What varies is the host controller's response to the
> > resulting stop-endpoint timeout. On 6.8, xhci-hcd takes the
> > TRB_STOP_RING timeout straight to hc_died()
>
> Nope, this is controller dependent because Stop Endpoint is a command
> to the controller and it has no reason to fail. Something is broken.
>
> Could you boot a newer kernel (compile 7.0-rc5 yourself or at least get
> latest release (or beta) of your distribution), enable dynamic debug
>
> echo 'module xhci_hcd +p' >/proc/dynamic_debug/control
> echo 'module usbcore +p' >/proc/dynamic_debug/control
>
> then connect the camera, crash it again and send dmesg output?
>
> Regards,
> Michal
--
Jeffrey Pine Hein
Just plain helpful.
jphein.com ☀️ techempower.org
(530) 798-4099
^ permalink raw reply
* [PATCH RFC net-next] usb: gadget: u_ether: fix eth_get_drvinfo() oops
From: Russell King (Oracle) @ 2026-03-29 11:32 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Guilherme G. Piccoli, Kees Cook, linux-usb, netdev, Tony Luck
The following oops was observed on the nVidia Jetson Xavier NX1
platform with 7.0.0-rc5, which is caused by dev->gadget becoming NULL
while the u_ether device is still visible to userspace. Adding some
debug reveals that we see this sequence:
net usb1: gadget detached
tegra-xudc 3550000.usb: ep 0 disabled
net usb1: no gadget for drvinfo
usb1 (unregistering): left allmulticast mode
usb1 (unregistering): left promiscuous mode
l4tbr0: port 2(usb1) entered disabled state
Put a sticky plaster over this (it's racy!) I suspect
gether_detach_gadget() needs to at least take the RTNL as well to
prevent ethtool ops running concurrently with the gadget being
detached.
Unable to handle kernel NULL pointer dereference at virtual address 000000000000
00d0
Mem abort info:
ESR = 0x0000000096000004
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
CM = 0, WnR = 0, TnD = 0, TagAccess = 0
GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=00000001086b1000
[00000000000000d0] pgd=0000000000000000, p4d=0000000000000000
Internal error: Oops: 0000000096000004 [#1] SMP
Modules linked in: snd_soc_tegra210_mvc snd_soc_tegra186_asrc snd_soc_tegra210_m
ixer snd_soc_tegra210_admaif snd_soc_tegra_pcm snd_soc_tegra210_ope snd_soc_tegr
a186_dspk snd_soc_tegra210_i2s snd_soc_simple_card_utils snd_soc_tegra210_dmic s
nd_soc_tegra210_amx snd_soc_tegra210_adx snd_soc_tegra210_sfc rtw88_8822ce rtw88
_8822c rtw88_pci rtw88_core mac80211 libarc4 sha256 cfg80211 snd_soc_tegra210_ah
ub snd_soc_core snd_pcm_dmaengine snd_pcm snd_timer tegra210_adma zram tegra_drm
zsmalloc usb_f_ncm syscopyarea usb_f_mass_storage drm_client_lib sysfillrect sy
simgblt ramoops fb_sys_fops reed_solomon nls_iso8859_1 drm_display_helper nls_cp437 vfat drm_kms_helper fb fat cec drm_dp_aux_bus tegra_aconnect usb_f_acm u_serial usb_f_rndis u_ether max77620_wdt realtek tegra_xudc phy_package snd phy_tegra194_p2u dwmac_dwc_qos_eth soundcore stmmac_platform pwm_tegra rtc_efi tegra186_gpc_dma virt_dma tegra_bpmp_thermal host1x pcie_tegra194 libcomposite udc_core drm drm_panel_orientation_quirks backlight efivarfs ip_tables
x_tables
CPU: 1 UID: 0 PID: 4181 Comm: systemd-udevd Not tainted 7.0.0-rc5-net-next+ #640 PREEMPT
Hardware name: NVIDIA NVIDIA Jetson Xavier NX Developer Kit/Jetson, BIOS 6.0-37391689 08/28/2024
pstate: 80400009 (Nzcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : eth_get_drvinfo+0x4c/0x80 [u_ether]
lr : eth_get_drvinfo+0x40/0x80 [u_ether]
sp : ffff80008484bb60
x29: ffff80008484bb60 x28: ffff000083de18c0 x27: 0000000000000000
x26: 0000000000000000 x25: ffff0000947da0a8 x24: 0000000000004000
x23: ffff0000a38a5600 x22: 0000000000000000 x21: ffff80007a3a5558
x20: ffff0000947da000 x19: ffff0000a38a5608 x18: 0000000000000000
x17: 0000000000000000 x16: 0000000000000000 x15: 0000ffffe09af760
x14: 0000000000000000 x13: 0000000000000020 x12: 0101010101010101
x11: 7f7f7f7f7f7f7f7f x10: 00007ffff0071c50 x9 : 0000000000000020
x8 : 0101010101010101 x7 : ffff0000a38a5634 x6 : fefefefefefefeff
x5 : ffff0000a38a562c x4 : 0000000000383030 x3 : 0000000000383030
x2 : 0000000000000020 x1 : 0000000000000000 x0 : ffff0000a38a564c
Call trace:
eth_get_drvinfo+0x4c/0x80 [u_ether] (P)
ethtool_get_drvinfo+0x54/0x1e0
__dev_ethtool+0x698/0x2000
dev_ethtool+0x90/0x1a0
dev_ioctl+0x37c/0x594
sock_ioctl+0x390/0x5bc
__arm64_sys_ioctl+0x408/0xeb4
invoke_syscall.constprop.0+0x50/0xe0
do_el0_svc+0x40/0xc0
el0_svc+0x48/0x2a0
el0t_64_sync_handler+0xa0/0xe4
el0t_64_sync+0x19c/0x1a0
Code: 95a68811 f9468a81 d2800402 91011260 (f9406821)
---[ end trace 0000000000000000 ]---
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
1. get_maintainers.pl is picking up on the PSTORE folk because of the
mention of "ramoops" in the commit message... even though this has
nothing to do with them.
2. I think there's more bugs here as mentioned in the commit message,
since setting dev->gadget to NULL without taking any locking while
the u_ether device is still published to userspace looks mightily
dangerous. Seems to be another kernel driver that fails to obey the
basics discussed in OLS some twenty years ago concerning the order
of setup/publish/unpublish/teardown... and we still don't seem to
be able to get it right. :(
3. Patch generated against the net-next tree, because that's what I'm
working with being focussed on stmmac at the moment - but I would like
my test platform not to oops and reboot itself when I instead want it
to power off.
drivers/usb/gadget/function/u_ether.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c
index 1a9e7c495e2e..d1ae91889537 100644
--- a/drivers/usb/gadget/function/u_ether.c
+++ b/drivers/usb/gadget/function/u_ether.c
@@ -113,6 +113,10 @@ static void eth_get_drvinfo(struct net_device *net, struct ethtool_drvinfo *p)
strscpy(p->driver, "g_ether", sizeof(p->driver));
strscpy(p->version, UETH__VERSION, sizeof(p->version));
+
+ if (!dev->gadget)
+ return;
+
strscpy(p->fw_version, dev->gadget->name, sizeof(p->fw_version));
strscpy(p->bus_info, dev_name(&dev->gadget->dev), sizeof(p->bus_info));
}
--
2.47.3
^ permalink raw reply related
* [PATCH] usb: cdns3: gadget: fix NULL pointer dereference in ep_queue
From: Yongchao Wu @ 2026-03-29 1:34 UTC (permalink / raw)
To: Peter Chen
Cc: Pawel Laszczak, Roger Quadros, Greg Kroah-Hartman, linux-usb,
linux-kernel, Yongchao Wu
In-Reply-To: <20260328143842.57315-1-yongchao.wu@autochips.com>
When the gadget endpoint is disabled or not yet configured, the ep->desc
pointer can be NULL. This leads to a NULL pointer dereference when
__cdns3_gadget_ep_queue() is called, causing a kernel crash.
Add a check to return -ESHUTDOWN if ep->desc is NULL, which is the
standard return code for unconfigured endpoints.
This prevents potential crashes when ep_queue is called on endpoints
that are not ready.
Signed-off-by: Yongchao Wu <yongchao.wu@autochips.com>
---
drivers/usb/cdns3/cdns3-gadget.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
index d59a60a16..96d2a4c38 100644
--- a/drivers/usb/cdns3/cdns3-gadget.c
+++ b/drivers/usb/cdns3/cdns3-gadget.c
@@ -2589,6 +2589,9 @@ static int __cdns3_gadget_ep_queue(struct usb_ep *ep,
struct cdns3_request *priv_req;
int ret = 0;
+ if (!ep->desc)
+ return -ESHUTDOWN;
+
request->actual = 0;
request->status = -EINPROGRESS;
priv_req = to_cdns3_request(request);
--
2.43.0
^ permalink raw reply related
* Re: UAS peripheral with unrecognized TRIM support
From: Greg KH @ 2026-03-29 6:21 UTC (permalink / raw)
To: Johan Gill; +Cc: linux-usb
In-Reply-To: <1717beab-848b-4927-8fa5-26fb5ae05495@bahnhof.se>
On Sat, Mar 28, 2026 at 05:23:48PM +0100, Johan Gill wrote:
> Hi, I have tested running Linux on the StarTech USB312SAT3CB USB 3.1 -> SATA
> adapter. It's a more stable UAS implementation than my previous adapter but
> the device is not recognized as supporting TRIM out of the box (kernel
> 6.19.9). The technical specifications at
> https://www.startech.com/en-se/hdd/usb312sat3cb indicate that TRIM is
> supported, so I enabled it by setting ATTR{provisioning_mode}="unmap" in a
> udev rule and then fstrim indeed seems to work when I run it. The
> specification claims that it uses ASMedia ASM235CM, but the device uses the
> StarTech vendor id and identifies as 14b0:0207.
>
> It would be nice to have the adapter recognized as more capable out of the
> box. Should I aim at patching drivers/usb/storage/unusual_uas.h?
Isn't that a scsi issue, not a USB/UAS issue to be reporting that value
properly? That sysfs attribute comes from the storage portion of the
kernel, not the USB portion.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] usb: common: fix all alignment warnings in usb-otg-fsm.c
From: Greg KH @ 2026-03-29 6:20 UTC (permalink / raw)
To: Stanisław Maciej Molsa; +Cc: peter.chen, linux-usb
In-Reply-To: <acgQzzfSTX-5foU-@stanislaw-QEMU-Virtual-Machine>
On Sat, Mar 28, 2026 at 05:33:03PM +0000, Stanisław Maciej Molsa wrote:
> Hi Peter,
>
> This is my first small contribution to the USB subsystem.
No need for this here.
>
> Fixed all "Alignment should match open parenthesis" warnings in
> usb-otg-fsm.c, as
> reported by checkpatch.pl
>
> Signed-off-by: Stanisław Maciej Molsa <stanislawmolsa@gmail.com>
> ---
> drivers/usb/common/usb-otg-fsm.c | 42 ++++++++++++++++----------------
> 1 file changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/usb/common/usb-otg-fsm.c b/drivers/usb/common/usb-otg-fsm.c
> index e11803225775..5dcaa6c2057f 100644
> --- a/drivers/usb/common/usb-otg-fsm.c
> +++ b/drivers/usb/common/usb-otg-fsm.c
> @@ -32,7 +32,7 @@ static int otg_set_protocol(struct otg_fsm *fsm, int protocol)
>
> if (fsm->protocol != protocol) {
> VDBG("Changing role fsm->protocol= %d; new protocol= %d\n",
> - fsm->protocol, protocol);
> + fsm->protocol, protocol);
Please do not do coding style changes EXCEPT in subsystems that
specifically ask for them to be done in (like drivers/staging/),
otherwise there will be nothing but constant churn in the kernel tree.
thanks,
greg k-h
^ permalink raw reply
* [PATCH v2] usb: cdns3: gadget: fix NULL pointer dereference in ep_queue
From: Yongchao Wu @ 2026-03-29 3:20 UTC (permalink / raw)
To: peter.chen; +Cc: pawell, rogerq, gregkh, linux-usb, linux-kernel, Yongchao Wu
When the gadget endpoint is disabled or not yet configured, the ep->desc
pointer can be NULL. This leads to a NULL pointer dereference when
__cdns3_gadget_ep_queue() is called, causing a kernel crash.
Add a check to return -ESHUTDOWN if ep->desc is NULL, which is the
standard return code for unconfigured endpoints.
This prevents potential crashes when ep_queue is called on endpoints
that are not ready.
Signed-off-by: Yongchao Wu <yongchao.wu@autochips.com>
---
Changes in v2:
- Fix author name format (use "Yongchao Wu" instead of "yongchao.wu")
---
drivers/usb/cdns3/cdns3-gadget.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
index d59a60a16..96d2a4c38 100644
--- a/drivers/usb/cdns3/cdns3-gadget.c
+++ b/drivers/usb/cdns3/cdns3-gadget.c
@@ -2589,6 +2589,9 @@ static int __cdns3_gadget_ep_queue(struct usb_ep *ep,
struct cdns3_request *priv_req;
int ret = 0;
+ if (!ep->desc)
+ return -ESHUTDOWN;
+
request->actual = 0;
request->status = -EINPROGRESS;
priv_req = to_cdns3_request(request);
--
2.43.0
^ permalink raw reply related
* Re: correctly handling EPROTO
From: Alan Stern @ 2026-03-29 1:52 UTC (permalink / raw)
To: Michal Pecio; +Cc: Thinh Nguyen, Oliver Neukum, Bjørn Mork, USB list
In-Reply-To: <20260328222217.297200bd.michal.pecio@gmail.com>
On Sat, Mar 28, 2026 at 10:22:17PM +0100, Michal Pecio wrote:
> On Sat, 21 Mar 2026 11:58:53 -0400, Alan Stern wrote:
> > On Sat, Mar 21, 2026 at 06:54:24AM +0100, Michal Pecio wrote:
> > > Nope, for many years now, if not forever, xhci-hcd has been
> > > restarting the endpoint after giving back the failed URB if its
> > > completion hasn't unlinked all remaining URBs.
> >
> > How can that work in the presence of BH givebacks?
>
> Certainly not reliably and I started a similar thread two years ago
> after coming to this exact realization.
>
> Does anyone know class drivers affected by this which could be used
> to validate such changes? Writing a patch is one thing, knowing whether
> it does any good is another. I recall that last time Mathias tried to
> touch this logic it caused a regression by unearthing more issues.
>
> I was reluctant to touch this mess in absence of known impact. The race
> is as old as BH giveback (2019) and automatic restarting is even older.
> It could get awkward if users (or driver developers) learned to expect
> this behavior.
>
> But if somebody can point out serious issues like data loss in storage
> then it's a different ball game.
Storage is not a good example. It's so obviously critical that a huge
amount of effort has gone into making it exceptionally robust. Very few
of the other drivers do as good a job of error recovery.
Other drivers that might be affected include things like HID, various
serial protocols, network drivers, video & audio. Those are what come
to mind when I try to think of the most consequential USB drivers; maybe
other people will add a few more. But a lot of class drivers are in
relatively less widespread use.
At any rate, I think most of the people on this thread have agreed that
automatic restart of endpoints following transaction errors is not the
best approach. Automatic cancellation of later URBs in the queue, on
the other hand, might be a useful service, and it's something that
usbcore could do fairly easily. Automatic clear-halt is more
questionable, and I would leave it out for now, at least.
Alan Stern
^ permalink raw reply
* [Bug 221293] New: Thunderbolt: "failed to calculate available bandwidth" causes DisplayPort tunnel drop with Apple Studio Display
From: bugzilla-daemon @ 2026-03-28 22:26 UTC (permalink / raw)
To: linux-usb
https://bugzilla.kernel.org/show_bug.cgi?id=221293
Bug ID: 221293
Summary: Thunderbolt: "failed to calculate available bandwidth"
causes DisplayPort tunnel drop with Apple Studio
Display
Product: Drivers
Version: 2.5
Hardware: All
OS: Linux
Status: NEW
Severity: normal
Priority: P3
Component: USB
Assignee: drivers_usb@kernel-bugs.kernel.org
Reporter: aylin.ahmed@jetbot.co.uk
Regression: No
Created attachment 309779
--> https://bugzilla.kernel.org/attachment.cgi?id=309779&action=edit
dmesg thunderbolt log and hardware details
Apple Studio Display connected via Thunderbolt to ThinkPad X1 Carbon
intermittently loses DisplayPort tunnel. The kernel logs:
thunderbolt 0000:00:0d.3: 0:12 <-> 1:20 (USB3): failed to calculate available
bandwidth
followed by "failed to reach state TB_PORT_UP" and device disconnect.
USB3 hub (05ac:8014) and DP contend for bandwidth. Manually unbinding the USB3
hub restores DP, confirming contention. Suspend/resume recovers correctly but
DPMS wake does not.
Hardware: ThinkPad X1 Carbon, Intel Gen12 TB controller, kernel
6.8.0-106-generic, Ubuntu 24.x Wayland.
Full dmesg and hardware details attached.
--
You may reply to this email to add a comment.
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: correctly handling EPROTO
From: Michal Pecio @ 2026-03-28 21:22 UTC (permalink / raw)
To: Alan Stern; +Cc: Thinh Nguyen, Oliver Neukum, Bjørn Mork, USB list
In-Reply-To: <d3fd1c0b-d0cf-40e2-9f21-b4c5de1c421b@rowland.harvard.edu>
On Sat, 21 Mar 2026 11:58:53 -0400, Alan Stern wrote:
> On Sat, Mar 21, 2026 at 06:54:24AM +0100, Michal Pecio wrote:
> > Nope, for many years now, if not forever, xhci-hcd has been
> > restarting the endpoint after giving back the failed URB if its
> > completion hasn't unlinked all remaining URBs.
>
> How can that work in the presence of BH givebacks?
Certainly not reliably and I started a similar thread two years ago
after coming to this exact realization.
Does anyone know class drivers affected by this which could be used
to validate such changes? Writing a patch is one thing, knowing whether
it does any good is another. I recall that last time Mathias tried to
touch this logic it caused a regression by unearthing more issues.
I was reluctant to touch this mess in absence of known impact. The race
is as old as BH giveback (2019) and automatic restarting is even older.
It could get awkward if users (or driver developers) learned to expect
this behavior.
But if somebody can point out serious issues like data loss in storage
then it's a different ball game.
Regards,
Michal
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox