All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring
@ 2018-07-18 14:34 Phil Edworthy
  2018-07-20 11:21 ` Geert Uytterhoeven
  0 siblings, 1 reply; 11+ messages in thread
From: Phil Edworthy @ 2018-07-18 14:34 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, linux-clk, linux-kernel, linux-renesas-soc, Phil Edworthy

To avoid all SoC peripheral drivers deferring their probes, both clock and
pinctrl drivers should already be probed. Since the pinctrl driver requires
a clock to access the registers, the clock driver should be probed before
the pinctrl driver.

Therefore, move the clock driver from subsys_initcall to core_initcall.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
---
 drivers/clk/renesas/r9a06g032-clocks.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
index a0b6ecd..b03d616 100644
--- a/drivers/clk/renesas/r9a06g032-clocks.c
+++ b/drivers/clk/renesas/r9a06g032-clocks.c
@@ -877,17 +877,18 @@ static const struct of_device_id r9a06g032_match[] = {
 	{ }
 };
 
-static struct platform_driver r9a06g032_clock_driver = {
+static struct platform_driver r9a06g032_clock_driver __refdata = {
 	.driver		= {
 		.name	= "renesas,r9a06g032-sysctrl",
 		.of_match_table = r9a06g032_match,
 	},
+	.probe = r9a06g032_clocks_probe,
 };
 
 static int __init r9a06g032_clocks_init(void)
 {
-	return platform_driver_probe(&r9a06g032_clock_driver,
-			r9a06g032_clocks_probe);
+	platform_driver_register(&r9a06g032_clock_driver);
+	return 0;
 }
 
-subsys_initcall(r9a06g032_clocks_init);
+core_initcall(r9a06g032_clocks_init);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring
  2018-07-18 14:34 [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring Phil Edworthy
@ 2018-07-20 11:21 ` Geert Uytterhoeven
  2018-07-20 12:06     ` Phil Edworthy
  0 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-07-20 11:21 UTC (permalink / raw)
  To: Phil Edworthy
  Cc: Simon Horman, linux-clk, Linux Kernel Mailing List, Linux-Renesas

Hi Phil,

On Wed, Jul 18, 2018 at 4:34 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> To avoid all SoC peripheral drivers deferring their probes, both clock and
> pinctrl drivers should already be probed. Since the pinctrl driver requires
> a clock to access the registers, the clock driver should be probed before
> the pinctrl driver.
>
> Therefore, move the clock driver from subsys_initcall to core_initcall.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

Thanks for your patch!

The (not yet upstreamed) pinctrl driver uses postcore_initcall(), right?

> --- a/drivers/clk/renesas/r9a06g032-clocks.c
> +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> @@ -877,17 +877,18 @@ static const struct of_device_id r9a06g032_match[] = {
>         { }
>  };
>
> -static struct platform_driver r9a06g032_clock_driver = {
> +static struct platform_driver r9a06g032_clock_driver __refdata = {
>         .driver         = {
>                 .name   = "renesas,r9a06g032-sysctrl",
>                 .of_match_table = r9a06g032_match,
>         },
> +       .probe = r9a06g032_clocks_probe,
>  };
>
>  static int __init r9a06g032_clocks_init(void)
>  {
> -       return platform_driver_probe(&r9a06g032_clock_driver,
> -                       r9a06g032_clocks_probe);
> +       platform_driver_register(&r9a06g032_clock_driver);
> +       return 0;
>  }

Why are all of the above changes needed?
Shouldn't the platform_driver_probe() keep on working?
If it does not, it means the clock driver has some other dependency, and
cannot be bound immediately.  This is potentially a dangerous situation,
as r9a06g032_clocks_probe() is __init, but can still be called at any time
later.  Hence using platform_driver_probe() is the safe thing to do,
possibly with a different reshuffling of the clock and pinctrl initcall
priorities.

> -subsys_initcall(r9a06g032_clocks_init);
> +core_initcall(r9a06g032_clocks_init);

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring
  2018-07-20 11:21 ` Geert Uytterhoeven
@ 2018-07-20 12:06     ` Phil Edworthy
  0 siblings, 0 replies; 11+ messages in thread
From: Phil Edworthy @ 2018-07-20 12:06 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, linux-clk, Linux Kernel Mailing List, Linux-Renesas

Hi Geert,

On 20 July 2018 12:21, Geert Uytterhoeven wrote:
> On Wed, Jul 18, 2018 at 4:34 PM Phil Edworthy wrote:
> > To avoid all SoC peripheral drivers deferring their probes, both clock
> > and pinctrl drivers should already be probed. Since the pinctrl driver
> > requires a clock to access the registers, the clock driver should be
> > probed before the pinctrl driver.
> >
> > Therefore, move the clock driver from subsys_initcall to core_initcall.
> >
> > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> 
> Thanks for your patch!
Thanks for your review!

> The (not yet upstreamed) pinctrl driver uses postcore_initcall(), right?
No, the pinctrl driver uses subsys_initcall, but postcore_initcall or 
arch_initcall may be better to make it clear about the dependencies.

> > --- a/drivers/clk/renesas/r9a06g032-clocks.c
> > +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> > @@ -877,17 +877,18 @@ static const struct of_device_id
> r9a06g032_match[] = {
> >         { }
> >  };
> >
> > -static struct platform_driver r9a06g032_clock_driver = {
> > +static struct platform_driver r9a06g032_clock_driver __refdata = {
> >         .driver         = {
> >                 .name   = "renesas,r9a06g032-sysctrl",
> >                 .of_match_table = r9a06g032_match,
> >         },
> > +       .probe = r9a06g032_clocks_probe,
> >  };
> >
> >  static int __init r9a06g032_clocks_init(void)  {
> > -       return platform_driver_probe(&r9a06g032_clock_driver,
> > -                       r9a06g032_clocks_probe);
> > +       platform_driver_register(&r9a06g032_clock_driver);
> > +       return 0;
That should be:
+       return platform_driver_register(&r9a06g032_clock_driver);

> >  }
> 
> Why are all of the above changes needed?
> Shouldn't the platform_driver_probe() keep on working?
> If it does not, it means the clock driver has some other dependency, and
> cannot be bound immediately.  This is potentially a dangerous situation, as
> r9a06g032_clocks_probe() is __init, but can still be called at any time later.
> Hence using platform_driver_probe() is the safe thing to do, possibly with a
> different reshuffling of the clock and pinctrl initcall priorities.
No, you cannot call platform_driver_probe() from core_initcall.
All drivers that are in core_initcall call platform_driver_register().

Thanks
Phil

> > -subsys_initcall(r9a06g032_clocks_init);
> > +core_initcall(r9a06g032_clocks_init);
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring
@ 2018-07-20 12:06     ` Phil Edworthy
  0 siblings, 0 replies; 11+ messages in thread
From: Phil Edworthy @ 2018-07-20 12:06 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, linux-clk, Linux Kernel Mailing List, Linux-Renesas

SGkgR2VlcnQsDQoNCk9uIDIwIEp1bHkgMjAxOCAxMjoyMSwgR2VlcnQgVXl0dGVyaG9ldmVuIHdy
b3RlOg0KPiBPbiBXZWQsIEp1bCAxOCwgMjAxOCBhdCA0OjM0IFBNIFBoaWwgRWR3b3J0aHkgd3Jv
dGU6DQo+ID4gVG8gYXZvaWQgYWxsIFNvQyBwZXJpcGhlcmFsIGRyaXZlcnMgZGVmZXJyaW5nIHRo
ZWlyIHByb2JlcywgYm90aCBjbG9jaw0KPiA+IGFuZCBwaW5jdHJsIGRyaXZlcnMgc2hvdWxkIGFs
cmVhZHkgYmUgcHJvYmVkLiBTaW5jZSB0aGUgcGluY3RybCBkcml2ZXINCj4gPiByZXF1aXJlcyBh
IGNsb2NrIHRvIGFjY2VzcyB0aGUgcmVnaXN0ZXJzLCB0aGUgY2xvY2sgZHJpdmVyIHNob3VsZCBi
ZQ0KPiA+IHByb2JlZCBiZWZvcmUgdGhlIHBpbmN0cmwgZHJpdmVyLg0KPiA+DQo+ID4gVGhlcmVm
b3JlLCBtb3ZlIHRoZSBjbG9jayBkcml2ZXIgZnJvbSBzdWJzeXNfaW5pdGNhbGwgdG8gY29yZV9p
bml0Y2FsbC4NCj4gPg0KPiA+IFNpZ25lZC1vZmYtYnk6IFBoaWwgRWR3b3J0aHkgPHBoaWwuZWR3
b3J0aHlAcmVuZXNhcy5jb20+DQo+IA0KPiBUaGFua3MgZm9yIHlvdXIgcGF0Y2ghDQpUaGFua3Mg
Zm9yIHlvdXIgcmV2aWV3IQ0KDQo+IFRoZSAobm90IHlldCB1cHN0cmVhbWVkKSBwaW5jdHJsIGRy
aXZlciB1c2VzIHBvc3Rjb3JlX2luaXRjYWxsKCksIHJpZ2h0Pw0KTm8sIHRoZSBwaW5jdHJsIGRy
aXZlciB1c2VzIHN1YnN5c19pbml0Y2FsbCwgYnV0IHBvc3Rjb3JlX2luaXRjYWxsIG9yIA0KYXJj
aF9pbml0Y2FsbCBtYXkgYmUgYmV0dGVyIHRvIG1ha2UgaXQgY2xlYXIgYWJvdXQgdGhlIGRlcGVu
ZGVuY2llcy4NCg0KPiA+IC0tLSBhL2RyaXZlcnMvY2xrL3JlbmVzYXMvcjlhMDZnMDMyLWNsb2Nr
cy5jDQo+ID4gKysrIGIvZHJpdmVycy9jbGsvcmVuZXNhcy9yOWEwNmcwMzItY2xvY2tzLmMNCj4g
PiBAQCAtODc3LDE3ICs4NzcsMTggQEAgc3RhdGljIGNvbnN0IHN0cnVjdCBvZl9kZXZpY2VfaWQN
Cj4gcjlhMDZnMDMyX21hdGNoW10gPSB7DQo+ID4gICAgICAgICB7IH0NCj4gPiAgfTsNCj4gPg0K
PiA+IC1zdGF0aWMgc3RydWN0IHBsYXRmb3JtX2RyaXZlciByOWEwNmcwMzJfY2xvY2tfZHJpdmVy
ID0gew0KPiA+ICtzdGF0aWMgc3RydWN0IHBsYXRmb3JtX2RyaXZlciByOWEwNmcwMzJfY2xvY2tf
ZHJpdmVyIF9fcmVmZGF0YSA9IHsNCj4gPiAgICAgICAgIC5kcml2ZXIgICAgICAgICA9IHsNCj4g
PiAgICAgICAgICAgICAgICAgLm5hbWUgICA9ICJyZW5lc2FzLHI5YTA2ZzAzMi1zeXNjdHJsIiwN
Cj4gPiAgICAgICAgICAgICAgICAgLm9mX21hdGNoX3RhYmxlID0gcjlhMDZnMDMyX21hdGNoLA0K
PiA+ICAgICAgICAgfSwNCj4gPiArICAgICAgIC5wcm9iZSA9IHI5YTA2ZzAzMl9jbG9ja3NfcHJv
YmUsDQo+ID4gIH07DQo+ID4NCj4gPiAgc3RhdGljIGludCBfX2luaXQgcjlhMDZnMDMyX2Nsb2Nr
c19pbml0KHZvaWQpICB7DQo+ID4gLSAgICAgICByZXR1cm4gcGxhdGZvcm1fZHJpdmVyX3Byb2Jl
KCZyOWEwNmcwMzJfY2xvY2tfZHJpdmVyLA0KPiA+IC0gICAgICAgICAgICAgICAgICAgICAgIHI5
YTA2ZzAzMl9jbG9ja3NfcHJvYmUpOw0KPiA+ICsgICAgICAgcGxhdGZvcm1fZHJpdmVyX3JlZ2lz
dGVyKCZyOWEwNmcwMzJfY2xvY2tfZHJpdmVyKTsNCj4gPiArICAgICAgIHJldHVybiAwOw0KVGhh
dCBzaG91bGQgYmU6DQorICAgICAgIHJldHVybiBwbGF0Zm9ybV9kcml2ZXJfcmVnaXN0ZXIoJnI5
YTA2ZzAzMl9jbG9ja19kcml2ZXIpOw0KDQo+ID4gIH0NCj4gDQo+IFdoeSBhcmUgYWxsIG9mIHRo
ZSBhYm92ZSBjaGFuZ2VzIG5lZWRlZD8NCj4gU2hvdWxkbid0IHRoZSBwbGF0Zm9ybV9kcml2ZXJf
cHJvYmUoKSBrZWVwIG9uIHdvcmtpbmc/DQo+IElmIGl0IGRvZXMgbm90LCBpdCBtZWFucyB0aGUg
Y2xvY2sgZHJpdmVyIGhhcyBzb21lIG90aGVyIGRlcGVuZGVuY3ksIGFuZA0KPiBjYW5ub3QgYmUg
Ym91bmQgaW1tZWRpYXRlbHkuICBUaGlzIGlzIHBvdGVudGlhbGx5IGEgZGFuZ2Vyb3VzIHNpdHVh
dGlvbiwgYXMNCj4gcjlhMDZnMDMyX2Nsb2Nrc19wcm9iZSgpIGlzIF9faW5pdCwgYnV0IGNhbiBz
dGlsbCBiZSBjYWxsZWQgYXQgYW55IHRpbWUgbGF0ZXIuDQo+IEhlbmNlIHVzaW5nIHBsYXRmb3Jt
X2RyaXZlcl9wcm9iZSgpIGlzIHRoZSBzYWZlIHRoaW5nIHRvIGRvLCBwb3NzaWJseSB3aXRoIGEN
Cj4gZGlmZmVyZW50IHJlc2h1ZmZsaW5nIG9mIHRoZSBjbG9jayBhbmQgcGluY3RybCBpbml0Y2Fs
bCBwcmlvcml0aWVzLg0KTm8sIHlvdSBjYW5ub3QgY2FsbCBwbGF0Zm9ybV9kcml2ZXJfcHJvYmUo
KSBmcm9tIGNvcmVfaW5pdGNhbGwuDQpBbGwgZHJpdmVycyB0aGF0IGFyZSBpbiBjb3JlX2luaXRj
YWxsIGNhbGwgcGxhdGZvcm1fZHJpdmVyX3JlZ2lzdGVyKCkuDQoNClRoYW5rcw0KUGhpbA0KDQo+
ID4gLXN1YnN5c19pbml0Y2FsbChyOWEwNmcwMzJfY2xvY2tzX2luaXQpOw0KPiA+ICtjb3JlX2lu
aXRjYWxsKHI5YTA2ZzAzMl9jbG9ja3NfaW5pdCk7DQo+IA0KPiBHcntvZXRqZSxlZXRpbmd9cywN
Cj4gDQo+ICAgICAgICAgICAgICAgICAgICAgICAgIEdlZXJ0DQo+IA0KPiAtLQ0KPiBHZWVydCBV
eXR0ZXJob2V2ZW4gLS0gVGhlcmUncyBsb3RzIG9mIExpbnV4IGJleW9uZCBpYTMyIC0tIGdlZXJ0
QGxpbnV4LQ0KPiBtNjhrLm9yZw0KPiANCj4gSW4gcGVyc29uYWwgY29udmVyc2F0aW9ucyB3aXRo
IHRlY2huaWNhbCBwZW9wbGUsIEkgY2FsbCBteXNlbGYgYSBoYWNrZXIuIEJ1dA0KPiB3aGVuIEkn
bSB0YWxraW5nIHRvIGpvdXJuYWxpc3RzIEkganVzdCBzYXkgInByb2dyYW1tZXIiIG9yIHNvbWV0
aGluZyBsaWtlIHRoYXQuDQo+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgLS0gTGlu
dXMgVG9ydmFsZHMNCg==

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring
  2018-07-20 12:06     ` Phil Edworthy
  (?)
@ 2018-07-20 12:12     ` Geert Uytterhoeven
  2018-07-20 12:26         ` Phil Edworthy
  -1 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-07-20 12:12 UTC (permalink / raw)
  To: Phil Edworthy
  Cc: Simon Horman, linux-clk, Linux Kernel Mailing List, Linux-Renesas

Hi Phil,

On Fri, Jul 20, 2018 at 2:06 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> On 20 July 2018 12:21, Geert Uytterhoeven wrote:
> > On Wed, Jul 18, 2018 at 4:34 PM Phil Edworthy wrote:
> > > To avoid all SoC peripheral drivers deferring their probes, both clock
> > > and pinctrl drivers should already be probed. Since the pinctrl driver
> > > requires a clock to access the registers, the clock driver should be
> > > probed before the pinctrl driver.
> > >
> > > Therefore, move the clock driver from subsys_initcall to core_initcall.
> > >
> > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> >
> > Thanks for your patch!
> Thanks for your review!
>
> > The (not yet upstreamed) pinctrl driver uses postcore_initcall(), right?
> No, the pinctrl driver uses subsys_initcall, but postcore_initcall or
> arch_initcall may be better to make it clear about the dependencies.

if the pinctrl driver uses subsys_initcall(), ...

> > > --- a/drivers/clk/renesas/r9a06g032-clocks.c
> > > +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> > > @@ -877,17 +877,18 @@ static const struct of_device_id
> > r9a06g032_match[] = {
> > >         { }
> > >  };
> > >
> > > -static struct platform_driver r9a06g032_clock_driver = {
> > > +static struct platform_driver r9a06g032_clock_driver __refdata = {
> > >         .driver         = {
> > >                 .name   = "renesas,r9a06g032-sysctrl",
> > >                 .of_match_table = r9a06g032_match,
> > >         },
> > > +       .probe = r9a06g032_clocks_probe,
> > >  };
> > >
> > >  static int __init r9a06g032_clocks_init(void)  {
> > > -       return platform_driver_probe(&r9a06g032_clock_driver,
> > > -                       r9a06g032_clocks_probe);
> > > +       platform_driver_register(&r9a06g032_clock_driver);
> > > +       return 0;
> That should be:
> +       return platform_driver_register(&r9a06g032_clock_driver);
>
> > >  }
> >
> > Why are all of the above changes needed?
> > Shouldn't the platform_driver_probe() keep on working?
> > If it does not, it means the clock driver has some other dependency, and
> > cannot be bound immediately.  This is potentially a dangerous situation, as
> > r9a06g032_clocks_probe() is __init, but can still be called at any time later.
> > Hence using platform_driver_probe() is the safe thing to do, possibly with a
> > different reshuffling of the clock and pinctrl initcall priorities.
> No, you cannot call platform_driver_probe() from core_initcall.
> All drivers that are in core_initcall call platform_driver_register().

Hence they cannot have their probe function __init.

>
> Thanks
> Phil
>
> > > -subsys_initcall(r9a06g032_clocks_init);
> > > +core_initcall(r9a06g032_clocks_init);

... using postcore_initcall() or arch_initcall() here, should work with
platform_driver_probe()?

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring
  2018-07-20 12:12     ` Geert Uytterhoeven
@ 2018-07-20 12:26         ` Phil Edworthy
  0 siblings, 0 replies; 11+ messages in thread
From: Phil Edworthy @ 2018-07-20 12:26 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, linux-clk, Linux Kernel Mailing List, Linux-Renesas

Hi Geert,

On 20 July 2018 13:12, Geert Uytterhoeven wrote:
> On Fri, Jul 20, 2018 at 2:06 PM Phil Edworthy  wrote:
> > On 20 July 2018 12:21, Geert Uytterhoeven wrote:
> > > On Wed, Jul 18, 2018 at 4:34 PM Phil Edworthy wrote:
> > > > To avoid all SoC peripheral drivers deferring their probes, both
> > > > clock and pinctrl drivers should already be probed. Since the
> > > > pinctrl driver requires a clock to access the registers, the clock
> > > > driver should be probed before the pinctrl driver.
> > > >
> > > > Therefore, move the clock driver from subsys_initcall to core_initcall.
> > > >
> > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > >
> > > Thanks for your patch!
> > Thanks for your review!
> >
> > > The (not yet upstreamed) pinctrl driver uses postcore_initcall(), right?
> > No, the pinctrl driver uses subsys_initcall, but postcore_initcall or
> > arch_initcall may be better to make it clear about the dependencies.
> 
> if the pinctrl driver uses subsys_initcall(), ...
> 
> > > > --- a/drivers/clk/renesas/r9a06g032-clocks.c
> > > > +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> > > > @@ -877,17 +877,18 @@ static const struct of_device_id
> > > r9a06g032_match[] = {
> > > >         { }
> > > >  };
> > > >
> > > > -static struct platform_driver r9a06g032_clock_driver = {
> > > > +static struct platform_driver r9a06g032_clock_driver __refdata =
> > > > +{
> > > >         .driver         = {
> > > >                 .name   = "renesas,r9a06g032-sysctrl",
> > > >                 .of_match_table = r9a06g032_match,
> > > >         },
> > > > +       .probe = r9a06g032_clocks_probe,
> > > >  };
> > > >
> > > >  static int __init r9a06g032_clocks_init(void)  {
> > > > -       return platform_driver_probe(&r9a06g032_clock_driver,
> > > > -                       r9a06g032_clocks_probe);
> > > > +       platform_driver_register(&r9a06g032_clock_driver);
> > > > +       return 0;
> > That should be:
> > +       return platform_driver_register(&r9a06g032_clock_driver);
> >
> > > >  }
> > >
> > > Why are all of the above changes needed?
> > > Shouldn't the platform_driver_probe() keep on working?
> > > If it does not, it means the clock driver has some other dependency,
> > > and cannot be bound immediately.  This is potentially a dangerous
> > > situation, as
> > > r9a06g032_clocks_probe() is __init, but can still be called at any time later.
> > > Hence using platform_driver_probe() is the safe thing to do,
> > > possibly with a different reshuffling of the clock and pinctrl initcall
> priorities.
> > No, you cannot call platform_driver_probe() from core_initcall.
> > All drivers that are in core_initcall call platform_driver_register().
> 
> Hence they cannot have their probe function __init.
> 
> >
> > Thanks
> > Phil
> >
> > > > -subsys_initcall(r9a06g032_clocks_init);
> > > > +core_initcall(r9a06g032_clocks_init);
> 
> ... using postcore_initcall() or arch_initcall() here, should work with
> platform_driver_probe()?
Nope, you have to use platform_driver_register() for DT based drivers.
subsys_initcall is the earliest you can use platform_driver_probe().

Thanks
Phil

> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring
@ 2018-07-20 12:26         ` Phil Edworthy
  0 siblings, 0 replies; 11+ messages in thread
From: Phil Edworthy @ 2018-07-20 12:26 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, linux-clk, Linux Kernel Mailing List, Linux-Renesas

SGkgR2VlcnQsDQoNCk9uIDIwIEp1bHkgMjAxOCAxMzoxMiwgR2VlcnQgVXl0dGVyaG9ldmVuIHdy
b3RlOg0KPiBPbiBGcmksIEp1bCAyMCwgMjAxOCBhdCAyOjA2IFBNIFBoaWwgRWR3b3J0aHkgIHdy
b3RlOg0KPiA+IE9uIDIwIEp1bHkgMjAxOCAxMjoyMSwgR2VlcnQgVXl0dGVyaG9ldmVuIHdyb3Rl
Og0KPiA+ID4gT24gV2VkLCBKdWwgMTgsIDIwMTggYXQgNDozNCBQTSBQaGlsIEVkd29ydGh5IHdy
b3RlOg0KPiA+ID4gPiBUbyBhdm9pZCBhbGwgU29DIHBlcmlwaGVyYWwgZHJpdmVycyBkZWZlcnJp
bmcgdGhlaXIgcHJvYmVzLCBib3RoDQo+ID4gPiA+IGNsb2NrIGFuZCBwaW5jdHJsIGRyaXZlcnMg
c2hvdWxkIGFscmVhZHkgYmUgcHJvYmVkLiBTaW5jZSB0aGUNCj4gPiA+ID4gcGluY3RybCBkcml2
ZXIgcmVxdWlyZXMgYSBjbG9jayB0byBhY2Nlc3MgdGhlIHJlZ2lzdGVycywgdGhlIGNsb2NrDQo+
ID4gPiA+IGRyaXZlciBzaG91bGQgYmUgcHJvYmVkIGJlZm9yZSB0aGUgcGluY3RybCBkcml2ZXIu
DQo+ID4gPiA+DQo+ID4gPiA+IFRoZXJlZm9yZSwgbW92ZSB0aGUgY2xvY2sgZHJpdmVyIGZyb20g
c3Vic3lzX2luaXRjYWxsIHRvIGNvcmVfaW5pdGNhbGwuDQo+ID4gPiA+DQo+ID4gPiA+IFNpZ25l
ZC1vZmYtYnk6IFBoaWwgRWR3b3J0aHkgPHBoaWwuZWR3b3J0aHlAcmVuZXNhcy5jb20+DQo+ID4g
Pg0KPiA+ID4gVGhhbmtzIGZvciB5b3VyIHBhdGNoIQ0KPiA+IFRoYW5rcyBmb3IgeW91ciByZXZp
ZXchDQo+ID4NCj4gPiA+IFRoZSAobm90IHlldCB1cHN0cmVhbWVkKSBwaW5jdHJsIGRyaXZlciB1
c2VzIHBvc3Rjb3JlX2luaXRjYWxsKCksIHJpZ2h0Pw0KPiA+IE5vLCB0aGUgcGluY3RybCBkcml2
ZXIgdXNlcyBzdWJzeXNfaW5pdGNhbGwsIGJ1dCBwb3N0Y29yZV9pbml0Y2FsbCBvcg0KPiA+IGFy
Y2hfaW5pdGNhbGwgbWF5IGJlIGJldHRlciB0byBtYWtlIGl0IGNsZWFyIGFib3V0IHRoZSBkZXBl
bmRlbmNpZXMuDQo+IA0KPiBpZiB0aGUgcGluY3RybCBkcml2ZXIgdXNlcyBzdWJzeXNfaW5pdGNh
bGwoKSwgLi4uDQo+IA0KPiA+ID4gPiAtLS0gYS9kcml2ZXJzL2Nsay9yZW5lc2FzL3I5YTA2ZzAz
Mi1jbG9ja3MuYw0KPiA+ID4gPiArKysgYi9kcml2ZXJzL2Nsay9yZW5lc2FzL3I5YTA2ZzAzMi1j
bG9ja3MuYw0KPiA+ID4gPiBAQCAtODc3LDE3ICs4NzcsMTggQEAgc3RhdGljIGNvbnN0IHN0cnVj
dCBvZl9kZXZpY2VfaWQNCj4gPiA+IHI5YTA2ZzAzMl9tYXRjaFtdID0gew0KPiA+ID4gPiAgICAg
ICAgIHsgfQ0KPiA+ID4gPiAgfTsNCj4gPiA+ID4NCj4gPiA+ID4gLXN0YXRpYyBzdHJ1Y3QgcGxh
dGZvcm1fZHJpdmVyIHI5YTA2ZzAzMl9jbG9ja19kcml2ZXIgPSB7DQo+ID4gPiA+ICtzdGF0aWMg
c3RydWN0IHBsYXRmb3JtX2RyaXZlciByOWEwNmcwMzJfY2xvY2tfZHJpdmVyIF9fcmVmZGF0YSA9
DQo+ID4gPiA+ICt7DQo+ID4gPiA+ICAgICAgICAgLmRyaXZlciAgICAgICAgID0gew0KPiA+ID4g
PiAgICAgICAgICAgICAgICAgLm5hbWUgICA9ICJyZW5lc2FzLHI5YTA2ZzAzMi1zeXNjdHJsIiwN
Cj4gPiA+ID4gICAgICAgICAgICAgICAgIC5vZl9tYXRjaF90YWJsZSA9IHI5YTA2ZzAzMl9tYXRj
aCwNCj4gPiA+ID4gICAgICAgICB9LA0KPiA+ID4gPiArICAgICAgIC5wcm9iZSA9IHI5YTA2ZzAz
Ml9jbG9ja3NfcHJvYmUsDQo+ID4gPiA+ICB9Ow0KPiA+ID4gPg0KPiA+ID4gPiAgc3RhdGljIGlu
dCBfX2luaXQgcjlhMDZnMDMyX2Nsb2Nrc19pbml0KHZvaWQpICB7DQo+ID4gPiA+IC0gICAgICAg
cmV0dXJuIHBsYXRmb3JtX2RyaXZlcl9wcm9iZSgmcjlhMDZnMDMyX2Nsb2NrX2RyaXZlciwNCj4g
PiA+ID4gLSAgICAgICAgICAgICAgICAgICAgICAgcjlhMDZnMDMyX2Nsb2Nrc19wcm9iZSk7DQo+
ID4gPiA+ICsgICAgICAgcGxhdGZvcm1fZHJpdmVyX3JlZ2lzdGVyKCZyOWEwNmcwMzJfY2xvY2tf
ZHJpdmVyKTsNCj4gPiA+ID4gKyAgICAgICByZXR1cm4gMDsNCj4gPiBUaGF0IHNob3VsZCBiZToN
Cj4gPiArICAgICAgIHJldHVybiBwbGF0Zm9ybV9kcml2ZXJfcmVnaXN0ZXIoJnI5YTA2ZzAzMl9j
bG9ja19kcml2ZXIpOw0KPiA+DQo+ID4gPiA+ICB9DQo+ID4gPg0KPiA+ID4gV2h5IGFyZSBhbGwg
b2YgdGhlIGFib3ZlIGNoYW5nZXMgbmVlZGVkPw0KPiA+ID4gU2hvdWxkbid0IHRoZSBwbGF0Zm9y
bV9kcml2ZXJfcHJvYmUoKSBrZWVwIG9uIHdvcmtpbmc/DQo+ID4gPiBJZiBpdCBkb2VzIG5vdCwg
aXQgbWVhbnMgdGhlIGNsb2NrIGRyaXZlciBoYXMgc29tZSBvdGhlciBkZXBlbmRlbmN5LA0KPiA+
ID4gYW5kIGNhbm5vdCBiZSBib3VuZCBpbW1lZGlhdGVseS4gIFRoaXMgaXMgcG90ZW50aWFsbHkg
YSBkYW5nZXJvdXMNCj4gPiA+IHNpdHVhdGlvbiwgYXMNCj4gPiA+IHI5YTA2ZzAzMl9jbG9ja3Nf
cHJvYmUoKSBpcyBfX2luaXQsIGJ1dCBjYW4gc3RpbGwgYmUgY2FsbGVkIGF0IGFueSB0aW1lIGxh
dGVyLg0KPiA+ID4gSGVuY2UgdXNpbmcgcGxhdGZvcm1fZHJpdmVyX3Byb2JlKCkgaXMgdGhlIHNh
ZmUgdGhpbmcgdG8gZG8sDQo+ID4gPiBwb3NzaWJseSB3aXRoIGEgZGlmZmVyZW50IHJlc2h1ZmZs
aW5nIG9mIHRoZSBjbG9jayBhbmQgcGluY3RybCBpbml0Y2FsbA0KPiBwcmlvcml0aWVzLg0KPiA+
IE5vLCB5b3UgY2Fubm90IGNhbGwgcGxhdGZvcm1fZHJpdmVyX3Byb2JlKCkgZnJvbSBjb3JlX2lu
aXRjYWxsLg0KPiA+IEFsbCBkcml2ZXJzIHRoYXQgYXJlIGluIGNvcmVfaW5pdGNhbGwgY2FsbCBw
bGF0Zm9ybV9kcml2ZXJfcmVnaXN0ZXIoKS4NCj4gDQo+IEhlbmNlIHRoZXkgY2Fubm90IGhhdmUg
dGhlaXIgcHJvYmUgZnVuY3Rpb24gX19pbml0Lg0KPiANCj4gPg0KPiA+IFRoYW5rcw0KPiA+IFBo
aWwNCj4gPg0KPiA+ID4gPiAtc3Vic3lzX2luaXRjYWxsKHI5YTA2ZzAzMl9jbG9ja3NfaW5pdCk7
DQo+ID4gPiA+ICtjb3JlX2luaXRjYWxsKHI5YTA2ZzAzMl9jbG9ja3NfaW5pdCk7DQo+IA0KPiAu
Li4gdXNpbmcgcG9zdGNvcmVfaW5pdGNhbGwoKSBvciBhcmNoX2luaXRjYWxsKCkgaGVyZSwgc2hv
dWxkIHdvcmsgd2l0aA0KPiBwbGF0Zm9ybV9kcml2ZXJfcHJvYmUoKT8NCk5vcGUsIHlvdSBoYXZl
IHRvIHVzZSBwbGF0Zm9ybV9kcml2ZXJfcmVnaXN0ZXIoKSBmb3IgRFQgYmFzZWQgZHJpdmVycy4N
CnN1YnN5c19pbml0Y2FsbCBpcyB0aGUgZWFybGllc3QgeW91IGNhbiB1c2UgcGxhdGZvcm1fZHJp
dmVyX3Byb2JlKCkuDQoNClRoYW5rcw0KUGhpbA0KDQo+IEdye29ldGplLGVldGluZ31zLA0KPiAN
Cj4gICAgICAgICAgICAgICAgICAgICAgICAgR2VlcnQNCj4gDQo+IC0tDQo+IEdlZXJ0IFV5dHRl
cmhvZXZlbiAtLSBUaGVyZSdzIGxvdHMgb2YgTGludXggYmV5b25kIGlhMzIgLS0gZ2VlcnRAbGlu
dXgtDQo+IG02OGsub3JnDQo+IA0KPiBJbiBwZXJzb25hbCBjb252ZXJzYXRpb25zIHdpdGggdGVj
aG5pY2FsIHBlb3BsZSwgSSBjYWxsIG15c2VsZiBhIGhhY2tlci4gQnV0DQo+IHdoZW4gSSdtIHRh
bGtpbmcgdG8gam91cm5hbGlzdHMgSSBqdXN0IHNheSAicHJvZ3JhbW1lciIgb3Igc29tZXRoaW5n
IGxpa2UgdGhhdC4NCj4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAtLSBMaW51cyBU
b3J2YWxkcw0K

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring
  2018-07-20 12:26         ` Phil Edworthy
  (?)
@ 2018-07-20 12:41         ` Geert Uytterhoeven
  2018-07-20 13:57             ` Phil Edworthy
  -1 siblings, 1 reply; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-07-20 12:41 UTC (permalink / raw)
  To: Phil Edworthy
  Cc: Simon Horman, linux-clk, Linux Kernel Mailing List, Linux-Renesas

Hi Phil,

On Fri, Jul 20, 2018 at 2:26 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> On 20 July 2018 13:12, Geert Uytterhoeven wrote:
> > On Fri, Jul 20, 2018 at 2:06 PM Phil Edworthy  wrote:
> > > On 20 July 2018 12:21, Geert Uytterhoeven wrote:
> > > > On Wed, Jul 18, 2018 at 4:34 PM Phil Edworthy wrote:
> > > > > To avoid all SoC peripheral drivers deferring their probes, both
> > > > > clock and pinctrl drivers should already be probed. Since the
> > > > > pinctrl driver requires a clock to access the registers, the clock
> > > > > driver should be probed before the pinctrl driver.
> > > > >
> > > > > Therefore, move the clock driver from subsys_initcall to core_initcall.
> > > > >
> > > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > >
> > > > Thanks for your patch!
> > > Thanks for your review!
> > >
> > > > The (not yet upstreamed) pinctrl driver uses postcore_initcall(), right?
> > > No, the pinctrl driver uses subsys_initcall, but postcore_initcall or
> > > arch_initcall may be better to make it clear about the dependencies.
> >
> > if the pinctrl driver uses subsys_initcall(), ...

> > > > > -subsys_initcall(r9a06g032_clocks_init);
> > > > > +core_initcall(r9a06g032_clocks_init);
> >
> > ... using postcore_initcall() or arch_initcall() here, should work with
> > platform_driver_probe()?
> Nope, you have to use platform_driver_register() for DT based drivers.
> subsys_initcall is the earliest you can use platform_driver_probe().

So drivers/misc/atmel_tclib.c and drivers/pinctrl/pinctrl-coh901.c, which
use arch_initcall(), cannot work?

If that is really true, you can still use subsys_initcall() in the clock driver,
and subsys_initcall_sync() in the pinctrl driver.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring
  2018-07-20 12:41         ` Geert Uytterhoeven
@ 2018-07-20 13:57             ` Phil Edworthy
  0 siblings, 0 replies; 11+ messages in thread
From: Phil Edworthy @ 2018-07-20 13:57 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, linux-clk, Linux Kernel Mailing List, Linux-Renesas

Hi Geert,

On 20 July 2018 13:41, Geert Uytterhoeven wrote:
> On Fri, Jul 20, 2018 at 2:26 PM Phil Edworthy wrote:
> > On 20 July 2018 13:12, Geert Uytterhoeven wrote:
> > > On Fri, Jul 20, 2018 at 2:06 PM Phil Edworthy  wrote:
> > > > On 20 July 2018 12:21, Geert Uytterhoeven wrote:
> > > > > On Wed, Jul 18, 2018 at 4:34 PM Phil Edworthy wrote:
> > > > > > To avoid all SoC peripheral drivers deferring their probes,
> > > > > > both clock and pinctrl drivers should already be probed. Since
> > > > > > the pinctrl driver requires a clock to access the registers,
> > > > > > the clock driver should be probed before the pinctrl driver.
> > > > > >
> > > > > > Therefore, move the clock driver from subsys_initcall to
> core_initcall.
> > > > > >
> > > > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > > >
> > > > > Thanks for your patch!
> > > > Thanks for your review!
> > > >
> > > > > The (not yet upstreamed) pinctrl driver uses postcore_initcall(), right?
> > > > No, the pinctrl driver uses subsys_initcall, but postcore_initcall
> > > > or arch_initcall may be better to make it clear about the dependencies.
> > >
> > > if the pinctrl driver uses subsys_initcall(), ...
> 
> > > > > > -subsys_initcall(r9a06g032_clocks_init);
> > > > > > +core_initcall(r9a06g032_clocks_init);
> > >
> > > ... using postcore_initcall() or arch_initcall() here, should work
> > > with platform_driver_probe()?
> > Nope, you have to use platform_driver_register() for DT based drivers.
> > subsys_initcall is the earliest you can use platform_driver_probe().
> 
> So drivers/misc/atmel_tclib.c and drivers/pinctrl/pinctrl-coh901.c, which use
> arch_initcall(), cannot work?
How those drivers work I do not know. However, I tried with the rzn1 clock
driver and it does not work.

> If that is really true, you can still use subsys_initcall() in the clock driver, and
> subsys_initcall_sync() in the pinctrl driver.
True, I'm not sure how people decide which initcall to use and whether the
_sync variants of the initcalls have a special meaning or intention. As far as
I know they were introduced for threaded probes, so are we supposed to
use them for driver dependencies like this?

Do you know why the rza1 pinctrl driver is running from core_initcall()?

Thanks
Phil

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring
@ 2018-07-20 13:57             ` Phil Edworthy
  0 siblings, 0 replies; 11+ messages in thread
From: Phil Edworthy @ 2018-07-20 13:57 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Simon Horman, linux-clk, Linux Kernel Mailing List, Linux-Renesas

SGkgR2VlcnQsDQoNCk9uIDIwIEp1bHkgMjAxOCAxMzo0MSwgR2VlcnQgVXl0dGVyaG9ldmVuIHdy
b3RlOg0KPiBPbiBGcmksIEp1bCAyMCwgMjAxOCBhdCAyOjI2IFBNIFBoaWwgRWR3b3J0aHkgd3Jv
dGU6DQo+ID4gT24gMjAgSnVseSAyMDE4IDEzOjEyLCBHZWVydCBVeXR0ZXJob2V2ZW4gd3JvdGU6
DQo+ID4gPiBPbiBGcmksIEp1bCAyMCwgMjAxOCBhdCAyOjA2IFBNIFBoaWwgRWR3b3J0aHkgIHdy
b3RlOg0KPiA+ID4gPiBPbiAyMCBKdWx5IDIwMTggMTI6MjEsIEdlZXJ0IFV5dHRlcmhvZXZlbiB3
cm90ZToNCj4gPiA+ID4gPiBPbiBXZWQsIEp1bCAxOCwgMjAxOCBhdCA0OjM0IFBNIFBoaWwgRWR3
b3J0aHkgd3JvdGU6DQo+ID4gPiA+ID4gPiBUbyBhdm9pZCBhbGwgU29DIHBlcmlwaGVyYWwgZHJp
dmVycyBkZWZlcnJpbmcgdGhlaXIgcHJvYmVzLA0KPiA+ID4gPiA+ID4gYm90aCBjbG9jayBhbmQg
cGluY3RybCBkcml2ZXJzIHNob3VsZCBhbHJlYWR5IGJlIHByb2JlZC4gU2luY2UNCj4gPiA+ID4g
PiA+IHRoZSBwaW5jdHJsIGRyaXZlciByZXF1aXJlcyBhIGNsb2NrIHRvIGFjY2VzcyB0aGUgcmVn
aXN0ZXJzLA0KPiA+ID4gPiA+ID4gdGhlIGNsb2NrIGRyaXZlciBzaG91bGQgYmUgcHJvYmVkIGJl
Zm9yZSB0aGUgcGluY3RybCBkcml2ZXIuDQo+ID4gPiA+ID4gPg0KPiA+ID4gPiA+ID4gVGhlcmVm
b3JlLCBtb3ZlIHRoZSBjbG9jayBkcml2ZXIgZnJvbSBzdWJzeXNfaW5pdGNhbGwgdG8NCj4gY29y
ZV9pbml0Y2FsbC4NCj4gPiA+ID4gPiA+DQo+ID4gPiA+ID4gPiBTaWduZWQtb2ZmLWJ5OiBQaGls
IEVkd29ydGh5IDxwaGlsLmVkd29ydGh5QHJlbmVzYXMuY29tPg0KPiA+ID4gPiA+DQo+ID4gPiA+
ID4gVGhhbmtzIGZvciB5b3VyIHBhdGNoIQ0KPiA+ID4gPiBUaGFua3MgZm9yIHlvdXIgcmV2aWV3
IQ0KPiA+ID4gPg0KPiA+ID4gPiA+IFRoZSAobm90IHlldCB1cHN0cmVhbWVkKSBwaW5jdHJsIGRy
aXZlciB1c2VzIHBvc3Rjb3JlX2luaXRjYWxsKCksIHJpZ2h0Pw0KPiA+ID4gPiBObywgdGhlIHBp
bmN0cmwgZHJpdmVyIHVzZXMgc3Vic3lzX2luaXRjYWxsLCBidXQgcG9zdGNvcmVfaW5pdGNhbGwN
Cj4gPiA+ID4gb3IgYXJjaF9pbml0Y2FsbCBtYXkgYmUgYmV0dGVyIHRvIG1ha2UgaXQgY2xlYXIg
YWJvdXQgdGhlIGRlcGVuZGVuY2llcy4NCj4gPiA+DQo+ID4gPiBpZiB0aGUgcGluY3RybCBkcml2
ZXIgdXNlcyBzdWJzeXNfaW5pdGNhbGwoKSwgLi4uDQo+IA0KPiA+ID4gPiA+ID4gLXN1YnN5c19p
bml0Y2FsbChyOWEwNmcwMzJfY2xvY2tzX2luaXQpOw0KPiA+ID4gPiA+ID4gK2NvcmVfaW5pdGNh
bGwocjlhMDZnMDMyX2Nsb2Nrc19pbml0KTsNCj4gPiA+DQo+ID4gPiAuLi4gdXNpbmcgcG9zdGNv
cmVfaW5pdGNhbGwoKSBvciBhcmNoX2luaXRjYWxsKCkgaGVyZSwgc2hvdWxkIHdvcmsNCj4gPiA+
IHdpdGggcGxhdGZvcm1fZHJpdmVyX3Byb2JlKCk/DQo+ID4gTm9wZSwgeW91IGhhdmUgdG8gdXNl
IHBsYXRmb3JtX2RyaXZlcl9yZWdpc3RlcigpIGZvciBEVCBiYXNlZCBkcml2ZXJzLg0KPiA+IHN1
YnN5c19pbml0Y2FsbCBpcyB0aGUgZWFybGllc3QgeW91IGNhbiB1c2UgcGxhdGZvcm1fZHJpdmVy
X3Byb2JlKCkuDQo+IA0KPiBTbyBkcml2ZXJzL21pc2MvYXRtZWxfdGNsaWIuYyBhbmQgZHJpdmVy
cy9waW5jdHJsL3BpbmN0cmwtY29oOTAxLmMsIHdoaWNoIHVzZQ0KPiBhcmNoX2luaXRjYWxsKCks
IGNhbm5vdCB3b3JrPw0KSG93IHRob3NlIGRyaXZlcnMgd29yayBJIGRvIG5vdCBrbm93LiBIb3dl
dmVyLCBJIHRyaWVkIHdpdGggdGhlIHJ6bjEgY2xvY2sNCmRyaXZlciBhbmQgaXQgZG9lcyBub3Qg
d29yay4NCg0KPiBJZiB0aGF0IGlzIHJlYWxseSB0cnVlLCB5b3UgY2FuIHN0aWxsIHVzZSBzdWJz
eXNfaW5pdGNhbGwoKSBpbiB0aGUgY2xvY2sgZHJpdmVyLCBhbmQNCj4gc3Vic3lzX2luaXRjYWxs
X3N5bmMoKSBpbiB0aGUgcGluY3RybCBkcml2ZXIuDQpUcnVlLCBJJ20gbm90IHN1cmUgaG93IHBl
b3BsZSBkZWNpZGUgd2hpY2ggaW5pdGNhbGwgdG8gdXNlIGFuZCB3aGV0aGVyIHRoZQ0KX3N5bmMg
dmFyaWFudHMgb2YgdGhlIGluaXRjYWxscyBoYXZlIGEgc3BlY2lhbCBtZWFuaW5nIG9yIGludGVu
dGlvbi4gQXMgZmFyIGFzDQpJIGtub3cgdGhleSB3ZXJlIGludHJvZHVjZWQgZm9yIHRocmVhZGVk
IHByb2Jlcywgc28gYXJlIHdlIHN1cHBvc2VkIHRvDQp1c2UgdGhlbSBmb3IgZHJpdmVyIGRlcGVu
ZGVuY2llcyBsaWtlIHRoaXM/DQoNCkRvIHlvdSBrbm93IHdoeSB0aGUgcnphMSBwaW5jdHJsIGRy
aXZlciBpcyBydW5uaW5nIGZyb20gY29yZV9pbml0Y2FsbCgpPw0KDQpUaGFua3MNClBoaWwNCg==

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring
  2018-07-20 13:57             ` Phil Edworthy
  (?)
@ 2018-08-01 12:35             ` Geert Uytterhoeven
  -1 siblings, 0 replies; 11+ messages in thread
From: Geert Uytterhoeven @ 2018-08-01 12:35 UTC (permalink / raw)
  To: Phil Edworthy
  Cc: Simon Horman, linux-clk, Linux Kernel Mailing List,
	Linux-Renesas, Jacopo Mondi

Hi Phil,

CC Jacopo

On Fri, Jul 20, 2018 at 3:57 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> On 20 July 2018 13:41, Geert Uytterhoeven wrote:
> > On Fri, Jul 20, 2018 at 2:26 PM Phil Edworthy wrote:
> > > On 20 July 2018 13:12, Geert Uytterhoeven wrote:
> > > > On Fri, Jul 20, 2018 at 2:06 PM Phil Edworthy  wrote:
> > > > > On 20 July 2018 12:21, Geert Uytterhoeven wrote:
> > > > > > On Wed, Jul 18, 2018 at 4:34 PM Phil Edworthy wrote:
> > > > > > > To avoid all SoC peripheral drivers deferring their probes,
> > > > > > > both clock and pinctrl drivers should already be probed. Since
> > > > > > > the pinctrl driver requires a clock to access the registers,
> > > > > > > the clock driver should be probed before the pinctrl driver.
> > > > > > >
> > > > > > > Therefore, move the clock driver from subsys_initcall to
> > core_initcall.
> > > > > > >
> > > > > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > > > >
> > > > > > Thanks for your patch!
> > > > > Thanks for your review!
> > > > >
> > > > > > The (not yet upstreamed) pinctrl driver uses postcore_initcall(), right?
> > > > > No, the pinctrl driver uses subsys_initcall, but postcore_initcall
> > > > > or arch_initcall may be better to make it clear about the dependencies.
> > > >
> > > > if the pinctrl driver uses subsys_initcall(), ...
> >
> > > > > > > -subsys_initcall(r9a06g032_clocks_init);
> > > > > > > +core_initcall(r9a06g032_clocks_init);
> > > >
> > > > ... using postcore_initcall() or arch_initcall() here, should work
> > > > with platform_driver_probe()?
> > > Nope, you have to use platform_driver_register() for DT based drivers.
> > > subsys_initcall is the earliest you can use platform_driver_probe().
> >
> > So drivers/misc/atmel_tclib.c and drivers/pinctrl/pinctrl-coh901.c, which use
> > arch_initcall(), cannot work?
> How those drivers work I do not know. However, I tried with the rzn1 clock
> driver and it does not work.
>
> > If that is really true, you can still use subsys_initcall() in the clock driver, and
> > subsys_initcall_sync() in the pinctrl driver.
> True, I'm not sure how people decide which initcall to use and whether the
> _sync variants of the initcalls have a special meaning or intention. As far as
> I know they were introduced for threaded probes, so are we supposed to
> use them for driver dependencies like this?
>
> Do you know why the rza1 pinctrl driver is running from core_initcall()?

Probably because it can?
Note that drivers/clk/renesas/clk-rz.c uses CLK_OF_DECLARE().

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-08-01 12:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-18 14:34 [PATCH] clk: renesas: r9a06g032: Avoid needless probe deferring Phil Edworthy
2018-07-20 11:21 ` Geert Uytterhoeven
2018-07-20 12:06   ` Phil Edworthy
2018-07-20 12:06     ` Phil Edworthy
2018-07-20 12:12     ` Geert Uytterhoeven
2018-07-20 12:26       ` Phil Edworthy
2018-07-20 12:26         ` Phil Edworthy
2018-07-20 12:41         ` Geert Uytterhoeven
2018-07-20 13:57           ` Phil Edworthy
2018-07-20 13:57             ` Phil Edworthy
2018-08-01 12:35             ` Geert Uytterhoeven

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.