Am 05.07.24 um 10:48 schrieb Lukas Wunner:
On Thu, Jul 04, 2024 at 03:14:50PM +0100, Florian Fainelli wrote:
On 6/30/2024 4:36 PM, Stefan Wahren wrote:
On resume of the Raspberry Pi the dwc2 driver fails to enable
HCD_FLAG_HW_ACCESSIBLE before re-enabling the interrupts.
This causes a situation where both handler ignore a incoming port
interrupt and force the upper layers to disable the dwc2 interrupt line.
This leaves the USB interface in a unusable state:
irq 66: nobody cared (try booting with the "irqpoll" option)
CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 6.10.0-rc3
Hardware name: BCM2835
Call trace:
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x50/0x64
dump_stack_lvl from __report_bad_irq+0x38/0xc0
__report_bad_irq from note_interrupt+0x2ac/0x2f4
note_interrupt from handle_irq_event+0x88/0x8c
handle_irq_event from handle_level_irq+0xb4/0x1ac
handle_level_irq from generic_handle_domain_irq+0x24/0x34
generic_handle_domain_irq from bcm2836_chained_handle_irq+0x24/0x28
bcm2836_chained_handle_irq from generic_handle_domain_irq+0x24/0x34
generic_handle_domain_irq from generic_handle_arch_irq+0x34/0x44
generic_handle_arch_irq from __irq_svc+0x88/0xb0
A similar issue was reported for Agilex platforms back in 2021:
https://lore.kernel.org/all/5e8cbce0-3260-2971-484f-fc73a3b2bd28@xxxxxxxxxxxx/
It was fixed by commit 3d8d3504d233 ("usb: dwc2: Add platform specific
data for Intel's Agilex"), which sets the no_clock_gating flag on that
platform.
Looking at drivers/usb/dwc2/params.c, numerous other platforms need
the same flag.
Please amend the commit message to mention the Agilex issue and
resulting commit.
From my understanding Samsung noticed this issue at first and
introduced the no_clock_gating flag [1] and they referenced 0112b7ce68ea
("usb: dwc2: Update dwc2_handle_usb_suspend_intr function.") as I did in
this patch. Later some platforms like Rockchip and Agilex followed.
Should i better refer to the Samsung bugfix instead of the Agilex bugfix?
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -23,6 +23,7 @@ static void dwc2_set_bcm_params(struct dwc2_hsotg *hsotg)
p->max_transfer_size = 65535;
p->max_packet_count = 511;
p->ahbcfg = 0x10;
+ p->no_clock_gating = true;
Could we set this depending upon whether the dwc2 host controller is a
wake-up source for the system or not?
The flag seems to mean whether the platform is actually capable of
disabling the clock of the USB controller. BCM2835 seems to be
incapable and as a result, even though dwc2_host_enter_clock_gating()
is called, the chip signals interrupts.
That's why I was asking about this in the initial bug report. Since I
didn't get a reply, I submitted this workaround.
There doesn't seem to be a relation to using the controller as a
wakeup source, so your comment doesn't seem to make sense.
If the clock can't be gated, the chip can always serve as a
wakeup source.
I wouldn't conclude that the chip can always serve as a wakeup source
(e.g. power down the USB domain would also prevent this), but i agree
creating a relation between wakeup source and clock gating is a bad idea.
The real question is whether BCM2848 platforms likewise cannot disable
the clock of the dwc2 controller or whether this is specific to the
BCM2835. Right now dwc2_set_bcm_params() is applied to both the
BCM2848 and BCM2835. If the BCM2848 behaves differently in this
regard, we'd have to duplicate dwc2_set_bcm_params() for the BCM2835.
From my understand BCM2848 refers to the same SoC, but the ACPI
implementation uses a different ID [2]. So I think this is safe.
Thanks,
Lukas
[1] -
https://lore.kernel.org/linux-usb/20210716050127.4406-1-m.szyprowski@xxxxxxxxxxx/
[2] -
https://patches.linaro.org/project/linux-usb/patch/20210413215834.3126447-2-jeremy.linton@xxxxxxx/
