[PATCH V2 14/16] WIP: usb: dwc2: Implement recovery after PM domain off

[Stefan Wahren]

Hi Doug,

Am 15.08.24 um 21:37 schrieb Doug Anderson:
> Hi,
>
> On Wed, Aug 14, 2024 at 2:48 PM Stefan Wahren <wahrenst at gmx.net> wrote:
>>>> You're saying that your
>>>> registers get saved _unless_ the power domain gets turned off, right?
>> On BCM2835 there is no need to store the registers because there is no
>> power management supported by USB core except of the power domain. So
>> DWC2 don't expect a register loss.
>>>> ...and the device core keeps power domains on for suspended devices if
>>>> they are wakeup sources, which makes sense.
>>>>
>>>> So with that, your patch sounds like a plausible way to do it. I guess
>>>> one other way to do it would be some sort of "canary" approach. You
>>>> could _always_ save registers and then, at resume time, you could
>>>> detect if some "canary" register had reset to its power-on default. If
>>>> you see this then you can assume power was lost and re-init all the
>>>> registers. This could be pretty much any register that you know won't
>>>> be its power on default. In some ways a "canary" approach is uglier
>>>> but it also might be more reliable across more configurations?
>> I don't have enough knowledge about DWC2 and i also don't have the
>> databook to figure out if there is a magic register which could be used
>> for the canary approach. But all these different platforms, host vs
>> gadget role, different low modes let me think the resulting solution
>> would be also fragile and ugly.
> I won't admit to having a DWC2 databook. ;-)
you convinced me of the canary approach. I missed one critical point the
whole time. The used power domain notifier can/will be called while the
USB clock is disabled. So this worked for the Raspberry Pi because the
clock is fixed.
> ...but don't think it's too hard to find a good canary. What about
> "GAHBCFG_GLBL_INTR_EN" ? From a quick glance it looks like the driver
> seems to set that bit during driver startup and then it stays on until
> driver shutdown. The databook that I definitely won't admit to having
> almost certainly says that this register resets to 0 on all hardware
> and it's applicable to both host and device. I think you could say
> that if the register is 0 at resume time that registers must have been
> lost and you can restore them.
There are several reason to not use the GAHBCFG_GLBL_INTR_EN. One is the
fact that the driver disabled this flag at several places, not just on
shutdown. Another reason is that the register layout of GAHBCFG on
BCM2835 is customized ( see last page of datasheet [1]).

I dumped the relevant registers with a unmodified dwc2 driver and the
outcome is a little bit unexpected (0 = PRE_POWER_OFF, 3 = POWER_ON):

[  169.101071] dwc2 20980000.usb: dwc2_suspend enter GAHBCFG = 00000031
[  169.101143] dwc2 20980000.usb: dwc2_suspend enter GUSBCFG = 20001707
[  169.101172] dwc2 20980000.usb: dwc2_suspend enter GINTMSK = f3000806
[  169.105888] dwc2 20980000.usb: dwc2_power_notifier: 0 GAHBCFG = 00000031
[  169.105962] dwc2 20980000.usb: dwc2_power_notifier: 0 GUSBCFG = 20001707
[  169.105994] dwc2 20980000.usb: dwc2_power_notifier: 0 GINTMSK = f3000806
[  174.248046] dwc2 20980000.usb: dwc2_power_notifier: 3 GAHBCFG = 0000001f
[  174.248118] dwc2 20980000.usb: dwc2_power_notifier: 3 GUSBCFG = 20402700
[  174.248148] dwc2 20980000.usb: dwc2_power_notifier: 3 GINTMSK = f3000806
[  174.253086] dwc2 20980000.usb: dwc2_resume enter: GAHBCFG = 0000001f
[  174.253162] dwc2 20980000.usb: dwc2_resume enter: GUSBCFG = 20402700
[  174.253190] dwc2 20980000.usb: dwc2_resume enter: GINTMSK = f3000806
> I guess if that doesn't work then "GUSBCFG_TOUTCAL" could be used (I
> think that resets to 0 but must be initted to non-0 by the driver).
Yes this looks good and match with the trace above. The driver seems to
initialize this once and a quick test seems to work so far. I will stick
to this.
> Yet another register that could probably work as a canary would be
> "GINTMSK". I believe that inits to all 0 (everything is masked) and
> obviously to use the device we've got to unmask _some_ interrupts.
I don't know why but this didn't worked according to trace, but i also
didn't noticed a interrupt after enabling of the power domain.Thanks [1]
-
https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
> I can look for more, if need be.
>
> -Doug
>