[PATCH] pinctrl: tegra1x4: initialize at arch_initcall time

[PATCH] pinctrl: tegra1x4: initialize at arch_initcall time

[Andrew Bresticker]

Many devices rely on pinctrl/pinmux settings being applied
before probing and some of these may probe before device_initcall
time (e.g. i2c at subsys_initcall).  Move Tegra1x4 pinctrl driver
registration to arch_initcall time so that proper pin settings
can be applied earlier.

Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
---
 drivers/pinctrl/pinctrl-tegra114.c | 13 ++++++++++++-
 drivers/pinctrl/pinctrl-tegra124.c | 13 ++++++++++++-
 2 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-tegra114.c b/drivers/pinctrl/pinctrl-tegra114.c
index 93c9e38..5d0c950 100644
--- a/drivers/pinctrl/pinctrl-tegra114.c
+++ b/drivers/pinctrl/pinctrl-tegra114.c
@@ -2761,7 +2761,18 @@ static struct platform_driver tegra114_pinctrl_driver = {
 	.probe = tegra114_pinctrl_probe,
 	.remove = tegra_pinctrl_remove,
 };
-module_platform_driver(tegra114_pinctrl_driver);
+
+static int __init tegra114_pinctrl_init(void)
+{
+	return platform_driver_register(&tegra114_pinctrl_driver);
+}
+arch_initcall(tegra114_pinctrl_init);
+
+static void __exit tegra114_pinctrl_exit(void)
+{
+	platform_driver_unregister(&tegra114_pinctrl_driver);
+}
+module_exit(tegra114_pinctrl_exit);
 
 MODULE_AUTHOR("Pritesh Raithatha <praithatha@nvidia.com>");
 MODULE_DESCRIPTION("NVIDIA Tegra114 pinctrl driver");
diff --git a/drivers/pinctrl/pinctrl-tegra124.c b/drivers/pinctrl/pinctrl-tegra124.c
index c20e0e1..02ac976 100644
--- a/drivers/pinctrl/pinctrl-tegra124.c
+++ b/drivers/pinctrl/pinctrl-tegra124.c
@@ -3130,7 +3130,18 @@ static struct platform_driver tegra124_pinctrl_driver = {
 	.probe = tegra124_pinctrl_probe,
 	.remove = tegra_pinctrl_remove,
 };
-module_platform_driver(tegra124_pinctrl_driver);
+
+static int __init tegra124_pinctrl_init(void)
+{
+	return platform_driver_register(&tegra124_pinctrl_driver);
+}
+arch_initcall(tegra124_pinctrl_init);
+
+static void __exit tegra124_pinctrl_exit(void)
+{
+	platform_driver_unregister(&tegra124_pinctrl_driver);
+}
+module_exit(tegra124_pinctrl_exit);
 
 MODULE_AUTHOR("Ashwini Ghuge <aghuge@nvidia.com>");
 MODULE_DESCRIPTION("NVIDIA Tegra124 pinctrl driver");
-- 
1.8.5.1

Re: [PATCH] pinctrl: tegra1x4: initialize at arch_initcall time

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1152 bytes --]

On Fri, Jan 10, 2014 at 04:50:16PM -0800, Andrew Bresticker wrote:
> Many devices rely on pinctrl/pinmux settings being applied
> before probing and some of these may probe before device_initcall
> time (e.g. i2c at subsys_initcall).  Move Tegra1x4 pinctrl driver
> registration to arch_initcall time so that proper pin settings
> can be applied earlier.
> 
> Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> ---
>  drivers/pinctrl/pinctrl-tegra114.c | 13 ++++++++++++-
>  drivers/pinctrl/pinctrl-tegra124.c | 13 ++++++++++++-
>  2 files changed, 24 insertions(+), 2 deletions(-)

I wonder if a better approach would be to add pin control and pin mux
settings for devices that explicitly need them to the device tree nodes
of the devices themselves. That way they should be able to use deferred
probing to handle this kind of dependency rather than relying on magic
initcall ordering.

This does make the Tegra114 and Tegra124 drivers consistent with what we
have on Tegra20 an Tegra30, though, so perhaps pinctrl is one of the few
exceptions where initcall ordering would be okay.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] pinctrl: tegra1x4: initialize at arch_initcall time

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 1125 bytes --]

On Fri, Jan 10, 2014 at 04:50:16PM -0800, Andrew Bresticker wrote:
> Many devices rely on pinctrl/pinmux settings being applied
> before probing and some of these may probe before device_initcall
> time (e.g. i2c at subsys_initcall).  Move Tegra1x4 pinctrl driver
> registration to arch_initcall time so that proper pin settings
> can be applied earlier.
> 
> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
> ---
>  drivers/pinctrl/pinctrl-tegra114.c | 13 ++++++++++++-
>  drivers/pinctrl/pinctrl-tegra124.c | 13 ++++++++++++-
>  2 files changed, 24 insertions(+), 2 deletions(-)

I wonder if a better approach would be to add pin control and pin mux
settings for devices that explicitly need them to the device tree nodes
of the devices themselves. That way they should be able to use deferred
probing to handle this kind of dependency rather than relying on magic
initcall ordering.

This does make the Tegra114 and Tegra124 drivers consistent with what we
have on Tegra20 an Tegra30, though, so perhaps pinctrl is one of the few
exceptions where initcall ordering would be okay.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] pinctrl: tegra1x4: initialize at arch_initcall time

[Stephen Warren]

On 01/10/2014 05:50 PM, Andrew Bresticker wrote:
> Many devices rely on pinctrl/pinmux settings being applied
> before probing and some of these may probe before device_initcall
> time (e.g. i2c at subsys_initcall).  Move Tegra1x4 pinctrl driver
> registration to arch_initcall time so that proper pin settings
> can be applied earlier.

NAK. We shouldn't play games with initcall levels to achieve probe
ordering. It's not scalable and there's no actual guarantee it'll
produce the desired result.

Instead, if one device depends on another, it should do so explicitly.
In other words, the device that relies on a particular pinctrl setting
should have a pinctrl state defined, that references the pin controller,
which then triggers deferred probe until the pin controller is initialized.

Since we initialize the whole pinctrl setup in one go in a state
associated with the pin controller, rather than as separate states
associated with each device, you'll need to have the leaf devices
reference an empty pinctrl state defined in the desired pin controller.

Re: [PATCH] pinctrl: tegra1x4: initialize at arch_initcall time

[Stephen Warren]

On 01/13/2014 02:02 AM, Thierry Reding wrote:
> On Fri, Jan 10, 2014 at 04:50:16PM -0800, Andrew Bresticker wrote:
>> Many devices rely on pinctrl/pinmux settings being applied
>> before probing and some of these may probe before device_initcall
>> time (e.g. i2c at subsys_initcall).  Move Tegra1x4 pinctrl driver
>> registration to arch_initcall time so that proper pin settings
>> can be applied earlier.
>>
>> Signed-off-by: Andrew Bresticker <abrestic-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> ---
>>  drivers/pinctrl/pinctrl-tegra114.c | 13 ++++++++++++-
>>  drivers/pinctrl/pinctrl-tegra124.c | 13 ++++++++++++-
>>  2 files changed, 24 insertions(+), 2 deletions(-)
> 
> I wonder if a better approach would be to add pin control and pin mux
> settings for devices that explicitly need them to the device tree nodes
> of the devices themselves.

We can't do that; the entire pinctrl state needs to be programmed at
once to avoid the conflict of multiple pins/groups being muxed to the
same function. However, we can make this work with empty states I believe.

Re: [PATCH] pinctrl: tegra1x4: initialize at arch_initcall time

[Stephen Warren]

On 01/13/2014 02:02 AM, Thierry Reding wrote:
> On Fri, Jan 10, 2014 at 04:50:16PM -0800, Andrew Bresticker wrote:
>> Many devices rely on pinctrl/pinmux settings being applied
>> before probing and some of these may probe before device_initcall
>> time (e.g. i2c at subsys_initcall).  Move Tegra1x4 pinctrl driver
>> registration to arch_initcall time so that proper pin settings
>> can be applied earlier.
>>
>> Signed-off-by: Andrew Bresticker <abrestic@chromium.org>
>> ---
>>  drivers/pinctrl/pinctrl-tegra114.c | 13 ++++++++++++-
>>  drivers/pinctrl/pinctrl-tegra124.c | 13 ++++++++++++-
>>  2 files changed, 24 insertions(+), 2 deletions(-)
> 
> I wonder if a better approach would be to add pin control and pin mux
> settings for devices that explicitly need them to the device tree nodes
> of the devices themselves.

We can't do that; the entire pinctrl state needs to be programmed at
once to avoid the conflict of multiple pins/groups being muxed to the
same function. However, we can make this work with empty states I believe.