[PATCH 0/3] i2c: rcar: adapt PM usage to multi master case

[PATCH 0/3] i2c: rcar: adapt PM usage to multi master case

[Wolfram Sang]

If we are in a multi-master scenario, we need to block runtime PM so the
arbitration circuit stays awake.

So, we define a new binding and adapt the i2c-rcar driver to have an example
implementation.

Changes since RFC:

* added acks from Rob and Geert (thanks!)
* use of_property_read_bool() now instead of of_get_property (thanks Sergei)
* way more testing


Wolfram Sang (3):
  i2c: document binding for multi-master case
  i2c: rcar: remove macros dealing with flags
  i2c: rcar: disable PM in multi-master mode

 Documentation/devicetree/bindings/i2c/i2c.txt |  5 ++++
 drivers/i2c/busses/i2c-rcar.c                 | 39 ++++++++++++++++-----------
 2 files changed, 28 insertions(+), 16 deletions(-)

-- 
2.1.4

[PATCH 0/3] i2c: rcar: adapt PM usage to multi master case

[Wolfram Sang]

If we are in a multi-master scenario, we need to block runtime PM so the
arbitration circuit stays awake.

So, we define a new binding and adapt the i2c-rcar driver to have an example
implementation.

Changes since RFC:

* added acks from Rob and Geert (thanks!)
* use of_property_read_bool() now instead of of_get_property (thanks Sergei)
* way more testing


Wolfram Sang (3):
  i2c: document binding for multi-master case
  i2c: rcar: remove macros dealing with flags
  i2c: rcar: disable PM in multi-master mode

 Documentation/devicetree/bindings/i2c/i2c.txt |  5 ++++
 drivers/i2c/busses/i2c-rcar.c                 | 39 ++++++++++++++++-----------
 2 files changed, 28 insertions(+), 16 deletions(-)

-- 
2.1.4

[PATCH 1/3] i2c: document binding for multi-master case

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

We need this binding because some I2C master drivers will need to adapt
their PM settings for the arbitration circuitry.

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 Documentation/devicetree/bindings/i2c/i2c.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index a00219f5ee0733..c8d977ed847f3c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -54,6 +54,11 @@ wants to support one of the below features, it should adapt the bindings below.
 	"irq" and "wakeup" names are recognized by I2C core, other names are
 	left to individual drivers.
 
+- multi-master
+	states that there is another master active on this bus. The OS can use
+	this information to adapt power management to keep the arbitration awake
+	all the time, for example.
+
 - wakeup-source
 	device can be used as a wakeup source.
 
-- 
2.1.4

[PATCH 1/3] i2c: document binding for multi-master case

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

We need this binding because some I2C master drivers will need to adapt
their PM settings for the arbitration circuitry.

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 Documentation/devicetree/bindings/i2c/i2c.txt | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index a00219f5ee0733..c8d977ed847f3c 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -54,6 +54,11 @@ wants to support one of the below features, it should adapt the bindings below.
 	"irq" and "wakeup" names are recognized by I2C core, other names are
 	left to individual drivers.
 
+- multi-master
+	states that there is another master active on this bus. The OS can use
+	this information to adapt power management to keep the arbitration awake
+	all the time, for example.
+
 - wakeup-source
 	device can be used as a wakeup source.
 
-- 
2.1.4

[PATCH 2/3] i2c: rcar: remove macros dealing with flags

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

These macros don't really hide complexity, but C idioms. Removing them
makes the code easier to read IMO and make a planned extension easier.

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 74077e9ab8cad6..fab841899e65f2 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -122,9 +122,6 @@ struct rcar_i2c_priv {
 #define rcar_i2c_priv_to_dev(p)		((p)->adap.dev.parent)
 #define rcar_i2c_is_recv(p)		((p)->msg->flags & I2C_M_RD)
 
-#define rcar_i2c_flags_set(p, f)	((p)->flags |= (f))
-#define rcar_i2c_flags_has(p, f)	((p)->flags & (f))
-
 #define LOOP_TIMEOUT	1024
 
 
@@ -258,7 +255,7 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
 
 	priv->pos = 0;
 	if (priv->msgs_left = 1)
-		rcar_i2c_flags_set(priv, ID_LAST_MSG);
+		priv->flags |= ID_LAST_MSG;
 
 	rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | read);
 	/*
@@ -266,7 +263,7 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
 	 * of ICMSR and ICMCR depends on whether we issue START or REP_START. Since
 	 * it didn't cause a drawback for me, let's rather be safe than sorry.
 	 */
-	if (rcar_i2c_flags_has(priv, ID_FIRST_MSG)) {
+	if (priv->flags & ID_FIRST_MSG) {
 		rcar_i2c_write(priv, ICMSR, 0);
 		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
 	} else {
@@ -438,7 +435,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 
 	/* Arbitration lost */
 	if (msr & MAL) {
-		rcar_i2c_flags_set(priv, (ID_DONE | ID_ARBLOST));
+		priv->flags |= ID_DONE | ID_ARBLOST;
 		goto out;
 	}
 
@@ -446,14 +443,14 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 	if (msr & MNR) {
 		/* HW automatically sends STOP after received NACK */
 		rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP);
-		rcar_i2c_flags_set(priv, ID_NACK);
+		priv->flags |= ID_NACK;
 		goto out;
 	}
 
 	/* Stop */
 	if (msr & MST) {
 		priv->msgs_left--; /* The last message also made it */
-		rcar_i2c_flags_set(priv, ID_DONE);
+		priv->flags |= ID_DONE;
 		goto out;
 	}
 
@@ -463,7 +460,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 		rcar_i2c_irq_send(priv, msr);
 
 out:
-	if (rcar_i2c_flags_has(priv, ID_DONE)) {
+	if (priv->flags & ID_DONE) {
 		rcar_i2c_write(priv, ICMIER, 0);
 		rcar_i2c_write(priv, ICMSR, 0);
 		wake_up(&priv->wait);
@@ -501,15 +498,14 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 	priv->flags = ID_FIRST_MSG;
 	rcar_i2c_prepare_msg(priv);
 
-	time_left = wait_event_timeout(priv->wait,
-				     rcar_i2c_flags_has(priv, ID_DONE),
+	time_left = wait_event_timeout(priv->wait, priv->flags & ID_DONE,
 				     num * adap->timeout);
 	if (!time_left) {
 		rcar_i2c_init(priv);
 		ret = -ETIMEDOUT;
-	} else if (rcar_i2c_flags_has(priv, ID_NACK)) {
+	} else if (priv->flags & ID_NACK) {
 		ret = -ENXIO;
-	} else if (rcar_i2c_flags_has(priv, ID_ARBLOST)) {
+	} else if (priv->flags & ID_ARBLOST) {
 		ret = -EAGAIN;
 	} else {
 		ret = num - priv->msgs_left; /* The number of transfer */
-- 
2.1.4

[PATCH 2/3] i2c: rcar: remove macros dealing with flags

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

These macros don't really hide complexity, but C idioms. Removing them
makes the code easier to read IMO and make a planned extension easier.

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 74077e9ab8cad6..fab841899e65f2 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -122,9 +122,6 @@ struct rcar_i2c_priv {
 #define rcar_i2c_priv_to_dev(p)		((p)->adap.dev.parent)
 #define rcar_i2c_is_recv(p)		((p)->msg->flags & I2C_M_RD)
 
-#define rcar_i2c_flags_set(p, f)	((p)->flags |= (f))
-#define rcar_i2c_flags_has(p, f)	((p)->flags & (f))
-
 #define LOOP_TIMEOUT	1024
 
 
@@ -258,7 +255,7 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
 
 	priv->pos = 0;
 	if (priv->msgs_left == 1)
-		rcar_i2c_flags_set(priv, ID_LAST_MSG);
+		priv->flags |= ID_LAST_MSG;
 
 	rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | read);
 	/*
@@ -266,7 +263,7 @@ static void rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
 	 * of ICMSR and ICMCR depends on whether we issue START or REP_START. Since
 	 * it didn't cause a drawback for me, let's rather be safe than sorry.
 	 */
-	if (rcar_i2c_flags_has(priv, ID_FIRST_MSG)) {
+	if (priv->flags & ID_FIRST_MSG) {
 		rcar_i2c_write(priv, ICMSR, 0);
 		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
 	} else {
@@ -438,7 +435,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 
 	/* Arbitration lost */
 	if (msr & MAL) {
-		rcar_i2c_flags_set(priv, (ID_DONE | ID_ARBLOST));
+		priv->flags |= ID_DONE | ID_ARBLOST;
 		goto out;
 	}
 
@@ -446,14 +443,14 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 	if (msr & MNR) {
 		/* HW automatically sends STOP after received NACK */
 		rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP);
-		rcar_i2c_flags_set(priv, ID_NACK);
+		priv->flags |= ID_NACK;
 		goto out;
 	}
 
 	/* Stop */
 	if (msr & MST) {
 		priv->msgs_left--; /* The last message also made it */
-		rcar_i2c_flags_set(priv, ID_DONE);
+		priv->flags |= ID_DONE;
 		goto out;
 	}
 
@@ -463,7 +460,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 		rcar_i2c_irq_send(priv, msr);
 
 out:
-	if (rcar_i2c_flags_has(priv, ID_DONE)) {
+	if (priv->flags & ID_DONE) {
 		rcar_i2c_write(priv, ICMIER, 0);
 		rcar_i2c_write(priv, ICMSR, 0);
 		wake_up(&priv->wait);
@@ -501,15 +498,14 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 	priv->flags = ID_FIRST_MSG;
 	rcar_i2c_prepare_msg(priv);
 
-	time_left = wait_event_timeout(priv->wait,
-				     rcar_i2c_flags_has(priv, ID_DONE),
+	time_left = wait_event_timeout(priv->wait, priv->flags & ID_DONE,
 				     num * adap->timeout);
 	if (!time_left) {
 		rcar_i2c_init(priv);
 		ret = -ETIMEDOUT;
-	} else if (rcar_i2c_flags_has(priv, ID_NACK)) {
+	} else if (priv->flags & ID_NACK) {
 		ret = -ENXIO;
-	} else if (rcar_i2c_flags_has(priv, ID_ARBLOST)) {
+	} else if (priv->flags & ID_ARBLOST) {
 		ret = -EAGAIN;
 	} else {
 		ret = num - priv->msgs_left; /* The number of transfer */
-- 
2.1.4

[PATCH 3/3] i2c: rcar: disable PM in multi-master mode

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

In multi master mode, the IP core needs to be always active for
arbitration reasons. Get the config from DT and set up PM depending on
the config.

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index fab841899e65f2..1abeadc8ab7959 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -96,6 +96,9 @@
 #define ID_DONE		(1 << 2)
 #define ID_ARBLOST	(1 << 3)
 #define ID_NACK		(1 << 4)
+/* persistent flags */
+#define ID_P_PM_BLOCKED	(1 << 31)
+#define ID_P_MASK	ID_P_PM_BLOCKED
 
 enum rcar_i2c_type {
 	I2C_RCAR_GEN1,
@@ -277,7 +280,7 @@ static void rcar_i2c_next_msg(struct rcar_i2c_priv *priv)
 {
 	priv->msg++;
 	priv->msgs_left--;
-	priv->flags = 0;
+	priv->flags &= ID_P_MASK;
 	rcar_i2c_prepare_msg(priv);
 }
 
@@ -495,7 +498,7 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 	/* init first message */
 	priv->msg = msgs;
 	priv->msgs_left = num;
-	priv->flags = ID_FIRST_MSG;
+	priv->flags = (priv->flags & ID_P_MASK) | ID_FIRST_MSG;
 	rcar_i2c_prepare_msg(priv);
 
 	time_left = wait_event_timeout(priv->wait, priv->flags & ID_DONE,
@@ -630,7 +633,13 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 		goto out_pm_put;
 
 	rcar_i2c_init(priv);
-	pm_runtime_put(dev);
+
+	/* Don't suspend when multi-master to keep arbitration working */
+	if (of_property_read_bool(dev->of_node, "multi-master"))
+		priv->flags |= ID_P_PM_BLOCKED;
+	else
+		pm_runtime_put(dev);
+
 
 	irq = platform_get_irq(pdev, 0);
 	ret = devm_request_irq(dev, irq, rcar_i2c_irq, 0, dev_name(dev), priv);
@@ -664,6 +673,8 @@ static int rcar_i2c_remove(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 
 	i2c_del_adapter(&priv->adap);
+	if (priv->flags & ID_P_PM_BLOCKED)
+		pm_runtime_put(dev);
 	pm_runtime_disable(dev);
 
 	return 0;
-- 
2.1.4

[PATCH 3/3] i2c: rcar: disable PM in multi-master mode

[Wolfram Sang]

From: Wolfram Sang <wsa+renesas@sang-engineering.com>

In multi master mode, the IP core needs to be always active for
arbitration reasons. Get the config from DT and set up PM depending on
the config.

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index fab841899e65f2..1abeadc8ab7959 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -96,6 +96,9 @@
 #define ID_DONE		(1 << 2)
 #define ID_ARBLOST	(1 << 3)
 #define ID_NACK		(1 << 4)
+/* persistent flags */
+#define ID_P_PM_BLOCKED	(1 << 31)
+#define ID_P_MASK	ID_P_PM_BLOCKED
 
 enum rcar_i2c_type {
 	I2C_RCAR_GEN1,
@@ -277,7 +280,7 @@ static void rcar_i2c_next_msg(struct rcar_i2c_priv *priv)
 {
 	priv->msg++;
 	priv->msgs_left--;
-	priv->flags = 0;
+	priv->flags &= ID_P_MASK;
 	rcar_i2c_prepare_msg(priv);
 }
 
@@ -495,7 +498,7 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 	/* init first message */
 	priv->msg = msgs;
 	priv->msgs_left = num;
-	priv->flags = ID_FIRST_MSG;
+	priv->flags = (priv->flags & ID_P_MASK) | ID_FIRST_MSG;
 	rcar_i2c_prepare_msg(priv);
 
 	time_left = wait_event_timeout(priv->wait, priv->flags & ID_DONE,
@@ -630,7 +633,13 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 		goto out_pm_put;
 
 	rcar_i2c_init(priv);
-	pm_runtime_put(dev);
+
+	/* Don't suspend when multi-master to keep arbitration working */
+	if (of_property_read_bool(dev->of_node, "multi-master"))
+		priv->flags |= ID_P_PM_BLOCKED;
+	else
+		pm_runtime_put(dev);
+
 
 	irq = platform_get_irq(pdev, 0);
 	ret = devm_request_irq(dev, irq, rcar_i2c_irq, 0, dev_name(dev), priv);
@@ -664,6 +673,8 @@ static int rcar_i2c_remove(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 
 	i2c_del_adapter(&priv->adap);
+	if (priv->flags & ID_P_PM_BLOCKED)
+		pm_runtime_put(dev);
 	pm_runtime_disable(dev);
 
 	return 0;
-- 
2.1.4

Re: [PATCH 0/3] i2c: rcar: adapt PM usage to multi master case

[Wolfram Sang]

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

On Wed, Dec 23, 2015 at 05:56:31PM +0100, Wolfram Sang wrote:
> If we are in a multi-master scenario, we need to block runtime PM so the
> arbitration circuit stays awake.
> 
> So, we define a new binding and adapt the i2c-rcar driver to have an example
> implementation.

Applied to for-next, thanks!


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

Re: [PATCH 0/3] i2c: rcar: adapt PM usage to multi master case

[Wolfram Sang]

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

On Wed, Dec 23, 2015 at 05:56:31PM +0100, Wolfram Sang wrote:
> If we are in a multi-master scenario, we need to block runtime PM so the
> arbitration circuit stays awake.
> 
> So, we define a new binding and adapt the i2c-rcar driver to have an example
> implementation.

Applied to for-next, thanks!


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