| Message ID | 20190215110627.32038-1-alexandru.ardelean@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | dma: axi-dmac: Split too large segments | expand |
Hi Lars-Peter,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc4 next-20190215]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Alexandru-Ardelean/dma-axi-dmac-Split-too-large-segments/20190216-160002
config: xtensa-allyesconfig (attached as .config)
compiler: xtensa-linux-gcc (GCC) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=xtensa
All errors (new ones prefixed by >>):
drivers/dma/dma-axi-dmac.c: In function 'axi_dmac_prep_slave_sg':
>> drivers/dma/dma-axi-dmac.c:443:12: error: implicit declaration of function 'sg_nents_for_dma'; did you mean 'sg_nents_for_len'? [-Werror=implicit-function-declaration]
num_sgs = sg_nents_for_dma(sgl, sg_len, chan->max_length);
^~~~~~~~~~~~~~~~
sg_nents_for_len
cc1: some warnings being treated as errors
vim +443 drivers/dma/dma-axi-dmac.c
427
428 static struct dma_async_tx_descriptor *axi_dmac_prep_slave_sg(
429 struct dma_chan *c, struct scatterlist *sgl,
430 unsigned int sg_len, enum dma_transfer_direction direction,
431 unsigned long flags, void *context)
432 {
433 struct axi_dmac_chan *chan = to_axi_dmac_chan(c);
434 struct axi_dmac_desc *desc;
435 struct axi_dmac_sg *dsg;
436 struct scatterlist *sg;
437 unsigned int num_sgs;
438 unsigned int i;
439
440 if (direction != chan->direction)
441 return NULL;
442
> 443 num_sgs = sg_nents_for_dma(sgl, sg_len, chan->max_length);
444 desc = axi_dmac_alloc_desc(num_sgs);
445 if (!desc)
446 return NULL;
447
448 dsg = desc->sg;
449
450 for_each_sg(sgl, sg, sg_len, i) {
451 if (!axi_dmac_check_addr(chan, sg_dma_address(sg)) ||
452 !axi_dmac_check_len(chan, sg_dma_len(sg))) {
453 kfree(desc);
454 return NULL;
455 }
456
457 dsg = axi_dmac_fill_linear_sg(chan, direction, sg_dma_address(sg), 1,
458 sg_dma_len(sg), dsg);
459 }
460
461 desc->cyclic = false;
462
463 return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags);
464 }
465
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Sat, 2019-02-16 at 17:03 +0800, kbuild test robot wrote: > My bad here. I took this patch from our kernel tree and sent it. I assumed it works, since it works in our tree. I'll take a look and see about the order of patches, and which one(s) need(s) to be sent before this one Thanks Alex > > Hi Lars-Peter, > > I love your patch! Yet something to improve: > > [auto build test ERROR on linus/master] > [also build test ERROR on v5.0-rc4 next-20190215] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Alexandru-Ardelean/dma-axi-dmac-Split-too-large-segments/20190216-160002 > config: nds32-allyesconfig (attached as .config) > compiler: nds32le-linux-gcc (GCC) 6.4.0 > reproduce: > wget > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross > -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > GCC_VERSION=6.4.0 make.cross ARCH=nds32 > > All errors (new ones prefixed by >>): > > drivers//dma/dma-axi-dmac.c: In function 'axi_dmac_prep_slave_sg': > > > drivers//dma/dma-axi-dmac.c:443:12: error: implicit declaration of > > > function 'sg_nents_for_dma' [-Werror=implicit-function-declaration] > > num_sgs = sg_nents_for_dma(sgl, sg_len, chan->max_length); > ^~~~~~~~~~~~~~~~ > cc1: some warnings being treated as errors > > vim +/sg_nents_for_dma +443 drivers//dma/dma-axi-dmac.c > > 427 > 428 static struct dma_async_tx_descriptor *axi_dmac_prep_slave_sg( > 429 struct dma_chan *c, struct scatterlist *sgl, > 430 unsigned int sg_len, enum dma_transfer_direction > direction, > 431 unsigned long flags, void *context) > 432 { > 433 struct axi_dmac_chan *chan = to_axi_dmac_chan(c); > 434 struct axi_dmac_desc *desc; > 435 struct axi_dmac_sg *dsg; > 436 struct scatterlist *sg; > 437 unsigned int num_sgs; > 438 unsigned int i; > 439 > 440 if (direction != chan->direction) > 441 return NULL; > 442 > > 443 num_sgs = sg_nents_for_dma(sgl, sg_len, chan- > >max_length); > 444 desc = axi_dmac_alloc_desc(num_sgs); > 445 if (!desc) > 446 return NULL; > 447 > 448 dsg = desc->sg; > 449 > 450 for_each_sg(sgl, sg, sg_len, i) { > 451 if (!axi_dmac_check_addr(chan, > sg_dma_address(sg)) || > 452 !axi_dmac_check_len(chan, sg_dma_len(sg))) { > 453 kfree(desc); > 454 return NULL; > 455 } > 456 > 457 dsg = axi_dmac_fill_linear_sg(chan, direction, > sg_dma_address(sg), 1, > 458 sg_dma_len(sg), dsg); > 459 } > 460 > 461 desc->cyclic = false; > 462 > 463 return vchan_tx_prep(&chan->vchan, &desc->vdesc, flags); > 464 } > 465 > > --- > 0-DAY kernel test infrastructure Open Source Technology > Center > https://lists.01.org/pipermail/kbuild-all Intel > Corporation
On 2/18/19 8:28 AM, Ardelean, Alexandru wrote: > On Sat, 2019-02-16 at 17:03 +0800, kbuild test robot wrote: >> > > My bad here. > I took this patch from our kernel tree and sent it. > I assumed it works, since it works in our tree. > I'll take a look and see about the order of patches, and which one(s) > need(s) to be sent before this one https://github.com/analogdevicesinc/linux/commit/414a555896b2abca3518c3c878f21e7b1ea95904#diff-b5b2fd5430b4f9aff1ae97bae60dd5c5 That patch was sent upstream, not sure why it was never merged. But if necessary you can also re-write this patch to not rely on the other one.
On 18-02-19, 08:34, Lars-Peter Clausen wrote: > On 2/18/19 8:28 AM, Ardelean, Alexandru wrote: > > On Sat, 2019-02-16 at 17:03 +0800, kbuild test robot wrote: > >> > > > > My bad here. > > I took this patch from our kernel tree and sent it. > > I assumed it works, since it works in our tree. > > I'll take a look and see about the order of patches, and which one(s) > > need(s) to be sent before this one > > https://github.com/analogdevicesinc/linux/commit/414a555896b2abca3518c3c878f21e7b1ea95904#diff-b5b2fd5430b4f9aff1ae97bae60dd5c5 > > That patch was sent upstream, not sure why it was never merged. > > But if necessary you can also re-write this patch to not rely on the > other one. Or send this patch upstream if you find it very useful :)
diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c index ffc0adc2f6ce..6a41e1f49077 100644 --- a/drivers/dma/dma-axi-dmac.c +++ b/drivers/dma/dma-axi-dmac.c @@ -83,6 +83,7 @@ struct axi_dmac_sg { unsigned int dest_stride; unsigned int src_stride; unsigned int id; + bool last; bool schedule_when_free; }; @@ -166,7 +167,7 @@ static int axi_dmac_dest_is_mem(struct axi_dmac_chan *chan) static bool axi_dmac_check_len(struct axi_dmac_chan *chan, unsigned int len) { - if (len == 0 || len > chan->max_length) + if (len == 0) return false; if ((len & chan->align_mask) != 0) /* Not aligned */ return false; @@ -379,6 +380,50 @@ static struct axi_dmac_desc *axi_dmac_alloc_desc(unsigned int num_sgs) return desc; } +static struct axi_dmac_sg *axi_dmac_fill_linear_sg(struct axi_dmac_chan *chan, + enum dma_transfer_direction direction, dma_addr_t addr, + unsigned int num_periods, unsigned int period_len, + struct axi_dmac_sg *sg) +{ + unsigned int num_segments, i; + unsigned int segment_size; + unsigned int len; + + /* Split into multiple equally sized segments if necessary */ + num_segments = DIV_ROUND_UP(period_len, chan->max_length); + segment_size = DIV_ROUND_UP(period_len, num_segments); + /* Take care of alignment */ + segment_size = ((segment_size - 1) | chan->align_mask) + 1; + + for (i = 0; i < num_periods; i++) { + len = period_len; + + while (len > segment_size) { + if (direction == DMA_DEV_TO_MEM) + sg->dest_addr = addr; + else + sg->src_addr = addr; + sg->x_len = segment_size; + sg->y_len = 1; + sg++; + addr += segment_size; + len -= segment_size; + } + + if (direction == DMA_DEV_TO_MEM) + sg->dest_addr = addr; + else + sg->src_addr = addr; + sg->x_len = len; + sg->y_len = 1; + sg->last = true; + sg++; + addr += len; + } + + return sg; +} + static struct dma_async_tx_descriptor *axi_dmac_prep_slave_sg( struct dma_chan *c, struct scatterlist *sgl, unsigned int sg_len, enum dma_transfer_direction direction, @@ -386,16 +431,21 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_slave_sg( { struct axi_dmac_chan *chan = to_axi_dmac_chan(c); struct axi_dmac_desc *desc; + struct axi_dmac_sg *dsg; struct scatterlist *sg; + unsigned int num_sgs; unsigned int i; if (direction != chan->direction) return NULL; - desc = axi_dmac_alloc_desc(sg_len); + num_sgs = sg_nents_for_dma(sgl, sg_len, chan->max_length); + desc = axi_dmac_alloc_desc(num_sgs); if (!desc) return NULL; + dsg = desc->sg; + for_each_sg(sgl, sg, sg_len, i) { if (!axi_dmac_check_addr(chan, sg_dma_address(sg)) || !axi_dmac_check_len(chan, sg_dma_len(sg))) { @@ -403,12 +453,8 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_slave_sg( return NULL; } - if (direction == DMA_DEV_TO_MEM) - desc->sg[i].dest_addr = sg_dma_address(sg); - else - desc->sg[i].src_addr = sg_dma_address(sg); - desc->sg[i].x_len = sg_dma_len(sg); - desc->sg[i].y_len = 1; + dsg = axi_dmac_fill_linear_sg(chan, direction, sg_dma_address(sg), 1, + sg_dma_len(sg), dsg); } desc->cyclic = false; @@ -423,7 +469,7 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_dma_cyclic( { struct axi_dmac_chan *chan = to_axi_dmac_chan(c); struct axi_dmac_desc *desc; - unsigned int num_periods, i; + unsigned int num_periods, num_segments; if (direction != chan->direction) return NULL; @@ -436,20 +482,14 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_dma_cyclic( return NULL; num_periods = buf_len / period_len; + num_segments = DIV_ROUND_UP(period_len, chan->max_length); - desc = axi_dmac_alloc_desc(num_periods); + desc = axi_dmac_alloc_desc(num_periods * num_segments); if (!desc) return NULL; - for (i = 0; i < num_periods; i++) { - if (direction == DMA_DEV_TO_MEM) - desc->sg[i].dest_addr = buf_addr; - else - desc->sg[i].src_addr = buf_addr; - desc->sg[i].x_len = period_len; - desc->sg[i].y_len = 1; - buf_addr += period_len; - } + axi_dmac_fill_linear_sg(chan, direction, buf_addr, num_periods, + buf_len, desc->sg); desc->cyclic = true; @@ -647,7 +687,7 @@ static int axi_dmac_probe(struct platform_device *pdev) of_node_put(of_channels); pdev->dev.dma_parms = &dmac->dma_parms; - dma_set_max_seg_size(&pdev->dev, dmac->chan.max_length); + dma_set_max_seg_size(&pdev->dev, UINT_MAX); dma_dev = &dmac->dma_dev; dma_cap_set(DMA_SLAVE, dma_dev->cap_mask);