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

[Lad, Prabhakar]

Hi Morimoto san,

On Mon, Mar 31, 2025 at 1:05 AM Kuninori Morimoto
<kuninori.morimoto.gx@xxxxxxxxxxx> wrote:
>
>
> Hi Lad, Prabhakar
>
> > > >     usbhs_probe()
> > > >         usbhs_sys_clock_ctrl()
> > > >             usbhs_bset()
> > > >                 usbhs_write()
> > > >                     iowrite16()  <-- Register access before enabling clocks
> > >
> > > This patch itself is not so bad idea, but basically, we should not assume
> > > the power/clock was enabled since kernel boot.
> > > In this driver, register access happen only during power enable <->  power
> > > disable (except your issue case), and this is good idea. So, the strange
> > > is usbhs_sys_clock_ctrl() call in usbhs_probe() (without power enable) I
> > > guess.
> > >
> > > According to the comment, it is just caring boot loader, and
> > > usbhs_sys_clock_ctrl() itself will be called when usbhsc_power_ctrl() was
> > > called anyway. And more, if my understanding was correct, Renesas clock
> > > driver will stop all unused devices clock/power after boot.
> > > So maybe we can just remove strange usbhs_sys_clock_ctrl() from usbhs_probe() ?
> > >
> >
> > After dropping usbhs_sys_clock_ctrl and removing the clock enabling
> > done in this patch and with  `CONFIG_USB_G_SERIAL=y` I hit the same
> > issue.
>
> Ah...
> OK, not only usbhs_sys_clock_ctrl(), but other initializer also
> missing power in probe(). Thank you for reporting, your original patch
> is reasonable.
>
Thanks! I'll send a V2 incorporating your review comments next week
after v6.15-rc1 is released.

Cheers,
Prabhakar