All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
@ 2014-02-13 14:09 ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Alessandro Rubini, Linus Walleij, Wolfram Sang, Chris Ball,
	Mark Brown, linux-kernel, linux-i2c, linux-spi, linux-mmc,
	Ulf Hansson

Use devm_* functions to simplify code and error handling.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |   29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index bc8ba02..5faf9e2 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -981,7 +981,7 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 		return -ENODEV;
 	}
 
-	dev = kzalloc(sizeof(struct nmk_i2c_dev), GFP_KERNEL);
+	dev = devm_kzalloc(&adev->dev, sizeof(struct nmk_i2c_dev), GFP_KERNEL);
 	if (!dev) {
 		dev_err(&adev->dev, "cannot allocate memory\n");
 		ret = -ENOMEM;
@@ -1011,27 +1011,28 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	/* If possible, let's go to idle until the first transfer */
 	pinctrl_pm_select_idle_state(&adev->dev);
 
-	dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
+	dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
+				resource_size(&adev->res));
 	if (!dev->virtbase) {
 		ret = -ENOMEM;
-		goto err_no_ioremap;
+		goto err_no_mem;
 	}
 
 	dev->irq = adev->irq[0];
-	ret = request_irq(dev->irq, i2c_irq_handler, 0,
+	ret = devm_request_irq(&adev->dev, dev->irq, i2c_irq_handler, 0,
 				DRIVER_NAME, dev);
 	if (ret) {
 		dev_err(&adev->dev, "cannot claim the irq %d\n", dev->irq);
-		goto err_irq;
+		goto err_no_mem;
 	}
 
 	pm_suspend_ignore_children(&adev->dev, true);
 
-	dev->clk = clk_get(&adev->dev, NULL);
+	dev->clk = devm_clk_get(&adev->dev, NULL);
 	if (IS_ERR(dev->clk)) {
 		dev_err(&adev->dev, "could not get i2c clock\n");
 		ret = PTR_ERR(dev->clk);
-		goto err_no_clk;
+		goto err_no_mem;
 	}
 
 	adap = &dev->adap;
@@ -1053,21 +1054,13 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	ret = i2c_add_adapter(adap);
 	if (ret) {
 		dev_err(&adev->dev, "failed to add adapter\n");
-		goto err_add_adap;
+		goto err_no_mem;
 	}
 
 	pm_runtime_put(&adev->dev);
 
 	return 0;
 
- err_add_adap:
-	clk_put(dev->clk);
- err_no_clk:
-	free_irq(dev->irq, dev);
- err_irq:
-	iounmap(dev->virtbase);
- err_no_ioremap:
-	kfree(dev);
  err_no_mem:
 
 	return ret;
@@ -1084,13 +1077,9 @@ static int nmk_i2c_remove(struct amba_device *adev)
 	clear_all_interrupts(dev);
 	/* disable the controller */
 	i2c_clr_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
-	free_irq(dev->irq, dev);
-	iounmap(dev->virtbase);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
-	clk_put(dev->clk);
 	pm_runtime_disable(&adev->dev);
-	kfree(dev);
 
 	return 0;
 }
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
@ 2014-02-13 14:09 ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Alessandro Rubini, Linus Walleij, Wolfram Sang, Chris Ball,
	Mark Brown, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson

Use devm_* functions to simplify code and error handling.

Cc: Alessandro Rubini <rubini-9wsNiZum9E8@public.gmane.org>
Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |   29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index bc8ba02..5faf9e2 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -981,7 +981,7 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 		return -ENODEV;
 	}
 
-	dev = kzalloc(sizeof(struct nmk_i2c_dev), GFP_KERNEL);
+	dev = devm_kzalloc(&adev->dev, sizeof(struct nmk_i2c_dev), GFP_KERNEL);
 	if (!dev) {
 		dev_err(&adev->dev, "cannot allocate memory\n");
 		ret = -ENOMEM;
@@ -1011,27 +1011,28 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	/* If possible, let's go to idle until the first transfer */
 	pinctrl_pm_select_idle_state(&adev->dev);
 
-	dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
+	dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
+				resource_size(&adev->res));
 	if (!dev->virtbase) {
 		ret = -ENOMEM;
-		goto err_no_ioremap;
+		goto err_no_mem;
 	}
 
 	dev->irq = adev->irq[0];
-	ret = request_irq(dev->irq, i2c_irq_handler, 0,
+	ret = devm_request_irq(&adev->dev, dev->irq, i2c_irq_handler, 0,
 				DRIVER_NAME, dev);
 	if (ret) {
 		dev_err(&adev->dev, "cannot claim the irq %d\n", dev->irq);
-		goto err_irq;
+		goto err_no_mem;
 	}
 
 	pm_suspend_ignore_children(&adev->dev, true);
 
-	dev->clk = clk_get(&adev->dev, NULL);
+	dev->clk = devm_clk_get(&adev->dev, NULL);
 	if (IS_ERR(dev->clk)) {
 		dev_err(&adev->dev, "could not get i2c clock\n");
 		ret = PTR_ERR(dev->clk);
-		goto err_no_clk;
+		goto err_no_mem;
 	}
 
 	adap = &dev->adap;
@@ -1053,21 +1054,13 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	ret = i2c_add_adapter(adap);
 	if (ret) {
 		dev_err(&adev->dev, "failed to add adapter\n");
-		goto err_add_adap;
+		goto err_no_mem;
 	}
 
 	pm_runtime_put(&adev->dev);
 
 	return 0;
 
- err_add_adap:
-	clk_put(dev->clk);
- err_no_clk:
-	free_irq(dev->irq, dev);
- err_irq:
-	iounmap(dev->virtbase);
- err_no_ioremap:
-	kfree(dev);
  err_no_mem:
 
 	return ret;
@@ -1084,13 +1077,9 @@ static int nmk_i2c_remove(struct amba_device *adev)
 	clear_all_interrupts(dev);
 	/* disable the controller */
 	i2c_clr_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
-	free_irq(dev->irq, dev);
-	iounmap(dev->virtbase);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
-	clk_put(dev->clk);
 	pm_runtime_disable(&adev->dev);
-	kfree(dev);
 
 	return 0;
 }
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
@ 2014-02-13 14:09 ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: linux-arm-kernel

Use devm_* functions to simplify code and error handling.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |   29 +++++++++--------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index bc8ba02..5faf9e2 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -981,7 +981,7 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 		return -ENODEV;
 	}
 
-	dev = kzalloc(sizeof(struct nmk_i2c_dev), GFP_KERNEL);
+	dev = devm_kzalloc(&adev->dev, sizeof(struct nmk_i2c_dev), GFP_KERNEL);
 	if (!dev) {
 		dev_err(&adev->dev, "cannot allocate memory\n");
 		ret = -ENOMEM;
@@ -1011,27 +1011,28 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	/* If possible, let's go to idle until the first transfer */
 	pinctrl_pm_select_idle_state(&adev->dev);
 
-	dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
+	dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
+				resource_size(&adev->res));
 	if (!dev->virtbase) {
 		ret = -ENOMEM;
-		goto err_no_ioremap;
+		goto err_no_mem;
 	}
 
 	dev->irq = adev->irq[0];
-	ret = request_irq(dev->irq, i2c_irq_handler, 0,
+	ret = devm_request_irq(&adev->dev, dev->irq, i2c_irq_handler, 0,
 				DRIVER_NAME, dev);
 	if (ret) {
 		dev_err(&adev->dev, "cannot claim the irq %d\n", dev->irq);
-		goto err_irq;
+		goto err_no_mem;
 	}
 
 	pm_suspend_ignore_children(&adev->dev, true);
 
-	dev->clk = clk_get(&adev->dev, NULL);
+	dev->clk = devm_clk_get(&adev->dev, NULL);
 	if (IS_ERR(dev->clk)) {
 		dev_err(&adev->dev, "could not get i2c clock\n");
 		ret = PTR_ERR(dev->clk);
-		goto err_no_clk;
+		goto err_no_mem;
 	}
 
 	adap = &dev->adap;
@@ -1053,21 +1054,13 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	ret = i2c_add_adapter(adap);
 	if (ret) {
 		dev_err(&adev->dev, "failed to add adapter\n");
-		goto err_add_adap;
+		goto err_no_mem;
 	}
 
 	pm_runtime_put(&adev->dev);
 
 	return 0;
 
- err_add_adap:
-	clk_put(dev->clk);
- err_no_clk:
-	free_irq(dev->irq, dev);
- err_irq:
-	iounmap(dev->virtbase);
- err_no_ioremap:
-	kfree(dev);
  err_no_mem:
 
 	return ret;
@@ -1084,13 +1077,9 @@ static int nmk_i2c_remove(struct amba_device *adev)
 	clear_all_interrupts(dev);
 	/* disable the controller */
 	i2c_clr_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
-	free_irq(dev->irq, dev);
-	iounmap(dev->virtbase);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
-	clk_put(dev->clk);
 	pm_runtime_disable(&adev->dev);
-	kfree(dev);
 
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 12/17] i2c: nomadik: Remove redundant call to pm_runtime_disable
@ 2014-02-13 14:09   ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Alessandro Rubini, Linus Walleij, Wolfram Sang, Chris Ball,
	Mark Brown, linux-kernel, linux-i2c, linux-spi, linux-mmc,
	Ulf Hansson

The amba bus are responsible for pm_runtime_enable|disable, remove the
redundant pm_runtime_disable at driver removal.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 5faf9e2..d800d0f 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -1079,7 +1079,6 @@ static int nmk_i2c_remove(struct amba_device *adev)
 	i2c_clr_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
-	pm_runtime_disable(&adev->dev);
 
 	return 0;
 }
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 12/17] i2c: nomadik: Remove redundant call to pm_runtime_disable
@ 2014-02-13 14:09   ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: Alessandro Rubini, Linus Walleij, Wolfram Sang, Chris Ball,
	Mark Brown, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA, Ulf Hansson

The amba bus are responsible for pm_runtime_enable|disable, remove the
redundant pm_runtime_disable at driver removal.

Cc: Alessandro Rubini <rubini-9wsNiZum9E8@public.gmane.org>
Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 5faf9e2..d800d0f 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -1079,7 +1079,6 @@ static int nmk_i2c_remove(struct amba_device *adev)
 	i2c_clr_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
-	pm_runtime_disable(&adev->dev);
 
 	return 0;
 }
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 12/17] i2c: nomadik: Remove redundant call to pm_runtime_disable
@ 2014-02-13 14:09   ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: linux-arm-kernel

The amba bus are responsible for pm_runtime_enable|disable, remove the
redundant pm_runtime_disable at driver removal.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 5faf9e2..d800d0f 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -1079,7 +1079,6 @@ static int nmk_i2c_remove(struct amba_device *adev)
 	i2c_clr_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
-	pm_runtime_disable(&adev->dev);
 
 	return 0;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 14/17] i2c: nomadik: Fixup deployment of runtime PM
  2014-02-13 14:09 ` Ulf Hansson
@ 2014-02-13 14:09   ` Ulf Hansson
  -1 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Alessandro Rubini, Linus Walleij, Wolfram Sang, Chris Ball,
	Mark Brown, linux-kernel, linux-i2c, linux-spi, linux-mmc,
	Ulf Hansson

Since the runtime PM state is expected to be active according to the
amba bus, we must align our behaviour while probing to it.

Moreover, this is needed to be able to have the driver fully functional
without depending on CONFIG_RUNTIME_PM.

Since the device is active while a successful probe has been completed,
the reference counting for the clock will be screwed up and never reach
zero. We resolve this by implementing runtime PM callbacks and let them
handle the resources accordingly, including the clock.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.
	Squashed with PATCH 13/17 from previous version.
	Add error handling to align to previous behavior.

---
 drivers/i2c/busses/i2c-nomadik.c |   77 +++++++++++++++++++++++++-------------
 1 file changed, 51 insertions(+), 26 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index d800d0f..01b78df 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -666,7 +666,7 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *dev, u16 flags)
 static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 		struct i2c_msg msgs[], int num_msgs)
 {
-	int status;
+	int status = 0;
 	int i;
 	struct nmk_i2c_dev *dev = i2c_get_adapdata(i2c_adap);
 	int j;
@@ -675,19 +675,6 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 
 	pm_runtime_get_sync(&dev->adev->dev);
 
-	status = clk_prepare_enable(dev->clk);
-	if (status) {
-		dev_err(&dev->adev->dev, "can't prepare_enable clock\n");
-		goto out_clk;
-	}
-
-	/* Optionaly enable pins to be muxed in and configured */
-	pinctrl_pm_select_default_state(&dev->adev->dev);
-
-	status = init_hw(dev);
-	if (status)
-		goto out;
-
 	/* Attempt three times to send the message queue */
 	for (j = 0; j < 3; j++) {
 		/* setup the i2c controller */
@@ -708,12 +695,6 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 			break;
 	}
 
-out:
-	clk_disable_unprepare(dev->clk);
-out_clk:
-	/* Optionally let pins go into idle state */
-	pinctrl_pm_select_idle_state(&dev->adev->dev);
-
 	pm_runtime_put_sync(&dev->adev->dev);
 
 	dev->busy = false;
@@ -930,6 +911,41 @@ static int nmk_i2c_resume(struct device *dev)
 #define nmk_i2c_resume	NULL
 #endif
 
+#if CONFIG_PM
+static int nmk_i2c_runtime_suspend(struct device *dev)
+{
+	struct amba_device *adev = to_amba_device(dev);
+	struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+
+	clk_disable_unprepare(nmk_i2c->clk);
+	pinctrl_pm_select_idle_state(dev);
+	return 0;
+}
+
+static int nmk_i2c_runtime_resume(struct device *dev)
+{
+	struct amba_device *adev = to_amba_device(dev);
+	struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+	int ret;
+
+	ret = clk_prepare_enable(nmk_i2c->clk);
+	if (ret) {
+		dev_err(dev, "can't prepare_enable clock\n");
+		return ret;
+	}
+
+	pinctrl_pm_select_default_state(dev);
+
+	ret = init_hw(nmk_i2c);
+	if (ret) {
+		clk_disable_unprepare(nmk_i2c->clk);
+		pinctrl_pm_select_idle_state(dev);
+	}
+
+	return ret;
+}
+#endif
+
 /*
  * We use noirq so that we suspend late and resume before the wakeup interrupt
  * to ensure that we do the !pm_runtime_suspended() check in resume before
@@ -938,6 +954,9 @@ static int nmk_i2c_resume(struct device *dev)
 static const struct dev_pm_ops nmk_i2c_pm = {
 	.suspend_noirq	= nmk_i2c_suspend,
 	.resume_noirq	= nmk_i2c_resume,
+	SET_PM_RUNTIME_PM_OPS(nmk_i2c_runtime_suspend,
+			nmk_i2c_runtime_resume,
+			NULL)
 };
 
 static unsigned int nmk_i2c_functionality(struct i2c_adapter *adap)
@@ -1006,11 +1025,6 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 
 	amba_set_drvdata(adev, dev);
 
-	/* Select default pin state */
-	pinctrl_pm_select_default_state(&adev->dev);
-	/* If possible, let's go to idle until the first transfer */
-	pinctrl_pm_select_idle_state(&adev->dev);
-
 	dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
 				resource_size(&adev->res));
 	if (!dev->virtbase) {
@@ -1035,6 +1049,14 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 		goto err_no_mem;
 	}
 
+	ret = clk_prepare_enable(dev->clk);
+	if (ret) {
+		dev_err(&adev->dev, "can't prepare_enable clock\n");
+		goto err_no_mem;
+	}
+
+	init_hw(dev);
+
 	adap = &dev->adap;
 	adap->dev.of_node = np;
 	adap->dev.parent = &adev->dev;
@@ -1054,13 +1076,15 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	ret = i2c_add_adapter(adap);
 	if (ret) {
 		dev_err(&adev->dev, "failed to add adapter\n");
-		goto err_no_mem;
+		goto err_no_adap;
 	}
 
 	pm_runtime_put(&adev->dev);
 
 	return 0;
 
+ err_no_adap:
+	clk_disable_unprepare(dev->clk);
  err_no_mem:
 
 	return ret;
@@ -1077,6 +1101,7 @@ static int nmk_i2c_remove(struct amba_device *adev)
 	clear_all_interrupts(dev);
 	/* disable the controller */
 	i2c_clr_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
+	clk_disable_unprepare(dev->clk);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 14/17] i2c: nomadik: Fixup deployment of runtime PM
@ 2014-02-13 14:09   ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: linux-arm-kernel

Since the runtime PM state is expected to be active according to the
amba bus, we must align our behaviour while probing to it.

Moreover, this is needed to be able to have the driver fully functional
without depending on CONFIG_RUNTIME_PM.

Since the device is active while a successful probe has been completed,
the reference counting for the clock will be screwed up and never reach
zero. We resolve this by implementing runtime PM callbacks and let them
handle the resources accordingly, including the clock.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.
	Squashed with PATCH 13/17 from previous version.
	Add error handling to align to previous behavior.

---
 drivers/i2c/busses/i2c-nomadik.c |   77 +++++++++++++++++++++++++-------------
 1 file changed, 51 insertions(+), 26 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index d800d0f..01b78df 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -666,7 +666,7 @@ static int nmk_i2c_xfer_one(struct nmk_i2c_dev *dev, u16 flags)
 static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 		struct i2c_msg msgs[], int num_msgs)
 {
-	int status;
+	int status = 0;
 	int i;
 	struct nmk_i2c_dev *dev = i2c_get_adapdata(i2c_adap);
 	int j;
@@ -675,19 +675,6 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 
 	pm_runtime_get_sync(&dev->adev->dev);
 
-	status = clk_prepare_enable(dev->clk);
-	if (status) {
-		dev_err(&dev->adev->dev, "can't prepare_enable clock\n");
-		goto out_clk;
-	}
-
-	/* Optionaly enable pins to be muxed in and configured */
-	pinctrl_pm_select_default_state(&dev->adev->dev);
-
-	status = init_hw(dev);
-	if (status)
-		goto out;
-
 	/* Attempt three times to send the message queue */
 	for (j = 0; j < 3; j++) {
 		/* setup the i2c controller */
@@ -708,12 +695,6 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 			break;
 	}
 
-out:
-	clk_disable_unprepare(dev->clk);
-out_clk:
-	/* Optionally let pins go into idle state */
-	pinctrl_pm_select_idle_state(&dev->adev->dev);
-
 	pm_runtime_put_sync(&dev->adev->dev);
 
 	dev->busy = false;
@@ -930,6 +911,41 @@ static int nmk_i2c_resume(struct device *dev)
 #define nmk_i2c_resume	NULL
 #endif
 
+#if CONFIG_PM
+static int nmk_i2c_runtime_suspend(struct device *dev)
+{
+	struct amba_device *adev = to_amba_device(dev);
+	struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+
+	clk_disable_unprepare(nmk_i2c->clk);
+	pinctrl_pm_select_idle_state(dev);
+	return 0;
+}
+
+static int nmk_i2c_runtime_resume(struct device *dev)
+{
+	struct amba_device *adev = to_amba_device(dev);
+	struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+	int ret;
+
+	ret = clk_prepare_enable(nmk_i2c->clk);
+	if (ret) {
+		dev_err(dev, "can't prepare_enable clock\n");
+		return ret;
+	}
+
+	pinctrl_pm_select_default_state(dev);
+
+	ret = init_hw(nmk_i2c);
+	if (ret) {
+		clk_disable_unprepare(nmk_i2c->clk);
+		pinctrl_pm_select_idle_state(dev);
+	}
+
+	return ret;
+}
+#endif
+
 /*
  * We use noirq so that we suspend late and resume before the wakeup interrupt
  * to ensure that we do the !pm_runtime_suspended() check in resume before
@@ -938,6 +954,9 @@ static int nmk_i2c_resume(struct device *dev)
 static const struct dev_pm_ops nmk_i2c_pm = {
 	.suspend_noirq	= nmk_i2c_suspend,
 	.resume_noirq	= nmk_i2c_resume,
+	SET_PM_RUNTIME_PM_OPS(nmk_i2c_runtime_suspend,
+			nmk_i2c_runtime_resume,
+			NULL)
 };
 
 static unsigned int nmk_i2c_functionality(struct i2c_adapter *adap)
@@ -1006,11 +1025,6 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 
 	amba_set_drvdata(adev, dev);
 
-	/* Select default pin state */
-	pinctrl_pm_select_default_state(&adev->dev);
-	/* If possible, let's go to idle until the first transfer */
-	pinctrl_pm_select_idle_state(&adev->dev);
-
 	dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
 				resource_size(&adev->res));
 	if (!dev->virtbase) {
@@ -1035,6 +1049,14 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 		goto err_no_mem;
 	}
 
+	ret = clk_prepare_enable(dev->clk);
+	if (ret) {
+		dev_err(&adev->dev, "can't prepare_enable clock\n");
+		goto err_no_mem;
+	}
+
+	init_hw(dev);
+
 	adap = &dev->adap;
 	adap->dev.of_node = np;
 	adap->dev.parent = &adev->dev;
@@ -1054,13 +1076,15 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 	ret = i2c_add_adapter(adap);
 	if (ret) {
 		dev_err(&adev->dev, "failed to add adapter\n");
-		goto err_no_mem;
+		goto err_no_adap;
 	}
 
 	pm_runtime_put(&adev->dev);
 
 	return 0;
 
+ err_no_adap:
+	clk_disable_unprepare(dev->clk);
  err_no_mem:
 
 	return ret;
@@ -1077,6 +1101,7 @@ static int nmk_i2c_remove(struct amba_device *adev)
 	clear_all_interrupts(dev);
 	/* disable the controller */
 	i2c_clr_bit(dev->virtbase + I2C_CR, I2C_CR_PE);
+	clk_disable_unprepare(dev->clk);
 	if (res)
 		release_mem_region(res->start, resource_size(res));
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 15/17] i2c: nomadik: Convert to late and early system PM callbacks
  2014-02-13 14:09 ` Ulf Hansson
@ 2014-02-13 14:09   ` Ulf Hansson
  -1 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Alessandro Rubini, Linus Walleij, Wolfram Sang, Chris Ball,
	Mark Brown, linux-kernel, linux-i2c, linux-spi, linux-mmc,
	Ulf Hansson

At system suspend_late, runtime PM has been disabled by the PM core
which means we can safely operate on these resources. Consequentially
we no longer have to wait until the noirq phase of the system suspend.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |   18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 01b78df..80b0fd3 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -882,9 +882,8 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
-
-#ifdef CONFIG_PM
-static int nmk_i2c_suspend(struct device *dev)
+#ifdef CONFIG_PM_SLEEP
+static int nmk_i2c_suspend_late(struct device *dev)
 {
 	struct amba_device *adev = to_amba_device(dev);
 	struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
@@ -897,7 +896,7 @@ static int nmk_i2c_suspend(struct device *dev)
 	return 0;
 }
 
-static int nmk_i2c_resume(struct device *dev)
+static int nmk_i2c_resume_early(struct device *dev)
 {
 	/* First go to the default state */
 	pinctrl_pm_select_default_state(dev);
@@ -906,9 +905,6 @@ static int nmk_i2c_resume(struct device *dev)
 
 	return 0;
 }
-#else
-#define nmk_i2c_suspend	NULL
-#define nmk_i2c_resume	NULL
 #endif
 
 #if CONFIG_PM
@@ -946,14 +942,8 @@ static int nmk_i2c_runtime_resume(struct device *dev)
 }
 #endif
 
-/*
- * We use noirq so that we suspend late and resume before the wakeup interrupt
- * to ensure that we do the !pm_runtime_suspended() check in resume before
- * there has been a regular pm runtime resume (via pm_runtime_get_sync()).
- */
 static const struct dev_pm_ops nmk_i2c_pm = {
-	.suspend_noirq	= nmk_i2c_suspend,
-	.resume_noirq	= nmk_i2c_resume,
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(nmk_i2c_suspend_late, nmk_i2c_resume_early)
 	SET_PM_RUNTIME_PM_OPS(nmk_i2c_runtime_suspend,
 			nmk_i2c_runtime_resume,
 			NULL)
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 15/17] i2c: nomadik: Convert to late and early system PM callbacks
@ 2014-02-13 14:09   ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: linux-arm-kernel

At system suspend_late, runtime PM has been disabled by the PM core
which means we can safely operate on these resources. Consequentially
we no longer have to wait until the noirq phase of the system suspend.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |   18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 01b78df..80b0fd3 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -882,9 +882,8 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
-
-#ifdef CONFIG_PM
-static int nmk_i2c_suspend(struct device *dev)
+#ifdef CONFIG_PM_SLEEP
+static int nmk_i2c_suspend_late(struct device *dev)
 {
 	struct amba_device *adev = to_amba_device(dev);
 	struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
@@ -897,7 +896,7 @@ static int nmk_i2c_suspend(struct device *dev)
 	return 0;
 }
 
-static int nmk_i2c_resume(struct device *dev)
+static int nmk_i2c_resume_early(struct device *dev)
 {
 	/* First go to the default state */
 	pinctrl_pm_select_default_state(dev);
@@ -906,9 +905,6 @@ static int nmk_i2c_resume(struct device *dev)
 
 	return 0;
 }
-#else
-#define nmk_i2c_suspend	NULL
-#define nmk_i2c_resume	NULL
 #endif
 
 #if CONFIG_PM
@@ -946,14 +942,8 @@ static int nmk_i2c_runtime_resume(struct device *dev)
 }
 #endif
 
-/*
- * We use noirq so that we suspend late and resume before the wakeup interrupt
- * to ensure that we do the !pm_runtime_suspended() check in resume before
- * there has been a regular pm runtime resume (via pm_runtime_get_sync()).
- */
 static const struct dev_pm_ops nmk_i2c_pm = {
-	.suspend_noirq	= nmk_i2c_suspend,
-	.resume_noirq	= nmk_i2c_resume,
+	SET_LATE_SYSTEM_SLEEP_PM_OPS(nmk_i2c_suspend_late, nmk_i2c_resume_early)
 	SET_PM_RUNTIME_PM_OPS(nmk_i2c_runtime_suspend,
 			nmk_i2c_runtime_resume,
 			NULL)
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 16/17] i2c: nomadik: Remove busy check for transfers at suspend late
  2014-02-13 14:09 ` Ulf Hansson
@ 2014-02-13 14:09   ` Ulf Hansson
  -1 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Alessandro Rubini, Linus Walleij, Wolfram Sang, Chris Ball,
	Mark Brown, linux-kernel, linux-i2c, linux-spi, linux-mmc,
	Ulf Hansson

We should never be busy performing transfers at suspend late, thus
there are no reason to check for it.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 80b0fd3..8a5dc57b 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -167,7 +167,6 @@ struct i2c_nmk_client {
  * @stop: stop condition.
  * @xfer_complete: acknowledge completion for a I2C message.
  * @result: controller propogated result.
- * @busy: Busy doing transfer.
  */
 struct nmk_i2c_dev {
 	struct i2c_vendor_data		*vendor;
@@ -185,7 +184,6 @@ struct nmk_i2c_dev {
 	int				stop;
 	struct completion		xfer_complete;
 	int				result;
-	bool				busy;
 };
 
 /* controller's abort causes */
@@ -671,8 +669,6 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 	struct nmk_i2c_dev *dev = i2c_get_adapdata(i2c_adap);
 	int j;
 
-	dev->busy = true;
-
 	pm_runtime_get_sync(&dev->adev->dev);
 
 	/* Attempt three times to send the message queue */
@@ -697,8 +693,6 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 
 	pm_runtime_put_sync(&dev->adev->dev);
 
-	dev->busy = false;
-
 	/* return the no. messages processed */
 	if (status)
 		return status;
@@ -888,9 +882,6 @@ static int nmk_i2c_suspend_late(struct device *dev)
 	struct amba_device *adev = to_amba_device(dev);
 	struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
 
-	if (nmk_i2c->busy)
-		return -EBUSY;
-
 	pinctrl_pm_select_sleep_state(dev);
 
 	return 0;
@@ -997,7 +988,6 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 		goto err_no_mem;
 	}
 	dev->vendor = vendor;
-	dev->busy = false;
 	dev->adev = adev;
 	nmk_i2c_of_probe(np, dev);
 
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 16/17] i2c: nomadik: Remove busy check for transfers at suspend late
@ 2014-02-13 14:09   ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: linux-arm-kernel

We should never be busy performing transfers at suspend late, thus
there are no reason to check for it.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 80b0fd3..8a5dc57b 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -167,7 +167,6 @@ struct i2c_nmk_client {
  * @stop: stop condition.
  * @xfer_complete: acknowledge completion for a I2C message.
  * @result: controller propogated result.
- * @busy: Busy doing transfer.
  */
 struct nmk_i2c_dev {
 	struct i2c_vendor_data		*vendor;
@@ -185,7 +184,6 @@ struct nmk_i2c_dev {
 	int				stop;
 	struct completion		xfer_complete;
 	int				result;
-	bool				busy;
 };
 
 /* controller's abort causes */
@@ -671,8 +669,6 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 	struct nmk_i2c_dev *dev = i2c_get_adapdata(i2c_adap);
 	int j;
 
-	dev->busy = true;
-
 	pm_runtime_get_sync(&dev->adev->dev);
 
 	/* Attempt three times to send the message queue */
@@ -697,8 +693,6 @@ static int nmk_i2c_xfer(struct i2c_adapter *i2c_adap,
 
 	pm_runtime_put_sync(&dev->adev->dev);
 
-	dev->busy = false;
-
 	/* return the no. messages processed */
 	if (status)
 		return status;
@@ -888,9 +882,6 @@ static int nmk_i2c_suspend_late(struct device *dev)
 	struct amba_device *adev = to_amba_device(dev);
 	struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
 
-	if (nmk_i2c->busy)
-		return -EBUSY;
-
 	pinctrl_pm_select_sleep_state(dev);
 
 	return 0;
@@ -997,7 +988,6 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 		goto err_no_mem;
 	}
 	dev->vendor = vendor;
-	dev->busy = false;
 	dev->adev = adev;
 	nmk_i2c_of_probe(np, dev);
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 17/17] i2c: nomadik: Fixup system suspend
  2014-02-13 14:09 ` Ulf Hansson
@ 2014-02-13 14:09   ` Ulf Hansson
  -1 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: Russell King, linux-arm-kernel
  Cc: Alessandro Rubini, Linus Walleij, Wolfram Sang, Chris Ball,
	Mark Brown, linux-kernel, linux-i2c, linux-spi, linux-mmc,
	Ulf Hansson

For !CONFIG_PM_RUNTIME, the device were never put back into active
state while resuming.

For CONFIG_PM_RUNTIME, we blindly trusted the device to be inactive
while we were about to handle it at suspend late, which is just too
optimistic.

Even if the driver uses pm_runtime_put_sync() after each tranfer to
return it's runtime PM resources, there are no guarantees this will
actually mean the device will inactivated. The reason is that the PM
core will prevent runtime suspend during system suspend, and thus when
a transfer occurs during the early phases of system suspend the device
will be kept active after the transfer.

To handle both issues above, we need to re-use the runtime PM callbacks
and check the runtime PM state of the device before proceeding with our
operations for system suspend.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |   29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 8a5dc57b..21539f2 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -879,21 +879,36 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
 #ifdef CONFIG_PM_SLEEP
 static int nmk_i2c_suspend_late(struct device *dev)
 {
-	struct amba_device *adev = to_amba_device(dev);
-	struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+	if (!pm_runtime_status_suspended(dev)) {
+		int ret = 0;
+		if (dev->pm_domain && dev->pm_domain->ops.runtime_suspend)
+			ret = dev->pm_domain->ops.runtime_suspend(dev);
+		else
+			ret = dev->bus->pm->runtime_suspend(dev);
+		if (ret)
+			return ret;
+
+		pm_runtime_set_suspended(dev);
+	}
 
 	pinctrl_pm_select_sleep_state(dev);
-
 	return 0;
 }
 
 static int nmk_i2c_resume_early(struct device *dev)
 {
-	/* First go to the default state */
-	pinctrl_pm_select_default_state(dev);
-	/* Then let's idle the pins until the next transfer happens */
-	pinctrl_pm_select_idle_state(dev);
+	int ret = 0;
+
+	if (dev->pm_domain && dev->pm_domain->ops.runtime_resume)
+		ret = dev->pm_domain->ops.runtime_resume(dev);
+	else
+		ret = dev->bus->pm->runtime_resume(dev);
+	if (ret) {
+		dev_err(dev, "problem resuming\n");
+		return ret;
+	}
 
+	pm_runtime_set_active(dev);
 	return 0;
 }
 #endif
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH V2 17/17] i2c: nomadik: Fixup system suspend
@ 2014-02-13 14:09   ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-13 14:09 UTC (permalink / raw)
  To: linux-arm-kernel

For !CONFIG_PM_RUNTIME, the device were never put back into active
state while resuming.

For CONFIG_PM_RUNTIME, we blindly trusted the device to be inactive
while we were about to handle it at suspend late, which is just too
optimistic.

Even if the driver uses pm_runtime_put_sync() after each tranfer to
return it's runtime PM resources, there are no guarantees this will
actually mean the device will inactivated. The reason is that the PM
core will prevent runtime suspend during system suspend, and thus when
a transfer occurs during the early phases of system suspend the device
will be kept active after the transfer.

To handle both issues above, we need to re-use the runtime PM callbacks
and check the runtime PM state of the device before proceeding with our
operations for system suspend.

Cc: Alessandro Rubini <rubini@unipv.it>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Wolfram Sang <wsa@the-dreams.de>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

Changes in v2:
	Rebased on top of latest i2c-nomadik branch.

---
 drivers/i2c/busses/i2c-nomadik.c |   29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 8a5dc57b..21539f2 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -879,21 +879,36 @@ static irqreturn_t i2c_irq_handler(int irq, void *arg)
 #ifdef CONFIG_PM_SLEEP
 static int nmk_i2c_suspend_late(struct device *dev)
 {
-	struct amba_device *adev = to_amba_device(dev);
-	struct nmk_i2c_dev *nmk_i2c = amba_get_drvdata(adev);
+	if (!pm_runtime_status_suspended(dev)) {
+		int ret = 0;
+		if (dev->pm_domain && dev->pm_domain->ops.runtime_suspend)
+			ret = dev->pm_domain->ops.runtime_suspend(dev);
+		else
+			ret = dev->bus->pm->runtime_suspend(dev);
+		if (ret)
+			return ret;
+
+		pm_runtime_set_suspended(dev);
+	}
 
 	pinctrl_pm_select_sleep_state(dev);
-
 	return 0;
 }
 
 static int nmk_i2c_resume_early(struct device *dev)
 {
-	/* First go to the default state */
-	pinctrl_pm_select_default_state(dev);
-	/* Then let's idle the pins until the next transfer happens */
-	pinctrl_pm_select_idle_state(dev);
+	int ret = 0;
+
+	if (dev->pm_domain && dev->pm_domain->ops.runtime_resume)
+		ret = dev->pm_domain->ops.runtime_resume(dev);
+	else
+		ret = dev->bus->pm->runtime_resume(dev);
+	if (ret) {
+		dev_err(dev, "problem resuming\n");
+		return ret;
+	}
 
+	pm_runtime_set_active(dev);
 	return 0;
 }
 #endif
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
  2014-02-13 14:09 ` Ulf Hansson
@ 2014-02-15 15:03   ` Wolfram Sang
  -1 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2014-02-15 15:03 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Russell King, linux-arm-kernel, Alessandro Rubini, Linus Walleij,
	Chris Ball, Mark Brown, linux-kernel, linux-i2c, linux-spi,
	linux-mmc

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

On Thu, Feb 13, 2014 at 03:09:02PM +0100, Ulf Hansson wrote:
> Use devm_* functions to simplify code and error handling.
> 
> Cc: Alessandro Rubini <rubini@unipv.it>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	Rebased on top of latest i2c-nomadik branch.

Since this depends on Linus' patch already, I think it would be cleaner
if I pick this kinda unrelated (but wanted) devm patch, and ack the
PM stuff. If this for some reason makes things more complicated, I can
also simply ack this one.

> -	dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
> +	dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
> +				resource_size(&adev->res));
>  	if (!dev->virtbase) {
>  		ret = -ENOMEM;

IS_ERR()!


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
@ 2014-02-15 15:03   ` Wolfram Sang
  0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2014-02-15 15:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Feb 13, 2014 at 03:09:02PM +0100, Ulf Hansson wrote:
> Use devm_* functions to simplify code and error handling.
> 
> Cc: Alessandro Rubini <rubini@unipv.it>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> Changes in v2:
> 	Rebased on top of latest i2c-nomadik branch.

Since this depends on Linus' patch already, I think it would be cleaner
if I pick this kinda unrelated (but wanted) devm patch, and ack the
PM stuff. If this for some reason makes things more complicated, I can
also simply ack this one.

> -	dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
> +	dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
> +				resource_size(&adev->res));
>  	if (!dev->virtbase) {
>  		ret = -ENOMEM;

IS_ERR()!

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140215/ba91bab3/attachment.sig>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
  2014-02-15 15:03   ` Wolfram Sang
  (?)
@ 2014-02-16 13:44     ` Ulf Hansson
  -1 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-16 13:44 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Russell King, linux-arm-kernel, Alessandro Rubini, Linus Walleij,
	Chris Ball, Mark Brown, linux-kernel, linux-i2c, linux-spi,
	linux-mmc

On 15 February 2014 16:03, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Thu, Feb 13, 2014 at 03:09:02PM +0100, Ulf Hansson wrote:
>> Use devm_* functions to simplify code and error handling.
>>
>> Cc: Alessandro Rubini <rubini@unipv.it>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Wolfram Sang <wsa@the-dreams.de>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>
>> Changes in v2:
>>       Rebased on top of latest i2c-nomadik branch.
>
> Since this depends on Linus' patch already, I think it would be cleaner
> if I pick this kinda unrelated (but wanted) devm patch, and ack the
> PM stuff. If this for some reason makes things more complicated, I can
> also simply ack this one.

I think we will end up having merge conflicts, especially for the
changes in the probe function if we decide to split it.

Would a way forward be to let you carry all the patches through your
tree? I believe all but patch 17 can be safely merged. It is only this
one that depends on the changes in the amba bus, so we can put this
one on hold for a while.

Another option would be if you drop Linus' patch from you branch and
let him collect all the patches in a pull request instead?

I happy with whatever we thinks are easiest. :-)

>
>> -     dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
>> +     dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
>> +                             resource_size(&adev->res));
>>       if (!dev->virtbase) {
>>               ret = -ENOMEM;
>
> IS_ERR()!

Will fix in a v2!

>

Thanks!

Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
@ 2014-02-16 13:44     ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-16 13:44 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Russell King, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Alessandro Rubini, Linus Walleij, Chris Ball, Mark Brown,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-mmc

On 15 February 2014 16:03, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> On Thu, Feb 13, 2014 at 03:09:02PM +0100, Ulf Hansson wrote:
>> Use devm_* functions to simplify code and error handling.
>>
>> Cc: Alessandro Rubini <rubini-9wsNiZum9E8@public.gmane.org>
>> Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Cc: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
>> Signed-off-by: Ulf Hansson <ulf.hansson-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>
>> Changes in v2:
>>       Rebased on top of latest i2c-nomadik branch.
>
> Since this depends on Linus' patch already, I think it would be cleaner
> if I pick this kinda unrelated (but wanted) devm patch, and ack the
> PM stuff. If this for some reason makes things more complicated, I can
> also simply ack this one.

I think we will end up having merge conflicts, especially for the
changes in the probe function if we decide to split it.

Would a way forward be to let you carry all the patches through your
tree? I believe all but patch 17 can be safely merged. It is only this
one that depends on the changes in the amba bus, so we can put this
one on hold for a while.

Another option would be if you drop Linus' patch from you branch and
let him collect all the patches in a pull request instead?

I happy with whatever we thinks are easiest. :-)

>
>> -     dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
>> +     dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
>> +                             resource_size(&adev->res));
>>       if (!dev->virtbase) {
>>               ret = -ENOMEM;
>
> IS_ERR()!

Will fix in a v2!

>

Thanks!

Kind regards
Ulf Hansson
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
@ 2014-02-16 13:44     ` Ulf Hansson
  0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2014-02-16 13:44 UTC (permalink / raw)
  To: linux-arm-kernel

On 15 February 2014 16:03, Wolfram Sang <wsa@the-dreams.de> wrote:
> On Thu, Feb 13, 2014 at 03:09:02PM +0100, Ulf Hansson wrote:
>> Use devm_* functions to simplify code and error handling.
>>
>> Cc: Alessandro Rubini <rubini@unipv.it>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Wolfram Sang <wsa@the-dreams.de>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>>
>> Changes in v2:
>>       Rebased on top of latest i2c-nomadik branch.
>
> Since this depends on Linus' patch already, I think it would be cleaner
> if I pick this kinda unrelated (but wanted) devm patch, and ack the
> PM stuff. If this for some reason makes things more complicated, I can
> also simply ack this one.

I think we will end up having merge conflicts, especially for the
changes in the probe function if we decide to split it.

Would a way forward be to let you carry all the patches through your
tree? I believe all but patch 17 can be safely merged. It is only this
one that depends on the changes in the amba bus, so we can put this
one on hold for a while.

Another option would be if you drop Linus' patch from you branch and
let him collect all the patches in a pull request instead?

I happy with whatever we thinks are easiest. :-)

>
>> -     dev->virtbase = ioremap(adev->res.start, resource_size(&adev->res));
>> +     dev->virtbase = devm_ioremap(&adev->dev, adev->res.start,
>> +                             resource_size(&adev->res));
>>       if (!dev->virtbase) {
>>               ret = -ENOMEM;
>
> IS_ERR()!

Will fix in a v2!

>

Thanks!

Kind regards
Ulf Hansson

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
@ 2014-02-16 17:02       ` Wolfram Sang
  0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2014-02-16 17:02 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Russell King, linux-arm-kernel, Alessandro Rubini, Linus Walleij,
	Chris Ball, Mark Brown, linux-kernel, linux-i2c, linux-spi,
	linux-mmc

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


> Would a way forward be to let you carry all the patches through your
> tree? I believe all but patch 17 can be safely merged. It is only this
> one that depends on the changes in the amba bus, so we can put this
> one on hold for a while.

I'd favour this. Will do this next week unless somebody objects.


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
@ 2014-02-16 17:02       ` Wolfram Sang
  0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2014-02-16 17:02 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Russell King, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	Alessandro Rubini, Linus Walleij, Chris Ball, Mark Brown,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-spi-u79uwXL29TY76Z2rM5mHXA, linux-mmc

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


> Would a way forward be to let you carry all the patches through your
> tree? I believe all but patch 17 can be safely merged. It is only this
> one that depends on the changes in the amba bus, so we can put this
> one on hold for a while.

I'd favour this. Will do this next week unless somebody objects.


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH V2 11/17] i2c: nomadik: Convert to devm functions
@ 2014-02-16 17:02       ` Wolfram Sang
  0 siblings, 0 replies; 22+ messages in thread
From: Wolfram Sang @ 2014-02-16 17:02 UTC (permalink / raw)
  To: linux-arm-kernel


> Would a way forward be to let you carry all the patches through your
> tree? I believe all but patch 17 can be safely merged. It is only this
> one that depends on the changes in the amba bus, so we can put this
> one on hold for a while.

I'd favour this. Will do this next week unless somebody objects.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140216/a5322d11/attachment.sig>

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2014-02-16 17:02 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-13 14:09 [PATCH V2 11/17] i2c: nomadik: Convert to devm functions Ulf Hansson
2014-02-13 14:09 ` Ulf Hansson
2014-02-13 14:09 ` Ulf Hansson
2014-02-13 14:09 ` [PATCH V2 12/17] i2c: nomadik: Remove redundant call to pm_runtime_disable Ulf Hansson
2014-02-13 14:09   ` Ulf Hansson
2014-02-13 14:09   ` Ulf Hansson
2014-02-13 14:09 ` [PATCH V2 14/17] i2c: nomadik: Fixup deployment of runtime PM Ulf Hansson
2014-02-13 14:09   ` Ulf Hansson
2014-02-13 14:09 ` [PATCH V2 15/17] i2c: nomadik: Convert to late and early system PM callbacks Ulf Hansson
2014-02-13 14:09   ` Ulf Hansson
2014-02-13 14:09 ` [PATCH V2 16/17] i2c: nomadik: Remove busy check for transfers at suspend late Ulf Hansson
2014-02-13 14:09   ` Ulf Hansson
2014-02-13 14:09 ` [PATCH V2 17/17] i2c: nomadik: Fixup system suspend Ulf Hansson
2014-02-13 14:09   ` Ulf Hansson
2014-02-15 15:03 ` [PATCH V2 11/17] i2c: nomadik: Convert to devm functions Wolfram Sang
2014-02-15 15:03   ` Wolfram Sang
2014-02-16 13:44   ` Ulf Hansson
2014-02-16 13:44     ` Ulf Hansson
2014-02-16 13:44     ` Ulf Hansson
2014-02-16 17:02     ` Wolfram Sang
2014-02-16 17:02       ` Wolfram Sang
2014-02-16 17:02       ` Wolfram Sang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.