diff mbox

pinctrl: mediatek: convert to arch_initcall

Message ID 1450412497-25872-1-git-send-email-djkurtz@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Kurtz Dec. 18, 2015, 4:21 a.m. UTC
Move pinctrl initialization earlier in boot so that real devices can find
their pctldev without probe deferring.

Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
---
 drivers/pinctrl/mediatek/pinctrl-mt6397.c | 2 +-
 drivers/pinctrl/mediatek/pinctrl-mt8127.c | 2 +-
 drivers/pinctrl/mediatek/pinctrl-mt8135.c | 2 +-
 drivers/pinctrl/mediatek/pinctrl-mt8173.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

Comments

Yingjoe Chen Dec. 18, 2015, 3:06 p.m. UTC | #1
On Fri, 2015-12-18 at 12:21 +0800, Daniel Kurtz wrote:
> Move pinctrl initialization earlier in boot so that real devices can find
> their pctldev without probe deferring.
> 
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> ---
>  drivers/pinctrl/mediatek/pinctrl-mt6397.c | 2 +-
>  drivers/pinctrl/mediatek/pinctrl-mt8127.c | 2 +-
>  drivers/pinctrl/mediatek/pinctrl-mt8135.c | 2 +-
>  drivers/pinctrl/mediatek/pinctrl-mt8173.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt6397.c b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> index f9751ae..a3780d4 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> @@ -70,7 +70,7 @@ static int __init mtk_pinctrl_init(void)
>  	return platform_driver_register(&mtk_pinctrl_driver);
>  }
>  
> -module_init(mtk_pinctrl_init);
> +arch_initcall(mtk_pinctrl_init);


MT6397 is PMIC, which depends on pwrap on main AP to work. Since
pmic-wrap itself is module_platform_driver, I think it make sense to
keep this one as module_init. Maybe adding a comment to explain why it
is different from others will help.

Joe.C

>  
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("MediaTek MT6397 Pinctrl Driver");
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8127.c b/drivers/pinctrl/mediatek/pinctrl-mt8127.c
> index b317b0b..98e0beb 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mt8127.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt8127.c
> @@ -351,7 +351,7 @@ static int __init mtk_pinctrl_init(void)
>  	return platform_driver_register(&mtk_pinctrl_driver);
>  }
>  
> -module_init(mtk_pinctrl_init);
> +arch_initcall(mtk_pinctrl_init);
>  
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("MediaTek MT8127 Pinctrl Driver");
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8135.c b/drivers/pinctrl/mediatek/pinctrl-mt8135.c
> index 404f117..1c153b8 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mt8135.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt8135.c
> @@ -366,7 +366,7 @@ static int __init mtk_pinctrl_init(void)
>  	return platform_driver_register(&mtk_pinctrl_driver);
>  }
>  
> -module_init(mtk_pinctrl_init);
> +arch_initcall(mtk_pinctrl_init);
>  
>  MODULE_LICENSE("GPL");
>  MODULE_DESCRIPTION("MediaTek Pinctrl Driver");
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8173.c b/drivers/pinctrl/mediatek/pinctrl-mt8173.c
> index ad27184..a62514e 100644
> --- a/drivers/pinctrl/mediatek/pinctrl-mt8173.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt8173.c
> @@ -394,7 +394,7 @@ static int __init mtk_pinctrl_init(void)
>  	return platform_driver_register(&mtk_pinctrl_driver);
>  }
>  
> -module_init(mtk_pinctrl_init);
> +arch_initcall(mtk_pinctrl_init);
>  
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("MediaTek Pinctrl Driver");
Daniel Kurtz Dec. 21, 2015, 6:51 a.m. UTC | #2
On Fri, Dec 18, 2015 at 11:06 PM, Yingjoe Chen
<yingjoe.chen@mediatek.com> wrote:
> On Fri, 2015-12-18 at 12:21 +0800, Daniel Kurtz wrote:
>> Move pinctrl initialization earlier in boot so that real devices can find
>> their pctldev without probe deferring.
>>
>> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
>> ---
>>  drivers/pinctrl/mediatek/pinctrl-mt6397.c | 2 +-
>>  drivers/pinctrl/mediatek/pinctrl-mt8127.c | 2 +-
>>  drivers/pinctrl/mediatek/pinctrl-mt8135.c | 2 +-
>>  drivers/pinctrl/mediatek/pinctrl-mt8173.c | 2 +-
>>  4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt6397.c b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
>> index f9751ae..a3780d4 100644
>> --- a/drivers/pinctrl/mediatek/pinctrl-mt6397.c
>> +++ b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
>> @@ -70,7 +70,7 @@ static int __init mtk_pinctrl_init(void)
>>       return platform_driver_register(&mtk_pinctrl_driver);
>>  }
>>
>> -module_init(mtk_pinctrl_init);
>> +arch_initcall(mtk_pinctrl_init);
>
>
> MT6397 is PMIC, which depends on pwrap on main AP to work. Since
> pmic-wrap itself is module_platform_driver, I think it make sense to
> keep this one as module_init. Maybe adding a comment to explain why it
> is different from others will help.

I interpret this the other way - I think that since the PMIC wrapper
provides a bus required for the system PMIC it should also be a
builtin and use arch_initcall.

WDYT?

-Dan
Yingjoe Chen Dec. 21, 2015, 12:39 p.m. UTC | #3
On Mon, 2015-12-21 at 14:51 +0800, Daniel Kurtz wrote:
> On Fri, Dec 18, 2015 at 11:06 PM, Yingjoe Chen
> <yingjoe.chen@mediatek.com> wrote:
> > On Fri, 2015-12-18 at 12:21 +0800, Daniel Kurtz wrote:
> >> Move pinctrl initialization earlier in boot so that real devices can find
> >> their pctldev without probe deferring.
> >>
> >> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> >> ---
> >>  drivers/pinctrl/mediatek/pinctrl-mt6397.c | 2 +-
> >>  drivers/pinctrl/mediatek/pinctrl-mt8127.c | 2 +-
> >>  drivers/pinctrl/mediatek/pinctrl-mt8135.c | 2 +-
> >>  drivers/pinctrl/mediatek/pinctrl-mt8173.c | 2 +-
> >>  4 files changed, 4 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt6397.c b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> >> index f9751ae..a3780d4 100644
> >> --- a/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> >> +++ b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> >> @@ -70,7 +70,7 @@ static int __init mtk_pinctrl_init(void)
> >>       return platform_driver_register(&mtk_pinctrl_driver);
> >>  }
> >>
> >> -module_init(mtk_pinctrl_init);
> >> +arch_initcall(mtk_pinctrl_init);
> >
> >
> > MT6397 is PMIC, which depends on pwrap on main AP to work. Since
> > pmic-wrap itself is module_platform_driver, I think it make sense to
> > keep this one as module_init. Maybe adding a comment to explain why it
> > is different from others will help.
> 
> I interpret this the other way - I think that since the PMIC wrapper
> provides a bus required for the system PMIC it should also be a
> builtin and use arch_initcall.

We'll have to change mt8173 PMIC wrapper and mt6397 MFD core to
arch_initcall if we want to do it.

I think regulators on PMIC is more important than pinctrl. For all
mt8173 systems, few drivers depends on PMIC pinctrl to work but many
depends on the regulators. So if we adjust pinctrl to arch_initcall, we
should change mt6397 regulator as well.

Joe.C
Daniel Kurtz Dec. 21, 2015, 12:45 p.m. UTC | #4
On Mon, Dec 21, 2015 at 8:39 PM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
>
> On Mon, 2015-12-21 at 14:51 +0800, Daniel Kurtz wrote:
> > On Fri, Dec 18, 2015 at 11:06 PM, Yingjoe Chen
> > <yingjoe.chen@mediatek.com> wrote:
> > > On Fri, 2015-12-18 at 12:21 +0800, Daniel Kurtz wrote:
> > >> Move pinctrl initialization earlier in boot so that real devices can find
> > >> their pctldev without probe deferring.
> > >>
> > >> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> > >> ---
> > >>  drivers/pinctrl/mediatek/pinctrl-mt6397.c | 2 +-
> > >>  drivers/pinctrl/mediatek/pinctrl-mt8127.c | 2 +-
> > >>  drivers/pinctrl/mediatek/pinctrl-mt8135.c | 2 +-
> > >>  drivers/pinctrl/mediatek/pinctrl-mt8173.c | 2 +-
> > >>  4 files changed, 4 insertions(+), 4 deletions(-)
> > >>
> > >> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt6397.c b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> > >> index f9751ae..a3780d4 100644
> > >> --- a/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> > >> +++ b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
> > >> @@ -70,7 +70,7 @@ static int __init mtk_pinctrl_init(void)
> > >>       return platform_driver_register(&mtk_pinctrl_driver);
> > >>  }
> > >>
> > >> -module_init(mtk_pinctrl_init);
> > >> +arch_initcall(mtk_pinctrl_init);
> > >
> > >
> > > MT6397 is PMIC, which depends on pwrap on main AP to work. Since
> > > pmic-wrap itself is module_platform_driver, I think it make sense to
> > > keep this one as module_init. Maybe adding a comment to explain why it
> > > is different from others will help.
> >
> > I interpret this the other way - I think that since the PMIC wrapper
> > provides a bus required for the system PMIC it should also be a
> > builtin and use arch_initcall.
>
> We'll have to change mt8173 PMIC wrapper and mt6397 MFD core to
> arch_initcall if we want to do it.
>
> I think regulators on PMIC is more important than pinctrl. For all
> mt8173 systems, few drivers depends on PMIC pinctrl to work but many
> depends on the regulators. So if we adjust pinctrl to arch_initcall, we
> should change mt6397 regulator as well.

ACK to changing all of mt6397 to arch_initcall, especially the regulators.
Linus Walleij Dec. 22, 2015, 10:23 a.m. UTC | #5
On Fri, Dec 18, 2015 at 5:21 AM, Daniel Kurtz <djkurtz@chromium.org> wrote:

> Move pinctrl initialization earlier in boot so that real devices can find
> their pctldev without probe deferring.
>
> Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>

I interpret the discussion as I should wait for a new version of this?

Or should it be applied?

Yours,
Linus Walleij
Yingjoe Chen Dec. 22, 2015, 1:36 p.m. UTC | #6
On Tue, 2015-12-22 at 11:23 +0100, Linus Walleij wrote:
> On Fri, Dec 18, 2015 at 5:21 AM, Daniel Kurtz <djkurtz@chromium.org> wrote:
> 
> > Move pinctrl initialization earlier in boot so that real devices can find
> > their pctldev without probe deferring.
> >
> > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> 
> I interpret the discussion as I should wait for a new version of this?
> 
> Or should it be applied?

We'll have another patch to change init orders for pwrap, mt6397 mfd
core and mt6397 regulator. Before that, the mt6397 pinctrl changes in
this patch won't work as expect. Apply this now won't cause any problem
either, unless someone think we should stink to current orders.

So I think we could apply this now:

Acked-by: Yingjoe Chen <yingjoe.chen@mediatek.com>

Joe.C
Daniel Kurtz Dec. 22, 2015, 1:41 p.m. UTC | #7
On Tue, Dec 22, 2015 at 9:36 PM, Yingjoe Chen <yingjoe.chen@mediatek.com> wrote:
> On Tue, 2015-12-22 at 11:23 +0100, Linus Walleij wrote:
>> On Fri, Dec 18, 2015 at 5:21 AM, Daniel Kurtz <djkurtz@chromium.org> wrote:
>>
>> > Move pinctrl initialization earlier in boot so that real devices can find
>> > their pctldev without probe deferring.
>> >
>> > Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
>>
>> I interpret the discussion as I should wait for a new version of this?
>>
>> Or should it be applied?
>
> We'll have another patch to change init orders for pwrap, mt6397 mfd
> core and mt6397 regulator. Before that, the mt6397 pinctrl changes in
> this patch won't work as expect. Apply this now won't cause any problem
> either, unless someone think we should stink to current orders.
>
> So I think we could apply this now:
>
> Acked-by: Yingjoe Chen <yingjoe.chen@mediatek.com>
>
> Joe.C

Joe.C, Linus,

Thanks for the Ack.
I think it is cleaner if I just send a new patch without the mt6397 change.
So, I'll do that in a bit.
diff mbox

Patch

diff --git a/drivers/pinctrl/mediatek/pinctrl-mt6397.c b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
index f9751ae..a3780d4 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mt6397.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mt6397.c
@@ -70,7 +70,7 @@  static int __init mtk_pinctrl_init(void)
 	return platform_driver_register(&mtk_pinctrl_driver);
 }
 
-module_init(mtk_pinctrl_init);
+arch_initcall(mtk_pinctrl_init);
 
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("MediaTek MT6397 Pinctrl Driver");
diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8127.c b/drivers/pinctrl/mediatek/pinctrl-mt8127.c
index b317b0b..98e0beb 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mt8127.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mt8127.c
@@ -351,7 +351,7 @@  static int __init mtk_pinctrl_init(void)
 	return platform_driver_register(&mtk_pinctrl_driver);
 }
 
-module_init(mtk_pinctrl_init);
+arch_initcall(mtk_pinctrl_init);
 
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("MediaTek MT8127 Pinctrl Driver");
diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8135.c b/drivers/pinctrl/mediatek/pinctrl-mt8135.c
index 404f117..1c153b8 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mt8135.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mt8135.c
@@ -366,7 +366,7 @@  static int __init mtk_pinctrl_init(void)
 	return platform_driver_register(&mtk_pinctrl_driver);
 }
 
-module_init(mtk_pinctrl_init);
+arch_initcall(mtk_pinctrl_init);
 
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("MediaTek Pinctrl Driver");
diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8173.c b/drivers/pinctrl/mediatek/pinctrl-mt8173.c
index ad27184..a62514e 100644
--- a/drivers/pinctrl/mediatek/pinctrl-mt8173.c
+++ b/drivers/pinctrl/mediatek/pinctrl-mt8173.c
@@ -394,7 +394,7 @@  static int __init mtk_pinctrl_init(void)
 	return platform_driver_register(&mtk_pinctrl_driver);
 }
 
-module_init(mtk_pinctrl_init);
+arch_initcall(mtk_pinctrl_init);
 
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("MediaTek Pinctrl Driver");