All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] i2c: rcar: cleanup series
@ 2014-05-28  7:44 ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

Here are some patches to let the rcar driver have a diet without losing
functionality. IMO the code becomes more readable, too, although mileages may
vary about this or that change. Bus-free handling is improved, too. Tested with
renesas-devel-v3.15-rc7-20140526 on a Lager and Koelsch board. The git tree can
be found here, including one proof-of-concept enablement patch:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/rcar-cleanup

Please test, review, comment.

Thanks,

   Wolfram


Wolfram Sang (11):
  i2c: rcar: not everything needs to be a function
  i2c: rcar: no need to store irq number
  i2c: rcar: refactor bus state machine
  i2c: rcar: refactor irq state machine
  i2c: rcar: check bus free before first message
  i2c: rcar: refactor setting up msg
  i2c: rcar: refactor status bit handling
  i2c: rcar: remove spinlock
  i2c: rcar: reuse status bits as enable bits
  i2c: rcar: janitorial cleanup after refactoring
  i2c: rcar: update copyright and license information

 drivers/i2c/busses/i2c-rcar.c | 252 ++++++++----------------------------------
 1 file changed, 48 insertions(+), 204 deletions(-)

-- 
2.0.0.rc2


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

* [PATCH 00/11] i2c: rcar: cleanup series
@ 2014-05-28  7:44 ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

Here are some patches to let the rcar driver have a diet without losing
functionality. IMO the code becomes more readable, too, although mileages may
vary about this or that change. Bus-free handling is improved, too. Tested with
renesas-devel-v3.15-rc7-20140526 on a Lager and Koelsch board. The git tree can
be found here, including one proof-of-concept enablement patch:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/rcar-cleanup

Please test, review, comment.

Thanks,

   Wolfram


Wolfram Sang (11):
  i2c: rcar: not everything needs to be a function
  i2c: rcar: no need to store irq number
  i2c: rcar: refactor bus state machine
  i2c: rcar: refactor irq state machine
  i2c: rcar: check bus free before first message
  i2c: rcar: refactor setting up msg
  i2c: rcar: refactor status bit handling
  i2c: rcar: remove spinlock
  i2c: rcar: reuse status bits as enable bits
  i2c: rcar: janitorial cleanup after refactoring
  i2c: rcar: update copyright and license information

 drivers/i2c/busses/i2c-rcar.c | 252 ++++++++----------------------------------
 1 file changed, 48 insertions(+), 204 deletions(-)

-- 
2.0.0.rc2


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

* [PATCH 01/11] i2c: rcar: not everything needs to be a function
  2014-05-28  7:44 ` Wolfram Sang
@ 2014-05-28  7:44   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

Very basic operations, just called once, can also go to the caller.

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

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 06d47aafbb79..32df4e1f3738 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -312,18 +312,9 @@ scgd_find:
 	return 0;
 }
 
-static void rcar_i2c_clock_start(struct rcar_i2c_priv *priv)
-{
-	rcar_i2c_write(priv, ICCCR, priv->icccr);
-}
-
 /*
  *		status functions
  */
-static u32 rcar_i2c_status_get(struct rcar_i2c_priv *priv)
-{
-	return rcar_i2c_read(priv, ICMSR);
-}
 
 #define rcar_i2c_status_clear(priv) rcar_i2c_status_bit_clear(priv, 0xffffffff)
 static void rcar_i2c_status_bit_clear(struct rcar_i2c_priv *priv, u32 bit)
@@ -480,7 +471,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 	/*-------------- spin lock -----------------*/
 	spin_lock(&priv->lock);
 
-	msr = rcar_i2c_status_get(priv);
+	msr = rcar_i2c_read(priv, ICMSR);
 
 	/*
 	 * Arbitration lost
@@ -554,7 +545,8 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 	spin_lock_irqsave(&priv->lock, flags);
 
 	rcar_i2c_init(priv);
-	rcar_i2c_clock_start(priv);
+	/* start clock */
+	rcar_i2c_write(priv, ICCCR, priv->icccr);
 
 	spin_unlock_irqrestore(&priv->lock, flags);
 	/*-------------- spin unlock -----------------*/
-- 
2.0.0.rc2


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

* [PATCH 01/11] i2c: rcar: not everything needs to be a function
@ 2014-05-28  7:44   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

Very basic operations, just called once, can also go to the caller.

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

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 06d47aafbb79..32df4e1f3738 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -312,18 +312,9 @@ scgd_find:
 	return 0;
 }
 
-static void rcar_i2c_clock_start(struct rcar_i2c_priv *priv)
-{
-	rcar_i2c_write(priv, ICCCR, priv->icccr);
-}
-
 /*
  *		status functions
  */
-static u32 rcar_i2c_status_get(struct rcar_i2c_priv *priv)
-{
-	return rcar_i2c_read(priv, ICMSR);
-}
 
 #define rcar_i2c_status_clear(priv) rcar_i2c_status_bit_clear(priv, 0xffffffff)
 static void rcar_i2c_status_bit_clear(struct rcar_i2c_priv *priv, u32 bit)
@@ -480,7 +471,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 	/*-------------- spin lock -----------------*/
 	spin_lock(&priv->lock);
 
-	msr = rcar_i2c_status_get(priv);
+	msr = rcar_i2c_read(priv, ICMSR);
 
 	/*
 	 * Arbitration lost
@@ -554,7 +545,8 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 	spin_lock_irqsave(&priv->lock, flags);
 
 	rcar_i2c_init(priv);
-	rcar_i2c_clock_start(priv);
+	/* start clock */
+	rcar_i2c_write(priv, ICCCR, priv->icccr);
 
 	spin_unlock_irqrestore(&priv->lock, flags);
 	/*-------------- spin unlock -----------------*/
-- 
2.0.0.rc2


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

* [PATCH 02/11] i2c: rcar: no need to store irq number
  2014-05-28  7:44 ` Wolfram Sang
@ 2014-05-28  7:44   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

We use devm, so irq number is only needed during probe.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 32df4e1f3738..88efbf29a182 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -116,7 +116,6 @@ struct rcar_i2c_priv {
 	wait_queue_head_t wait;
 
 	int pos;
-	int irq;
 	u32 icccr;
 	u32 flags;
 	enum rcar_i2c_type	devtype;
@@ -650,7 +649,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct device *dev = &pdev->dev;
 	u32 bus_speed;
-	int ret;
+	int irq, ret;
 
 	priv = devm_kzalloc(dev, sizeof(struct rcar_i2c_priv), GFP_KERNEL);
 	if (!priv) {
@@ -684,7 +683,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->io))
 		return PTR_ERR(priv->io);
 
-	priv->irq = platform_get_irq(pdev, 0);
+	irq = platform_get_irq(pdev, 0);
 	init_waitqueue_head(&priv->wait);
 	spin_lock_init(&priv->lock);
 
@@ -698,10 +697,10 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	i2c_set_adapdata(adap, priv);
 	strlcpy(adap->name, pdev->name, sizeof(adap->name));
 
-	ret = devm_request_irq(dev, priv->irq, rcar_i2c_irq, 0,
+	ret = devm_request_irq(dev, irq, rcar_i2c_irq, 0,
 			       dev_name(dev), priv);
 	if (ret < 0) {
-		dev_err(dev, "cannot get irq %d\n", priv->irq);
+		dev_err(dev, "cannot get irq %d\n", irq);
 		return ret;
 	}
 
-- 
2.0.0.rc2


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

* [PATCH 02/11] i2c: rcar: no need to store irq number
@ 2014-05-28  7:44   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

We use devm, so irq number is only needed during probe.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 32df4e1f3738..88efbf29a182 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -116,7 +116,6 @@ struct rcar_i2c_priv {
 	wait_queue_head_t wait;
 
 	int pos;
-	int irq;
 	u32 icccr;
 	u32 flags;
 	enum rcar_i2c_type	devtype;
@@ -650,7 +649,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct device *dev = &pdev->dev;
 	u32 bus_speed;
-	int ret;
+	int irq, ret;
 
 	priv = devm_kzalloc(dev, sizeof(struct rcar_i2c_priv), GFP_KERNEL);
 	if (!priv) {
@@ -684,7 +683,7 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	if (IS_ERR(priv->io))
 		return PTR_ERR(priv->io);
 
-	priv->irq = platform_get_irq(pdev, 0);
+	irq = platform_get_irq(pdev, 0);
 	init_waitqueue_head(&priv->wait);
 	spin_lock_init(&priv->lock);
 
@@ -698,10 +697,10 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	i2c_set_adapdata(adap, priv);
 	strlcpy(adap->name, pdev->name, sizeof(adap->name));
 
-	ret = devm_request_irq(dev, priv->irq, rcar_i2c_irq, 0,
+	ret = devm_request_irq(dev, irq, rcar_i2c_irq, 0,
 			       dev_name(dev), priv);
 	if (ret < 0) {
-		dev_err(dev, "cannot get irq %d\n", priv->irq);
+		dev_err(dev, "cannot get irq %d\n", irq);
 		return ret;
 	}
 
-- 
2.0.0.rc2


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

* [PATCH 03/11] i2c: rcar: refactor bus state machine
       [not found] ` <1401263086-13720-1-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
@ 2014-05-28  7:44     ` Wolfram Sang
  2014-05-28  7:44     ` Wolfram Sang
  2014-05-28  8:26     ` Geert Uytterhoeven
  2 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-sh-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Magnus Damm,
	Simon Horman, Laurent Pinchart, Geert Uytterhoeven,
	Kuninori Morimoto

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

Remove the seperate functions and use designated constants. As readable
but less overhead.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 37 ++++++++++---------------------------
 1 file changed, 10 insertions(+), 27 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 88efbf29a182..e0cdab488cc5 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -79,11 +79,9 @@
 #define MATE	(1 << 0)	/* address sent irq en */
 
 
-enum {
-	RCAR_BUS_PHASE_ADDR,
-	RCAR_BUS_PHASE_DATA,
-	RCAR_BUS_PHASE_STOP,
-};
+#define RCAR_BUS_PHASE_START	(MDBS | MIE | ESG)
+#define RCAR_BUS_PHASE_DATA	(MDBS | MIE)
+#define RCAR_BUS_PHASE_STOP	(MDBS | MIE | FSB)
 
 enum {
 	RCAR_IRQ_CLOSE,
@@ -204,21 +202,6 @@ static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
 	return -EBUSY;
 }
 
-static void rcar_i2c_bus_phase(struct rcar_i2c_priv *priv, int phase)
-{
-	switch (phase) {
-	case RCAR_BUS_PHASE_ADDR:
-		rcar_i2c_write(priv, ICMCR, MDBS | MIE | ESG);
-		break;
-	case RCAR_BUS_PHASE_DATA:
-		rcar_i2c_write(priv, ICMCR, MDBS | MIE);
-		break;
-	case RCAR_BUS_PHASE_STOP:
-		rcar_i2c_write(priv, ICMCR, MDBS | MIE | FSB);
-		break;
-	}
-}
-
 /*
  *		clock function
  */
@@ -328,7 +311,7 @@ static int rcar_i2c_recv(struct rcar_i2c_priv *priv)
 {
 	rcar_i2c_set_addr(priv, 1);
 	rcar_i2c_status_clear(priv);
-	rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_ADDR);
+	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
 	rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_RECV);
 
 	return 0;
@@ -347,7 +330,7 @@ static int rcar_i2c_send(struct rcar_i2c_priv *priv)
 
 	rcar_i2c_set_addr(priv, 0);
 	rcar_i2c_status_clear(priv);
-	rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_ADDR);
+	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
 	rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_SEND);
 
 	return 0;
@@ -376,7 +359,7 @@ static int rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 	 * goto data phase.
 	 */
 	if (msr & MAT)
-		rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_DATA);
+		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_DATA);
 
 	if (priv->pos < msg->len) {
 		/*
@@ -404,7 +387,7 @@ static int rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 			 * prepare stop condition here.
 			 * ID_DONE will be set on STOP irq.
 			 */
-			rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_STOP);
+			rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
 		else
 			/*
 			 * If current msg is _NOT_ last msg,
@@ -452,9 +435,9 @@ static int rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 	 * otherwise, go to DATA phase.
 	 */
 	if (priv->pos + 1 >= msg->len)
-		rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_STOP);
+		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
 	else
-		rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_DATA);
+		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_DATA);
 
 	rcar_i2c_recv_restart(priv);
 
@@ -502,7 +485,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 		dev_dbg(dev, "Nack\n");
 
 		/* go to stop phase */
-		rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_STOP);
+		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
 		rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_STOP);
 		rcar_i2c_flags_set(priv, ID_NACK);
 		goto out;
-- 
2.0.0.rc2


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

* [PATCH 03/11] i2c: rcar: refactor bus state machine
@ 2014-05-28  7:44     ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-sh-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Magnus Damm,
	Simon Horman, Laurent Pinchart, Geert Uytterhoeven,
	Kuninori Morimoto

From: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>

Remove the seperate functions and use designated constants. As readable
but less overhead.

Signed-off-by: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>
---
 drivers/i2c/busses/i2c-rcar.c | 37 ++++++++++---------------------------
 1 file changed, 10 insertions(+), 27 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 88efbf29a182..e0cdab488cc5 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -79,11 +79,9 @@
 #define MATE	(1 << 0)	/* address sent irq en */
 
 
-enum {
-	RCAR_BUS_PHASE_ADDR,
-	RCAR_BUS_PHASE_DATA,
-	RCAR_BUS_PHASE_STOP,
-};
+#define RCAR_BUS_PHASE_START	(MDBS | MIE | ESG)
+#define RCAR_BUS_PHASE_DATA	(MDBS | MIE)
+#define RCAR_BUS_PHASE_STOP	(MDBS | MIE | FSB)
 
 enum {
 	RCAR_IRQ_CLOSE,
@@ -204,21 +202,6 @@ static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
 	return -EBUSY;
 }
 
-static void rcar_i2c_bus_phase(struct rcar_i2c_priv *priv, int phase)
-{
-	switch (phase) {
-	case RCAR_BUS_PHASE_ADDR:
-		rcar_i2c_write(priv, ICMCR, MDBS | MIE | ESG);
-		break;
-	case RCAR_BUS_PHASE_DATA:
-		rcar_i2c_write(priv, ICMCR, MDBS | MIE);
-		break;
-	case RCAR_BUS_PHASE_STOP:
-		rcar_i2c_write(priv, ICMCR, MDBS | MIE | FSB);
-		break;
-	}
-}
-
 /*
  *		clock function
  */
@@ -328,7 +311,7 @@ static int rcar_i2c_recv(struct rcar_i2c_priv *priv)
 {
 	rcar_i2c_set_addr(priv, 1);
 	rcar_i2c_status_clear(priv);
-	rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_ADDR);
+	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
 	rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_RECV);
 
 	return 0;
@@ -347,7 +330,7 @@ static int rcar_i2c_send(struct rcar_i2c_priv *priv)
 
 	rcar_i2c_set_addr(priv, 0);
 	rcar_i2c_status_clear(priv);
-	rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_ADDR);
+	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
 	rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_SEND);
 
 	return 0;
@@ -376,7 +359,7 @@ static int rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 	 * goto data phase.
 	 */
 	if (msr & MAT)
-		rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_DATA);
+		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_DATA);
 
 	if (priv->pos < msg->len) {
 		/*
@@ -404,7 +387,7 @@ static int rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 			 * prepare stop condition here.
 			 * ID_DONE will be set on STOP irq.
 			 */
-			rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_STOP);
+			rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
 		else
 			/*
 			 * If current msg is _NOT_ last msg,
@@ -452,9 +435,9 @@ static int rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 	 * otherwise, go to DATA phase.
 	 */
 	if (priv->pos + 1 >= msg->len)
-		rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_STOP);
+		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
 	else
-		rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_DATA);
+		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_DATA);
 
 	rcar_i2c_recv_restart(priv);
 
@@ -502,7 +485,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 		dev_dbg(dev, "Nack\n");
 
 		/* go to stop phase */
-		rcar_i2c_bus_phase(priv, RCAR_BUS_PHASE_STOP);
+		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
 		rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_STOP);
 		rcar_i2c_flags_set(priv, ID_NACK);
 		goto out;
-- 
2.0.0.rc2

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

* [PATCH 04/11] i2c: rcar: refactor irq state machine
  2014-05-28  7:44 ` Wolfram Sang
@ 2014-05-28  7:44   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

Remove the seperate functions and use designated constants. As readable
but less overhead. Actually, this is even more readable since the old
function used a mix of "=" and "|=".

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 39 +++++++--------------------------------
 1 file changed, 7 insertions(+), 32 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index e0cdab488cc5..9757adf33f37 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -83,12 +83,9 @@
 #define RCAR_BUS_PHASE_DATA	(MDBS | MIE)
 #define RCAR_BUS_PHASE_STOP	(MDBS | MIE | FSB)
 
-enum {
-	RCAR_IRQ_CLOSE,
-	RCAR_IRQ_OPEN_FOR_SEND,
-	RCAR_IRQ_OPEN_FOR_RECV,
-	RCAR_IRQ_OPEN_FOR_STOP,
-};
+#define RCAR_IRQ_SEND	(MNRE | MALE | MSTE | MATE | MDEE)
+#define RCAR_IRQ_RECV	(MNRE | MALE | MSTE | MATE | MDRE)
+#define RCAR_IRQ_STOP	(MSTE)
 
 /*
  * flags
@@ -158,28 +155,6 @@ static void rcar_i2c_init(struct rcar_i2c_priv *priv)
 	rcar_i2c_write(priv, ICMAR, 0);
 }
 
-static void rcar_i2c_irq_mask(struct rcar_i2c_priv *priv, int open)
-{
-	u32 val = MNRE | MALE | MSTE | MATE; /* default */
-
-	switch (open) {
-	case RCAR_IRQ_OPEN_FOR_SEND:
-		val |= MDEE; /* default + send */
-		break;
-	case RCAR_IRQ_OPEN_FOR_RECV:
-		val |= MDRE; /* default + read */
-		break;
-	case RCAR_IRQ_OPEN_FOR_STOP:
-		val = MSTE; /* stop irq only */
-		break;
-	case RCAR_IRQ_CLOSE:
-	default:
-		val = 0; /* all close */
-		break;
-	}
-	rcar_i2c_write(priv, ICMIER, val);
-}
-
 static void rcar_i2c_set_addr(struct rcar_i2c_priv *priv, u32 recv)
 {
 	rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | recv);
@@ -312,7 +287,7 @@ static int rcar_i2c_recv(struct rcar_i2c_priv *priv)
 	rcar_i2c_set_addr(priv, 1);
 	rcar_i2c_status_clear(priv);
 	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
-	rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_RECV);
+	rcar_i2c_write(priv, ICMIER, RCAR_IRQ_RECV);
 
 	return 0;
 }
@@ -331,7 +306,7 @@ static int rcar_i2c_send(struct rcar_i2c_priv *priv)
 	rcar_i2c_set_addr(priv, 0);
 	rcar_i2c_status_clear(priv);
 	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
-	rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_SEND);
+	rcar_i2c_write(priv, ICMIER, RCAR_IRQ_SEND);
 
 	return 0;
 }
@@ -486,7 +461,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 
 		/* go to stop phase */
 		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
-		rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_STOP);
+		rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP);
 		rcar_i2c_flags_set(priv, ID_NACK);
 		goto out;
 	}
@@ -501,7 +476,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 
 out:
 	if (rcar_i2c_flags_has(priv, ID_DONE)) {
-		rcar_i2c_irq_mask(priv, RCAR_IRQ_CLOSE);
+		rcar_i2c_write(priv, ICMIER, 0);
 		rcar_i2c_status_clear(priv);
 		wake_up(&priv->wait);
 	}
-- 
2.0.0.rc2


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

* [PATCH 04/11] i2c: rcar: refactor irq state machine
@ 2014-05-28  7:44   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

Remove the seperate functions and use designated constants. As readable
but less overhead. Actually, this is even more readable since the old
function used a mix of "=" and "|=".

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 39 +++++++--------------------------------
 1 file changed, 7 insertions(+), 32 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index e0cdab488cc5..9757adf33f37 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -83,12 +83,9 @@
 #define RCAR_BUS_PHASE_DATA	(MDBS | MIE)
 #define RCAR_BUS_PHASE_STOP	(MDBS | MIE | FSB)
 
-enum {
-	RCAR_IRQ_CLOSE,
-	RCAR_IRQ_OPEN_FOR_SEND,
-	RCAR_IRQ_OPEN_FOR_RECV,
-	RCAR_IRQ_OPEN_FOR_STOP,
-};
+#define RCAR_IRQ_SEND	(MNRE | MALE | MSTE | MATE | MDEE)
+#define RCAR_IRQ_RECV	(MNRE | MALE | MSTE | MATE | MDRE)
+#define RCAR_IRQ_STOP	(MSTE)
 
 /*
  * flags
@@ -158,28 +155,6 @@ static void rcar_i2c_init(struct rcar_i2c_priv *priv)
 	rcar_i2c_write(priv, ICMAR, 0);
 }
 
-static void rcar_i2c_irq_mask(struct rcar_i2c_priv *priv, int open)
-{
-	u32 val = MNRE | MALE | MSTE | MATE; /* default */
-
-	switch (open) {
-	case RCAR_IRQ_OPEN_FOR_SEND:
-		val |= MDEE; /* default + send */
-		break;
-	case RCAR_IRQ_OPEN_FOR_RECV:
-		val |= MDRE; /* default + read */
-		break;
-	case RCAR_IRQ_OPEN_FOR_STOP:
-		val = MSTE; /* stop irq only */
-		break;
-	case RCAR_IRQ_CLOSE:
-	default:
-		val = 0; /* all close */
-		break;
-	}
-	rcar_i2c_write(priv, ICMIER, val);
-}
-
 static void rcar_i2c_set_addr(struct rcar_i2c_priv *priv, u32 recv)
 {
 	rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | recv);
@@ -312,7 +287,7 @@ static int rcar_i2c_recv(struct rcar_i2c_priv *priv)
 	rcar_i2c_set_addr(priv, 1);
 	rcar_i2c_status_clear(priv);
 	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
-	rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_RECV);
+	rcar_i2c_write(priv, ICMIER, RCAR_IRQ_RECV);
 
 	return 0;
 }
@@ -331,7 +306,7 @@ static int rcar_i2c_send(struct rcar_i2c_priv *priv)
 	rcar_i2c_set_addr(priv, 0);
 	rcar_i2c_status_clear(priv);
 	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
-	rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_SEND);
+	rcar_i2c_write(priv, ICMIER, RCAR_IRQ_SEND);
 
 	return 0;
 }
@@ -486,7 +461,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 
 		/* go to stop phase */
 		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
-		rcar_i2c_irq_mask(priv, RCAR_IRQ_OPEN_FOR_STOP);
+		rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP);
 		rcar_i2c_flags_set(priv, ID_NACK);
 		goto out;
 	}
@@ -501,7 +476,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 
 out:
 	if (rcar_i2c_flags_has(priv, ID_DONE)) {
-		rcar_i2c_irq_mask(priv, RCAR_IRQ_CLOSE);
+		rcar_i2c_write(priv, ICMIER, 0);
 		rcar_i2c_status_clear(priv);
 		wake_up(&priv->wait);
 	}
-- 
2.0.0.rc2


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

* [PATCH 05/11] i2c: rcar: check bus free before first message
  2014-05-28  7:44 ` Wolfram Sang
@ 2014-05-28  7:44   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

We should always check if the bus is free, independently if it is a read
or write. It should be done before the first message, though. After
that, we ourselves keep the bus busy. Remove a 'ret' assignment which
only silenced a build warning.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 9757adf33f37..a97a14955ddf 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -294,15 +294,6 @@ static int rcar_i2c_recv(struct rcar_i2c_priv *priv)
 
 static int rcar_i2c_send(struct rcar_i2c_priv *priv)
 {
-	int ret;
-
-	/*
-	 * It should check bus status when send case
-	 */
-	ret = rcar_i2c_bus_barrier(priv);
-	if (ret < 0)
-		return ret;
-
 	rcar_i2c_set_addr(priv, 0);
 	rcar_i2c_status_clear(priv);
 	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
@@ -508,7 +499,10 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 	spin_unlock_irqrestore(&priv->lock, flags);
 	/*-------------- spin unlock -----------------*/
 
-	ret = -EINVAL;
+	ret = rcar_i2c_bus_barrier(priv);
+	if (ret < 0)
+		goto out;
+
 	for (i = 0; i < num; i++) {
 		/* This HW can't send STOP after address phase */
 		if (msgs[i].len = 0) {
@@ -569,7 +563,7 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 
 		ret = i + 1; /* The number of transfer */
 	}
-
+out:
 	pm_runtime_put(dev);
 
 	if (ret < 0 && ret != -ENXIO)
-- 
2.0.0.rc2


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

* [PATCH 05/11] i2c: rcar: check bus free before first message
@ 2014-05-28  7:44   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

We should always check if the bus is free, independently if it is a read
or write. It should be done before the first message, though. After
that, we ourselves keep the bus busy. Remove a 'ret' assignment which
only silenced a build warning.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 9757adf33f37..a97a14955ddf 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -294,15 +294,6 @@ static int rcar_i2c_recv(struct rcar_i2c_priv *priv)
 
 static int rcar_i2c_send(struct rcar_i2c_priv *priv)
 {
-	int ret;
-
-	/*
-	 * It should check bus status when send case
-	 */
-	ret = rcar_i2c_bus_barrier(priv);
-	if (ret < 0)
-		return ret;
-
 	rcar_i2c_set_addr(priv, 0);
 	rcar_i2c_status_clear(priv);
 	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
@@ -508,7 +499,10 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 	spin_unlock_irqrestore(&priv->lock, flags);
 	/*-------------- spin unlock -----------------*/
 
-	ret = -EINVAL;
+	ret = rcar_i2c_bus_barrier(priv);
+	if (ret < 0)
+		goto out;
+
 	for (i = 0; i < num; i++) {
 		/* This HW can't send STOP after address phase */
 		if (msgs[i].len == 0) {
@@ -569,7 +563,7 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 
 		ret = i + 1; /* The number of transfer */
 	}
-
+out:
 	pm_runtime_put(dev);
 
 	if (ret < 0 && ret != -ENXIO)
-- 
2.0.0.rc2


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

* [PATCH 06/11] i2c: rcar: refactor setting up msg
       [not found] ` <1401263086-13720-1-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
@ 2014-05-28  7:44     ` Wolfram Sang
  2014-05-28  7:44     ` Wolfram Sang
  2014-05-28  8:26     ` Geert Uytterhoeven
  2 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-sh-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Magnus Damm,
	Simon Horman, Laurent Pinchart, Geert Uytterhoeven,
	Kuninori Morimoto

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

Setting up a read or write message is similar enough to be done in one
function. Also, move a helper function into the new function since it is
only used here.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index a97a14955ddf..f7b14d049d5c 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -155,11 +155,6 @@ static void rcar_i2c_init(struct rcar_i2c_priv *priv)
 	rcar_i2c_write(priv, ICMAR, 0);
 }
 
-static void rcar_i2c_set_addr(struct rcar_i2c_priv *priv, u32 recv)
-{
-	rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | recv);
-}
-
 /*
  *		bus control functions
  */
@@ -279,25 +274,14 @@ static void rcar_i2c_status_bit_clear(struct rcar_i2c_priv *priv, u32 bit)
 	rcar_i2c_write(priv, ICMSR, ~bit);
 }
 
-/*
- *		recv/send functions
- */
-static int rcar_i2c_recv(struct rcar_i2c_priv *priv)
+static int rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
 {
-	rcar_i2c_set_addr(priv, 1);
-	rcar_i2c_status_clear(priv);
-	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
-	rcar_i2c_write(priv, ICMIER, RCAR_IRQ_RECV);
-
-	return 0;
-}
+	int read = !!rcar_i2c_is_recv(priv);
 
-static int rcar_i2c_send(struct rcar_i2c_priv *priv)
-{
-	rcar_i2c_set_addr(priv, 0);
+	rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | read);
 	rcar_i2c_status_clear(priv);
 	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
-	rcar_i2c_write(priv, ICMIER, RCAR_IRQ_SEND);
+	rcar_i2c_write(priv, ICMIER, read ? RCAR_IRQ_RECV : RCAR_IRQ_SEND);
 
 	return 0;
 }
@@ -520,11 +504,7 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 		if (priv->msg = &msgs[num - 1])
 			rcar_i2c_flags_set(priv, ID_LAST_MSG);
 
-		/* start send/recv */
-		if (rcar_i2c_is_recv(priv))
-			ret = rcar_i2c_recv(priv);
-		else
-			ret = rcar_i2c_send(priv);
+		ret = rcar_i2c_prepare_msg(priv);
 
 		spin_unlock_irqrestore(&priv->lock, flags);
 		/*-------------- spin unlock -----------------*/
-- 
2.0.0.rc2


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

* [PATCH 06/11] i2c: rcar: refactor setting up msg
@ 2014-05-28  7:44     ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-sh-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang, Magnus Damm,
	Simon Horman, Laurent Pinchart, Geert Uytterhoeven,
	Kuninori Morimoto

From: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>

Setting up a read or write message is similar enough to be done in one
function. Also, move a helper function into the new function since it is
only used here.

Signed-off-by: Wolfram Sang <wsa+renesas-jBu1N2QxHDJrcw3mvpCnnVaTQe2KTcn/@public.gmane.org>
---
 drivers/i2c/busses/i2c-rcar.c | 30 +++++-------------------------
 1 file changed, 5 insertions(+), 25 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index a97a14955ddf..f7b14d049d5c 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -155,11 +155,6 @@ static void rcar_i2c_init(struct rcar_i2c_priv *priv)
 	rcar_i2c_write(priv, ICMAR, 0);
 }
 
-static void rcar_i2c_set_addr(struct rcar_i2c_priv *priv, u32 recv)
-{
-	rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | recv);
-}
-
 /*
  *		bus control functions
  */
@@ -279,25 +274,14 @@ static void rcar_i2c_status_bit_clear(struct rcar_i2c_priv *priv, u32 bit)
 	rcar_i2c_write(priv, ICMSR, ~bit);
 }
 
-/*
- *		recv/send functions
- */
-static int rcar_i2c_recv(struct rcar_i2c_priv *priv)
+static int rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
 {
-	rcar_i2c_set_addr(priv, 1);
-	rcar_i2c_status_clear(priv);
-	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
-	rcar_i2c_write(priv, ICMIER, RCAR_IRQ_RECV);
-
-	return 0;
-}
+	int read = !!rcar_i2c_is_recv(priv);
 
-static int rcar_i2c_send(struct rcar_i2c_priv *priv)
-{
-	rcar_i2c_set_addr(priv, 0);
+	rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | read);
 	rcar_i2c_status_clear(priv);
 	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
-	rcar_i2c_write(priv, ICMIER, RCAR_IRQ_SEND);
+	rcar_i2c_write(priv, ICMIER, read ? RCAR_IRQ_RECV : RCAR_IRQ_SEND);
 
 	return 0;
 }
@@ -520,11 +504,7 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 		if (priv->msg == &msgs[num - 1])
 			rcar_i2c_flags_set(priv, ID_LAST_MSG);
 
-		/* start send/recv */
-		if (rcar_i2c_is_recv(priv))
-			ret = rcar_i2c_recv(priv);
-		else
-			ret = rcar_i2c_send(priv);
+		ret = rcar_i2c_prepare_msg(priv);
 
 		spin_unlock_irqrestore(&priv->lock, flags);
 		/*-------------- spin unlock -----------------*/
-- 
2.0.0.rc2

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

* [PATCH 07/11] i2c: rcar: refactor status bit handling
  2014-05-28  7:44 ` Wolfram Sang
@ 2014-05-28  7:44   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

The old macros made it harder to see what was actually happening.
Replace them with something more readable.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index f7b14d049d5c..e82d64b5acef 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -87,6 +87,9 @@
 #define RCAR_IRQ_RECV	(MNRE | MALE | MSTE | MATE | MDRE)
 #define RCAR_IRQ_STOP	(MSTE)
 
+#define RCAR_IRQ_ACK_SEND	(~(MAT | MDE))
+#define RCAR_IRQ_ACK_RECV	(~(MAT | MDR))
+
 /*
  * flags
  */
@@ -268,27 +271,18 @@ scgd_find:
  *		status functions
  */
 
-#define rcar_i2c_status_clear(priv) rcar_i2c_status_bit_clear(priv, 0xffffffff)
-static void rcar_i2c_status_bit_clear(struct rcar_i2c_priv *priv, u32 bit)
-{
-	rcar_i2c_write(priv, ICMSR, ~bit);
-}
-
 static int rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
 {
 	int read = !!rcar_i2c_is_recv(priv);
 
 	rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | read);
-	rcar_i2c_status_clear(priv);
+	rcar_i2c_write(priv, ICMSR, 0);
 	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
 	rcar_i2c_write(priv, ICMIER, read ? RCAR_IRQ_RECV : RCAR_IRQ_SEND);
 
 	return 0;
 }
 
-#define rcar_i2c_send_restart(priv) rcar_i2c_status_bit_clear(priv, (MAT | MDE))
-#define rcar_i2c_recv_restart(priv) rcar_i2c_status_bit_clear(priv, (MAT | MDR))
-
 /*
  *		interrupt functions
  */
@@ -348,7 +342,7 @@ static int rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 			return ID_DONE;
 	}
 
-	rcar_i2c_send_restart(priv);
+	rcar_i2c_write(priv, ICMSR, RCAR_IRQ_ACK_SEND);
 
 	return 0;
 }
@@ -389,7 +383,7 @@ static int rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 	else
 		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_DATA);
 
-	rcar_i2c_recv_restart(priv);
+	rcar_i2c_write(priv, ICMSR, RCAR_IRQ_ACK_RECV);
 
 	return 0;
 }
@@ -452,7 +446,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 out:
 	if (rcar_i2c_flags_has(priv, ID_DONE)) {
 		rcar_i2c_write(priv, ICMIER, 0);
-		rcar_i2c_status_clear(priv);
+		rcar_i2c_write(priv, ICMSR, 0);
 		wake_up(&priv->wait);
 	}
 
-- 
2.0.0.rc2


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

* [PATCH 07/11] i2c: rcar: refactor status bit handling
@ 2014-05-28  7:44   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

The old macros made it harder to see what was actually happening.
Replace them with something more readable.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index f7b14d049d5c..e82d64b5acef 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -87,6 +87,9 @@
 #define RCAR_IRQ_RECV	(MNRE | MALE | MSTE | MATE | MDRE)
 #define RCAR_IRQ_STOP	(MSTE)
 
+#define RCAR_IRQ_ACK_SEND	(~(MAT | MDE))
+#define RCAR_IRQ_ACK_RECV	(~(MAT | MDR))
+
 /*
  * flags
  */
@@ -268,27 +271,18 @@ scgd_find:
  *		status functions
  */
 
-#define rcar_i2c_status_clear(priv) rcar_i2c_status_bit_clear(priv, 0xffffffff)
-static void rcar_i2c_status_bit_clear(struct rcar_i2c_priv *priv, u32 bit)
-{
-	rcar_i2c_write(priv, ICMSR, ~bit);
-}
-
 static int rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
 {
 	int read = !!rcar_i2c_is_recv(priv);
 
 	rcar_i2c_write(priv, ICMAR, (priv->msg->addr << 1) | read);
-	rcar_i2c_status_clear(priv);
+	rcar_i2c_write(priv, ICMSR, 0);
 	rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_START);
 	rcar_i2c_write(priv, ICMIER, read ? RCAR_IRQ_RECV : RCAR_IRQ_SEND);
 
 	return 0;
 }
 
-#define rcar_i2c_send_restart(priv) rcar_i2c_status_bit_clear(priv, (MAT | MDE))
-#define rcar_i2c_recv_restart(priv) rcar_i2c_status_bit_clear(priv, (MAT | MDR))
-
 /*
  *		interrupt functions
  */
@@ -348,7 +342,7 @@ static int rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 			return ID_DONE;
 	}
 
-	rcar_i2c_send_restart(priv);
+	rcar_i2c_write(priv, ICMSR, RCAR_IRQ_ACK_SEND);
 
 	return 0;
 }
@@ -389,7 +383,7 @@ static int rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 	else
 		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_DATA);
 
-	rcar_i2c_recv_restart(priv);
+	rcar_i2c_write(priv, ICMSR, RCAR_IRQ_ACK_RECV);
 
 	return 0;
 }
@@ -452,7 +446,7 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 out:
 	if (rcar_i2c_flags_has(priv, ID_DONE)) {
 		rcar_i2c_write(priv, ICMIER, 0);
-		rcar_i2c_status_clear(priv);
+		rcar_i2c_write(priv, ICMSR, 0);
 		wake_up(&priv->wait);
 	}
 
-- 
2.0.0.rc2


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

* [PATCH 08/11] i2c: rcar: remove spinlock
  2014-05-28  7:44 ` Wolfram Sang
@ 2014-05-28  7:44   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

The i2c core has per-adapter locks, so no need to protect again.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index e82d64b5acef..4b088e67f2fd 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -36,7 +36,6 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
-#include <linux/spinlock.h>
 
 /* register offsets */
 #define ICSCR	0x00	/* slave ctrl */
@@ -110,7 +109,6 @@ struct rcar_i2c_priv {
 	struct i2c_msg	*msg;
 	struct clk *clk;
 
-	spinlock_t lock;
 	wait_queue_head_t wait;
 
 	int pos;
@@ -394,9 +392,6 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 	struct device *dev = rcar_i2c_priv_to_dev(priv);
 	u32 msr;
 
-	/*-------------- spin lock -----------------*/
-	spin_lock(&priv->lock);
-
 	msr = rcar_i2c_read(priv, ICMSR);
 
 	/*
@@ -450,9 +445,6 @@ out:
 		wake_up(&priv->wait);
 	}
 
-	spin_unlock(&priv->lock);
-	/*-------------- spin unlock -----------------*/
-
 	return IRQ_HANDLED;
 }
 
@@ -462,21 +454,14 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 {
 	struct rcar_i2c_priv *priv = i2c_get_adapdata(adap);
 	struct device *dev = rcar_i2c_priv_to_dev(priv);
-	unsigned long flags;
 	int i, ret, timeout;
 
 	pm_runtime_get_sync(dev);
 
-	/*-------------- spin lock -----------------*/
-	spin_lock_irqsave(&priv->lock, flags);
-
 	rcar_i2c_init(priv);
 	/* start clock */
 	rcar_i2c_write(priv, ICCCR, priv->icccr);
 
-	spin_unlock_irqrestore(&priv->lock, flags);
-	/*-------------- spin unlock -----------------*/
-
 	ret = rcar_i2c_bus_barrier(priv);
 	if (ret < 0)
 		goto out;
@@ -488,9 +473,6 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 			break;
 		}
 
-		/*-------------- spin lock -----------------*/
-		spin_lock_irqsave(&priv->lock, flags);
-
 		/* init each data */
 		priv->msg	= &msgs[i];
 		priv->pos	= 0;
@@ -500,9 +482,6 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 
 		ret = rcar_i2c_prepare_msg(priv);
 
-		spin_unlock_irqrestore(&priv->lock, flags);
-		/*-------------- spin unlock -----------------*/
-
 		if (ret < 0)
 			break;
 
@@ -611,7 +590,6 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 
 	irq = platform_get_irq(pdev, 0);
 	init_waitqueue_head(&priv->wait);
-	spin_lock_init(&priv->lock);
 
 	adap			= &priv->adap;
 	adap->nr		= pdev->id;
-- 
2.0.0.rc2


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

* [PATCH 08/11] i2c: rcar: remove spinlock
@ 2014-05-28  7:44   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

The i2c core has per-adapter locks, so no need to protect again.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 22 ----------------------
 1 file changed, 22 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index e82d64b5acef..4b088e67f2fd 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -36,7 +36,6 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/slab.h>
-#include <linux/spinlock.h>
 
 /* register offsets */
 #define ICSCR	0x00	/* slave ctrl */
@@ -110,7 +109,6 @@ struct rcar_i2c_priv {
 	struct i2c_msg	*msg;
 	struct clk *clk;
 
-	spinlock_t lock;
 	wait_queue_head_t wait;
 
 	int pos;
@@ -394,9 +392,6 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 	struct device *dev = rcar_i2c_priv_to_dev(priv);
 	u32 msr;
 
-	/*-------------- spin lock -----------------*/
-	spin_lock(&priv->lock);
-
 	msr = rcar_i2c_read(priv, ICMSR);
 
 	/*
@@ -450,9 +445,6 @@ out:
 		wake_up(&priv->wait);
 	}
 
-	spin_unlock(&priv->lock);
-	/*-------------- spin unlock -----------------*/
-
 	return IRQ_HANDLED;
 }
 
@@ -462,21 +454,14 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 {
 	struct rcar_i2c_priv *priv = i2c_get_adapdata(adap);
 	struct device *dev = rcar_i2c_priv_to_dev(priv);
-	unsigned long flags;
 	int i, ret, timeout;
 
 	pm_runtime_get_sync(dev);
 
-	/*-------------- spin lock -----------------*/
-	spin_lock_irqsave(&priv->lock, flags);
-
 	rcar_i2c_init(priv);
 	/* start clock */
 	rcar_i2c_write(priv, ICCCR, priv->icccr);
 
-	spin_unlock_irqrestore(&priv->lock, flags);
-	/*-------------- spin unlock -----------------*/
-
 	ret = rcar_i2c_bus_barrier(priv);
 	if (ret < 0)
 		goto out;
@@ -488,9 +473,6 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 			break;
 		}
 
-		/*-------------- spin lock -----------------*/
-		spin_lock_irqsave(&priv->lock, flags);
-
 		/* init each data */
 		priv->msg	= &msgs[i];
 		priv->pos	= 0;
@@ -500,9 +482,6 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 
 		ret = rcar_i2c_prepare_msg(priv);
 
-		spin_unlock_irqrestore(&priv->lock, flags);
-		/*-------------- spin unlock -----------------*/
-
 		if (ret < 0)
 			break;
 
@@ -611,7 +590,6 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 
 	irq = platform_get_irq(pdev, 0);
 	init_waitqueue_head(&priv->wait);
-	spin_lock_init(&priv->lock);
 
 	adap			= &priv->adap;
 	adap->nr		= pdev->id;
-- 
2.0.0.rc2


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

* [PATCH 09/11] i2c: rcar: reuse status bits as enable bits
  2014-05-28  7:44 ` Wolfram Sang
@ 2014-05-28  7:44   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

Status register and enable register are identical regarding their
layout. Use the bit definitions for both.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 4b088e67f2fd..d489d54ebc6f 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -59,7 +59,7 @@
 #define FSB	(1 << 1)	/* force stop bit */
 #define ESG	(1 << 0)	/* en startbit gen */
 
-/* ICMSR */
+/* ICMSR (also for ICMIE) */
 #define MNR	(1 << 6)	/* nack received */
 #define MAL	(1 << 5)	/* arbitration lost */
 #define MST	(1 << 4)	/* sent a stop */
@@ -68,23 +68,14 @@
 #define MDR	(1 << 1)
 #define MAT	(1 << 0)	/* slave addr xfer done */
 
-/* ICMIE */
-#define MNRE	(1 << 6)	/* nack irq en */
-#define MALE	(1 << 5)	/* arblos irq en */
-#define MSTE	(1 << 4)	/* stop irq en */
-#define MDEE	(1 << 3)
-#define MDTE	(1 << 2)
-#define MDRE	(1 << 1)
-#define MATE	(1 << 0)	/* address sent irq en */
-
 
 #define RCAR_BUS_PHASE_START	(MDBS | MIE | ESG)
 #define RCAR_BUS_PHASE_DATA	(MDBS | MIE)
 #define RCAR_BUS_PHASE_STOP	(MDBS | MIE | FSB)
 
-#define RCAR_IRQ_SEND	(MNRE | MALE | MSTE | MATE | MDEE)
-#define RCAR_IRQ_RECV	(MNRE | MALE | MSTE | MATE | MDRE)
-#define RCAR_IRQ_STOP	(MSTE)
+#define RCAR_IRQ_SEND	(MNR | MAL | MST | MAT | MDE)
+#define RCAR_IRQ_RECV	(MNR | MAL | MST | MAT | MDR)
+#define RCAR_IRQ_STOP	(MST)
 
 #define RCAR_IRQ_ACK_SEND	(~(MAT | MDE))
 #define RCAR_IRQ_ACK_RECV	(~(MAT | MDR))
-- 
2.0.0.rc2


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

* [PATCH 09/11] i2c: rcar: reuse status bits as enable bits
@ 2014-05-28  7:44   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

Status register and enable register are identical regarding their
layout. Use the bit definitions for both.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 4b088e67f2fd..d489d54ebc6f 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -59,7 +59,7 @@
 #define FSB	(1 << 1)	/* force stop bit */
 #define ESG	(1 << 0)	/* en startbit gen */
 
-/* ICMSR */
+/* ICMSR (also for ICMIE) */
 #define MNR	(1 << 6)	/* nack received */
 #define MAL	(1 << 5)	/* arbitration lost */
 #define MST	(1 << 4)	/* sent a stop */
@@ -68,23 +68,14 @@
 #define MDR	(1 << 1)
 #define MAT	(1 << 0)	/* slave addr xfer done */
 
-/* ICMIE */
-#define MNRE	(1 << 6)	/* nack irq en */
-#define MALE	(1 << 5)	/* arblos irq en */
-#define MSTE	(1 << 4)	/* stop irq en */
-#define MDEE	(1 << 3)
-#define MDTE	(1 << 2)
-#define MDRE	(1 << 1)
-#define MATE	(1 << 0)	/* address sent irq en */
-
 
 #define RCAR_BUS_PHASE_START	(MDBS | MIE | ESG)
 #define RCAR_BUS_PHASE_DATA	(MDBS | MIE)
 #define RCAR_BUS_PHASE_STOP	(MDBS | MIE | FSB)
 
-#define RCAR_IRQ_SEND	(MNRE | MALE | MSTE | MATE | MDEE)
-#define RCAR_IRQ_RECV	(MNRE | MALE | MSTE | MATE | MDRE)
-#define RCAR_IRQ_STOP	(MSTE)
+#define RCAR_IRQ_SEND	(MNR | MAL | MST | MAT | MDE)
+#define RCAR_IRQ_RECV	(MNR | MAL | MST | MAT | MDR)
+#define RCAR_IRQ_STOP	(MST)
 
 #define RCAR_IRQ_ACK_SEND	(~(MAT | MDE))
 #define RCAR_IRQ_ACK_RECV	(~(MAT | MDR))
-- 
2.0.0.rc2


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

* [PATCH 10/11] i2c: rcar: janitorial cleanup after refactoring
  2014-05-28  7:44 ` Wolfram Sang
@ 2014-05-28  7:44   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

Remove some obvious comments, remove some superfluous debug output (the
error code carries the same information), some white space fixing...

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 50 +++++--------------------------------------
 1 file changed, 5 insertions(+), 45 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index d489d54ebc6f..b4cbbc412ca9 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -80,9 +80,6 @@
 #define RCAR_IRQ_ACK_SEND	(~(MAT | MDE))
 #define RCAR_IRQ_ACK_RECV	(~(MAT | MDR))
 
-/*
- * flags
- */
 #define ID_LAST_MSG	(1 << 0)
 #define ID_IOERROR	(1 << 1)
 #define ID_DONE		(1 << 2)
@@ -105,7 +102,7 @@ struct rcar_i2c_priv {
 	int pos;
 	u32 icccr;
 	u32 flags;
-	enum rcar_i2c_type	devtype;
+	enum rcar_i2c_type devtype;
 };
 
 #define rcar_i2c_priv_to_dev(p)		((p)->adap.dev.parent)
@@ -116,9 +113,7 @@ struct rcar_i2c_priv {
 
 #define LOOP_TIMEOUT	1024
 
-/*
- *		basic functions
- */
+
 static void rcar_i2c_write(struct rcar_i2c_priv *priv, int reg, u32 val)
 {
 	writel(val, priv->io + reg);
@@ -147,9 +142,6 @@ static void rcar_i2c_init(struct rcar_i2c_priv *priv)
 	rcar_i2c_write(priv, ICMAR, 0);
 }
 
-/*
- *		bus control functions
- */
 static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
 {
 	int i;
@@ -164,9 +156,6 @@ static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
 	return -EBUSY;
 }
 
-/*
- *		clock function
- */
 static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
 				    u32 bus_speed,
 				    struct device *dev)
@@ -256,10 +245,6 @@ scgd_find:
 	return 0;
 }
 
-/*
- *		status functions
- */
-
 static int rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
 {
 	int read = !!rcar_i2c_is_recv(priv);
@@ -380,40 +365,24 @@ static int rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 {
 	struct rcar_i2c_priv *priv = ptr;
-	struct device *dev = rcar_i2c_priv_to_dev(priv);
 	u32 msr;
 
 	msr = rcar_i2c_read(priv, ICMSR);
 
-	/*
-	 * Arbitration lost
-	 */
+	/* Arbitration lost */
 	if (msr & MAL) {
-		/*
-		 * CAUTION
-		 *
-		 * When arbitration lost, device become _slave_ mode.
-		 */
-		dev_dbg(dev, "Arbitration Lost\n");
 		rcar_i2c_flags_set(priv, (ID_DONE | ID_ARBLOST));
 		goto out;
 	}
 
-	/*
-	 * Stop
-	 */
+	/* Stop */
 	if (msr & MST) {
-		dev_dbg(dev, "Stop\n");
 		rcar_i2c_flags_set(priv, ID_DONE);
 		goto out;
 	}
 
-	/*
-	 * Nack
-	 */
+	/* Nack */
 	if (msr & MNR) {
-		dev_dbg(dev, "Nack\n");
-
 		/* go to stop phase */
 		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
 		rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP);
@@ -421,9 +390,6 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 		goto out;
 	}
 
-	/*
-	 * recv/send
-	 */
 	if (rcar_i2c_is_recv(priv))
 		rcar_i2c_flags_set(priv, rcar_i2c_irq_recv(priv, msr));
 	else
@@ -476,9 +442,6 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 		if (ret < 0)
 			break;
 
-		/*
-		 * wait result
-		 */
 		timeout = wait_event_timeout(priv->wait,
 					     rcar_i2c_flags_has(priv, ID_DONE),
 					     5 * HZ);
@@ -487,9 +450,6 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 			break;
 		}
 
-		/*
-		 * error handling
-		 */
 		if (rcar_i2c_flags_has(priv, ID_NACK)) {
 			ret = -ENXIO;
 			break;
-- 
2.0.0.rc2


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

* [PATCH 10/11] i2c: rcar: janitorial cleanup after refactoring
@ 2014-05-28  7:44   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

Remove some obvious comments, remove some superfluous debug output (the
error code carries the same information), some white space fixing...

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 50 +++++--------------------------------------
 1 file changed, 5 insertions(+), 45 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index d489d54ebc6f..b4cbbc412ca9 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -80,9 +80,6 @@
 #define RCAR_IRQ_ACK_SEND	(~(MAT | MDE))
 #define RCAR_IRQ_ACK_RECV	(~(MAT | MDR))
 
-/*
- * flags
- */
 #define ID_LAST_MSG	(1 << 0)
 #define ID_IOERROR	(1 << 1)
 #define ID_DONE		(1 << 2)
@@ -105,7 +102,7 @@ struct rcar_i2c_priv {
 	int pos;
 	u32 icccr;
 	u32 flags;
-	enum rcar_i2c_type	devtype;
+	enum rcar_i2c_type devtype;
 };
 
 #define rcar_i2c_priv_to_dev(p)		((p)->adap.dev.parent)
@@ -116,9 +113,7 @@ struct rcar_i2c_priv {
 
 #define LOOP_TIMEOUT	1024
 
-/*
- *		basic functions
- */
+
 static void rcar_i2c_write(struct rcar_i2c_priv *priv, int reg, u32 val)
 {
 	writel(val, priv->io + reg);
@@ -147,9 +142,6 @@ static void rcar_i2c_init(struct rcar_i2c_priv *priv)
 	rcar_i2c_write(priv, ICMAR, 0);
 }
 
-/*
- *		bus control functions
- */
 static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
 {
 	int i;
@@ -164,9 +156,6 @@ static int rcar_i2c_bus_barrier(struct rcar_i2c_priv *priv)
 	return -EBUSY;
 }
 
-/*
- *		clock function
- */
 static int rcar_i2c_clock_calculate(struct rcar_i2c_priv *priv,
 				    u32 bus_speed,
 				    struct device *dev)
@@ -256,10 +245,6 @@ scgd_find:
 	return 0;
 }
 
-/*
- *		status functions
- */
-
 static int rcar_i2c_prepare_msg(struct rcar_i2c_priv *priv)
 {
 	int read = !!rcar_i2c_is_recv(priv);
@@ -380,40 +365,24 @@ static int rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
 static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 {
 	struct rcar_i2c_priv *priv = ptr;
-	struct device *dev = rcar_i2c_priv_to_dev(priv);
 	u32 msr;
 
 	msr = rcar_i2c_read(priv, ICMSR);
 
-	/*
-	 * Arbitration lost
-	 */
+	/* Arbitration lost */
 	if (msr & MAL) {
-		/*
-		 * CAUTION
-		 *
-		 * When arbitration lost, device become _slave_ mode.
-		 */
-		dev_dbg(dev, "Arbitration Lost\n");
 		rcar_i2c_flags_set(priv, (ID_DONE | ID_ARBLOST));
 		goto out;
 	}
 
-	/*
-	 * Stop
-	 */
+	/* Stop */
 	if (msr & MST) {
-		dev_dbg(dev, "Stop\n");
 		rcar_i2c_flags_set(priv, ID_DONE);
 		goto out;
 	}
 
-	/*
-	 * Nack
-	 */
+	/* Nack */
 	if (msr & MNR) {
-		dev_dbg(dev, "Nack\n");
-
 		/* go to stop phase */
 		rcar_i2c_write(priv, ICMCR, RCAR_BUS_PHASE_STOP);
 		rcar_i2c_write(priv, ICMIER, RCAR_IRQ_STOP);
@@ -421,9 +390,6 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
 		goto out;
 	}
 
-	/*
-	 * recv/send
-	 */
 	if (rcar_i2c_is_recv(priv))
 		rcar_i2c_flags_set(priv, rcar_i2c_irq_recv(priv, msr));
 	else
@@ -476,9 +442,6 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 		if (ret < 0)
 			break;
 
-		/*
-		 * wait result
-		 */
 		timeout = wait_event_timeout(priv->wait,
 					     rcar_i2c_flags_has(priv, ID_DONE),
 					     5 * HZ);
@@ -487,9 +450,6 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 			break;
 		}
 
-		/*
-		 * error handling
-		 */
 		if (rcar_i2c_flags_has(priv, ID_NACK)) {
 			ret = -ENXIO;
 			break;
-- 
2.0.0.rc2


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

* [PATCH 11/11] i2c: rcar: update copyright and license information
  2014-05-28  7:44 ` Wolfram Sang
@ 2014-05-28  7:44   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

Make clear that the driver is GPL v2 only. Remove FSF address. Remove
filename in comment. Update copyright information.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index b4cbbc412ca9..f1ae1a3e187e 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -1,7 +1,9 @@
 /*
- *  drivers/i2c/busses/i2c-rcar.c
+ * Driver for the Renesas RCar I2C unit
  *
- * Copyright (C) 2012 Renesas Solutions Corp.
+ * Copyright (C) 2014 Wolfram Sang <wsa@sang-engineering.com>
+ *
+ * Copyright (C) 2012-14 Renesas Solutions Corp.
  * Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
  *
  * This file is based on the drivers/i2c/busses/i2c-sh7760.c
@@ -12,16 +14,12 @@
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License
+ * the Free Software Foundation; version 2 of the License.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include <linux/clk.h>
 #include <linux/delay.h>
@@ -605,6 +603,6 @@ static struct platform_driver rcar_i2c_driver = {
 
 module_platform_driver(rcar_i2c_driver);
 
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Renesas R-Car I2C bus driver");
 MODULE_AUTHOR("Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>");
-- 
2.0.0.rc2


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

* [PATCH 11/11] i2c: rcar: update copyright and license information
@ 2014-05-28  7:44   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-05-28  7:44 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Wolfram Sang, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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

Make clear that the driver is GPL v2 only. Remove FSF address. Remove
filename in comment. Update copyright information.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/i2c/busses/i2c-rcar.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index b4cbbc412ca9..f1ae1a3e187e 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -1,7 +1,9 @@
 /*
- *  drivers/i2c/busses/i2c-rcar.c
+ * Driver for the Renesas RCar I2C unit
  *
- * Copyright (C) 2012 Renesas Solutions Corp.
+ * Copyright (C) 2014 Wolfram Sang <wsa@sang-engineering.com>
+ *
+ * Copyright (C) 2012-14 Renesas Solutions Corp.
  * Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
  *
  * This file is based on the drivers/i2c/busses/i2c-sh7760.c
@@ -12,16 +14,12 @@
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License
+ * the Free Software Foundation; version 2 of the License.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
 #include <linux/clk.h>
 #include <linux/delay.h>
@@ -605,6 +603,6 @@ static struct platform_driver rcar_i2c_driver = {
 
 module_platform_driver(rcar_i2c_driver);
 
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Renesas R-Car I2C bus driver");
 MODULE_AUTHOR("Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>");
-- 
2.0.0.rc2


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

* Re: [PATCH 00/11] i2c: rcar: cleanup series
  2014-05-28  7:44 ` Wolfram Sang
@ 2014-05-28  8:11   ` Kuninori Morimoto
  -1 siblings, 0 replies; 38+ messages in thread
From: Kuninori Morimoto @ 2014-05-28  8:11 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, linux-sh, Magnus Damm, Simon Horman, Laurent Pinchart,
	Geert Uytterhoeven


Hi

> Here are some patches to let the rcar driver have a diet without losing
> functionality. IMO the code becomes more readable, too, although mileages may
> vary about this or that change. Bus-free handling is improved, too. Tested with
> renesas-devel-v3.15-rc7-20140526 on a Lager and Koelsch board. The git tree can
> be found here, including one proof-of-concept enablement patch:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/rcar-cleanup
> 
> Please test, review, comment.
> 
> Thanks,
> 
>    Wolfram
> 
> 
> Wolfram Sang (11):
>   i2c: rcar: not everything needs to be a function
>   i2c: rcar: no need to store irq number
>   i2c: rcar: refactor bus state machine
>   i2c: rcar: refactor irq state machine
>   i2c: rcar: check bus free before first message
>   i2c: rcar: refactor setting up msg
>   i2c: rcar: refactor status bit handling
>   i2c: rcar: remove spinlock
>   i2c: rcar: reuse status bits as enable bits
>   i2c: rcar: janitorial cleanup after refactoring
>   i2c: rcar: update copyright and license information
> 
>  drivers/i2c/busses/i2c-rcar.c | 252 ++++++++----------------------------------
>  1 file changed, 48 insertions(+), 204 deletions(-)

For all patches

Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>


My small opinion is that keeping comment of "Arbitration lost"
(= "When arbitration lost, device become _slave_ mode")
is helpful for non-renesas freak.
I forgot detail, but, this behavior is not documented (?)


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

* Re: [PATCH 00/11] i2c: rcar: cleanup series
@ 2014-05-28  8:11   ` Kuninori Morimoto
  0 siblings, 0 replies; 38+ messages in thread
From: Kuninori Morimoto @ 2014-05-28  8:11 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-i2c, linux-sh, Magnus Damm, Simon Horman, Laurent Pinchart,
	Geert Uytterhoeven


Hi

> Here are some patches to let the rcar driver have a diet without losing
> functionality. IMO the code becomes more readable, too, although mileages may
> vary about this or that change. Bus-free handling is improved, too. Tested with
> renesas-devel-v3.15-rc7-20140526 on a Lager and Koelsch board. The git tree can
> be found here, including one proof-of-concept enablement patch:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/rcar-cleanup
> 
> Please test, review, comment.
> 
> Thanks,
> 
>    Wolfram
> 
> 
> Wolfram Sang (11):
>   i2c: rcar: not everything needs to be a function
>   i2c: rcar: no need to store irq number
>   i2c: rcar: refactor bus state machine
>   i2c: rcar: refactor irq state machine
>   i2c: rcar: check bus free before first message
>   i2c: rcar: refactor setting up msg
>   i2c: rcar: refactor status bit handling
>   i2c: rcar: remove spinlock
>   i2c: rcar: reuse status bits as enable bits
>   i2c: rcar: janitorial cleanup after refactoring
>   i2c: rcar: update copyright and license information
> 
>  drivers/i2c/busses/i2c-rcar.c | 252 ++++++++----------------------------------
>  1 file changed, 48 insertions(+), 204 deletions(-)

For all patches

Acked-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>


My small opinion is that keeping comment of "Arbitration lost"
(= "When arbitration lost, device become _slave_ mode")
is helpful for non-renesas freak.
I forgot detail, but, this behavior is not documented (?)


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

* Re: [PATCH 00/11] i2c: rcar: cleanup series
       [not found] ` <1401263086-13720-1-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
@ 2014-05-28  8:26     ` Geert Uytterhoeven
  2014-05-28  7:44     ` Wolfram Sang
  2014-05-28  8:26     ` Geert Uytterhoeven
  2 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2014-05-28  8:26 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm, Simon Horman,
	Laurent Pinchart, Kuninori Morimoto

Hi Wolfram,

On Wed, May 28, 2014 at 9:44 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> Wolfram Sang (11):
>   i2c: rcar: not everything needs to be a function
>   i2c: rcar: no need to store irq number
>   i2c: rcar: refactor bus state machine
>   i2c: rcar: refactor irq state machine
>   i2c: rcar: check bus free before first message
>   i2c: rcar: refactor setting up msg
>   i2c: rcar: refactor status bit handling
>   i2c: rcar: remove spinlock
>   i2c: rcar: reuse status bits as enable bits
>   i2c: rcar: janitorial cleanup after refactoring
>   i2c: rcar: update copyright and license information

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 00/11] i2c: rcar: cleanup series
@ 2014-05-28  8:26     ` Geert Uytterhoeven
  0 siblings, 0 replies; 38+ messages in thread
From: Geert Uytterhoeven @ 2014-05-28  8:26 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: Linux I2C, Linux-sh list, Magnus Damm, Simon Horman,
	Laurent Pinchart, Kuninori Morimoto

Hi Wolfram,

On Wed, May 28, 2014 at 9:44 AM, Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org> wrote:
> Wolfram Sang (11):
>   i2c: rcar: not everything needs to be a function
>   i2c: rcar: no need to store irq number
>   i2c: rcar: refactor bus state machine
>   i2c: rcar: refactor irq state machine
>   i2c: rcar: check bus free before first message
>   i2c: rcar: refactor setting up msg
>   i2c: rcar: refactor status bit handling
>   i2c: rcar: remove spinlock
>   i2c: rcar: reuse status bits as enable bits
>   i2c: rcar: janitorial cleanup after refactoring
>   i2c: rcar: update copyright and license information

Acked-by: Geert Uytterhoeven <geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

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

* Re: [PATCH 00/11] i2c: rcar: cleanup series
  2014-05-28  7:44 ` Wolfram Sang
@ 2014-06-01 20:24   ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-06-01 20:24 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Magnus Damm, Simon Horman, Laurent Pinchart,
	Geert Uytterhoeven, Kuninori Morimoto

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

On Wed, May 28, 2014 at 09:44:35AM +0200, Wolfram Sang wrote:
> Here are some patches to let the rcar driver have a diet without losing
> functionality. IMO the code becomes more readable, too, although mileages may
> vary about this or that change. Bus-free handling is improved, too. Tested with
> renesas-devel-v3.15-rc7-20140526 on a Lager and Koelsch board. The git tree can
> be found here, including one proof-of-concept enablement patch:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/rcar-cleanup
> 
> Please test, review, comment.
> 
> Thanks,
> 
>    Wolfram
> 
> 
> Wolfram Sang (11):
>   i2c: rcar: not everything needs to be a function
>   i2c: rcar: no need to store irq number
>   i2c: rcar: refactor bus state machine
>   i2c: rcar: refactor irq state machine
>   i2c: rcar: check bus free before first message
>   i2c: rcar: refactor setting up msg
>   i2c: rcar: refactor status bit handling
>   i2c: rcar: remove spinlock
>   i2c: rcar: reuse status bits as enable bits
>   i2c: rcar: janitorial cleanup after refactoring
>   i2c: rcar: update copyright and license information
> 

Applied to for-next, thanks!


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

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

* Re: [PATCH 00/11] i2c: rcar: cleanup series
@ 2014-06-01 20:24   ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-06-01 20:24 UTC (permalink / raw)
  To: linux-i2c
  Cc: linux-sh, Magnus Damm, Simon Horman, Laurent Pinchart,
	Geert Uytterhoeven, Kuninori Morimoto

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

On Wed, May 28, 2014 at 09:44:35AM +0200, Wolfram Sang wrote:
> Here are some patches to let the rcar driver have a diet without losing
> functionality. IMO the code becomes more readable, too, although mileages may
> vary about this or that change. Bus-free handling is improved, too. Tested with
> renesas-devel-v3.15-rc7-20140526 on a Lager and Koelsch board. The git tree can
> be found here, including one proof-of-concept enablement patch:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/rcar-cleanup
> 
> Please test, review, comment.
> 
> Thanks,
> 
>    Wolfram
> 
> 
> Wolfram Sang (11):
>   i2c: rcar: not everything needs to be a function
>   i2c: rcar: no need to store irq number
>   i2c: rcar: refactor bus state machine
>   i2c: rcar: refactor irq state machine
>   i2c: rcar: check bus free before first message
>   i2c: rcar: refactor setting up msg
>   i2c: rcar: refactor status bit handling
>   i2c: rcar: remove spinlock
>   i2c: rcar: reuse status bits as enable bits
>   i2c: rcar: janitorial cleanup after refactoring
>   i2c: rcar: update copyright and license information
> 

Applied to for-next, thanks!


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

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

* Re: [PATCH 08/11] i2c: rcar: remove spinlock
  2014-05-28  7:44   ` Wolfram Sang
@ 2014-08-22 23:54     ` Sergei Shtylyov
  -1 siblings, 0 replies; 38+ messages in thread
From: Sergei Shtylyov @ 2014-08-22 23:54 UTC (permalink / raw)
  To: Wolfram Sang, linux-i2c
  Cc: linux-sh, Magnus Damm, Simon Horman, Laurent Pinchart,
	Geert Uytterhoeven, Kuninori Morimoto

Hello.

On 05/28/2014 11:44 AM, Wolfram Sang wrote:

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

> The i2c core has per-adapter locks, so no need to protect again.

    The core's lock is unable to protect from the IRQs. So I'm proposing to 
revert this patch. It's a pity I hadn't noticed this issue when the patch was 
posted.

> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>   drivers/i2c/busses/i2c-rcar.c | 22 ----------------------
>   1 file changed, 22 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index e82d64b5acef..4b088e67f2fd 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
[...]
> @@ -110,7 +109,6 @@ struct rcar_i2c_priv {
>   	struct i2c_msg	*msg;
>   	struct clk *clk;
>
> -	spinlock_t lock;

    Needed a comment (that it protects the I2C registers).

[...]
> @@ -462,21 +454,14 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
>   {
>   	struct rcar_i2c_priv *priv = i2c_get_adapdata(adap);
>   	struct device *dev = rcar_i2c_priv_to_dev(priv);
> -	unsigned long flags;
>   	int i, ret, timeout;
>
>   	pm_runtime_get_sync(dev);
>
> -	/*-------------- spin lock -----------------*/
> -	spin_lock_irqsave(&priv->lock, flags);
> -
>   	rcar_i2c_init(priv);
>   	/* start clock */
>   	rcar_i2c_write(priv, ICCCR, priv->icccr);
>
> -	spin_unlock_irqrestore(&priv->lock, flags);
> -	/*-------------- spin unlock -----------------*/
> -

    I'm afraid this unlock is misplaced, the code continues to access the 
registers.

>   	ret = rcar_i2c_bus_barrier(priv);

    Should probably unlock here instead?

>   	if (ret < 0)
>   		goto out;

WBR, Sergei


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

* Re: [PATCH 08/11] i2c: rcar: remove spinlock
@ 2014-08-22 23:54     ` Sergei Shtylyov
  0 siblings, 0 replies; 38+ messages in thread
From: Sergei Shtylyov @ 2014-08-22 23:54 UTC (permalink / raw)
  To: Wolfram Sang, linux-i2c
  Cc: linux-sh, Magnus Damm, Simon Horman, Laurent Pinchart,
	Geert Uytterhoeven, Kuninori Morimoto

Hello.

On 05/28/2014 11:44 AM, Wolfram Sang wrote:

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

> The i2c core has per-adapter locks, so no need to protect again.

    The core's lock is unable to protect from the IRQs. So I'm proposing to 
revert this patch. It's a pity I hadn't noticed this issue when the patch was 
posted.

> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>   drivers/i2c/busses/i2c-rcar.c | 22 ----------------------
>   1 file changed, 22 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index e82d64b5acef..4b088e67f2fd 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
[...]
> @@ -110,7 +109,6 @@ struct rcar_i2c_priv {
>   	struct i2c_msg	*msg;
>   	struct clk *clk;
>
> -	spinlock_t lock;

    Needed a comment (that it protects the I2C registers).

[...]
> @@ -462,21 +454,14 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
>   {
>   	struct rcar_i2c_priv *priv = i2c_get_adapdata(adap);
>   	struct device *dev = rcar_i2c_priv_to_dev(priv);
> -	unsigned long flags;
>   	int i, ret, timeout;
>
>   	pm_runtime_get_sync(dev);
>
> -	/*-------------- spin lock -----------------*/
> -	spin_lock_irqsave(&priv->lock, flags);
> -
>   	rcar_i2c_init(priv);
>   	/* start clock */
>   	rcar_i2c_write(priv, ICCCR, priv->icccr);
>
> -	spin_unlock_irqrestore(&priv->lock, flags);
> -	/*-------------- spin unlock -----------------*/
> -

    I'm afraid this unlock is misplaced, the code continues to access the 
registers.

>   	ret = rcar_i2c_bus_barrier(priv);

    Should probably unlock here instead?

>   	if (ret < 0)
>   		goto out;

WBR, Sergei


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

* Re: [PATCH 08/11] i2c: rcar: remove spinlock
  2014-08-22 23:54     ` Sergei Shtylyov
@ 2014-08-23  3:08       ` Wolfram Sang
  -1 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-08-23  3:08 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: linux-i2c, linux-sh, Magnus Damm, Simon Horman, Laurent Pinchart,
	Geert Uytterhoeven, Kuninori Morimoto

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


> >The i2c core has per-adapter locks, so no need to protect again.
> 
>    The core's lock is unable to protect from the IRQs. So I'm proposing to
> revert this patch. It's a pity I hadn't noticed this issue when the patch
> was posted.

Yes, you are right. I noticed a while ago, too, but then got
side-tracked :(

>    I'm afraid this unlock is misplaced, the code continues to access the
> registers.
> 
> >  	ret = rcar_i2c_bus_barrier(priv);
> 
>    Should probably unlock here instead?

I can check details next week earliest. If you are confident with your
suggestions, feel free to send one patch with the revert and the updates
you mentioned.

Thanks,

   Wolfram


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

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

* Re: [PATCH 08/11] i2c: rcar: remove spinlock
@ 2014-08-23  3:08       ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-08-23  3:08 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: linux-i2c, linux-sh, Magnus Damm, Simon Horman, Laurent Pinchart,
	Geert Uytterhoeven, Kuninori Morimoto

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


> >The i2c core has per-adapter locks, so no need to protect again.
> 
>    The core's lock is unable to protect from the IRQs. So I'm proposing to
> revert this patch. It's a pity I hadn't noticed this issue when the patch
> was posted.

Yes, you are right. I noticed a while ago, too, but then got
side-tracked :(

>    I'm afraid this unlock is misplaced, the code continues to access the
> registers.
> 
> >  	ret = rcar_i2c_bus_barrier(priv);
> 
>    Should probably unlock here instead?

I can check details next week earliest. If you are confident with your
suggestions, feel free to send one patch with the revert and the updates
you mentioned.

Thanks,

   Wolfram


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

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

* Re: [PATCH 08/11] i2c: rcar: remove spinlock
  2014-08-22 23:54     ` Sergei Shtylyov
@ 2014-09-02 22:10       ` Sergei Shtylyov
  -1 siblings, 0 replies; 38+ messages in thread
From: Sergei Shtylyov @ 2014-09-02 22:10 UTC (permalink / raw)
  To: Wolfram Sang, linux-i2c
  Cc: linux-sh, Magnus Damm, Simon Horman, Laurent Pinchart,
	Geert Uytterhoeven, Kuninori Morimoto

On 08/23/2014 03:54 AM, Sergei Shtylyov wrote:

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

>> The i2c core has per-adapter locks, so no need to protect again.

>     The core's lock is unable to protect from the IRQs. So I'm proposing to
> revert this patch. It's a pity I hadn't noticed this issue when the patch was
> posted.

>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> ---
>>   drivers/i2c/busses/i2c-rcar.c | 22 ----------------------
>>   1 file changed, 22 deletions(-)
>> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
>> index e82d64b5acef..4b088e67f2fd 100644
>> --- a/drivers/i2c/busses/i2c-rcar.c
>> +++ b/drivers/i2c/busses/i2c-rcar.c
[...]
>> @@ -462,21 +454,14 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
>>   {
>>       struct rcar_i2c_priv *priv = i2c_get_adapdata(adap);
>>       struct device *dev = rcar_i2c_priv_to_dev(priv);
>> -    unsigned long flags;
>>       int i, ret, timeout;
>>
>>       pm_runtime_get_sync(dev);
>>
>> -    /*-------------- spin lock -----------------*/
>> -    spin_lock_irqsave(&priv->lock, flags);
>> -
>>       rcar_i2c_init(priv);
>>       /* start clock */
>>       rcar_i2c_write(priv, ICCCR, priv->icccr);
>>
>> -    spin_unlock_irqrestore(&priv->lock, flags);
>> -    /*-------------- spin unlock -----------------*/
>> -

>     I'm afraid this unlock is misplaced, the code continues to access the
> registers.

>>       ret = rcar_i2c_bus_barrier(priv);

>     Should probably unlock here instead?

    After looking at the code, it looks like it was a false alarm on my side.
The I2C interrupts are disabled here and other threads should be locked out by 
the I2C core locks. So it doesn't make sense to hold IRQs disabled during a 
possibly long polling loop in rcar_i2c_bus_barrier(). So I'll just repost the 
revert patch (if still needed).

>>       if (ret < 0)
>>           goto out;

WBR, Sergei


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

* Re: [PATCH 08/11] i2c: rcar: remove spinlock
@ 2014-09-02 22:10       ` Sergei Shtylyov
  0 siblings, 0 replies; 38+ messages in thread
From: Sergei Shtylyov @ 2014-09-02 22:10 UTC (permalink / raw)
  To: Wolfram Sang, linux-i2c
  Cc: linux-sh, Magnus Damm, Simon Horman, Laurent Pinchart,
	Geert Uytterhoeven, Kuninori Morimoto

On 08/23/2014 03:54 AM, Sergei Shtylyov wrote:

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

>> The i2c core has per-adapter locks, so no need to protect again.

>     The core's lock is unable to protect from the IRQs. So I'm proposing to
> revert this patch. It's a pity I hadn't noticed this issue when the patch was
> posted.

>> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>> ---
>>   drivers/i2c/busses/i2c-rcar.c | 22 ----------------------
>>   1 file changed, 22 deletions(-)
>> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
>> index e82d64b5acef..4b088e67f2fd 100644
>> --- a/drivers/i2c/busses/i2c-rcar.c
>> +++ b/drivers/i2c/busses/i2c-rcar.c
[...]
>> @@ -462,21 +454,14 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
>>   {
>>       struct rcar_i2c_priv *priv = i2c_get_adapdata(adap);
>>       struct device *dev = rcar_i2c_priv_to_dev(priv);
>> -    unsigned long flags;
>>       int i, ret, timeout;
>>
>>       pm_runtime_get_sync(dev);
>>
>> -    /*-------------- spin lock -----------------*/
>> -    spin_lock_irqsave(&priv->lock, flags);
>> -
>>       rcar_i2c_init(priv);
>>       /* start clock */
>>       rcar_i2c_write(priv, ICCCR, priv->icccr);
>>
>> -    spin_unlock_irqrestore(&priv->lock, flags);
>> -    /*-------------- spin unlock -----------------*/
>> -

>     I'm afraid this unlock is misplaced, the code continues to access the
> registers.

>>       ret = rcar_i2c_bus_barrier(priv);

>     Should probably unlock here instead?

    After looking at the code, it looks like it was a false alarm on my side.
The I2C interrupts are disabled here and other threads should be locked out by 
the I2C core locks. So it doesn't make sense to hold IRQs disabled during a 
possibly long polling loop in rcar_i2c_bus_barrier(). So I'll just repost the 
revert patch (if still needed).

>>       if (ret < 0)
>>           goto out;

WBR, Sergei


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

* Re: [PATCH 08/11] i2c: rcar: remove spinlock
       [not found]       ` <5406404A.7090509-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
@ 2014-09-02 22:18           ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-09-02 22:18 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-sh-u79uwXL29TY76Z2rM5mHXA, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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


>    After looking at the code, it looks like it was a false alarm on my side.
> The I2C interrupts are disabled here and other threads should be locked out
> by the I2C core locks. So it doesn't make sense to hold IRQs disabled during
> a possibly long polling loop in rcar_i2c_bus_barrier(). So I'll just repost
> the revert patch (if still needed).

Nope. Thanks for looking into it.


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

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

* Re: [PATCH 08/11] i2c: rcar: remove spinlock
@ 2014-09-02 22:18           ` Wolfram Sang
  0 siblings, 0 replies; 38+ messages in thread
From: Wolfram Sang @ 2014-09-02 22:18 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-sh-u79uwXL29TY76Z2rM5mHXA, Magnus Damm, Simon Horman,
	Laurent Pinchart, Geert Uytterhoeven, Kuninori Morimoto

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


>    After looking at the code, it looks like it was a false alarm on my side.
> The I2C interrupts are disabled here and other threads should be locked out
> by the I2C core locks. So it doesn't make sense to hold IRQs disabled during
> a possibly long polling loop in rcar_i2c_bus_barrier(). So I'll just repost
> the revert patch (if still needed).

Nope. Thanks for looking into it.


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

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

end of thread, other threads:[~2014-09-02 22:18 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-28  7:44 [PATCH 00/11] i2c: rcar: cleanup series Wolfram Sang
2014-05-28  7:44 ` Wolfram Sang
2014-05-28  7:44 ` [PATCH 01/11] i2c: rcar: not everything needs to be a function Wolfram Sang
2014-05-28  7:44   ` Wolfram Sang
2014-05-28  7:44 ` [PATCH 02/11] i2c: rcar: no need to store irq number Wolfram Sang
2014-05-28  7:44   ` Wolfram Sang
2014-05-28  7:44 ` [PATCH 04/11] i2c: rcar: refactor irq state machine Wolfram Sang
2014-05-28  7:44   ` Wolfram Sang
2014-05-28  7:44 ` [PATCH 05/11] i2c: rcar: check bus free before first message Wolfram Sang
2014-05-28  7:44   ` Wolfram Sang
2014-05-28  7:44 ` [PATCH 07/11] i2c: rcar: refactor status bit handling Wolfram Sang
2014-05-28  7:44   ` Wolfram Sang
2014-05-28  7:44 ` [PATCH 08/11] i2c: rcar: remove spinlock Wolfram Sang
2014-05-28  7:44   ` Wolfram Sang
2014-08-22 23:54   ` Sergei Shtylyov
2014-08-22 23:54     ` Sergei Shtylyov
2014-08-23  3:08     ` Wolfram Sang
2014-08-23  3:08       ` Wolfram Sang
2014-09-02 22:10     ` Sergei Shtylyov
2014-09-02 22:10       ` Sergei Shtylyov
     [not found]       ` <5406404A.7090509-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
2014-09-02 22:18         ` Wolfram Sang
2014-09-02 22:18           ` Wolfram Sang
2014-05-28  7:44 ` [PATCH 09/11] i2c: rcar: reuse status bits as enable bits Wolfram Sang
2014-05-28  7:44   ` Wolfram Sang
2014-05-28  7:44 ` [PATCH 10/11] i2c: rcar: janitorial cleanup after refactoring Wolfram Sang
2014-05-28  7:44   ` Wolfram Sang
2014-05-28  7:44 ` [PATCH 11/11] i2c: rcar: update copyright and license information Wolfram Sang
2014-05-28  7:44   ` Wolfram Sang
2014-05-28  8:11 ` [PATCH 00/11] i2c: rcar: cleanup series Kuninori Morimoto
2014-05-28  8:11   ` Kuninori Morimoto
     [not found] ` <1401263086-13720-1-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
2014-05-28  7:44   ` [PATCH 03/11] i2c: rcar: refactor bus state machine Wolfram Sang
2014-05-28  7:44     ` Wolfram Sang
2014-05-28  7:44   ` [PATCH 06/11] i2c: rcar: refactor setting up msg Wolfram Sang
2014-05-28  7:44     ` Wolfram Sang
2014-05-28  8:26   ` [PATCH 00/11] i2c: rcar: cleanup series Geert Uytterhoeven
2014-05-28  8:26     ` Geert Uytterhoeven
2014-06-01 20:24 ` Wolfram Sang
2014-06-01 20:24   ` 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.