Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1351075 > unrolled thread

[PATCH 1/2] crypto: s5p-sss - Minor coding cleanups

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] crypto: s5p-sss - Minor coding cleanups Krzysztof Kozlowski <krzk@kernel.org> - 2016-03-06 11:20 +0100
    Re: [PATCH 1/2] crypto: s5p-sss - Minor coding cleanups Vladimir Zapolskiy <vz@mleia.com> - 2016-03-07 02:10 +0100
      Re: [PATCH 1/2] crypto: s5p-sss - Minor coding cleanups Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-03-07 05:10 +0100
        Re: [PATCH 1/2] crypto: s5p-sss - Minor coding cleanups Vladimir Zapolskiy <vz@mleia.com> - 2016-03-10 00:50 +0100

#1351075 — [PATCH 1/2] crypto: s5p-sss - Minor coding cleanups

Remove unneeded inclusion of delay.h and get rid of indentation from
labels.

Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
---
 drivers/crypto/s5p-sss.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 5f161a9777e3..60f835455a41 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -11,7 +11,6 @@
  *
  */
 
-#include <linux/delay.h>
 #include <linux/err.h>
 #include <linux/module.h>
 #include <linux/init.h>
@@ -284,7 +283,7 @@ static int s5p_set_outdata(struct s5p_aes_dev *dev, struct scatterlist *sg)
 	dev->sg_dst = sg;
 	err = 0;
 
- exit:
+exit:
 	return err;
 }
 
@@ -310,7 +309,7 @@ static int s5p_set_indata(struct s5p_aes_dev *dev, struct scatterlist *sg)
 	dev->sg_src = sg;
 	err = 0;
 
- exit:
+exit:
 	return err;
 }
 
@@ -452,10 +451,10 @@ static void s5p_aes_crypt_start(struct s5p_aes_dev *dev, unsigned long mode)
 
 	return;
 
- outdata_error:
+outdata_error:
 	s5p_unset_indata(dev);
 
- indata_error:
+indata_error:
 	s5p_aes_complete(dev, err);
 	spin_unlock_irqrestore(&dev->lock, flags);
 }
@@ -506,7 +505,7 @@ static int s5p_aes_handle_req(struct s5p_aes_dev *dev,
 
 	tasklet_schedule(&dev->tasklet);
 
- exit:
+exit:
 	return err;
 }
 
@@ -705,7 +704,7 @@ static int s5p_aes_probe(struct platform_device *pdev)
 
 	return 0;
 
- err_algs:
+err_algs:
 	dev_err(dev, "can't register '%s': %d\n", algs[i].cra_name, err);
 
 	for (j = 0; j < i; j++)
@@ -713,7 +712,7 @@ static int s5p_aes_probe(struct platform_device *pdev)
 
 	tasklet_kill(&pdata->tasklet);
 
- err_irq:
+err_irq:
 	clk_disable_unprepare(pdata->clk);
 
 	s5p_dev = NULL;
-- 
2.1.4

[toc] | [next] | [standalone]

#1351235

Hi Krzysztof,

On 06.03.2016 12:17, Krzysztof Kozlowski wrote:
> Remove unneeded inclusion of delay.h and get rid of indentation from
> labels.

just in case are you aware of this discussion regarding goto labels
http://lkml.iu.edu/hypermail/linux/kernel/0706.0/0809.html ?

Practically there is no big need (neither it is a clean-up IMHO)
to change column 1 indentation of goto labels, however I do agree that
^[a-z0-9_]\+:$ style of goto labels is 10 times more commonly used,
and that means it's a good time to update my ~/.emacs config.

Someone may want to fix the style of another 5000+ goto labels one day,
though it is better to have an officially agreed policy beforehand.

Anyway I do not strictly object against the change, see one my comment
below and feel free to add my

Acked-by: Vladimir Zapolskiy <vz@mleia.com>

> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
> ---
>  drivers/crypto/s5p-sss.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
> index 5f161a9777e3..60f835455a41 100644
> --- a/drivers/crypto/s5p-sss.c
> +++ b/drivers/crypto/s5p-sss.c
> @@ -11,7 +11,6 @@
>   *
>   */
>  
> -#include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/module.h>
>  #include <linux/init.h>

Since you are here, can you sort headers out, please?

And I'm pretty sure some more inclusions can be safely removed
from the list.

> @@ -284,7 +283,7 @@ static int s5p_set_outdata(struct s5p_aes_dev *dev, struct scatterlist *sg)
>  	dev->sg_dst = sg;
>  	err = 0;
>  
> - exit:
> +exit:
>  	return err;
>  }
>  
> @@ -310,7 +309,7 @@ static int s5p_set_indata(struct s5p_aes_dev *dev, struct scatterlist *sg)
>  	dev->sg_src = sg;
>  	err = 0;
>  
> - exit:
> +exit:
>  	return err;
>  }
>  
> @@ -452,10 +451,10 @@ static void s5p_aes_crypt_start(struct s5p_aes_dev *dev, unsigned long mode)
>  
>  	return;
>  
> - outdata_error:
> +outdata_error:
>  	s5p_unset_indata(dev);
>  
> - indata_error:
> +indata_error:
>  	s5p_aes_complete(dev, err);
>  	spin_unlock_irqrestore(&dev->lock, flags);
>  }
> @@ -506,7 +505,7 @@ static int s5p_aes_handle_req(struct s5p_aes_dev *dev,
>  
>  	tasklet_schedule(&dev->tasklet);
>  
> - exit:
> +exit:
>  	return err;
>  }
>  
> @@ -705,7 +704,7 @@ static int s5p_aes_probe(struct platform_device *pdev)
>  
>  	return 0;
>  
> - err_algs:
> +err_algs:
>  	dev_err(dev, "can't register '%s': %d\n", algs[i].cra_name, err);
>  
>  	for (j = 0; j < i; j++)
> @@ -713,7 +712,7 @@ static int s5p_aes_probe(struct platform_device *pdev)
>  
>  	tasklet_kill(&pdata->tasklet);
>  
> - err_irq:
> +err_irq:
>  	clk_disable_unprepare(pdata->clk);
>  
>  	s5p_dev = NULL;
> 

--
With best wishes,
Vladimir

[toc] | [prev] | [next] | [standalone]

#1351301

On 07.03.2016 10:01, Vladimir Zapolskiy wrote:
> Hi Krzysztof,
> 
> On 06.03.2016 12:17, Krzysztof Kozlowski wrote:
>> Remove unneeded inclusion of delay.h and get rid of indentation from
>> labels.
> 
> just in case are you aware of this discussion regarding goto labels
> http://lkml.iu.edu/hypermail/linux/kernel/0706.0/0809.html ?
> 
> Practically there is no big need (neither it is a clean-up IMHO)
> to change column 1 indentation of goto labels, however I do agree that
> ^[a-z0-9_]\+:$ style of goto labels is 10 times more commonly used,
> and that means it's a good time to update my ~/.emacs config.
> 
> Someone may want to fix the style of another 5000+ goto labels one day,
> though it is better to have an officially agreed policy beforehand.
> 
> Anyway I do not strictly object against the change, see one my comment
> below and feel free to add my
> 
> Acked-by: Vladimir Zapolskiy <vz@mleia.com>

Actually I was not aware of that discussion, thanks for pointing this
out. Mostly I encountered the style without indentation but of course
that does not mean it is the only one.

> 
>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>> ---
>>  drivers/crypto/s5p-sss.c | 15 +++++++--------
>>  1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
>> index 5f161a9777e3..60f835455a41 100644
>> --- a/drivers/crypto/s5p-sss.c
>> +++ b/drivers/crypto/s5p-sss.c
>> @@ -11,7 +11,6 @@
>>   *
>>   */
>>  
>> -#include <linux/delay.h>
>>  #include <linux/err.h>
>>  #include <linux/module.h>
>>  #include <linux/init.h>
> 
> Since you are here, can you sort headers out, please?

You mean sort by name (or by christmas tree)? I can... I am not sure
about benefits of such patch (which should be the motivation for change).


> And I'm pretty sure some more inclusions can be safely removed
> from the list.

I checked them while removing delay - all of them seem to be used.

Best regards,
Krzysztof

[toc] | [prev] | [next] | [standalone]

#1354677

Hi Krzysztof,

On 07.03.2016 06:05, Krzysztof Kozlowski wrote:
> On 07.03.2016 10:01, Vladimir Zapolskiy wrote:
>> Hi Krzysztof,
>>
>> On 06.03.2016 12:17, Krzysztof Kozlowski wrote:
>>> Remove unneeded inclusion of delay.h and get rid of indentation from
>>> labels.
>>
>> just in case are you aware of this discussion regarding goto labels
>> http://lkml.iu.edu/hypermail/linux/kernel/0706.0/0809.html ?
>>
>> Practically there is no big need (neither it is a clean-up IMHO)
>> to change column 1 indentation of goto labels, however I do agree that
>> ^[a-z0-9_]\+:$ style of goto labels is 10 times more commonly used,
>> and that means it's a good time to update my ~/.emacs config.
>>
>> Someone may want to fix the style of another 5000+ goto labels one day,
>> though it is better to have an officially agreed policy beforehand.
>>
>> Anyway I do not strictly object against the change, see one my comment
>> below and feel free to add my
>>
>> Acked-by: Vladimir Zapolskiy <vz@mleia.com>
> 
> Actually I was not aware of that discussion, thanks for pointing this
> out. Mostly I encountered the style without indentation but of course
> that does not mean it is the only one.
> 
>>
>>> Signed-off-by: Krzysztof Kozlowski <krzk@kernel.org>
>>> ---
>>>  drivers/crypto/s5p-sss.c | 15 +++++++--------
>>>  1 file changed, 7 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
>>> index 5f161a9777e3..60f835455a41 100644
>>> --- a/drivers/crypto/s5p-sss.c
>>> +++ b/drivers/crypto/s5p-sss.c
>>> @@ -11,7 +11,6 @@
>>>   *
>>>   */
>>>  
>>> -#include <linux/delay.h>
>>>  #include <linux/err.h>
>>>  #include <linux/module.h>
>>>  #include <linux/init.h>
>>
>> Since you are here, can you sort headers out, please?
> 
> You mean sort by name (or by christmas tree)? I can... 

Thank you in advance.

> I am not sure
> about benefits of such patch (which should be the motivation for change).

In general it simplifies navigation through the list of headers for
humans IMHO.

By example, let's look at drivers/media/usb/dvb-usb/dw2102.c :

#include "dw2102.h"
#include "si21xx.h"
#include "stv0299.h"
#include "z0194a.h"
#include "stv0288.h"
#include "stb6000.h"
#include "eds1547.h"
#include "cx24116.h"
#include "tda1002x.h"
#include "mt312.h"
#include "zl10039.h"
#include "ts2020.h"
#include "ds3000.h"
#include "stv0900.h"
#include "stv6110.h"
#include "stb6100.h"
#include "stb6100_proc.h"
#include "m88rs2000.h"
#include "tda18271.h"
#include "cxd2820r.h"
#include "m88ds3103.h"
#include "ts2020.h"

Was it obvious for an author/reviewers that commit 0fecb6c0944f adds a
duplicate?

What is the chance to add a duplicate, if the list of headers is sorted?

>> And I'm pretty sure some more inclusions can be safely removed
>> from the list.
> 
> I checked them while removing delay - all of them seem to be used.

Good, thank you for checking it :)

--
With best wishes,
Vladimir

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web