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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CA+V-a8tErqUK=nHBOzA5HWH=xJX1JnTuzBAqwTA3fg-Snq02JQ@mail.gmail.com>
Date: Thu, 17 Apr 2025 17:56:27 +0100
From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To: Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>, 
	Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@...renesas.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Kuninori Morimoto <kuninori.morimoto.gx@...esas.com>, 
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, 
	"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>, Biju Das <biju.das.jz@...renesas.com>, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>, 
	Fabrizio Castro <fabrizio.castro.jz@...esas.com>
Subject: Re: [PATCH v2 3/3] usb: renesas_usbhs: Reorder clock handling and
 power management in probe

Hi Shimoda-san,

On Thu, Apr 17, 2025 at 5:32 PM Lad, Prabhakar
<prabhakar.csengg@...il.com> wrote:
>
> Hi Shimoda-san,
>
> Sorry for the delayed response.
>
> On Thu, Apr 10, 2025 at 3:48 AM Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@...esas.com> wrote:
> >
> > Hello Geert-san,
> >
> > > From: Yoshihiro Shimoda, Sent: Wednesday, April 9, 2025 10:10 AM
> > > To: Geert Uytterhoeven <geert@...ux-m68k.org>
> > > Cc: Prabhakar <prabhakar.csengg@...il.com>; Greg Kroah-Hartman <gregkh@...uxfoundation.org>; Kuninori Morimoto
> > > <kuninori.morimoto.gx@...esas.com>; linux-usb@...r.kernel.org; linux-kernel@...r.kernel.org;
> > > linux-renesas-soc@...r.kernel.org; Biju Das <biju.das.jz@...renesas.com>; Claudiu Beznea
> > > <claudiu.beznea.uj@...renesas.com>; Fabrizio Castro <fabrizio.castro.jz@...esas.com>; Prabhakar Mahadev Lad
> > > <prabhakar.mahadev-lad.rj@...renesas.com>
> > > Subject: RE: [PATCH v2 3/3] usb: renesas_usbhs: Reorder clock handling and power management in probe
> > >
> > > Hello Geert-san,
> > >
> > > > From: Geert Uytterhoeven, Sent: Wednesday, April 9, 2025 12:43 AM
> > > >
> > > > Hi Shimoda-san,
> > > >
> > > > On Tue, 8 Apr 2025 at 12:40, Yoshihiro Shimoda
> > > > <yoshihiro.shimoda.uh@...esas.com> 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?
> > >
> > > I'm guessing that:
> > > - EHCI/OHCI drivers on R-Car Gen3 enabled both the USB clocks (EHCI/OHCI and USBHS).
> > > - RZ/V2H didn't enable the USBHS clock only.
> > >
> > > So, I'm also guessing that the R-Car V2H issue can be reproduced if we disable EHCI/OHCI on R-Car Gen3.
> > > # However, for some reasons, I don't have time to test for it today. (I'll test it tomorrow or later.)
> >
> > I tested this topic, and I realized that my guess was incorrect.
> > In other words, the current code seems no problem except accessing register offset 0.
> As Geert mentioned, we don't get synchronous aborts on R-Car Gen3--we
> only get a 0 for register reads when the clock is not enabled, as seen
> in the test you ran. On the RZ/V2H, however, if we try to access an IP
> before enabling the clocks, error interrupts are triggered, which is
> why we're seeing synchronous aborts.
>
Another simpler way to test this is using devmem2:

Case #1] On RZ/G2M (which is similar to R-Car Gen3) by disabling all
the ehci/ohci/phy/hsusb and running the devem I see the below:

$devmem2 0xe6590000/dev/mem opened.
Memory mapped at address 0xffffa854c000
Read at address 0xE6590000 (0xffffa854c000): 0x00000000

Case #2] If I do the same on RZ/V2H and run devmem I get the below:
root@...2h-evk:~# devmem2 0x15820000
/dev/mem opened.[   34.056718] kauditd_printk_skb: 5 callbacks suppressed

Memory mapped [   34.056734] audit: type=1701
audit(1744907878.287:22): auid=4294967295 uid=0 gid=0 ses=4294967295
pid=313 comm="devmem2" exe="/usr/bin/devmem2" sig=7 res=1
at address 0xffff92854000.
Bus error (core dumped)

Cheers,
Prabhakar