From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail.kernel.org ([198.145.29.99]:56842 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726102AbeLEIdG (ORCPT ); Wed, 5 Dec 2018 03:33:06 -0500 Date: Wed, 5 Dec 2018 14:02:56 +0530 From: Vinod Koul To: Peter Ujfalusi Cc: Bin Liu , dmaengine@vger.kernel.org, linux-usb@vger.kernel.org, Vinod Koul , stable@vger.kernel.org Subject: Re: [PATCH] dma: cppi41: delete channel from pending list when stop channel Message-ID: <20181205083256.GG2847@vkoul-mobl> References: <20181112154049.24129-1-b-liu@ti.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: stable-owner@vger.kernel.org List-ID: On 28-11-18, 13:15, Peter Ujfalusi wrote: > > > On 12/11/2018 17.40, Bin Liu wrote: > > Can you fix up the subject line to: > dmaengine: ti: cppi4: delete channel from pending list when stop channel > > > The driver defines three states for a cppi channel. > > - idle: .chan_busy == 0 && not in .pending list > > - pending: .chan_busy == 0 && in .pending list > > - busy: .chan_busy == 1 && not in .pending list > > > > There are cases in which the cppi channel could be in the pending state > > when cppi41_dma_issue_pending() is called after cppi41_runtime_suspend() > > is called. > > > > cppi41_stop_chan() has a bug for these cases to set channels to idle state. > > It only checks the .chan_busy flag, but not the .pending list, then later > > when cppi41_runtime_resume() is called the channels in .pending list will > > be transitioned to busy state. > > > > Removing channels from the .pending list solves the problem. > > So, let me see if I understand this correctly: > - client issued a transfer _after_ the cppi4 driver is suspended > - cppi41_dma_issue_pending() will place it to pending list and will not > start the transfer right away as cdd->is_suspended is true. > - on resume the cppi4 will pick up the pending transfers from the > pending list > > This is so far a sane thing to do. > > If I guess right, then after the issue_pending the client driver will > call terminate_all, presumably from it's suspend callback? > > As per the purpose of terminate_all we should terminated all future > transfers on the channel, so clearing the pending list is the correct > thing to do. > > With the fixed subject: > Reviewed-by: Peter Ujfalusi Thanks Peter, Applied after fixing the title, thanks -- ~Vinod