Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
MIME-Version: 1.0
References: <20231102121623.31924-1-kaiwei.liu@unisoc.com> <ZWCg9hmfvexyn7xK@matsya>
In-Reply-To: <ZWCg9hmfvexyn7xK@matsya>
From:   liu kaiwei <liukaiwei086@gmail.com>
Date:   Wed, 6 Dec 2023 17:32:12 +0800
Message-ID: <CAOgAA6FzZ4q=rdmh8ySJRhojkGCgyV4PVjT6JAOUix+CF9PFtw@mail.gmail.com>
Subject: Re: [PATCH 1/2] dmaengine: sprd: delete enable opreation in probe
To:     Vinod Koul <vkoul@kernel.org>
Cc:     Kaiwei Liu <kaiwei.liu@unisoc.com>,
        Orson Zhai <orsonzhai@gmail.com>,
        Baolin Wang <baolin.wang@linux.alibaba.com>,
        Chunyan Zhang <zhang.lyra@gmail.com>,
        dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org,
        Wenming Wu <wenming.wu@unisoc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Nov 24, 2023 at 9:11=E2=80=AFPM Vinod Koul <vkoul@kernel.org> wrote=
:
>
> On 02-11-23, 20:16, Kaiwei Liu wrote:
> > From: "kaiwei.liu" <kaiwei.liu@unisoc.com>
>
> 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@unisoc.com>
> > ---
> >  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_devic=
e *pdev)
> >       }
> >
> >       platform_set_drvdata(pdev, sdev);
> > -     ret =3D sprd_dma_enable(sdev);
> > -     if (ret)
> > -             return ret;
> > -
> > -     pm_runtime_set_active(&pdev->dev);
> > -     pm_runtime_enable(&pdev->dev);
> > -
> > -     ret =3D pm_runtime_get_sync(&pdev->dev);
> > -     if (ret < 0)
> > -             goto err_rpm;
> >
> >       ret =3D dma_async_device_register(&sdev->dma_dev);
> >       if (ret < 0) {
> >               dev_err(&pdev->dev, "register dma device failed:%d\n", re=
t);
> > -             goto err_register;
> > +             return ret;
> >       }
> >
> >       sprd_dma_info.dma_cap =3D sdev->dma_dev.cap_mask;
> > @@ -1226,16 +1216,11 @@ static int sprd_dma_probe(struct platform_devic=
e *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