mbox series

[00/46] Use dmaenginem_async_device_register to simplify code

Message ID 20180803072016.21544-1-sjhuang@iluvatar.ai (mailing list archive)
Headers show
Series Use dmaenginem_async_device_register to simplify code | expand

Message

Huang Shijie Aug. 3, 2018, 7:19 a.m. UTC
All the patches are using dmaenginem_async_device_register to simplify code
except the last one:
  dmaengine: add COMPILE_TEST for the drivers

I use the last one to do the compiler test.  
There are still 20 drivers which do not use the dmaenginem_async_device_register.
Let me take a rest, if this patch set is accepted, I will do the rest.

Huang Shijie (46):
  dmaengine: zx_dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: zynqmp_dma: use dmaenginem_async_device_register to
    simplify the code
  dmaengine: xilinx_dma: use dmaenginem_async_device_register to
    simplify the code
  dmaengine: xgene-dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: txx9dmac: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: timb_dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: omap-dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: edma: use dmaenginem_async_device_register to simplify the
    code
  dmaengine: cppi41: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: tegra210-adma: use helper dmaenginem_async_device_register
  dmaengine: tegra20-apb-dma: use helper
    dmaenginem_async_device_register
  dmaengine: sun6i-dma: use helper dmaenginem_async_device_register
  dmaengine: sun4i-dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: coh901318: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: s3c24xx-dma: use dmaenginem_async_device_register to
    simplify the code
  dmaengine: ste_dma40: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: stm32-mdma: use dmaenginem_async_device_register to
    simplify the code
  dmaengine: stm32-dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: sprd-dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: sirf-dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: bam_dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: sudmac: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: sa11x0-dma: use dmaenginem_async_device_register to
    simplify the code
  dmaengine: nbpfaxi: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: mmp_tdma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: shdmac: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: usb-dmac: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: rcar-dmac: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: hidma: use dmaenginem_async_device_register to simplify the
    code
  dmaengine: pxa_dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: moxart-dma: use dmaenginem_async_device_register to
    simplify the code
  dmaengine: pl330: use dmaenginem_async_device_register to simplify the
    code
  dmaengine: pch_dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: mxs-dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: mtk-hsdma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: k3dma: use dmaenginem_async_device_register to simplify the
    code
  dmaengine: imx-sdma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: imx-dma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: img-mdc-dma: use dmaenginem_async_device_register to
    simplify the code
  dmaengine: fsl-edma: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: at_hdmac: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: at_xdmac: use dmaenginem_async_device_register to simplify
    the code
  dmaengine: dma-jz4780: use dmaenginem_async_device_register to
    simplify the code
  dmaengine: dma-jz4740: use dmaenginem_async_device_register to
    simplify the code
  dmaengine: dma-axi-dmac: use dmaenginem_async_device_register to
    simplify the code
  dmaengine: add COMPILE_TEST for the drivers

 drivers/dma/Kconfig              | 24 ++++++++++++------------
 drivers/dma/at_hdmac.c           |  4 +---
 drivers/dma/at_xdmac.c           |  7 ++-----
 drivers/dma/coh901318.c          | 14 ++++----------
 drivers/dma/dma-axi-dmac.c       |  7 ++-----
 drivers/dma/dma-jz4740.c         |  7 ++-----
 drivers/dma/dma-jz4780.c         |  8 ++------
 drivers/dma/fsl-edma.c           |  4 +---
 drivers/dma/img-mdc-dma.c        |  7 ++-----
 drivers/dma/imx-dma.c            |  8 ++------
 drivers/dma/imx-sdma.c           |  7 ++-----
 drivers/dma/k3dma.c              |  7 ++-----
 drivers/dma/mediatek/mtk-hsdma.c |  4 +---
 drivers/dma/mmp_tdma.c           |  7 ++-----
 drivers/dma/moxart-dma.c         |  5 +----
 drivers/dma/mxs-dma.c            |  3 +--
 drivers/dma/nbpfaxi.c            |  7 ++-----
 drivers/dma/pch_dma.c            |  4 +---
 drivers/dma/pl330.c              |  4 +---
 drivers/dma/pxa_dma.c            |  3 +--
 drivers/dma/qcom/bam_dma.c       |  7 ++-----
 drivers/dma/qcom/hidma.c         |  3 +--
 drivers/dma/s3c24xx-dma.c        | 11 +++--------
 drivers/dma/sa11x0-dma.c         |  4 +---
 drivers/dma/sh/rcar-dmac.c       |  5 +----
 drivers/dma/sh/shdmac.c          |  5 +----
 drivers/dma/sh/sudmac.c          |  4 +---
 drivers/dma/sh/usb-dmac.c        |  3 +--
 drivers/dma/sirf-dma.c           |  7 ++-----
 drivers/dma/sprd-dma.c           |  7 ++-----
 drivers/dma/ste_dma40.c          | 14 +++++---------
 drivers/dma/stm32-dma.c          |  3 +--
 drivers/dma/stm32-mdma.c         |  4 +---
 drivers/dma/sun4i-dma.c          |  7 ++-----
 drivers/dma/sun6i-dma.c          |  7 ++-----
 drivers/dma/tegra20-apb-dma.c    |  8 ++------
 drivers/dma/tegra210-adma.c      |  8 ++------
 drivers/dma/ti/Kconfig           |  2 +-
 drivers/dma/ti/cppi41.c          |  7 ++-----
 drivers/dma/ti/edma.c            |  8 ++------
 drivers/dma/ti/omap-dma.c        |  5 +----
 drivers/dma/timb_dma.c           |  3 +--
 drivers/dma/txx9dmac.c           |  4 +---
 drivers/dma/xgene-dma.c          | 16 ++--------------
 drivers/dma/xilinx/xilinx_dma.c  |  5 +----
 drivers/dma/xilinx/zynqmp_dma.c  |  4 +---
 drivers/dma/zx_dma.c             |  7 ++-----
 47 files changed, 88 insertions(+), 221 deletions(-)

Comments

Lars-Peter Clausen Aug. 3, 2018, 7:51 a.m. UTC | #1
On 08/03/2018 09:19 AM, Huang Shijie wrote:
> All the patches are using dmaenginem_async_device_register to simplify code
> except the last one:
>   dmaengine: add COMPILE_TEST for the drivers
> 
> I use the last one to do the compiler test.  
> There are still 20 drivers which do not use the dmaenginem_async_device_register.
> Let me take a rest, if this patch set is accepted, I will do the rest.

Lots of race conditions in this series. The DMA device needs to be removed
before any of the resources it uses are disabled/released.

As a rule of thumb you can only convert something to a managed
allocation/reregistration if it is the last action in remove.
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Huang Shijie Aug. 3, 2018, 7:59 a.m. UTC | #2
On Fri, Aug 03, 2018 at 09:51:43AM +0200, Lars-Peter Clausen wrote:
> On 08/03/2018 09:19 AM, Huang Shijie wrote:
> > All the patches are using dmaenginem_async_device_register to simplify code
> > except the last one:
> >   dmaengine: add COMPILE_TEST for the drivers
> > 
> > I use the last one to do the compiler test.  
> > There are still 20 drivers which do not use the dmaenginem_async_device_register.
> > Let me take a rest, if this patch set is accepted, I will do the rest.
> 
> Lots of race conditions in this series. The DMA device needs to be removed
> before any of the resources it uses are disabled/released.
> 
> As a rule of thumb you can only convert something to a managed
> allocation/reregistration if it is the last action in remove.
Yes. Agree.

I am collecting the DMA maintainers opinions now..
If the managed unregistration is not the last action, we can drop the patch.

Thanks
Huang Shijie

--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Ujfalusi Aug. 3, 2018, 8:20 a.m. UTC | #3
Hi,

On 2018-08-03 10:19, Huang Shijie wrote:
> All the patches are using dmaenginem_async_device_register to simplify code
> except the last one:
>   dmaengine: add COMPILE_TEST for the drivers
> 
> I use the last one to do the compiler test.  
> There are still 20 drivers which do not use the dmaenginem_async_device_register.
> Let me take a rest, if this patch set is accepted, I will do the rest.

I think for most of the drivers this series is going to open a race
which is essentially:

prior:
_remove()
{
...
	dma_async_device_register()
	/* Free resources, disable HW, etc */
	return 0;
}

after:
_remove()
{
...
	/* Free resources, disable HW, etc */
	/*
	 * The dma device is still registered and a channel can be
	 * requested
	 */

	dma_async_device_register()
	return 0;
}

It might be theoretical, but conversion to managed device resources is
not straight forward in some cases.

- Péter

> 
> Huang Shijie (46):
>   dmaengine: zx_dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: zynqmp_dma: use dmaenginem_async_device_register to
>     simplify the code
>   dmaengine: xilinx_dma: use dmaenginem_async_device_register to
>     simplify the code
>   dmaengine: xgene-dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: txx9dmac: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: timb_dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: omap-dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: edma: use dmaenginem_async_device_register to simplify the
>     code
>   dmaengine: cppi41: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: tegra210-adma: use helper dmaenginem_async_device_register
>   dmaengine: tegra20-apb-dma: use helper
>     dmaenginem_async_device_register
>   dmaengine: sun6i-dma: use helper dmaenginem_async_device_register
>   dmaengine: sun4i-dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: coh901318: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: s3c24xx-dma: use dmaenginem_async_device_register to
>     simplify the code
>   dmaengine: ste_dma40: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: stm32-mdma: use dmaenginem_async_device_register to
>     simplify the code
>   dmaengine: stm32-dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: sprd-dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: sirf-dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: bam_dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: sudmac: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: sa11x0-dma: use dmaenginem_async_device_register to
>     simplify the code
>   dmaengine: nbpfaxi: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: mmp_tdma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: shdmac: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: usb-dmac: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: rcar-dmac: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: hidma: use dmaenginem_async_device_register to simplify the
>     code
>   dmaengine: pxa_dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: moxart-dma: use dmaenginem_async_device_register to
>     simplify the code
>   dmaengine: pl330: use dmaenginem_async_device_register to simplify the
>     code
>   dmaengine: pch_dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: mxs-dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: mtk-hsdma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: k3dma: use dmaenginem_async_device_register to simplify the
>     code
>   dmaengine: imx-sdma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: imx-dma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: img-mdc-dma: use dmaenginem_async_device_register to
>     simplify the code
>   dmaengine: fsl-edma: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: at_hdmac: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: at_xdmac: use dmaenginem_async_device_register to simplify
>     the code
>   dmaengine: dma-jz4780: use dmaenginem_async_device_register to
>     simplify the code
>   dmaengine: dma-jz4740: use dmaenginem_async_device_register to
>     simplify the code
>   dmaengine: dma-axi-dmac: use dmaenginem_async_device_register to
>     simplify the code
>   dmaengine: add COMPILE_TEST for the drivers
> 
>  drivers/dma/Kconfig              | 24 ++++++++++++------------
>  drivers/dma/at_hdmac.c           |  4 +---
>  drivers/dma/at_xdmac.c           |  7 ++-----
>  drivers/dma/coh901318.c          | 14 ++++----------
>  drivers/dma/dma-axi-dmac.c       |  7 ++-----
>  drivers/dma/dma-jz4740.c         |  7 ++-----
>  drivers/dma/dma-jz4780.c         |  8 ++------
>  drivers/dma/fsl-edma.c           |  4 +---
>  drivers/dma/img-mdc-dma.c        |  7 ++-----
>  drivers/dma/imx-dma.c            |  8 ++------
>  drivers/dma/imx-sdma.c           |  7 ++-----
>  drivers/dma/k3dma.c              |  7 ++-----
>  drivers/dma/mediatek/mtk-hsdma.c |  4 +---
>  drivers/dma/mmp_tdma.c           |  7 ++-----
>  drivers/dma/moxart-dma.c         |  5 +----
>  drivers/dma/mxs-dma.c            |  3 +--
>  drivers/dma/nbpfaxi.c            |  7 ++-----
>  drivers/dma/pch_dma.c            |  4 +---
>  drivers/dma/pl330.c              |  4 +---
>  drivers/dma/pxa_dma.c            |  3 +--
>  drivers/dma/qcom/bam_dma.c       |  7 ++-----
>  drivers/dma/qcom/hidma.c         |  3 +--
>  drivers/dma/s3c24xx-dma.c        | 11 +++--------
>  drivers/dma/sa11x0-dma.c         |  4 +---
>  drivers/dma/sh/rcar-dmac.c       |  5 +----
>  drivers/dma/sh/shdmac.c          |  5 +----
>  drivers/dma/sh/sudmac.c          |  4 +---
>  drivers/dma/sh/usb-dmac.c        |  3 +--
>  drivers/dma/sirf-dma.c           |  7 ++-----
>  drivers/dma/sprd-dma.c           |  7 ++-----
>  drivers/dma/ste_dma40.c          | 14 +++++---------
>  drivers/dma/stm32-dma.c          |  3 +--
>  drivers/dma/stm32-mdma.c         |  4 +---
>  drivers/dma/sun4i-dma.c          |  7 ++-----
>  drivers/dma/sun6i-dma.c          |  7 ++-----
>  drivers/dma/tegra20-apb-dma.c    |  8 ++------
>  drivers/dma/tegra210-adma.c      |  8 ++------
>  drivers/dma/ti/Kconfig           |  2 +-
>  drivers/dma/ti/cppi41.c          |  7 ++-----
>  drivers/dma/ti/edma.c            |  8 ++------
>  drivers/dma/ti/omap-dma.c        |  5 +----
>  drivers/dma/timb_dma.c           |  3 +--
>  drivers/dma/txx9dmac.c           |  4 +---
>  drivers/dma/xgene-dma.c          | 16 ++--------------
>  drivers/dma/xilinx/xilinx_dma.c  |  5 +----
>  drivers/dma/xilinx/zynqmp_dma.c  |  4 +---
>  drivers/dma/zx_dma.c             |  7 ++-----
>  47 files changed, 88 insertions(+), 221 deletions(-)
> 

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Huang Shijie Aug. 3, 2018, 8:48 a.m. UTC | #4
On Fri, Aug 03, 2018 at 11:20:49AM +0300, Peter Ujfalusi wrote:
> Hi,
> 
> On 2018-08-03 10:19, Huang Shijie wrote:
> > All the patches are using dmaenginem_async_device_register to simplify code
> > except the last one:
> >   dmaengine: add COMPILE_TEST for the drivers
> > 
> > I use the last one to do the compiler test.  
> > There are still 20 drivers which do not use the dmaenginem_async_device_register.
> > Let me take a rest, if this patch set is accepted, I will do the rest.
> 
> I think for most of the drivers this series is going to open a race
> which is essentially:
> 
> prior:
> _remove()
> {
> ...
> 	dma_async_device_register()
> 	/* Free resources, disable HW, etc */
> 	return 0;
> }
> 
> after:
> _remove()
> {
> ...
> 	/* Free resources, disable HW, etc */
> 	/*
> 	 * The dma device is still registered and a channel can be
> 	 * requested
> 	 */
> 
> 	dma_async_device_register()
> 	return 0;
> }
> 
> It might be theoretical, but conversion to managed device resources is
> not straight forward in some cases. 
Yes. 

okay, let me think again..

@Vinod, please just ignore this patch set.

Thanks
Huang Shijie
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Huang Shijie Aug. 5, 2018, 1:03 a.m. UTC | #5
On Fri, Aug 03, 2018 at 11:20:49AM +0300, Peter Ujfalusi wrote:
> Hi,
> 
> On 2018-08-03 10:19, Huang Shijie wrote:
> > All the patches are using dmaenginem_async_device_register to simplify code
> > except the last one:
> >   dmaengine: add COMPILE_TEST for the drivers
> > 
> > I use the last one to do the compiler test.  
> > There are still 20 drivers which do not use the dmaenginem_async_device_register.
> > Let me take a rest, if this patch set is accepted, I will do the rest.
> 
> I think for most of the drivers this series is going to open a race
> which is essentially:
> 
> prior:
> _remove()
> {
> ...
> 	dma_async_device_register()
> 	/* Free resources, disable HW, etc */

If these resources are managed by the devm_* APIs.
We can use the dmaenginem_async_device_register().

The Devres will release all the things in the following order:
       ................................................
 	dma_async_device_unregister()
 	/* Free resources, disable HW, etc */
       ................................................

    
Thanks
Huang Shijie
--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html