[PATCH 1/2] i2c: xiic: Implement Power management

[PATCH 1/2] i2c: xiic: Implement Power management

[Shubhrajyoti Datta]

Enable power management. This patch enables the clocks
before transfer and disables after the transfer.

Signed-off-by: Shubhrajyoti Datta <shubhraj@xilinx.com>
---
 drivers/i2c/busses/i2c-xiic.c |   86 +++++++++++++++++++++++++++++++++++++---
 1 files changed, 79 insertions(+), 7 deletions(-)

diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index 0b20449..b464a35 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -37,6 +37,8 @@
 #include <linux/io.h>
 #include <linux/slab.h>
 #include <linux/of.h>
+#include <linux/clk.h>
+#include <linux/pm_runtime.h>
 
 #define DRIVER_NAME "xiic-i2c"
 
@@ -66,6 +68,7 @@ enum xiic_endian {
  * @endianness: big/little-endian byte order
  */
 struct xiic_i2c {
+	struct device		*dev;
 	void __iomem		*base;
 	wait_queue_head_t	wait;
 	struct i2c_adapter	adap;
@@ -77,6 +80,7 @@ struct xiic_i2c {
 	struct i2c_msg		*rx_msg;
 	int			rx_pos;
 	enum xiic_endian	endianness;
+	struct clk *clk;
 };
 
 
@@ -164,6 +168,7 @@ struct xiic_i2c {
 
 #define XIIC_RESET_MASK             0xAUL
 
+#define XIIC_PM_TIMEOUT		1000	/* ms */
 /*
  * The following constant is used for the device global interrupt enable
  * register, to enable all interrupts for the device, this is the only bit
@@ -676,9 +681,13 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	dev_dbg(adap->dev.parent, "%s entry SR: 0x%x\n", __func__,
 		xiic_getreg8(i2c, XIIC_SR_REG_OFFSET));
 
+	err = pm_runtime_get_sync(i2c->dev);
+	if (err < 0)
+		return err;
+
 	err = xiic_busy(i2c);
 	if (err)
-		return err;
+		goto out;
 
 	i2c->tx_msg = msgs;
 	i2c->nmsgs = num;
@@ -686,14 +695,20 @@ static int xiic_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
 	xiic_start_xfer(i2c);
 
 	if (wait_event_timeout(i2c->wait, (i2c->state == STATE_ERROR) ||
-		(i2c->state == STATE_DONE), HZ))
-		return (i2c->state == STATE_DONE) ? num : -EIO;
-	else {
+		(i2c->state == STATE_DONE), HZ)) {
+		err = (i2c->state == STATE_DONE) ? num : -EIO;
+		goto out;
+	} else {
 		i2c->tx_msg = NULL;
 		i2c->rx_msg = NULL;
 		i2c->nmsgs = 0;
-		return -ETIMEDOUT;
+		err = -ETIMEDOUT;
+		goto out;
 	}
+out:
+	pm_runtime_mark_last_busy(i2c->dev);
+	pm_runtime_put_autosuspend(i2c->dev);
+	return err;
 }
 
 static u32 xiic_func(struct i2c_adapter *adap)
@@ -748,13 +763,26 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	spin_lock_init(&i2c->lock);
 	init_waitqueue_head(&i2c->wait);
 
+	i2c->clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(i2c->clk)) {
+		dev_err(&pdev->dev, "input clock not found.\n");
+		return PTR_ERR(i2c->clk);
+	}
+	ret = clk_prepare_enable(i2c->clk);
+	if (ret)
+		dev_err(&pdev->dev, "Unable to enable clock.\n");
+
+	pm_runtime_enable(i2c->dev);
+	pm_runtime_set_autosuspend_delay(i2c->dev, XIIC_PM_TIMEOUT);
+	pm_runtime_use_autosuspend(i2c->dev);
+	pm_runtime_set_active(i2c->dev);
 	ret = devm_request_threaded_irq(&pdev->dev, irq, xiic_isr,
 					xiic_process, IRQF_ONESHOT,
 					pdev->name, i2c);
 
 	if (ret < 0) {
 		dev_err(&pdev->dev, "Cannot claim IRQ\n");
-		return ret;
+		goto err_clk_dis;
 	}
 
 	/*
@@ -776,7 +804,7 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to add adapter\n");
 		xiic_deinit(i2c);
-		return ret;
+		goto err_clk_dis;
 	}
 
 	if (pdata) {
@@ -786,16 +814,30 @@ static int xiic_i2c_probe(struct platform_device *pdev)
 	}
 
 	return 0;
+
+err_clk_dis:
+	clk_disable_unprepare(i2c->clk);
+	pm_runtime_set_suspended(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+	return ret;
 }
 
 static int xiic_i2c_remove(struct platform_device *pdev)
 {
 	struct xiic_i2c *i2c = platform_get_drvdata(pdev);
+	int ret;
 
 	/* remove adapter & data */
 	i2c_del_adapter(&i2c->adap);
 
+	ret = clk_prepare_enable(i2c->clk);
+	if (ret) {
+		dev_err(&pdev->dev, "Unable to enable clock.\n");
+		return ret;
+	}
 	xiic_deinit(i2c);
+	clk_disable_unprepare(i2c->clk);
+	pm_runtime_disable(&pdev->dev);
 
 	return 0;
 }
@@ -808,12 +850,42 @@ static const struct of_device_id xiic_of_match[] = {
 MODULE_DEVICE_TABLE(of, xiic_of_match);
 #endif
 
+static int __maybe_unused cdns_i2c_runtime_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct xiic_i2c *i2c = platform_get_drvdata(pdev);
+
+	clk_disable(i2c->clk);
+
+	return 0;
+}
+
+static int __maybe_unused cdns_i2c_runtime_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct xiic_i2c *i2c = platform_get_drvdata(pdev);
+	int ret;
+
+	ret = clk_enable(i2c->clk);
+	if (ret) {
+		dev_err(dev, "Cannot enable clock.\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops xiic_dev_pm_ops = {
+	SET_RUNTIME_PM_OPS(cdns_i2c_runtime_suspend,
+			   cdns_i2c_runtime_resume, NULL)
+};
 static struct platform_driver xiic_i2c_driver = {
 	.probe   = xiic_i2c_probe,
 	.remove  = xiic_i2c_remove,
 	.driver  = {
 		.name = DRIVER_NAME,
 		.of_match_table = of_match_ptr(xiic_of_match),
+		.pm = &xiic_dev_pm_ops,
 	},
 };
 
-- 
1.7.1

[PATCH 2/2] bindings: i2c: Add clock entries for i2c-xiic

[Shubhrajyoti Datta]

Add clock description for i2c-xiic

Signed-off-by: Shubhrajyoti Datta <shubhraj@xilinx.com>
---
 Documentation/devicetree/bindings/i2c/i2c-xiic.txt |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-xiic.txt b/Documentation/devicetree/bindings/i2c/i2c-xiic.txt
index ceabbe9..caf42e9 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-xiic.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-xiic.txt
@@ -6,14 +6,17 @@ Required properties:
 - interrupts : IIC controller unterrupt
 - #address-cells = <1>
 - #size-cells = <0>
+- clocks: Input clock specifier. Refer to common clock bindings.
 
 Optional properties:
 - Child nodes conforming to i2c bus binding
+- clock-names: Input clock name, should be 'pclk'.
 
 Example:
 
 	axi_iic_0: i2c@40800000 {
 		compatible = "xlnx,xps-iic-2.00.a";
+		clocks = <&clkc 15>;
 		interrupts = < 1 2 >;
 		reg = < 0x40800000 0x10000 >;
 
-- 
1.7.1

Re: [PATCH 2/2] bindings: i2c: Add clock entries for i2c-xiic

[Rob Herring]

On Mon, Jan 04, 2016 at 01:20:34PM +0530, Shubhrajyoti Datta wrote:
> Add clock description for i2c-xiic
> 
> Signed-off-by: Shubhrajyoti Datta <shubhraj@xilinx.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-xiic.txt |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)

Acked-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/2] bindings: i2c: Add clock entries for i2c-xiic

[Shubhrajyoti Datta]

On Mon, Jan 4, 2016 at 7:48 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Jan 04, 2016 at 01:20:34PM +0530, Shubhrajyoti Datta wrote:
>> Add clock description for i2c-xiic
>>
>> Signed-off-by: Shubhrajyoti Datta <shubhraj@xilinx.com>
>> ---
>>  Documentation/devicetree/bindings/i2c/i2c-xiic.txt |    3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>
> Acked-by: Rob Herring <robh@kernel.org>

Thanks.

Re: [PATCH 1/2] i2c: xiic: Implement Power management

[Shubhrajyoti Datta]

On Mon, Jan 4, 2016 at 1:20 PM, Shubhrajyoti Datta
<shubhrajyoti.datta-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org> wrote:
> Enable power management. This patch enables the clocks
> before transfer and disables after the transfer.
>
> Signed-off-by: Shubhrajyoti Datta <shubhraj-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---

If there are no further comments can this be considered for merge?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] i2c: xiic: Implement Power management

[Wolfram Sang]

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


> +	ret = clk_prepare_enable(i2c->clk);
> +	if (ret)
> +		dev_err(&pdev->dev, "Unable to enable clock.\n");

Don't you want to bail out here? The clk refcounting will be messed up
if you continue.

> +err_clk_dis:
> +	clk_disable_unprepare(i2c->clk);
> +	pm_runtime_set_suspended(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +	return ret;

To match the inverse of the activation order, I'd put the
clk_disable_unprepare after the pm_* calls. Couldn't it happen that
those calls need to change the clk state?


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/2] i2c: xiic: Implement Power management

[Shubhrajyoti Datta]

On Tue, Mar 1, 2016 at 11:59 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
>
>> +     ret = clk_prepare_enable(i2c->clk);
>> +     if (ret)
>> +             dev_err(&pdev->dev, "Unable to enable clock.\n");
>
> Don't you want to bail out here? The clk refcounting will be messed up
> if you continue.

Agree missed it will fix it.
>
>> +err_clk_dis:
>> +     clk_disable_unprepare(i2c->clk);
>> +     pm_runtime_set_suspended(&pdev->dev);
>> +     pm_runtime_disable(&pdev->dev);
>> +     return ret;
>
> To match the inverse of the activation order, I'd put the
> clk_disable_unprepare after the pm_* calls. Couldn't it happen that
> those calls need to change the clk state?
>
makes sense will change in the next version