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

Groups > linux.kernel > #1690036 > unrolled thread

[PATCH v3 0/4] i2c: document DMA handling and add helpers for it

Back to article view | Back to linux.kernel

Contents

  [PATCH v3 0/4] i2c: document DMA handling and add helpers for it Wolfram Sang <wsa+renesas@sang-engineering.com> - 2017-07-18 12:30 +0200
    [PATCH v3 4/4] i2c: rcar: check for DMA-capable buffers Wolfram Sang <wsa+renesas@sang-engineering.com> - 2017-07-18 12:30 +0200
      Re: [PATCH v3 4/4] i2c: rcar: check for DMA-capable buffers Niklas Söderlund          <niklas.soderlund@ragnatech.se> - 2017-07-19 11:50 +0200
    [PATCH v3 1/4] i2c: add helpers to ease DMA handling Wolfram Sang <wsa+renesas@sang-engineering.com> - 2017-07-18 12:30 +0200
      Re: [PATCH v3 1/4] i2c: add helpers to ease DMA handling Niklas Söderlund          <niklas.soderlund@ragnatech.se> - 2017-07-19 11:30 +0200
      Re: [PATCH v3 1/4] i2c: add helpers to ease DMA handling Jonathan Cameron <jic23@jic23.retrosnub.co.uk> - 2017-07-23 13:30 +0200

#1690036 — [PATCH v3 0/4] i2c: document DMA handling and add helpers for it

So, after revisiting old mail threads and taking part in a similar discussion
on the USB list, 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 2
for further details.

All patches have been tested with a Renesas Salvator-X board (r8a7796/M3-W) and
a Renesas Lager board (r8a7790/H2). A more detailed test description can be
found here: http://elinux.org/Tests:I2C-core-DMA

The branch can be found here:

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

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

Regards,

   Wolfram


Changes since v2:

* rebased to v4.13-rc1
* helper functions are not inlined anymore but moved to i2c core
* __must_check has been added to the buffer check helper
* the release function has been renamed to contain 'dma' as well
* documentation updates. Hopefully better wording now
* removed the doubled Signed-offs
* adding more potentially interested parties to CC


Wolfram Sang (4):
  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: check for DMA-capable buffers

 Documentation/i2c/DMA-considerations | 38 ++++++++++++++++++++
 drivers/i2c/busses/i2c-rcar.c        | 18 +++++++---
 drivers/i2c/busses/i2c-sh_mobile.c   |  8 +++--
 drivers/i2c/i2c-core-base.c          | 68 ++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h                  |  5 +++
 5 files changed, 130 insertions(+), 7 deletions(-)
 create mode 100644 Documentation/i2c/DMA-considerations

-- 
2.11.0

[toc] | [next] | [standalone]

#1690037 — [PATCH v3 4/4] i2c: rcar: check for DMA-capable buffers

Handling this is special for this driver. Because the hardware needs to
initialize the next message in interrupt context, we cannot use the
i2c_check_msg_for_dma() directly. This helper only works reliably in
process context. So, we need to check during initial preparation of the
whole transfer and need to disable DMA completely for the whole transfer
once a message with a not-DMA-capable buffer is found.

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

diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 93c1a54981df08..5d0e820d708853 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -111,8 +111,11 @@
 #define ID_ARBLOST	(1 << 3)
 #define ID_NACK		(1 << 4)
 /* persistent flags */
+#define ID_P_NODMA	(1 << 30)
 #define ID_P_PM_BLOCKED	(1 << 31)
-#define ID_P_MASK	ID_P_PM_BLOCKED
+#define ID_P_MASK	(ID_P_PM_BLOCKED | ID_P_NODMA)
+
+#define RCAR_DMA_THRESHOLD 8
 
 enum rcar_i2c_type {
 	I2C_RCAR_GEN1,
@@ -358,8 +361,7 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
 	unsigned char *buf;
 	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 < RCAR_DMA_THRESHOLD || priv->flags & ID_P_NODMA)
 		return;
 
 	if (read) {
@@ -657,11 +659,15 @@ static void rcar_i2c_request_dma(struct rcar_i2c_priv *priv,
 				 struct i2c_msg *msg)
 {
 	struct device *dev = rcar_i2c_priv_to_dev(priv);
-	bool read;
+	bool read = msg->flags & I2C_M_RD;
 	struct dma_chan *chan;
 	enum dma_transfer_direction dir;
 
-	read = msg->flags & I2C_M_RD;
+	/* we need to check here because we need the 'current' context */
+	if (i2c_check_msg_for_dma(msg, RCAR_DMA_THRESHOLD, NULL) == -EFAULT) {
+		dev_dbg(dev, "skipping DMA for this whole transfer\n");
+		priv->flags |= ID_P_NODMA;
+	}
 
 	chan = read ? priv->dma_rx : priv->dma_tx;
 	if (PTR_ERR(chan) != -EPROBE_DEFER)
@@ -740,6 +746,8 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
 	if (ret < 0 && ret != -ENXIO)
 		dev_err(dev, "error %d : %x\n", ret, priv->flags);
 
+	priv->flags &= ~ID_P_NODMA;
+
 	return ret;
 }
 
-- 
2.11.0

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

#1691210 — Re: [PATCH v3 4/4] i2c: rcar: check for DMA-capable buffers

Hi Wolfram,

On 2017-07-18 12:23:39 +0200, Wolfram Sang wrote:
> Handling this is special for this driver. Because the hardware needs to
> initialize the next message in interrupt context, we cannot use the
> i2c_check_msg_for_dma() directly. This helper only works reliably in
> process context. So, we need to check during initial preparation of the
> whole transfer and need to disable DMA completely for the whole transfer
> once a message with a not-DMA-capable buffer is found.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>  drivers/i2c/busses/i2c-rcar.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index 93c1a54981df08..5d0e820d708853 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -111,8 +111,11 @@
>  #define ID_ARBLOST	(1 << 3)
>  #define ID_NACK		(1 << 4)
>  /* persistent flags */
> +#define ID_P_NODMA	(1 << 30)
>  #define ID_P_PM_BLOCKED	(1 << 31)
> -#define ID_P_MASK	ID_P_PM_BLOCKED
> +#define ID_P_MASK	(ID_P_PM_BLOCKED | ID_P_NODMA)
> +
> +#define RCAR_DMA_THRESHOLD 8
>  
>  enum rcar_i2c_type {
>  	I2C_RCAR_GEN1,
> @@ -358,8 +361,7 @@ static void rcar_i2c_dma(struct rcar_i2c_priv *priv)
>  	unsigned char *buf;
>  	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 < RCAR_DMA_THRESHOLD || priv->flags & ID_P_NODMA)
>  		return;
>  
>  	if (read) {
> @@ -657,11 +659,15 @@ static void rcar_i2c_request_dma(struct rcar_i2c_priv *priv,
>  				 struct i2c_msg *msg)
>  {
>  	struct device *dev = rcar_i2c_priv_to_dev(priv);
> -	bool read;
> +	bool read = msg->flags & I2C_M_RD;
>  	struct dma_chan *chan;
>  	enum dma_transfer_direction dir;
>  
> -	read = msg->flags & I2C_M_RD;
> +	/* we need to check here because we need the 'current' context */

Maybe extend the comment here explaining that the check is primary here 
to make sure the msg->buf is valid for DMA and that the bounce buffer 
can't be used due to the special hardware feature? Else someone might be 
tempted to try and enable the bounce buffer feature in the future?

Nitpicking aside:

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> +	if (i2c_check_msg_for_dma(msg, RCAR_DMA_THRESHOLD, NULL) == -EFAULT) {
> +		dev_dbg(dev, "skipping DMA for this whole transfer\n");
> +		priv->flags |= ID_P_NODMA;
> +	}
>  
>  	chan = read ? priv->dma_rx : priv->dma_tx;
>  	if (PTR_ERR(chan) != -EPROBE_DEFER)
> @@ -740,6 +746,8 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
>  	if (ret < 0 && ret != -ENXIO)
>  		dev_err(dev, "error %d : %x\n", ret, priv->flags);
>  
> +	priv->flags &= ~ID_P_NODMA;
> +
>  	return ret;
>  }
>  
> -- 
> 2.11.0
> 

-- 
Regards,
Niklas Söderlund

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

#1690047 — [PATCH v3 1/4] i2c: add helpers to ease DMA handling

One helper checks if DMA is suitable and optionally creates a bounce
buffer, if not. The other function returns the bounce buffer and makes
sure the data is properly copied back to the message.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Changes since v2:

* rebased to v4.13-rc1
* helper functions are not inlined anymore but moved to i2c core
* __must_check has been added to the buffer check helper
* the release function has been renamed to contain 'dma' as well

 drivers/i2c/i2c-core-base.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/i2c.h         |  5 ++++
 2 files changed, 73 insertions(+)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index c89dac7fd2e7b7..7326a9d2e4eb69 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -34,6 +34,7 @@
 #include <linux/irqflags.h>
 #include <linux/jump_label.h>
 #include <linux/kernel.h>
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
@@ -44,6 +45,7 @@
 #include <linux/pm_wakeirq.h>
 #include <linux/property.h>
 #include <linux/rwsem.h>
+#include <linux/sched/task_stack.h>
 #include <linux/slab.h>
 
 #include "i2c-core.h"
@@ -2240,6 +2242,72 @@ void i2c_put_adapter(struct i2c_adapter *adap)
 }
 EXPORT_SYMBOL(i2c_put_adapter);
 
+/**
+ * i2c_check_msg_for_dma() - check if a message is suitable for DMA
+ * @msg: the message to be checked
+ * @threshold: the amount of byte from which using DMA makes sense
+ * @ptr_for_bounce_buf: if not NULL, a bounce buffer will be attached to this
+ *			ptr, if needed. The bounce buffer must be freed by the
+ *			caller using i2c_release_dma_bounce_buf().
+ *
+ * Return: -ERANGE if message is smaller than threshold
+ *	   -EFAULT if message buffer is not DMA capable and no bounce buffer
+ *		   was requested
+ *	   -ENOMEM if a bounce buffer could not be created
+ *	   0 if message is suitable for DMA
+ *
+ * The return value must be checked.
+ *
+ * Note: This function should only be called from process context! It uses
+ * helper functions which work on the 'current' task.
+ */
+int i2c_check_msg_for_dma(struct i2c_msg *msg, unsigned int threshold,
+					u8 **ptr_for_bounce_buf)
+{
+	if (ptr_for_bounce_buf)
+		*ptr_for_bounce_buf = NULL;
+
+	if (msg->len < threshold)
+		return -ERANGE;
+
+	if (!virt_addr_valid(msg->buf) || object_is_on_stack(msg->buf)) {
+		pr_debug("msg buffer to 0x%04x is not DMA safe%s\n", msg->addr,
+			 ptr_for_bounce_buf ? ", trying bounce buffer" : "");
+		if (ptr_for_bounce_buf) {
+			if (msg->flags & I2C_M_RD)
+				*ptr_for_bounce_buf = kzalloc(msg->len, GFP_KERNEL);
+			else
+				*ptr_for_bounce_buf = kmemdup(msg->buf, msg->len,
+							      GFP_KERNEL);
+			if (!*ptr_for_bounce_buf)
+				return -ENOMEM;
+		} else {
+			return -EFAULT;
+		}
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_check_msg_for_dma);
+
+/**
+ * i2c_release_bounce_buf - copy data back from bounce buffer and release it
+ * @msg: the message to be copied back to
+ * @bounce_buf: the bounce buffer obtained from i2c_check_msg_for_dma().
+ *		May be NULL.
+ */
+void i2c_release_dma_bounce_buf(struct i2c_msg *msg, u8 *bounce_buf)
+{
+	if (!bounce_buf)
+		return;
+
+	if (msg->flags & I2C_M_RD)
+		memcpy(msg->buf, bounce_buf, msg->len);
+
+	kfree(bounce_buf);
+}
+EXPORT_SYMBOL_GPL(i2c_release_bounce_buf);
+
 MODULE_AUTHOR("Simon G. Vogl <simon@tk.uni-linz.ac.at>");
 MODULE_DESCRIPTION("I2C-Bus main module");
 MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 00ca5b86a753f8..ac02287b6c0d8f 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -766,6 +766,11 @@ static inline u8 i2c_8bit_addr_from_msg(const struct i2c_msg *msg)
 	return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
 }
 
+int __must_check i2c_check_msg_for_dma(struct i2c_msg *msg, unsigned int threshold,
+					u8 **ptr_for_bounce_buf);
+
+void i2c_release_dma_bounce_buf(struct i2c_msg *msg, u8 *bounce_buf);
+
 int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr);
 /**
  * module_i2c_driver() - Helper macro for registering a modular I2C driver
-- 
2.11.0

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

#1691178 — Re: [PATCH v3 1/4] i2c: add helpers to ease DMA handling

Hi Wolfram,

On 2017-07-18 12:23:36 +0200, Wolfram Sang wrote:
> One helper checks if DMA is suitable and optionally creates a bounce
> buffer, if not. The other function returns the bounce buffer and makes
> sure the data is properly copied back to the message.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> Changes since v2:
> 
> * rebased to v4.13-rc1
> * helper functions are not inlined anymore but moved to i2c core
> * __must_check has been added to the buffer check helper
> * the release function has been renamed to contain 'dma' as well
> 
>  drivers/i2c/i2c-core-base.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c.h         |  5 ++++
>  2 files changed, 73 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index c89dac7fd2e7b7..7326a9d2e4eb69 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -34,6 +34,7 @@
>  #include <linux/irqflags.h>
>  #include <linux/jump_label.h>
>  #include <linux/kernel.h>
> +#include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/of_device.h>
> @@ -44,6 +45,7 @@
>  #include <linux/pm_wakeirq.h>
>  #include <linux/property.h>
>  #include <linux/rwsem.h>
> +#include <linux/sched/task_stack.h>
>  #include <linux/slab.h>
>  
>  #include "i2c-core.h"
> @@ -2240,6 +2242,72 @@ void i2c_put_adapter(struct i2c_adapter *adap)
>  }
>  EXPORT_SYMBOL(i2c_put_adapter);
>  
> +/**
> + * i2c_check_msg_for_dma() - check if a message is suitable for DMA
> + * @msg: the message to be checked
> + * @threshold: the amount of byte from which using DMA makes sense
> + * @ptr_for_bounce_buf: if not NULL, a bounce buffer will be attached to this
> + *			ptr, if needed. The bounce buffer must be freed by the
> + *			caller using i2c_release_dma_bounce_buf().
> + *
> + * Return: -ERANGE if message is smaller than threshold
> + *	   -EFAULT if message buffer is not DMA capable and no bounce buffer
> + *		   was requested
> + *	   -ENOMEM if a bounce buffer could not be created
> + *	   0 if message is suitable for DMA
> + *
> + * The return value must be checked.
> + *
> + * Note: This function should only be called from process context! It uses
> + * helper functions which work on the 'current' task.
> + */
> +int i2c_check_msg_for_dma(struct i2c_msg *msg, unsigned int threshold,
> +					u8 **ptr_for_bounce_buf)
> +{
> +	if (ptr_for_bounce_buf)
> +		*ptr_for_bounce_buf = NULL;
> +
> +	if (msg->len < threshold)
> +		return -ERANGE;
> +
> +	if (!virt_addr_valid(msg->buf) || object_is_on_stack(msg->buf)) {
> +		pr_debug("msg buffer to 0x%04x is not DMA safe%s\n", msg->addr,
> +			 ptr_for_bounce_buf ? ", trying bounce buffer" : "");
> +		if (ptr_for_bounce_buf) {
> +			if (msg->flags & I2C_M_RD)
> +				*ptr_for_bounce_buf = kzalloc(msg->len, GFP_KERNEL);
> +			else
> +				*ptr_for_bounce_buf = kmemdup(msg->buf, msg->len,
> +							      GFP_KERNEL);
> +			if (!*ptr_for_bounce_buf)
> +				return -ENOMEM;
> +		} else {
> +			return -EFAULT;
> +		}
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_check_msg_for_dma);
> +
> +/**
> + * i2c_release_bounce_buf - copy data back from bounce buffer and release it
> + * @msg: the message to be copied back to
> + * @bounce_buf: the bounce buffer obtained from i2c_check_msg_for_dma().
> + *		May be NULL.
> + */
> +void i2c_release_dma_bounce_buf(struct i2c_msg *msg, u8 *bounce_buf)
> +{
> +	if (!bounce_buf)
> +		return;
> +
> +	if (msg->flags & I2C_M_RD)
> +		memcpy(msg->buf, bounce_buf, msg->len);
> +
> +	kfree(bounce_buf);
> +}
> +EXPORT_SYMBOL_GPL(i2c_release_bounce_buf);
> +
>  MODULE_AUTHOR("Simon G. Vogl <simon@tk.uni-linz.ac.at>");
>  MODULE_DESCRIPTION("I2C-Bus main module");
>  MODULE_LICENSE("GPL");
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 00ca5b86a753f8..ac02287b6c0d8f 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -766,6 +766,11 @@ static inline u8 i2c_8bit_addr_from_msg(const struct i2c_msg *msg)
>  	return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
>  }
>  
> +int __must_check i2c_check_msg_for_dma(struct i2c_msg *msg, unsigned int threshold,
> +					u8 **ptr_for_bounce_buf);
> +
> +void i2c_release_dma_bounce_buf(struct i2c_msg *msg, u8 *bounce_buf);
> +
>  int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr);
>  /**
>   * module_i2c_driver() - Helper macro for registering a modular I2C driver
> -- 
> 2.11.0
> 

-- 
Regards,
Niklas Söderlund

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

#1694298 — Re: [PATCH v3 1/4] i2c: add helpers to ease DMA handling

On Tue, 18 Jul 2017 12:23:36 +0200
Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:

> One helper checks if DMA is suitable and optionally creates a bounce
> buffer, if not. The other function returns the bounce buffer and makes
> sure the data is properly copied back to the message.
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
My knowledge of the exact requirements for dma on all platforms isn't
all that good.  Mostly it's based around the assumption that if one
forces a heap allocation and makes sure nothing else is in the
cacheline all is good.

I like the basic idea of this patch set a lot btw!

Jonathan
> ---
> Changes since v2:
> 
> * rebased to v4.13-rc1
> * helper functions are not inlined anymore but moved to i2c core
> * __must_check has been added to the buffer check helper
> * the release function has been renamed to contain 'dma' as well
> 
>  drivers/i2c/i2c-core-base.c | 68 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/i2c.h         |  5 ++++
>  2 files changed, 73 insertions(+)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index c89dac7fd2e7b7..7326a9d2e4eb69 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -34,6 +34,7 @@
>  #include <linux/irqflags.h>
>  #include <linux/jump_label.h>
>  #include <linux/kernel.h>
> +#include <linux/mm.h>
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/of_device.h>
> @@ -44,6 +45,7 @@
>  #include <linux/pm_wakeirq.h>
>  #include <linux/property.h>
>  #include <linux/rwsem.h>
> +#include <linux/sched/task_stack.h>
>  #include <linux/slab.h>
>  
>  #include "i2c-core.h"
> @@ -2240,6 +2242,72 @@ void i2c_put_adapter(struct i2c_adapter *adap)
>  }
>  EXPORT_SYMBOL(i2c_put_adapter);
>  
> +/**
> + * i2c_check_msg_for_dma() - check if a message is suitable for DMA
> + * @msg: the message to be checked
> + * @threshold: the amount of byte from which using DMA makes sense
> + * @ptr_for_bounce_buf: if not NULL, a bounce buffer will be attached to this
> + *			ptr, if needed. The bounce buffer must be freed by the
> + *			caller using i2c_release_dma_bounce_buf().
> + *
> + * Return: -ERANGE if message is smaller than threshold
> + *	   -EFAULT if message buffer is not DMA capable and no bounce buffer
> + *		   was requested
> + *	   -ENOMEM if a bounce buffer could not be created
> + *	   0 if message is suitable for DMA
> + *
> + * The return value must be checked.
> + *
> + * Note: This function should only be called from process context! It uses
> + * helper functions which work on the 'current' task.
> + */
> +int i2c_check_msg_for_dma(struct i2c_msg *msg, unsigned int threshold,
> +					u8 **ptr_for_bounce_buf)
> +{
> +	if (ptr_for_bounce_buf)
> +		*ptr_for_bounce_buf = NULL;
> +
> +	if (msg->len < threshold)
> +		return -ERANGE;
> +
> +	if (!virt_addr_valid(msg->buf) || object_is_on_stack(msg->buf)) {
> +		pr_debug("msg buffer to 0x%04x is not DMA safe%s\n", msg->addr,
> +			 ptr_for_bounce_buf ? ", trying bounce buffer" : "");
> +		if (ptr_for_bounce_buf) {
> +			if (msg->flags & I2C_M_RD)
> +				*ptr_for_bounce_buf = kzalloc(msg->len, GFP_KERNEL);
> +			else
> +				*ptr_for_bounce_buf = kmemdup(msg->buf, msg->len,
> +							      GFP_KERNEL);
> +			if (!*ptr_for_bounce_buf)
> +				return -ENOMEM;
> +		} else {
> +			return -EFAULT;
> +		}
> +	}
I'm trying to get my head around whether this is a sufficient set of conditions
for a dma safe buffer and I'm not sure it is.

We go to a lot of effort with SPI buffers to ensure they are in their own cache
lines.  Do we not need to do the same here?  The issue would be the
classic case of embedding a buffer inside a larger structure which is on
the stack.  Need the magic of __cacheline_aligned to force it into it's
own line for devices with dma-incoherent caches.
DMA-API-HOWTO.txt (not read that for a while at it is a rather good
at describing this stuff these days!)

So that one is easy enough to check by checking if it is cache line aligned,
but what do you do to know there is nothing much after it?

Not sure there is a way around this short of auditing i2c drivers to
change any that do this.
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(i2c_check_msg_for_dma);
> +
> +/**
> + * i2c_release_bounce_buf - copy data back from bounce buffer and release it
> + * @msg: the message to be copied back to
> + * @bounce_buf: the bounce buffer obtained from i2c_check_msg_for_dma().
> + *		May be NULL.
> + */
> +void i2c_release_dma_bounce_buf(struct i2c_msg *msg, u8 *bounce_buf)
> +{
> +	if (!bounce_buf)
> +		return;
> +
> +	if (msg->flags & I2C_M_RD)
> +		memcpy(msg->buf, bounce_buf, msg->len);
> +
> +	kfree(bounce_buf);
> +}
> +EXPORT_SYMBOL_GPL(i2c_release_bounce_buf);
> +
>  MODULE_AUTHOR("Simon G. Vogl <simon@tk.uni-linz.ac.at>");
>  MODULE_DESCRIPTION("I2C-Bus main module");
>  MODULE_LICENSE("GPL");
> diff --git a/include/linux/i2c.h b/include/linux/i2c.h
> index 00ca5b86a753f8..ac02287b6c0d8f 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -766,6 +766,11 @@ static inline u8 i2c_8bit_addr_from_msg(const struct i2c_msg *msg)
>  	return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
>  }
>  
> +int __must_check i2c_check_msg_for_dma(struct i2c_msg *msg, unsigned int threshold,
> +					u8 **ptr_for_bounce_buf);
> +
> +void i2c_release_dma_bounce_buf(struct i2c_msg *msg, u8 *bounce_buf);
> +
>  int i2c_handle_smbus_host_notify(struct i2c_adapter *adap, unsigned short addr);
>  /**
>   * module_i2c_driver() - Helper macro for registering a modular I2C driver

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web