dmaengine_terminate_all() is deprecated in favor of explicitly saying if
it should be sync or async. Update the drivers I audited.
Wolfram Sang (7):
i2c: at91-master: : use proper DMAENGINE API for termination
i2c: imx: : use proper DMAENGINE API for termination
i2c: mxs: : use proper DMAENGINE API for termination
i2c: qup: : use proper DMAENGINE API for termination
i2c: rcar: : use proper DMAENGINE API for termination
i2c: sh_mobile: : use proper DMAENGINE API for termination
i2c: stm32f7: : use proper DMAENGINE API for termination
drivers/i2c/busses/i2c-at91-master.c | 4 ++--
drivers/i2c/busses/i2c-imx.c | 6 +++---
drivers/i2c/busses/i2c-mxs.c | 4 ++--
drivers/i2c/busses/i2c-qup.c | 2 +-
drivers/i2c/busses/i2c-rcar.c | 4 ++--
drivers/i2c/busses/i2c-sh_mobile.c | 4 ++--
drivers/i2c/busses/i2c-stm32f7.c | 8 ++++----
7 files changed, 16 insertions(+), 16 deletions(-)
--
2.30.2
dmaengine_terminate_all() is deprecated in favor of explicitly saying if
it should be sync or async. Here, we want dmaengine_terminate_sync()
because there is no other synchronization code in the driver to handle
an async case.
Signed-off-by: Wolfram Sang <[email protected]>
---
drivers/i2c/busses/i2c-at91-master.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-at91-master.c b/drivers/i2c/busses/i2c-at91-master.c
index 1cceb6866689..b0eae94909f4 100644
--- a/drivers/i2c/busses/i2c-at91-master.c
+++ b/drivers/i2c/busses/i2c-at91-master.c
@@ -138,9 +138,9 @@ static void at91_twi_dma_cleanup(struct at91_twi_dev *dev)
if (dma->xfer_in_progress) {
if (dma->direction == DMA_FROM_DEVICE)
- dmaengine_terminate_all(dma->chan_rx);
+ dmaengine_terminate_sync(dma->chan_rx);
else
- dmaengine_terminate_all(dma->chan_tx);
+ dmaengine_terminate_sync(dma->chan_tx);
dma->xfer_in_progress = false;
}
if (dma->buf_mapped) {
--
2.30.2
dmaengine_terminate_all() is deprecated in favor of explicitly saying if
it should be sync or async. Here, we want dmaengine_terminate_sync()
because there is no other synchronization code in the driver to handle
an async case.
Signed-off-by: Wolfram Sang <[email protected]>
---
drivers/i2c/busses/i2c-qup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-qup.c b/drivers/i2c/busses/i2c-qup.c
index 61dc20fd1191..fcd35e8de83c 100644
--- a/drivers/i2c/busses/i2c-qup.c
+++ b/drivers/i2c/busses/i2c-qup.c
@@ -778,7 +778,7 @@ static int qup_i2c_bam_schedule_desc(struct qup_i2c_dev *qup)
ret = -EINVAL;
/* abort TX descriptors */
- dmaengine_terminate_all(qup->btx.dma);
+ dmaengine_terminate_sync(qup->btx.dma);
goto desc_err;
}
--
2.30.2
dmaengine_terminate_all() is deprecated in favor of explicitly saying if
it should be sync or async. Here, we want dmaengine_terminate_sync()
because there is no other synchronization code in the driver to handle
an async case.
Signed-off-by: Wolfram Sang <[email protected]>
---
drivers/i2c/busses/i2c-rcar.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 327c092a4130..41ff327882ef 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -391,9 +391,9 @@ static void rcar_i2c_cleanup_dma(struct rcar_i2c_priv *priv)
if (priv->dma_direction == DMA_NONE)
return;
else if (priv->dma_direction == DMA_FROM_DEVICE)
- dmaengine_terminate_all(priv->dma_rx);
+ dmaengine_terminate_sync(priv->dma_rx);
else if (priv->dma_direction == DMA_TO_DEVICE)
- dmaengine_terminate_all(priv->dma_tx);
+ dmaengine_terminate_sync(priv->dma_tx);
rcar_i2c_dma_unmap(priv);
}
--
2.30.2
dmaengine_terminate_all() is deprecated in favor of explicitly saying if
it should be sync or async. Here, we want dmaengine_terminate_sync()
because there is no other synchronization code in the driver to handle
an async case.
Signed-off-by: Wolfram Sang <[email protected]>
---
drivers/i2c/busses/i2c-imx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
index dc5ca71906db..b224e82924d2 100644
--- a/drivers/i2c/busses/i2c-imx.c
+++ b/drivers/i2c/busses/i2c-imx.c
@@ -423,7 +423,7 @@ static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
return 0;
err_submit:
- dmaengine_terminate_all(dma->chan_using);
+ dmaengine_terminate_sync(dma->chan_using);
err_desc:
dma_unmap_single(chan_dev, dma->dma_buf,
dma->dma_len, dma->dma_data_dir);
@@ -899,7 +899,7 @@ static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx,
&i2c_imx->dma->cmd_complete,
msecs_to_jiffies(DMA_TIMEOUT));
if (time_left == 0) {
- dmaengine_terminate_all(dma->chan_using);
+ dmaengine_terminate_sync(dma->chan_using);
return -ETIMEDOUT;
}
@@ -954,7 +954,7 @@ static int i2c_imx_dma_read(struct imx_i2c_struct *i2c_imx,
&i2c_imx->dma->cmd_complete,
msecs_to_jiffies(DMA_TIMEOUT));
if (time_left == 0) {
- dmaengine_terminate_all(dma->chan_using);
+ dmaengine_terminate_sync(dma->chan_using);
return -ETIMEDOUT;
}
--
2.30.2
dmaengine_terminate_all() is deprecated in favor of explicitly saying if
it should be sync or async. Here, we want dmaengine_terminate_sync()
because there is no other synchronization code in the driver to handle
an async case.
Signed-off-by: Wolfram Sang <[email protected]>
---
drivers/i2c/busses/i2c-stm32f7.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 0138317ea600..9ea074a7ced6 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -1567,7 +1567,7 @@ static irqreturn_t stm32f7_i2c_isr_event_thread(int irq, void *data)
if (!ret) {
dev_dbg(i2c_dev->dev, "<%s>: Timed out\n", __func__);
stm32f7_i2c_disable_dma_req(i2c_dev);
- dmaengine_terminate_all(dma->chan_using);
+ dmaengine_terminate_sync(dma->chan_using);
f7_msg->result = -ETIMEDOUT;
}
@@ -1637,7 +1637,7 @@ static irqreturn_t stm32f7_i2c_isr_error(int irq, void *data)
/* Disable dma */
if (i2c_dev->use_dma) {
stm32f7_i2c_disable_dma_req(i2c_dev);
- dmaengine_terminate_all(dma->chan_using);
+ dmaengine_terminate_sync(dma->chan_using);
}
i2c_dev->master_mode = false;
@@ -1678,7 +1678,7 @@ static int stm32f7_i2c_xfer(struct i2c_adapter *i2c_adap,
dev_dbg(i2c_dev->dev, "Access to slave 0x%x timed out\n",
i2c_dev->msg->addr);
if (i2c_dev->use_dma)
- dmaengine_terminate_all(dma->chan_using);
+ dmaengine_terminate_sync(dma->chan_using);
ret = -ETIMEDOUT;
}
@@ -1727,7 +1727,7 @@ static int stm32f7_i2c_smbus_xfer(struct i2c_adapter *adapter, u16 addr,
if (!timeout) {
dev_dbg(dev, "Access to slave 0x%x timed out\n", f7_msg->addr);
if (i2c_dev->use_dma)
- dmaengine_terminate_all(dma->chan_using);
+ dmaengine_terminate_sync(dma->chan_using);
ret = -ETIMEDOUT;
goto pm_free;
}
--
2.30.2
dmaengine_terminate_all() is deprecated in favor of explicitly saying if
it should be sync or async. Here, we want dmaengine_terminate_sync()
because there is no other synchronization code in the driver to handle
an async case.
Signed-off-by: Wolfram Sang <[email protected]>
---
drivers/i2c/busses/i2c-sh_mobile.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/i2c/busses/i2c-sh_mobile.c b/drivers/i2c/busses/i2c-sh_mobile.c
index 2d2e630fd438..db8fa4186814 100644
--- a/drivers/i2c/busses/i2c-sh_mobile.c
+++ b/drivers/i2c/busses/i2c-sh_mobile.c
@@ -458,9 +458,9 @@ static void sh_mobile_i2c_cleanup_dma(struct sh_mobile_i2c_data *pd)
if (pd->dma_direction == DMA_NONE)
return;
else if (pd->dma_direction == DMA_FROM_DEVICE)
- dmaengine_terminate_all(pd->dma_rx);
+ dmaengine_terminate_sync(pd->dma_rx);
else if (pd->dma_direction == DMA_TO_DEVICE)
- dmaengine_terminate_all(pd->dma_tx);
+ dmaengine_terminate_sync(pd->dma_tx);
sh_mobile_i2c_dma_unmap(pd);
}
--
2.30.2
Hi Wolfram,
On Wed, Jun 23, 2021 at 12:01 PM Wolfram Sang
<[email protected]> wrote:
> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> it should be sync or async. Here, we want dmaengine_terminate_sync()
> because there is no other synchronization code in the driver to handle
> an async case.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Thanks for your patch!
Is this safe? The driver is not using a threaded irq, and DMA termination
may be called from the interrupt handler.
Have you tried triggering DMA termination, with lockdep enabled?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
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
On Wed, Jun 23, 2021 at 11:59:41AM +0200, Wolfram Sang wrote:
> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> it should be sync or async. Here, we want dmaengine_terminate_sync()
> because there is no other synchronization code in the driver to handle
> an async case.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Eeks, this is called from irq context as well. Broken patch :(
> Is this safe? The driver is not using a threaded irq, and DMA termination
> may be called from the interrupt handler.
You are right, this will not work. Not my best day today, I overlooked
it for i2c-rcar and lost the note pointing out the same issue for
stm32f7 :(
> Have you tried triggering DMA termination, with lockdep enabled?
Nope. As the code didn't show signs of async nature, I assumed sync was
desired anyhow.
Hello Wolfram,
On Wed, Jun 23, 2021 at 11:59:36AM +0200, Wolfram Sang wrote:
> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> it should be sync or async. Here, we want dmaengine_terminate_sync()
> because there is no other synchronization code in the driver to handle
> an async case.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Reviewed-by: Oleksij Rempel <[email protected]
Thank you!
> ---
> drivers/i2c/busses/i2c-imx.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index dc5ca71906db..b224e82924d2 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -423,7 +423,7 @@ static int i2c_imx_dma_xfer(struct imx_i2c_struct *i2c_imx,
> return 0;
>
> err_submit:
> - dmaengine_terminate_all(dma->chan_using);
> + dmaengine_terminate_sync(dma->chan_using);
> err_desc:
> dma_unmap_single(chan_dev, dma->dma_buf,
> dma->dma_len, dma->dma_data_dir);
> @@ -899,7 +899,7 @@ static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx,
> &i2c_imx->dma->cmd_complete,
> msecs_to_jiffies(DMA_TIMEOUT));
> if (time_left == 0) {
> - dmaengine_terminate_all(dma->chan_using);
> + dmaengine_terminate_sync(dma->chan_using);
> return -ETIMEDOUT;
> }
>
> @@ -954,7 +954,7 @@ static int i2c_imx_dma_read(struct imx_i2c_struct *i2c_imx,
> &i2c_imx->dma->cmd_complete,
> msecs_to_jiffies(DMA_TIMEOUT));
> if (time_left == 0) {
> - dmaengine_terminate_all(dma->chan_using);
> + dmaengine_terminate_sync(dma->chan_using);
> return -ETIMEDOUT;
> }
>
> --
> 2.30.2
>
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Hi Wolfram,
thanks for the update. If you are ok with that I modify the patch
to partially rely on _async / synchronize whenever needed and push it again
separately.
Regards,
Alain
On Wed, Jun 23, 2021 at 04:03:57PM +0200, Wolfram Sang wrote:
> On Wed, Jun 23, 2021 at 11:59:41AM +0200, Wolfram Sang wrote:
> > dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> > it should be sync or async. Here, we want dmaengine_terminate_sync()
> > because there is no other synchronization code in the driver to handle
> > an async case.
> >
> > Signed-off-by: Wolfram Sang <[email protected]>
>
> Eeks, this is called from irq context as well. Broken patch :(
>
Hi Alain,
> thanks for the update. If you are ok with that I modify the patch
> to partially rely on _async / synchronize whenever needed and push it again
> separately.
Perfect, thanks!
All the best,
Wolfram
On 23.06.2021 12:59, Wolfram Sang wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> it should be sync or async. Here, we want dmaengine_terminate_sync()
> because there is no other synchronization code in the driver to handle
> an async case.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Reviewed-by: Codrin Ciubotariu <[email protected]>
Thanks!
On Wed, Jun 23, 2021 at 11:59:35AM +0200, Wolfram Sang wrote:
> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> it should be sync or async. Here, we want dmaengine_terminate_sync()
> because there is no other synchronization code in the driver to handle
> an async case.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Wed, Jun 23, 2021 at 11:59:36AM +0200, Wolfram Sang wrote:
> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> it should be sync or async. Here, we want dmaengine_terminate_sync()
> because there is no other synchronization code in the driver to handle
> an async case.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Wed, Jun 23, 2021 at 11:59:34AM +0200, Wolfram Sang wrote:
> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> it should be sync or async. Update the drivers I audited.
I applied the patches now, except for i2c-rcar and i2c-stm32f7 where
this approach can't be used because of interrupt context. I will check
for i2c-rcar how to do this properly and Alain for i2c-stm32f7 and we
will resend then.
On Wed, Jun 23, 2021 at 11:59:38AM +0200, Wolfram Sang wrote:
> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> it should be sync or async. Here, we want dmaengine_terminate_sync()
> because there is no other synchronization code in the driver to handle
> an async case.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On Wed, Jun 23, 2021 at 11:59:40AM +0200, Wolfram Sang wrote:
> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
> it should be sync or async. Here, we want dmaengine_terminate_sync()
> because there is no other synchronization code in the driver to handle
> an async case.
>
> Signed-off-by: Wolfram Sang <[email protected]>
Applied to for-next, thanks!
On 11.08.2021 16:24, Wolfram Sang wrote:
> On Wed, Jun 23, 2021 at 11:59:35AM +0200, Wolfram Sang wrote:
>> dmaengine_terminate_all() is deprecated in favor of explicitly saying if
>> it should be sync or async. Here, we want dmaengine_terminate_sync()
>> because there is no other synchronization code in the driver to handle
>> an async case.
>>
>> Signed-off-by: Wolfram Sang <[email protected]>
>
> Applied to for-next, thanks!
>
just saw it now, there is a double : : in the subject. Maybe you could
fix it if it's not too late...
Thanks and best regards,
Codrin