Re: [PATCH 09/11] usb: dwc2: Skip clock gating on Broadcom SoCs

Stefan Wahren
2024-07-04T15:34:06Z

Hi Florian,

Am 04.07.24 um 16:14 schrieb Florian Fainelli:
>
>
> 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
>> Exception stack(0xc1b01f20 to 0xc1b01f68)
>> 1f20: 0005c0d4 00000001 00000000 00000000 c1b09780 c1d6b32c c1b04e54
>> c1a5eae8
>> 1f40: c1b04e90 00000000 00000000 00000000 c1d6a8a0 c1b01f70 c11d2da8
>> c11d4160
>> 1f60: 60000013 ffffffff
>> __irq_svc from default_idle_call+0x1c/0xb0
>> default_idle_call from do_idle+0x21c/0x284
>> do_idle from cpu_startup_entry+0x28/0x2c
>> cpu_startup_entry from kernel_init+0x0/0x12c
>> handlers:
>> [<f539e0f4>] dwc2_handle_common_intr
>> [<75cd278b>] usb_hcd_irq
>> Disabling IRQ #66
>>
>> Disabling clock gatling workaround this issue.
>
> Typo: gatling/gating.
>
>>
>> Fixes: 0112b7ce68ea ("usb: dwc2: Update dwc2_handle_usb_suspend_intr
>> function.")
>> Link:
>> https://lore.kernel.org/linux-usb/3fd0c2fb-4752-45b3-94eb-42352703e1fd@gmx.net/T/
>> Signed-off-by: Stefan Wahren <wahrenst@gmx.net>
>> ---
>>   drivers/usb/dwc2/params.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
>> index 5a1500d0bdd9..66580de52882 100644
>> --- 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?

I would prefer to fix the suspend/resume behavior reported here [1]
instead of making tricky workarounds. But i don't have an idea how to
achieve this.

[1] -