On Mon, Feb 24, 2020 at 01:24:04PM +0000, Andreas Tobler wrote: > Hi all, > > On 21.02.20 08:39, Greg Kroah-Hartman wrote: > > From: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> > > > > [ Upstream commit 02939cd167095f16328a1bd5cab5a90b550606df ] > > > > The current descriptor is not on any list of the virtual DMA channel. > > Once sdma_terminate_all() is called when a descriptor is currently > > in flight then this one is forgotten to be freed. We have to call > > vchan_terminate_vdesc() on this descriptor to re-add it to the lists. > > Now that we also free the currently running descriptor we can (and > > actually have to) remove the current descriptor from its list also > > for the cyclic case. > > > > Signed-off-by: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> > > Reviewed-by: Robin Gong <yibin.gong@xxxxxxx> > > Tested-by: Robin Gong <yibin.gong@xxxxxxx> > > Link: https://lore.kernel.org/r/20191216105328.15198-10-s.hauer@xxxxxxxxxxxxxx > > Signed-off-by: Vinod Koul <vkoul@xxxxxxxxxx> > > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> > > --- > > drivers/dma/imx-sdma.c | 19 +++++++++++-------- > > 1 file changed, 11 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c > > index c27e206a764c3..66f1b2ac5cde4 100644 > > --- a/drivers/dma/imx-sdma.c > > +++ b/drivers/dma/imx-sdma.c > > @@ -760,12 +760,8 @@ static void sdma_start_desc(struct sdma_channel *sdmac) > > return; > > } > > sdmac->desc = desc = to_sdma_desc(&vd->tx); > > - /* > > - * Do not delete the node in desc_issued list in cyclic mode, otherwise > > - * the desc allocated will never be freed in vchan_dma_desc_free_list > > - */ > > - if (!(sdmac->flags & IMX_DMA_SG_LOOP)) > > - list_del(&vd->node); > > + > > + list_del(&vd->node); > > > > sdma->channel_control[channel].base_bd_ptr = desc->bd_phys; > > sdma->channel_control[channel].current_bd_ptr = desc->bd_phys; > > @@ -1071,7 +1067,6 @@ static void sdma_channel_terminate_work(struct work_struct *work) > > > > spin_lock_irqsave(&sdmac->vc.lock, flags); > > vchan_get_all_descriptors(&sdmac->vc, &head); > > - sdmac->desc = NULL; > > spin_unlock_irqrestore(&sdmac->vc.lock, flags); > > vchan_dma_desc_free_list(&sdmac->vc, &head); > > sdmac->context_loaded = false; > > @@ -1080,11 +1075,19 @@ static void sdma_channel_terminate_work(struct work_struct *work) > > static int sdma_disable_channel_async(struct dma_chan *chan) > > { > > struct sdma_channel *sdmac = to_sdma_chan(chan); > > + unsigned long flags; > > + > > + spin_lock_irqsave(&sdmac->vc.lock, flags); > > > > sdma_disable_channel(chan); > > > > - if (sdmac->desc) > > + if (sdmac->desc) { > > + vchan_terminate_vdesc(&sdmac->desc->vd); > > + sdmac->desc = NULL; > > schedule_work(&sdmac->terminate_worker); > > + } > > + > > + spin_unlock_irqrestore(&sdmac->vc.lock, flags); > > > > return 0; > > } > > > > This patch breaks our imx6 board with the attached trace. Reverting the > patch makes it boot again. > I tried also 5.6-rc3 and it booted too. A closer look into imx-sdma.c > from 5.6-rc3 showed me some details which might have to be backported as > well to make this patch work. > I tried a1ff6a07f5a3951fcac84f064a76d1ad79c10e40 and was somehow > successful. I still have one trace but the board boots now. > > Any insights from the experts? This series should be applied as a whole or not, only 7/9 is optional. It seems I have to avoid the trigger word "fix" in my commit messages or make sure these patches won't apply without their dependencies :-/ Sascha -- Pengutronix e.K. | | Steuerwalder Str. 21 | http://www.pengutronix.de/ | 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
