[1/2] dma: axi-dmac: don't check the number of frames for alignment

[1/2] dma: axi-dmac: don't check the number of frames for alignment

[Ardelean, Alexandru]

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

[1/2] dma: axi-dmac: don't check the number of frames for alignment

[Ardelean, Alexandru]

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

[1/2] dma: axi-dmac: don't check the number of frames for alignment

[Vinod Koul]

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?

[1/2] dma: axi-dmac: don't check the number of frames for alignment

[Vinod Koul]

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

[1/2] dma: axi-dmac: don't check the number of frames for alignment

[Alexandru Ardelean]

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(-)

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)