[PATCH 0/5] i2c: tegra: Code cleanups and suspend/resume related changes

[PATCH 0/5] i2c: tegra: Code cleanups and suspend/resume related changes

[Laxman Dewangan]

This patch series does:
- Code cleanups like removing unused variables, using already define function
- Use clock_disable_unprepare in place of clock_disable()
- Use dev_pm_ops for suspend/resume inplace of legacy suspend/resume callbacks.
- Convert suspend/resume to _noirq version to support late suspend/early resume.

Laxman Dewangan (5):
  i2c: tegra: remove unused member variable
  i2c: tegra: use clk_disable_unprepare in place of clk_disable
  i2c: tegra: use of_match_ptr() for match_table initialization
  i2c: tegra: Use struct dev_pm_ops for power management
  i2c: tegra: convert normal suspend/resume to *_noirq

 drivers/i2c/busses/i2c-tegra.c |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

[PATCH 1/5] i2c: tegra: remove unused member variable

[Laxman Dewangan]

Remove unused member variable "iomem" of the
i2c device structure.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
This patch is resend of earlier patch
 i2c: tegra: remove unused member variable.
and sending it as part fo series.

 drivers/i2c/busses/i2c-tegra.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index f15cd49..00cdc10 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -119,7 +119,6 @@ enum msg_end_type {
  * @adapter: core i2c layer adapter information
  * @clk: clock reference for i2c controller
  * @i2c_clk: clock reference for i2c bus
- * @iomem: memory resource for registers
  * @base: ioremapped registers cookie
  * @cont_id: i2c controller id, used for for packet header
  * @irq: irq number of transfer complete interrupt
@@ -137,7 +136,6 @@ struct tegra_i2c_dev {
 	struct i2c_adapter adapter;
 	struct clk *clk;
 	struct clk *i2c_clk;
-	struct resource *iomem;
 	void __iomem *base;
 	int cont_id;
 	int irq;
-- 
1.7.1.1

Re: [PATCH 1/5] i2c: tegra: remove unused member variable

[Shubhrajyoti Datta]

Hi Laxman,

On Tue, Jul 10, 2012 at 4:50 PM, Laxman Dewangan <ldewangan@nvidia.com> wrote:
> Remove unused member variable "iomem" of the
> i2c device structure.

You might want to mention the commit which removed the last usage etc.

>
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
> This patch is resend of earlier patch
>  i2c: tegra: remove unused member variable.
> and sending it as part fo series.
>

[PATCH 2/5] i2c: tegra: use clk_disable_unprepare in place of clk_disable

[Laxman Dewangan]

Use clk_disable_unprepare() inplace of clk_disable().
This was missed as part of moving clock enable/disable to
prepare/unprepare for using the common clock framework.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
This is already discussed in patch
[PATCH] i2c: tegra: use clk_disable_unprepare in place of clk_disable
for applying in Wolfram Tree. Resending this as patch series.

 drivers/i2c/busses/i2c-tegra.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 00cdc10..d071cf0 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -696,7 +696,7 @@ static int __devinit tegra_i2c_probe(struct platform_device *pdev)
 	ret = i2c_add_numbered_adapter(&i2c_dev->adapter);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to add I2C adapter\n");
-		clk_disable(i2c_dev->i2c_clk);
+		clk_disable_unprepare(i2c_dev->i2c_clk);
 		return ret;
 	}
 
-- 
1.7.1.1

Re: [PATCH 2/5] i2c: tegra: use clk_disable_unprepare in place of clk_disable

[Wolfram Sang]

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

On Tue, Jul 10, 2012 at 04:50:41PM +0530, Laxman Dewangan wrote:
> Use clk_disable_unprepare() inplace of clk_disable().
> This was missed as part of moving clock enable/disable to
> prepare/unprepare for using the common clock framework.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

OK, as per the previous discussion, I applied it to my next-branch.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[PATCH 3/5] i2c: tegra: use of_match_ptr() for match_table initialization

[Laxman Dewangan]

In place of defining match_table for non-DT based as NULL,
use of_match_ptr() for initialzing the of_match_table.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index d071cf0..0792674 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -754,8 +754,6 @@ static const struct of_device_id tegra_i2c_of_match[] __devinitconst = {
 	{},
 };
 MODULE_DEVICE_TABLE(of, tegra_i2c_of_match);
-#else
-#define tegra_i2c_of_match NULL
 #endif
 
 static struct platform_driver tegra_i2c_driver = {
@@ -768,7 +766,7 @@ static struct platform_driver tegra_i2c_driver = {
 	.driver  = {
 		.name  = "tegra-i2c",
 		.owner = THIS_MODULE,
-		.of_match_table = tegra_i2c_of_match,
+		.of_match_table = of_match_ptr(tegra_i2c_of_match),
 	},
 };
 
-- 
1.7.1.1

Re: [PATCH 3/5] i2c: tegra: use of_match_ptr() for match_table initialization

[Wolfram Sang]

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

On Tue, Jul 10, 2012 at 04:50:42PM +0530, Laxman Dewangan wrote:
> In place of defining match_table for non-DT based as NULL,
> use of_match_ptr() for initialzing the of_match_table.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

Applied to next, thanks.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[PATCH 4/5] i2c: tegra: Use struct dev_pm_ops for power management

[Laxman Dewangan]

Make the tegra i2c driver define its PM callbacks through
struct dev_pm_ops objects rather than by using legacy PM hooks
in struct platform_driver.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 0792674..5f0572d 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -713,8 +713,9 @@ static int __devexit tegra_i2c_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM
-static int tegra_i2c_suspend(struct platform_device *pdev, pm_message_t state)
+static int tegra_i2c_suspend(struct device *dev)
 {
+	struct platform_device *pdev = to_platform_device(dev);
 	struct tegra_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
 
 	i2c_lock_adapter(&i2c_dev->adapter);
@@ -724,8 +725,9 @@ static int tegra_i2c_suspend(struct platform_device *pdev, pm_message_t state)
 	return 0;
 }
 
-static int tegra_i2c_resume(struct platform_device *pdev)
+static int tegra_i2c_resume(struct device *dev)
 {
+	struct platform_device *pdev = to_platform_device(dev);
 	struct tegra_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
 	int ret;
 
@@ -744,6 +746,14 @@ static int tegra_i2c_resume(struct platform_device *pdev)
 
 	return 0;
 }
+
+static const struct dev_pm_ops tegra_i2c_dev_pm_ops = {
+	.suspend = tegra_i2c_suspend,
+	.resume = tegra_i2c_resume,
+};
+#define TEGRA_I2C_DEV_PM_OPS (&tegra_i2c_dev_pm_ops)
+#else
+#define TEGRA_I2C_DEV_PM_OPS NULL
 #endif
 
 #if defined(CONFIG_OF)
@@ -759,14 +769,11 @@ MODULE_DEVICE_TABLE(of, tegra_i2c_of_match);
 static struct platform_driver tegra_i2c_driver = {
 	.probe   = tegra_i2c_probe,
 	.remove  = __devexit_p(tegra_i2c_remove),
-#ifdef CONFIG_PM
-	.suspend = tegra_i2c_suspend,
-	.resume  = tegra_i2c_resume,
-#endif
 	.driver  = {
 		.name  = "tegra-i2c",
 		.owner = THIS_MODULE,
 		.of_match_table = of_match_ptr(tegra_i2c_of_match),
+		.pm = TEGRA_I2C_DEV_PM_OPS,
 	},
 };
 
-- 
1.7.1.1

Re: [PATCH 4/5] i2c: tegra: Use struct dev_pm_ops for power management

[Wolfram Sang]

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

On Tue, Jul 10, 2012 at 04:50:43PM +0530, Laxman Dewangan wrote:
> Make the tegra i2c driver define its PM callbacks through
> struct dev_pm_ops objects rather than by using legacy PM hooks
> in struct platform_driver.
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  drivers/i2c/busses/i2c-tegra.c |   19 +++++++++++++------
>  1 files changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
> index 0792674..5f0572d 100644
> --- a/drivers/i2c/busses/i2c-tegra.c
> +++ b/drivers/i2c/busses/i2c-tegra.c
> @@ -713,8 +713,9 @@ static int __devexit tegra_i2c_remove(struct platform_device *pdev)
>  }
>  
>  #ifdef CONFIG_PM
> -static int tegra_i2c_suspend(struct platform_device *pdev, pm_message_t state)
> +static int tegra_i2c_suspend(struct device *dev)
>  {
> +	struct platform_device *pdev = to_platform_device(dev);
>  	struct tegra_i2c_dev *i2c_dev = platform_get_drvdata(pdev);

I like Rafael's patch better because of dev_get_drvdata. I'll pick his
and add your 5/5 on top of that.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

[PATCH 5/5] i2c: tegra: convert normal suspend/resume to *_noirq

[Laxman Dewangan]

To provide the late suspend and early resume for i2c
driver, convert the suspend/resume as
	suspend-> suspend_noirq
	resume -> resume_noirq

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 drivers/i2c/busses/i2c-tegra.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index 5f0572d..99e6ae5 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -713,7 +713,7 @@ static int __devexit tegra_i2c_remove(struct platform_device *pdev)
 }
 
 #ifdef CONFIG_PM
-static int tegra_i2c_suspend(struct device *dev)
+static int tegra_i2c_suspend_noirq(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct tegra_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
@@ -725,7 +725,7 @@ static int tegra_i2c_suspend(struct device *dev)
 	return 0;
 }
 
-static int tegra_i2c_resume(struct device *dev)
+static int tegra_i2c_resume_noirq(struct device *dev)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct tegra_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
@@ -748,8 +748,8 @@ static int tegra_i2c_resume(struct device *dev)
 }
 
 static const struct dev_pm_ops tegra_i2c_dev_pm_ops = {
-	.suspend = tegra_i2c_suspend,
-	.resume = tegra_i2c_resume,
+	.suspend_noirq = tegra_i2c_suspend_noirq,
+	.resume_noirq = tegra_i2c_resume_noirq,
 };
 #define TEGRA_I2C_DEV_PM_OPS (&tegra_i2c_dev_pm_ops)
 #else
-- 
1.7.1.1

Re: [PATCH 5/5] i2c: tegra: convert normal suspend/resume to *_noirq

[Wolfram Sang]

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

On Tue, Jul 10, 2012 at 04:50:44PM +0530, Laxman Dewangan wrote:
> To provide the late suspend and early resume for i2c
> driver, convert the suspend/resume as
> 	suspend-> suspend_noirq
> 	resume -> resume_noirq
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

Applied to next on top of Rafael's patches. Please have a look if I did
the fixup right.

Thanks,

   Wolfram

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH 5/5] i2c: tegra: convert normal suspend/resume to *_noirq

[Laxman Dewangan]

On Thursday 12 July 2012 04:05 PM, Wolfram Sang wrote:
> * PGP Signed by an unknown key
>
> On Tue, Jul 10, 2012 at 04:50:44PM +0530, Laxman Dewangan wrote:
>> To provide the late suspend and early resume for i2c
>> driver, convert the suspend/resume as
>> 	suspend->  suspend_noirq
>> 	resume ->  resume_noirq
>>
>> Signed-off-by: Laxman Dewangan<ldewangan@nvidia.com>
> Applied to next on top of Rafael's patches. Please have a look if I did
> the fixup right.
Thanks for taking care.
Can you please point me your git path so that I can verify it?

Thanks,
Laxman

Re: [PATCH 5/5] i2c: tegra: convert normal suspend/resume to *_noirq

[Wolfram Sang]

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


> Can you please point me your git path so that I can verify it?

git://git.pengutronix.de/git/wsa/linux.git i2c-embedded/for-next

or

http://git.pengutronix.de/?p=wsa/linux.git;a=shortlog;h=refs/heads/i2c-embedded/for-next

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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

Re: [PATCH 5/5] i2c: tegra: convert normal suspend/resume to *_noirq

[Laxman Dewangan]

On Thursday 12 July 2012 05:28 PM, Wolfram Sang wrote:
> * PGP Signed by an unknown key
>
>
>> Can you please point me your git path so that I can verify it?
> git://git.pengutronix.de/git/wsa/linux.git i2c-embedded/for-next
>
> or
>
> http://git.pengutronix.de/?p=wsa/linux.git;a=shortlog;h=refs/heads/i2c-embedded/for-next
>

The change is fine.  No issue here.

Thanks,
Laxman

Re: [PATCH 5/5] i2c: tegra: convert normal suspend/resume to *_noirq

[Stephen Warren]

On 07/10/2012 05:20 AM, Laxman Dewangan wrote:
> To provide the late suspend and early resume for i2c
> driver, convert the suspend/resume as
> 	suspend-> suspend_noirq
> 	resume -> resume_noirq
> 
> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>

Why do we need this change?

IIRC, I proposed it before solely to solve some suspend/resume ordering
issues, and Colin Cross NAKd it. These days, deferred probe should make
this change unnecessary.

Unless there's a really good reason for this change, I'd tend to request
reverting it.

Re: [PATCH 5/5] i2c: tegra: convert normal suspend/resume to *_noirq

[Peter De Schrijver]

On Mon, Jul 23, 2012 at 08:24:38PM +0200, Stephen Warren wrote:
> On 07/10/2012 05:20 AM, Laxman Dewangan wrote:
> > To provide the late suspend and early resume for i2c
> > driver, convert the suspend/resume as
> > 	suspend-> suspend_noirq
> > 	resume -> resume_noirq
> > 
> > Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> 
> Why do we need this change?
> 
> IIRC, I proposed it before solely to solve some suspend/resume ordering
> issues, and Colin Cross NAKd it. These days, deferred probe should make
> this change unnecessary.
> 

How does deferred probe solve this? Iirc the problem is that i2c is needed to
talk to the PMIC, so we need to make sure i2c is shutdown last because drivers
might want to do regulator control in the suspend function.

Cheers,

Peter.

Re: [PATCH 5/5] i2c: tegra: convert normal suspend/resume to *_noirq

[Mark Brown]

On Tue, Jul 24, 2012 at 12:13:58PM +0300, Peter De Schrijver wrote:
> On Mon, Jul 23, 2012 at 08:24:38PM +0200, Stephen Warren wrote:

> > IIRC, I proposed it before solely to solve some suspend/resume ordering
> > issues, and Colin Cross NAKd it. These days, deferred probe should make
> > this change unnecessary.

> How does deferred probe solve this? Iirc the problem is that i2c is needed to
> talk to the PMIC, so we need to make sure i2c is shutdown last because drivers
> might want to do regulator control in the suspend function.

With probe deferral the drivers will be after the regulator driver in
the DPM list and so be suspended first.

Re: [PATCH 5/5] i2c: tegra: convert normal suspend/resume to *_noirq

[Wolfram Sang]

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

On Mon, Jul 23, 2012 at 12:24:38PM -0600, Stephen Warren wrote:
> On 07/10/2012 05:20 AM, Laxman Dewangan wrote:
> > To provide the late suspend and early resume for i2c
> > driver, convert the suspend/resume as
> > 	suspend-> suspend_noirq
> > 	resume -> resume_noirq
> > 
> > Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> 
> Why do we need this change?
> 
> IIRC, I proposed it before solely to solve some suspend/resume ordering
> issues, and Colin Cross NAKd it. These days, deferred probe should make
> this change unnecessary.
> 
> Unless there's a really good reason for this change, I'd tend to request
> reverting it.

I will drop it from my tree and exclude it from my pull request. Please
resend if it should go in later.

-- 
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

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