[PATCH] pinctrl: tegra: add suspend/resume support

[PATCH] pinctrl: tegra: add suspend/resume support

[Dmitry Osipenko]

Added suspend/resume pm ops. We need to store current regs vals on suspend and
restore them on resume.

Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Tested on my tablet.

 drivers/pinctrl/pinctrl-tegra.c | 94 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 91 insertions(+), 3 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-tegra.c b/drivers/pinctrl/pinctrl-tegra.c
index 7da0b37..03a3afb 100644
--- a/drivers/pinctrl/pinctrl-tegra.c
+++ b/drivers/pinctrl/pinctrl-tegra.c
@@ -41,6 +41,11 @@ struct tegra_pmx {
 
 	int nbanks;
 	void __iomem **regs;
+
+	int *bank_size;
+#ifdef CONFIG_PM_SLEEP
+	u32 *regs_store;
+#endif
 };
 
 static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
@@ -685,11 +690,72 @@ static struct pinctrl_desc tegra_pinctrl_desc = {
 	.owner = THIS_MODULE,
 };
 
+#ifdef CONFIG_PM_SLEEP
+static int tegra_pinctrl_suspend_noirq(struct device *dev)
+{
+	struct tegra_pmx *pmx = dev_get_drvdata(dev);
+	int store_offset = 0;
+	int i, reg;
+	u32 val;
+
+	if (!pmx)
+		return -ENOMEM;
+
+	for (i = 0; i < pmx->nbanks; i++) {
+
+		for (reg = 0; reg < pmx->bank_size[i]; reg += 4) {
+			val = pmx_readl(pmx, i, reg);
+			pmx->regs_store[store_offset] = val;
+			store_offset++;
+
+			dev_dbg(dev, "stored val: 0x%x bank: %d reg: 0x%x\n",
+				val, i, reg);
+		}
+	}
+
+	return 0;
+}
+
+static int tegra_pinctrl_resume_noirq(struct device *dev)
+{
+	struct tegra_pmx *pmx = dev_get_drvdata(dev);
+	int store_offset = 0;
+	int i, reg;
+	u32 val;
+
+	if (!pmx)
+		return -ENOMEM;
+
+	for (i = 0; i < pmx->nbanks; i++) {
+
+		for (reg = 0; reg < pmx->bank_size[i]; reg += 4) {
+			val = pmx->regs_store[store_offset];
+			pmx_writel(pmx, val, i, reg);
+			store_offset++;
+
+			dev_dbg(dev, "restored val: 0x%x bank: %d reg: 0x%x\n",
+				val, i, reg);
+		}
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops tegra_pinctrl_pm_ops = {
+	.suspend_noirq = tegra_pinctrl_suspend_noirq,
+	.resume_noirq = tegra_pinctrl_resume_noirq,
+};
+#define TEGRA_PINCTRL_PM	(&tegra_pinctrl_pm_ops)
+#else
+#define TEGRA_PINCTRL_PM	NULL
+#endif
+
 int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 			const struct tegra_pinctrl_soc_data *soc_data)
 {
 	struct tegra_pmx *pmx;
 	struct resource *res;
+	int nregs = 0;
 	int i;
 
 	pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
@@ -712,6 +778,13 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 	}
 	pmx->nbanks = i;
 
+	pmx->bank_size = devm_kzalloc(&pdev->dev, pmx->nbanks * sizeof(int),
+				      GFP_KERNEL);
+	if (!pmx->bank_size) {
+		dev_err(&pdev->dev, "Can't alloc banks sizes pointer\n");
+		return -ENODEV;
+	}
+
 	pmx->regs = devm_kzalloc(&pdev->dev, pmx->nbanks * sizeof(*pmx->regs),
 				 GFP_KERNEL);
 	if (!pmx->regs) {
@@ -726,22 +799,35 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 			return -ENODEV;
 		}
 
+		pmx->bank_size[i] = resource_size(res);
+
 		if (!devm_request_mem_region(&pdev->dev, res->start,
-					    resource_size(res),
-					    dev_name(&pdev->dev))) {
+					     pmx->bank_size[i],
+					     dev_name(&pdev->dev))) {
 			dev_err(&pdev->dev,
 				"Couldn't request MEM resource %d\n", i);
 			return -ENODEV;
 		}
 
 		pmx->regs[i] = devm_ioremap(&pdev->dev, res->start,
-					    resource_size(res));
+					    pmx->bank_size[i]);
 		if (!pmx->regs[i]) {
 			dev_err(&pdev->dev, "Couldn't ioremap regs %d\n", i);
 			return -ENODEV;
 		}
+
+		nregs += pmx->bank_size[i] / 4;
 	}
 
+#ifdef CONFIG_PM_SLEEP
+	pmx->regs_store = devm_kzalloc(&pdev->dev, nregs * sizeof(u32),
+				       GFP_KERNEL);
+	if (!pmx->regs_store) {
+		dev_err(&pdev->dev, "Can't alloc regs store pointer\n");
+		return -ENODEV;
+	}
+#endif
+
 	pmx->pctl = pinctrl_register(&tegra_pinctrl_desc, &pdev->dev, pmx);
 	if (!pmx->pctl) {
 		dev_err(&pdev->dev, "Couldn't register pinctrl driver\n");
@@ -752,6 +838,8 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 
 	platform_set_drvdata(pdev, pmx);
 
+	pdev->dev.driver->pm = TEGRA_PINCTRL_PM;
+
 	dev_dbg(&pdev->dev, "Probed Tegra pinctrl driver\n");
 
 	return 0;
-- 
1.7.12

Re: [PATCH] pinctrl: tegra: add suspend/resume support

[Stephen Warren]

On 11/01/2012 09:53 AM, Dmitry Osipenko wrote:
> Added suspend/resume pm ops. We need to store current regs vals on suspend and
> restore them on resume.

Interesting. Just literally a couple days ago, I was reviewing a patch
to our internal kernel tree that implemented the same thing for the
pinctrl driver, in almost the same way!

> ---
> Tested on my tablet.

I'm curious how; in what environment. As far as I know, the Tegra
support in the mainline kernel doesn't actually support suspend/resume.
I assume you cherry-picked this pinctrl driver into some other kernel,
and tested this patch there?

> +#ifdef CONFIG_PM_SLEEP
> +static int tegra_pinctrl_suspend_noirq(struct device *dev)

The one major different between this patch and the downstream patch I
reviewed is how suspend/resume is triggered. This uses suspend_noirq,
whereas the downstream patch registers the callbacks using
register_syscore_ops(). Apparently the latter is required (at least in
our downstream kernel) in order to ensure that pinctrl gets suspended
after all other drivers.

I Cc'd Pritesh to comment on this.

Still, perhaps device probe ordering should ensure this upstream so
using register_syscore_ops() might not be necessary, although that
relies on drivers probing in the correct order, which they may not
without explicitly pinctrl_get() calls... back to that same problem again!

...
> +static const struct dev_pm_ops tegra_pinctrl_pm_ops = {
> +	.suspend_noirq = tegra_pinctrl_suspend_noirq,
> +	.resume_noirq = tegra_pinctrl_resume_noirq,
> +};
> +#define TEGRA_PINCTRL_PM	(&tegra_pinctrl_pm_ops)
> +#else
> +#define TEGRA_PINCTRL_PM	NULL
> +#endif

I was going to suggest using something like SET_SYSTEM_SLEEP_PM_OPS, but
I guess there's no equivalent for the _noirq variants.

> @@ -752,6 +838,8 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
>  
>  	platform_set_drvdata(pdev, pmx);
>  
> +	pdev->dev.driver->pm = TEGRA_PINCTRL_PM;

That might be better done in the struct platform_driver initialization
in pinctrl-tegra*.c?

Re: [PATCH] pinctrl: tegra: add suspend/resume support

[Dmitry Osipenko]

On 01.11.2012 21:23, Stephen Warren wrote:
> I'm curious how; in what environment. As far as I know, the Tegra
> support in the mainline kernel doesn't actually support suspend/resume.
> I assume you cherry-picked this pinctrl driver into some other kernel,
> and tested this patch there?

I added support for suspend/resume since 3.5-rc1, when actually started
updating kernel to mainline. The only differences from mainline 3.7 kernel now
are: custom clk framework from downstream's kernel with some modifications,
dvfs support, video, wifi and other specific drivers for my tablet. I will
continue posting patches in order to support suspending in mainline kernel.
Also will try to implement dvfs with common clk framework... have some thoughts
how better realise it.

> The one major different between this patch and the downstream patch I
> reviewed is how suspend/resume is triggered. This uses suspend_noirq,
> whereas the downstream patch registers the callbacks using
> register_syscore_ops(). Apparently the latter is required (at least in
> our downstream kernel) in order to ensure that pinctrl gets suspended
> after all other drivers.
> 
> I Cc'd Pritesh to comment on this.
> 
> Still, perhaps device probe ordering should ensure this upstream so
> using register_syscore_ops() might not be necessary, although that
> relies on drivers probing in the correct order, which they may not
> without explicitly pinctrl_get() calls... back to that same problem again!

I know about that difference. My first realisation used syscore_ops, but then
I thought that possibly may be more than one pinctrl device that uses tegra
driver and decided to use _noirq pm ops. From your msg I assume that we can
have only one device and besides current realisation needs some changes to
support more than one device. So should I send V2 or my patch will be useless
because Pritesh already realised it?

Re: [PATCH] pinctrl: tegra: add suspend/resume support

[Stephen Warren]

On 11/01/2012 02:08 PM, Dmitry Osipenko wrote:
> On 01.11.2012 21:23, Stephen Warren wrote:
>> I'm curious how; in what environment. As far as I know, the Tegra
>> support in the mainline kernel doesn't actually support suspend/resume.
>> I assume you cherry-picked this pinctrl driver into some other kernel,
>> and tested this patch there?
> 
> I added support for suspend/resume since 3.5-rc1, when actually started
> updating kernel to mainline. The only differences from mainline 3.7 kernel now
> are: custom clk framework from downstream's kernel with some modifications,
> dvfs support, video, wifi and other specific drivers for my tablet. I will
> continue posting patches in order to support suspending in mainline kernel.
> Also will try to implement dvfs with common clk framework... have some thoughts
> how better realise it.

OK, it sounds like you'll make some very useful contributions. Thanks in
advance.

>> The one major different between this patch and the downstream patch I
>> reviewed is how suspend/resume is triggered. This uses suspend_noirq,
>> whereas the downstream patch registers the callbacks using
>> register_syscore_ops(). Apparently the latter is required (at least in
>> our downstream kernel) in order to ensure that pinctrl gets suspended
>> after all other drivers.
>>
>> I Cc'd Pritesh to comment on this.
>>
>> Still, perhaps device probe ordering should ensure this upstream so
>> using register_syscore_ops() might not be necessary, although that
>> relies on drivers probing in the correct order, which they may not
>> without explicitly pinctrl_get() calls... back to that same problem again!
> 
> I know about that difference. My first realisation used syscore_ops, but then
> I thought that possibly may be more than one pinctrl device that uses tegra
> driver and decided to use _noirq pm ops. From your msg I assume that we can
> have only one device

Yes, there certainly is only one pinmux device on Tegra. I agree it's a
little icky to have to use global variables for syscore_ops, but I don't
think it will actually cause any practical issue.

> and besides current realisation needs some changes to
> support more than one device. So should I send V2 or my patch will be useless
> because Pritesh already realised it?

I'd suggest that Pritesh posts his patch too, and you can both look at
each-other's work and come up with a final solution.

By the way, your mailer created a rather odd header:

> In-Reply-To: <5092B007.7050609-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>

That makes sense, but:

> Reply-To: 5092B007.7050609-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org

That looks like a botched In-Reply-To header. That text certainly isn't
a valid email address, but rather a message ID.

[PATCH V2] pinctrl: tegra: add suspend/resume support

[Dmitry Osipenko]

Added suspend/resume support using pm ops. We need to store current regs vals on
suspend and restore them on resume. Platform driver registering function moved to
core init level to ensure that device driver will be in the end of suspend and in
the beginning of resume lists.

Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
I pondered a moment with suspend/resume level bit more and decided that there is
no real necessity in syscore_ops. If registering function left on arch init level,
then i2c devices will be after pinctrl on suspend. Also cleaned up code a bit based
on Pritesh's patch.

Here is suspend/resume sequence from my dmesg log with this patch applied:

.....
<7>[  399.651179] tegra-i2c tegra-i2c.0: noirq driver suspend
<7>[  399.651627] tegra20-pinctrl tegra20-pinctrl: noirq driver suspend
<7>[  399.652482] power power.0: noirq driver suspend
<6>[  399.652951] PM: noirq suspend of devices complete after 23.677 msecs
<4>[  399.653694] Disabling non-boot CPUs ...
<5>[  399.661492] CPU1: shutdown
<6>[  399.668317] PM: Calling timekeeping_suspend+0x0/0x190
<6>[  399.669282] PM: Calling suspend_time_syscore_suspend+0x0/0x28
<6>[  399.669831] PM: Calling sched_clock_suspend+0x0/0x3c
<6>[  399.670697] PM: Calling fw_suspend+0x0/0x2c
<6>[  399.671135] PM: Calling tegra_pm_irq_syscore_suspend+0x0/0x11c
<6>[  399.671963] Wake[31-0] level=0x880192
<6>[  399.672693] Wake[31-0] enable=0x800c0180
<6>[  399.673156] PM: Calling tegra_legacy_irq_suspend+0x0/0xe0
<6>[  399.673961] PM: Calling cpufreq_bp_suspend+0x0/0x7c
<6>[  399.674441] PM: Calling cpu_pm_suspend+0x0/0x28
<6>[  399.674929] PM: Calling tegra_timer_suspend+0x0/0x4c
<6>[  399.675648] PM: Calling tegra_clk_suspend+0x0/0x2ec
<6>[  399.675648] PM: Calling tegra_gpio_suspend+0x0/0x124
<6>[  399.675648] PM: Calling tegra_gpio_resume+0x0/0x11c
<6>[  399.675648] PM: Calling tegra_clk_resume+0x0/0x3f8
<6>[  399.675648] PM: Calling tegra_timer_resume+0x0/0x48
<6>[  399.675675] PM: Calling cpu_pm_resume+0x0/0x20
<6>[  399.675978] PM: Calling cpufreq_bp_resume+0x0/0x70
<6>[  399.676531] PM: Calling tegra_legacy_irq_resume+0x0/0x134
<6>[  399.676895] PM: Calling tegra_pm_irq_syscore_resume+0x0/0x110
<6>[  399.677362]  legacy wake status=0x40000
<6>[  399.677628] Resume caused by WAKE18, tps6586x
<6>[  399.677937] PM: Calling sched_clock_resume+0x0/0x4c
<6>[  399.678441] PM: Calling suspend_time_syscore_resume+0x0/0xa0
<6>[  399.678710] Suspended for 5.870 seconds
<6>[  399.679185] PM: Calling timekeeping_resume+0x0/0x130
<6>[  399.679532] PM: Calling irq_pm_syscore_resume+0x0/0x20
<6>[  399.680301] Enabling non-boot CPUs ...
<4>[  399.693910] CPU1: Booted secondary processor
<6>[  399.695858] CPU1 is up
<7>[  399.696236] tegra20-pinctrl tegra20-pinctrl: noirq driver resume
<7>[  399.696932] tegra-i2c tegra-i2c.0: noirq driver resume
.....

 drivers/pinctrl/pinctrl-tegra.c   | 72 ++++++++++++++++++++++++++++++++++++---
 drivers/pinctrl/pinctrl-tegra.h   |  7 ++++
 drivers/pinctrl/pinctrl-tegra20.c |  3 +-
 drivers/pinctrl/pinctrl-tegra30.c |  3 +-
 4 files changed, 79 insertions(+), 6 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-tegra.c b/drivers/pinctrl/pinctrl-tegra.c
index 7da0b37..de3ba4f 100644
--- a/drivers/pinctrl/pinctrl-tegra.c
+++ b/drivers/pinctrl/pinctrl-tegra.c
@@ -41,6 +41,9 @@ struct tegra_pmx {
 
 	int nbanks;
 	void __iomem **regs;
+
+	int *bank_size;
+	u32 *regs_storage;
 };
 
 static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
@@ -685,12 +688,51 @@ static struct pinctrl_desc tegra_pinctrl_desc = {
 	.owner = THIS_MODULE,
 };
 
+#ifdef CONFIG_PM_SLEEP
+static int tegra_pinctrl_suspend_noirq(struct device *dev)
+{
+	struct tegra_pmx *pmx = dev_get_drvdata(dev);
+	u32 *regs_storage = pmx->regs_storage;
+	u32 *regs;
+	int i, j;
+
+	for (i = 0; i < pmx->nbanks; i++) {
+		regs = pmx->regs[i];
+		for (j = 0; j < pmx->bank_size[i] / 4; j++)
+			*regs_storage++ = readl(regs++);
+	}
+
+	return 0;
+}
+
+static int tegra_pinctrl_resume_noirq(struct device *dev)
+{
+	struct tegra_pmx *pmx = dev_get_drvdata(dev);
+	u32 *regs_storage = pmx->regs_storage;
+	u32 *regs;
+	int i, j;
+
+	for (i = 0; i < pmx->nbanks; i++) {
+		regs = pmx->regs[i];
+		for (j = 0; j < pmx->bank_size[i] / 4; j++)
+			writel(*regs_storage++, regs++);
+	}
+
+	return 0;
+}
+
+const struct dev_pm_ops tegra_pinctrl_pm_ops = {
+	.suspend_noirq = tegra_pinctrl_suspend_noirq,
+	.resume_noirq = tegra_pinctrl_resume_noirq,
+};
+#endif
+
 int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 			const struct tegra_pinctrl_soc_data *soc_data)
 {
 	struct tegra_pmx *pmx;
 	struct resource *res;
-	int i;
+	int i, regs_storage_sz = 0;
 
 	pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
 	if (!pmx) {
@@ -712,6 +754,13 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 	}
 	pmx->nbanks = i;
 
+	pmx->bank_size = devm_kzalloc(&pdev->dev, pmx->nbanks * sizeof(int),
+				      GFP_KERNEL);
+	if (!pmx->bank_size) {
+		dev_err(&pdev->dev, "Can't alloc banks sizes pointer\n");
+		return -ENODEV;
+	}
+
 	pmx->regs = devm_kzalloc(&pdev->dev, pmx->nbanks * sizeof(*pmx->regs),
 				 GFP_KERNEL);
 	if (!pmx->regs) {
@@ -726,22 +775,37 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 			return -ENODEV;
 		}
 
+		pmx->bank_size[i] = resource_size(res);
+
 		if (!devm_request_mem_region(&pdev->dev, res->start,
-					    resource_size(res),
-					    dev_name(&pdev->dev))) {
+					     pmx->bank_size[i],
+					     dev_name(&pdev->dev))) {
 			dev_err(&pdev->dev,
 				"Couldn't request MEM resource %d\n", i);
 			return -ENODEV;
 		}
 
 		pmx->regs[i] = devm_ioremap(&pdev->dev, res->start,
-					    resource_size(res));
+					    pmx->bank_size[i]);
 		if (!pmx->regs[i]) {
 			dev_err(&pdev->dev, "Couldn't ioremap regs %d\n", i);
 			return -ENODEV;
 		}
+
+		regs_storage_sz += pmx->bank_size[i];
 	}
 
+#ifdef CONFIG_PM_SLEEP
+	pmx->regs_storage = devm_kzalloc(&pdev->dev, regs_storage_sz,
+					 GFP_KERNEL);
+	if (!pmx->regs_storage) {
+		dev_err(&pdev->dev, "Can't alloc regs storage pointer\n");
+		return -ENODEV;
+	}
+#else
+	devm_kfree(&pdev->dev, pmx->bank_size);
+#endif
+
 	pmx->pctl = pinctrl_register(&tegra_pinctrl_desc, &pdev->dev, pmx);
 	if (!pmx->pctl) {
 		dev_err(&pdev->dev, "Couldn't register pinctrl driver\n");
diff --git a/drivers/pinctrl/pinctrl-tegra.h b/drivers/pinctrl/pinctrl-tegra.h
index 62e3809..bbe27cd 100644
--- a/drivers/pinctrl/pinctrl-tegra.h
+++ b/drivers/pinctrl/pinctrl-tegra.h
@@ -187,4 +187,11 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
 			const struct tegra_pinctrl_soc_data *soc_data);
 int tegra_pinctrl_remove(struct platform_device *pdev);
 
+#ifdef CONFIG_PM_SLEEP
+extern const struct dev_pm_ops tegra_pinctrl_pm_ops;
+#define TEGRA_PINCTRL_PM	(&tegra_pinctrl_pm_ops)
+#else
+#define TEGRA_PINCTRL_PM	NULL
+#endif
+
 #endif
diff --git a/drivers/pinctrl/pinctrl-tegra20.c b/drivers/pinctrl/pinctrl-tegra20.c
index a74f9a5..6f09023 100644
--- a/drivers/pinctrl/pinctrl-tegra20.c
+++ b/drivers/pinctrl/pinctrl-tegra20.c
@@ -2871,6 +2871,7 @@ static struct platform_driver tegra20_pinctrl_driver = {
 		.name = "tegra20-pinctrl",
 		.owner = THIS_MODULE,
 		.of_match_table = tegra20_pinctrl_of_match,
+		.pm = TEGRA_PINCTRL_PM,
 	},
 	.probe = tegra20_pinctrl_probe,
 	.remove = __devexit_p(tegra_pinctrl_remove),
@@ -2880,7 +2881,7 @@ static int __init tegra20_pinctrl_init(void)
 {
 	return platform_driver_register(&tegra20_pinctrl_driver);
 }
-arch_initcall(tegra20_pinctrl_init);
+core_initcall(tegra20_pinctrl_init);
 
 static void __exit tegra20_pinctrl_exit(void)
 {
diff --git a/drivers/pinctrl/pinctrl-tegra30.c b/drivers/pinctrl/pinctrl-tegra30.c
index 7894f14..2e90632 100644
--- a/drivers/pinctrl/pinctrl-tegra30.c
+++ b/drivers/pinctrl/pinctrl-tegra30.c
@@ -3737,6 +3737,7 @@ static struct platform_driver tegra30_pinctrl_driver = {
 		.name = "tegra30-pinctrl",
 		.owner = THIS_MODULE,
 		.of_match_table = tegra30_pinctrl_of_match,
+		.pm = TEGRA_PINCTRL_PM,
 	},
 	.probe = tegra30_pinctrl_probe,
 	.remove = __devexit_p(tegra_pinctrl_remove),
@@ -3746,7 +3747,7 @@ static int __init tegra30_pinctrl_init(void)
 {
 	return platform_driver_register(&tegra30_pinctrl_driver);
 }
-arch_initcall(tegra30_pinctrl_init);
+core_initcall(tegra30_pinctrl_init);
 
 static void __exit tegra30_pinctrl_exit(void)
 {
-- 
1.7.12

Re: [PATCH V2] pinctrl: tegra: add suspend/resume support

[Pritesh Raithatha]

On Saturday 03 November 2012 06:00 AM, Dmitry Osipenko wrote:
> Added suspend/resume support using pm ops. We need to store current regs vals on
> suspend and restore them on resume. Platform driver registering function moved to
> core init level to ensure that device driver will be in the end of suspend and in
> the beginning of resume lists.
> 
> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> I pondered a moment with suspend/resume level bit more and decided that there is
> no real necessity in syscore_ops. If registering function left on arch init level,
> then i2c devices will be after pinctrl on suspend. Also cleaned up code a bit based
> on Pritesh's patch.
> 
> Here is suspend/resume sequence from my dmesg log with this patch applied:
> 
> .....
> <7>[  399.651179] tegra-i2c tegra-i2c.0: noirq driver suspend
> <7>[  399.651627] tegra20-pinctrl tegra20-pinctrl: noirq driver suspend
> <7>[  399.652482] power power.0: noirq driver suspend
> <6>[  399.652951] PM: noirq suspend of devices complete after 23.677 msecs
> <4>[  399.653694] Disabling non-boot CPUs ...
> <5>[  399.661492] CPU1: shutdown
> <6>[  399.668317] PM: Calling timekeeping_suspend+0x0/0x190
> <6>[  399.669282] PM: Calling suspend_time_syscore_suspend+0x0/0x28
> <6>[  399.669831] PM: Calling sched_clock_suspend+0x0/0x3c
> <6>[  399.670697] PM: Calling fw_suspend+0x0/0x2c
> <6>[  399.671135] PM: Calling tegra_pm_irq_syscore_suspend+0x0/0x11c
> <6>[  399.671963] Wake[31-0] level=0x880192
> <6>[  399.672693] Wake[31-0] enable=0x800c0180
> <6>[  399.673156] PM: Calling tegra_legacy_irq_suspend+0x0/0xe0
> <6>[  399.673961] PM: Calling cpufreq_bp_suspend+0x0/0x7c
> <6>[  399.674441] PM: Calling cpu_pm_suspend+0x0/0x28
> <6>[  399.674929] PM: Calling tegra_timer_suspend+0x0/0x4c
> <6>[  399.675648] PM: Calling tegra_clk_suspend+0x0/0x2ec
> <6>[  399.675648] PM: Calling tegra_gpio_suspend+0x0/0x124
> <6>[  399.675648] PM: Calling tegra_gpio_resume+0x0/0x11c
> <6>[  399.675648] PM: Calling tegra_clk_resume+0x0/0x3f8
> <6>[  399.675648] PM: Calling tegra_timer_resume+0x0/0x48
> <6>[  399.675675] PM: Calling cpu_pm_resume+0x0/0x20
> <6>[  399.675978] PM: Calling cpufreq_bp_resume+0x0/0x70
> <6>[  399.676531] PM: Calling tegra_legacy_irq_resume+0x0/0x134
> <6>[  399.676895] PM: Calling tegra_pm_irq_syscore_resume+0x0/0x110
> <6>[  399.677362]  legacy wake status=0x40000
> <6>[  399.677628] Resume caused by WAKE18, tps6586x
> <6>[  399.677937] PM: Calling sched_clock_resume+0x0/0x4c
> <6>[  399.678441] PM: Calling suspend_time_syscore_resume+0x0/0xa0
> <6>[  399.678710] Suspended for 5.870 seconds
> <6>[  399.679185] PM: Calling timekeeping_resume+0x0/0x130
> <6>[  399.679532] PM: Calling irq_pm_syscore_resume+0x0/0x20
> <6>[  399.680301] Enabling non-boot CPUs ...
> <4>[  399.693910] CPU1: Booted secondary processor
> <6>[  399.695858] CPU1 is up
> <7>[  399.696236] tegra20-pinctrl tegra20-pinctrl: noirq driver resume
> <7>[  399.696932] tegra-i2c tegra-i2c.0: noirq driver resume
> .....
> 
>  drivers/pinctrl/pinctrl-tegra.c   | 72 ++++++++++++++++++++++++++++++++++++---
>  drivers/pinctrl/pinctrl-tegra.h   |  7 ++++
>  drivers/pinctrl/pinctrl-tegra20.c |  3 +-
>  drivers/pinctrl/pinctrl-tegra30.c |  3 +-
>  4 files changed, 79 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pinctrl/pinctrl-tegra.c b/drivers/pinctrl/pinctrl-tegra.c
> index 7da0b37..de3ba4f 100644
> --- a/drivers/pinctrl/pinctrl-tegra.c
> +++ b/drivers/pinctrl/pinctrl-tegra.c
> @@ -41,6 +41,9 @@ struct tegra_pmx {
>  
>  	int nbanks;
>  	void __iomem **regs;
> +
> +	int *bank_size;
> +	u32 *regs_storage;
>  };
>  
>  static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
> @@ -685,12 +688,51 @@ static struct pinctrl_desc tegra_pinctrl_desc = {
>  	.owner = THIS_MODULE,
>  };
>  
> +#ifdef CONFIG_PM_SLEEP
> +static int tegra_pinctrl_suspend_noirq(struct device *dev)
> +{
> +	struct tegra_pmx *pmx = dev_get_drvdata(dev);
> +	u32 *regs_storage = pmx->regs_storage;
> +	u32 *regs;
> +	int i, j;
> +
> +	for (i = 0; i < pmx->nbanks; i++) {
> +		regs = pmx->regs[i];
> +		for (j = 0; j < pmx->bank_size[i] / 4; j++)
> +			*regs_storage++ = readl(regs++);
> +	}
> +
> +	return 0;
> +}
> +
> +static int tegra_pinctrl_resume_noirq(struct device *dev)
> +{
> +	struct tegra_pmx *pmx = dev_get_drvdata(dev);
> +	u32 *regs_storage = pmx->regs_storage;
> +	u32 *regs;
> +	int i, j;
> +
> +	for (i = 0; i < pmx->nbanks; i++) {
> +		regs = pmx->regs[i];
> +		for (j = 0; j < pmx->bank_size[i] / 4; j++)
> +			writel(*regs_storage++, regs++);
> +	}
> +
> +	return 0;
> +}
> +
> +const struct dev_pm_ops tegra_pinctrl_pm_ops = {
> +	.suspend_noirq = tegra_pinctrl_suspend_noirq,
> +	.resume_noirq = tegra_pinctrl_resume_noirq,
> +};
> +#endif
> +
>  int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
>  			const struct tegra_pinctrl_soc_data *soc_data)
>  {
>  	struct tegra_pmx *pmx;
>  	struct resource *res;
> -	int i;
> +	int i, regs_storage_sz = 0;
>  
>  	pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
>  	if (!pmx) {
> @@ -712,6 +754,13 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
>  	}
>  	pmx->nbanks = i;
>  
> +	pmx->bank_size = devm_kzalloc(&pdev->dev, pmx->nbanks * sizeof(int),

Use sizeof(*(pmx->bank_size)) instead of sizeof(int) and enclose this
inside #ifdef CONFIG_PM_SLEEP

> +				      GFP_KERNEL);
> +	if (!pmx->bank_size) {
> +		dev_err(&pdev->dev, "Can't alloc banks sizes pointer\n");
> +		return -ENODEV;
> +	}
> +
>  	pmx->regs = devm_kzalloc(&pdev->dev, pmx->nbanks * sizeof(*pmx->regs),
>  				 GFP_KERNEL);
>  	if (!pmx->regs) {
> @@ -726,22 +775,37 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
>  			return -ENODEV;
>  		}
>  
> +		pmx->bank_size[i] = resource_size(res);
> +
>  		if (!devm_request_mem_region(&pdev->dev, res->start,
> -					    resource_size(res),
> -					    dev_name(&pdev->dev))) {
> +					     pmx->bank_size[i],
> +					     dev_name(&pdev->dev))) {
>  			dev_err(&pdev->dev,
>  				"Couldn't request MEM resource %d\n", i);
>  			return -ENODEV;
>  		}
>  
>  		pmx->regs[i] = devm_ioremap(&pdev->dev, res->start,
> -					    resource_size(res));
> +					    pmx->bank_size[i]);
>  		if (!pmx->regs[i]) {
>  			dev_err(&pdev->dev, "Couldn't ioremap regs %d\n", i);
>  			return -ENODEV;
>  		}
> +
> +		regs_storage_sz += pmx->bank_size[i];
>  	}
>  
> +#ifdef CONFIG_PM_SLEEP
> +	pmx->regs_storage = devm_kzalloc(&pdev->dev, regs_storage_sz,
> +					 GFP_KERNEL);
> +	if (!pmx->regs_storage) {
> +		dev_err(&pdev->dev, "Can't alloc regs storage pointer\n");
> +		return -ENODEV;
> +	}
> +#else
> +	devm_kfree(&pdev->dev, pmx->bank_size);
> +#endif
> +
>  	pmx->pctl = pinctrl_register(&tegra_pinctrl_desc, &pdev->dev, pmx);
>  	if (!pmx->pctl) {
>  		dev_err(&pdev->dev, "Couldn't register pinctrl driver\n");
> diff --git a/drivers/pinctrl/pinctrl-tegra.h b/drivers/pinctrl/pinctrl-tegra.h
> index 62e3809..bbe27cd 100644
> --- a/drivers/pinctrl/pinctrl-tegra.h
> +++ b/drivers/pinctrl/pinctrl-tegra.h
> @@ -187,4 +187,11 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
>  			const struct tegra_pinctrl_soc_data *soc_data);
>  int tegra_pinctrl_remove(struct platform_device *pdev);
>  
> +#ifdef CONFIG_PM_SLEEP
> +extern const struct dev_pm_ops tegra_pinctrl_pm_ops;
> +#define TEGRA_PINCTRL_PM	(&tegra_pinctrl_pm_ops)
> +#else
> +#define TEGRA_PINCTRL_PM	NULL
> +#endif
> +
>  #endif
> diff --git a/drivers/pinctrl/pinctrl-tegra20.c b/drivers/pinctrl/pinctrl-tegra20.c
> index a74f9a5..6f09023 100644
> --- a/drivers/pinctrl/pinctrl-tegra20.c
> +++ b/drivers/pinctrl/pinctrl-tegra20.c
> @@ -2871,6 +2871,7 @@ static struct platform_driver tegra20_pinctrl_driver = {
>  		.name = "tegra20-pinctrl",
>  		.owner = THIS_MODULE,
>  		.of_match_table = tegra20_pinctrl_of_match,
> +		.pm = TEGRA_PINCTRL_PM,
>  	},
>  	.probe = tegra20_pinctrl_probe,
>  	.remove = __devexit_p(tegra_pinctrl_remove),
> @@ -2880,7 +2881,7 @@ static int __init tegra20_pinctrl_init(void)
>  {
>  	return platform_driver_register(&tegra20_pinctrl_driver);
>  }
> -arch_initcall(tegra20_pinctrl_init);
> +core_initcall(tegra20_pinctrl_init);
>  
>  static void __exit tegra20_pinctrl_exit(void)
>  {
> diff --git a/drivers/pinctrl/pinctrl-tegra30.c b/drivers/pinctrl/pinctrl-tegra30.c
> index 7894f14..2e90632 100644
> --- a/drivers/pinctrl/pinctrl-tegra30.c
> +++ b/drivers/pinctrl/pinctrl-tegra30.c
> @@ -3737,6 +3737,7 @@ static struct platform_driver tegra30_pinctrl_driver = {
>  		.name = "tegra30-pinctrl",
>  		.owner = THIS_MODULE,
>  		.of_match_table = tegra30_pinctrl_of_match,
> +		.pm = TEGRA_PINCTRL_PM,
>  	},
>  	.probe = tegra30_pinctrl_probe,
>  	.remove = __devexit_p(tegra_pinctrl_remove),
> @@ -3746,7 +3747,7 @@ static int __init tegra30_pinctrl_init(void)
>  {
>  	return platform_driver_register(&tegra30_pinctrl_driver);
>  }
> -arch_initcall(tegra30_pinctrl_init);
> +core_initcall(tegra30_pinctrl_init);
>  
>  static void __exit tegra30_pinctrl_exit(void)
>  {

I would suggest to make tagra_pinctrl_probe function as follow:
(rest looks fine with me)

 int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 			const struct tegra_pinctrl_soc_data *soc_data)
 	{
 	struct tegra_pmx *pmx;
 	struct resource *res;
-	int i;
+	int i, bank_size, regs_storage_sz = 0;

 	pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
 	if (!pmx) {
@@ -711,6 +751,7 @@ int __devinit tegra_pinctrl_probe(struct
platform_device *pdev,
 		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
 		if (!res)
 			break;
+		regs_storage_sz += resource_size(res);
 	}
 	pmx->nbanks = i;

@@ -721,6 +762,23 @@ int __devinit tegra_pinctrl_probe(struct
platform_device *pdev,
 		return -ENODEV;
 	}

+#ifdef CONFIG_PM_SLEEP
+	pmx->bank_size = devm_kzalloc(&pdev->dev,
+					pmx->nbanks * sizeof(*(pmx->bank_size)),
+					GFP_KERNEL);
+	if (!pmx->bank_size) {
+		dev_err(&pdev->dev, "Can't alloc banks sizes pointer\n");
+		return -ENODEV;
+	}
+
+	pmx->regs_storage = devm_kzalloc(&pdev->dev, regs_storage_sz,
+					 GFP_KERNEL);
+	if (!pmx->regs_storage) {
+		dev_err(&pdev->dev, "Can't alloc regs storage pointer\n");
+		return -ENODEV;
+	}
+#endif
+
 	for (i = 0; i < pmx->nbanks; i++) {
 		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
 		if (!res) {
@@ -728,20 +786,26 @@ int __devinit tegra_pinctrl_probe(struct
platform_device *pdev,
 			return -ENODEV;
 		}

+		bank_size = resource_size(res);
+
 		if (!devm_request_mem_region(&pdev->dev, res->start,
-					    resource_size(res),
-					    dev_name(&pdev->dev))) {
+					    	bank_size,
+					    	dev_name(&pdev->dev))) {
 			dev_err(&pdev->dev,
 				"Couldn't request MEM resource %d\n", i);
 			return -ENODEV;
 		}

 		pmx->regs[i] = devm_ioremap(&pdev->dev, res->start,
-					    resource_size(res));
+					    	bank_size);
 		if (!pmx->regs[i]) {
 			dev_err(&pdev->dev, "Couldn't ioremap regs %d\n", i);
 			return -ENODEV;
 		}
+
+#ifdef CONFIG_PM_SLEEP
+		pmx->bank_size[i] = bank_size;
+#endif
 	}

 	pmx->pctl = pinctrl_register(&tegra_pinctrl_desc, &pdev->dev, pmx);

Re: [PATCH V2] pinctrl: tegra: add suspend/resume support

[Stephen Warren]

On 11/02/2012 06:30 PM, Dmitry Osipenko wrote:
> Added suspend/resume support using pm ops. We need to store current regs vals on
> suspend and restore them on resume. Platform driver registering function moved to
> core init level to ensure that device driver will be in the end of suspend and in
> the beginning of resume lists.

Thanks for this. I'll let you and Pritesh sort out a final version and
then take a look at that.

BTW, can you please post a link to the source for your downstream kernel
that this came from - I'd be very interested in seeing what work has
been done there. Thanks.

[PATCH V3] pinctrl: tegra: add suspend/resume support

[Dmitry Osipenko]

Added suspend/resume support using pm ops. We need to store current regs vals on
suspend and restore them on resume. Platform driver registering function moved to
core init level to ensure that device driver will be in the end of suspend and in
the beginning of resume lists.

Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Pritesh, what do you think about this patch?

> BTW, can you please post a link to the source for your downstream kernel
> that this came from - I'd be very interested in seeing what work has
> been done there. Thanks.

I have sent invitation to you on bitbucket.

 drivers/pinctrl/pinctrl-tegra.c   | 98 ++++++++++++++++++++++++++++++++++-----
 drivers/pinctrl/pinctrl-tegra.h   |  7 +++
 drivers/pinctrl/pinctrl-tegra20.c |  3 +-
 drivers/pinctrl/pinctrl-tegra30.c |  3 +-
 4 files changed, 98 insertions(+), 13 deletions(-)

diff --git a/drivers/pinctrl/pinctrl-tegra.c b/drivers/pinctrl/pinctrl-tegra.c
index 7da0b37..69c08d5 100644
--- a/drivers/pinctrl/pinctrl-tegra.c
+++ b/drivers/pinctrl/pinctrl-tegra.c
@@ -41,6 +41,11 @@ struct tegra_pmx {
 
 	int nbanks;
 	void __iomem **regs;
+
+#ifdef CONFIG_PM_SLEEP
+	int *bank_nregs;
+	u32 *regs_storage;
+#endif
 };
 
 static inline u32 pmx_readl(struct tegra_pmx *pmx, u32 bank, u32 reg)
@@ -685,12 +690,83 @@ static struct pinctrl_desc tegra_pinctrl_desc = {
 	.owner = THIS_MODULE,
 };
 
+#ifdef CONFIG_PM_SLEEP
+static int tegra_pinctrl_suspend_noirq(struct device *dev)
+{
+	struct tegra_pmx *pmx = dev_get_drvdata(dev);
+	u32 *regs_storage = pmx->regs_storage;
+	u32 *regs;
+	int i, j;
+
+	for (i = 0; i < pmx->nbanks; i++) {
+		regs = pmx->regs[i];
+		for (j = 0; j < pmx->bank_nregs[i]; j++)
+			*regs_storage++ = readl(regs++);
+	}
+
+	return 0;
+}
+
+static int tegra_pinctrl_resume_noirq(struct device *dev)
+{
+	struct tegra_pmx *pmx = dev_get_drvdata(dev);
+	u32 *regs_storage = pmx->regs_storage;
+	u32 *regs;
+	int i, j;
+
+	for (i = 0; i < pmx->nbanks; i++) {
+		regs = pmx->regs[i];
+		for (j = 0; j < pmx->bank_nregs[i]; j++)
+			writel(*regs_storage++, regs++);
+	}
+
+	return 0;
+}
+
+const struct dev_pm_ops tegra_pinctrl_pm_ops = {
+	.suspend_noirq = tegra_pinctrl_suspend_noirq,
+	.resume_noirq = tegra_pinctrl_resume_noirq,
+};
+
+static int __devinit tegra_pinctrl_pm_init(struct tegra_pmx *pmx)
+{
+	struct platform_device *pdev = to_platform_device(pmx->dev);
+	struct resource *res;
+	int i, bank_size, total_banks_size = 0;
+
+	pmx->bank_nregs = devm_kzalloc(&pdev->dev,
+				       pmx->nbanks * sizeof(*pmx->bank_nregs),
+				       GFP_KERNEL);
+	if (!pmx->bank_nregs) {
+		dev_err(&pdev->dev, "Can't alloc bank_nregs pointer\n");
+		return -ENODEV;
+	}
+
+	for (i = 0; i < pmx->nbanks; i++) {
+		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
+		bank_size = resource_size(res);
+
+		pmx->bank_nregs[i] = bank_size / 4;
+		total_banks_size += bank_size;
+	}
+
+	pmx->regs_storage = devm_kzalloc(&pdev->dev, total_banks_size,
+					 GFP_KERNEL);
+	if (!pmx->regs_storage) {
+		dev_err(&pdev->dev, "Can't alloc regs_storage pointer\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+#endif
+
 int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 			const struct tegra_pinctrl_soc_data *soc_data)
 {
 	struct tegra_pmx *pmx;
 	struct resource *res;
-	int i;
+	int i, ret = 0;
 
 	pmx = devm_kzalloc(&pdev->dev, sizeof(*pmx), GFP_KERNEL);
 	if (!pmx) {
@@ -699,19 +775,13 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 	}
 	pmx->dev = &pdev->dev;
 	pmx->soc = soc_data;
+	pmx->nbanks = pdev->num_resources;
 
 	tegra_pinctrl_gpio_range.npins = pmx->soc->ngpios;
 	tegra_pinctrl_desc.name = dev_name(&pdev->dev);
 	tegra_pinctrl_desc.pins = pmx->soc->pins;
 	tegra_pinctrl_desc.npins = pmx->soc->npins;
 
-	for (i = 0; ; i++) {
-		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
-		if (!res)
-			break;
-	}
-	pmx->nbanks = i;
-
 	pmx->regs = devm_kzalloc(&pdev->dev, pmx->nbanks * sizeof(*pmx->regs),
 				 GFP_KERNEL);
 	if (!pmx->regs) {
@@ -727,8 +797,8 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 		}
 
 		if (!devm_request_mem_region(&pdev->dev, res->start,
-					    resource_size(res),
-					    dev_name(&pdev->dev))) {
+					     resource_size(res),
+					     dev_name(&pdev->dev))) {
 			dev_err(&pdev->dev,
 				"Couldn't request MEM resource %d\n", i);
 			return -ENODEV;
@@ -742,6 +812,12 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 		}
 	}
 
+#ifdef CONFIG_PM_SLEEP
+	ret = tegra_pinctrl_pm_init(pmx);
+	if (ret)
+		return ret;
+#endif
+
 	pmx->pctl = pinctrl_register(&tegra_pinctrl_desc, &pdev->dev, pmx);
 	if (!pmx->pctl) {
 		dev_err(&pdev->dev, "Couldn't register pinctrl driver\n");
@@ -754,7 +830,7 @@ int __devinit tegra_pinctrl_probe(struct platform_device *pdev,
 
 	dev_dbg(&pdev->dev, "Probed Tegra pinctrl driver\n");
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_GPL(tegra_pinctrl_probe);
 
diff --git a/drivers/pinctrl/pinctrl-tegra.h b/drivers/pinctrl/pinctrl-tegra.h
index 62e3809..bbe27cd 100644
--- a/drivers/pinctrl/pinctrl-tegra.h
+++ b/drivers/pinctrl/pinctrl-tegra.h
@@ -187,4 +187,11 @@ int tegra_pinctrl_probe(struct platform_device *pdev,
 			const struct tegra_pinctrl_soc_data *soc_data);
 int tegra_pinctrl_remove(struct platform_device *pdev);
 
+#ifdef CONFIG_PM_SLEEP
+extern const struct dev_pm_ops tegra_pinctrl_pm_ops;
+#define TEGRA_PINCTRL_PM	(&tegra_pinctrl_pm_ops)
+#else
+#define TEGRA_PINCTRL_PM	NULL
+#endif
+
 #endif
diff --git a/drivers/pinctrl/pinctrl-tegra20.c b/drivers/pinctrl/pinctrl-tegra20.c
index a74f9a5..6f09023 100644
--- a/drivers/pinctrl/pinctrl-tegra20.c
+++ b/drivers/pinctrl/pinctrl-tegra20.c
@@ -2871,6 +2871,7 @@ static struct platform_driver tegra20_pinctrl_driver = {
 		.name = "tegra20-pinctrl",
 		.owner = THIS_MODULE,
 		.of_match_table = tegra20_pinctrl_of_match,
+		.pm = TEGRA_PINCTRL_PM,
 	},
 	.probe = tegra20_pinctrl_probe,
 	.remove = __devexit_p(tegra_pinctrl_remove),
@@ -2880,7 +2881,7 @@ static int __init tegra20_pinctrl_init(void)
 {
 	return platform_driver_register(&tegra20_pinctrl_driver);
 }
-arch_initcall(tegra20_pinctrl_init);
+core_initcall(tegra20_pinctrl_init);
 
 static void __exit tegra20_pinctrl_exit(void)
 {
diff --git a/drivers/pinctrl/pinctrl-tegra30.c b/drivers/pinctrl/pinctrl-tegra30.c
index 7894f14..2e90632 100644
--- a/drivers/pinctrl/pinctrl-tegra30.c
+++ b/drivers/pinctrl/pinctrl-tegra30.c
@@ -3737,6 +3737,7 @@ static struct platform_driver tegra30_pinctrl_driver = {
 		.name = "tegra30-pinctrl",
 		.owner = THIS_MODULE,
 		.of_match_table = tegra30_pinctrl_of_match,
+		.pm = TEGRA_PINCTRL_PM,
 	},
 	.probe = tegra30_pinctrl_probe,
 	.remove = __devexit_p(tegra_pinctrl_remove),
@@ -3746,7 +3747,7 @@ static int __init tegra30_pinctrl_init(void)
 {
 	return platform_driver_register(&tegra30_pinctrl_driver);
 }
-arch_initcall(tegra30_pinctrl_init);
+core_initcall(tegra30_pinctrl_init);
 
 static void __exit tegra30_pinctrl_exit(void)
 {
-- 
1.7.12

Re: [PATCH V3] pinctrl: tegra: add suspend/resume support

[Stephen Warren]

On 11/05/2012 06:37 PM, Dmitry Osipenko wrote:
> Added suspend/resume support using pm ops. We need to store current regs vals on
> suspend and restore them on resume. Platform driver registering function moved to
> core init level to ensure that device driver will be in the end of suspend and in
> the beginning of resume lists.
> 
> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> Pritesh, what do you think about this patch?
> 
>> BTW, can you please post a link to the source for your downstream kernel
>> that this came from - I'd be very interested in seeing what work has
>> been done there. Thanks.
> 
> I have sent invitation to you on bitbucket.

Oh, it's not public? I've been asked not to sign any NDAs/agreements/...
without review from NVIDIA legal, and a non-public git repo seems like
it'd fall into the same category. I'm left wondering how the repo is
useful if it isn't public.

Re: [PATCH V3] pinctrl: tegra: add suspend/resume support

[Dmitry Osipenko]

> 06.11.2012 07:41, Stephen Warren wrote:
> Oh, it's not public? I've been asked not to sign any NDAs/agreements/...
> without review from NVIDIA legal, and a non-public git repo seems like
> it'd fall into the same category. I'm left wondering how the repo is
> useful if it isn't public.

I think you are wrong. It's personal private repo. I and my companion are
working on it for our android rom in our spare time, it's just a hobby. The
reason we are keeping it private is that there are some "devs" (on
xda-developers especially) that will copy-paste all your work with all original
author info wiped. We think it's a bit unfair, so keeping code private for now.
There are no nda's, no agreements and should be 100% safe for you. All those
nda's just makes people unhappy...

I never contributed to mainline kernel before and want to ask what should I do
when my patches are reviewed/acked? Should I resend patch with reviewed/acked
line or something else?

Re: [PATCH V3] pinctrl: tegra: add suspend/resume support

[Stephen Warren]

On 11/06/2012 06:08 AM, Dmitry Osipenko wrote:
>> 06.11.2012 07:41, Stephen Warren wrote:
>> Oh, it's not public? I've been asked not to sign any NDAs/agreements/...
>> without review from NVIDIA legal, and a non-public git repo seems like
>> it'd fall into the same category. I'm left wondering how the repo is
>> useful if it isn't public.
> 
> I think you are wrong. It's personal private repo. I and my companion are
> working on it for our android rom in our spare time, it's just a hobby. The
> reason we are keeping it private is that there are some "devs" (on

OK, so only you and he have the binaries built from this repository? Or,
are those binaries distributed to other people too? If the binaries are
distributed, you need to distribute (or offer to make available) the
source too. See the GPL for exact requirements.

> xda-developers especially) that will copy-paste all your work

That's kindof the whole point of the license...

> with all original author info wiped. We think it's a bit unfair,

Ok, that's certainly not right. However, I don't believe it affects your
obligations under the GPL.

(Of course, I Am Not A Lawyer, and in fact I complete hate SW licensing
issues; I'd far rather be able for everyone to publish everything as
public domain or something equivalently simple, yet still have everyone
else behave nicely and contribute their changes back. Still, that's
evidently a naive desire).

> There are no nda's, no agreements and should be 100% safe for you. All those
> nda's just makes people unhappy...

OK, so if I accept the private repo link, download the source, and
repost it on my github account, you're fine with that? If not, then
you're requesting something semantically equivalent to an NDA.

> I never contributed to mainline kernel before and want to ask what should I do
> when my patches are reviewed/acked? Should I resend patch with reviewed/acked
> line or something else?

You should certainly repost the patch to the people that
scripts/get_maintainers.pl recommends; the pinctrl subsystem maintainer
would be the person applying this patch rather than me as the Tegra
maintainer. Include the ack(s) you've received when you repost.

However, I'd ask that we resolve the distribution issues of the source
kernel first to avoid any tainting of the patch.

Re: [PATCH V3] pinctrl: tegra: add suspend/resume support

[Dmitry Osipenko]

06.11.2012 21:38, Stephen Warren wrote:
> OK, so only you and he have the binaries built from this repository? Or,
> are those binaries distributed to other people too? If the binaries are
> distributed, you need to distribute (or offer to make available) the
> source too. See the GPL for exact requirements.

I'm not against GPL, but for now repo is private. It's something like nvidia's
private downstream kernel that I'm working on. Surely it will become public but
bit later.

> OK, so if I accept the private repo link, download the source, and
> repost it on my github account, you're fine with that? If not, then
> you're requesting something semantically equivalent to an NDA.

For me it's not very important, but my companion may be unhappy with that. I
just believe that you are not so evil. As I understand NDA should be some
legally valid official document. I'm sure you are much better in this than me,
so let's stop discussing it.

> However, I'd ask that we resolve the distribution issues of the source
> kernel first to avoid any tainting of the patch.

I don't see any issues. It's my personal work that I'm contributing to the
kernel community. If nvidia is against of any public contributions just tell me.

If issue is resolved, then it will be interesting to see your comment to V3 patch.

> You should certainly repost the patch to the people that
> scripts/get_maintainers.pl recommends; the pinctrl subsystem maintainer
> would be the person applying this patch rather than me as the Tegra
> maintainer. Include the ack(s) you've received when you repost.

Thank you for answering.

Re: [PATCH V3] pinctrl: tegra: add suspend/resume support

[Stephen Warren]

On 11/06/2012 01:06 PM, Dmitry Osipenko wrote:
> 06.11.2012 21:38, Stephen Warren wrote:
>> OK, so only you and he have the binaries built from this repository? Or,
>> are those binaries distributed to other people too? If the binaries are
>> distributed, you need to distribute (or offer to make available) the
>> source too. See the GPL for exact requirements.
> 
> I'm not against GPL, but for now repo is private. It's something like nvidia's
> private downstream kernel that I'm working on. Surely it will become public but
> bit later.
> 
>> OK, so if I accept the private repo link, download the source, and
>> repost it on my github account, you're fine with that? If not, then
>> you're requesting something semantically equivalent to an NDA.
> 
> For me it's not very important, but my companion may be unhappy with that. I
> just believe that you are not so evil. As I understand NDA should be some
> legally valid official document. I'm sure you are much better in this than me,
> so let's stop discussing it.
> 
>> However, I'd ask that we resolve the distribution issues of the source
>> kernel first to avoid any tainting of the patch.
> 
> I don't see any issues. It's my personal work that I'm contributing to the
> kernel community. If nvidia is against of any public contributions just tell me.

NVIDIA and indeed the kernel community welcome public contributions.

However, the rules in SubmittingPatches (as set by the kernel community,
not NVIDIA) are clear re: the licensing requirements for patches. If
you're taking the patches from a downstream kernel that's published as
binaries and not source, I believe that makes the patches non-compliant
(since there's a GPL violation in the downstream kernel, so the patches
can't be passed off as being GPL compliant), and hence your
signed-off-by line is not valid.

Once the downstream kernel's source is publicly available, I imagine
there will be no problem accepting patches that are derived from it.

Re: [PATCH V3] pinctrl: tegra: add suspend/resume support

[Dmitry Osipenko]

07.11.2012 01:45, Stephen Warren wrote:
> On 11/06/2012 01:06 PM, Dmitry Osipenko wrote:
> NVIDIA and indeed the kernel community welcome public contributions.
> 
> However, the rules in SubmittingPatches (as set by the kernel community,
> not NVIDIA) are clear re: the licensing requirements for patches. If
> you're taking the patches from a downstream kernel that's published as
> binaries and not source, I believe that makes the patches non-compliant
> (since there's a GPL violation in the downstream kernel, so the patches
> can't be passed off as being GPL compliant), and hence your
> signed-off-by line is not valid.
> 
> Once the downstream kernel's source is publicly available, I imagine
> there will be no problem accepting patches that are derived from it.

Why you are forcing me to do what I don't want? I downloaded source code from
kernel.org made some changes and now submitting patches. I mustn't show you all
my changes. From your logic you must make nvidia's private downstream kernel
public. Notice, I didn't told you that we distribute binaries from *that* repo
to anyone. Oh, my... How to explain.. *it's my personal work I'm doing for
myself*. So again, if you don't want accept my patches anymore for some reason,
tell me and we won't waste our time anymore. I just wanted to help a bit and
have feeling that you misunderstands me... My english is not ideal, so maybe
it's the source of it.

Re: [PATCH V3] pinctrl: tegra: add suspend/resume support

[Dmitry Osipenko]

07.11.2012 01:45, Stephen Warren пишет:
> On 11/06/2012 01:06 PM, Dmitry Osipenko wrote:
>> 06.11.2012 21:38, Stephen Warren wrote:
>>> OK, so only you and he have the binaries built from this repository? Or,
>>> are those binaries distributed to other people too? If the binaries are
>>> distributed, you need to distribute (or offer to make available) the
>>> source too. See the GPL for exact requirements.
>>
>> I'm not against GPL, but for now repo is private. It's something like nvidia's
>> private downstream kernel that I'm working on. Surely it will become public but
>> bit later.
>>
>>> OK, so if I accept the private repo link, download the source, and
>>> repost it on my github account, you're fine with that? If not, then
>>> you're requesting something semantically equivalent to an NDA.
>>
>> For me it's not very important, but my companion may be unhappy with that. I
>> just believe that you are not so evil. As I understand NDA should be some
>> legally valid official document. I'm sure you are much better in this than me,
>> so let's stop discussing it.
>>
>>> However, I'd ask that we resolve the distribution issues of the source
>>> kernel first to avoid any tainting of the patch.
>>
>> I don't see any issues. It's my personal work that I'm contributing to the
>> kernel community. If nvidia is against of any public contributions just tell me.
> 
> NVIDIA and indeed the kernel community welcome public contributions.
> 
> However, the rules in SubmittingPatches (as set by the kernel community,
> not NVIDIA) are clear re: the licensing requirements for patches. If
> you're taking the patches from a downstream kernel that's published as
> binaries and not source, I believe that makes the patches non-compliant
> (since there's a GPL violation in the downstream kernel, so the patches
> can't be passed off as being GPL compliant), and hence your
> signed-off-by line is not valid.
> 
> Once the downstream kernel's source is publicly available, I imagine
> there will be no problem accepting patches that are derived from it.
> 

Hello, Stephen. I made my recent work on kernel public and it's available
at https://bitbucket.org/digetx/picasso-kernel/ It contains all patches that
I have sent and has some small fixes that I will send later. Hope there is no
problem anymore and you would like to continue reviewing my patches.

Re: [PATCH V3] pinctrl: tegra: add suspend/resume support

[Stephen Warren]

On 03/04/2013 05:13 PM, Dmitry Osipenko wrote:
> 07.11.2012 01:45, Stephen Warren пишет:
>> On 11/06/2012 01:06 PM, Dmitry Osipenko wrote:
>>> 06.11.2012 21:38, Stephen Warren wrote:
>>>> OK, so only you and he have the binaries built from this repository? Or,
>>>> are those binaries distributed to other people too? If the binaries are
>>>> distributed, you need to distribute (or offer to make available) the
>>>> source too. See the GPL for exact requirements.
>>>
>>> I'm not against GPL, but for now repo is private. It's something like nvidia's
>>> private downstream kernel that I'm working on. Surely it will become public but
>>> bit later.
>>>
>>>> OK, so if I accept the private repo link, download the source, and
>>>> repost it on my github account, you're fine with that? If not, then
>>>> you're requesting something semantically equivalent to an NDA.
>>>
>>> For me it's not very important, but my companion may be unhappy with that. I
>>> just believe that you are not so evil. As I understand NDA should be some
>>> legally valid official document. I'm sure you are much better in this than me,
>>> so let's stop discussing it.
>>>
>>>> However, I'd ask that we resolve the distribution issues of the source
>>>> kernel first to avoid any tainting of the patch.
>>>
>>> I don't see any issues. It's my personal work that I'm contributing to the
>>> kernel community. If nvidia is against of any public contributions just tell me.
>>
>> NVIDIA and indeed the kernel community welcome public contributions.
>>
>> However, the rules in SubmittingPatches (as set by the kernel community,
>> not NVIDIA) are clear re: the licensing requirements for patches. If
>> you're taking the patches from a downstream kernel that's published as
>> binaries and not source, I believe that makes the patches non-compliant
>> (since there's a GPL violation in the downstream kernel, so the patches
>> can't be passed off as being GPL compliant), and hence your
>> signed-off-by line is not valid.
>>
>> Once the downstream kernel's source is publicly available, I imagine
>> there will be no problem accepting patches that are derived from it.
>>
> 
> Hello, Stephen. I made my recent work on kernel public and it's available
> at https://bitbucket.org/digetx/picasso-kernel/ It contains all patches that
> I have sent and has some small fixes that I will send later. Hope there is no
> problem anymore and you would like to continue reviewing my patches.

Sure. So long as you've read Documentation/SubmittingPatches, and fully
understood exactly what Signed-off-by means and the GPL, I have no
problem taking patches.

It's been a while, so I'd suggest reposting any patches.

Re: [PATCH V3] pinctrl: tegra: add suspend/resume support

[Stephen Warren]

On 11/06/2012 06:08 AM, Dmitry Osipenko wrote:
...
> I never contributed to mainline kernel before and want to ask what should I do
> when my patches are reviewed/acked? Should I resend patch with reviewed/acked
> line or something else?

Oh, and I should mention that you should also read all of
Documentation/SubmittingPatches in the kernel source tree.

Re: [PATCH V3] pinctrl: tegra: add suspend/resume support

[Pritesh Raithatha]

On Tuesday 06 November 2012 07:07 AM, Dmitry Osipenko wrote:
> Added suspend/resume support using pm ops. We need to store current regs vals on
> suspend and restore them on resume. Platform driver registering function moved to
> core init level to ensure that device driver will be in the end of suspend and in
> the beginning of resume lists.
> 
> Signed-off-by: Dmitry Osipenko <digetx-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> Pritesh, what do you think about this patch?
> 
 It looks good.

 Acked-by: Pritesh Raithatha <praithatha-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>