* [RFC PATCH v5 1/6] i2c: add a message flag for DMA safe buffers
2017-09-20 18:59 [RFC PATCH v5 0/6] i2c: document DMA handling and add helpers for it Wolfram Sang
@ 2017-09-20 18:59 ` Wolfram Sang
2017-09-20 18:59 ` [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling Wolfram Sang
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Wolfram Sang @ 2017-09-20 18:59 UTC (permalink / raw)
To: linux-i2c
Cc: linux-kernel, linux-renesas-soc, linux-iio, linux-input,
linux-media, dri-devel, Wolfram Sang
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling
2017-09-20 18:59 [RFC PATCH v5 0/6] i2c: document DMA handling and add helpers for it Wolfram Sang
2017-09-20 18:59 ` [RFC PATCH v5 1/6] i2c: add a message flag for DMA safe buffers Wolfram Sang
@ 2017-09-20 18:59 ` Wolfram Sang
2017-09-21 13:59 ` Jonathan Cameron
2017-09-20 18:59 ` [RFC PATCH v5 3/6] i2c: add docs to clarify " Wolfram Sang
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Wolfram Sang @ 2017-09-20 18:59 UTC (permalink / raw)
To: linux-i2c
Cc: linux-kernel, linux-renesas-soc, linux-iio, linux-input,
linux-media, dri-devel, Wolfram Sang
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>
---
drivers/i2c/i2c-core-base.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/i2c.h | 3 +++
2 files changed, 48 insertions(+)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 56e46581b84bdb..925a22794d3ced 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -2241,6 +2241,51 @@ void i2c_put_adapter(struct i2c_adapter *adap)
}
EXPORT_SYMBOL(i2c_put_adapter);
+/**
+ * i2c_get_dma_safe_msg_buf() - get a DMA safe buffer for the given i2c_msg
+ * @msg: the message to be checked
+ * @threshold: the amount of byte from which using DMA makes sense
+ *
+ * Return: NULL if a DMA safe buffer was not obtained. Use msg->buf with PIO.
+ *
+ * Or a valid pointer to be used with DMA. Note that it can either be
+ * msg->buf or a bounce buffer. After use, release it by calling
+ * i2c_release_dma_safe_msg_buf().
+ *
+ * This function must only be called from process context!
+ */
+u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold)
+{
+ if (msg->len < threshold)
+ return NULL;
+
+ if (msg->flags & I2C_M_DMA_SAFE)
+ return msg->buf;
+
+ if (msg->flags & I2C_M_RD)
+ return kzalloc(msg->len, GFP_KERNEL);
+ else
+ return kmemdup(msg->buf, msg->len, GFP_KERNEL);
+}
+EXPORT_SYMBOL_GPL(i2c_get_dma_safe_msg_buf);
+
+/**
+ * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync with i2c_msg
+ * @msg: the message to be synced with
+ * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL.
+ */
+void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
+{
+ if (!buf || buf == msg->buf)
+ return;
+
+ if (msg->flags & I2C_M_RD)
+ memcpy(msg->buf, buf, msg->len);
+
+ kfree(buf);
+}
+EXPORT_SYMBOL_GPL(i2c_release_dma_safe_msg_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 d501d3956f13f0..1e99342f180f45 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -767,6 +767,9 @@ static inline u8 i2c_8bit_addr_from_msg(const struct i2c_msg *msg)
return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
}
+u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold);
+void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling
2017-09-20 18:59 ` [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling Wolfram Sang
@ 2017-09-21 13:59 ` Jonathan Cameron
[not found] ` <20170921145922.000017b5-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2017-09-21 13:59 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, linux-kernel, linux-renesas-soc, linux-iio,
linux-input, linux-media, dri-devel
On Wed, 20 Sep 2017 20:59:52 +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>
One minor suggestion for wording. Otherwise looks good to me.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> drivers/i2c/i2c-core-base.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/i2c.h | 3 +++
> 2 files changed, 48 insertions(+)
>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 56e46581b84bdb..925a22794d3ced 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -2241,6 +2241,51 @@ void i2c_put_adapter(struct i2c_adapter *adap)
> }
> EXPORT_SYMBOL(i2c_put_adapter);
>
> +/**
> + * i2c_get_dma_safe_msg_buf() - get a DMA safe buffer for the given i2c_msg
> + * @msg: the message to be checked
> + * @threshold: the amount of byte from which using DMA makes sense
the minimum number of bytes for which using DMA makes sense.
> + *
> + * Return: NULL if a DMA safe buffer was not obtained. Use msg->buf with PIO.
> + *
> + * Or a valid pointer to be used with DMA. Note that it can either be
> + * msg->buf or a bounce buffer. After use, release it by calling
> + * i2c_release_dma_safe_msg_buf().
> + *
> + * This function must only be called from process context!
> + */
> +u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold)
> +{
> + if (msg->len < threshold)
> + return NULL;
> +
> + if (msg->flags & I2C_M_DMA_SAFE)
> + return msg->buf;
> +
> + if (msg->flags & I2C_M_RD)
> + return kzalloc(msg->len, GFP_KERNEL);
> + else
> + return kmemdup(msg->buf, msg->len, GFP_KERNEL);
> +}
> +EXPORT_SYMBOL_GPL(i2c_get_dma_safe_msg_buf);
> +
> +/**
> + * i2c_release_dma_safe_msg_buf - release DMA safe buffer and sync with i2c_msg
> + * @msg: the message to be synced with
> + * @buf: the buffer obtained from i2c_get_dma_safe_msg_buf(). May be NULL.
> + */
> +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *buf)
> +{
> + if (!buf || buf == msg->buf)
> + return;
> +
> + if (msg->flags & I2C_M_RD)
> + memcpy(msg->buf, buf, msg->len);
> +
> + kfree(buf);
> +}
> +EXPORT_SYMBOL_GPL(i2c_release_dma_safe_msg_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 d501d3956f13f0..1e99342f180f45 100644
> --- a/include/linux/i2c.h
> +++ b/include/linux/i2c.h
> @@ -767,6 +767,9 @@ static inline u8 i2c_8bit_addr_from_msg(const struct i2c_msg *msg)
> return (msg->addr << 1) | (msg->flags & I2C_M_RD ? 1 : 0);
> }
>
> +u8 *i2c_get_dma_safe_msg_buf(struct i2c_msg *msg, unsigned int threshold);
> +void i2c_release_dma_safe_msg_buf(struct i2c_msg *msg, u8 *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
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v5 3/6] i2c: add docs to clarify DMA handling
2017-09-20 18:59 [RFC PATCH v5 0/6] i2c: document DMA handling and add helpers for it Wolfram Sang
2017-09-20 18:59 ` [RFC PATCH v5 1/6] i2c: add a message flag for DMA safe buffers Wolfram Sang
2017-09-20 18:59 ` [RFC PATCH v5 2/6] i2c: add helpers to ease DMA handling Wolfram Sang
@ 2017-09-20 18:59 ` Wolfram Sang
2017-09-20 19:56 ` Mauro Carvalho Chehab
2017-09-20 18:59 ` [RFC PATCH v5 4/6] i2c: sh_mobile: use helper to decide if DMA is useful Wolfram Sang
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Wolfram Sang @ 2017-09-20 18:59 UTC (permalink / raw)
To: linux-i2c
Cc: linux-kernel, linux-renesas-soc, linux-iio, linux-input,
linux-media, dri-devel, Wolfram Sang
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
Documentation/i2c/DMA-considerations | 58 ++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
create mode 100644 Documentation/i2c/DMA-considerations
diff --git a/Documentation/i2c/DMA-considerations b/Documentation/i2c/DMA-considerations
new file mode 100644
index 00000000000000..5a63355c6a9b6f
--- /dev/null
+++ b/Documentation/i2c/DMA-considerations
@@ -0,0 +1,58 @@
+=================
+Linux I2C and DMA
+=================
+
+Given that I2C is a low-speed bus where largely small messages are transferred,
+it is not considered a prime user of DMA access. At this time of writing, only
+10% of I2C bus master drivers have DMA support implemented. And the vast
+majority of transactions are so small that setting up DMA for it will likely
+add more overhead than a plain PIO transfer.
+
+Therefore, it is *not* mandatory that the buffer of an I2C message is DMA safe.
+It does not seem reasonable to apply additional burdens when the feature is so
+rarely used. However, it is recommended to use a DMA-safe buffer if your
+message size is likely applicable for DMA. Most drivers have this threshold
+around 8 bytes (as of today, this is mostly an educated guess, however). For
+any message of 16 byte or larger, it is probably a really good idea. Please
+note that other subsystems you use might add requirements. E.g., if your
+I2C bus master driver is using USB as a bridge, then you need to have DMA
+safe buffers always, because USB requires it.
+
+For clients, if you use a DMA safe buffer in i2c_msg, set the I2C_M_DMA_SAFE
+flag with it. Then, the I2C core and drivers know they can safely operate DMA
+on it. Note that using this flag is optional. I2C host drivers which are not
+updated to use this flag will work like before. And like before, they risk
+using an unsafe DMA buffer. To improve this situation, using I2C_M_DMA_SAFE in
+more and more clients and host drivers is the planned way forward. Note also
+that setting this flag makes only sense in kernel space. User space data is
+copied into kernel space anyhow. The I2C core makes sure the destination
+buffers in kernel space are always DMA capable.
+
+FIXME: Need to implement i2c_master_{send|receive}_dma and proper buffers for i2c_smbus_xfer_emulated.
+
+Drivers wishing to implement safe DMA can use helper functions from the I2C
+core. One gives you a DMA-safe buffer for a given i2c_msg as long as a certain
+threshold is met::
+
+ dma_buf = i2c_get_dma_safe_msg_buf(msg, threshold_in_byte);
+
+If a buffer is returned, it is either msg->buf for the I2C_M_DMA_SAFE case or a
+bounce buffer. But you don't need to care about that detail, just use the
+returned buffer. If NULL is returned, the threshold was not met or a bounce
+buffer could not be allocated. Fall back to PIO in that case.
+
+In any case, a buffer obtained from above needs to be released. It ensures data
+is copied back to the message and a potentially used bounce buffer is freed::
+
+ i2c_release_dma_safe_msg_buf(msg, dma_buf);
+
+The bounce buffer handling from the core is generic and simple. It will always
+allocate a new bounce buffer. If you want a more sophisticated handling (e.g.
+reusing pre-allocated buffers), you are free to implement your own.
+
+Please also check the in-kernel documentation for details. The i2c-sh_mobile
+driver can be used as a reference example how to use the above helpers.
+
+Final note: If you plan to use DMA with I2C (or with anything else, actually)
+make sure you have CONFIG_DMA_API_DEBUG enabled during development. It can help
+you find various issues which can be complex to debug otherwise.
--
2.11.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [RFC PATCH v5 3/6] i2c: add docs to clarify DMA handling
2017-09-20 18:59 ` [RFC PATCH v5 3/6] i2c: add docs to clarify " Wolfram Sang
@ 2017-09-20 19:56 ` Mauro Carvalho Chehab
2017-09-21 14:08 ` Jonathan Cameron
0 siblings, 1 reply; 15+ messages in thread
From: Mauro Carvalho Chehab @ 2017-09-20 19:56 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, linux-kernel, linux-renesas-soc, linux-iio,
linux-input, linux-media, dri-devel
Em Wed, 20 Sep 2017 20:59:53 +0200
Wolfram Sang <wsa+renesas@sang-engineering.com> escreveu:
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Documentation looks OK on my eyes. So:
Reviewed-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> ---
> Documentation/i2c/DMA-considerations | 58 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 58 insertions(+)
> create mode 100644 Documentation/i2c/DMA-considerations
>
> diff --git a/Documentation/i2c/DMA-considerations b/Documentation/i2c/DMA-considerations
> new file mode 100644
> index 00000000000000..5a63355c6a9b6f
> --- /dev/null
> +++ b/Documentation/i2c/DMA-considerations
> @@ -0,0 +1,58 @@
> +=================
> +Linux I2C and DMA
> +=================
> +
> +Given that I2C is a low-speed bus where largely small messages are transferred,
> +it is not considered a prime user of DMA access. At this time of writing, only
> +10% of I2C bus master drivers have DMA support implemented. And the vast
> +majority of transactions are so small that setting up DMA for it will likely
> +add more overhead than a plain PIO transfer.
> +
> +Therefore, it is *not* mandatory that the buffer of an I2C message is DMA safe.
> +It does not seem reasonable to apply additional burdens when the feature is so
> +rarely used. However, it is recommended to use a DMA-safe buffer if your
> +message size is likely applicable for DMA. Most drivers have this threshold
> +around 8 bytes (as of today, this is mostly an educated guess, however). For
> +any message of 16 byte or larger, it is probably a really good idea. Please
> +note that other subsystems you use might add requirements. E.g., if your
> +I2C bus master driver is using USB as a bridge, then you need to have DMA
> +safe buffers always, because USB requires it.
> +
> +For clients, if you use a DMA safe buffer in i2c_msg, set the I2C_M_DMA_SAFE
> +flag with it. Then, the I2C core and drivers know they can safely operate DMA
> +on it. Note that using this flag is optional. I2C host drivers which are not
> +updated to use this flag will work like before. And like before, they risk
> +using an unsafe DMA buffer. To improve this situation, using I2C_M_DMA_SAFE in
> +more and more clients and host drivers is the planned way forward. Note also
> +that setting this flag makes only sense in kernel space. User space data is
> +copied into kernel space anyhow. The I2C core makes sure the destination
> +buffers in kernel space are always DMA capable.
> +
> +FIXME: Need to implement i2c_master_{send|receive}_dma and proper buffers for i2c_smbus_xfer_emulated.
> +
> +Drivers wishing to implement safe DMA can use helper functions from the I2C
> +core. One gives you a DMA-safe buffer for a given i2c_msg as long as a certain
> +threshold is met::
> +
> + dma_buf = i2c_get_dma_safe_msg_buf(msg, threshold_in_byte);
> +
> +If a buffer is returned, it is either msg->buf for the I2C_M_DMA_SAFE case or a
> +bounce buffer. But you don't need to care about that detail, just use the
> +returned buffer. If NULL is returned, the threshold was not met or a bounce
> +buffer could not be allocated. Fall back to PIO in that case.
> +
> +In any case, a buffer obtained from above needs to be released. It ensures data
> +is copied back to the message and a potentially used bounce buffer is freed::
> +
> + i2c_release_dma_safe_msg_buf(msg, dma_buf);
> +
> +The bounce buffer handling from the core is generic and simple. It will always
> +allocate a new bounce buffer. If you want a more sophisticated handling (e.g.
> +reusing pre-allocated buffers), you are free to implement your own.
> +
> +Please also check the in-kernel documentation for details. The i2c-sh_mobile
> +driver can be used as a reference example how to use the above helpers.
> +
> +Final note: If you plan to use DMA with I2C (or with anything else, actually)
> +make sure you have CONFIG_DMA_API_DEBUG enabled during development. It can help
> +you find various issues which can be complex to debug otherwise.
Thanks,
Mauro
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC PATCH v5 3/6] i2c: add docs to clarify DMA handling
2017-09-20 19:56 ` Mauro Carvalho Chehab
@ 2017-09-21 14:08 ` Jonathan Cameron
0 siblings, 0 replies; 15+ messages in thread
From: Jonathan Cameron @ 2017-09-21 14:08 UTC (permalink / raw)
To: Mauro Carvalho Chehab
Cc: Wolfram Sang, linux-i2c, linux-kernel, linux-renesas-soc,
linux-iio, linux-input, linux-media, dri-devel
On Wed, 20 Sep 2017 16:56:48 -0300
Mauro Carvalho Chehab <mchehab@s-opensource.com> wrote:
> Em Wed, 20 Sep 2017 20:59:53 +0200
> Wolfram Sang <wsa+renesas@sang-engineering.com> escreveu:
>
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Documentation looks OK on my eyes. So:
>
> Reviewed-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Really minor suggestion inline. I don't really care either way as
what you had is perfectly comprehensible.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> > ---
> > Documentation/i2c/DMA-considerations | 58 ++++++++++++++++++++++++++++++++++++
> > 1 file changed, 58 insertions(+)
> > create mode 100644 Documentation/i2c/DMA-considerations
> >
> > diff --git a/Documentation/i2c/DMA-considerations b/Documentation/i2c/DMA-considerations
> > new file mode 100644
> > index 00000000000000..5a63355c6a9b6f
> > --- /dev/null
> > +++ b/Documentation/i2c/DMA-considerations
> > @@ -0,0 +1,58 @@
> > +=================
> > +Linux I2C and DMA
> > +=================
> > +
> > +Given that I2C is a low-speed bus where largely small messages are transferred,
Slightly nicer as:
Given that i2c is a low-speed bus, over which the majority of messages transferred are small,
> > +it is not considered a prime user of DMA access. At this time of writing, only
> > +10% of I2C bus master drivers have DMA support implemented. And the vast
> > +majority of transactions are so small that setting up DMA for it will likely
> > +add more overhead than a plain PIO transfer.
> > +
> > +Therefore, it is *not* mandatory that the buffer of an I2C message is DMA safe.
> > +It does not seem reasonable to apply additional burdens when the feature is so
> > +rarely used. However, it is recommended to use a DMA-safe buffer if your
> > +message size is likely applicable for DMA. Most drivers have this threshold
> > +around 8 bytes (as of today, this is mostly an educated guess, however). For
> > +any message of 16 byte or larger, it is probably a really good idea. Please
> > +note that other subsystems you use might add requirements. E.g., if your
> > +I2C bus master driver is using USB as a bridge, then you need to have DMA
> > +safe buffers always, because USB requires it.
> > +
> > +For clients, if you use a DMA safe buffer in i2c_msg, set the I2C_M_DMA_SAFE
> > +flag with it. Then, the I2C core and drivers know they can safely operate DMA
> > +on it. Note that using this flag is optional. I2C host drivers which are not
> > +updated to use this flag will work like before. And like before, they risk
> > +using an unsafe DMA buffer. To improve this situation, using I2C_M_DMA_SAFE in
> > +more and more clients and host drivers is the planned way forward. Note also
> > +that setting this flag makes only sense in kernel space. User space data is
> > +copied into kernel space anyhow. The I2C core makes sure the destination
> > +buffers in kernel space are always DMA capable.
> > +
> > +FIXME: Need to implement i2c_master_{send|receive}_dma and proper buffers for i2c_smbus_xfer_emulated.
> > +
> > +Drivers wishing to implement safe DMA can use helper functions from the I2C
> > +core. One gives you a DMA-safe buffer for a given i2c_msg as long as a certain
> > +threshold is met::
> > +
> > + dma_buf = i2c_get_dma_safe_msg_buf(msg, threshold_in_byte);
> > +
> > +If a buffer is returned, it is either msg->buf for the I2C_M_DMA_SAFE case or a
> > +bounce buffer. But you don't need to care about that detail, just use the
> > +returned buffer. If NULL is returned, the threshold was not met or a bounce
> > +buffer could not be allocated. Fall back to PIO in that case.
> > +
> > +In any case, a buffer obtained from above needs to be released. It ensures data
> > +is copied back to the message and a potentially used bounce buffer is freed::
> > +
> > + i2c_release_dma_safe_msg_buf(msg, dma_buf);
> > +
> > +The bounce buffer handling from the core is generic and simple. It will always
> > +allocate a new bounce buffer. If you want a more sophisticated handling (e.g.
> > +reusing pre-allocated buffers), you are free to implement your own.
> > +
> > +Please also check the in-kernel documentation for details. The i2c-sh_mobile
> > +driver can be used as a reference example how to use the above helpers.
> > +
> > +Final note: If you plan to use DMA with I2C (or with anything else, actually)
> > +make sure you have CONFIG_DMA_API_DEBUG enabled during development. It can help
> > +you find various issues which can be complex to debug otherwise.
>
>
>
> Thanks,
> Mauro
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC PATCH v5 4/6] i2c: sh_mobile: use helper to decide if DMA is useful
2017-09-20 18:59 [RFC PATCH v5 0/6] i2c: document DMA handling and add helpers for it Wolfram Sang
` (2 preceding siblings ...)
2017-09-20 18:59 ` [RFC PATCH v5 3/6] i2c: add docs to clarify " Wolfram Sang
@ 2017-09-20 18:59 ` Wolfram Sang
2017-09-20 18:59 ` [RFC PATCH v5 5/6] i2c: rcar: skip DMA if buffer is not safe Wolfram Sang
2017-09-20 18:59 ` [RFC PATCH v5 6/6] i2c: dev: mark RDWR buffers as DMA_SAFE Wolfram Sang
5 siblings, 0 replies; 15+ messages in thread
From: Wolfram Sang @ 2017-09-20 18:59 UTC (permalink / raw)
To: linux-i2c
Cc: linux-kernel, linux-renesas-soc, linux-iio, linux-input,
linux-media, dri-devel, Wolfram Sang
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 6f2aaeb7c4fa15..b76399d8a3abd3 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [RFC PATCH v5 5/6] i2c: rcar: skip DMA if buffer is not safe
2017-09-20 18:59 [RFC PATCH v5 0/6] i2c: document DMA handling and add helpers for it Wolfram Sang
` (3 preceding siblings ...)
2017-09-20 18:59 ` [RFC PATCH v5 4/6] i2c: sh_mobile: use helper to decide if DMA is useful Wolfram Sang
@ 2017-09-20 18:59 ` Wolfram Sang
2017-09-20 18:59 ` [RFC PATCH v5 6/6] i2c: dev: mark RDWR buffers as DMA_SAFE Wolfram Sang
5 siblings, 0 replies; 15+ messages in thread
From: Wolfram Sang @ 2017-09-20 18:59 UTC (permalink / raw)
To: linux-i2c
Cc: linux-kernel, linux-renesas-soc, linux-iio, linux-input,
linux-media, dri-devel, Wolfram Sang
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 15d764afec3b29..8a2ae3e6c561c4 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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [RFC PATCH v5 6/6] i2c: dev: mark RDWR buffers as DMA_SAFE
2017-09-20 18:59 [RFC PATCH v5 0/6] i2c: document DMA handling and add helpers for it Wolfram Sang
` (4 preceding siblings ...)
2017-09-20 18:59 ` [RFC PATCH v5 5/6] i2c: rcar: skip DMA if buffer is not safe Wolfram Sang
@ 2017-09-20 18:59 ` Wolfram Sang
2017-09-21 14:17 ` Jonathan Cameron
5 siblings, 1 reply; 15+ messages in thread
From: Wolfram Sang @ 2017-09-20 18:59 UTC (permalink / raw)
To: linux-i2c
Cc: linux-kernel, linux-renesas-soc, linux-iio, linux-input,
linux-media, dri-devel, Wolfram Sang
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/i2c-dev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
index 6f638bbc922db4..bbc7aadb4c899d 100644
--- a/drivers/i2c/i2c-dev.c
+++ b/drivers/i2c/i2c-dev.c
@@ -280,6 +280,8 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
res = PTR_ERR(rdwr_pa[i].buf);
break;
}
+ /* memdup_user allocates with GFP_KERNEL, so DMA is ok */
+ rdwr_pa[i].flags |= I2C_M_DMA_SAFE;
/*
* If the message length is received from the slave (similar
--
2.11.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [RFC PATCH v5 6/6] i2c: dev: mark RDWR buffers as DMA_SAFE
2017-09-20 18:59 ` [RFC PATCH v5 6/6] i2c: dev: mark RDWR buffers as DMA_SAFE Wolfram Sang
@ 2017-09-21 14:17 ` Jonathan Cameron
2017-09-21 14:23 ` Wolfram Sang
0 siblings, 1 reply; 15+ messages in thread
From: Jonathan Cameron @ 2017-09-21 14:17 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, linux-kernel, linux-renesas-soc, linux-iio,
linux-input, linux-media, dri-devel
On Wed, 20 Sep 2017 20:59:56 +0200
Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Makes sense as do the other drivers.
Feel free to add
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
to all of them (though they hardly took a lot of reviewing given how simple
the patches were :)
> ---
> drivers/i2c/i2c-dev.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/i2c/i2c-dev.c b/drivers/i2c/i2c-dev.c
> index 6f638bbc922db4..bbc7aadb4c899d 100644
> --- a/drivers/i2c/i2c-dev.c
> +++ b/drivers/i2c/i2c-dev.c
> @@ -280,6 +280,8 @@ static noinline int i2cdev_ioctl_rdwr(struct i2c_client *client,
> res = PTR_ERR(rdwr_pa[i].buf);
> break;
> }
> + /* memdup_user allocates with GFP_KERNEL, so DMA is ok */
> + rdwr_pa[i].flags |= I2C_M_DMA_SAFE;
>
> /*
> * If the message length is received from the slave (similar
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFC PATCH v5 6/6] i2c: dev: mark RDWR buffers as DMA_SAFE
2017-09-21 14:17 ` Jonathan Cameron
@ 2017-09-21 14:23 ` Wolfram Sang
0 siblings, 0 replies; 15+ messages in thread
From: Wolfram Sang @ 2017-09-21 14:23 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Wolfram Sang, linux-i2c, linux-kernel, linux-renesas-soc,
linux-iio, linux-input, linux-media, dri-devel
[-- Attachment #1: Type: text/plain, Size: 542 bytes --]
On Thu, Sep 21, 2017 at 03:17:44PM +0100, Jonathan Cameron wrote:
> On Wed, 20 Sep 2017 20:59:56 +0200
> Wolfram Sang <wsa+renesas@sang-engineering.com> wrote:
>
> > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Makes sense as do the other drivers.
>
> Feel free to add
>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
> to all of them (though they hardly took a lot of reviewing given how simple
> the patches were :)
Well, bugs can slip in everywhere, so thanks for the review!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread