[PATCH 0/3] i2c: rcar: make DMA more robust

[PATCH 0/3] i2c: rcar: make DMA more robust

[Wolfram Sang]

The Renesas BSP has a patch to wait for the data register being empty before
starting TX-DMA. This is according to the data sheet, so this small series does
the same in patch 2 & 3. However, refactored, so there is less to compute and
it is hopefully also easy to read. During that work, I noticed an implicit
assumption about a required minimal DMA length. It is now explicit and
documented :)

Tested on a Renesas Salvator XS (R-Car M3N) and now regressions found when
writing data with DMA.

I am unsure if patches 2+3 should be squashed or not, but at least for
reviewing, seperate patches is easier.

Glad to hear what you think,

   Wolfram


Wolfram Sang (3):
  i2c: rcar: sanity check for minimal DMA length
  i2c: rcar: let DMA enable routine return success status
  i2c: rcar: wait for data empty before starting DMA

 drivers/i2c/busses/i2c-rcar.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

-- 
2.11.0

[PATCH 1/3] i2c: rcar: sanity check for minimal DMA length

[Wolfram Sang]

Use a macro for the hardcoded value and apply a build check. If it is
not met, the driver logic will not work anymore.

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

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 3ce74edcd70c..925858915569 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -85,6 +85,7 @@
 /* ICFBSCR */
 #define TCYC17	0x0f		/* 17*Tcyc delay 1st bit between SDA and SCL */
 
+#define RCAR_MIN_DMA_LEN	8
 
 #define RCAR_BUS_PHASE_START	(MDBS | MIE | ESG)
 #define RCAR_BUS_PHASE_DATA	(MDBS | MIE)
@@ -412,8 +413,8 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
 	int len;
 
 	/* Do various checks to see if DMA is feasible at all */
-	if (IS_ERR(chan) || msg->len < 8 || !(msg->flags & I2C_M_DMA_SAFE) ||
-	    (read && priv->flags & ID_P_NO_RXDMA))
+	if (IS_ERR(chan) || msg->len < RCAR_MIN_DMA_LEN ||
+	    !(msg->flags & I2C_M_DMA_SAFE) || (read && priv->flags & ID_P_NO_RXDMA))
 		return;
 
 	if (read) {
@@ -921,6 +922,9 @@ static int rcar_i2c_probe(struct platform_device *pdev)
 	struct i2c_timings i2c_t;
 	int irq, ret;
 
+	/* Otherwise logic will break because some bytes must always use PIO */
+	BUILD_BUG_ON_MSG(RCAR_MIN_DMA_LEN < 3, "Invalid min DMA length");
+
 	priv = devm_kzalloc(dev, sizeof(struct rcar_i2c_priv), GFP_KERNEL);
 	if (!priv)
 		return -ENOMEM;
-- 
2.11.0

Re: [PATCH 1/3] i2c: rcar: sanity check for minimal DMA length

[Geert Uytterhoeven]

On Tue, Mar 5, 2019 at 7:52 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Use a macro for the hardcoded value and apply a build check. If it is
> not met, the driver logic will not work anymore.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH 1/3] i2c: rcar: sanity check for minimal DMA length

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Mar 5, 2019 at 7:52 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Use a macro for the hardcoded value and apply a build check. If it is
> not met, the driver logic will not work anymore.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/i2c/busses/i2c-rcar.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index 3ce74edcd70c..925858915569 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c

> @@ -921,6 +922,9 @@ static int rcar_i2c_probe(struct platform_device *pdev)
>         struct i2c_timings i2c_t;
>         int irq, ret;
>
> +       /* Otherwise logic will break because some bytes must always use PIO */
> +       BUILD_BUG_ON_MSG(RCAR_MIN_DMA_LEN < 3, "Invalid min DMA length");

Given patch 3/3, it should still work with RCAR_MIN_DMA_LEN == 2, right?

> +
>         priv = devm_kzalloc(dev, sizeof(struct rcar_i2c_priv), GFP_KERNEL);
>         if (!priv)
>                 return -ENOMEM;

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

Re: [PATCH 1/3] i2c: rcar: sanity check for minimal DMA length

[Wolfram Sang]

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

On Mon, Mar 11, 2019 at 11:08:13AM +0100, Geert Uytterhoeven wrote:
> Hi Wolfram,
> 
> On Tue, Mar 5, 2019 at 7:52 PM Wolfram Sang
> <wsa+renesas@sang-engineering.com> wrote:
> > Use a macro for the hardcoded value and apply a build check. If it is
> > not met, the driver logic will not work anymore.
> >
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> > ---
> >  drivers/i2c/busses/i2c-rcar.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> > index 3ce74edcd70c..925858915569 100644
> > --- a/drivers/i2c/busses/i2c-rcar.c
> > +++ b/drivers/i2c/busses/i2c-rcar.c
> 
> > @@ -921,6 +922,9 @@ static int rcar_i2c_probe(struct platform_device *pdev)
> >         struct i2c_timings i2c_t;
> >         int irq, ret;
> >
> > +       /* Otherwise logic will break because some bytes must always use PIO */
> > +       BUILD_BUG_ON_MSG(RCAR_MIN_DMA_LEN < 3, "Invalid min DMA length");
> 
> Given patch 3/3, it should still work with RCAR_MIN_DMA_LEN == 2, right?

Nope. It is not that we transfer one byte more with PIO now. The change
in patch 3 is that we explicitly wait for an interrupt when the (already
existing) PIO transfer ended. Before that patch, we assumed DMA would
take over on its own once the data register is empty again. Should I
update the commit message to make this more clear?

Also, it is the _read_ case which needs the minimum lenght of 3. This is
fixing the _write_ code path :)


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

Re: [PATCH 1/3] i2c: rcar: sanity check for minimal DMA length

[Simon Horman]

On Tue, Mar 05, 2019 at 06:54:32PM +0100, Wolfram Sang wrote:
> Use a macro for the hardcoded value and apply a build check. If it is
> not met, the driver logic will not work anymore.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/i2c/busses/i2c-rcar.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index 3ce74edcd70c..925858915569 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -85,6 +85,7 @@
>  /* ICFBSCR */
>  #define TCYC17	0x0f		/* 17*Tcyc delay 1st bit between SDA and SCL */
>  
> +#define RCAR_MIN_DMA_LEN	8
>  
>  #define RCAR_BUS_PHASE_START	(MDBS | MIE | ESG)
>  #define RCAR_BUS_PHASE_DATA	(MDBS | MIE)
> @@ -412,8 +413,8 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
>  	int len;
>  
>  	/* Do various checks to see if DMA is feasible at all */
> -	if (IS_ERR(chan) || msg->len < 8 || !(msg->flags & I2C_M_DMA_SAFE) ||
> -	    (read && priv->flags & ID_P_NO_RXDMA))
> +	if (IS_ERR(chan) || msg->len < RCAR_MIN_DMA_LEN ||
> +	    !(msg->flags & I2C_M_DMA_SAFE) || (read && priv->flags & ID_P_NO_RXDMA))
>  		return;
>  
>  	if (read) {
> @@ -921,6 +922,9 @@ static int rcar_i2c_probe(struct platform_device *pdev)
>  	struct i2c_timings i2c_t;
>  	int irq, ret;
>  
> +	/* Otherwise logic will break because some bytes must always use PIO */
> +	BUILD_BUG_ON_MSG(RCAR_MIN_DMA_LEN < 3, "Invalid min DMA length");
> +
>  	priv = devm_kzalloc(dev, sizeof(struct rcar_i2c_priv), GFP_KERNEL);
>  	if (!priv)
>  		return -ENOMEM;
> -- 
> 2.11.0
> 

Re: [PATCH 1/3] i2c: rcar: sanity check for minimal DMA length

[Wolfram Sang]

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

On Tue, Mar 05, 2019 at 06:54:32PM +0100, Wolfram Sang wrote:
> Use a macro for the hardcoded value and apply a build check. If it is
> not met, the driver logic will not work anymore.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-next, thanks!


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

[PATCH 2/3] i2c: rcar: let DMA enable routine return success status

[Wolfram Sang]

We will need to know if enabling DMA was successful in a later patch.

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

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 925858915569..7e009febe97f 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -399,7 +399,7 @@ static void rcar_i2c_dma_callback(void *data)
 	rcar_i2c_dma_unmap(priv);
 }
 
-static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
+static bool rcar_i2c_dma(struct rcar_i2c_priv *priv)
 {
 	struct device *dev = rcar_i2c_priv_to_dev(priv);
 	struct i2c_msg *msg = priv->msg;
@@ -415,7 +415,7 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
 	/* Do various checks to see if DMA is feasible at all */
 	if (IS_ERR(chan) || msg->len < RCAR_MIN_DMA_LEN ||
 	    !(msg->flags & I2C_M_DMA_SAFE) || (read && priv->flags & ID_P_NO_RXDMA))
-		return;
+		return false;
 
 	if (read) {
 		/*
@@ -435,7 +435,7 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
 	dma_addr = dma_map_single(chan->device->dev, buf, len, dir);
 	if (dma_mapping_error(chan->device->dev, dma_addr)) {
 		dev_dbg(dev, "dma map failed, using PIO\n");
-		return;
+		return false;
 	}
 
 	sg_dma_len(&priv->sg) = len;
@@ -449,7 +449,7 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
 	if (!txdesc) {
 		dev_dbg(dev, "dma prep slave sg failed, using PIO\n");
 		rcar_i2c_cleanup_dma(priv);
-		return;
+		return false;
 	}
 
 	txdesc->callback = rcar_i2c_dma_callback;
@@ -459,7 +459,7 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
 	if (dma_submit_error(cookie)) {
 		dev_dbg(dev, "submitting dma failed, using PIO\n");
 		rcar_i2c_cleanup_dma(priv);
-		return;
+		return false;
 	}
 
 	/* Enable DMA Master Received/Transmitted */
@@ -469,6 +469,7 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
 		rcar_i2c_write(priv, ICDMAER, TMDMAE);
 
 	dma_async_issue_pending(chan);
+	return true;
 }
 
 static void rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
-- 
2.11.0

Re: [PATCH 2/3] i2c: rcar: let DMA enable routine return success status

[Geert Uytterhoeven]

On Tue, Mar 5, 2019 at 7:11 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> We will need to know if enabling DMA was successful in a later patch.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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

Re: [PATCH 2/3] i2c: rcar: let DMA enable routine return success status

[Simon Horman]

On Tue, Mar 05, 2019 at 06:54:33PM +0100, Wolfram Sang wrote:
> We will need to know if enabling DMA was successful in a later patch.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

Re: [PATCH 2/3] i2c: rcar: let DMA enable routine return success status

[Wolfram Sang]

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

On Tue, Mar 05, 2019 at 06:54:33PM +0100, Wolfram Sang wrote:
> We will need to know if enabling DMA was successful in a later patch.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-next, thanks!


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

[PATCH 3/3] i2c: rcar: wait for data empty before starting DMA

[Wolfram Sang]

When sending with DMA, the driver transfers the first byte with PIO (as
documented). However, it started DMA right after the first byte was
written. This worked, but was not according to the datasheet which
suggests to wait until data register was empty again. Implement this.

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

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 7e009febe97f..63286b3cc69b 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -480,6 +480,10 @@ static void rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 	if (!(msr & MDE))
 		return;
 
+	/* Check if DMA can be enabled and take over */
+	if (priv->pos == 1 && rcar_i2c_dma(priv))
+		return;
+
 	if (priv->pos < msg->len) {
 		/*
 		 * Prepare next data to ICRXTX register.
@@ -490,13 +494,6 @@ static void rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
 		 */
 		rcar_i2c_write(priv, ICRXTX, msg->buf[priv->pos]);
 		priv->pos++;
-
-		/*
-		 * Try to use DMA to transmit the rest of the data if
-		 * address transfer phase just finished.
-		 */
-		if (msr & MAT)
-			rcar_i2c_dma(priv);
 	} else {
 		/*
 		 * The last data was pushed to ICRXTX on _PREV_ empty irq.
-- 
2.11.0

Re: [PATCH 3/3] i2c: rcar: wait for data empty before starting DMA

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Mar 5, 2019 at 7:11 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> When sending with DMA, the driver transfers the first byte with PIO (as
> documented). However, it started DMA right after the first byte was
> written. This worked, but was not according to the datasheet which
> suggests to wait until data register was empty again. Implement this.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thanks for your patch!

> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -480,6 +480,10 @@ static void rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
>         if (!(msr & MDE))
>                 return;
>
> +       /* Check if DMA can be enabled and take over */
> +       if (priv->pos == 1 && rcar_i2c_dma(priv))

Shouldn't you check if MSR.MAT is set?

> +               return;

Hence ICMCR.ESG is not cleared, violating 57.3.8 (Master Transmit
Operation), Step 3?

> +
>         if (priv->pos < msg->len) {
>                 /*
>                  * Prepare next data to ICRXTX register.
> @@ -490,13 +494,6 @@ static void rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
>                  */
>                 rcar_i2c_write(priv, ICRXTX, msg->buf[priv->pos]);
>                 priv->pos++;
> -
> -               /*
> -                * Try to use DMA to transmit the rest of the data if
> -                * address transfer phase just finished.
> -                */
> -               if (msr & MAT)
> -                       rcar_i2c_dma(priv);
>         } else {
>                 /*
>                  * The last data was pushed to ICRXTX on _PREV_ empty irq.

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

Re: [PATCH 3/3] i2c: rcar: wait for data empty before starting DMA

[Wolfram Sang]

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

> > +       /* Check if DMA can be enabled and take over */
> > +       if (priv->pos == 1 && rcar_i2c_dma(priv))
> 
> Shouldn't you check if MSR.MAT is set?

It was set before, when priv->pos == 0.

> 
> > +               return;
> 
> Hence ICMCR.ESG is not cleared, violating 57.3.8 (Master Transmit
> Operation), Step 3?

Clearing ESG is the very first thing we do in the ISR. Otherwise, we run
into the race issue we have on this HW and generate an unwanted repeated
message.


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

Re: [PATCH 3/3] i2c: rcar: wait for data empty before starting DMA

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Mar 12, 2019 at 1:50 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> > > +       /* Check if DMA can be enabled and take over */
> > > +       if (priv->pos == 1 && rcar_i2c_dma(priv))
> >
> > Shouldn't you check if MSR.MAT is set?
>
> It was set before, when priv->pos == 0.
>
> >
> > > +               return;
> >
> > Hence ICMCR.ESG is not cleared, violating 57.3.8 (Master Transmit
> > Operation), Step 3?
>
> Clearing ESG is the very first thing we do in the ISR. Otherwise, we run
> into the race issue we have on this HW and generate an unwanted repeated
> message.

Thanks!

Obviously I should work on improving my i2c-rcar foo ... ;-)

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

Re: [PATCH 3/3] i2c: rcar: wait for data empty before starting DMA

[Wolfram Sang]

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


> Obviously I should work on improving my i2c-rcar foo ... ;-)

There be dragons! ;)


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

Re: [PATCH 3/3] i2c: rcar: wait for data empty before starting DMA

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Mar 12, 2019 at 2:18 PM Wolfram Sang <wsa@the-dreams.de> wrote:
> > Obviously I should work on improving my i2c-rcar foo ... ;-)
>
> There be dragons! ;)

Fortunately I've enjoyed "How to Train Your Dragon 3" with my girls last
weekend ;-)

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

Re: [PATCH 3/3] i2c: rcar: wait for data empty before starting DMA

[Simon Horman]

On Tue, Mar 05, 2019 at 06:54:34PM +0100, Wolfram Sang wrote:
> When sending with DMA, the driver transfers the first byte with PIO (as
> documented). However, it started DMA right after the first byte was
> written. This worked, but was not according to the datasheet which
> suggests to wait until data register was empty again. Implement this.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Simon Horman <horms+renesas@verge.net.au>

> ---
>  drivers/i2c/busses/i2c-rcar.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index 7e009febe97f..63286b3cc69b 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -480,6 +480,10 @@ static void rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
>  	if (!(msr & MDE))
>  		return;
>  
> +	/* Check if DMA can be enabled and take over */
> +	if (priv->pos == 1 && rcar_i2c_dma(priv))
> +		return;
> +
>  	if (priv->pos < msg->len) {
>  		/*
>  		 * Prepare next data to ICRXTX register.
> @@ -490,13 +494,6 @@ static void rcar_i2c_irq_send(struct rcar_i2c_priv *priv, u32 msr)
>  		 */
>  		rcar_i2c_write(priv, ICRXTX, msg->buf[priv->pos]);
>  		priv->pos++;
> -
> -		/*
> -		 * Try to use DMA to transmit the rest of the data if
> -		 * address transfer phase just finished.
> -		 */
> -		if (msr & MAT)
> -			rcar_i2c_dma(priv);
>  	} else {
>  		/*
>  		 * The last data was pushed to ICRXTX on _PREV_ empty irq.
> -- 
> 2.11.0
> 

Re: [PATCH 3/3] i2c: rcar: wait for data empty before starting DMA

[Wolfram Sang]

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

On Tue, Mar 05, 2019 at 06:54:34PM +0100, Wolfram Sang wrote:
> When sending with DMA, the driver transfers the first byte with PIO (as
> documented). However, it started DMA right after the first byte was
> written. This worked, but was not according to the datasheet which
> suggests to wait until data register was empty again. Implement this.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Applied to for-next, thanks!


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

Re: [PATCH 0/3] i2c: rcar: make DMA more robust

[Geert Uytterhoeven]

Hi Wolfram,

On Tue, Mar 5, 2019 at 7:52 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> The Renesas BSP has a patch to wait for the data register being empty before
> starting TX-DMA. This is according to the data sheet, so this small series does
> the same in patch 2 & 3. However, refactored, so there is less to compute and
> it is hopefully also easy to read. During that work, I noticed an implicit
> assumption about a required minimal DMA length. It is now explicit and
> documented :)
>
> Tested on a Renesas Salvator XS (R-Car M3N) and now regressions found when

no regressions? ;-)

> writing data with DMA.

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

Re: [PATCH 0/3] i2c: rcar: make DMA more robust

[Wolfram Sang]

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


> > Tested on a Renesas Salvator XS (R-Car M3N) and now regressions found when
> 
> no regressions? ;-)

:DDD


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