diff mbox

[RFC,2/2] DMA: imx-sdma: (cosmetic) simplify a loop

Message ID Pine.LNX.4.64.1401201327520.12653@axis700.grange (mailing list archive)
State Rejected
Delegated to: Vinod Koul
Headers show

Commit Message

Guennadi Liakhovetski Jan. 20, 2014, 12:33 p.m. UTC
Instead of initialising variables in their definitions and using a while
loop switch to using a for loop.

Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---

Just seems a bit cleaner to me, but that's subjective :)

 drivers/dma/imx-sdma.c |   13 +++++--------
 1 files changed, 5 insertions(+), 8 deletions(-)

Comments

Sascha Hauer Jan. 22, 2014, 6:55 a.m. UTC | #1
On Mon, Jan 20, 2014 at 01:33:54PM +0100, Guennadi Liakhovetski wrote:
> Instead of initialising variables in their definitions and using a while
> loop switch to using a for loop.
> 
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> 
> Just seems a bit cleaner to me, but that's subjective :)

Indeed, that's subjective ;)

I just find it harder to read.

Sascha

> 
>  drivers/dma/imx-sdma.c |   13 +++++--------
>  1 files changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index c75679d..875d08e 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1113,9 +1113,10 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
>  {
>  	struct sdma_channel *sdmac = to_sdma_chan(chan);
>  	struct sdma_engine *sdma = sdmac->sdma;
> +	struct sdma_buffer_descriptor *bd;
>  	int num_periods = buf_len / period_len;
>  	int channel = sdmac->channel;
> -	int ret, i = 0, buf = 0;
> +	int ret, i, buf;
>  
>  	dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
>  
> @@ -1144,8 +1145,9 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
>  		goto err_out;
>  	}
>  
> -	while (buf < buf_len) {
> -		struct sdma_buffer_descriptor *bd = &sdmac->bd[i];
> +	for (i = 0, buf = 0, bd = sdmac->bd;
> +	     buf < buf_len;
> +	     i++, bd++, buf += period_len, dma_addr += period_len) {
>  		int param;
>  
>  		bd->buffer_addr = dma_addr;
> @@ -1169,11 +1171,6 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
>  				param & BD_INTR ? " intr" : "");
>  
>  		bd->mode.status = param;
> -
> -		dma_addr += period_len;
> -		buf += period_len;
> -
> -		i++;
>  	}
>  
>  	sdmac->num_bd = num_periods;
> -- 
> 1.7.2.5
> 
>
Vinod Koul March 6, 2014, 9:15 a.m. UTC | #2
On Wed, Jan 22, 2014 at 07:55:25AM +0100, Sascha Hauer wrote:
> On Mon, Jan 20, 2014 at 01:33:54PM +0100, Guennadi Liakhovetski wrote:
> > Instead of initialising variables in their definitions and using a while
> > loop switch to using a for loop.
> > 
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> > ---
> > 
> > Just seems a bit cleaner to me, but that's subjective :)
> 
> Indeed, that's subjective ;)
> 
> I just find it harder to read.
> 
> Sascha
> 
> > 
> >  drivers/dma/imx-sdma.c |   13 +++++--------
> >  1 files changed, 5 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> > index c75679d..875d08e 100644
> > --- a/drivers/dma/imx-sdma.c
> > +++ b/drivers/dma/imx-sdma.c
> > @@ -1113,9 +1113,10 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
> >  {
> >  	struct sdma_channel *sdmac = to_sdma_chan(chan);
> >  	struct sdma_engine *sdma = sdmac->sdma;
> > +	struct sdma_buffer_descriptor *bd;
> >  	int num_periods = buf_len / period_len;
> >  	int channel = sdmac->channel;
> > -	int ret, i = 0, buf = 0;
> > +	int ret, i, buf;
> >  
> >  	dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
> >  
> > @@ -1144,8 +1145,9 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
> >  		goto err_out;
> >  	}
> >  
> > -	while (buf < buf_len) {
> > -		struct sdma_buffer_descriptor *bd = &sdmac->bd[i];
> > +	for (i = 0, buf = 0, bd = sdmac->bd;
> > +	     buf < buf_len;
> > +	     i++, bd++, buf += period_len, dma_addr += period_len) {
well sorry but this line blew me.

to quote Documentation/CodingStyle:
"Don't put multiple assignments on a single line either.  Kernel coding style
is super simple.  Avoid tricky expressions."
diff mbox

Patch

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index c75679d..875d08e 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1113,9 +1113,10 @@  static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
 {
 	struct sdma_channel *sdmac = to_sdma_chan(chan);
 	struct sdma_engine *sdma = sdmac->sdma;
+	struct sdma_buffer_descriptor *bd;
 	int num_periods = buf_len / period_len;
 	int channel = sdmac->channel;
-	int ret, i = 0, buf = 0;
+	int ret, i, buf;
 
 	dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
 
@@ -1144,8 +1145,9 @@  static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
 		goto err_out;
 	}
 
-	while (buf < buf_len) {
-		struct sdma_buffer_descriptor *bd = &sdmac->bd[i];
+	for (i = 0, buf = 0, bd = sdmac->bd;
+	     buf < buf_len;
+	     i++, bd++, buf += period_len, dma_addr += period_len) {
 		int param;
 
 		bd->buffer_addr = dma_addr;
@@ -1169,11 +1171,6 @@  static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
 				param & BD_INTR ? " intr" : "");
 
 		bd->mode.status = param;
-
-		dma_addr += period_len;
-		buf += period_len;
-
-		i++;
 	}
 
 	sdmac->num_bd = num_periods;