| Message ID | 20190215091750.28035-1-alexandru.ardelean@analog.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] dma: axi-dmac: don't check the number of frames for alignment | expand |
On 15-02-19, 11:17, Alexandru Ardelean wrote: > Fixes commit 0e3b67b348b8 ("dmaengine: Add support for the Analog Devices > AXI-DMAC DMA controller") Do you mean to add a Fixes tag? > For 2D transfers, there is no requirement for Y_LENGTH to be aligned > to the bus-width (or anything). X_LENGTH is required to be aligned > though. > > So, we shouldn't check that the number of frames is aligned. Does this fix a bug as indicated by Fixes tag? Lastly, it is dmaengine: xxx not dma: xxx Please fix that. > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > --- > drivers/dma/dma-axi-dmac.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c > index ffc0adc2f6ce..2c999113b989 100644 > --- a/drivers/dma/dma-axi-dmac.c > +++ b/drivers/dma/dma-axi-dmac.c > @@ -485,7 +485,7 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_interleaved( > > if (chan->hw_2d) { > if (!axi_dmac_check_len(chan, xt->sgl[0].size) || > - !axi_dmac_check_len(chan, xt->numf)) > + xt->numf == 0) > return NULL; > if (xt->sgl[0].size + dst_icg > chan->max_length || > xt->sgl[0].size + src_icg > chan->max_length) > -- > 2.17.1
On Mon, 2019-02-25 at 12:22 +0530, Vinod Koul wrote: > [External] > > > On 15-02-19, 11:17, Alexandru Ardelean wrote: > > Fixes commit 0e3b67b348b8 ("dmaengine: Add support for the Analog > > Devices > > AXI-DMAC DMA controller") > > Do you mean to add a Fixes tag? Yep. I'm terrible at this apparently. I'll read about how to do that properly. Other maintainers have complained about this as well [i.e. the fact that I didn't add proper Fixes tags ]. > > > For 2D transfers, there is no requirement for Y_LENGTH to be aligned > > to the bus-width (or anything). X_LENGTH is required to be aligned > > though. > > > > So, we shouldn't check that the number of frames is aligned. > > Does this fix a bug as indicated by Fixes tag? Yes > > Lastly, it is dmaengine: xxx not dma: xxx Please fix that. Ack Will fix > > > > > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> > > --- > > drivers/dma/dma-axi-dmac.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c > > index ffc0adc2f6ce..2c999113b989 100644 > > --- a/drivers/dma/dma-axi-dmac.c > > +++ b/drivers/dma/dma-axi-dmac.c > > @@ -485,7 +485,7 @@ static struct dma_async_tx_descriptor > > *axi_dmac_prep_interleaved( > > > > if (chan->hw_2d) { > > if (!axi_dmac_check_len(chan, xt->sgl[0].size) || > > - !axi_dmac_check_len(chan, xt->numf)) > > + xt->numf == 0) > > return NULL; > > if (xt->sgl[0].size + dst_icg > chan->max_length || > > xt->sgl[0].size + src_icg > chan->max_length) > > -- > > 2.17.1 > > -- > ~Vinod
On 26-02-19, 07:14, Ardelean, Alexandru wrote: > On Mon, 2019-02-25 at 12:22 +0530, Vinod Koul wrote: > > [External] > > > > > > On 15-02-19, 11:17, Alexandru Ardelean wrote: > > > Fixes commit 0e3b67b348b8 ("dmaengine: Add support for the Analog > > > Devices > > > AXI-DMAC DMA controller") > > > > Do you mean to add a Fixes tag? > > Yep. > I'm terrible at this apparently. I'll read about how to do that properly. > Other maintainers have complained about this as well [i.e. the fact that I > didn't add proper Fixes tags ]. So if it fixes a bug in original commit and should go immediately to next rc-X then fixes is mostly appropriate.. (also backporting to stable). In this case it doesnt seem so. Also canonical form is Fixes: abcdef : ("buggy patch") and this line should be before the s-o-b line.. > > > > > For 2D transfers, there is no requirement for Y_LENGTH to be aligned > > > to the bus-width (or anything). X_LENGTH is required to be aligned > > > though. > > > > > > So, we shouldn't check that the number of frames is aligned. > > > > Does this fix a bug as indicated by Fixes tag? > > Yes well if it is aligned it shouldn't cause break right. Yes not having aligned helps the driver but seems to be correct the wrong interpretation/implementation.. right?
On Tue, 2019-02-26 at 13:31 +0530, Vinod Koul wrote: > [External] > > > On 26-02-19, 07:14, Ardelean, Alexandru wrote: > > On Mon, 2019-02-25 at 12:22 +0530, Vinod Koul wrote: > > > [External] > > > > > > > > > On 15-02-19, 11:17, Alexandru Ardelean wrote: > > > > Fixes commit 0e3b67b348b8 ("dmaengine: Add support for the Analog > > > > Devices > > > > AXI-DMAC DMA controller") > > > > > > Do you mean to add a Fixes tag? > > > > Yep. > > I'm terrible at this apparently. I'll read about how to do that > > properly. > > Other maintainers have complained about this as well [i.e. the fact > > that I > > didn't add proper Fixes tags ]. > > So if it fixes a bug in original commit and should go immediately to > next rc-X then fixes is mostly appropriate.. (also backporting to > stable). In this case it doesnt seem so. > > Also canonical form is > > Fixes: abcdef : ("buggy patch") > > and this line should be before the s-o-b line.. I just found this in the submitting-patch-docs. > > > > > > > > For 2D transfers, there is no requirement for Y_LENGTH to be > > > > aligned > > > > to the bus-width (or anything). X_LENGTH is required to be aligned > > > > though. > > > > > > > > So, we shouldn't check that the number of frames is aligned. > > > > > > Does this fix a bug as indicated by Fixes tag? > > > > Yes > > well if it is aligned it shouldn't cause break right. Yes not having > aligned helps the driver but seems to be correct the wrong > interpretation/implementation.. right? I'm preparing a V2 here. 2D transfers are typically used when DMA talks to video, where Y_LENGTH is the number of rows, which don't need to be aligned. I'll try to make the comment more helpful. > > -- > ~Vinod
diff --git a/drivers/dma/dma-axi-dmac.c b/drivers/dma/dma-axi-dmac.c index ffc0adc2f6ce..2c999113b989 100644 --- a/drivers/dma/dma-axi-dmac.c +++ b/drivers/dma/dma-axi-dmac.c @@ -485,7 +485,7 @@ static struct dma_async_tx_descriptor *axi_dmac_prep_interleaved( if (chan->hw_2d) { if (!axi_dmac_check_len(chan, xt->sgl[0].size) || - !axi_dmac_check_len(chan, xt->numf)) + xt->numf == 0) return NULL; if (xt->sgl[0].size + dst_icg > chan->max_length || xt->sgl[0].size + src_icg > chan->max_length)
Fixes commit 0e3b67b348b8 ("dmaengine: Add support for the Analog Devices AXI-DMAC DMA controller") For 2D transfers, there is no requirement for Y_LENGTH to be aligned to the bus-width (or anything). X_LENGTH is required to be aligned though. So, we shouldn't check that the number of frames is aligned. Signed-off-by: Alexandru Ardelean <alexandru.ardelean@analog.com> --- drivers/dma/dma-axi-dmac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)