| Message ID | 20230515085352.25794-3-biju.das.jz@bp.renesas.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | Add RZ/G2L DMA Reset support | expand |
Hi! > Add reset support for DMAC module found on RZ/G2L alike SoCs. > > For booting the board, reset release of the DMAC module is required > otherwise we don't get GIC interrupts. Currently the reset release > was done by the bootloader now move this to the driver instead. > @@ -896,6 +903,10 @@ static int rz_dmac_probe(struct platform_device *pdev) > goto err_pm_disable; > } > > + ret = reset_control_deassert(dmac->rstc); > + if (ret) > + goto err_pm_runtime_put; > + > for (i = 0; i < dmac->n_channels; i++) { > ret = rz_dmac_chan_probe(dmac, &dmac->channels[i], i); > if (ret < 0) > @@ -940,6 +951,7 @@ static int rz_dmac_probe(struct platform_device *pdev) > dma_register_err: > of_dma_controller_free(pdev->dev.of_node); > err: > + reset_control_assert(dmac->rstc); > channel_num = i ? i - 1 : 0; > for (i = 0; i < channel_num; i++) { > struct rz_dmac_chan *channel = &dmac->channels[i]; I'd put it reset_control_assert() after the for () loop freeing the channels, to be symetrical with the init order. > @@ -972,6 +985,7 @@ static int rz_dmac_remove(struct platform_device *pdev) > } > of_dma_controller_free(pdev->dev.of_node); > dma_async_device_unregister(&dmac->engine); > + reset_control_assert(dmac->rstc); > pm_runtime_put(&pdev->dev); > pm_runtime_disable(&pdev->dev); > AFAICT dma_async_device_unregister(&dmac->engine); of_dma_controller_free(pdev->dev.of_node); would better match the init code, too. (This does not need fixing for -cip merge). Thanks and best regards, Pavel
Hi Pavel, Thanks for the feedback. > Subject: Re: [PATCH 5.10.y-cip 2/4] dmaengine: sh: rz-dmac: Add reset > support > > Hi! > > > Add reset support for DMAC module found on RZ/G2L alike SoCs. > > > > For booting the board, reset release of the DMAC module is required > > otherwise we don't get GIC interrupts. Currently the reset release was > > done by the bootloader now move this to the driver instead. > > > @@ -896,6 +903,10 @@ static int rz_dmac_probe(struct platform_device > *pdev) > > goto err_pm_disable; > > } > > > > + ret = reset_control_deassert(dmac->rstc); > > + if (ret) > > + goto err_pm_runtime_put; > > + > > for (i = 0; i < dmac->n_channels; i++) { > > ret = rz_dmac_chan_probe(dmac, &dmac->channels[i], i); > > if (ret < 0) > > @@ -940,6 +951,7 @@ static int rz_dmac_probe(struct platform_device > > *pdev) > > dma_register_err: > > of_dma_controller_free(pdev->dev.of_node); > > err: > > + reset_control_assert(dmac->rstc); > > channel_num = i ? i - 1 : 0; > > for (i = 0; i < channel_num; i++) { > > struct rz_dmac_chan *channel = &dmac->channels[i]; > > I'd put it reset_control_assert() after the for () loop freeing the > channels, to be symetrical with the init order. Agreed. Will move reset_control_assert() after the for () loop. > > > @@ -972,6 +985,7 @@ static int rz_dmac_remove(struct platform_device > *pdev) > > } > > of_dma_controller_free(pdev->dev.of_node); > > dma_async_device_unregister(&dmac->engine); > > + reset_control_assert(dmac->rstc); > > pm_runtime_put(&pdev->dev); > > pm_runtime_disable(&pdev->dev); > > > > AFAICT > > dma_async_device_unregister(&dmac->engine); > of_dma_controller_free(pdev->dev.of_node); > > would better match the init code, too. Good catch. Will fix this as well in mainline. Cheers, Biju
diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index 476847a4916b..6b62e01ba658 100644 --- a/drivers/dma/sh/rz-dmac.c +++ b/drivers/dma/sh/rz-dmac.c @@ -20,6 +20,7 @@ #include <linux/of_platform.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> +#include <linux/reset.h> #include <linux/slab.h> #include <linux/spinlock.h> @@ -92,6 +93,7 @@ struct rz_dmac_chan { struct rz_dmac { struct dma_device engine; struct device *dev; + struct reset_control *rstc; void __iomem *base; void __iomem *ext_base; @@ -889,6 +891,11 @@ static int rz_dmac_probe(struct platform_device *pdev) /* Initialize the channels. */ INIT_LIST_HEAD(&dmac->engine.channels); + dmac->rstc = devm_reset_control_array_get_exclusive(&pdev->dev); + if (IS_ERR(dmac->rstc)) + return dev_err_probe(&pdev->dev, PTR_ERR(dmac->rstc), + "failed to get resets\n"); + pm_runtime_enable(&pdev->dev); ret = pm_runtime_resume_and_get(&pdev->dev); if (ret < 0) { @@ -896,6 +903,10 @@ static int rz_dmac_probe(struct platform_device *pdev) goto err_pm_disable; } + ret = reset_control_deassert(dmac->rstc); + if (ret) + goto err_pm_runtime_put; + for (i = 0; i < dmac->n_channels; i++) { ret = rz_dmac_chan_probe(dmac, &dmac->channels[i], i); if (ret < 0) @@ -940,6 +951,7 @@ static int rz_dmac_probe(struct platform_device *pdev) dma_register_err: of_dma_controller_free(pdev->dev.of_node); err: + reset_control_assert(dmac->rstc); channel_num = i ? i - 1 : 0; for (i = 0; i < channel_num; i++) { struct rz_dmac_chan *channel = &dmac->channels[i]; @@ -950,6 +962,7 @@ static int rz_dmac_probe(struct platform_device *pdev) channel->lmdesc.base_dma); } +err_pm_runtime_put: pm_runtime_put(&pdev->dev); err_pm_disable: pm_runtime_disable(&pdev->dev); @@ -972,6 +985,7 @@ static int rz_dmac_remove(struct platform_device *pdev) } of_dma_controller_free(pdev->dev.of_node); dma_async_device_unregister(&dmac->engine); + reset_control_assert(dmac->rstc); pm_runtime_put(&pdev->dev); pm_runtime_disable(&pdev->dev);