[PATCH] pinctrl-zynq: Initialize early

[PATCH] pinctrl-zynq: Initialize early

[Mike Looijmans]

Supplying pinmux configuration for e.g. gpio pins leads to deferred
probes because the pinctrl device is probed much later than gpio.
Move the init call to a much earlier stage so it probes before the
devices that may need it.

Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
---
 drivers/pinctrl/pinctrl-zynq.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/pinctrl/pinctrl-zynq.c b/drivers/pinctrl/pinctrl-zynq.c
index fd4602d..36984f1 100644
--- a/drivers/pinctrl/pinctrl-zynq.c
+++ b/drivers/pinctrl/pinctrl-zynq.c
@@ -1173,7 +1173,17 @@ static struct platform_driver zynq_pinctrl_driver = {
 	.remove = zynq_pinctrl_remove,
 };
 
-module_platform_driver(zynq_pinctrl_driver);
+static int __init zynq_pinctrl_init(void)
+{
+	return platform_driver_register(&zynq_pinctrl_driver);
+}
+arch_initcall(zynq_pinctrl_init);
+
+static void __exit zynq_pinctrl_exit(void)
+{
+	platform_driver_unregister(&zynq_pinctrl_driver);
+}
+module_exit(zynq_pinctrl_exit);
 
 MODULE_AUTHOR("S??ren Brinkmann <soren.brinkmann@xilinx.com>");
 MODULE_DESCRIPTION("Xilinx Zynq pinctrl driver");
-- 
1.9.1

[PATCH] pinctrl-zynq: Initialize early

[Sören Brinkmann]

Hi Mike,

On Thu, 2015-10-22 at 01:30PM +0200, Mike Looijmans wrote:
> Supplying pinmux configuration for e.g. gpio pins leads to deferred
> probes because the pinctrl device is probed much later than gpio.
> Move the init call to a much earlier stage so it probes before the
> devices that may need it.
> 
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>

in general, the change should be OK, but neither on zc702 nor zc706 do I
see a difference in respect to deferred probes. With and without the
patch I see:
    root at zynq:~# dmesg | grep -i defer
    [    0.097021] zynq-gpio e000a000.gpio: could not find pctldev for node /amba/slcr at f8000000/pinctrl at 700/gpio0-default, deferring probe
    root at zynq:~# 

If you have a case this patch improves things though, feel free to add my
Tested-by: S?ren Brinkmann <soren.brinkmann@xilinx.com>

	Thanks,
	S?ren

[PATCH] pinctrl-zynq: Initialize early

[Mike Looijmans]

?On 22-10-15 18:07, S?ren Brinkmann wrote:
> Hi Mike,
>
> On Thu, 2015-10-22 at 01:30PM +0200, Mike Looijmans wrote:
>> Supplying pinmux configuration for e.g. gpio pins leads to deferred
>> probes because the pinctrl device is probed much later than gpio.
>> Move the init call to a much earlier stage so it probes before the
>> devices that may need it.
>>
>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>
> in general, the change should be OK, but neither on zc702 nor zc706 do I
> see a difference in respect to deferred probes. With and without the
> patch I see:
>      root at zynq:~# dmesg | grep -i defer
>      [    0.097021] zynq-gpio e000a000.gpio: could not find pctldev for node /amba/slcr at f8000000/pinctrl at 700/gpio0-default, deferring probe
>      root at zynq:~#
>
> If you have a case this patch improves things though, feel free to add my
> Tested-by: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>

On the Florida boards there are i2c controlled clocks, power supplies and 
reset signals. Replacing the Cadence I2C controller with a GPIO-bitbang 
controller solved the I2C problems but caused a storm of dozens of deferred 
probes because of the pinmux driver arriving even after the first probe 
attempt of the i2c bus driver. Moving the pinmux driver to an earlier stage 
solved that problem neatly, now the "zynq-pinctrl 700.pinctrl: zynq pinctrl 
initialized" message appears after the OCM driver.
Judging from your comment the GPIO driver still probes earlier (I don't have 
any GPIO-only pinmuxes yet), so maybe we should amend the patch to probe even 
earlier. The pinmux driver doesn't depend on anything, so it can potentially 
probe very early. What do you think?

Mike.


Kind regards,

Mike Looijmans
System Expert

TOPIC Embedded Products
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
Telefax: +31 (0) 499 33 69 70
E-mail: mike.looijmans at topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

Visit us at : Aerospace Electrical Systems Expo Europe which will be held from 17.11.2015 till 19.11.2015, Findorffstrasse 101 Bremen, Germany, Hall 5, stand number C65
http://www.aesexpo.eu

[PATCH] pinctrl-zynq: Initialize early

[Mike Looijmans]

?On 23-10-15 07:31, Mike Looijmans wrote:
> On 22-10-15 18:07, S?ren Brinkmann wrote:
>> Hi Mike,
>>
>> On Thu, 2015-10-22 at 01:30PM +0200, Mike Looijmans wrote:
>>> Supplying pinmux configuration for e.g. gpio pins leads to deferred
>>> probes because the pinctrl device is probed much later than gpio.
>>> Move the init call to a much earlier stage so it probes before the
>>> devices that may need it.
>>>
>>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>>
>> in general, the change should be OK, but neither on zc702 nor zc706 do I
>> see a difference in respect to deferred probes. With and without the
>> patch I see:
>>      root at zynq:~# dmesg | grep -i defer
>>      [    0.097021] zynq-gpio e000a000.gpio: could not find pctldev for node
>> /amba/slcr at f8000000/pinctrl at 700/gpio0-default, deferring probe
>>      root at zynq:~#
>>
>> If you have a case this patch improves things though, feel free to add my
>> Tested-by: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>
>
> On the Florida boards there are i2c controlled clocks, power supplies and
> reset signals. Replacing the Cadence I2C controller with a GPIO-bitbang
> controller solved the I2C problems but caused a storm of dozens of deferred
> probes because of the pinmux driver arriving even after the first probe
> attempt of the i2c bus driver. Moving the pinmux driver to an earlier stage
> solved that problem neatly, now the "zynq-pinctrl 700.pinctrl: zynq pinctrl
> initialized" message appears after the OCM driver.
> Judging from your comment the GPIO driver still probes earlier (I don't have
> any GPIO-only pinmuxes yet), so maybe we should amend the patch to probe even
> earlier. The pinmux driver doesn't depend on anything, so it can potentially
> probe very early. What do you think?

Minor addition:
The gpio-zynq driver probes at "postcore_initcall", so to beat that, the 
zynq-pinmux driver should move to "core_initcall" (instead of 
"arch_initcall"). That would make the gpio deferral go away.

An alternative would be to move the gpio driver to "arch", then "postcore" 
would be enough for the pinmux. If the gpio probe gets deferred, it apparently 
has already been moved to below "subsys" already, with aparently no ill effects.

Mike.


Kind regards,

Mike Looijmans
System Expert

TOPIC Embedded Products
Eindhovenseweg 32-C, NL-5683 KH Best
Postbus 440, NL-5680 AK Best
Telefoon: +31 (0) 499 33 69 79
Telefax: +31 (0) 499 33 69 70
E-mail: mike.looijmans at topicproducts.com
Website: www.topicproducts.com

Please consider the environment before printing this e-mail

Visit us at : Aerospace Electrical Systems Expo Europe which will be held from 17.11.2015 till 19.11.2015, Findorffstrasse 101 Bremen, Germany, Hall 5, stand number C65
http://www.aesexpo.eu

[PATCH] pinctrl-zynq: Initialize early

[Sören Brinkmann]

On Fri, 2015-10-23 at 07:43AM +0200, Mike Looijmans wrote:
> On 23-10-15 07:31, Mike Looijmans wrote:
> >On 22-10-15 18:07, S?ren Brinkmann wrote:
> >>Hi Mike,
> >>
> >>On Thu, 2015-10-22 at 01:30PM +0200, Mike Looijmans wrote:
> >>>Supplying pinmux configuration for e.g. gpio pins leads to deferred
> >>>probes because the pinctrl device is probed much later than gpio.
> >>>Move the init call to a much earlier stage so it probes before the
> >>>devices that may need it.
> >>>
> >>>Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> >>
> >>in general, the change should be OK, but neither on zc702 nor zc706 do I
> >>see a difference in respect to deferred probes. With and without the
> >>patch I see:
> >>     root at zynq:~# dmesg | grep -i defer
> >>     [    0.097021] zynq-gpio e000a000.gpio: could not find pctldev for node
> >>/amba/slcr at f8000000/pinctrl at 700/gpio0-default, deferring probe
> >>     root at zynq:~#
> >>
> >>If you have a case this patch improves things though, feel free to add my
> >>Tested-by: S?ren Brinkmann <soren.brinkmann@xilinx.com>
> >>
> >
> >On the Florida boards there are i2c controlled clocks, power supplies and
> >reset signals. Replacing the Cadence I2C controller with a GPIO-bitbang
> >controller solved the I2C problems but caused a storm of dozens of deferred
> >probes because of the pinmux driver arriving even after the first probe
> >attempt of the i2c bus driver. Moving the pinmux driver to an earlier stage
> >solved that problem neatly, now the "zynq-pinctrl 700.pinctrl: zynq pinctrl
> >initialized" message appears after the OCM driver.
> >Judging from your comment the GPIO driver still probes earlier (I don't have
> >any GPIO-only pinmuxes yet), so maybe we should amend the patch to probe even
> >earlier. The pinmux driver doesn't depend on anything, so it can potentially
> >probe very early. What do you think?
> 
> Minor addition:
> The gpio-zynq driver probes at "postcore_initcall", so to beat that, the
> zynq-pinmux driver should move to "core_initcall" (instead of
> "arch_initcall"). That would make the gpio deferral go away.
> 
> An alternative would be to move the gpio driver to "arch", then "postcore"
> would be enough for the pinmux. If the gpio probe gets deferred, it
> apparently has already been moved to below "subsys" already, with aparently
> no ill effects.

Makes me wonder whether a single deferred probe is worth the risk of
breaking something by moving these things around. But probably worth a
try.

	S?ren

[PATCH] pinctrl-zynq: Initialize early

[Sören Brinkmann]

On Fri, 2015-10-23 at 07:31AM +0200, Mike Looijmans wrote:
> On 22-10-15 18:07, S?ren Brinkmann wrote:
> >Hi Mike,
> >
> >On Thu, 2015-10-22 at 01:30PM +0200, Mike Looijmans wrote:
> >>Supplying pinmux configuration for e.g. gpio pins leads to deferred
> >>probes because the pinctrl device is probed much later than gpio.
> >>Move the init call to a much earlier stage so it probes before the
> >>devices that may need it.
> >>
> >>Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
> >
> >in general, the change should be OK, but neither on zc702 nor zc706 do I
> >see a difference in respect to deferred probes. With and without the
> >patch I see:
> >     root at zynq:~# dmesg | grep -i defer
> >     [    0.097021] zynq-gpio e000a000.gpio: could not find pctldev for node /amba/slcr at f8000000/pinctrl at 700/gpio0-default, deferring probe
> >     root at zynq:~#
> >
> >If you have a case this patch improves things though, feel free to add my
> >Tested-by: S?ren Brinkmann <soren.brinkmann@xilinx.com>
> >
> 
> On the Florida boards there are i2c controlled clocks, power supplies and
> reset signals. Replacing the Cadence I2C controller with a GPIO-bitbang
> controller solved the I2C problems but caused a storm of dozens of deferred
> probes because of the pinmux driver arriving even after the first probe
> attempt of the i2c bus driver. Moving the pinmux driver to an earlier stage
> solved that problem neatly, now the "zynq-pinctrl 700.pinctrl: zynq pinctrl
> initialized" message appears after the OCM driver.

OK, makes sense. Thanks for the background.

> Judging from your comment the GPIO driver still probes earlier (I don't have
> any GPIO-only pinmuxes yet), so maybe we should amend the patch to probe
> even earlier. The pinmux driver doesn't depend on anything, so it can
> potentially probe very early. What do you think?

I'm pretty neutral on this one :) Hasn't the probe deferral mechanism
been introduced to avoid having to create ordering through the initcall
stages? But I agree, having the probe deferral notices is not particularly
pretty. So, I'd definitely not oppose changing this.
Though, there is one dependency on the SLCR regmap, but that is initialized
fairly early.

	S?ren

[PATCH] pinctrl-zynq: Initialize early

[Michal Simek]

Hi Linus,

On 10/23/2015 07:44 AM, S?ren Brinkmann wrote:
> On Fri, 2015-10-23 at 07:31AM +0200, Mike Looijmans wrote:
>> On 22-10-15 18:07, S?ren Brinkmann wrote:
>>> Hi Mike,
>>>
>>> On Thu, 2015-10-22 at 01:30PM +0200, Mike Looijmans wrote:
>>>> Supplying pinmux configuration for e.g. gpio pins leads to deferred
>>>> probes because the pinctrl device is probed much later than gpio.
>>>> Move the init call to a much earlier stage so it probes before the
>>>> devices that may need it.
>>>>
>>>> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>
>>>
>>> in general, the change should be OK, but neither on zc702 nor zc706 do I
>>> see a difference in respect to deferred probes. With and without the
>>> patch I see:
>>>     root at zynq:~# dmesg | grep -i defer
>>>     [    0.097021] zynq-gpio e000a000.gpio: could not find pctldev for node /amba/slcr at f8000000/pinctrl at 700/gpio0-default, deferring probe
>>>     root at zynq:~#
>>>
>>> If you have a case this patch improves things though, feel free to add my
>>> Tested-by: S?ren Brinkmann <soren.brinkmann@xilinx.com>
>>>
>>
>> On the Florida boards there are i2c controlled clocks, power supplies and
>> reset signals. Replacing the Cadence I2C controller with a GPIO-bitbang
>> controller solved the I2C problems but caused a storm of dozens of deferred
>> probes because of the pinmux driver arriving even after the first probe
>> attempt of the i2c bus driver. Moving the pinmux driver to an earlier stage
>> solved that problem neatly, now the "zynq-pinctrl 700.pinctrl: zynq pinctrl
>> initialized" message appears after the OCM driver.
> 
> OK, makes sense. Thanks for the background.
> 
>> Judging from your comment the GPIO driver still probes earlier (I don't have
>> any GPIO-only pinmuxes yet), so maybe we should amend the patch to probe
>> even earlier. The pinmux driver doesn't depend on anything, so it can
>> potentially probe very early. What do you think?
> 
> I'm pretty neutral on this one :) Hasn't the probe deferral mechanism
> been introduced to avoid having to create ordering through the initcall
> stages? But I agree, having the probe deferral notices is not particularly
> pretty. So, I'd definitely not oppose changing this.
> Though, there is one dependency on the SLCR regmap, but that is initialized
> fairly early.

Any comment on this one?

Thanks,
Michal

[PATCH] pinctrl-zynq: Initialize early

[Linus Walleij]

On Thu, Oct 22, 2015 at 1:30 PM, Mike Looijmans <mike.looijmans@topic.nl> wrote:

> Supplying pinmux configuration for e.g. gpio pins leads to deferred
> probes because the pinctrl device is probed much later than gpio.
> Move the init call to a much earlier stage so it probes before the
> devices that may need it.
>
> Signed-off-by: Mike Looijmans <mike.looijmans@topic.nl>

Patch applied with S?ren's Tested-by tag.

I am liberal with initcall reordering because I think the deferred
probe mechanism is not really working.

Take some time to review Rafael Wysocki's latest device link
concept that can get initcall reordering closer to the device core.

Yours,
Linus Walleij