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

[Lukas Wunner <lukas@wunner.de>]

On Fri, Jul 05, 2024 at 12:22:33PM +0200, Stefan Wahren wrote:
> Am 05.07.24 um 10:48 schrieb Lukas Wunner:
> > A similar issue was reported for Agilex platforms back in 2021:
> > 
> > https://lore.kernel.org/all/5e8cbce0-3260-2971-484f-fc73a3b2bd28@synopsys.com/
> > 
> > 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.
> 
> 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?

I'd say mention both.  The Samsung one because it was the first
occurrence and the Agilex one because it specifically mentions
the interrupt storm which you've also witnessed on the Raspberry Pi.
Samsung's report mentions other symptoms than an interrupt storm.


> > 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.
> [2] -
> https://patches.linaro.org/project/linux-usb/patch/20210413215834.3126447-2-jeremy.linton@arm.com/

Careful there, the patch vaguely says...

    With that added and identified as "BCM2848",
    an id in use by other OSs for this device, the dw2
    controller on the BCM2711 will work.

...which sounds like they copy-pasted the BCM2848 id from somewhere else.
I would assume that BCM2848 is really a different SoC and not just
a different name for the BCM2835, but hopefully BroadCom folks will
be able to confirm or deny this (and thus the necessity of the quirk
on BCM2848 and not just on BCM2835).

Thanks,

Lukas