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 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 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 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
RE: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
Hi Linus, On Tue, Jan 29, 2013 at 16:29:54, Linus Walleij wrote: > On Tue, Jan 29, 2013 at 8:38 AM, Vishwanathrao Badarkhe, Manish > 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 > > So your I2C driver is not returning -EPROBE_DEFER if it cannot find its pins? Yes. In Davinci I2C driver, it's not implemented to return -EPROBE_DEFER if it can't find its pins. Please see below discussion for more details: https://patchwork2.kernel.org/patch/2031321/ > > Hm, well I can live with this, if Tony ACKs it. > > Yours, > Linus Walleij > Regards, Manish ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
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 > 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 > > 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 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
* Linus Walleij [130129 03:03]: > On Tue, Jan 29, 2013 at 8:38 AM, Vishwanathrao Badarkhe, Manish > 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 > > 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 ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
* Tony Lindgren [130201 09:12]: > * Linus Walleij [130129 03:03]: > > On Tue, Jan 29, 2013 at 8:38 AM, Vishwanathrao Badarkhe, Manish > > 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 > > > > 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-disc...@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss ___ Davinci-linux-open-source mailing list Davinci-linux-open-source@linux.davincidsp.com http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
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 [130201 09:12]:
> > * Linus Walleij [130129 03:03]:
> > > On Tue, Jan 29, 2013 at 8:38 AM, Vishwanathrao Badarkhe, Manish
> > > 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
> > >
> > > 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
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
Re: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module_exit
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.
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
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
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
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
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
> 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
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
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
___
Davinci-linux-open-source mailing list
Davinci-linux-open-source@linux.davincidsp.com
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
