* [PATCH v2 0/2] Append some fixes and improvements @ 2018-04-11 14:44 Pierre-Yves MORDRET 2018-04-11 14:44 ` [PATCH v2 1/2] dmaengine: stm32-mdma: align TLEN and buffer length on burst Pierre-Yves MORDRET 2018-04-11 14:44 ` [PATCH v2 2/2] dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator Pierre-Yves MORDRET 0 siblings, 2 replies; 12+ messages in thread From: Pierre-Yves MORDRET @ 2018-04-11 14:44 UTC (permalink / raw) To: Vinod Koul, Maxime Coquelin, Alexandre Torgue, Dan Williams, M'boumba Cedric Madianga, dmaengine, linux-arm-kernel, linux-kernel Cc: Pierre-Yves MORDRET Fix an issue with FIFO Size and burst size. Fix an incomplete allocator for Hardware descriptors: memory badly allocated. --- Version history: v1: * Initial v2: * Fix kbuild warning format: /0x%08x/%pad/ --- Pierre-Yves MORDRET (2): dmaengine: stm32-mdma: align TLEN and buffer length on burst dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator drivers/dma/stm32-mdma.c | 92 ++++++++++++++++++++++++++++++------------------ 1 file changed, 57 insertions(+), 35 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 1/2] dmaengine: stm32-mdma: align TLEN and buffer length on burst 2018-04-11 14:44 [PATCH v2 0/2] Append some fixes and improvements Pierre-Yves MORDRET @ 2018-04-11 14:44 ` Pierre-Yves MORDRET 2018-04-11 15:14 ` Robin Murphy 2018-04-11 14:44 ` [PATCH v2 2/2] dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator Pierre-Yves MORDRET 1 sibling, 1 reply; 12+ messages in thread From: Pierre-Yves MORDRET @ 2018-04-11 14:44 UTC (permalink / raw) To: Vinod Koul, Maxime Coquelin, Alexandre Torgue, Dan Williams, M'boumba Cedric Madianga, dmaengine, linux-arm-kernel, linux-kernel Cc: Pierre-Yves MORDRET Both buffer Transfer Length (TLEN if any) and transfer size have to be aligned on burst size (burst beats*bus width). Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com> --- Version history: v1: * Initial v2: --- --- drivers/dma/stm32-mdma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c index daa1602..fbcffa2 100644 --- a/drivers/dma/stm32-mdma.c +++ b/drivers/dma/stm32-mdma.c @@ -413,7 +413,7 @@ static u32 stm32_mdma_get_best_burst(u32 buf_len, u32 tlen, u32 max_burst, u32 best_burst = max_burst; u32 burst_len = best_burst * width; - while ((burst_len > 0) && (tlen % burst_len)) { + while ((burst_len > 0) && (((tlen | buf_len) & (burst_len - 1)) != 0)) { best_burst = best_burst >> 1; burst_len = best_burst * width; } -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dmaengine: stm32-mdma: align TLEN and buffer length on burst 2018-04-11 14:44 ` [PATCH v2 1/2] dmaengine: stm32-mdma: align TLEN and buffer length on burst Pierre-Yves MORDRET @ 2018-04-11 15:14 ` Robin Murphy 2018-04-13 9:45 ` Pierre Yves MORDRET 0 siblings, 1 reply; 12+ messages in thread From: Robin Murphy @ 2018-04-11 15:14 UTC (permalink / raw) To: Pierre-Yves MORDRET, Vinod Koul, Maxime Coquelin, Alexandre Torgue, Dan Williams, M'boumba Cedric Madianga, dmaengine, linux-arm-kernel, linux-kernel On 11/04/18 15:44, Pierre-Yves MORDRET wrote: > Both buffer Transfer Length (TLEN if any) and transfer size have to be > aligned on burst size (burst beats*bus width). > > Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com> > --- > Version history: > v1: > * Initial > v2: > --- > --- > drivers/dma/stm32-mdma.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c > index daa1602..fbcffa2 100644 > --- a/drivers/dma/stm32-mdma.c > +++ b/drivers/dma/stm32-mdma.c > @@ -413,7 +413,7 @@ static u32 stm32_mdma_get_best_burst(u32 buf_len, u32 tlen, u32 max_burst, > u32 best_burst = max_burst; > u32 burst_len = best_burst * width; > > - while ((burst_len > 0) && (tlen % burst_len)) { > + while ((burst_len > 0) && (((tlen | buf_len) & (burst_len - 1)) != 0)) { > best_burst = best_burst >> 1; > burst_len = best_burst * width; > } FWIW, doesn't that whole loop come down to just: burst_len = min(ffs(tlen | buf_len), max_burst * width); ? Robin. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dmaengine: stm32-mdma: align TLEN and buffer length on burst 2018-04-11 15:14 ` Robin Murphy @ 2018-04-13 9:45 ` Pierre Yves MORDRET 2018-04-13 11:09 ` Robin Murphy 0 siblings, 1 reply; 12+ messages in thread From: Pierre Yves MORDRET @ 2018-04-13 9:45 UTC (permalink / raw) To: Robin Murphy, Vinod Koul, Maxime Coquelin, Alexandre Torgue, Dan Williams, M'boumba Cedric Madianga, dmaengine, linux-arm-kernel, linux-kernel Hi Robin On 04/11/2018 05:14 PM, Robin Murphy wrote: > On 11/04/18 15:44, Pierre-Yves MORDRET wrote: >> Both buffer Transfer Length (TLEN if any) and transfer size have to be >> aligned on burst size (burst beats*bus width). >> >> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com> >> --- >> Version history: >> v1: >> * Initial >> v2: >> --- >> --- >> drivers/dma/stm32-mdma.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c >> index daa1602..fbcffa2 100644 >> --- a/drivers/dma/stm32-mdma.c >> +++ b/drivers/dma/stm32-mdma.c >> @@ -413,7 +413,7 @@ static u32 stm32_mdma_get_best_burst(u32 buf_len, u32 tlen, u32 max_burst, >> u32 best_burst = max_burst; >> u32 burst_len = best_burst * width; >> >> - while ((burst_len > 0) && (tlen % burst_len)) { >> + while ((burst_len > 0) && (((tlen | buf_len) & (burst_len - 1)) != 0)) { >> best_burst = best_burst >> 1; >> burst_len = best_burst * width; >> } > > FWIW, doesn't that whole loop come down to just: > > burst_len = min(ffs(tlen | buf_len), max_burst * width); No sure it ends as expected. or I miss something or don't understand this statement I tried with "relevant value" : i.e. best_burst = 32, Tlen=128(default) and buf_len = 64, width= 4. This statements gets me something wrong output => 7 instead of 16 * 4. I doubt :) > > ? > > Robin. > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dmaengine: stm32-mdma: align TLEN and buffer length on burst 2018-04-13 9:45 ` Pierre Yves MORDRET @ 2018-04-13 11:09 ` Robin Murphy 2018-04-13 12:30 ` Pierre Yves MORDRET 0 siblings, 1 reply; 12+ messages in thread From: Robin Murphy @ 2018-04-13 11:09 UTC (permalink / raw) To: Pierre Yves MORDRET, Vinod Koul, Maxime Coquelin, Alexandre Torgue, Dan Williams, M'boumba Cedric Madianga, dmaengine, linux-arm-kernel, linux-kernel On 13/04/18 10:45, Pierre Yves MORDRET wrote: > Hi Robin > > On 04/11/2018 05:14 PM, Robin Murphy wrote: >> On 11/04/18 15:44, Pierre-Yves MORDRET wrote: >>> Both buffer Transfer Length (TLEN if any) and transfer size have to be >>> aligned on burst size (burst beats*bus width). >>> >>> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com> >>> --- >>> Version history: >>> v1: >>> * Initial >>> v2: >>> --- >>> --- >>> drivers/dma/stm32-mdma.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c >>> index daa1602..fbcffa2 100644 >>> --- a/drivers/dma/stm32-mdma.c >>> +++ b/drivers/dma/stm32-mdma.c >>> @@ -413,7 +413,7 @@ static u32 stm32_mdma_get_best_burst(u32 buf_len, u32 tlen, u32 max_burst, >>> u32 best_burst = max_burst; >>> u32 burst_len = best_burst * width; >>> >>> - while ((burst_len > 0) && (tlen % burst_len)) { >>> + while ((burst_len > 0) && (((tlen | buf_len) & (burst_len - 1)) != 0)) { >>> best_burst = best_burst >> 1; >>> burst_len = best_burst * width; >>> } >> >> FWIW, doesn't that whole loop come down to just: >> >> burst_len = min(ffs(tlen | buf_len), max_burst * width); > > No sure it ends as expected. or I miss something or don't understand this statement > I tried with "relevant value" : i.e. best_burst = 32, Tlen=128(default) and > buf_len = 64, width= 4. This statements gets me something wrong output => 7 > instead of 16 * 4. > I doubt :) Heh, seems I confused myself halfway through and started thinking max_burst and width were the exponents x rather than the values 2^x... A more representative guess should be: min(1 << __ffs(tlen | buf_len), max_burst * width); but the general point I was trying to make is that a loop checking whether the bottom n bits of something are zero for different values of n is unnecessary when n can simply be calculated directly*. Robin. * in the case of this "just the lowest set bit" idiom there's also the shift-free ((x & (x - 1)) ^ x), but as well as being unreadable it's generally less efficient than (1 << __ffs(x)) for most modern ISAs. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 1/2] dmaengine: stm32-mdma: align TLEN and buffer length on burst 2018-04-13 11:09 ` Robin Murphy @ 2018-04-13 12:30 ` Pierre Yves MORDRET 0 siblings, 0 replies; 12+ messages in thread From: Pierre Yves MORDRET @ 2018-04-13 12:30 UTC (permalink / raw) To: Robin Murphy, Vinod Koul, Maxime Coquelin, Alexandre Torgue, Dan Williams, M'boumba Cedric Madianga, dmaengine, linux-arm-kernel, linux-kernel On 04/13/2018 01:09 PM, Robin Murphy wrote: > On 13/04/18 10:45, Pierre Yves MORDRET wrote: >> Hi Robin >> >> On 04/11/2018 05:14 PM, Robin Murphy wrote: >>> On 11/04/18 15:44, Pierre-Yves MORDRET wrote: >>>> Both buffer Transfer Length (TLEN if any) and transfer size have to be >>>> aligned on burst size (burst beats*bus width). >>>> >>>> Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com> >>>> --- >>>> Version history: >>>> v1: >>>> * Initial >>>> v2: >>>> --- >>>> --- >>>> drivers/dma/stm32-mdma.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c >>>> index daa1602..fbcffa2 100644 >>>> --- a/drivers/dma/stm32-mdma.c >>>> +++ b/drivers/dma/stm32-mdma.c >>>> @@ -413,7 +413,7 @@ static u32 stm32_mdma_get_best_burst(u32 buf_len, u32 tlen, u32 max_burst, >>>> u32 best_burst = max_burst; >>>> u32 burst_len = best_burst * width; >>>> >>>> - while ((burst_len > 0) && (tlen % burst_len)) { >>>> + while ((burst_len > 0) && (((tlen | buf_len) & (burst_len - 1)) != 0)) { >>>> best_burst = best_burst >> 1; >>>> burst_len = best_burst * width; >>>> } >>> >>> FWIW, doesn't that whole loop come down to just: >>> >>> burst_len = min(ffs(tlen | buf_len), max_burst * width); >> >> No sure it ends as expected. or I miss something or don't understand this statement >> I tried with "relevant value" : i.e. best_burst = 32, Tlen=128(default) and >> buf_len = 64, width= 4. This statements gets me something wrong output => 7 >> instead of 16 * 4. >> I doubt :) > > Heh, seems I confused myself halfway through and started thinking > max_burst and width were the exponents x rather than the values 2^x... > > A more representative guess should be: > > min(1 << __ffs(tlen | buf_len), max_burst * width); > > but the general point I was trying to make is that a loop checking > whether the bottom n bits of something are zero for different values of > n is unnecessary when n can simply be calculated directly*. > > Robin. Got the point. I figure how to compute this value with __ffs. Your last statement doesn't provide the good value, but the spirit is here. I just have to adjust with what I want. Thx > > > * in the case of this "just the lowest set bit" idiom there's also the > shift-free ((x & (x - 1)) ^ x), but as well as being unreadable it's > generally less efficient than (1 << __ffs(x)) for most modern ISAs. > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2 2/2] dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator 2018-04-11 14:44 [PATCH v2 0/2] Append some fixes and improvements Pierre-Yves MORDRET 2018-04-11 14:44 ` [PATCH v2 1/2] dmaengine: stm32-mdma: align TLEN and buffer length on burst Pierre-Yves MORDRET @ 2018-04-11 14:44 ` Pierre-Yves MORDRET 2018-04-13 4:02 ` Vinod Koul 2018-04-13 11:48 ` Robin Murphy 1 sibling, 2 replies; 12+ messages in thread From: Pierre-Yves MORDRET @ 2018-04-11 14:44 UTC (permalink / raw) To: Vinod Koul, Maxime Coquelin, Alexandre Torgue, Dan Williams, M'boumba Cedric Madianga, dmaengine, linux-arm-kernel, linux-kernel Cc: Pierre-Yves MORDRET Only 1 Hw Descriptor is allocated. Loop over required Hw descriptor for proper allocation. Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com> --- Version history: v1: * Initial v2: * Fix kbuild warning format: /0x%08x/%pad/ --- --- drivers/dma/stm32-mdma.c | 90 ++++++++++++++++++++++++++++++------------------ 1 file changed, 56 insertions(+), 34 deletions(-) diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c index fbcffa2..3a477bc 100644 --- a/drivers/dma/stm32-mdma.c +++ b/drivers/dma/stm32-mdma.c @@ -252,13 +252,17 @@ struct stm32_mdma_hwdesc { u32 cmdr; } __aligned(64); +struct stm32_mdma_desc_node { + struct stm32_mdma_hwdesc *hwdesc; + dma_addr_t hwdesc_phys; +}; + struct stm32_mdma_desc { struct virt_dma_desc vdesc; u32 ccr; - struct stm32_mdma_hwdesc *hwdesc; - dma_addr_t hwdesc_phys; bool cyclic; u32 count; + struct stm32_mdma_desc_node node[]; }; struct stm32_mdma_chan { @@ -344,30 +348,43 @@ static struct stm32_mdma_desc *stm32_mdma_alloc_desc( struct stm32_mdma_chan *chan, u32 count) { struct stm32_mdma_desc *desc; + int i; - desc = kzalloc(sizeof(*desc), GFP_NOWAIT); + desc = kzalloc(sizeof(*desc) + + sizeof(struct stm32_mdma_desc_node) * count, GFP_NOWAIT); if (!desc) return NULL; - desc->hwdesc = dma_pool_alloc(chan->desc_pool, GFP_NOWAIT, - &desc->hwdesc_phys); - if (!desc->hwdesc) { - dev_err(chan2dev(chan), "Failed to allocate descriptor\n"); - kfree(desc); - return NULL; + for (i = 0; i < count; i++) { + desc->node[i].hwdesc = + dma_pool_alloc(chan->desc_pool, GFP_NOWAIT, + &desc->node[i].hwdesc_phys); + if (!desc->node[i].hwdesc) + goto err; } desc->count = count; return desc; + +err: + dev_err(chan2dev(chan), "Failed to allocate descriptor\n"); + while (--i >= 0) + dma_pool_free(chan->desc_pool, desc->node[i].hwdesc, + desc->node[i].hwdesc_phys); + kfree(desc); + return NULL; } static void stm32_mdma_desc_free(struct virt_dma_desc *vdesc) { struct stm32_mdma_desc *desc = to_stm32_mdma_desc(vdesc); struct stm32_mdma_chan *chan = to_stm32_mdma_chan(vdesc->tx.chan); + int i; - dma_pool_free(chan->desc_pool, desc->hwdesc, desc->hwdesc_phys); + for (i = 0; i < desc->count; i++) + dma_pool_free(chan->desc_pool, desc->node[i].hwdesc, + desc->node[i].hwdesc_phys); kfree(desc); } @@ -669,18 +686,18 @@ static int stm32_mdma_set_xfer_param(struct stm32_mdma_chan *chan, } static void stm32_mdma_dump_hwdesc(struct stm32_mdma_chan *chan, - struct stm32_mdma_hwdesc *hwdesc) + struct stm32_mdma_desc_node *node) { - dev_dbg(chan2dev(chan), "hwdesc: 0x%p\n", hwdesc); - dev_dbg(chan2dev(chan), "CTCR: 0x%08x\n", hwdesc->ctcr); - dev_dbg(chan2dev(chan), "CBNDTR: 0x%08x\n", hwdesc->cbndtr); - dev_dbg(chan2dev(chan), "CSAR: 0x%08x\n", hwdesc->csar); - dev_dbg(chan2dev(chan), "CDAR: 0x%08x\n", hwdesc->cdar); - dev_dbg(chan2dev(chan), "CBRUR: 0x%08x\n", hwdesc->cbrur); - dev_dbg(chan2dev(chan), "CLAR: 0x%08x\n", hwdesc->clar); - dev_dbg(chan2dev(chan), "CTBR: 0x%08x\n", hwdesc->ctbr); - dev_dbg(chan2dev(chan), "CMAR: 0x%08x\n", hwdesc->cmar); - dev_dbg(chan2dev(chan), "CMDR: 0x%08x\n\n", hwdesc->cmdr); + dev_dbg(chan2dev(chan), "hwdesc: %pad\n", &node->hwdesc_phys); + dev_dbg(chan2dev(chan), "CTCR: 0x%08x\n", node->hwdesc->ctcr); + dev_dbg(chan2dev(chan), "CBNDTR: 0x%08x\n", node->hwdesc->cbndtr); + dev_dbg(chan2dev(chan), "CSAR: 0x%08x\n", node->hwdesc->csar); + dev_dbg(chan2dev(chan), "CDAR: 0x%08x\n", node->hwdesc->cdar); + dev_dbg(chan2dev(chan), "CBRUR: 0x%08x\n", node->hwdesc->cbrur); + dev_dbg(chan2dev(chan), "CLAR: 0x%08x\n", node->hwdesc->clar); + dev_dbg(chan2dev(chan), "CTBR: 0x%08x\n", node->hwdesc->ctbr); + dev_dbg(chan2dev(chan), "CMAR: 0x%08x\n", node->hwdesc->cmar); + dev_dbg(chan2dev(chan), "CMDR: 0x%08x\n\n", node->hwdesc->cmdr); } static void stm32_mdma_setup_hwdesc(struct stm32_mdma_chan *chan, @@ -694,7 +711,7 @@ static void stm32_mdma_setup_hwdesc(struct stm32_mdma_chan *chan, struct stm32_mdma_hwdesc *hwdesc; u32 next = count + 1; - hwdesc = &desc->hwdesc[count]; + hwdesc = desc->node[count].hwdesc; hwdesc->ctcr = ctcr; hwdesc->cbndtr &= ~(STM32_MDMA_CBNDTR_BRC_MK | STM32_MDMA_CBNDTR_BRDUM | @@ -704,19 +721,20 @@ static void stm32_mdma_setup_hwdesc(struct stm32_mdma_chan *chan, hwdesc->csar = src_addr; hwdesc->cdar = dst_addr; hwdesc->cbrur = 0; - hwdesc->clar = desc->hwdesc_phys + next * sizeof(*hwdesc); hwdesc->ctbr = ctbr; hwdesc->cmar = config->mask_addr; hwdesc->cmdr = config->mask_data; if (is_last) { if (is_cyclic) - hwdesc->clar = desc->hwdesc_phys; + hwdesc->clar = desc->node[0].hwdesc_phys; else hwdesc->clar = 0; + } else { + hwdesc->clar = desc->node[next].hwdesc_phys; } - stm32_mdma_dump_hwdesc(chan, hwdesc); + stm32_mdma_dump_hwdesc(chan, &desc->node[count]); } static int stm32_mdma_setup_xfer(struct stm32_mdma_chan *chan, @@ -780,7 +798,7 @@ stm32_mdma_prep_slave_sg(struct dma_chan *c, struct scatterlist *sgl, { struct stm32_mdma_chan *chan = to_stm32_mdma_chan(c); struct stm32_mdma_desc *desc; - int ret; + int i, ret; /* * Once DMA is in setup cyclic mode the channel we cannot assign this @@ -806,7 +824,9 @@ stm32_mdma_prep_slave_sg(struct dma_chan *c, struct scatterlist *sgl, return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags); xfer_setup_err: - dma_pool_free(chan->desc_pool, &desc->hwdesc, desc->hwdesc_phys); + for (i = 0; i < desc->count; i++) + dma_pool_free(chan->desc_pool, desc->node[i].hwdesc, + desc->node[i].hwdesc_phys); kfree(desc); return NULL; } @@ -895,7 +915,9 @@ stm32_mdma_prep_dma_cyclic(struct dma_chan *c, dma_addr_t buf_addr, return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags); xfer_setup_err: - dma_pool_free(chan->desc_pool, &desc->hwdesc, desc->hwdesc_phys); + for (i = 0; i < desc->count; i++) + dma_pool_free(chan->desc_pool, desc->node[i].hwdesc, + desc->node[i].hwdesc_phys); kfree(desc); return NULL; } @@ -1009,7 +1031,7 @@ stm32_mdma_prep_dma_memcpy(struct dma_chan *c, dma_addr_t dest, dma_addr_t src, ctcr |= STM32_MDMA_CTCR_PKE; /* Prepare hardware descriptor */ - hwdesc = desc->hwdesc; + hwdesc = desc->node[0].hwdesc; hwdesc->ctcr = ctcr; hwdesc->cbndtr = cbndtr; hwdesc->csar = src; @@ -1020,7 +1042,7 @@ stm32_mdma_prep_dma_memcpy(struct dma_chan *c, dma_addr_t dest, dma_addr_t src, hwdesc->cmar = 0; hwdesc->cmdr = 0; - stm32_mdma_dump_hwdesc(chan, hwdesc); + stm32_mdma_dump_hwdesc(chan, &desc->node[0]); } else { /* Setup a LLI transfer */ ctcr |= STM32_MDMA_CTCR_TRGM(STM32_MDMA_LINKED_LIST) | @@ -1120,7 +1142,7 @@ static void stm32_mdma_start_transfer(struct stm32_mdma_chan *chan) } chan->desc = to_stm32_mdma_desc(vdesc); - hwdesc = chan->desc->hwdesc; + hwdesc = chan->desc->node[0].hwdesc; chan->curr_hwdesc = 0; stm32_mdma_write(dmadev, STM32_MDMA_CCR(id), chan->desc->ccr); @@ -1198,7 +1220,7 @@ static int stm32_mdma_resume(struct dma_chan *c) unsigned long flags; u32 status, reg; - hwdesc = &chan->desc->hwdesc[chan->curr_hwdesc]; + hwdesc = chan->desc->node[chan->curr_hwdesc].hwdesc; spin_lock_irqsave(&chan->vchan.lock, flags); @@ -1268,13 +1290,13 @@ static size_t stm32_mdma_desc_residue(struct stm32_mdma_chan *chan, u32 curr_hwdesc) { struct stm32_mdma_device *dmadev = stm32_mdma_get_dev(chan); + struct stm32_mdma_hwdesc *hwdesc = desc->node[0].hwdesc; u32 cbndtr, residue, modulo, burst_size; int i; residue = 0; for (i = curr_hwdesc + 1; i < desc->count; i++) { - struct stm32_mdma_hwdesc *hwdesc = &desc->hwdesc[i]; - + hwdesc = desc->node[i].hwdesc; residue += STM32_MDMA_CBNDTR_BNDT(hwdesc->cbndtr); } cbndtr = stm32_mdma_read(dmadev, STM32_MDMA_CBNDTR(chan->id)); -- 2.7.4 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator 2018-04-11 14:44 ` [PATCH v2 2/2] dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator Pierre-Yves MORDRET @ 2018-04-13 4:02 ` Vinod Koul 2018-04-13 8:39 ` Geert Uytterhoeven 2018-04-13 11:48 ` Robin Murphy 1 sibling, 1 reply; 12+ messages in thread From: Vinod Koul @ 2018-04-13 4:02 UTC (permalink / raw) To: Pierre-Yves MORDRET Cc: Maxime Coquelin, Alexandre Torgue, Dan Williams, M'boumba Cedric Madianga, dmaengine, linux-arm-kernel, linux-kernel On Wed, Apr 11, 2018 at 04:44:39PM +0200, Pierre-Yves MORDRET wrote: > struct stm32_mdma_desc { > struct virt_dma_desc vdesc; > u32 ccr; > - struct stm32_mdma_hwdesc *hwdesc; > - dma_addr_t hwdesc_phys; > bool cyclic; > u32 count; > + struct stm32_mdma_desc_node node[]; some ppl use node[0] for this but i think either is fine.. > static void stm32_mdma_dump_hwdesc(struct stm32_mdma_chan *chan, > - struct stm32_mdma_hwdesc *hwdesc) > + struct stm32_mdma_desc_node *node) > { > - dev_dbg(chan2dev(chan), "CTCR: 0x%08x\n", hwdesc->ctcr); > + dev_dbg(chan2dev(chan), "CTCR: 0x%08x\n", node->hwdesc->ctcr); this is noise for this patch and IIUC you should be able to pass node->hwdesc and keep fn same? -- ~Vinod ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator 2018-04-13 4:02 ` Vinod Koul @ 2018-04-13 8:39 ` Geert Uytterhoeven 2018-04-13 10:09 ` Vinod Koul 0 siblings, 1 reply; 12+ messages in thread From: Geert Uytterhoeven @ 2018-04-13 8:39 UTC (permalink / raw) To: Vinod Koul Cc: Pierre-Yves MORDRET, Alexandre Torgue, Linux Kernel Mailing List, dmaengine, Maxime Coquelin, M'boumba Cedric Madianga, Dan Williams, Linux ARM Hi Vinod, On Fri, Apr 13, 2018 at 6:02 AM, Vinod Koul <vinod.koul@intel.com> wrote: > On Wed, Apr 11, 2018 at 04:44:39PM +0200, Pierre-Yves MORDRET wrote: > >> struct stm32_mdma_desc { >> struct virt_dma_desc vdesc; >> u32 ccr; >> - struct stm32_mdma_hwdesc *hwdesc; >> - dma_addr_t hwdesc_phys; >> bool cyclic; >> u32 count; >> + struct stm32_mdma_desc_node node[]; > > some ppl use node[0] for this but i think either is fine.. node[] is the correct one, node[0] may hide future bugs, cfr. commit a158531f3c92467d ("gpio: 74x164: Fix crash during .remove()") Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator 2018-04-13 8:39 ` Geert Uytterhoeven @ 2018-04-13 10:09 ` Vinod Koul 2018-04-13 10:40 ` Geert Uytterhoeven 0 siblings, 1 reply; 12+ messages in thread From: Vinod Koul @ 2018-04-13 10:09 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Pierre-Yves MORDRET, Alexandre Torgue, Linux Kernel Mailing List, dmaengine, Maxime Coquelin, M'boumba Cedric Madianga, Dan Williams, Linux ARM On Fri, Apr 13, 2018 at 10:39:48AM +0200, Geert Uytterhoeven wrote: > Hi Vinod, > > On Fri, Apr 13, 2018 at 6:02 AM, Vinod Koul <vinod.koul@intel.com> wrote: > > On Wed, Apr 11, 2018 at 04:44:39PM +0200, Pierre-Yves MORDRET wrote: > > > >> struct stm32_mdma_desc { > >> struct virt_dma_desc vdesc; > >> u32 ccr; > >> - struct stm32_mdma_hwdesc *hwdesc; > >> - dma_addr_t hwdesc_phys; > >> bool cyclic; > >> u32 count; > >> + struct stm32_mdma_desc_node node[]; > > > > some ppl use node[0] for this but i think either is fine.. > > node[] is the correct one, node[0] may hide future bugs, cfr. commit > a158531f3c92467d ("gpio: 74x164: Fix crash during .remove()") Yeah but it this case it is the last element. But yes it helps to avoid such mistakes in future.. -- ~Vinod ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator 2018-04-13 10:09 ` Vinod Koul @ 2018-04-13 10:40 ` Geert Uytterhoeven 0 siblings, 0 replies; 12+ messages in thread From: Geert Uytterhoeven @ 2018-04-13 10:40 UTC (permalink / raw) To: Vinod Koul Cc: Pierre-Yves MORDRET, Alexandre Torgue, Linux Kernel Mailing List, dmaengine, Maxime Coquelin, M'boumba Cedric Madianga, Dan Williams, Linux ARM Hi Vinod, On Fri, Apr 13, 2018 at 12:09 PM, Vinod Koul <vinod.koul@intel.com> wrote: > On Fri, Apr 13, 2018 at 10:39:48AM +0200, Geert Uytterhoeven wrote: >> On Fri, Apr 13, 2018 at 6:02 AM, Vinod Koul <vinod.koul@intel.com> wrote: >> > On Wed, Apr 11, 2018 at 04:44:39PM +0200, Pierre-Yves MORDRET wrote: >> > >> >> struct stm32_mdma_desc { >> >> struct virt_dma_desc vdesc; >> >> u32 ccr; >> >> - struct stm32_mdma_hwdesc *hwdesc; >> >> - dma_addr_t hwdesc_phys; >> >> bool cyclic; >> >> u32 count; >> >> + struct stm32_mdma_desc_node node[]; >> > >> > some ppl use node[0] for this but i think either is fine.. >> >> node[] is the correct one, node[0] may hide future bugs, cfr. commit >> a158531f3c92467d ("gpio: 74x164: Fix crash during .remove()") > > Yeah but it this case it is the last element. But yes it helps to avoid such > mistakes in future.. It was the last element in 74x164, too. Unless someone changed that. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 2/2] dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator 2018-04-11 14:44 ` [PATCH v2 2/2] dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator Pierre-Yves MORDRET 2018-04-13 4:02 ` Vinod Koul @ 2018-04-13 11:48 ` Robin Murphy 1 sibling, 0 replies; 12+ messages in thread From: Robin Murphy @ 2018-04-13 11:48 UTC (permalink / raw) To: Pierre-Yves MORDRET, Vinod Koul, Maxime Coquelin, Alexandre Torgue, Dan Williams, M'boumba Cedric Madianga, dmaengine, linux-arm-kernel, linux-kernel On 11/04/18 15:44, Pierre-Yves MORDRET wrote: > Only 1 Hw Descriptor is allocated. Loop over required Hw descriptor for > proper allocation. > > Signed-off-by: Pierre-Yves MORDRET <pierre-yves.mordret@st.com> > --- > Version history: > v1: > * Initial > v2: > * Fix kbuild warning format: /0x%08x/%pad/ > --- > --- > drivers/dma/stm32-mdma.c | 90 ++++++++++++++++++++++++++++++------------------ > 1 file changed, 56 insertions(+), 34 deletions(-) > > diff --git a/drivers/dma/stm32-mdma.c b/drivers/dma/stm32-mdma.c > index fbcffa2..3a477bc 100644 > --- a/drivers/dma/stm32-mdma.c > +++ b/drivers/dma/stm32-mdma.c > @@ -252,13 +252,17 @@ struct stm32_mdma_hwdesc { > u32 cmdr; > } __aligned(64); > > +struct stm32_mdma_desc_node { > + struct stm32_mdma_hwdesc *hwdesc; > + dma_addr_t hwdesc_phys; > +}; > + > struct stm32_mdma_desc { > struct virt_dma_desc vdesc; > u32 ccr; > - struct stm32_mdma_hwdesc *hwdesc; > - dma_addr_t hwdesc_phys; > bool cyclic; > u32 count; > + struct stm32_mdma_desc_node node[]; > }; > > struct stm32_mdma_chan { > @@ -344,30 +348,43 @@ static struct stm32_mdma_desc *stm32_mdma_alloc_desc( > struct stm32_mdma_chan *chan, u32 count) > { > struct stm32_mdma_desc *desc; > + int i; > > - desc = kzalloc(sizeof(*desc), GFP_NOWAIT); > + desc = kzalloc(sizeof(*desc) + > + sizeof(struct stm32_mdma_desc_node) * count, GFP_NOWAIT); You could use "offsetof(typeof(*desc), node[count])" instead of the explicit calculation here. Robin. > if (!desc) > return NULL; > > - desc->hwdesc = dma_pool_alloc(chan->desc_pool, GFP_NOWAIT, > - &desc->hwdesc_phys); > - if (!desc->hwdesc) { > - dev_err(chan2dev(chan), "Failed to allocate descriptor\n"); > - kfree(desc); > - return NULL; > + for (i = 0; i < count; i++) { > + desc->node[i].hwdesc = > + dma_pool_alloc(chan->desc_pool, GFP_NOWAIT, > + &desc->node[i].hwdesc_phys); > + if (!desc->node[i].hwdesc) > + goto err; > } > > desc->count = count; > > return desc; > + > +err: > + dev_err(chan2dev(chan), "Failed to allocate descriptor\n"); > + while (--i >= 0) > + dma_pool_free(chan->desc_pool, desc->node[i].hwdesc, > + desc->node[i].hwdesc_phys); > + kfree(desc); > + return NULL; > } > > static void stm32_mdma_desc_free(struct virt_dma_desc *vdesc) > { > struct stm32_mdma_desc *desc = to_stm32_mdma_desc(vdesc); > struct stm32_mdma_chan *chan = to_stm32_mdma_chan(vdesc->tx.chan); > + int i; > > - dma_pool_free(chan->desc_pool, desc->hwdesc, desc->hwdesc_phys); > + for (i = 0; i < desc->count; i++) > + dma_pool_free(chan->desc_pool, desc->node[i].hwdesc, > + desc->node[i].hwdesc_phys); > kfree(desc); > } > > @@ -669,18 +686,18 @@ static int stm32_mdma_set_xfer_param(struct stm32_mdma_chan *chan, > } > > static void stm32_mdma_dump_hwdesc(struct stm32_mdma_chan *chan, > - struct stm32_mdma_hwdesc *hwdesc) > + struct stm32_mdma_desc_node *node) > { > - dev_dbg(chan2dev(chan), "hwdesc: 0x%p\n", hwdesc); > - dev_dbg(chan2dev(chan), "CTCR: 0x%08x\n", hwdesc->ctcr); > - dev_dbg(chan2dev(chan), "CBNDTR: 0x%08x\n", hwdesc->cbndtr); > - dev_dbg(chan2dev(chan), "CSAR: 0x%08x\n", hwdesc->csar); > - dev_dbg(chan2dev(chan), "CDAR: 0x%08x\n", hwdesc->cdar); > - dev_dbg(chan2dev(chan), "CBRUR: 0x%08x\n", hwdesc->cbrur); > - dev_dbg(chan2dev(chan), "CLAR: 0x%08x\n", hwdesc->clar); > - dev_dbg(chan2dev(chan), "CTBR: 0x%08x\n", hwdesc->ctbr); > - dev_dbg(chan2dev(chan), "CMAR: 0x%08x\n", hwdesc->cmar); > - dev_dbg(chan2dev(chan), "CMDR: 0x%08x\n\n", hwdesc->cmdr); > + dev_dbg(chan2dev(chan), "hwdesc: %pad\n", &node->hwdesc_phys); > + dev_dbg(chan2dev(chan), "CTCR: 0x%08x\n", node->hwdesc->ctcr); > + dev_dbg(chan2dev(chan), "CBNDTR: 0x%08x\n", node->hwdesc->cbndtr); > + dev_dbg(chan2dev(chan), "CSAR: 0x%08x\n", node->hwdesc->csar); > + dev_dbg(chan2dev(chan), "CDAR: 0x%08x\n", node->hwdesc->cdar); > + dev_dbg(chan2dev(chan), "CBRUR: 0x%08x\n", node->hwdesc->cbrur); > + dev_dbg(chan2dev(chan), "CLAR: 0x%08x\n", node->hwdesc->clar); > + dev_dbg(chan2dev(chan), "CTBR: 0x%08x\n", node->hwdesc->ctbr); > + dev_dbg(chan2dev(chan), "CMAR: 0x%08x\n", node->hwdesc->cmar); > + dev_dbg(chan2dev(chan), "CMDR: 0x%08x\n\n", node->hwdesc->cmdr); > } > > static void stm32_mdma_setup_hwdesc(struct stm32_mdma_chan *chan, > @@ -694,7 +711,7 @@ static void stm32_mdma_setup_hwdesc(struct stm32_mdma_chan *chan, > struct stm32_mdma_hwdesc *hwdesc; > u32 next = count + 1; > > - hwdesc = &desc->hwdesc[count]; > + hwdesc = desc->node[count].hwdesc; > hwdesc->ctcr = ctcr; > hwdesc->cbndtr &= ~(STM32_MDMA_CBNDTR_BRC_MK | > STM32_MDMA_CBNDTR_BRDUM | > @@ -704,19 +721,20 @@ static void stm32_mdma_setup_hwdesc(struct stm32_mdma_chan *chan, > hwdesc->csar = src_addr; > hwdesc->cdar = dst_addr; > hwdesc->cbrur = 0; > - hwdesc->clar = desc->hwdesc_phys + next * sizeof(*hwdesc); > hwdesc->ctbr = ctbr; > hwdesc->cmar = config->mask_addr; > hwdesc->cmdr = config->mask_data; > > if (is_last) { > if (is_cyclic) > - hwdesc->clar = desc->hwdesc_phys; > + hwdesc->clar = desc->node[0].hwdesc_phys; > else > hwdesc->clar = 0; > + } else { > + hwdesc->clar = desc->node[next].hwdesc_phys; > } > > - stm32_mdma_dump_hwdesc(chan, hwdesc); > + stm32_mdma_dump_hwdesc(chan, &desc->node[count]); > } > > static int stm32_mdma_setup_xfer(struct stm32_mdma_chan *chan, > @@ -780,7 +798,7 @@ stm32_mdma_prep_slave_sg(struct dma_chan *c, struct scatterlist *sgl, > { > struct stm32_mdma_chan *chan = to_stm32_mdma_chan(c); > struct stm32_mdma_desc *desc; > - int ret; > + int i, ret; > > /* > * Once DMA is in setup cyclic mode the channel we cannot assign this > @@ -806,7 +824,9 @@ stm32_mdma_prep_slave_sg(struct dma_chan *c, struct scatterlist *sgl, > return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags); > > xfer_setup_err: > - dma_pool_free(chan->desc_pool, &desc->hwdesc, desc->hwdesc_phys); > + for (i = 0; i < desc->count; i++) > + dma_pool_free(chan->desc_pool, desc->node[i].hwdesc, > + desc->node[i].hwdesc_phys); > kfree(desc); > return NULL; > } > @@ -895,7 +915,9 @@ stm32_mdma_prep_dma_cyclic(struct dma_chan *c, dma_addr_t buf_addr, > return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags); > > xfer_setup_err: > - dma_pool_free(chan->desc_pool, &desc->hwdesc, desc->hwdesc_phys); > + for (i = 0; i < desc->count; i++) > + dma_pool_free(chan->desc_pool, desc->node[i].hwdesc, > + desc->node[i].hwdesc_phys); > kfree(desc); > return NULL; > } > @@ -1009,7 +1031,7 @@ stm32_mdma_prep_dma_memcpy(struct dma_chan *c, dma_addr_t dest, dma_addr_t src, > ctcr |= STM32_MDMA_CTCR_PKE; > > /* Prepare hardware descriptor */ > - hwdesc = desc->hwdesc; > + hwdesc = desc->node[0].hwdesc; > hwdesc->ctcr = ctcr; > hwdesc->cbndtr = cbndtr; > hwdesc->csar = src; > @@ -1020,7 +1042,7 @@ stm32_mdma_prep_dma_memcpy(struct dma_chan *c, dma_addr_t dest, dma_addr_t src, > hwdesc->cmar = 0; > hwdesc->cmdr = 0; > > - stm32_mdma_dump_hwdesc(chan, hwdesc); > + stm32_mdma_dump_hwdesc(chan, &desc->node[0]); > } else { > /* Setup a LLI transfer */ > ctcr |= STM32_MDMA_CTCR_TRGM(STM32_MDMA_LINKED_LIST) | > @@ -1120,7 +1142,7 @@ static void stm32_mdma_start_transfer(struct stm32_mdma_chan *chan) > } > > chan->desc = to_stm32_mdma_desc(vdesc); > - hwdesc = chan->desc->hwdesc; > + hwdesc = chan->desc->node[0].hwdesc; > chan->curr_hwdesc = 0; > > stm32_mdma_write(dmadev, STM32_MDMA_CCR(id), chan->desc->ccr); > @@ -1198,7 +1220,7 @@ static int stm32_mdma_resume(struct dma_chan *c) > unsigned long flags; > u32 status, reg; > > - hwdesc = &chan->desc->hwdesc[chan->curr_hwdesc]; > + hwdesc = chan->desc->node[chan->curr_hwdesc].hwdesc; > > spin_lock_irqsave(&chan->vchan.lock, flags); > > @@ -1268,13 +1290,13 @@ static size_t stm32_mdma_desc_residue(struct stm32_mdma_chan *chan, > u32 curr_hwdesc) > { > struct stm32_mdma_device *dmadev = stm32_mdma_get_dev(chan); > + struct stm32_mdma_hwdesc *hwdesc = desc->node[0].hwdesc; > u32 cbndtr, residue, modulo, burst_size; > int i; > > residue = 0; > for (i = curr_hwdesc + 1; i < desc->count; i++) { > - struct stm32_mdma_hwdesc *hwdesc = &desc->hwdesc[i]; > - > + hwdesc = desc->node[i].hwdesc; > residue += STM32_MDMA_CBNDTR_BNDT(hwdesc->cbndtr); > } > cbndtr = stm32_mdma_read(dmadev, STM32_MDMA_CBNDTR(chan->id)); > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-04-13 12:31 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-11 14:44 [PATCH v2 0/2] Append some fixes and improvements Pierre-Yves MORDRET 2018-04-11 14:44 ` [PATCH v2 1/2] dmaengine: stm32-mdma: align TLEN and buffer length on burst Pierre-Yves MORDRET 2018-04-11 15:14 ` Robin Murphy 2018-04-13 9:45 ` Pierre Yves MORDRET 2018-04-13 11:09 ` Robin Murphy 2018-04-13 12:30 ` Pierre Yves MORDRET 2018-04-11 14:44 ` [PATCH v2 2/2] dmaengine: stm32-mdma: Fix incomplete Hw descriptors allocator Pierre-Yves MORDRET 2018-04-13 4:02 ` Vinod Koul 2018-04-13 8:39 ` Geert Uytterhoeven 2018-04-13 10:09 ` Vinod Koul 2018-04-13 10:40 ` Geert Uytterhoeven 2018-04-13 11:48 ` Robin Murphy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).