Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1714077 > unrolled thread

[RFC PATCH v4 0/6] i2c: document DMA handling and add helpers for it

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH v4 0/6] i2c: document DMA handling and add helpers for it Wolfram Sang <wsa+renesas@sang-engineering.com> - 2017-08-17 16:20 +0200
    [RFC PATCH v4 4/6] i2c: sh_mobile: use helper to decide if DMA is useful Wolfram Sang <wsa+renesas@sang-engineering.com> - 2017-08-17 16:20 +0200
    [RFC PATCH v4 1/6] i2c: add a message flag for DMA safe buffers Wolfram Sang <wsa+renesas@sang-engineering.com> - 2017-08-17 16:20 +0200
    [RFC PATCH v4 5/6] i2c: rcar: skip DMA if buffer is not safe Wolfram Sang <wsa+renesas@sang-engineering.com> - 2017-08-17 16:20 +0200
    Re: [RFC PATCH v4 0/6] i2c: document DMA handling and add helpers  for it Jonathan Cameron <jic23@jic23.retrosnub.co.uk> - 2017-08-20 12:20 +0200

#1714077 — [RFC PATCH v4 0/6] i2c: document DMA handling and add helpers for it

So, after revisiting old mail threads, taking part in a similar discussion on
the USB list, and implementing a not-convincing solution before, here is what I
cooked up to document and ease DMA handling for I2C within Linux. Please have a
look at the documentation introduced in patch 3 for details.

While the previous versions tried to magically apply bounce buffers when
needed, it became clear that detecting DMA safe buffers is too fragile. This
approach is now opt-in, a DMA_SAFE flag needs to be set on an i2c_msg. The
outcome so far is very convincing IMO. The core additions are simple and easy
to understand (makes me even think of inlining them again?). The driver changes
for the Renesas IP cores became easier to understand, too. While only a tad for
the i2c-sh_mobile driver, the situation became a LOT better for the i2c-rcar
driver. No more DMA disabling for the whole transfer in case of unsafe buffers,
we are back to per-msg handling. And the code fix is now an easy to understand
one line change. Yay!

Of course, we must now whitelist DMA safe buffers. An example for I2C_RDWR case
is in this series. It makes the i2ctransfer utility have DMA_SAFE buffers,
which is nice for testing as i2cdump will (currently) not use DMA_SAFE buffers.
My plan is to add two new calls: i2c_master_{send|receive}_dma_safe which can
be used if DMA_SAFE buffers are provided. So, drivers can simply switch to
them. Also, the buffers used within i2c_smbus_xfer_emulated() need to be
converted to be DMA_SAFE which will cover a huge bunch of use cases. The rest
is then updating drivers which can be done when needed.

As these conversions are not done yet, this patch series has RFC status. But I
already would like to get opinions on this approach, so I'll cc mailing lists
of the heavier I2C users. Please let me know what you think.

All patches have been tested with a Renesas Salvator-X board (r8a7796/M3-W).

The branch can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/topic/i2c-core-dma-rfc-v4

And big kudos to Renesas Electronics for funding this work, thank you very much!

Regards,

   Wolfram

Changes since v3:
	* completely redesigned

Wolfram Sang (6):
  i2c: add a message flag for DMA safe buffers
  i2c: add helpers to ease DMA handling
  i2c: add docs to clarify DMA handling
  i2c: sh_mobile: use helper to decide if DMA is useful
  i2c: rcar: skip DMA if buffer is not safe
  i2c: dev: mark RDWR buffers as DMA_SAFE

 Documentation/i2c/DMA-considerations | 50 ++++++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-rcar.c        |  2 +-
 drivers/i2c/busses/i2c-sh_mobile.c   |  8 ++++--
 drivers/i2c/i2c-core-base.c          | 45 ++++++++++++++++++++++++++++++++
 drivers/i2c/i2c-dev.c                |  2 ++
 include/linux/i2c.h                  |  3 +++
 include/uapi/linux/i2c.h             |  3 +++
 7 files changed, 110 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/i2c/DMA-considerations

-- 
2.11.0

[toc] | [next] | [standalone]

#1714078 — [RFC PATCH v4 4/6] i2c: sh_mobile: use helper to decide if DMA is useful

This ensures that we fall back to PIO if the message length is too small
for DMA being useful. Otherwise, we use DMA. A bounce buffer might be
applied by the helper if the original message buffer is not DMA safe.

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

diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index 2e097d97d258bc..5efdb7becd83d6 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -145,6 +145,7 @@ struct sh_mobile_i2c_data {
 	struct dma_chan *dma_rx;
 	struct scatterlist sg;
 	enum dma_data_direction dma_direction;
+	u8 *dma_buf;
 };
 
 struct sh_mobile_dt_config {
@@ -548,6 +549,8 @@ static void sh_mobile_i2c_dma_callback(void *data)
 	pd->pos = pd->msg->len;
 	pd->stop_after_dma = true;
 
+	i2c_release_dma_safe_msg_buf(pd->msg, pd->dma_buf);
+
 	iic_set_clr(pd, ICIC, 0, ICIC_TDMAE | ICIC_RDMAE);
 }
 
@@ -608,7 +611,7 @@ static void sh_mobile_i2c_xfer_dma(struct sh_mobile_i2c_data *pd)
 	if (IS_ERR(chan))
 		return;
 
-	dma_addr = dma_map_single(chan->device->dev, pd->msg->buf, pd->msg->len, dir);
+	dma_addr = dma_map_single(chan->device->dev, pd->dma_buf, pd->msg->len, dir);
 	if (dma_mapping_error(chan->device->dev, dma_addr)) {
 		dev_dbg(pd->dev, "dma map failed, using PIO\n");
 		return;
@@ -665,7 +668,8 @@ static int start_ch(struct sh_mobile_i2c_data *pd, struct i2c_msg *usr_msg,
 	pd->pos = -1;
 	pd->sr = 0;
 
-	if (pd->msg->len > 8)
+	pd->dma_buf = i2c_get_dma_safe_msg_buf(pd->msg, 8);
+	if (pd->dma_buf)
 		sh_mobile_i2c_xfer_dma(pd);
 
 	/* Enable all interrupts to begin with */
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1714079 — [RFC PATCH v4 1/6] i2c: add a message flag for DMA safe buffers

I2C has no requirement that the buffer of a message needs to be DMA
safe. In case it is, it can now be flagged, so drivers wishing to
do DMA can use the buffer directly.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 include/uapi/linux/i2c.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/uapi/linux/i2c.h b/include/uapi/linux/i2c.h
index 009e27bb9abe19..1c683cb319e4b7 100644
--- a/include/uapi/linux/i2c.h
+++ b/include/uapi/linux/i2c.h
@@ -71,6 +71,9 @@ struct i2c_msg {
 #define I2C_M_RD		0x0001	/* read data, from slave to master */
 					/* I2C_M_RD is guaranteed to be 0x0001! */
 #define I2C_M_TEN		0x0010	/* this is a ten bit chip address */
+#define I2C_M_DMA_SAFE		0x0200	/* the buffer of this message is DMA safe */
+					/* makes only sense in kernelspace */
+					/* userspace buffers are copied anyway */
 #define I2C_M_RECV_LEN		0x0400	/* length will be first received byte */
 #define I2C_M_NO_RD_ACK		0x0800	/* if I2C_FUNC_PROTOCOL_MANGLING */
 #define I2C_M_IGNORE_NAK	0x1000	/* if I2C_FUNC_PROTOCOL_MANGLING */
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1714080 — [RFC PATCH v4 5/6] i2c: rcar: skip DMA if buffer is not safe

This HW is prone to races, so it needs to setup new messages in irq
context. That means we can't alloc bounce buffers if a message buffer is
not DMA safe. So, in that case, simply fall back to PIO.

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

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 93c1a54981df08..5654a7142bffec 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -359,7 +359,7 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
 	int len;
 
 	/* Do not use DMA if it's not available or for messages < 8 bytes */
-	if (IS_ERR(chan) || msg->len < 8)
+	if (IS_ERR(chan) || msg->len < 8 || !(msg->flags & I2C_M_DMA_SAFE))
 		return;
 
 	if (read) {
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1715915 — Re: [RFC PATCH v4 0/6] i2c: document DMA handling and add helpers for it

On Thu, 17 Aug 2017 16:14:43 +0200
Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:

> So, after revisiting old mail threads, taking part in a similar discussion on
> the USB list, and implementing a not-convincing solution before, here is what I
> cooked up to document and ease DMA handling for I2C within Linux. Please have a
> look at the documentation introduced in patch 3 for details.
> 
> While the previous versions tried to magically apply bounce buffers when
> needed, it became clear that detecting DMA safe buffers is too fragile. This
> approach is now opt-in, a DMA_SAFE flag needs to be set on an i2c_msg. The
> outcome so far is very convincing IMO. The core additions are simple and easy
> to understand (makes me even think of inlining them again?). The driver changes
> for the Renesas IP cores became easier to understand, too. While only a tad for
> the i2c-sh_mobile driver, the situation became a LOT better for the i2c-rcar
> driver. No more DMA disabling for the whole transfer in case of unsafe buffers,
> we are back to per-msg handling. And the code fix is now an easy to understand
> one line change. Yay!
> 
> Of course, we must now whitelist DMA safe buffers. An example for I2C_RDWR case
> is in this series. It makes the i2ctransfer utility have DMA_SAFE buffers,
> which is nice for testing as i2cdump will (currently) not use DMA_SAFE buffers.
> My plan is to add two new calls: i2c_master_{send|receive}_dma_safe which can
> be used if DMA_SAFE buffers are provided. So, drivers can simply switch to
> them. Also, the buffers used within i2c_smbus_xfer_emulated() need to be
> converted to be DMA_SAFE which will cover a huge bunch of use cases. The rest
> is then updating drivers which can be done when needed.
> 
> As these conversions are not done yet, this patch series has RFC status. But I
> already would like to get opinions on this approach, so I'll cc mailing lists
> of the heavier I2C users. Please let me know what you think.
> 
> All patches have been tested with a Renesas Salvator-X board (r8a7796/M3-W).
> 
> The branch can be found here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/topic/i2c-core-dma-rfc-v4
> 
> And big kudos to Renesas Electronics for funding this work, thank you very much!

All looks good to me.  I should really set up some testing for this to see if it
gains us much on the various platforms I use, but that will 'a while' so don't wait
on me!  This is particularly true as none of them have dma support in the master
drivers yet.

Thanks for you work on this and cool than Renesas are funding it!

Jonathan

> 
> Regards,
> 
>    Wolfram
> 
> Changes since v3:
> 	* completely redesigned
> 
> Wolfram Sang (6):
>   i2c: add a message flag for DMA safe buffers
>   i2c: add helpers to ease DMA handling
>   i2c: add docs to clarify DMA handling
>   i2c: sh_mobile: use helper to decide if DMA is useful
>   i2c: rcar: skip DMA if buffer is not safe
>   i2c: dev: mark RDWR buffers as DMA_SAFE
> 
>  Documentation/i2c/DMA-considerations | 50 ++++++++++++++++++++++++++++++++++++
>  drivers/i2c/busses/i2c-rcar.c        |  2 +-
>  drivers/i2c/busses/i2c-sh_mobile.c   |  8 ++++--
>  drivers/i2c/i2c-core-base.c          | 45 ++++++++++++++++++++++++++++++++
>  drivers/i2c/i2c-dev.c                |  2 ++
>  include/linux/i2c.h                  |  3 +++
>  include/uapi/linux/i2c.h             |  3 +++
>  7 files changed, 110 insertions(+), 3 deletions(-)
>  create mode 100644 Documentation/i2c/DMA-considerations
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web