Re: [PATCH 1/2] dmaengine: sprd: delete enable opreation in probe

[liu kaiwei]

On Fri, Nov 24, 2023 at 9:11 PM Vinod Koul <vkoul@xxxxxxxxxx> wrote:
>
> On 02-11-23, 20:16, Kaiwei Liu wrote:
> > From: "kaiwei.liu" <kaiwei.liu@xxxxxxxxxx>
>
> Typo is subject line
>
> >
> > In the probe of dma, it will allocate device memory and do some
> > initalization settings. All operations are only at the software
> > level and don't need the DMA hardware power on. It doesn't need
> > to resume the device and set the device active as well. here
> > delete unnecessary operation.
>
> Don't you need to read or write to the device? Without enable that wont
> work right?
>

Yes, it doesn't need to read or write to the device in the probe of DMA.
We will enable the DMA when allocating the DMA channel.

> Lastly patches appear disjoint, pls thread them properly
>
> >
> > Signed-off-by: kaiwei.liu <kaiwei.liu@xxxxxxxxxx>
> > ---
> >  drivers/dma/sprd-dma.c | 19 ++-----------------
> >  1 file changed, 2 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/dma/sprd-dma.c b/drivers/dma/sprd-dma.c
> > index 08fcf1ec368c..8ab5a9082fc5 100644
> > --- a/drivers/dma/sprd-dma.c
> > +++ b/drivers/dma/sprd-dma.c
> > @@ -1203,21 +1203,11 @@ static int sprd_dma_probe(struct platform_device *pdev)
> >       }
> >
> >       platform_set_drvdata(pdev, sdev);
> > -     ret = sprd_dma_enable(sdev);
> > -     if (ret)
> > -             return ret;
> > -
> > -     pm_runtime_set_active(&pdev->dev);
> > -     pm_runtime_enable(&pdev->dev);
> > -
> > -     ret = pm_runtime_get_sync(&pdev->dev);
> > -     if (ret < 0)
> > -             goto err_rpm;
> >
> >       ret = dma_async_device_register(&sdev->dma_dev);
> >       if (ret < 0) {
> >               dev_err(&pdev->dev, "register dma device failed:%d\n", ret);
> > -             goto err_register;
> > +             return ret;
> >       }
> >
> >       sprd_dma_info.dma_cap = sdev->dma_dev.cap_mask;
> > @@ -1226,16 +1216,11 @@ static int sprd_dma_probe(struct platform_device *pdev)
> >       if (ret)
> >               goto err_of_register;
> >
> > -     pm_runtime_put(&pdev->dev);
> > +     pm_runtime_enable(&pdev->dev);
> >       return 0;
> >
> >  err_of_register:
> >       dma_async_device_unregister(&sdev->dma_dev);
> > -err_register:
> > -     pm_runtime_put_noidle(&pdev->dev);
> > -     pm_runtime_disable(&pdev->dev);
> > -err_rpm:
> > -     sprd_dma_disable(sdev);
> >       return ret;
> >  }
> >
> > --
> > 2.17.1
>
> --
> ~Vinod