| Message ID | 20210623095942.3325-1-wsa+renesas@sang-engineering.com |
|---|---|
| Headers | show |
| Series | i2c: use proper DMAENGINE API for termination | expand |
Hi Wolfram, On Wed, Jun 23, 2021 at 12:01 PM Wolfram Sang <wsa+renesas@sang-engineering.com> 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 <wsa+renesas@sang-engineering.com> 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
> 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 <wsa+renesas@sang-engineering.com> Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de 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 > >
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 <wsa+renesas@sang-engineering.com> Reviewed-by: Codrin Ciubotariu <codrin.ciubotariu@microchip.com> 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 <wsa+renesas@sang-engineering.com> 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 <wsa+renesas@sang-engineering.com> 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 <wsa+renesas@sang-engineering.com> 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 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 <wsa+renesas@sang-engineering.com> > > 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