* [PATCH 0/2] dma: mmp_pdma: Fix phy channels not protected issue @ 2013-06-03 8:02 Xiang Wang 2013-06-03 8:02 ` [PATCH 1/2] dma: mmp_pdma: add protect when alloc/free phy channels Xiang Wang 2013-06-03 8:02 ` [PATCH 2/2] dma: mmp_pdma: clear DRCMR when free a phy channel Xiang Wang 0 siblings, 2 replies; 6+ messages in thread From: Xiang Wang @ 2013-06-03 8:02 UTC (permalink / raw) To: Dan Williams, Vinod Koul, linux-kernel; +Cc: wangxfdu, cxie4, Xiang Wang From: Xiang Wang <wangx@marvell.com> This patch set deals with the issues that 1) phy channels are not protected in mmp_pdma. 2) dma request<->channel mapping is not cleared when a phy chan is freed. Xiang Wang (2): dma: mmp_pdma: add protect when alloc/free phy channels dma: mmp_pdma: clear DRCMR when free a phy channel drivers/dma/mmp_pdma.c | 47 +++++++++++++++++++++++++++++++---------------- 1 files changed, 31 insertions(+), 16 deletions(-) -- 1.7.5.4 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] dma: mmp_pdma: add protect when alloc/free phy channels 2013-06-03 8:02 [PATCH 0/2] dma: mmp_pdma: Fix phy channels not protected issue Xiang Wang @ 2013-06-03 8:02 ` Xiang Wang 2013-06-17 13:26 ` Vinod Koul 2013-06-03 8:02 ` [PATCH 2/2] dma: mmp_pdma: clear DRCMR when free a phy channel Xiang Wang 1 sibling, 1 reply; 6+ messages in thread From: Xiang Wang @ 2013-06-03 8:02 UTC (permalink / raw) To: Dan Williams, Vinod Koul, linux-kernel; +Cc: wangxfdu, cxie4, Xiang Wang From: Xiang Wang <wangx@marvell.com> In mmp pdma, phy channels are allocated/freed dynamically and frequently. But no proper protection is added. Conflict will happen when multi-users are requesting phy channels at the same time. Use spinlock to protect. Signed-off-by: Xiang Wang <wangx@marvell.com> --- drivers/dma/mmp_pdma.c | 41 +++++++++++++++++++++++++---------------- 1 files changed, 25 insertions(+), 16 deletions(-) diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c index c26699f..21820c9 100644 --- a/drivers/dma/mmp_pdma.c +++ b/drivers/dma/mmp_pdma.c @@ -121,6 +121,7 @@ struct mmp_pdma_device { struct device *dev; struct dma_device device; struct mmp_pdma_phy *phy; + spinlock_t phy_lock; /* protect alloc/free phy channels */ }; #define tx_to_mmp_pdma_desc(tx) container_of(tx, struct mmp_pdma_desc_sw, async_tx) @@ -219,6 +220,7 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan) int prio, i; struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device); struct mmp_pdma_phy *phy; + unsigned long flags; /* * dma channel priorities @@ -227,6 +229,8 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan) * ch 8 - 11, 24 - 27 <--> (2) * ch 12 - 15, 28 - 31 <--> (3) */ + + spin_lock_irqsave(&pdev->phy_lock, flags); for (prio = 0; prio <= (((pdev->dma_channels - 1) & 0xf) >> 2); prio++) { for (i = 0; i < pdev->dma_channels; i++) { if (prio != ((i & 0xf) >> 2)) @@ -234,14 +238,29 @@ static struct mmp_pdma_phy *lookup_phy(struct mmp_pdma_chan *pchan) phy = &pdev->phy[i]; if (!phy->vchan) { phy->vchan = pchan; + spin_unlock_irqrestore(&pdev->phy_lock, flags); return phy; } } } + spin_unlock_irqrestore(&pdev->phy_lock, flags); return NULL; } +static void free_phy(struct mmp_pdma_chan *pchan) +{ + struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device); + unsigned long flags; + if (!pchan->phy) + return; + + spin_lock_irqsave(&pdev->phy_lock, flags); + pchan->phy->vchan = NULL; + pchan->phy = NULL; + spin_unlock_irqrestore(&pdev->phy_lock, flags); +} + /* desc->tx_list ==> pending list */ static void append_pending_queue(struct mmp_pdma_chan *chan, struct mmp_pdma_desc_sw *desc) @@ -277,10 +296,7 @@ static void start_pending_queue(struct mmp_pdma_chan *chan) if (list_empty(&chan->chain_pending)) { /* chance to re-fetch phy channel with higher prio */ - if (chan->phy) { - chan->phy->vchan = NULL; - chan->phy = NULL; - } + free_phy(chan); dev_dbg(chan->dev, "no pending list\n"); return; } @@ -377,10 +393,7 @@ static int mmp_pdma_alloc_chan_resources(struct dma_chan *dchan) dev_err(chan->dev, "unable to allocate descriptor pool\n"); return -ENOMEM; } - if (chan->phy) { - chan->phy->vchan = NULL; - chan->phy = NULL; - } + free_phy(chan); chan->idle = true; chan->dev_addr = 0; return 1; @@ -411,10 +424,7 @@ static void mmp_pdma_free_chan_resources(struct dma_chan *dchan) chan->desc_pool = NULL; chan->idle = true; chan->dev_addr = 0; - if (chan->phy) { - chan->phy->vchan = NULL; - chan->phy = NULL; - } + free_phy(chan); return; } @@ -581,10 +591,7 @@ static int mmp_pdma_control(struct dma_chan *dchan, enum dma_ctrl_cmd cmd, switch (cmd) { case DMA_TERMINATE_ALL: disable_chan(chan->phy); - if (chan->phy) { - chan->phy->vchan = NULL; - chan->phy = NULL; - } + free_phy(chan); spin_lock_irqsave(&chan->desc_lock, flags); mmp_pdma_free_desc_list(chan, &chan->chain_pending); mmp_pdma_free_desc_list(chan, &chan->chain_running); @@ -777,6 +784,8 @@ static int mmp_pdma_probe(struct platform_device *op) return -ENOMEM; pdev->dev = &op->dev; + spin_lock_init(&pdev->phy_lock); + iores = platform_get_resource(op, IORESOURCE_MEM, 0); if (!iores) return -EINVAL; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] dma: mmp_pdma: add protect when alloc/free phy channels 2013-06-03 8:02 ` [PATCH 1/2] dma: mmp_pdma: add protect when alloc/free phy channels Xiang Wang @ 2013-06-17 13:26 ` Vinod Koul 2013-06-18 8:47 ` Xiang Wang 0 siblings, 1 reply; 6+ messages in thread From: Vinod Koul @ 2013-06-17 13:26 UTC (permalink / raw) To: Xiang Wang; +Cc: Dan Williams, linux-kernel, cxie4, Xiang Wang On Mon, Jun 03, 2013 at 04:02:08PM +0800, Xiang Wang wrote: > From: Xiang Wang <wangx@marvell.com> > > In mmp pdma, phy channels are allocated/freed dynamically > and frequently. But no proper protection is added. > Conflict will happen when multi-users are requesting phy > channels at the same time. Use spinlock to protect. > > Signed-off-by: Xiang Wang <wangx@marvell.com> > --- > drivers/dma/mmp_pdma.c | 41 +++++++++++++++++++++++++---------------- > 1 files changed, 25 insertions(+), 16 deletions(-) > > +static void free_phy(struct mmp_pdma_chan *pchan) pls namespace this > +{ > + struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device); > + unsigned long flags; empty line pls > + if (!pchan->phy) > + return; > + > + spin_lock_irqsave(&pdev->phy_lock, flags); > + pchan->phy->vchan = NULL; > + pchan->phy = NULL; > + spin_unlock_irqrestore(&pdev->phy_lock, flags); > +} > + rest looks okay -- ~Vinod ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] dma: mmp_pdma: add protect when alloc/free phy channels 2013-06-17 13:26 ` Vinod Koul @ 2013-06-18 8:47 ` Xiang Wang 0 siblings, 0 replies; 6+ messages in thread From: Xiang Wang @ 2013-06-18 8:47 UTC (permalink / raw) To: Vinod Koul; +Cc: Dan Williams, linux-kernel@vger.kernel.org, cxie4, Xiang Wang 2013/6/17 Vinod Koul <vinod.koul@intel.com>: > On Mon, Jun 03, 2013 at 04:02:08PM +0800, Xiang Wang wrote: >> From: Xiang Wang <wangx@marvell.com> >> >> In mmp pdma, phy channels are allocated/freed dynamically >> and frequently. But no proper protection is added. >> Conflict will happen when multi-users are requesting phy >> channels at the same time. Use spinlock to protect. >> >> Signed-off-by: Xiang Wang <wangx@marvell.com> >> --- >> drivers/dma/mmp_pdma.c | 41 +++++++++++++++++++++++++---------------- >> 1 files changed, 25 insertions(+), 16 deletions(-) >> > >> +static void free_phy(struct mmp_pdma_chan *pchan) > pls namespace this Updated in patch set V3. Thanks! >> +{ >> + struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device); >> + unsigned long flags; > empty line pls Updated in patch set V3. Thanks! > >> + if (!pchan->phy) >> + return; >> + >> + spin_lock_irqsave(&pdev->phy_lock, flags); >> + pchan->phy->vchan = NULL; >> + pchan->phy = NULL; >> + spin_unlock_irqrestore(&pdev->phy_lock, flags); >> +} >> + > rest looks okay > > -- > ~Vinod -- Regards, Xiang ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] dma: mmp_pdma: clear DRCMR when free a phy channel 2013-06-03 8:02 [PATCH 0/2] dma: mmp_pdma: Fix phy channels not protected issue Xiang Wang 2013-06-03 8:02 ` [PATCH 1/2] dma: mmp_pdma: add protect when alloc/free phy channels Xiang Wang @ 2013-06-03 8:02 ` Xiang Wang 2013-06-03 10:07 ` Andy Shevchenko 1 sibling, 1 reply; 6+ messages in thread From: Xiang Wang @ 2013-06-03 8:02 UTC (permalink / raw) To: Dan Williams, Vinod Koul, linux-kernel; +Cc: wangxfdu, cxie4, Xiang Wang From: Xiang Wang <wangx@marvell.com> In mmp pdma, phy channels are allocated/freed dynamically. The mapping from DMA request to DMA channel number in DRCMR should be cleared when a phy channel is freed. Otherwise conflicts will happen when: 1. A is using channel 2 and free it after finished, but A still maps to channel 2 in DRCMR of A. 2. Now another one B gets channel 2. So B maps to channel 2 too in DRCMR of B. In the datasheet, it is described that "Do not map two active requests to the same channel since it produces unpredictable results" and we can observe that during test. Signed-off-by: Xiang Wang <wangx@marvell.com> --- drivers/dma/mmp_pdma.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/dma/mmp_pdma.c b/drivers/dma/mmp_pdma.c index 21820c9..4cd7276 100644 --- a/drivers/dma/mmp_pdma.c +++ b/drivers/dma/mmp_pdma.c @@ -252,9 +252,15 @@ static void free_phy(struct mmp_pdma_chan *pchan) { struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device); unsigned long flags; + u32 reg; if (!pchan->phy) return; + /* clear the channel mapping in DRCMR */ + reg = pchan->phy->vchan->drcmr; + reg = (((reg) < 64) ? 0x0100 : 0x1100) + (((reg) & 0x3f) << 2); + writel(0, pchan->phy->base + reg); + spin_lock_irqsave(&pdev->phy_lock, flags); pchan->phy->vchan = NULL; pchan->phy = NULL; -- 1.7.5.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] dma: mmp_pdma: clear DRCMR when free a phy channel 2013-06-03 8:02 ` [PATCH 2/2] dma: mmp_pdma: clear DRCMR when free a phy channel Xiang Wang @ 2013-06-03 10:07 ` Andy Shevchenko 0 siblings, 0 replies; 6+ messages in thread From: Andy Shevchenko @ 2013-06-03 10:07 UTC (permalink / raw) To: Xiang Wang Cc: Dan Williams, Vinod Koul, linux-kernel@vger.kernel.org, cxie4, Xiang Wang On Mon, Jun 3, 2013 at 11:02 AM, Xiang Wang <wangxfdu@gmail.com> wrote: > From: Xiang Wang <wangx@marvell.com> > > In mmp pdma, phy channels are allocated/freed dynamically. > The mapping from DMA request to DMA channel number in DRCMR > should be cleared when a phy channel is freed. Otherwise > conflicts will happen when: > 1. A is using channel 2 and free it after finished, but A > still maps to channel 2 in DRCMR of A. > 2. Now another one B gets channel 2. So B maps to channel 2 > too in DRCMR of B. > In the datasheet, it is described that "Do not map two active > requests to the same channel since it produces unpredictable > results" and we can observe that during test. > --- a/drivers/dma/mmp_pdma.c > +++ b/drivers/dma/mmp_pdma.c > @@ -252,9 +252,15 @@ static void free_phy(struct mmp_pdma_chan *pchan) > { > struct mmp_pdma_device *pdev = to_mmp_pdma_dev(pchan->chan.device); > unsigned long flags; > + u32 reg; May be empty line here (and maybe even in previous patch)? > if (!pchan->phy) > return; > > + /* clear the channel mapping in DRCMR */ > + reg = pchan->phy->vchan->drcmr; > + reg = (((reg) < 64) ? 0x0100 : 0x1100) + (((reg) & 0x3f) << 2); Too many braces. It is not a macro, you don't need to embrace reg. > + writel(0, pchan->phy->base + reg); > + > spin_lock_irqsave(&pdev->phy_lock, flags); > pchan->phy->vchan = NULL; > pchan->phy = NULL; -- With Best Regards, Andy Shevchenko ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-18 8:47 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-06-03 8:02 [PATCH 0/2] dma: mmp_pdma: Fix phy channels not protected issue Xiang Wang 2013-06-03 8:02 ` [PATCH 1/2] dma: mmp_pdma: add protect when alloc/free phy channels Xiang Wang 2013-06-17 13:26 ` Vinod Koul 2013-06-18 8:47 ` Xiang Wang 2013-06-03 8:02 ` [PATCH 2/2] dma: mmp_pdma: clear DRCMR when free a phy channel Xiang Wang 2013-06-03 10:07 ` Andy Shevchenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox