From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com ([192.55.52.115]:37410 "EHLO mga14.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751844AbdENNMS (ORCPT ); Sun, 14 May 2017 09:12:18 -0400 Date: Sun, 14 May 2017 18:44:25 +0530 From: Vinod Koul To: Alexander Sverdlin Cc: dmaengine@vger.kernel.org, Dan Williams , stable@vger.kernel.org Subject: Re: [PATCH 1/2] dmaengine: ep93xx: Always start from BASE0 Message-ID: <20170514131424.GM6263@localhost> References: <20170513221327.10114-1-alexander.sverdlin@gmail.com> <20170513221327.10114-2-alexander.sverdlin@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170513221327.10114-2-alexander.sverdlin@gmail.com> Sender: stable-owner@vger.kernel.org List-ID: On Sun, May 14, 2017 at 12:13:26AM +0200, Alexander Sverdlin wrote: > The current buffer is being reset to zero on device_free_chan_resources() > but not on device_terminate_all(). It could happen that HW is restarted and > expects BASE0 to be used, but the driver is not synchronized and will start > from BASE1. Therefore, it's better to reset the buffer explicitly in > m2p_hw_setup(). While here, check that HW really starts from BASE0 and > warn otherwise. > > Signed-off-by: Alexander Sverdlin > Cc: stable@vger.kernel.org > --- > drivers/dma/ep93xx_dma.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c > index d37e8dda8079..a3d946d2a8e1 100644 > --- a/drivers/dma/ep93xx_dma.c > +++ b/drivers/dma/ep93xx_dma.c > @@ -45,6 +45,7 @@ > > #define M2P_PPALLOC 0x0008 > #define M2P_STATUS 0x000c > +#define M2P_STATUS_NEXTBUFFER BIT(6) > > #define M2P_MAXCNT0 0x0020 > #define M2P_BASE0 0x0024 > @@ -312,6 +313,13 @@ static void m2p_set_control(struct ep93xx_dma_chan *edmac, u32 control) > readl(edmac->regs + M2P_CONTROL); > } > > +static inline u32 m2p_channel_nextbuf(struct ep93xx_dma_chan *edmac) > +{ > + if (readl(edmac->regs + M2P_STATUS) & M2P_STATUS_NEXTBUFFER) > + return 1; > + return 0; > +} > + > static int m2p_hw_setup(struct ep93xx_dma_chan *edmac) > { > struct ep93xx_dma_data *data = edmac->chan.private; > @@ -323,6 +331,10 @@ static int m2p_hw_setup(struct ep93xx_dma_chan *edmac) > | M2P_CONTROL_ENABLE; > m2p_set_control(edmac, control); > > + if (m2p_channel_nextbuf(edmac) != 0) > + dev_warn(chan2dev(edmac), "M2P: Starting from BASE1\n"); But then you are actually not restarting from BASE1 as you reset, so the warn is wrong.. Perhaps a more meaningful msg would be to say "expected 0 but found 1, so resetting" > + edmac->buffer = 0; > + > return 0; > } > > -- > 2.12.2 > -- ~Vinod