Re: [PATCH v2 3/3] usb: renesas_usbhs: Reorder clock handling and power management in probe

[Lad, Prabhakar]

Hi Geert,

On Tue, Apr 8, 2025 at 4:43 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Shimoda-san,
>
> On Tue, 8 Apr 2025 at 12:40, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> > > From: Prabhakar, Sent: Monday, April 7, 2025 7:50 PM
> > >
> > > Reorder the initialization sequence in `usbhs_probe()` to enable runtime
> > > PM before accessing registers, preventing potential crashes due to
> > > uninitialized clocks.
> >
> > Just for a record. I don't know why, but the issue didn't occur on the original code
> > with my environment (R-Car H3). But, anyway, I understood that we need this patch for RZ/V2H.
>
> On R-Car Gen3 and later, the firmware must trap the external abort,
> as usually no crash happens, but register reads return zero when
> the module clock is turned off.  I am wondering why RZ/V2H behaves
> differently than R-Car Gen3?
>
>From section 4.4.8.4.1 of RZ/V2H HW manual:
In this mode, the frequency of the clock signals supplied to specified
peripheral units is stopped to obtain lower power
consumption.
This is achieved by stopping supply of the clock signals by the CPG
and, through the given CPG registers, switching
the bus MSTOP ports. Error interrupts notify the system of attempted
access to units in the MSTOP state.

> On R-Car Gen2, you do get an external abort when accessing hardware
> registers while the module's clock is turned off.  Has anyone tested
> usbhs on R-Car Gen2 recently?
>
Yes I tested this with the patch applied,
https://gist.github.com/prabhakarlad/3d1bbb6f745d8d867c8e6e009ab93f8d

1] For R-Car Gen3 looking at the r8a77951.dtsi we have the below:

hsusb: usb@e6590000 {
        compatible = "renesas,usbhs-r8a7795",
        ...
        clocks = <&cpg CPG_MOD 703>, <&cpg CPG_MOD 704>;
        ....
};

The same clocks are used for ehci0/ohci0/phy0 in  r8a77951.dtsi,
probably by the time we reach probing the usbhs driver these clocks
may have been already enabled hence register reads were sensible.

2] For the R-Car Gen2, looking at the RZ/G1H USBHS node we have the below:

hsusb: usb@e6590000 {
    ....
    clocks = <&cpg CPG_MOD 704>;
    ....
};

Same clock is for USBPHY, even in this case the PHY driver is probed
first due to which the clock is ON and then we probe usbhs driver..

Cheers,
Prabhakar