Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
Well it should be pretty trivial to update drivers to use deferred probing. Maybe some spatch to check for that in driver probes would help getting an idea how many might be affected? That's what I am trying to say. It surely is easy to fix the drivers, once we know there is something in need of fixing. My question was if there is common sense to simply risk breaking things and fix them later (then I'd apply patches switching from subsys_initcall to module_init right away), or if we can gather ideas how to minimize the impact of regressions (before applying such patches). signature.asc Description: Digital signature
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
* Tony Lindgren t...@atomide.com [131008 15:19]:
* Wolfram Sang w...@the-dreams.de [131008 14:01]:
On Fri, Aug 30, 2013 at 01:27:13AM -0700, Tony Lindgren wrote:
* zhangfei gao zhangfei@gmail.com [130829 23:36]:
What about concerns from Wolfram:
Other people might be
depending on subsys_initcall to get I2C active before they want to
activate, say, PMICs. So, I fear regressions, since deferred probing
might not be available in the needed places to avoid these regressions.
There should not be any reason to get a PMIC activated
early on. The system should be booting already at that point,
and the PMIC related init can be done later on.
Okay, here is a more concrete example:
Consider the amplifier driver 'sound/soc/codecs/max9768.c'. Back then,
unaware
of deferred probing, I wrote the following code to get the GPIOs (which are
optional):
err = gpio_request_one(pdata-mute_gpio, GPIOF_INIT_HIGH, MAX9768
Mute);
max9768-mute_gpio = err ?: pdata-mute_gpio;
And later in the process:
if (gpio_is_valid(max9768-mute_gpio)) {
ret = snd_soc_add_codec_controls(codec, max9768_mute,
ARRAY_SIZE(max9768_mute));
if (ret)
return ret;
}
So, the mute control will only be added if the gpio_request succeeded. On
that
particular board, the mute GPIO was wired to an I2C GPIO controller. If I
now
change the I2C (or GPIO) driver from subsys_initcall to module_init, then
the
gpio_request in the amplifier driver could hit -EPROBE_DEFER and the mute
control will then disappear. Yes, the driver can be fixed easily, yet I
fear a
number of regressions like this. Instead of people digging into why things
disappear after a kernel update, I wonder if there is a way to guide users
if
this happens. I didn't have time for that, though, sadly. Still, it makes me
wonder how easily we could shift from subsys_initcall to module_init,
although
I'd really love to get away from subsys_initcall in device drivers.
Well it should be pretty trivial to update drivers to use deferred
probing. Maybe some spatch to check for that in driver probes would
help getting an idea how many might be affected?
Anyways, it should be fixed as otherwise we'll just dig ourselves
deeper into the mess of things not working as loadable modules.
BTW, another place where things can go wrong is if there's an irqchip
driver that is being set up at module_init time. If an interrupt client
driver does irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0), the
resources may not have been initialize for the DT case as those are
populated triggered by of_platform_populate().
The fix there is to use irq = irq_of_parse_and_map(pdev-dev.of_node, 0)
instead. Or somehow make of_platform_populate() support -EPROBE_DEFER.
Just FYI,
Tony
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
On Fri, Aug 30, 2013 at 01:27:13AM -0700, Tony Lindgren wrote:
* zhangfei gao zhangfei@gmail.com [130829 23:36]:
What about concerns from Wolfram:
Other people might be
depending on subsys_initcall to get I2C active before they want to
activate, say, PMICs. So, I fear regressions, since deferred probing
might not be available in the needed places to avoid these regressions.
There should not be any reason to get a PMIC activated
early on. The system should be booting already at that point,
and the PMIC related init can be done later on.
Okay, here is a more concrete example:
Consider the amplifier driver 'sound/soc/codecs/max9768.c'. Back then, unaware
of deferred probing, I wrote the following code to get the GPIOs (which are
optional):
err = gpio_request_one(pdata-mute_gpio, GPIOF_INIT_HIGH, MAX9768
Mute);
max9768-mute_gpio = err ?: pdata-mute_gpio;
And later in the process:
if (gpio_is_valid(max9768-mute_gpio)) {
ret = snd_soc_add_codec_controls(codec, max9768_mute,
ARRAY_SIZE(max9768_mute));
if (ret)
return ret;
}
So, the mute control will only be added if the gpio_request succeeded. On that
particular board, the mute GPIO was wired to an I2C GPIO controller. If I now
change the I2C (or GPIO) driver from subsys_initcall to module_init, then the
gpio_request in the amplifier driver could hit -EPROBE_DEFER and the mute
control will then disappear. Yes, the driver can be fixed easily, yet I fear a
number of regressions like this. Instead of people digging into why things
disappear after a kernel update, I wonder if there is a way to guide users if
this happens. I didn't have time for that, though, sadly. Still, it makes me
wonder how easily we could shift from subsys_initcall to module_init, although
I'd really love to get away from subsys_initcall in device drivers.
Regards,
Wolfram
signature.asc
Description: Digital signature
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
* Wolfram Sang w...@the-dreams.de [131008 14:01]:
On Fri, Aug 30, 2013 at 01:27:13AM -0700, Tony Lindgren wrote:
* zhangfei gao zhangfei@gmail.com [130829 23:36]:
What about concerns from Wolfram:
Other people might be
depending on subsys_initcall to get I2C active before they want to
activate, say, PMICs. So, I fear regressions, since deferred probing
might not be available in the needed places to avoid these regressions.
There should not be any reason to get a PMIC activated
early on. The system should be booting already at that point,
and the PMIC related init can be done later on.
Okay, here is a more concrete example:
Consider the amplifier driver 'sound/soc/codecs/max9768.c'. Back then, unaware
of deferred probing, I wrote the following code to get the GPIOs (which are
optional):
err = gpio_request_one(pdata-mute_gpio, GPIOF_INIT_HIGH, MAX9768
Mute);
max9768-mute_gpio = err ?: pdata-mute_gpio;
And later in the process:
if (gpio_is_valid(max9768-mute_gpio)) {
ret = snd_soc_add_codec_controls(codec, max9768_mute,
ARRAY_SIZE(max9768_mute));
if (ret)
return ret;
}
So, the mute control will only be added if the gpio_request succeeded. On that
particular board, the mute GPIO was wired to an I2C GPIO controller. If I now
change the I2C (or GPIO) driver from subsys_initcall to module_init, then the
gpio_request in the amplifier driver could hit -EPROBE_DEFER and the mute
control will then disappear. Yes, the driver can be fixed easily, yet I fear a
number of regressions like this. Instead of people digging into why things
disappear after a kernel update, I wonder if there is a way to guide users if
this happens. I didn't have time for that, though, sadly. Still, it makes me
wonder how easily we could shift from subsys_initcall to module_init, although
I'd really love to get away from subsys_initcall in device drivers.
Well it should be pretty trivial to update drivers to use deferred
probing. Maybe some spatch to check for that in driver probes would
help getting an idea how many might be affected?
Anyways, it should be fixed as otherwise we'll just dig ourselves
deeper into the mess of things not working as loadable modules.
Regards,
Tony
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
On Thu, Sep 12, 2013 at 9:12 AM, zhangfei gao zhangfei@gmail.com wrote: On Fri, Aug 30, 2013 at 4:27 PM, Tony Lindgren t...@atomide.com wrote: * zhangfei gao zhangfei@gmail.com [130829 23:36]: What about concerns from Wolfram: Other people might be depending on subsys_initcall to get I2C active before they want to activate, say, PMICs. So, I fear regressions, since deferred probing might not be available in the needed places to avoid these regressions. There should not be any reason to get a PMIC activated early on. The system should be booting already at that point, and the PMIC related init can be done later on. Is it too late using module_init for PMIC? You can probably do it as a fix early on during the -rc cycle too. Of course it needs to be verified to work first :) Dear Wolfram What's your suggestion about this issue. Use subsys_initcall, deferred probing still exist if base on pin control driver. Thanks Dear Wolfram Any plan about the patch? On one hand, module_X_driver is trend to replace subsys_initcall Refer from Mark We're trying to move away from needing to do this and to using deferred probing to resolve init ordering issues. Should we not be able to convert the drivers to module_X_driver()? On the other hand, subsys_initcall still been defered if pin controller driver been relied on. Thanks -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
Any plan about the patch? I am right now working on a patch series dealing with this. signature.asc Description: Digital signature
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
On Fri, Aug 30, 2013 at 4:27 PM, Tony Lindgren t...@atomide.com wrote: * zhangfei gao zhangfei@gmail.com [130829 23:36]: What about concerns from Wolfram: Other people might be depending on subsys_initcall to get I2C active before they want to activate, say, PMICs. So, I fear regressions, since deferred probing might not be available in the needed places to avoid these regressions. There should not be any reason to get a PMIC activated early on. The system should be booting already at that point, and the PMIC related init can be done later on. Is it too late using module_init for PMIC? You can probably do it as a fix early on during the -rc cycle too. Of course it needs to be verified to work first :) Dear Wolfram What's your suggestion about this issue. Use subsys_initcall, deferred probing still exist if base on pin control driver. Thanks -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
On Fri, Aug 30, 2013 at 1:48 PM, Tony Lindgren t...@atomide.com wrote: * zhangfei gao zhangfei@gmail.com [130829 04:03]: On Thu, Aug 29, 2013 at 4:58 PM, Linus Walleij linus.wall...@linaro.org wrote: On Wed, Aug 28, 2013 at 11:57 AM, Wolfram Sang w...@the-dreams.de wrote: On Tue, Aug 20, 2013 at 04:32:28PM +0800, Zhangfei Gao wrote: Instead of use platform_driver_probe, use module_platform_driver To support deferred probing Also subsys_initcall may too early to auto set pinctl Signed-off-by: Zhangfei Gao zhangfei@linaro.org Acked-by: Baruch Siach bar...@tkos.co.il This patch is tougher than it looks. You need it, because subsys_initcall may be too early for pinctrl. pinctrl is initialized very early, core_initcall(). This is more a question of individual pin control drivers and when they probe, and dependencies trying to take a pinctrl handle before the pin controller is available will be deferred. Even by those grabbed in the core by drivers/base/pinctrl.c. Thanks Linus. Your explanation is really make sense. We use drivers/pinctrl/pinctrl-single.c, if subsys_initcall for pinctrl-single, no issue at all. So far we've seen that if you have issues with this, the real problem is that some other driver is trying to initialize way too early probably because of legacy reasons that no longer apply. FYI, it's best to have all the drivers initialize with just module_init and make them work as loadable modules because of the following reasons: 1. You will get real console error messages when something goes wrong with no need for debug_ll and earlyprintk 2. By creating loadable driver modules you're already getting some protection from spaghetti code as the interfaces are defined 3. It will be easier for distros to support various ARM SoCs with loadable driver modules Regards, Tony Thanks Tony, What about concerns from Wolfram: Other people might be depending on subsys_initcall to get I2C active before they want to activate, say, PMICs. So, I fear regressions, since deferred probing might not be available in the needed places to avoid these regressions. Is it too late using module_init for PMIC? Besides, the deferred probing still there if depend on late registered pin control driver. Thanks -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
* zhangfei gao zhangfei@gmail.com [130829 23:36]: What about concerns from Wolfram: Other people might be depending on subsys_initcall to get I2C active before they want to activate, say, PMICs. So, I fear regressions, since deferred probing might not be available in the needed places to avoid these regressions. There should not be any reason to get a PMIC activated early on. The system should be booting already at that point, and the PMIC related init can be done later on. Is it too late using module_init for PMIC? You can probably do it as a fix early on during the -rc cycle too. Of course it needs to be verified to work first :) Besides, the deferred probing still there if depend on late registered pin control driver. At least for omaps we have things working just fine with pinctrl-single and a PMIC on I2C controller. Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
On Wed, Aug 28, 2013 at 11:57 AM, Wolfram Sang w...@the-dreams.de wrote: On Tue, Aug 20, 2013 at 04:32:28PM +0800, Zhangfei Gao wrote: Instead of use platform_driver_probe, use module_platform_driver To support deferred probing Also subsys_initcall may too early to auto set pinctl Signed-off-by: Zhangfei Gao zhangfei@linaro.org Acked-by: Baruch Siach bar...@tkos.co.il This patch is tougher than it looks. You need it, because subsys_initcall may be too early for pinctrl. pinctrl is initialized very early, core_initcall(). This is more a question of individual pin control drivers and when they probe, and dependencies trying to take a pinctrl handle before the pin controller is available will be deferred. Even by those grabbed in the core by drivers/base/pinctrl.c. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
On Thu, Aug 29, 2013 at 4:58 PM, Linus Walleij linus.wall...@linaro.org wrote:
On Wed, Aug 28, 2013 at 11:57 AM, Wolfram Sang w...@the-dreams.de wrote:
On Tue, Aug 20, 2013 at 04:32:28PM +0800, Zhangfei Gao wrote:
Instead of use platform_driver_probe, use module_platform_driver
To support deferred probing
Also subsys_initcall may too early to auto set pinctl
Signed-off-by: Zhangfei Gao zhangfei@linaro.org
Acked-by: Baruch Siach bar...@tkos.co.il
This patch is tougher than it looks. You need it, because
subsys_initcall may be too early for pinctrl.
pinctrl is initialized very early, core_initcall().
This is more a question of individual pin control drivers
and when they probe, and dependencies trying to take
a pinctrl handle before the pin controller is available
will be deferred. Even by those grabbed in the core
by drivers/base/pinctrl.c.
Thanks Linus.
Your explanation is really make sense.
We use drivers/pinctrl/pinctrl-single.c, if subsys_initcall for
pinctrl-single, no issue at all.
Checked in 3.11-rc4, there is really deferring probe happen.
i2c_designware fcb08000.i2c: could not find pctldev for node
/amba/pinmux@fc803000/i2c0
_pmx_func, deferring probe
However, bus_probe_device failed, since the drv name list does not
have i2c_designware.
deferred_probe_work_func - bus_probe_device - device_attach -
bus_for_each_drv - __device_attach
It can be solved change
return platform_driver_probe(dw_i2c_driver, dw_i2c_probe);
to
return platform_driver_register(dw_i2c_driver);
static struct platform_driver dw_i2c_driver = {
.probe = dw_i2c_probe,
~
Dear Wolfram
Thanks for telling me the dependency about subsys_initcall.
Should I resubmit one patch using platform_driver_register while
keeping subsys_initcall?
Besides, also find platform_driver_probe is used in
drivers/i2c/busses/i2c-imx.c and drivers/i2c/busses/i2c-stu300.c.
Thanks
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
On Thu, Aug 29, 2013 at 12:55 PM, zhangfei gao zhangfei@gmail.com wrote: Besides, also find platform_driver_probe is used in drivers/i2c/busses/i2c-imx.c and drivers/i2c/busses/i2c-stu300.c. The platform_driver_probe() is basically a footprint optimization (more code can be discarded after boot) and I'm happy to patch it if it disturbs anything, it is *really* not important for this driver. Do you guys need a low footprint? Else there is no use to have platform_driver_probe() in there. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: getting rid of subsys_initcall usage? (was: Re: [PATCH RESEND] i2c: designware: use module_platform_driver)
* zhangfei gao zhangfei@gmail.com [130829 04:03]: On Thu, Aug 29, 2013 at 4:58 PM, Linus Walleij linus.wall...@linaro.org wrote: On Wed, Aug 28, 2013 at 11:57 AM, Wolfram Sang w...@the-dreams.de wrote: On Tue, Aug 20, 2013 at 04:32:28PM +0800, Zhangfei Gao wrote: Instead of use platform_driver_probe, use module_platform_driver To support deferred probing Also subsys_initcall may too early to auto set pinctl Signed-off-by: Zhangfei Gao zhangfei@linaro.org Acked-by: Baruch Siach bar...@tkos.co.il This patch is tougher than it looks. You need it, because subsys_initcall may be too early for pinctrl. pinctrl is initialized very early, core_initcall(). This is more a question of individual pin control drivers and when they probe, and dependencies trying to take a pinctrl handle before the pin controller is available will be deferred. Even by those grabbed in the core by drivers/base/pinctrl.c. Thanks Linus. Your explanation is really make sense. We use drivers/pinctrl/pinctrl-single.c, if subsys_initcall for pinctrl-single, no issue at all. So far we've seen that if you have issues with this, the real problem is that some other driver is trying to initialize way too early probably because of legacy reasons that no longer apply. FYI, it's best to have all the drivers initialize with just module_init and make them work as loadable modules because of the following reasons: 1. You will get real console error messages when something goes wrong with no need for debug_ll and earlyprintk 2. By creating loadable driver modules you're already getting some protection from spaghetti code as the interfaces are defined 3. It will be easier for distros to support various ARM SoCs with loadable driver modules Regards, Tony -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
