RE: [PATCH v2] clk: renesas: rcar-usb2-clock-sel: Add R-Car USB 2.0 clock selector PHY

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Geert-san,

> -----Original Message-----
> From: Geert Uytterhoeven
> Sent: Monday, July 24, 2017 9:59 PM
> 
> Hi Shimoda-san,
> 
> On Mon, Jul 24, 2017 at 12:33 PM, Yoshihiro Shimoda
> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> >> From: Geert Uytterhoeven
> >> Sent: Wednesday, July 5, 2017 9:22 PM
> >> On Wed, Jul 5, 2017 at 1:57 PM, Yoshihiro Shimoda
> >> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> >> >> From: Geert Uytterhoeven
> >> >> Sent: Wednesday, July 5, 2017 7:09 PM
> >> >> On Wed, Jun 28, 2017 at 8:28 AM, Yoshihiro Shimoda
> >> >> <yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> >> >> > R-Car USB 2.0 controller can change the clock source from an oscillator
> >> >> > to an external clock via a register. So, this patch adds support
> >> >> > the clock source selector as a clock driver.
> >>
> >> >> > --- /dev/null
> >> >> > +++ b/drivers/clk/renesas/rcar-usb2-clock-sel.c
> >>
> >> >> > +/* Since this driver needs other ccf drivers, this uses subsys_initcall_sync */
> >> >> > +subsys_initcall_sync(rcar_usb2_clock_sel_init);
> >> >>
> >> >> I suppose this is a workaround for the lack of probe deferral support in the
> >> >> USB subsystem?
> >> >
> >> > This is my fault. I added "power-domains" property into this device's node.
> >> > After I remove the power-domains like the cpg node, this driver can use subsys_initcall()
> >> > instead of subsys_initcall_sync().
> >>
> >> Does the clock sel module requires the functional clock "ehci_ohci" to be
> >> running before you can access its registers?
> >> If yes, I think there should be a "power-domains" property.
> >
> > Yes. But...
> >
> >> Then, you can simplify the code by calling
> >>
> >>         pm_runtime_enable(dev);
> >>         pm_runtime_get_sync(dev);
> >>
> >> and remove the explicit handling of the functional clock.
> >>
> >> That does not solve probe deferral handling in the USB subsystem, though.
> >
> > I added a debug message at end of cpg_mssr_probe(), and if I used subsys_initcall() on
> > rcar-usb2-clock-sel driver, kernel log output below:
> > ========================
> > [    0.272547] rcar-usb2-clock-sel e6590630.clock-controller: probe deferral not supported
> > [    0.273944]  ----------------- cpg_mssr_probe: probed!
> > ========================
> > So, it seems the renesas-cpg-mssr.c doesn't solve probe deferral handling.
> > (The driver renesas-cpg-mssr.c uses platform_driver_probe() for now.)
> >
> > So, IIUC, this rcar-usb2-clock-sel driver cannot use subsys_initcall().
> > Or, should we modify the renesas-cpg-mssr.c somehow?
> 
> Drivers should avoid using subsys_initcall().
> Why do you use a subsys_initcall()? To avoid probe deferral in the USB
> susbystem?

Oh, this is my fault.
I had assumed a clock driver should use subsys_initcall()...
Like clk-fixed-rate.c, we can use builtin_platform_driver in general.

> What happens if the rcar-usb2-clock-sel uses builtin_platform_driver()?

I tried it today, and then the rcar-usb2-clock-sel works :)
So, I will submit v3 patch.

Best regards,
Yoshihiro Shimoda

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux