* [PATCH] i2c: at91: use dma safe buffers
@ 2022-03-03 16:17 Michael Walle
2022-03-04 7:35 ` Christian König
` (2 more replies)
0 siblings, 3 replies; 14+ messages in thread
From: Michael Walle @ 2022-03-03 16:17 UTC (permalink / raw)
To: Codrin Ciubotariu, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, Sumit Semwal, Christian König
Cc: linux-kernel, dri-devel, linaro-mm-sig, Michael Walle, linux-i2c,
stable, linux-arm-kernel, linux-media
The supplied buffer might be on the stack and we get the following error
message:
[ 3.312058] at91_i2c e0070600.i2c: rejecting DMA map of vmalloc memory
Use i2c_{get,put}_dma_safe_msg_buf() to get a DMA-able memory region if
necessary.
Cc: stable@vger.kernel.org
Signed-off-by: Michael Walle <michael@walle.cc>
---
I'm not sure if or which Fixes: tag I should add to this patch. The issue
seems to be since a very long time, but nobody seem to have triggered it.
FWIW, I'm using the sff,sfp driver, which triggers this.
drivers/i2c/busses/i2c-at91-master.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
index b0eae94909f4..a7a22fedbaba 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -656,6 +656,7 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num)
unsigned int_addr_flag = 0;
struct i2c_msg *m_start = msg;
bool is_read;
+ u8 *dma_buf;
dev_dbg(&adap->dev, "at91_xfer: processing %d messages:\n", num);
@@ -703,7 +704,18 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num)
dev->msg = m_start;
dev->recv_len_abort = false;
+ if (dev->use_dma) {
+ dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
+ if (!dma_buf) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ dev->buf = dma_buf;
+ }
+
+
ret = at91_do_twi_transfer(dev);
+ i2c_put_dma_safe_msg_buf(dma_buf, m_start, !ret);
ret = (ret < 0) ? ret : num;
out:
--
2.30.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-03-03 16:17 [PATCH] i2c: at91: use dma safe buffers Michael Walle
@ 2022-03-04 7:35 ` Christian König
2022-03-04 8:04 ` Wolfram Sang
2022-03-28 7:35 ` Michael Walle
2022-04-05 9:23 ` Codrin.Ciubotariu
2 siblings, 1 reply; 14+ messages in thread
From: Christian König @ 2022-03-04 7:35 UTC (permalink / raw)
To: Michael Walle, Codrin Ciubotariu, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Sumit Semwal
Cc: linux-kernel, dri-devel, linaro-mm-sig, linux-i2c, stable,
linux-arm-kernel, linux-media
Am 03.03.22 um 17:17 schrieb Michael Walle:
> The supplied buffer might be on the stack and we get the following error
> message:
> [ 3.312058] at91_i2c e0070600.i2c: rejecting DMA map of vmalloc memory
>
> Use i2c_{get,put}_dma_safe_msg_buf() to get a DMA-able memory region if
> necessary.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Michael Walle <michael@walle.cc>
> ---
>
> I'm not sure if or which Fixes: tag I should add to this patch. The issue
> seems to be since a very long time, but nobody seem to have triggered it.
> FWIW, I'm using the sff,sfp driver, which triggers this.
>
> drivers/i2c/busses/i2c-at91-master.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
> index b0eae94909f4..a7a22fedbaba 100644
> --- a/drivers/i2c/busses/i2c-at91-master.c
> +++ b/drivers/i2c/busses/i2c-at91-master.c
> @@ -656,6 +656,7 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num)
> unsigned int_addr_flag = 0;
> struct i2c_msg *m_start = msg;
> bool is_read;
> + u8 *dma_buf;
Maybe call your variable differently. DMA-buf is an inter driver buffer
sharing frame we use for GPU acceleration and V4L.
It doesn't cause any technical issues, but the maintainer regex now
triggers on that. So you are CCing people not related to this code in
any way.
Regards,
Christian.
>
> dev_dbg(&adap->dev, "at91_xfer: processing %d messages:\n", num);
>
> @@ -703,7 +704,18 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num)
> dev->msg = m_start;
> dev->recv_len_abort = false;
>
> + if (dev->use_dma) {
> + dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
> + if (!dma_buf) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + dev->buf = dma_buf;
> + }
> +
> +
> ret = at91_do_twi_transfer(dev);
> + i2c_put_dma_safe_msg_buf(dma_buf, m_start, !ret);
>
> ret = (ret < 0) ? ret : num;
> out:
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-03-04 7:35 ` Christian König
@ 2022-03-04 8:04 ` Wolfram Sang
2022-03-04 8:10 ` Christian König
0 siblings, 1 reply; 14+ messages in thread
From: Wolfram Sang @ 2022-03-04 8:04 UTC (permalink / raw)
To: Christian König
Cc: Alexandre Belloni, linux-kernel, Nicolas Ferre, dri-devel,
Claudiu Beznea, linaro-mm-sig, Michael Walle, linux-i2c, stable,
Codrin Ciubotariu, Sumit Semwal, linux-arm-kernel, linux-media
[-- Attachment #1: Type: text/plain, Size: 897 bytes --]
Hi Christian,
> Maybe call your variable differently. DMA-buf is an inter driver buffer
> sharing frame we use for GPU acceleration and V4L.
>
> It doesn't cause any technical issues, but the maintainer regex now triggers
> on that. So you are CCing people not related to this code in any way.
Frankly, I think the 'dma_buf' regex is a bit too generic. 'dma_buf'
seems like a reasonable name to me if some subsystem has to deal with
different buffers which can be DMA or non-DMA, like I2C. If you git-grep
the tree, you will find it in quite some places.
We could now think of renaming the variable to 'dmabuf' but this is
a strange and kind of arbitrary rule to remember IMO.
I wonder if you'd miss a lot of patches if we remove 'dma_buf' from the
regex and keep 'dma_fence' and 'dma_resv'? Or extend it to 'dma_buf_' or
'struct dma_buf'?
All the best,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-03-04 8:04 ` Wolfram Sang
@ 2022-03-04 8:10 ` Christian König
2022-03-04 8:43 ` Wolfram Sang
0 siblings, 1 reply; 14+ messages in thread
From: Christian König @ 2022-03-04 8:10 UTC (permalink / raw)
To: Wolfram Sang, Michael Walle, Codrin Ciubotariu, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Sumit Semwal, linux-i2c,
linux-arm-kernel, linux-kernel, linux-media, dri-devel,
linaro-mm-sig, stable
Am 04.03.22 um 09:04 schrieb Wolfram Sang:
> Hi Christian,
>
>> Maybe call your variable differently. DMA-buf is an inter driver buffer
>> sharing frame we use for GPU acceleration and V4L.
>>
>> It doesn't cause any technical issues, but the maintainer regex now triggers
>> on that. So you are CCing people not related to this code in any way.
> Frankly, I think the 'dma_buf' regex is a bit too generic. 'dma_buf'
> seems like a reasonable name to me if some subsystem has to deal with
> different buffers which can be DMA or non-DMA, like I2C. If you git-grep
> the tree, you will find it in quite some places.
>
> We could now think of renaming the variable to 'dmabuf' but this is
> a strange and kind of arbitrary rule to remember IMO.
>
> I wonder if you'd miss a lot of patches if we remove 'dma_buf' from the
> regex and keep 'dma_fence' and 'dma_resv'? Or extend it to 'dma_buf_' or
> 'struct dma_buf'?
Yeah, I'm already considering something similar for a while.
I'm getting quite a bunch of unrelated mails because the regex is not
the best.
On the other hand the framework is used in a lot of drivers and I do
want to be notified when they mess with their interfaces.
Going to take a another look at that when I have time.
Thanks,
Christian.
>
> All the best,
>
> Wolfram
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-03-04 8:10 ` Christian König
@ 2022-03-04 8:43 ` Wolfram Sang
2022-03-04 8:54 ` Christian König
0 siblings, 1 reply; 14+ messages in thread
From: Wolfram Sang @ 2022-03-04 8:43 UTC (permalink / raw)
To: Christian König
Cc: Alexandre Belloni, linux-kernel, Nicolas Ferre, dri-devel,
Claudiu Beznea, linaro-mm-sig, Michael Walle, linux-i2c, stable,
Codrin Ciubotariu, Sumit Semwal, linux-arm-kernel, linux-media
[-- Attachment #1: Type: text/plain, Size: 488 bytes --]
> I'm getting quite a bunch of unrelated mails because the regex is not the
> best.
I can imagine!
> On the other hand the framework is used in a lot of drivers and I do want to
> be notified when they mess with their interfaces.
Sure thing. I am convinced the regex can be improved to ensure that to a
high degree. I think it is also less work for you than asking people to
rename their variable all the time :)
> Going to take a another look at that when I have time.
Thank you!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-03-04 8:43 ` Wolfram Sang
@ 2022-03-04 8:54 ` Christian König
0 siblings, 0 replies; 14+ messages in thread
From: Christian König @ 2022-03-04 8:54 UTC (permalink / raw)
To: Wolfram Sang, Michael Walle, Codrin Ciubotariu, Nicolas Ferre,
Alexandre Belloni, Claudiu Beznea, Sumit Semwal, linux-i2c,
linux-arm-kernel, linux-kernel, linux-media, dri-devel,
linaro-mm-sig, stable
Am 04.03.22 um 09:43 schrieb Wolfram Sang:
>> I'm getting quite a bunch of unrelated mails because the regex is not the
>> best.
> I can imagine!
>
>> On the other hand the framework is used in a lot of drivers and I do want to
>> be notified when they mess with their interfaces.
> Sure thing. I am convinced the regex can be improved to ensure that to a
> high degree. I think it is also less work for you than asking people to
> rename their variable all the time :)
Well not all the time. It's just that you absolutely hit the nail on the
head with the name.
The local variable for the DMA-buf handle is just nearly always named
dma_buf or dmabuf in the drivers.
Christian.
>
>> Going to take a another look at that when I have time.
> Thank you!
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-03-03 16:17 [PATCH] i2c: at91: use dma safe buffers Michael Walle
2022-03-04 7:35 ` Christian König
@ 2022-03-28 7:35 ` Michael Walle
2022-04-05 9:23 ` Codrin.Ciubotariu
2 siblings, 0 replies; 14+ messages in thread
From: Michael Walle @ 2022-03-28 7:35 UTC (permalink / raw)
To: Codrin Ciubotariu, Nicolas Ferre, Alexandre Belloni,
Claudiu Beznea, Sumit Semwal, Christian König
Cc: linux-kernel, dri-devel, linaro-mm-sig, linux-i2c, stable,
linux-arm-kernel, linux-media
Hi all,
Am 2022-03-03 17:17, schrieb Michael Walle:
> The supplied buffer might be on the stack and we get the following
> error
> message:
> [ 3.312058] at91_i2c e0070600.i2c: rejecting DMA map of vmalloc
> memory
>
> Use i2c_{get,put}_dma_safe_msg_buf() to get a DMA-able memory region if
> necessary.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Michael Walle <michael@walle.cc>
Any news here?
> ---
>
> I'm not sure if or which Fixes: tag I should add to this patch. The
> issue
> seems to be since a very long time, but nobody seem to have triggered
> it.
> FWIW, I'm using the sff,sfp driver, which triggers this.
-michael
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-03-03 16:17 [PATCH] i2c: at91: use dma safe buffers Michael Walle
2022-03-04 7:35 ` Christian König
2022-03-28 7:35 ` Michael Walle
@ 2022-04-05 9:23 ` Codrin.Ciubotariu
2022-04-05 9:38 ` Michael Walle
2 siblings, 1 reply; 14+ messages in thread
From: Codrin.Ciubotariu @ 2022-04-05 9:23 UTC (permalink / raw)
To: michael, Nicolas.Ferre, alexandre.belloni, Claudiu.Beznea,
sumit.semwal, christian.koenig
Cc: linux-kernel, dri-devel, linaro-mm-sig, linux-i2c, stable,
linux-arm-kernel, linux-media
On 03.03.2022 18:17, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> The supplied buffer might be on the stack and we get the following error
> message:
> [ 3.312058] at91_i2c e0070600.i2c: rejecting DMA map of vmalloc memory
>
> Use i2c_{get,put}_dma_safe_msg_buf() to get a DMA-able memory region if
> necessary.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
> ---
>
> I'm not sure if or which Fixes: tag I should add to this patch. The issue
> seems to be since a very long time, but nobody seem to have triggered it.
> FWIW, I'm using the sff,sfp driver, which triggers this.
I think it should be:
Fixes: 60937b2cdbf9 ("i2c: at91: add dma support")
> + if (dev->use_dma) {
> + dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
If you want, you could just dev->buf = i2c_get_dma_safe...
> + if (!dma_buf) {
> + ret = -ENOMEM;
> + goto out;
> + }
> + dev->buf = dma_buf;
Thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-04-05 9:23 ` Codrin.Ciubotariu
@ 2022-04-05 9:38 ` Michael Walle
2022-04-05 10:02 ` Codrin.Ciubotariu
0 siblings, 1 reply; 14+ messages in thread
From: Michael Walle @ 2022-04-05 9:38 UTC (permalink / raw)
To: Codrin.Ciubotariu
Cc: alexandre.belloni, linux-kernel, Nicolas.Ferre, dri-devel,
sumit.semwal, linaro-mm-sig, linux-i2c, stable, Claudiu.Beznea,
christian.koenig, linux-arm-kernel, linux-media
Am 2022-04-05 11:23, schrieb Codrin.Ciubotariu@microchip.com:
> On 03.03.2022 18:17, Michael Walle wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know
>> the content is safe
>>
>> The supplied buffer might be on the stack and we get the following
>> error
>> message:
>> [ 3.312058] at91_i2c e0070600.i2c: rejecting DMA map of vmalloc
>> memory
>>
>> Use i2c_{get,put}_dma_safe_msg_buf() to get a DMA-able memory region
>> if
>> necessary.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Michael Walle <michael@walle.cc>
>
> Reviewed-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com>
Thanks!
>> I'm not sure if or which Fixes: tag I should add to this patch. The
>> issue
>> seems to be since a very long time, but nobody seem to have triggered
>> it.
>> FWIW, I'm using the sff,sfp driver, which triggers this.
>
> I think it should be:
> Fixes: 60937b2cdbf9 ("i2c: at91: add dma support")
>
>> + if (dev->use_dma) {
>> + dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
>
> If you want, you could just dev->buf = i2c_get_dma_safe...
But where is the error handling in that case? dev->buf will
be NULL, which is eventually passed to dma_map_single().
Also, I need the dma_buf for the i2c_put_dma_safe_msg_buf()
call anyway, because dev->buf will be modified during
processing.
-michael
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-04-05 9:38 ` Michael Walle
@ 2022-04-05 10:02 ` Codrin.Ciubotariu
2022-04-05 11:09 ` Michael Walle
0 siblings, 1 reply; 14+ messages in thread
From: Codrin.Ciubotariu @ 2022-04-05 10:02 UTC (permalink / raw)
To: michael
Cc: alexandre.belloni, linux-kernel, Nicolas.Ferre, dri-devel,
sumit.semwal, linaro-mm-sig, linux-i2c, stable, Claudiu.Beznea,
christian.koenig, linux-arm-kernel, linux-media
On 05.04.2022 12:38, Michael Walle wrote:
> Am 2022-04-05 11:23, schrieb Codrin.Ciubotariu@microchip.com:
>>> + if (dev->use_dma) {
>>> + dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
>>
>> If you want, you could just dev->buf = i2c_get_dma_safe...
>
> But where is the error handling in that case? dev->buf will
> be NULL, which is eventually passed to dma_map_single().
>
> Also, I need the dma_buf for the i2c_put_dma_safe_msg_buf()
> call anyway, because dev->buf will be modified during
> processing.
You still:
if (!dev->buf) {
ret = -ENOMEM;
goto out;
}
So, at91_do_twi_transfer()/dma_map_single() will not be called.
Best regards,
Codrin
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-04-05 10:02 ` Codrin.Ciubotariu
@ 2022-04-05 11:09 ` Michael Walle
2022-04-05 13:58 ` Codrin.Ciubotariu
0 siblings, 1 reply; 14+ messages in thread
From: Michael Walle @ 2022-04-05 11:09 UTC (permalink / raw)
To: Codrin.Ciubotariu
Cc: alexandre.belloni, linux-kernel, Nicolas.Ferre, dri-devel,
sumit.semwal, linaro-mm-sig, linux-i2c, stable, Claudiu.Beznea,
christian.koenig, linux-arm-kernel, linux-media
Am 2022-04-05 12:02, schrieb Codrin.Ciubotariu@microchip.com:
> On 05.04.2022 12:38, Michael Walle wrote:
>> Am 2022-04-05 11:23, schrieb Codrin.Ciubotariu@microchip.com:
>>>> + if (dev->use_dma) {
>>>> + dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
>>>
>>> If you want, you could just dev->buf = i2c_get_dma_safe...
>>
>> But where is the error handling in that case? dev->buf will
>> be NULL, which is eventually passed to dma_map_single().
>>
>> Also, I need the dma_buf for the i2c_put_dma_safe_msg_buf()
>> call anyway, because dev->buf will be modified during
>> processing.
>
> You still:
> if (!dev->buf) {
> ret = -ENOMEM;
> goto out;
> }
>
> So, at91_do_twi_transfer()/dma_map_single() will not be called.
Ahh, I misunderstood you. Yes, but as I said, I need the dma_buf
temporary variable anyway, because dev->buf is modified, eg. see
at91_twi_read_data_dma_callback().
-michael
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-04-05 11:09 ` Michael Walle
@ 2022-04-05 13:58 ` Codrin.Ciubotariu
2022-04-05 14:08 ` Michael Walle
0 siblings, 1 reply; 14+ messages in thread
From: Codrin.Ciubotariu @ 2022-04-05 13:58 UTC (permalink / raw)
To: michael
Cc: alexandre.belloni, linux-kernel, Nicolas.Ferre, dri-devel,
sumit.semwal, linaro-mm-sig, linux-i2c, stable, Claudiu.Beznea,
christian.koenig, linux-arm-kernel, linux-media
On 05.04.2022 14:09, Michael Walle wrote:
> Am 2022-04-05 12:02, schrieb Codrin.Ciubotariu@microchip.com:
>> On 05.04.2022 12:38, Michael Walle wrote:
>>> Am 2022-04-05 11:23, schrieb Codrin.Ciubotariu@microchip.com:
>>>>> + if (dev->use_dma) {
>>>>> + dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
>>>>
>>>> If you want, you could just dev->buf = i2c_get_dma_safe...
>>>
>>> But where is the error handling in that case? dev->buf will
>>> be NULL, which is eventually passed to dma_map_single().
>>>
>>> Also, I need the dma_buf for the i2c_put_dma_safe_msg_buf()
>>> call anyway, because dev->buf will be modified during
>>> processing.
>>
>> You still:
>> if (!dev->buf) {
>> ret = -ENOMEM;
>> goto out;
>> }
>>
>> So, at91_do_twi_transfer()/dma_map_single() will not be called.
>
> Ahh, I misunderstood you. Yes, but as I said, I need the dma_buf
> temporary variable anyway, because dev->buf is modified, eg. see
> at91_twi_read_data_dma_callback().
at91_twi_read_data_dma_callback() is called as callback if
dma_async_issue_pending(dma->chan_rx) is called.
dma_async_issue_pending(dma->chan_rx) is called on
at91_twi_read_data_dma(), which is called in at91_do_twi_transfer(),
which we decided above to skip in case of error.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-04-05 13:58 ` Codrin.Ciubotariu
@ 2022-04-05 14:08 ` Michael Walle
2022-04-07 11:50 ` Codrin.Ciubotariu
0 siblings, 1 reply; 14+ messages in thread
From: Michael Walle @ 2022-04-05 14:08 UTC (permalink / raw)
To: Codrin.Ciubotariu
Cc: alexandre.belloni, linux-kernel, Nicolas.Ferre, dri-devel,
sumit.semwal, linaro-mm-sig, linux-i2c, stable, Claudiu.Beznea,
christian.koenig, linux-arm-kernel, linux-media
Am 2022-04-05 15:58, schrieb Codrin.Ciubotariu@microchip.com:
> On 05.04.2022 14:09, Michael Walle wrote:
>> Am 2022-04-05 12:02, schrieb Codrin.Ciubotariu@microchip.com:
>>> On 05.04.2022 12:38, Michael Walle wrote:
>>>> Am 2022-04-05 11:23, schrieb Codrin.Ciubotariu@microchip.com:
>>>>>> + if (dev->use_dma) {
>>>>>> + dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
>>>>>
>>>>> If you want, you could just dev->buf = i2c_get_dma_safe...
>>>>
>>>> But where is the error handling in that case? dev->buf will
>>>> be NULL, which is eventually passed to dma_map_single().
>>>>
>>>> Also, I need the dma_buf for the i2c_put_dma_safe_msg_buf()
>>>> call anyway, because dev->buf will be modified during
>>>> processing.
>>>
>>> You still:
>>> if (!dev->buf) {
>>> ret = -ENOMEM;
>>> goto out;
>>> }
>>>
>>> So, at91_do_twi_transfer()/dma_map_single() will not be called.
>>
>> Ahh, I misunderstood you. Yes, but as I said, I need the dma_buf
>> temporary variable anyway, because dev->buf is modified, eg. see
>> at91_twi_read_data_dma_callback().
> at91_twi_read_data_dma_callback() is called as callback if
> dma_async_issue_pending(dma->chan_rx) is called.
> dma_async_issue_pending(dma->chan_rx) is called on
> at91_twi_read_data_dma(), which is called in at91_do_twi_transfer(),
> which we decided above to skip in case of error.
It is not about errors, you need the exact same pointer you
got from i2c_get_dma_safe_msg_buf() to be passed to
i2c_put_dma_safe_msg_buf(). And because (in some cases, it
isn't really obvious) the dev->buf will be advanced a few
bytes, I cannot pass dev->buf to i2c_put_dma_safe_msg_buf().
-michael
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] i2c: at91: use dma safe buffers
2022-04-05 14:08 ` Michael Walle
@ 2022-04-07 11:50 ` Codrin.Ciubotariu
0 siblings, 0 replies; 14+ messages in thread
From: Codrin.Ciubotariu @ 2022-04-07 11:50 UTC (permalink / raw)
To: michael
Cc: alexandre.belloni, linux-kernel, Nicolas.Ferre, dri-devel,
sumit.semwal, linaro-mm-sig, linux-i2c, stable, Claudiu.Beznea,
christian.koenig, linux-arm-kernel, linux-media
On 05.04.2022 17:08, Michael Walle wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know
> the content is safe
>
> Am 2022-04-05 15:58, schrieb Codrin.Ciubotariu@microchip.com:
>> On 05.04.2022 14:09, Michael Walle wrote:
>>> Am 2022-04-05 12:02, schrieb Codrin.Ciubotariu@microchip.com:
>>>> On 05.04.2022 12:38, Michael Walle wrote:
>>>>> Am 2022-04-05 11:23, schrieb Codrin.Ciubotariu@microchip.com:
>>>>>>> + if (dev->use_dma) {
>>>>>>> + dma_buf = i2c_get_dma_safe_msg_buf(m_start, 1);
>>>>>>
>>>>>> If you want, you could just dev->buf = i2c_get_dma_safe...
>>>>>
>>>>> But where is the error handling in that case? dev->buf will
>>>>> be NULL, which is eventually passed to dma_map_single().
>>>>>
>>>>> Also, I need the dma_buf for the i2c_put_dma_safe_msg_buf()
>>>>> call anyway, because dev->buf will be modified during
>>>>> processing.
>>>>
>>>> You still:
>>>> if (!dev->buf) {
>>>> ret = -ENOMEM;
>>>> goto out;
>>>> }
>>>>
>>>> So, at91_do_twi_transfer()/dma_map_single() will not be called.
>>>
>>> Ahh, I misunderstood you. Yes, but as I said, I need the dma_buf
>>> temporary variable anyway, because dev->buf is modified, eg. see
>>> at91_twi_read_data_dma_callback().
>> at91_twi_read_data_dma_callback() is called as callback if
>> dma_async_issue_pending(dma->chan_rx) is called.
>> dma_async_issue_pending(dma->chan_rx) is called on
>> at91_twi_read_data_dma(), which is called in at91_do_twi_transfer(),
>> which we decided above to skip in case of error.
>
> It is not about errors, you need the exact same pointer you
> got from i2c_get_dma_safe_msg_buf() to be passed to
> i2c_put_dma_safe_msg_buf(). And because (in some cases, it
> isn't really obvious) the dev->buf will be advanced a few
> bytes, I cannot pass dev->buf to i2c_put_dma_safe_msg_buf().
You are right, when dev->use_dma && (dev->buf_len <= AT91_I2C_DMA_THRESHOLD)
got it. Thanks!
Best regards,
Codrin
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-04-08 7:11 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-03 16:17 [PATCH] i2c: at91: use dma safe buffers Michael Walle
2022-03-04 7:35 ` Christian König
2022-03-04 8:04 ` Wolfram Sang
2022-03-04 8:10 ` Christian König
2022-03-04 8:43 ` Wolfram Sang
2022-03-04 8:54 ` Christian König
2022-03-28 7:35 ` Michael Walle
2022-04-05 9:23 ` Codrin.Ciubotariu
2022-04-05 9:38 ` Michael Walle
2022-04-05 10:02 ` Codrin.Ciubotariu
2022-04-05 11:09 ` Michael Walle
2022-04-05 13:58 ` Codrin.Ciubotariu
2022-04-05 14:08 ` Michael Walle
2022-04-07 11:50 ` Codrin.Ciubotariu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).