linux-kernel - RE: [PATCH V2 1/6] pinctrl: pinctrl-single: use arch_initcall and module

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 5 Feb 2013 06:36:34 +0000
From:	"Vishwanathrao Badarkhe, Manish" <manishv.b@...com>
To:	Tony Lindgren <tony@...mide.com>,
	Linus Walleij <linus.walleij@...aro.org>
CC:	"davinci-linux-open-source@...ux.davincidsp.com" 
	<davinci-linux-open-source@...ux.davincidsp.com>,
	"linux@....linux.org.uk" <linux@....linux.org.uk>,
	"sameo@...ux.intel.com" <sameo@...ux.intel.com>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	"khilman@...prootsystems.com" <khilman@...prootsystems.com>,
	"devicetree-discuss@...ts.ozlabs.org" 
	<devicetree-discuss@...ts.ozlabs.org>,
	"broonie@...nsource.wolfsonmicro.com" 
	<broonie@...nsource.wolfsonmicro.com>,
	"Nori, Sekhar" <nsekhar@...com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	"hs@...x.de" <hs@...x.de>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: 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 <tony@...mide.com> [130201 09:12]:
> > * Linus Walleij <linus.walleij@...aro.org> [130129 03:03]:
> > > On Tue, Jan 29, 2013 at 8:38 AM, Vishwanathrao Badarkhe, Manish 
> > > <manishv.b@...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 <manishv.b@...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@...4120
/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 fffffe07"

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/