| Message ID | 1358494279-16503-10-git-send-email-haojian.zhuang@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jan 18, 2013 at 8:31 AM, Haojian Zhuang <haojian.zhuang@linaro.org> wrote: > Replace subsys initcall by module initcall level. Since pinctrl > driver is already launched before gpio driver. It's unnecessary > to set gpio driver in subsys init call level. > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> On you platform maybe it works, but have you made sure that nobody else will be affected? SPEAr of course, then these: arch/arm/mach-realview/core.c: * GPIO on PL061 is active, which is the proper arch/arm/mach-socfpga/Kconfig: select GPIO_PL061 if GPIOLIB Pawel, Dinh: are you OK with this change? Yours, Linus Walleij
On Mon, 2013-01-21 at 14:41 +0000, Linus Walleij wrote: > On Fri, Jan 18, 2013 at 8:31 AM, Haojian Zhuang > <haojian.zhuang@linaro.org> wrote: > > > Replace subsys initcall by module initcall level. Since pinctrl > > driver is already launched before gpio driver. It's unnecessary > > to set gpio driver in subsys init call level. > > > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> > > On you platform maybe it works, but have you made sure that nobody > else will be affected? > > SPEAr of course, then these: > > arch/arm/mach-realview/core.c: * GPIO on PL061 is active, > which is the proper > arch/arm/mach-socfpga/Kconfig: select GPIO_PL061 if GPIOLIB > > Pawel, Dinh: are you OK with this change? Hm. Doesn't this make the MMCI probing depending on the module_init order? As in: wouldn't it make the mmci probe completely fail (not even defer it) if it was called before the pl061? In that case even your, Linus, hack with inverting the CD status wouldn't work... Pawel
Hi Linus, On Mon, 2013-01-21 at 16:24 +0000, Pawel Moll wrote: > On Mon, 2013-01-21 at 14:41 +0000, Linus Walleij wrote: > > On Fri, Jan 18, 2013 at 8:31 AM, Haojian Zhuang > > <haojian.zhuang@linaro.org> wrote: > > > > > Replace subsys initcall by module initcall level. Since pinctrl > > > driver is already launched before gpio driver. It's unnecessary > > > to set gpio driver in subsys init call level. > > > > > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> > > > > On you platform maybe it works, but have you made sure that nobody > > else will be affected? > > > > SPEAr of course, then these: > > > > arch/arm/mach-realview/core.c: * GPIO on PL061 is active, > > which is the proper > > arch/arm/mach-socfpga/Kconfig: select GPIO_PL061 if GPIOLIB > > > > Pawel, Dinh: are you OK with this change? Still works ok on mach-socfpga. Dinh > > Hm. Doesn't this make the MMCI probing depending on the module_init > order? As in: wouldn't it make the mmci probe completely fail (not even > defer it) if it was called before the pl061? In that case even your, > Linus, hack with inverting the CD status wouldn't work... > > Pawel > > > >
On 22 January 2013 00:24, Pawel Moll <pawel.moll@arm.com> wrote: > On Mon, 2013-01-21 at 14:41 +0000, Linus Walleij wrote: >> On Fri, Jan 18, 2013 at 8:31 AM, Haojian Zhuang >> <haojian.zhuang@linaro.org> wrote: >> >> > Replace subsys initcall by module initcall level. Since pinctrl >> > driver is already launched before gpio driver. It's unnecessary >> > to set gpio driver in subsys init call level. >> > >> > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> >> >> On you platform maybe it works, but have you made sure that nobody >> else will be affected? >> >> SPEAr of course, then these: >> >> arch/arm/mach-realview/core.c: * GPIO on PL061 is active, >> which is the proper >> arch/arm/mach-socfpga/Kconfig: select GPIO_PL061 if GPIOLIB >> >> Pawel, Dinh: are you OK with this change? > > Hm. Doesn't this make the MMCI probing depending on the module_init > order? As in: wouldn't it make the mmci probe completely fail (not even > defer it) if it was called before the pl061? In that case even your, > Linus, hack with inverting the CD status wouldn't work... > > Pawel > > > The sequence of module probe is pinctrl --> gpio --> mmc. So the dependance of mmc on gpio isn't broken. Regards Haojian
On Mon, Jan 21, 2013 at 5:24 PM, Pawel Moll <pawel.moll@arm.com> wrote: > Hm. Doesn't this make the MMCI probing depending on the module_init > order? As in: wouldn't it make the mmci probe completely fail (not even > defer it) if it was called before the pl061? In that case even your, > Linus, hack with inverting the CD status wouldn't work... According to Haojian it works, but for sure the MMCI driver *should* (on the eternal list of SHOULDDO) bail out and return any -EPROBE_DEFER up to the AMBA bus core properly so it will be tried again later if this happens. AFAICT gpio_request() will return -EPROBE_DEFER if the GPIO cannot be located. Yours, Linus Walleij
On Mon, Jan 21, 2013 at 9:20 PM, Dinh Nguyen <dinguyen@altera.com> wrote: >> > Pawel, Dinh: are you OK with this change? > > Still works ok on mach-socfpga. Thanks! Adding your Tested-by tag, OK? Yours, Linus Walleij
On Fri, Jan 18, 2013 at 8:31 AM, Haojian Zhuang <haojian.zhuang@linaro.org> wrote: > Replace subsys initcall by module initcall level. Since pinctrl > driver is already launched before gpio driver. It's unnecessary > to set gpio driver in subsys init call level. > > Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> OK some consensus that this works, and moving initcalls to module_init() should be encouraged, so applied this and thrown at linux-next for testing. No pain no gain. Yours, Linus Walleij
On Tue, 2013-01-22 at 09:42 +0000, Linus Walleij wrote: > AFAICT gpio_request() will return -EPROBE_DEFER if the GPIO > cannot be located. If it does, I withdraw all my objections. Cheers! Pawe?
On Mon, 2013-01-21 at 23:33 +0000, Haojian Zhuang wrote: > The sequence of module probe is pinctrl --> gpio --> mmc. So the dependance > of mmc on gpio isn't broken. I don't think you can guarantee the "gpio --> mmc" sequence if both drivers use module_init (the order could be completely random, generally it depends on what Kbuild is doing), but if gpio_request() behaves as Linus said, the sequence in the problematic scenario would be "pinctrl --> mmc --> EPROBE_DEFER --> gpio --> mmc" which is fine (subject to mmci driver fix, but it's a separate issue). Pawe?
On Tue, Jan 22, 2013 at 10:42:11AM +0100, Linus Walleij wrote: > On Mon, Jan 21, 2013 at 5:24 PM, Pawel Moll <pawel.moll@arm.com> wrote: > > > Hm. Doesn't this make the MMCI probing depending on the module_init > > order? As in: wouldn't it make the mmci probe completely fail (not even > > defer it) if it was called before the pl061? In that case even your, > > Linus, hack with inverting the CD status wouldn't work... > > According to Haojian it works, but for sure the MMCI driver *should* > (on the eternal list of SHOULDDO) bail out and return any Rather than talking about what should and should not, why not look at the code - it only takes a few moments to check what the behaviour would be. And it is correct - errors are correctly propagated out of the probe function. That's hardly surprising given who's supposed to be the maintainer of that driver.
On Tue, Jan 22, 2013 at 11:41 AM, Russell King - ARM Linux <linux@arm.linux.org.uk> wrote: > On Tue, Jan 22, 2013 at 10:42:11AM +0100, Linus Walleij wrote: >> On Mon, Jan 21, 2013 at 5:24 PM, Pawel Moll <pawel.moll@arm.com> wrote: >> >> > Hm. Doesn't this make the MMCI probing depending on the module_init >> > order? As in: wouldn't it make the mmci probe completely fail (not even >> > defer it) if it was called before the pl061? In that case even your, >> > Linus, hack with inverting the CD status wouldn't work... >> >> According to Haojian it works, but for sure the MMCI driver *should* >> (on the eternal list of SHOULDDO) bail out and return any > > Rather than talking about what should and should not, why not look at > the code - it only takes a few moments to check what the behaviour > would be. And it is correct - errors are correctly propagated out of > the probe function. OK good! > That's hardly surprising given who's supposed to be the maintainer of > that driver. Aouh that hurt! ;-) Thanks Russell, Linus Walleij
Hi Linus, On Tue, 2013-01-22 at 10:44 +0100, Linus Walleij wrote: > On Mon, Jan 21, 2013 at 9:20 PM, Dinh Nguyen <dinguyen@altera.com> wrote: > > >> > Pawel, Dinh: are you OK with this change? > > > > Still works ok on mach-socfpga. > > Thanks! Adding your Tested-by tag, OK? Yep, thanks. Dinh > > Yours, > Linus Walleij >
diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c index 8cfdf60..76adf33 100644 --- a/drivers/gpio/gpio-pl061.c +++ b/drivers/gpio/gpio-pl061.c @@ -393,7 +393,7 @@ static int __init pl061_gpio_init(void) { return amba_driver_register(&pl061_gpio_driver); } -subsys_initcall(pl061_gpio_init); +module_init(pl061_gpio_init); MODULE_AUTHOR("Baruch Siach <baruch@tkos.co.il>"); MODULE_DESCRIPTION("PL061 GPIO driver");
Replace subsys initcall by module initcall level. Since pinctrl driver is already launched before gpio driver. It's unnecessary to set gpio driver in subsys init call level. Signed-off-by: Haojian Zhuang <haojian.zhuang@linaro.org> --- drivers/gpio/gpio-pl061.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)