| Message ID | 1457259436-32560-1-git-send-email-krzk@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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(-)