diff mbox

[01/10] pinctrl: use postcore_initcall

Message ID 1350551224-12857-1-git-send-email-haojian.zhuang@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Haojian Zhuang Oct. 18, 2012, 9:06 a.m. UTC
Since pins are configured in device driver, pinctrl driver should be
loaded by those device driver. module_platform_driver() only declares
pinctrl driver is in module_initcall privilege. Use postcore_initcall
privilege instead.

Signed-off-by: Haojian Zhuang <haojian.zhuang@gmail.com>
---
 drivers/pinctrl/pinctrl-single.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

Comments

Linus Walleij Oct. 18, 2012, 6:20 p.m. UTC | #1
This patch has the wrong subject, should be pinctrl: single: or
pinctrl/single: ...

Anyway, I need Tony to ACK them before merging.

Yours,
Linus Walleij
Stephen Warren Oct. 18, 2012, 10:18 p.m. UTC | #2
On 10/18/2012 03:06 AM, Haojian Zhuang wrote:
> Since pins are configured in device driver, pinctrl driver should be
> loaded by those device driver. module_platform_driver() only declares
> pinctrl driver is in module_initcall privilege. Use postcore_initcall
> privilege instead.

I'm not convinced this is needed; doesn't deferred probe sort out the
dependencies correctly?
Tony Lindgren Oct. 18, 2012, 10:28 p.m. UTC | #3
* Stephen Warren <swarren@wwwdotorg.org> [121018 15:20]:
> On 10/18/2012 03:06 AM, Haojian Zhuang wrote:
> > Since pins are configured in device driver, pinctrl driver should be
> > loaded by those device driver. module_platform_driver() only declares
> > pinctrl driver is in module_initcall privilege. Use postcore_initcall
> > privilege instead.
> 
> I'm not convinced this is needed; doesn't deferred probe sort out the
> dependencies correctly?

I'm a bit concerned about this need too as the trend is towards
initializing things later than earlier. The drivers/Makefile order
and deferred probe should be already enough?

Specifically could you decribe the cases where this issue happens?
Also check if one of your client drivers has some early initcall
that's no longer needed.

Regards,

Tony
Haojian Zhuang Oct. 19, 2012, 2:16 a.m. UTC | #4
On Fri, Oct 19, 2012 at 6:28 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Stephen Warren <swarren@wwwdotorg.org> [121018 15:20]:
>> On 10/18/2012 03:06 AM, Haojian Zhuang wrote:
>> > Since pins are configured in device driver, pinctrl driver should be
>> > loaded by those device driver. module_platform_driver() only declares
>> > pinctrl driver is in module_initcall privilege. Use postcore_initcall
>> > privilege instead.
>>
>> I'm not convinced this is needed; doesn't deferred probe sort out the
>> dependencies correctly?
>
> I'm a bit concerned about this need too as the trend is towards
> initializing things later than earlier. The drivers/Makefile order
> and deferred probe should be already enough?
>
> Specifically could you decribe the cases where this issue happens?
> Also check if one of your client drivers has some early initcall
> that's no longer needed.
>
> Regards,
>
> Tony

Yes, the special case is PMIC. Most of PMIC are based on I2C/SPI bus.
It means that I2C/SPI bus driver should be initialized firstly. For example,
we could find that PMIC mfd driver are initialized in subsys init call level.
It means that pinctrl should be initialized earlier than I2C/SPI bus driver.
Otherwise, pins of I2C bus may not be configured as I2C function since
pinctrl driver is module init call level.

Regards
Haojian
Jean-Christophe PLAGNIOL-VILLARD Oct. 19, 2012, 2:24 a.m. UTC | #5
On 17:06 Thu 18 Oct     , Haojian Zhuang wrote:
> Since pins are configured in device driver, pinctrl driver should be
> loaded by those device driver. module_platform_driver() only declares
> pinctrl driver is in module_initcall privilege. Use postcore_initcall
> privilege instead.
this  should not be mandatory as you can use defer probe

Best Regards,
J.
Tony Lindgren Oct. 19, 2012, 2:38 a.m. UTC | #6
* Haojian Zhuang <haojian.zhuang@gmail.com> [121018 19:17]:
> On Fri, Oct 19, 2012 at 6:28 AM, Tony Lindgren <tony@atomide.com> wrote:
> >
> > Specifically could you decribe the cases where this issue happens?
> > Also check if one of your client drivers has some early initcall
> > that's no longer needed.
> 
> Yes, the special case is PMIC. Most of PMIC are based on I2C/SPI bus.
> It means that I2C/SPI bus driver should be initialized firstly. For example,
> we could find that PMIC mfd driver are initialized in subsys init call level.
> It means that pinctrl should be initialized earlier than I2C/SPI bus driver.
> Otherwise, pins of I2C bus may not be configured as I2C function since
> pinctrl driver is module init call level.

Hmm, the order in drivers/Makefile is already:

pinctrl/
i2c/

Maybe check that your i2c drivers don't have non-standard initcalls?

Also the i2c drivers may need to return -EPROBE_DEFER?

Regards,

Tony
Haojian Zhuang Oct. 19, 2012, 2:53 a.m. UTC | #7
On Fri, Oct 19, 2012 at 10:38 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Haojian Zhuang <haojian.zhuang@gmail.com> [121018 19:17]:
>> On Fri, Oct 19, 2012 at 6:28 AM, Tony Lindgren <tony@atomide.com> wrote:
>> >
>> > Specifically could you decribe the cases where this issue happens?
>> > Also check if one of your client drivers has some early initcall
>> > that's no longer needed.
>>
>> Yes, the special case is PMIC. Most of PMIC are based on I2C/SPI bus.
>> It means that I2C/SPI bus driver should be initialized firstly. For example,
>> we could find that PMIC mfd driver are initialized in subsys init call level.
>> It means that pinctrl should be initialized earlier than I2C/SPI bus driver.
>> Otherwise, pins of I2C bus may not be configured as I2C function since
>> pinctrl driver is module init call level.
>
> Hmm, the order in drivers/Makefile is already:
>
> pinctrl/
> i2c/
>
> Maybe check that your i2c drivers don't have non-standard initcalls?
>
> Also the i2c drivers may need to return -EPROBE_DEFER?
>
> Regards,
>
> Tony

OK. I'll support -EPROBE_DEFER if failed to get pin from pinctrl system.
This solution could also resolve the issue.

Regards
Haojian
Tony Lindgren Oct. 19, 2012, 5:41 p.m. UTC | #8
* Haojian Zhuang <haojian.zhuang@gmail.com> [121018 19:54]:
> On Fri, Oct 19, 2012 at 10:38 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Haojian Zhuang <haojian.zhuang@gmail.com> [121018 19:17]:
> >> On Fri, Oct 19, 2012 at 6:28 AM, Tony Lindgren <tony@atomide.com> wrote:
> >> >
> >> > Specifically could you decribe the cases where this issue happens?
> >> > Also check if one of your client drivers has some early initcall
> >> > that's no longer needed.
> >>
> >> Yes, the special case is PMIC. Most of PMIC are based on I2C/SPI bus.
> >> It means that I2C/SPI bus driver should be initialized firstly. For example,
> >> we could find that PMIC mfd driver are initialized in subsys init call level.
> >> It means that pinctrl should be initialized earlier than I2C/SPI bus driver.
> >> Otherwise, pins of I2C bus may not be configured as I2C function since
> >> pinctrl driver is module init call level.
> >
> > Hmm, the order in drivers/Makefile is already:
> >
> > pinctrl/
> > i2c/
> >
> > Maybe check that your i2c drivers don't have non-standard initcalls?
> >
> > Also the i2c drivers may need to return -EPROBE_DEFER?
> >
> > Regards,
> >
> > Tony
> 
> OK. I'll support -EPROBE_DEFER if failed to get pin from pinctrl system.
> This solution could also resolve the issue.

OK good to hear.

Regards,

Tony
diff mbox

Patch

diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c
index 76a4260..64d109a 100644
--- a/drivers/pinctrl/pinctrl-single.c
+++ b/drivers/pinctrl/pinctrl-single.c
@@ -980,7 +980,17 @@  static struct platform_driver pcs_driver = {
 	},
 };
 
-module_platform_driver(pcs_driver);
+static int __init pcs_driver_init(void)
+{
+	return platform_driver_register(&pcs_driver);
+}
+postcore_initcall(pcs_driver_init);
+
+static void __exit pcs_driver_exit(void)
+{
+	platform_driver_unregister(&pcs_driver);
+}
+module_exit(pcs_driver_exit);
 
 MODULE_AUTHOR("Tony Lindgren <tony@atomide.com>");
 MODULE_DESCRIPTION("One-register-per-pin type device tree based pinctrl driver");