[v1] dmaengine: zynqmp_dma: Add shutdown operation support

[PATCH] dmaengine: zynqmp_dma: Add shutdown operation support

Posted by Abin Joseph 4 months ago

Implement shutdown hook to ensure dmaengine could be stopped inorder for
kexec to restart the new kernel.

Signed-off-by: Abin Joseph <abin.joseph@amd.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,
 };
 
 module_platform_driver(zynqmp_dma_driver);
-- 
2.34.1

Re: [PATCH] dmaengine: zynqmp_dma: Add shutdown operation support

Posted by Vinod Koul 3 months, 3 weeks ago

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@amd.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..?

>  };
>  
>  module_platform_driver(zynqmp_dma_driver);
> -- 
> 2.34.1

-- 
~Vinod

Re: [PATCH] dmaengine: zynqmp_dma: Add shutdown operation support

Posted by Uwe Kleine-König 3 months, 3 weeks ago

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@amd.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.

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?

Best regards
Uwe

Re: [PATCH] dmaengine: zynqmp_dma: Add shutdown operation support

Posted by Vinod Koul 3 months, 3 weeks ago

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@amd.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