[PATCH linux dev-4.17 1/7] clk: Aspeed: Modify clk-aspeed.c driver probe sequence

[Joel Stanley]

On 17 July 2018 at 19:15, Benjamin Herrenschmidt
<benh at kernel.crashing.org> wrote:
> On Tue, 2018-07-17 at 14:04 +0800, Ryan Chen wrote:
>> On Wed, Jul 11, 2018 at 03:47:56PM +1000, Benjamin Herrenschmidt wrote:
>> > On Wed, 2018-07-11 at 13:17 +0800, Ryan Chen wrote:
>> > > In Aspeed's SoC, all IP clk gating and pll parameter is in scu
>> > > controller, before IP driver probe, scu driver need prepare for it.
>> > > So buildin_platform_driver to core_initcall.
>> > >
>> > > Signed-off-by: Ryan Chen <ryanchen.aspeed at gmail.com>
>> > > ---
>> > >  drivers/clk/clk-aspeed.c | 7 ++++++-
>> > >  1 file changed, 6 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/clk/clk-aspeed.c b/drivers/clk/clk-aspeed.c
>> > > index 8796b8a..9e55743 100644
>> > > --- a/drivers/clk/clk-aspeed.c
>> > > +++ b/drivers/clk/clk-aspeed.c
>> > > @@ -573,7 +573,12 @@ static struct platform_driver aspeed_clk_driver = {
>> > >           .suppress_bind_attrs = true,
>> > >   },
>> > >  };
>> > > -builtin_platform_driver(aspeed_clk_driver);
>> > > +
>> > > +static int __init aspeed_clk_init(void)
>> > > +{
>> > > + return platform_driver_register(&aspeed_clk_driver);
>> > > +}
>> > > +core_initcall(aspeed_clk_init);
>> > >
>> > >  static void __init aspeed_ast2400_cc(struct regmap *map)
>> > >  {
>> >
>> > It's generally considered dangerous to register drivers at core
>> > initcall time.
>>
>> Understand.
>>
>> But if interrupt controller have clk gating.
>> the scu driver should be eraly than irq chip driver probe.
>> Is this point is reasonable?
>
> I'm not sure I understand what you are trying to solve other than
> making sure the clock driver is loaded before everything else. Joel, do
> we have a way to ensure that ? I noticed all other clock drivers use
> that macro to be initialized at of_clk_init, any reason we don't ?
>>
>> >
>> > Any reason we don't use the generic clock driver registration mechanism
>> > that runs at of_clk_init() time ?
>>
>> I will use "if (gate->reset_idx == aspeed_resets[ASPEED_RESET_SDHCI])",
>> is it suitable ?
>
> I'm on holiday, I don't have the code at hand to check. Joel ? What do
> you reckon ?

We don't need to do this. I mentioned this in my review; we have the
reset added to the clk driver already. With this, reset can be
released via the normal reset controller call in the sdhci driver.