Re: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
On Tue, Feb 5, 2013 at 7:36 AM, Vishwanathrao Badarkhe, Manish manish...@ti.com wrote: I made following changes, in order to update dip-p pointer with correct value: - if (!dpi-p) { + if (IS_ERR_OR_NULL(dpi-p)) { dpi-p = devm_pinctrl_get(dev); - if (IS_ERR_OR_NULL(dpi-p)) { - int ret = PTR_ERR(dpi-p); - - dev_dbg(dev, no pinctrl handle\n); - /* Only return deferrals */ - if (ret == -EPROBE_DEFER) - return ret; - return 0; - } + ret = PTR_ERR(dpi-p); + dev_dbg(dev, no pinctrl handle\n); + /* Only return deferrals */ + if (ret == -EPROBE_DEFER) + return ret; + return 0; This is based on some old code that I wrote ages ago. Check the pinctrl tree or linux-next for the latest core pin grabbing code. Use that instead. Yours, Linus Walleij ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
Hi Linus On Tue, Feb 05, 2013 at 18:30:48, Linus Walleij wrote: On Tue, Feb 5, 2013 at 7:36 AM, Vishwanathrao Badarkhe, Manish manish...@ti.com wrote: I made following changes, in order to update dip-p pointer with correct value: - if (!dpi-p) { + if (IS_ERR_OR_NULL(dpi-p)) { dpi-p = devm_pinctrl_get(dev); - if (IS_ERR_OR_NULL(dpi-p)) { - int ret = PTR_ERR(dpi-p); - - dev_dbg(dev, no pinctrl handle\n); - /* Only return deferrals */ - if (ret == -EPROBE_DEFER) - return ret; - return 0; - } + ret = PTR_ERR(dpi-p); + dev_dbg(dev, no pinctrl handle\n); + /* Only return deferrals */ + if (ret == -EPROBE_DEFER) + return ret; + return 0; This is based on some old code that I wrote ages ago. Check the pinctrl tree or linux-next for the latest core pin grabbing code. Use that instead. Thanks for your reply. I have checked with your latest changes regarding pinctrl in linux-next tree and these changes are working fine for me. Regards Manish Badarkhe ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
Hi Russell, On Tue, Feb 05, 2013 at 16:57:31, Russell King - ARM Linux wrote: On Tue, Feb 05, 2013 at 06:36:34AM +, Vishwanathrao Badarkhe, Manish wrote: I made following changes, in order to update dip-p pointer with correct value: - if (!dpi-p) { + if (IS_ERR_OR_NULL(dpi-p)) { dpi-p = devm_pinctrl_get(dev); - if (IS_ERR_OR_NULL(dpi-p)) { - int ret = PTR_ERR(dpi-p); - - dev_dbg(dev, no pinctrl handle\n); - /* Only return deferrals */ - if (ret == -EPROBE_DEFER) - return ret; - return 0; - } + ret = PTR_ERR(dpi-p); + dev_dbg(dev, no pinctrl handle\n); + /* Only return deferrals */ + if (ret == -EPROBE_DEFER) + return ret; + return 0; Is this intended change? The above looks totally broken to me. Oh, it's using IS_ERR_OR_NULL(), so it's bound to be broken. I went through discussion in community on usage of IS_ERR_OR_NULL. https://patchwork.kernel.org/patch/1953271/ Will take care not use this in future. Thanks, Manish ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
RE: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
On Fri, Feb 01, 2013 at 22:41:24, Tony Lindgren wrote: * Tony Lindgren t...@atomide.com [130201 09:12]: * Linus Walleij linus.wall...@linaro.org [130129 03:03]: On Tue, Jan 29, 2013 at 8:38 AM, Vishwanathrao Badarkhe, Manish manish...@ti.com wrote: Currently, I2C driver gets probed before pinctrl driver. To achieve I2C pin muxing via pinctrl driver before I2C probe get called, register pinctrl driver in arch_initcall. Also, add module_exit to unregister pinctrl driver. Signed-off-by: Vishwanathrao Badarkhe, Manish manish...@ti.com So your I2C driver is not returning -EPROBE_DEFER if it cannot find its pins? Hm, well I can live with this, if Tony ACKs it. Hmm pinctrl is before i2c in drivers/Makefile. Making initcalls happen earlier and earlier is usually the wrong way to go. Sounds like there's some other issue here that needs to be fixed instead. Let me guess: The i2c driver is wrongly set to run with arch_initcall? Hi Tony No, Currently i2c driver is set to subsys_initcall. I have seen some problem while using pin control grab functionality. Please see detailed explanation as below. Hi Linus I am using auto grab pin control facility to do pin muxing of I2C0 pins and seen problem as below: Currently, probe of I2C0 driver gets called before pin control driver. Hence, while calling I2C0 probe because of unavailability of pin control node information, its probe get deferred giving following messages: i2c_davinci i2c_davinci.1: could not find pctldev for node /soc/pinmux@1c14120 /pinmux_i2c0_pins, deferring probe As I2C0 is in deferred list (as auto grab patch handle this), its probe get called once again, During this second time probe of I2C0, I have observed following crash: Unable to handle kernel paging request at virtual address fe07 As per code analysis of auto grab functionality, I have seen in pinctrl_bind_pin function, pin control handle (dpi-p) is returned by devm_pinctrl_get function. Pin control handle is assigned with error pointer during 1st time probing of I2C0 (as pin control information is not available at this time). During 2nd time probing (deferred probe) of I2C0, same pin control handle (which was get assigned during 1st probe) is getting used instead of getting updated to correct pin control handle which leads to system crash. I made following changes, in order to update dip-p pointer with correct value: - if (!dpi-p) { + if (IS_ERR_OR_NULL(dpi-p)) { dpi-p = devm_pinctrl_get(dev); - if (IS_ERR_OR_NULL(dpi-p)) { - int ret = PTR_ERR(dpi-p); - - dev_dbg(dev, no pinctrl handle\n); - /* Only return deferrals */ - if (ret == -EPROBE_DEFER) - return ret; - return 0; - } + ret = PTR_ERR(dpi-p); + dev_dbg(dev, no pinctrl handle\n); + /* Only return deferrals */ + if (ret == -EPROBE_DEFER) + return ret; + return 0; Is this intended change? or am I missing something in order to use this auto grab functionality? With the above change, now deferred probing is working fine and there is no need to register pin control driver in arch_initcall. Regards, Manish ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
[PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
Currently, I2C driver gets probed before pinctrl driver. To achieve I2C pin muxing via pinctrl driver before I2C probe get called, register pinctrl driver in arch_initcall. Also, add module_exit to unregister pinctrl driver. Signed-off-by: Vishwanathrao Badarkhe, Manish manish...@ti.com --- :100644 100644 f6a360b... 3a96390... M drivers/pinctrl/pinctrl-single.c drivers/pinctrl/pinctrl-single.c | 12 +++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/drivers/pinctrl/pinctrl-single.c b/drivers/pinctrl/pinctrl-single.c index f6a360b..3a96390 100644 --- a/drivers/pinctrl/pinctrl-single.c +++ b/drivers/pinctrl/pinctrl-single.c @@ -1089,7 +1089,17 @@ static struct platform_driver pcs_driver = { }, }; -module_platform_driver(pcs_driver); +static int __init pcs_pinctrl_init(void) +{ + return platform_driver_register(pcs_driver); +} +arch_initcall(pcs_pinctrl_init); + +static void __exit pcs_pinctrl_exit(void) +{ + platform_driver_unregister(pcs_driver); +} +module_exit(pcs_pinctrl_exit); MODULE_AUTHOR(Tony Lindgren t...@atomide.com); MODULE_DESCRIPTION(One-register-per-pin type device tree based pinctrl driver); -- 1.7.4.1 ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
* Linus Walleij linus.wall...@linaro.org [130129 03:03]: On Tue, Jan 29, 2013 at 8:38 AM, Vishwanathrao Badarkhe, Manish manish...@ti.com wrote: Currently, I2C driver gets probed before pinctrl driver. To achieve I2C pin muxing via pinctrl driver before I2C probe get called, register pinctrl driver in arch_initcall. Also, add module_exit to unregister pinctrl driver. Signed-off-by: Vishwanathrao Badarkhe, Manish manish...@ti.com So your I2C driver is not returning -EPROBE_DEFER if it cannot find its pins? Hm, well I can live with this, if Tony ACKs it. Hmm pinctrl is before i2c in drivers/Makefile. Making initcalls happen earlier and earlier is usually the wrong way to go. Sounds like there's some other issue here that needs to be fixed instead. Regards, Tony ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
* Tony Lindgren t...@atomide.com [130201 09:12]: * Linus Walleij linus.wall...@linaro.org [130129 03:03]: On Tue, Jan 29, 2013 at 8:38 AM, Vishwanathrao Badarkhe, Manish manish...@ti.com wrote: Currently, I2C driver gets probed before pinctrl driver. To achieve I2C pin muxing via pinctrl driver before I2C probe get called, register pinctrl driver in arch_initcall. Also, add module_exit to unregister pinctrl driver. Signed-off-by: Vishwanathrao Badarkhe, Manish manish...@ti.com So your I2C driver is not returning -EPROBE_DEFER if it cannot find its pins? Hm, well I can live with this, if Tony ACKs it. Hmm pinctrl is before i2c in drivers/Makefile. Making initcalls happen earlier and earlier is usually the wrong way to go. Sounds like there's some other issue here that needs to be fixed instead. Let me guess: The i2c driver is wrongly set to run with arch_initcall? Regards, Tony ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
On 1/29/2013 4:29 PM, Linus Walleij wrote: On Tue, Jan 29, 2013 at 8:38 AM, Vishwanathrao Badarkhe, Manish manish...@ti.com wrote: Currently, I2C driver gets probed before pinctrl driver. To achieve I2C pin muxing via pinctrl driver before I2C probe get called, register pinctrl driver in arch_initcall. Also, add module_exit to unregister pinctrl driver. Signed-off-by: Vishwanathrao Badarkhe, Manish manish...@ti.com So your I2C driver is not returning -EPROBE_DEFER if it cannot find its pins? Hm, well I can live with this, if Tony ACKs it. FWIW, most of the other pinctrl drivers seem to be using arch_initcall() already. $ grep -r arch_initcall drivers/pinctrl/* drivers/pinctrl/pinctrl-at91.c:arch_initcall(at91_pinctrl_init); drivers/pinctrl/pinctrl-coh901.c:arch_initcall(u300_gpio_init); drivers/pinctrl/pinctrl-imx35.c:arch_initcall(imx35_pinctrl_init); drivers/pinctrl/pinctrl-imx51.c:arch_initcall(imx51_pinctrl_init); drivers/pinctrl/pinctrl-imx53.c:arch_initcall(imx53_pinctrl_init); drivers/pinctrl/pinctrl-imx6q.c:arch_initcall(imx6q_pinctrl_init); drivers/pinctrl/pinctrl-sirf.c:arch_initcall(sirfsoc_pinmux_init); drivers/pinctrl/pinctrl-tegra20.c:arch_initcall(tegra20_pinctrl_init); drivers/pinctrl/pinctrl-tegra30.c:arch_initcall(tegra30_pinctrl_init); drivers/pinctrl/pinctrl-u300.c:arch_initcall(u300_pmx_init); drivers/pinctrl/spear/pinctrl-spear300.c:arch_initcall(spear300_pinctrl_init); drivers/pinctrl/spear/pinctrl-spear320.c:arch_initcall(spear320_pinctrl_init); drivers/pinctrl/spear/pinctrl-spear310.c:arch_initcall(spear310_pinctrl_init); drivers/pinctrl/spear/pinctrl-spear1340.c:arch_initcall(spear1340_pinctrl_init); drivers/pinctrl/spear/pinctrl-spear1310.c:arch_initcall(spear1310_pinctrl_init); Thanks, Sekhar ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss
Re: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
On Tue, Jan 29, 2013 at 8:38 AM, Vishwanathrao Badarkhe, Manish manish...@ti.com wrote: Currently, I2C driver gets probed before pinctrl driver. To achieve I2C pin muxing via pinctrl driver before I2C probe get called, register pinctrl driver in arch_initcall. Also, add module_exit to unregister pinctrl driver. Signed-off-by: Vishwanathrao Badarkhe, Manish manish...@ti.com So your I2C driver is not returning -EPROBE_DEFER if it cannot find its pins? Hm, well I can live with this, if Tony ACKs it. Yours, Linus Walleij ___ devicetree-discuss mailing list devicetree-discuss@lists.ozlabs.org https://lists.ozlabs.org/listinfo/devicetree-discuss