[PATCH] net-thunder: One check less in nicvf_register_interrupts() after error detection

[PATCH] net-thunder: One check less in nicvf_register_interrupts() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 31 Dec 2015 22:40:39 +0100

Adjust a jump target to eliminate a check before error logging.
Use the identifier "report_failure" instead of "err".

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index c24cb2a..21e1579 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -922,7 +922,7 @@ static int nicvf_register_interrupts(struct nicvf *nic)
 		ret = request_irq(vector, nicvf_intr_handler,
 				  0, nic->irq_name[irq], nic->napi[irq]);
 		if (ret)
-			goto err;
+			goto report_failure;
 		nic->irq_allocated[irq] = true;
 	}
 
@@ -933,7 +933,7 @@ static int nicvf_register_interrupts(struct nicvf *nic)
 		ret = request_irq(vector, nicvf_rbdr_intr_handler,
 				  0, nic->irq_name[irq], nic);
 		if (ret)
-			goto err;
+			goto report_failure;
 		nic->irq_allocated[irq] = true;
 	}
 
@@ -944,13 +944,12 @@ static int nicvf_register_interrupts(struct nicvf *nic)
 	ret = request_irq(nic->msix_entries[irq].vector,
 			  nicvf_qs_err_intr_handler,
 			  0, nic->irq_name[irq], nic);
-	if (!ret)
+	if (!ret) {
 		nic->irq_allocated[irq] = true;
-
-err:
-	if (ret)
-		netdev_err(nic->netdev, "request_irq failed, vector %d\n", irq);
-
+		return 0;
+	}
+report_failure:
+	netdev_err(nic->netdev, "request_irq failed, vector %d\n", irq);
 	return ret;
 }
 
-- 
2.6.3

[PATCH] net-thunder: One check less in nicvf_register_interrupts() after error detection

[Robert Richter]

On 31.12.15 22:47:31, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 31 Dec 2015 22:40:39 +0100
> 
> Adjust a jump target to eliminate a check before error logging.
> Use the identifier "report_failure" instead of "err".

I don't see much value in those changes. Using the 'err' label is ok
as it is not misleading and common use. And, there is no need to
optimize the check since this is not the fast path and will be
compiler optimized anyway. So let's keep the code as it is with the
flavor of the original author.

-Robert

> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/net/ethernet/cavium/thunder/nicvf_main.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> index c24cb2a..21e1579 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> @@ -922,7 +922,7 @@ static int nicvf_register_interrupts(struct nicvf *nic)
>  		ret = request_irq(vector, nicvf_intr_handler,
>  				  0, nic->irq_name[irq], nic->napi[irq]);
>  		if (ret)
> -			goto err;
> +			goto report_failure;
>  		nic->irq_allocated[irq] = true;
>  	}
>  
> @@ -933,7 +933,7 @@ static int nicvf_register_interrupts(struct nicvf *nic)
>  		ret = request_irq(vector, nicvf_rbdr_intr_handler,
>  				  0, nic->irq_name[irq], nic);
>  		if (ret)
> -			goto err;
> +			goto report_failure;
>  		nic->irq_allocated[irq] = true;
>  	}
>  
> @@ -944,13 +944,12 @@ static int nicvf_register_interrupts(struct nicvf *nic)
>  	ret = request_irq(nic->msix_entries[irq].vector,
>  			  nicvf_qs_err_intr_handler,
>  			  0, nic->irq_name[irq], nic);
> -	if (!ret)
> +	if (!ret) {
>  		nic->irq_allocated[irq] = true;
> -
> -err:
> -	if (ret)
> -		netdev_err(nic->netdev, "request_irq failed, vector %d\n", irq);
> -
> +		return 0;
> +	}
> +report_failure:
> +	netdev_err(nic->netdev, "request_irq failed, vector %d\n", irq);
>  	return ret;
>  }

[PATCH] net-thunder: One check less in nicvf_register_interrupts() after error detection

[SF Markus Elfring]

>> Adjust a jump target to eliminate a check before error logging.
>> Use the identifier "report_failure" instead of "err".
> 
> I don't see much value in those changes.

Thanks for your feedback.


> Using the 'err' label is ok as it is not misleading and common use.

Is such a short jump label enough explanation for the information
"what" and "why"?


> And, there is no need to optimize the check since this is not the fast path

Really? - Is it a bit more efficient to avoid a double check for the
variable "ret" at the end of the current implementation for the
discussed function?
https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/drivers/net/ethernet/cavium/thunder/nicvf_main.c?id=40fb5f8a60f33133d36afde35a9ad865d35e4423#n940


> and will be compiler optimized anyway.

How sure are you about automatic software optimisations?

Can it occasionally help to jump to the really intended source code
location directly?


>> @@ -944,13 +944,12 @@ static int nicvf_register_interrupts(struct nicvf *nic)
>>  	ret = request_irq(nic->msix_entries[irq].vector,
>>  			  nicvf_qs_err_intr_handler,
>>  			  0, nic->irq_name[irq], nic);
>> -	if (!ret)
>> +	if (!ret) {
>>  		nic->irq_allocated[irq] = true;
>> -
>> -err:
>> -	if (ret)
>> -		netdev_err(nic->netdev, "request_irq failed, vector %d\n", irq);
>> -
>> +		return 0;
>> +	}
>> +report_failure:
>> +	netdev_err(nic->netdev, "request_irq failed, vector %d\n", irq);
>>  	return ret;
>>  }

[PATCH] net-thunder: One check less in nicvf_register_interrupts() after error detection

[Joe Perches]

On Thu, 2016-01-07 at 20:30 +0100, SF Markus Elfring wrote:
> > > Adjust a jump target to eliminate a check before error logging.
> > > Use the identifier "report_failure" instead of "err".
> > I don't see much value in those changes
> Thanks for your feedback.
> > Using the 'err' label is ok as it is not misleading and common use.
> Is such a short jump label enough explanation for the information
> "what" and "why"?

When there is only one type of error possible, yes.

> > And, there is no need to optimize the check since this is not the
> > fast path
> Really? - Is it a bit more efficient to avoid a double check for the
> variable "ret" at the end of the current implementation for the
> discussed function?

Before asking questions you could answer yourself,
please look at object code produced by the compiler
before and after your proposed changes.

net-thunder: One check less in nicvf_register_interrupts() after error detection

[SF Markus Elfring]

>> Is it a bit more efficient to avoid a double check for the
>> variable "ret" at the end of the current implementation for the
>> discussed function?
> 
> Before asking questions you could answer yourself,
> please look at object code produced by the compiler
> before and after your proposed changes.

* Do any more source code reviewers wonder about the need
  for such a double check?

* Which object code representations would you find representative
  for a further constructive discussion around this
  software component?

Regards,
Markus

net-thunder: One check less in nicvf_register_interrupts() after error detection

[Joe Perches]

On Thu, 2016-01-07 at 20:56 +0100, SF Markus Elfring wrote:
> > > Is it a bit more efficient to avoid a double check for the
> > > variable "ret" at the end of the current implementation for the
> > > discussed function?
> > 
> > Before asking questions you could answer yourself,
> > please look at object code produced by the compiler
> > before and after your proposed changes.
> 
> * Do any more source code reviewers wonder about the need
> ? for such a double check?

Given the feedback you've already received,
it seems so.

> * Which object code representations would you find representative
> ? for a further constructive discussion around this
> ? software component?

Evidence of actual object code improvement when
with compiled with optimizations.

net-thunder: One check less in nicvf_register_interrupts() after error detection

[SF Markus Elfring]

>> * Which object code representations would you find representative
>>   for a further constructive discussion around this
>>   software component?
> 
> Evidence of actual object code improvement

How do you think about to provide a function implementation
which looks a bit more efficient by default?


> when with compiled with optimizations.

Which combinations of hardware and software would you recommend
for corresponding system checks?

Regards,
Markus

net-thunder: One check less in nicvf_register_interrupts() after error detection

[Joe Perches]

On Thu, 2016-01-07 at 21:07 +0100, SF Markus Elfring wrote:
> > > * Which object code representations would you find representative
> > > ? for a further constructive discussion around this
> > > ? software component?
> > 
> > Evidence of actual object code improvement
> 
> How do you think about to provide a function implementation
> which looks a bit more efficient by default?

It's not a matter of "looks a bit more efficient".
it's taste, style, and repetition for various functions.

Some prefer that source code be "templatized" regardless
of the number of exit points that any particular use of a
specific function type.

Some of your patches are converting these templatized
functions to a different form for no added value.

These patches make the local source code inconsistent
and generally goes against the authors preferred style.

net-thunder: One check less in nicvf_register_interrupts() after error detection

[SF Markus Elfring]

> Some prefer that source code be "templatized" regardless
> of the number of exit points that any particular use of a
> specific function type.

This is another interesting view on involved implementation details.


> Some of your patches are converting these templatized
> functions to a different form for no added value.

Would you like to distinguish a bit more between my evolving
collection of update suggestions and the concrete proposal
for the function "nicvf_register_interrupts"?


> These patches make the local source code inconsistent
> and generally goes against the authors preferred style.

Which programming approach will be the leading one here finally?

Regards,
Markus

net-thunder: One check less in nicvf_register_interrupts() after error detection

[Joe Perches]

On Thu, 2016-01-07 at 21:38 +0100, SF Markus Elfring wrote:
> > Some prefer that source code be "templatized" regardless
> > of the number of exit points that any particular use of a
> > specific function type.
[]
> > Some of your patches are converting these templatized
> > functions to a different form for no added value.
> 
> Would you like to distinguish a bit more between my evolving
> collection of update suggestions and the concrete proposal
> for the function "nicvf_register_interrupts"?

No.

> > These patches make the local source code inconsistent
> > and generally goes against the authors preferred style.
> 
> Which programming approach will be the leading one here finally?

Whatever the developer wants.

There is no _best_ or _only_ style for this.

[PATCH 00/24] ste_dma40: Fine-tuning for several function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 16:56:56 +0200

Several update suggestions were taken into account
from static source code analysis.

Markus Elfring (24):
  Use kmalloc_array() in d40_lcla_allocate()
  Return directly after a failed kmalloc_array()
  Rename a jump label in d40_lcla_allocate()
  Move an assignment in d40_lcla_allocate()
  Improve a size determination in d40_of_probe()
  Replace four kzalloc() calls by kcalloc() in d40_hw_detect_init()
  Use kmalloc_array() in d40_hw_detect_init()
  Less checks in d40_hw_detect_init() after error detection
  Delete unnecessary variable initialisations in d40_hw_detect_init()
  Adjust the position of a jump label in d40_probe()
  Rename a jump label in d40_probe()
  Rename jump labels in d40_dmaengine_init()
  Rename a jump label in d40_alloc_chan_resources()
  One check less in d40_prep_sg() after error detection
  Move two assignments in d40_prep_sg()
  Rename a jump label in d40_prep_desc()
  Move an assignment in d40_prep_desc()
  Rename a jump label in d40_is_paused()
  Rename a jump label in d40_free_dma()
  Rename a jump label in d40_alloc_mask_free()
  Rename jump labels in d40_alloc_mask_set()
  Rename a jump label in dma_tasklet()
  Rename a jump label in __d40_execute_command_phy()
  Rename a jump label in d40_log_lli_to_lcxa()

 drivers/dma/ste_dma40.c | 253 +++++++++++++++++++++++-------------------------
 1 file changed, 122 insertions(+), 131 deletions(-)

-- 
2.10.0

[PATCH 01/24] ste_dma40: Use kmalloc_array() in d40_lcla_allocate()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 16 Sep 2016 17:56:07 +0200

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus use the corresponding function "kmalloc_array".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 73203ac..fbebeff 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3409,9 +3409,9 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 	 * To full fill this hardware requirement without wasting 256 kb
 	 * we allocate pages until we get an aligned one.
 	 */
-	page_list = kmalloc(sizeof(unsigned long) * MAX_LCLA_ALLOC_ATTEMPTS,
-			    GFP_KERNEL);
-
+	page_list = kmalloc_array(MAX_LCLA_ALLOC_ATTEMPTS,
+				  sizeof(*page_list),
+				  GFP_KERNEL);
 	if (!page_list) {
 		ret = -ENOMEM;
 		goto failure;
-- 
2.10.0

[PATCH 02/24] ste_dma40: Return directly after a failed kmalloc_array()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 08:21:30 +0200

Return directly after a memory allocation failed in this function
at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index fbebeff..80a199a 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3412,10 +3412,8 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 	page_list = kmalloc_array(MAX_LCLA_ALLOC_ATTEMPTS,
 				  sizeof(*page_list),
 				  GFP_KERNEL);
-	if (!page_list) {
-		ret = -ENOMEM;
-		goto failure;
-	}
+	if (!page_list)
+		return -ENOMEM;
 
 	/* Calculating how many pages that are required */
 	base->lcla_pool.pages = SZ_1K * base->num_phy_chans / PAGE_SIZE;
-- 
2.10.0

[PATCH 03/24] ste_dma40: Rename a jump label in d40_lcla_allocate()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 08:23:37 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 80a199a..76d63b6 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3429,7 +3429,7 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 
 			for (j = 0; j < i; j++)
 				free_pages(page_list[j], base->lcla_pool.pages);
-			goto failure;
+			goto free_page_list;
 		}
 
 		if ((virt_to_phys((void *)page_list[i]) &
@@ -3456,7 +3456,7 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 							 GFP_KERNEL);
 		if (!base->lcla_pool.base_unaligned) {
 			ret = -ENOMEM;
-			goto failure;
+			goto free_page_list;
 		}
 
 		base->lcla_pool.base = PTR_ALIGN(base->lcla_pool.base_unaligned,
@@ -3469,12 +3469,12 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 	if (dma_mapping_error(base->dev, pool->dma_addr)) {
 		pool->dma_addr = 0;
 		ret = -ENOMEM;
-		goto failure;
+		goto free_page_list;
 	}
 
 	writel(virt_to_phys(base->lcla_pool.base),
 	       base->virtbase + D40_DREG_LCLA);
-failure:
+ free_page_list:
 	kfree(page_list);
 	return ret;
 }
-- 
2.10.0

[PATCH 04/24] ste_dma40: Move an assignment in d40_lcla_allocate()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 08:24:46 +0200

Move one assignment for the local variable "ret" so that its setting
will only be performed after corresponding data processing succeeded
by this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 76d63b6..220129e 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3402,7 +3402,7 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 	struct d40_lcla_pool *pool = &base->lcla_pool;
 	unsigned long *page_list;
 	int i, j;
-	int ret = 0;
+	int ret;
 
 	/*
 	 * This is somewhat ugly. We need 8192 bytes that are 18 bit aligned,
@@ -3474,6 +3474,7 @@ static int __init d40_lcla_allocate(struct d40_base *base)
 
 	writel(virt_to_phys(base->lcla_pool.base),
 	       base->virtbase + D40_DREG_LCLA);
+	ret = 0;
  free_page_list:
 	kfree(page_list);
 	return ret;
-- 
2.10.0

[PATCH 05/24] ste_dma40: Improve a size determination in d40_of_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 08:28:05 +0200

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 220129e..57d87a8 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3487,9 +3487,7 @@ static int __init d40_of_probe(struct platform_device *pdev,
 	int num_phy = 0, num_memcpy = 0, num_disabled = 0;
 	const __be32 *list;
 
-	pdata = devm_kzalloc(&pdev->dev,
-			     sizeof(struct stedma40_platform_data),
-			     GFP_KERNEL);
+	pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
 	if (!pdata)
 		return -ENOMEM;
 
-- 
2.10.0

[PATCH 06/24] ste_dma40: Replace four kzalloc() calls by kcalloc() in d40_hw_detect_init()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 09:56:32 +0200

* The script "checkpatch.pl" can point information out like the following.

  WARNING: Prefer kcalloc over kzalloc with multiply

  Thus fix the affected source code places.

* Replace the specification of data types by pointer dereferences
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 57d87a8..b5d15a1 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3281,19 +3281,20 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 		base->gen_dmac.init_reg_size = ARRAY_SIZE(dma_init_reg_v4a);
 	}
 
-	base->phy_res = kzalloc(num_phy_chans * sizeof(struct d40_phy_res),
+	base->phy_res = kcalloc(num_phy_chans,
+				sizeof(*base->phy_res),
 				GFP_KERNEL);
 	if (!base->phy_res)
 		goto failure;
 
-	base->lookup_phy_chans = kzalloc(num_phy_chans *
-					 sizeof(struct d40_chan *),
+	base->lookup_phy_chans = kcalloc(num_phy_chans,
+					 sizeof(*base->lookup_phy_chans),
 					 GFP_KERNEL);
 	if (!base->lookup_phy_chans)
 		goto failure;
 
-	base->lookup_log_chans = kzalloc(num_log_chans *
-					 sizeof(struct d40_chan *),
+	base->lookup_log_chans = kcalloc(num_log_chans,
+					 sizeof(*base->lookup_log_chans),
 					 GFP_KERNEL);
 	if (!base->lookup_log_chans)
 		goto failure;
@@ -3304,9 +3305,10 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 	if (!base->reg_val_backup_chan)
 		goto failure;
 
-	base->lcla_pool.alloc_map =
-		kzalloc(num_phy_chans * sizeof(struct d40_desc *)
-			* D40_LCLA_LINK_PER_EVENT_GRP, GFP_KERNEL);
+	base->lcla_pool.alloc_map = kcalloc(num_phy_chans
+					    * D40_LCLA_LINK_PER_EVENT_GRP,
+					    sizeof(*base->lcla_pool.alloc_map),
+					    GFP_KERNEL);
 	if (!base->lcla_pool.alloc_map)
 		goto failure;
 
-- 
2.10.0

[PATCH 07/24] ste_dma40: Use kmalloc_array() in d40_hw_detect_init()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 11:44:55 +0200

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "kmalloc_array".

This issue was detected also by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index b5d15a1..f813056 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3299,9 +3299,9 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 	if (!base->lookup_log_chans)
 		goto failure;
 
-	base->reg_val_backup_chan = kmalloc(base->num_phy_chans *
-					    sizeof(d40_backup_regs_chan),
-					    GFP_KERNEL);
+	base->reg_val_backup_chan = kmalloc_array(base->num_phy_chans,
+						  sizeof(d40_backup_regs_chan),
+						  GFP_KERNEL);
 	if (!base->reg_val_backup_chan)
 		goto failure;
 
-- 
2.10.0

[PATCH 08/24] ste_dma40: Less checks in d40_hw_detect_init() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 14:10:47 +0200

Four checks could be repeated by the d40_hw_detect_init() function during
error handling even if the passed variables contained a null pointer.

* Adjust jump targets according to the Linux coding style convention.

* Call the interface "iounmap" only once at the end.

* Delete the repeated checks which became unnecessary with
  this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 67 ++++++++++++++++++++++++-------------------------
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index f813056..c680dd3 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3158,27 +3158,27 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 	clk = clk_get(&pdev->dev, NULL);
 	if (IS_ERR(clk)) {
 		d40_err(&pdev->dev, "No matching clock found\n");
-		goto failure;
+		goto check_prepare_enabled;
 	}
 
 	clk_ret = clk_prepare_enable(clk);
 	if (clk_ret) {
 		d40_err(&pdev->dev, "Failed to prepare/enable clock\n");
-		goto failure;
+		goto disable_unprepare;
 	}
 
 	/* Get IO for DMAC base address */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "base");
 	if (!res)
-		goto failure;
+		goto disable_unprepare;
 
 	if (request_mem_region(res->start, resource_size(res),
 			       D40_NAME " I/O base") == NULL)
-		goto failure;
+		goto release_region;
 
 	virtbase = ioremap(res->start, resource_size(res));
 	if (!virtbase)
-		goto failure;
+		goto release_region;
 
 	/* This is just a regular AMBA PrimeCell ID actually */
 	for (pid = 0, i = 0; i < 4; i++)
@@ -3190,13 +3190,13 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 
 	if (cid != AMBA_CID) {
 		d40_err(&pdev->dev, "Unknown hardware! No PrimeCell ID\n");
-		goto failure;
+		goto unmap_io;
 	}
 	if (AMBA_MANF_BITS(pid) != AMBA_VENDOR_ST) {
 		d40_err(&pdev->dev, "Unknown designer! Got %x wanted %x\n",
 			AMBA_MANF_BITS(pid),
 			AMBA_VENDOR_ST);
-		goto failure;
+		goto unmap_io;
 	}
 	/*
 	 * HW revision:
@@ -3210,7 +3210,7 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 	rev = AMBA_REV_BITS(pid);
 	if (rev < 2) {
 		d40_err(&pdev->dev, "hardware revision: %d is not supported", rev);
-		goto failure;
+		goto unmap_io;
 	}
 
 	/* The number of physical channels on this HW */
@@ -3236,7 +3236,7 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 		       sizeof(struct d40_chan), GFP_KERNEL);
 
 	if (base == NULL)
-		goto failure;
+		goto unmap_io;
 
 	base->rev = rev;
 	base->clk = clk;
@@ -3285,63 +3285,62 @@ static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 				sizeof(*base->phy_res),
 				GFP_KERNEL);
 	if (!base->phy_res)
-		goto failure;
+		goto free_base;
 
 	base->lookup_phy_chans = kcalloc(num_phy_chans,
 					 sizeof(*base->lookup_phy_chans),
 					 GFP_KERNEL);
 	if (!base->lookup_phy_chans)
-		goto failure;
+		goto free_phy_res;
 
 	base->lookup_log_chans = kcalloc(num_log_chans,
 					 sizeof(*base->lookup_log_chans),
 					 GFP_KERNEL);
 	if (!base->lookup_log_chans)
-		goto failure;
+		goto free_phy_chans;
 
 	base->reg_val_backup_chan = kmalloc_array(base->num_phy_chans,
 						  sizeof(d40_backup_regs_chan),
 						  GFP_KERNEL);
 	if (!base->reg_val_backup_chan)
-		goto failure;
+		goto free_log_chans;
 
 	base->lcla_pool.alloc_map = kcalloc(num_phy_chans
 					    * D40_LCLA_LINK_PER_EVENT_GRP,
 					    sizeof(*base->lcla_pool.alloc_map),
 					    GFP_KERNEL);
 	if (!base->lcla_pool.alloc_map)
-		goto failure;
+		goto free_backup_chan;
 
 	base->desc_slab = kmem_cache_create(D40_NAME, sizeof(struct d40_desc),
 					    0, SLAB_HWCACHE_ALIGN,
 					    NULL);
 	if (base->desc_slab == NULL)
-		goto failure;
+		goto free_map;
 
 	return base;
-
-failure:
+ free_map:
+	kfree(base->lcla_pool.alloc_map);
+ free_backup_chan:
+	kfree(base->reg_val_backup_chan);
+ free_log_chans:
+	kfree(base->lookup_log_chans);
+ free_phy_chans:
+	kfree(base->lookup_phy_chans);
+ free_phy_res:
+	kfree(base->phy_res);
+ free_base:
+	kfree(base);
+ unmap_io:
+	iounmap(virtbase);
+ release_region:
+	release_mem_region(res->start, resource_size(res));
+ check_prepare_enabled:
 	if (!clk_ret)
+ disable_unprepare:
 		clk_disable_unprepare(clk);
 	if (!IS_ERR(clk))
 		clk_put(clk);
-	if (virtbase)
-		iounmap(virtbase);
-	if (res)
-		release_mem_region(res->start,
-				   resource_size(res));
-	if (virtbase)
-		iounmap(virtbase);
-
-	if (base) {
-		kfree(base->lcla_pool.alloc_map);
-		kfree(base->reg_val_backup_chan);
-		kfree(base->lookup_log_chans);
-		kfree(base->lookup_phy_chans);
-		kfree(base->phy_res);
-		kfree(base);
-	}
-
 	return NULL;
 }
 
-- 
2.10.0

[PATCH 09/24] ste_dma40: Delete unnecessary variable initialisations in d40_hw_detect_init()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 14:34:18 +0200

Five local variables will be set to an appropriate value a bit later.
Thus omit the explicit initialisation which became unnecessary with
a previous update step.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index c680dd3..ebb00a8 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3142,11 +3142,11 @@ static int __init d40_phy_res_init(struct d40_base *base)
 static struct d40_base * __init d40_hw_detect_init(struct platform_device *pdev)
 {
 	struct stedma40_platform_data *plat_data = dev_get_platdata(&pdev->dev);
-	struct clk *clk = NULL;
-	void __iomem *virtbase = NULL;
-	struct resource *res = NULL;
-	struct d40_base *base = NULL;
-	int num_log_chans = 0;
+	struct clk *clk;
+	void __iomem *virtbase;
+	struct resource *res;
+	struct d40_base *base;
+	int num_log_chans;
 	int num_phy_chans;
 	int num_memcpy_chans;
 	int clk_ret = -EINVAL;
-- 
2.10.0

[PATCH 10/24] ste_dma40: Adjust the position of a jump label in d40_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 14:36:26 +0200

Add a space character before a single jump label in this function
according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index ebb00a8..4ebc825 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3728,7 +3728,7 @@ failure:
 	kfree(base->lookup_phy_chans);
 	kfree(base->phy_res);
 	kfree(base);
-report_failure:
+ report_failure:
 	d40_err(&pdev->dev, "probe failed\n");
 	return ret;
 }
-- 
2.10.0

[PATCH 11/24] ste_dma40: Rename a jump label in d40_probe()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 14:50:53 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 25 ++++++++++++-------------
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 4ebc825..ed96039 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -3570,7 +3570,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	if (!res) {
 		ret = -ENOENT;
 		d40_err(&pdev->dev, "No \"lcpa\" memory resource\n");
-		goto failure;
+		goto destroy_cache;
 	}
 	base->lcpa_size = resource_size(res);
 	base->phy_lcpa = res->start;
@@ -3579,7 +3579,7 @@ static int __init d40_probe(struct platform_device *pdev)
 			       D40_NAME " I/O lcpa") == NULL) {
 		ret = -EBUSY;
 		d40_err(&pdev->dev, "Failed to request LCPA region %pR\n", res);
-		goto failure;
+		goto destroy_cache;
 	}
 
 	/* We make use of ESRAM memory for this. */
@@ -3595,7 +3595,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	if (!base->lcpa_base) {
 		ret = -ENOMEM;
 		d40_err(&pdev->dev, "Failed to ioremap LCPA region\n");
-		goto failure;
+		goto destroy_cache;
 	}
 	/* If lcla has to be located in ESRAM we don't need to allocate */
 	if (base->plat_data->use_esram_lcla) {
@@ -3605,14 +3605,14 @@ static int __init d40_probe(struct platform_device *pdev)
 			ret = -ENOENT;
 			d40_err(&pdev->dev,
 				"No \"lcla_esram\" memory resource\n");
-			goto failure;
+			goto destroy_cache;
 		}
 		base->lcla_pool.base = ioremap(res->start,
 						resource_size(res));
 		if (!base->lcla_pool.base) {
 			ret = -ENOMEM;
 			d40_err(&pdev->dev, "Failed to ioremap LCLA region\n");
-			goto failure;
+			goto destroy_cache;
 		}
 		writel(res->start, base->virtbase + D40_DREG_LCLA);
 
@@ -3620,7 +3620,7 @@ static int __init d40_probe(struct platform_device *pdev)
 		ret = d40_lcla_allocate(base);
 		if (ret) {
 			d40_err(&pdev->dev, "Failed to allocate LCLA area\n");
-			goto failure;
+			goto destroy_cache;
 		}
 	}
 
@@ -3631,7 +3631,7 @@ static int __init d40_probe(struct platform_device *pdev)
 	ret = request_irq(base->irq, d40_handle_interrupt, 0, D40_NAME, base);
 	if (ret) {
 		d40_err(&pdev->dev, "No IRQ defined\n");
-		goto failure;
+		goto destroy_cache;
 	}
 
 	if (base->plat_data->use_esram_lcla) {
@@ -3641,7 +3641,7 @@ static int __init d40_probe(struct platform_device *pdev)
 			d40_err(&pdev->dev, "Failed to get lcpa_regulator\n");
 			ret = PTR_ERR(base->lcpa_regulator);
 			base->lcpa_regulator = NULL;
-			goto failure;
+			goto destroy_cache;
 		}
 
 		ret = regulator_enable(base->lcpa_regulator);
@@ -3650,7 +3650,7 @@ static int __init d40_probe(struct platform_device *pdev)
 				"Failed to enable lcpa_regulator\n");
 			regulator_put(base->lcpa_regulator);
 			base->lcpa_regulator = NULL;
-			goto failure;
+			goto destroy_cache;
 		}
 	}
 
@@ -3665,13 +3665,13 @@ static int __init d40_probe(struct platform_device *pdev)
 
 	ret = d40_dmaengine_init(base, num_reserved_chans);
 	if (ret)
-		goto failure;
+		goto destroy_cache;
 
 	base->dev->dma_parms = &base->dma_parms;
 	ret = dma_set_max_seg_size(base->dev, STEDMA40_MAX_SEG_SIZE);
 	if (ret) {
 		d40_err(&pdev->dev, "Failed to set dma max seg size\n");
-		goto failure;
+		goto destroy_cache;
 	}
 
 	d40_hw_init(base);
@@ -3685,8 +3685,7 @@ static int __init d40_probe(struct platform_device *pdev)
 
 	dev_info(base->dev, "initialized\n");
 	return 0;
-
-failure:
+ destroy_cache:
 	kmem_cache_destroy(base->desc_slab);
 	if (base->virtbase)
 		iounmap(base->virtbase);
-- 
2.10.0

[PATCH 12/24] ste_dma40: Rename jump labels in d40_dmaengine_init()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 15:10:15 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index ed96039..c7b73f1 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2889,7 +2889,7 @@ static int __init d40_dmaengine_init(struct d40_base *base,
 
 	if (err) {
 		d40_err(base->dev, "Failed to register slave channels\n");
-		goto failure1;
+		goto exit;
 	}
 
 	d40_chan_init(base, &base->dma_memcpy, base->log_chans,
@@ -2906,7 +2906,7 @@ static int __init d40_dmaengine_init(struct d40_base *base,
 	if (err) {
 		d40_err(base->dev,
 			"Failed to register memcpy only channels\n");
-		goto failure2;
+		goto unregister_slave;
 	}
 
 	d40_chan_init(base, &base->dma_both, base->phy_chans,
@@ -2924,14 +2924,14 @@ static int __init d40_dmaengine_init(struct d40_base *base,
 	if (err) {
 		d40_err(base->dev,
 			"Failed to register logical and physical capable channels\n");
-		goto failure3;
+		goto unregister_memcpy;
 	}
 	return 0;
-failure3:
+ unregister_memcpy:
 	dma_async_device_unregister(&base->dma_memcpy);
-failure2:
+ unregister_slave:
 	dma_async_device_unregister(&base->dma_slave);
-failure1:
+ exit:
 	return err;
 }
 
-- 
2.10.0

[PATCH 13/24] ste_dma40: Rename a jump label in d40_alloc_chan_resources()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 15:15:15 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index c7b73f1..0788add 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2424,7 +2424,7 @@ static int d40_alloc_chan_resources(struct dma_chan *chan)
 		err = d40_config_memcpy(d40c);
 		if (err) {
 			chan_err(d40c, "Failed to configure memcpy channel\n");
-			goto fail;
+			goto mark_last_busy;
 		}
 	}
 
@@ -2432,7 +2432,7 @@ static int d40_alloc_chan_resources(struct dma_chan *chan)
 	if (err) {
 		chan_err(d40c, "Failed to allocate channel\n");
 		d40c->configured = false;
-		goto fail;
+		goto mark_last_busy;
 	}
 
 	pm_runtime_get_sync(d40c->base->dev);
@@ -2466,7 +2466,7 @@ static int d40_alloc_chan_resources(struct dma_chan *chan)
 	 */
 	if (is_free_phy)
 		d40_config_write(d40c);
-fail:
+ mark_last_busy:
 	pm_runtime_mark_last_busy(d40c->base->dev);
 	pm_runtime_put_autosuspend(d40c->base->dev);
 	spin_unlock_irqrestore(&d40c->lock, flags);
-- 
2.10.0

[PATCH 14/24] ste_dma40: One check less in d40_prep_sg() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 15:34:07 +0200

* Adjust jump targets according to the Linux coding style convention.

* Delete a repeated check which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 0788add..a7e7cd0 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2251,7 +2251,7 @@ d40_prep_sg(struct dma_chan *dchan, struct scatterlist *sg_src,
 
 	desc = d40_prep_desc(chan, sg_src, sg_len, dma_flags);
 	if (desc == NULL)
-		goto err;
+		goto unlock;
 
 	if (sg_next(&sg_src[sg_len - 1]) == sg_src)
 		desc->cyclic = true;
@@ -2271,7 +2271,7 @@ d40_prep_sg(struct dma_chan *dchan, struct scatterlist *sg_src,
 	if (ret) {
 		chan_err(chan, "Failed to prepare %s sg job: %d\n",
 			 chan_is_logical(chan) ? "log" : "phy", ret);
-		goto err;
+		goto free_desc;
 	}
 
 	/*
@@ -2283,10 +2283,9 @@ d40_prep_sg(struct dma_chan *dchan, struct scatterlist *sg_src,
 	spin_unlock_irqrestore(&chan->lock, flags);
 
 	return &desc->txd;
-
-err:
-	if (desc)
-		d40_desc_free(chan, desc);
+ free_desc:
+	d40_desc_free(chan, desc);
+ unlock:
 	spin_unlock_irqrestore(&chan->lock, flags);
 	return NULL;
 }
-- 
2.10.0

[PATCH 15/24] ste_dma40: Move two assignments in d40_prep_sg()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 15:40:05 +0200

Move assignments for two local variables so that their setting
will only be performed after corresponding data processing succeeded
by this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index a7e7cd0..6725b66 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2236,8 +2236,8 @@ d40_prep_sg(struct dma_chan *dchan, struct scatterlist *sg_src,
 	    enum dma_transfer_direction direction, unsigned long dma_flags)
 {
 	struct d40_chan *chan = container_of(dchan, struct d40_chan, chan);
-	dma_addr_t src_dev_addr = 0;
-	dma_addr_t dst_dev_addr = 0;
+	dma_addr_t src_dev_addr;
+	dma_addr_t dst_dev_addr;
 	struct d40_desc *desc;
 	unsigned long flags;
 	int ret;
@@ -2256,6 +2256,8 @@ d40_prep_sg(struct dma_chan *dchan, struct scatterlist *sg_src,
 	if (sg_next(&sg_src[sg_len - 1]) == sg_src)
 		desc->cyclic = true;
 
+	src_dev_addr = 0;
+	dst_dev_addr = 0;
 	if (direction == DMA_DEV_TO_MEM)
 		src_dev_addr = chan->runtime_addr;
 	else if (direction == DMA_MEM_TO_DEV)
-- 
2.10.0

[PATCH 16/24] ste_dma40: Rename a jump label in d40_prep_desc()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 15:51:37 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 6725b66..7f38496 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2208,13 +2208,13 @@ d40_prep_desc(struct d40_chan *chan, struct scatterlist *sg,
 					cfg->dst_info.data_width);
 	if (desc->lli_len < 0) {
 		chan_err(chan, "Unaligned size\n");
-		goto err;
+		goto free_desc;
 	}
 
 	ret = d40_pool_lli_alloc(chan, desc, desc->lli_len);
 	if (ret < 0) {
 		chan_err(chan, "Could not allocate lli\n");
-		goto err;
+		goto free_desc;
 	}
 
 	desc->lli_current = 0;
@@ -2224,8 +2224,7 @@ d40_prep_desc(struct d40_chan *chan, struct scatterlist *sg,
 	dma_async_tx_descriptor_init(&desc->txd, &chan->chan);
 
 	return desc;
-
-err:
+ free_desc:
 	d40_desc_free(chan, desc);
 	return NULL;
 }
-- 
2.10.0

[PATCH 17/24] ste_dma40: Move an assignment in d40_prep_desc()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 15:54:12 +0200

Move one assignment for the local variable "cfg" so that its setting
will only be performed after a call of the function "d40_desc_get"
succeeded by this function.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 7f38496..4a21778 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2196,7 +2196,7 @@ static struct d40_desc *
 d40_prep_desc(struct d40_chan *chan, struct scatterlist *sg,
 	      unsigned int sg_len, unsigned long dma_flags)
 {
-	struct stedma40_chan_cfg *cfg = &chan->dma_cfg;
+	struct stedma40_chan_cfg *cfg;
 	struct d40_desc *desc;
 	int ret;
 
@@ -2204,6 +2204,7 @@ d40_prep_desc(struct d40_chan *chan, struct scatterlist *sg,
 	if (!desc)
 		return NULL;
 
+	cfg = &chan->dma_cfg;
 	desc->lli_len = d40_sg_2_dmalen(sg, sg_len, cfg->src_info.data_width,
 					cfg->dst_info.data_width);
 	if (desc->lli_len < 0) {
-- 
2.10.0

[PATCH 18/24] ste_dma40: Rename a jump label in d40_is_paused()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 16:00:05 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 4a21778..d99241c 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2092,8 +2092,7 @@ static bool d40_is_paused(struct d40_chan *d40c)
 			D40_CHAN_POS(d40c->phy_chan->num);
 		if (status == D40_DMA_SUSPENDED || status == D40_DMA_STOP)
 			is_paused = true;
-
-		goto _exit;
+		goto unlock;
 	}
 
 	if (d40c->dma_cfg.dir == DMA_MEM_TO_DEV ||
@@ -2103,7 +2102,7 @@ static bool d40_is_paused(struct d40_chan *d40c)
 		status = readl(chanbase + D40_CHAN_REG_SSLNK);
 	} else {
 		chan_err(d40c, "Unknown direction\n");
-		goto _exit;
+		goto unlock;
 	}
 
 	status = (status & D40_EVENTLINE_MASK(event)) >>
@@ -2111,7 +2110,7 @@ static bool d40_is_paused(struct d40_chan *d40c)
 
 	if (status != D40_DMA_RUN)
 		is_paused = true;
-_exit:
+ unlock:
 	spin_unlock_irqrestore(&d40c->lock, flags);
 	return is_paused;
 
-- 
2.10.0

[PATCH 19/24] ste_dma40: Rename a jump label in d40_free_dma()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 16:04:46 +0200

Adjust a jump label according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index d99241c..917b24a 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2045,7 +2045,7 @@ static int d40_free_dma(struct d40_chan *d40c)
 	res = d40_channel_execute_command(d40c, D40_DMA_STOP);
 	if (res) {
 		chan_err(d40c, "stop failed\n");
-		goto out;
+		goto mark_last_busy;
 	}
 
 	d40_alloc_mask_free(phy, is_src, chan_is_logical(d40c) ? event : 0);
@@ -2063,8 +2063,7 @@ static int d40_free_dma(struct d40_chan *d40c)
 	d40c->busy = false;
 	d40c->phy_chan = NULL;
 	d40c->configured = false;
-out:
-
+ mark_last_busy:
 	pm_runtime_mark_last_busy(d40c->base->dev);
 	pm_runtime_put_autosuspend(d40c->base->dev);
 	return res;
-- 
2.10.0

[PATCH 20/24] ste_dma40: Rename a jump label in d40_alloc_mask_free()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 16:10:41 +0200

Adjust a jump label according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 917b24a..fbe87a2 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -1829,7 +1829,7 @@ static bool d40_alloc_mask_free(struct d40_phy_res *phy, bool is_src,
 		phy->allocated_dst = D40_ALLOC_FREE;
 		phy->allocated_src = D40_ALLOC_FREE;
 		is_free = true;
-		goto out;
+		goto unlock;
 	}
 
 	/* Logical channel */
@@ -1845,8 +1845,7 @@ static bool d40_alloc_mask_free(struct d40_phy_res *phy, bool is_src,
 
 	is_free = ((phy->allocated_src | phy->allocated_dst) ==
 		   D40_ALLOC_FREE);
-
-out:
+ unlock:
 	spin_unlock_irqrestore(&phy->lock, flags);
 
 	return is_free;
-- 
2.10.0

[PATCH 21/24] ste_dma40: Rename jump labels in d40_alloc_mask_set()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 16:16:42 +0200

Adjust jump labels according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index fbe87a2..4a2f39b 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -1778,42 +1778,40 @@ static bool d40_alloc_mask_set(struct d40_phy_res *phy,
 		    phy->allocated_dst == D40_ALLOC_FREE) {
 			phy->allocated_dst = D40_ALLOC_PHY;
 			phy->allocated_src = D40_ALLOC_PHY;
-			goto found;
+			goto found_unlock;
 		} else
-			goto not_found;
+			goto not_found_unlock;
 	}
 
 	/* Logical channel */
 	if (is_src) {
 		if (phy->allocated_src == D40_ALLOC_PHY)
-			goto not_found;
+			goto not_found_unlock;
 
 		if (phy->allocated_src == D40_ALLOC_FREE)
 			phy->allocated_src = D40_ALLOC_LOG_FREE;
 
 		if (!(phy->allocated_src & BIT(log_event_line))) {
 			phy->allocated_src |= BIT(log_event_line);
-			goto found;
+			goto found_unlock;
 		} else
-			goto not_found;
+			goto not_found_unlock;
 	} else {
 		if (phy->allocated_dst == D40_ALLOC_PHY)
-			goto not_found;
+			goto not_found_unlock;
 
 		if (phy->allocated_dst == D40_ALLOC_FREE)
 			phy->allocated_dst = D40_ALLOC_LOG_FREE;
 
 		if (!(phy->allocated_dst & BIT(log_event_line))) {
 			phy->allocated_dst |= BIT(log_event_line);
-			goto found;
-		} else
-			goto not_found;
+			goto found_unlock;
+		}
 	}
-
-not_found:
+ not_found_unlock:
 	spin_unlock_irqrestore(&phy->lock, flags);
 	return false;
-found:
+ found_unlock:
 	spin_unlock_irqrestore(&phy->lock, flags);
 	return true;
 }
-- 
2.10.0

[PATCH 22/24] ste_dma40: Rename a jump label in dma_tasklet()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 16:23:43 +0200

Adjust a jump label according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 4a2f39b..0082ae0 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -1606,7 +1606,7 @@ static void dma_tasklet(unsigned long data)
 		/* Check if we have reached here for cyclic job */
 		d40d = d40_first_active_get(d40c);
 		if (d40d == NULL || !d40d->cyclic)
-			goto err;
+			goto check_pending_tx;
 	}
 
 	if (!d40d->cyclic)
@@ -1648,8 +1648,7 @@ static void dma_tasklet(unsigned long data)
 		dmaengine_desc_callback_invoke(&cb, NULL);
 
 	return;
-
-err:
+ check_pending_tx:
 	/* Rescue manouver if receiving double interrupts */
 	if (d40c->pending_tx > 0)
 		d40c->pending_tx--;
-- 
2.10.0

[PATCH 23/24] ste_dma40: Rename a jump label in __d40_execute_command_phy()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 16:28:54 +0200

Adjust a jump label according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 0082ae0..01fe3a2 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -1083,7 +1083,7 @@ static int __d40_execute_command_phy(struct d40_chan *d40c,
 			D40_CHAN_POS(d40c->phy_chan->num);
 
 		if (status == D40_DMA_SUSPENDED || status == D40_DMA_STOP)
-			goto done;
+			goto unlock;
 	}
 
 	wmask = 0xffffffff & ~(D40_CHAN_POS_MASK(d40c->phy_chan->num));
@@ -1119,7 +1119,7 @@ static int __d40_execute_command_phy(struct d40_chan *d40c,
 		}
 
 	}
-done:
+ unlock:
 	spin_unlock_irqrestore(&d40c->base->execmd_lock, flags);
 	return ret;
 }
-- 
2.10.0

[PATCH 24/24] ste_dma40: Rename a jump label in d40_log_lli_to_lcxa()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 17 Sep 2016 16:39:06 +0200

Adjust a jump label according to the current Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/dma/ste_dma40.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 01fe3a2..dcb26c6 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -874,7 +874,7 @@ static void d40_log_lli_to_lcxa(struct d40_chan *chan, struct d40_desc *desc)
 	}
 
 	if (curr_lcla < 0)
-		goto out;
+		goto set_current;
 
 	for (; lli_current < lli_len; lli_current++) {
 		unsigned int lcla_offset = chan->phy_chan->num * 1024 +
@@ -925,8 +925,7 @@ static void d40_log_lli_to_lcxa(struct d40_chan *chan, struct d40_desc *desc)
 			break;
 		}
 	}
-
-out:
+ set_current:
 	desc->lli_current = lli_current;
 }
 
-- 
2.10.0

[PATCH 00/24] ste_dma40: Fine-tuning for several function implementations

[Linus Walleij]

On Sat, Sep 17, 2016 at 5:05 PM, SF Markus Elfring
<elfring@users.sourceforge.net> wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 17 Sep 2016 16:56:56 +0200
>
> Several update suggestions were taken into account
> from static source code analysis.

All look very nice!

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Vinod, please apply them all!

Yours,
Linus Walleij

[PATCH 00/24] ste_dma40: Fine-tuning for several function implementations

[Vinod Koul]

On Sat, Sep 17, 2016 at 05:05:16PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 17 Sep 2016 16:56:56 +0200
> 
> Several update suggestions were taken into account
> from static source code analysis.

Applied all, thanks

-- 
~Vinod