linux-kernel - Re: [PATCH] dmaengine: zynqmp_dma: Add shutdown operation support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aFL91fq7SPAiXOak@vaman>
Date: Wed, 18 Jun 2025 23:26:37 +0530
From: Vinod Koul <vkoul@...nel.org>
To: Uwe Kleine-König <u.kleine-koenig@...libre.com>
Cc: Abin Joseph <abin.joseph@....com>, michal.simek@....com,
	yanzhen@...o.com, radhey.shyam.pandey@....com, palmer@...osinc.com,
	git@....com, dmaengine@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dmaengine: zynqmp_dma: Add shutdown operation support

On 17-06-25, 09:43, Uwe Kleine-König wrote:
> Hello Vinod,
> 
> On Tue, Jun 17, 2025 at 12:08:53PM +0530, Vinod Koul wrote:
> > On 12-06-25, 21:51, Abin Joseph wrote:
> > > Implement shutdown hook to ensure dmaengine could be stopped inorder for
> > > kexec to restart the new kernel.
> > > 
> > > Signed-off-by: Abin Joseph <abin.joseph@....com>
> > > ---
> > >  drivers/dma/xilinx/zynqmp_dma.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/drivers/dma/xilinx/zynqmp_dma.c b/drivers/dma/xilinx/zynqmp_dma.c
> > > index d05fc5fcc77d..8f9f1ef4f0bf 100644
> > > --- a/drivers/dma/xilinx/zynqmp_dma.c
> > > +++ b/drivers/dma/xilinx/zynqmp_dma.c
> > > @@ -1178,6 +1178,18 @@ static void zynqmp_dma_remove(struct platform_device *pdev)
> > >  		zynqmp_dma_runtime_suspend(zdev->dev);
> > >  }
> > >  
> > > +/**
> > > + * zynqmp_dma_shutdown - Driver shutdown function
> > > + * @pdev: Pointer to the platform_device structure
> > > + */
> > > +static void zynqmp_dma_shutdown(struct platform_device *pdev)
> > > +{
> > > +	struct zynqmp_dma_device *zdev = platform_get_drvdata(pdev);
> > > +
> > > +	zynqmp_dma_chan_remove(zdev->chan);
> > > +	pm_runtime_disable(zdev->dev);
> > > +}
> > > +
> > >  static const struct of_device_id zynqmp_dma_of_match[] = {
> > >  	{ .compatible = "amd,versal2-dma-1.0", .data = &versal2_dma_config },
> > >  	{ .compatible = "xlnx,zynqmp-dma-1.0", },
> > > @@ -1193,6 +1205,7 @@ static struct platform_driver zynqmp_dma_driver = {
> > >  	},
> > >  	.probe = zynqmp_dma_probe,
> > >  	.remove = zynqmp_dma_remove,
> > > +	.shutdown = zynqmp_dma_shutdown,
> > 
> > Why not do all operations performed in remove..?
> 
> .remove() isn't called on shutdown.

Yes that is correct

> Having said that, most other drivers also don't handle .shutdown(). IMHO
> this is special enough that this warrants a comment. Or is kexec a
> reason to silence *all* DMA and most drivers should have a .shutdown
> callback?

My point was remove does a lot of work to quiesce the dma transactions,
terminate the pending work etc, all those steps should be ideally done
on shutdown too, hence question of why not do all the steps done in
remove as well

But yes, you are bringing another good point for drivers, they should
have a shutdown callback implemented

-- 
~Vinod