Fix style in s3c-hsotg.c

Fix style in s3c-hsotg.c

[Pavel Machek]

Hi!

checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
macro with if should be really enclosed in do {} while, and puts is
going to be slightly faster.

Here's suggested patch. I don't have the hardware, so it is completely
untested.

Signed-off-by: Pavel Machek, <pavel@denx.de>

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index af22f24..f8e762a 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -2091,12 +2091,14 @@ static void kill_all_requests(struct s3c_hsotg *hsotg,
 }
 
 #define call_gadget(_hs, _entry) \
+do { \
 	if ((_hs)->gadget.speed != USB_SPEED_UNKNOWN &&	\
 	    (_hs)->driver && (_hs)->driver->_entry) { \
 		spin_unlock(&_hs->lock); \
 		(_hs)->driver->_entry(&(_hs)->gadget); \
 		spin_lock(&_hs->lock); \
 		}
+} while (0)
 
 /**
  * s3c_hsotg_disconnect - disconnect service
@@ -3204,7 +3206,7 @@ static int state_show(struct seq_file *seq, void *v)
 		   readl(regs + GNPTXSTS),
 		   readl(regs + GRXSTSR));
 
-	seq_printf(seq, "\nEndpoint status:\n");
+	seq_puts(seq, "\nEndpoint status:\n");
 
 	for (idx = 0; idx < 15; idx++) {
 		u32 in, out;
@@ -3221,7 +3223,7 @@ static int state_show(struct seq_file *seq, void *v)
 		seq_printf(seq, ", DIEPTSIZ=0x%08x, DOEPTSIZ=0x%08x",
 			   in, out);
 
-		seq_printf(seq, "\n");
+		seq_puts(seq, "\n");
 	}
 
 	return 0;
@@ -3255,7 +3257,7 @@ static int fifo_show(struct seq_file *seq, void *v)
 	u32 val;
 	int idx;
 
-	seq_printf(seq, "Non-periodic FIFOs:\n");
+	seq_puts(seq, "Non-periodic FIFOs:\n");
 	seq_printf(seq, "RXFIFO: Size %d\n", readl(regs + GRXFSIZ));
 
 	val = readl(regs + GNPTXFSIZ);
@@ -3263,7 +3265,7 @@ static int fifo_show(struct seq_file *seq, void *v)
 		   val >> GNPTXFSIZ_NPTxFDep_SHIFT,
 		   val & GNPTXFSIZ_NPTxFStAddr_MASK);
 
-	seq_printf(seq, "\nPeriodic TXFIFOs:\n");
+	seq_puts(seq, "\nPeriodic TXFIFOs:\n");
 
 	for (idx = 1; idx <= 15; idx++) {
 		val = readl(regs + DPTXFSIZn(idx));
@@ -3334,7 +3336,7 @@ static int ep_show(struct seq_file *seq, void *v)
 		   readl(regs + DIEPTSIZ(index)),
 		   readl(regs + DOEPTSIZ(index)));
 
-	seq_printf(seq, "\n");
+	seq_puts(seq, "\n");
 	seq_printf(seq, "mps %d\n", ep->ep.maxpacket);
 	seq_printf(seq, "total_data=%ld\n", ep->total_data);
 
@@ -3345,7 +3347,7 @@ static int ep_show(struct seq_file *seq, void *v)
 
 	list_for_each_entry(req, &ep->queue, queue) {
 		if (--show_limit < 0) {
-			seq_printf(seq, "not showing more requests...\n");
+			seq_puts(seq, "not showing more requests...\n");
 			break;
 		}
 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Fix style in s3c-hsotg.c

[Sergei Shtylyov]

Hello.

On 09/02/2013 05:58 PM, Pavel Machek wrote:

> checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
> macro with if should be really enclosed in do {} while, and puts is
> going to be slightly faster.

> Here's suggested patch. I don't have the hardware, so it is completely
> untested.

> Signed-off-by: Pavel Machek, <pavel@denx.de>

> diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
> index af22f24..f8e762a 100644
> --- a/drivers/usb/gadget/s3c-hsotg.c
> +++ b/drivers/usb/gadget/s3c-hsotg.c
> @@ -2091,12 +2091,14 @@ static void kill_all_requests(struct s3c_hsotg *hsotg,
>   }
>
>   #define call_gadget(_hs, _entry) \
> +do { \
>   	if ((_hs)->gadget.speed != USB_SPEED_UNKNOWN &&	\
>   	    (_hs)->driver && (_hs)->driver->_entry) { \
>   		spin_unlock(&_hs->lock); \
>   		(_hs)->driver->_entry(&(_hs)->gadget); \
>   		spin_lock(&_hs->lock); \
>   		}

    Indentation also needs fixing on this line, maybe in another patch...

> +} while (0)

WBR, Sergei

Re: Fix style in s3c-hsotg.c

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 495 bytes --]

Hi,

On Mon, Sep 02, 2013 at 03:58:32PM +0200, Pavel Machek wrote:
> Hi!
> 
> checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
> macro with if should be really enclosed in do {} while, and puts is
> going to be slightly faster.
> 
> Here's suggested patch. I don't have the hardware, so it is completely
> untested.
> 
> Signed-off-by: Pavel Machek, <pavel@denx.de>

this is not how you send a patch, please read
Documentation/SubmittingPatches

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Fix style in s3c-hsotg.c

[Pavel Machek]

On Tue 2013-09-17 10:42:30, Felipe Balbi wrote:
> Hi,
> 
> On Mon, Sep 02, 2013 at 03:58:32PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
> > macro with if should be really enclosed in do {} while, and puts is
> > going to be slightly faster.
> > 
> > Here's suggested patch. I don't have the hardware, so it is completely
> > untested.
> > 
> > Signed-off-by: Pavel Machek, <pavel@denx.de>
> 
> this is not how you send a patch, please read
> Documentation/SubmittingPatches

Have you considered possibility that this is how you nudge maintainer
into fix their coding style?

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Fix style in s3c-hsotg.c

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 902 bytes --]

On Wed, Sep 18, 2013 at 12:04:27AM +0200, Pavel Machek wrote:
> On Tue 2013-09-17 10:42:30, Felipe Balbi wrote:
> > Hi,
> > 
> > On Mon, Sep 02, 2013 at 03:58:32PM +0200, Pavel Machek wrote:
> > > Hi!
> > > 
> > > checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
> > > macro with if should be really enclosed in do {} while, and puts is
> > > going to be slightly faster.
> > > 
> > > Here's suggested patch. I don't have the hardware, so it is completely
> > > untested.
> > > 
> > > Signed-off-by: Pavel Machek, <pavel@denx.de>
> > 
> > this is not how you send a patch, please read
> > Documentation/SubmittingPatches
> 
> Have you considered possibility that this is how you nudge maintainer
> into fix their coding style?

cute...

Seriously though, read that file, you're commit log has garbage in it
which shouldn't go to git history.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Fix style in s3c-hsotg.c

[Pavel Machek]

On Tue 2013-09-17 20:45:39, Felipe Balbi wrote:
> On Wed, Sep 18, 2013 at 12:04:27AM +0200, Pavel Machek wrote:
> > On Tue 2013-09-17 10:42:30, Felipe Balbi wrote:
> > > Hi,
> > > 
> > > On Mon, Sep 02, 2013 at 03:58:32PM +0200, Pavel Machek wrote:
> > > > Hi!
> > > > 
> > > > checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
> > > > macro with if should be really enclosed in do {} while, and puts is
> > > > going to be slightly faster.
> > > > 
> > > > Here's suggested patch. I don't have the hardware, so it is completely
> > > > untested.
> > > > 
> > > > Signed-off-by: Pavel Machek, <pavel@denx.de>
> > > 
> > > this is not how you send a patch, please read
> > > Documentation/SubmittingPatches
> > 
> > Have you considered possibility that this is how you nudge maintainer
> > into fix their coding style?
> 
> cute...
> 
> Seriously though, read that file, you're commit log has garbage in it
> which shouldn't go to git history.

Run git log on SubmittingPatches.

Then, instead of telling me what to read, run checkpatch on your
files. You can either fix them yourself, or use my patch as a
basis. Note there's missing } or something, so it probably will not
compile, see the other mail. So you actually will have to modify that
patch. Stripping "Hi!" from it should not be that hard, neither should
be stripping note that patch is untested when you actually test
it. And as you are the maintainer, it is your job.

No, I'll not polish patch for hardware I don't have and have little
interest in.  wanted to help you, but according to your first reply,
you do not really want help.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Fix style in s3c-hsotg.c

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

Hi,

On Wed, Sep 18, 2013 at 11:20:27AM +0200, Pavel Machek wrote:
> On Tue 2013-09-17 20:45:39, Felipe Balbi wrote:
> > On Wed, Sep 18, 2013 at 12:04:27AM +0200, Pavel Machek wrote:
> > > On Tue 2013-09-17 10:42:30, Felipe Balbi wrote:
> > > > Hi,
> > > > 
> > > > On Mon, Sep 02, 2013 at 03:58:32PM +0200, Pavel Machek wrote:
> > > > > Hi!
> > > > > 
> > > > > checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
> > > > > macro with if should be really enclosed in do {} while, and puts is
> > > > > going to be slightly faster.
> > > > > 
> > > > > Here's suggested patch. I don't have the hardware, so it is completely
> > > > > untested.
> > > > > 
> > > > > Signed-off-by: Pavel Machek, <pavel@denx.de>
> > > > 
> > > > this is not how you send a patch, please read
> > > > Documentation/SubmittingPatches
> > > 
> > > Have you considered possibility that this is how you nudge maintainer
> > > into fix their coding style?
> > 
> > cute...
> > 
> > Seriously though, read that file, you're commit log has garbage in it
> > which shouldn't go to git history.
> 
> Run git log on SubmittingPatches.
> 
> Then, instead of telling me what to read, run checkpatch on your
> files. You can either fix them yourself, or use my patch as a
> basis. Note there's missing } or something, so it probably will not
> compile, see the other mail. So you actually will have to modify that
> patch. Stripping "Hi!" from it should not be that hard, neither should
> be stripping note that patch is untested when you actually test
> it. And as you are the maintainer, it is your job.

you misunderstand the work of maintainers. Our work is not to fix you
crappy patches. If we start allowing crappy patches, we'd be fixing your
nonsense forever and linux wouldn't move.

> No, I'll not polish patch for hardware I don't have and have little
> interest in.  wanted to help you, but according to your first reply,
> you do not really want help.

that's your call. Now how about you stop being such a baby and go fix
your mistakes to start with ? Just because I'm the maintainer of the
gadget framework, doesn't mean I'm the maintainer of s3c-hsotc.c file.
Maintainer != author too, btw.

Anyway, I got much better stuff to do than babysitting grown ups.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Fix style in s3c-hsotg.c

[Bartlomiej Zolnierkiewicz]

Hi,

On Wednesday, September 18, 2013 09:11:06 AM Felipe Balbi wrote:
> Hi,
> 
> On Wed, Sep 18, 2013 at 11:20:27AM +0200, Pavel Machek wrote:
> > On Tue 2013-09-17 20:45:39, Felipe Balbi wrote:
> > > On Wed, Sep 18, 2013 at 12:04:27AM +0200, Pavel Machek wrote:
> > > > On Tue 2013-09-17 10:42:30, Felipe Balbi wrote:
> > > > > Hi,
> > > > > 
> > > > > On Mon, Sep 02, 2013 at 03:58:32PM +0200, Pavel Machek wrote:
> > > > > > Hi!
> > > > > > 
> > > > > > checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
> > > > > > macro with if should be really enclosed in do {} while, and puts is
> > > > > > going to be slightly faster.
> > > > > > 
> > > > > > Here's suggested patch. I don't have the hardware, so it is completely
> > > > > > untested.
> > > > > > 
> > > > > > Signed-off-by: Pavel Machek, <pavel@denx.de>
> > > > > 
> > > > > this is not how you send a patch, please read
> > > > > Documentation/SubmittingPatches
> > > > 
> > > > Have you considered possibility that this is how you nudge maintainer
> > > > into fix their coding style?
> > > 
> > > cute...
> > > 
> > > Seriously though, read that file, you're commit log has garbage in it
> > > which shouldn't go to git history.
> > 
> > Run git log on SubmittingPatches.
> > 
> > Then, instead of telling me what to read, run checkpatch on your
> > files. You can either fix them yourself, or use my patch as a
> > basis. Note there's missing } or something, so it probably will not
> > compile, see the other mail. So you actually will have to modify that
> > patch. Stripping "Hi!" from it should not be that hard, neither should
> > be stripping note that patch is untested when you actually test
> > it. And as you are the maintainer, it is your job.
> 
> you misunderstand the work of maintainers. Our work is not to fix you
> crappy patches. If we start allowing crappy patches, we'd be fixing your
> nonsense forever and linux wouldn't move.

Pavel, Felipe's remark was valid..

> > No, I'll not polish patch for hardware I don't have and have little
> > interest in.  wanted to help you, but according to your first reply,
> > you do not really want help.
> 
> that's your call. Now how about you stop being such a baby and go fix
> your mistakes to start with ? Just because I'm the maintainer of the
> gadget framework, doesn't mean I'm the maintainer of s3c-hsotc.c file.
> Maintainer != author too, btw.
> 
> Anyway, I got much better stuff to do than babysitting grown ups.

..even if it could have been communicated in a gentler way.

Anyway s3c-hsotg.c is a driver for our hardware so I'm going to pick this
patch up, polish it and then re-submit it later. Thanks.

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

Re: Fix style in s3c-hsotg.c

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1140 bytes --]

Hi,

On Wed, Sep 25, 2013 at 05:39:39PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Pavel, Felipe's remark was valid..
> 
> > > No, I'll not polish patch for hardware I don't have and have little
> > > interest in.  wanted to help you, but according to your first reply,
> > > you do not really want help.
> > 
> > that's your call. Now how about you stop being such a baby and go fix
> > your mistakes to start with ? Just because I'm the maintainer of the
> > gadget framework, doesn't mean I'm the maintainer of s3c-hsotc.c file.
> > Maintainer != author too, btw.
> > 
> > Anyway, I got much better stuff to do than babysitting grown ups.
> 
> ..even if it could have been communicated in a gentler way.
> 
> Anyway s3c-hsotg.c is a driver for our hardware so I'm going to pick this
> patch up, polish it and then re-submit it later. Thanks.

I tried a few times being nice and the response was always in the lines
of "you should go fix the patch yourself if you don't like it" (not
exactly these words, but same message). After some time of being nice
and get nowhere, I just got fed up.

cheers

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Fix style in s3c-hsotg.c

[Pavel Machek]

Hi!

> > > No, I'll not polish patch for hardware I don't have and have little
> > > interest in.  wanted to help you, but according to your first reply,
> > > you do not really want help.
> > 
> > that's your call. Now how about you stop being such a baby and go fix
> > your mistakes to start with ? Just because I'm the maintainer of the
> > gadget framework, doesn't mean I'm the maintainer of s3c-hsotc.c file.
> > Maintainer != author too, btw.
> > 
> > Anyway, I got much better stuff to do than babysitting grown ups.
> 
> ..even if it could have been communicated in a gentler way.
> 
> Anyway s3c-hsotg.c is a driver for our hardware so I'm going to pick this
> patch up, polish it and then re-submit it later. Thanks.

Here's version that should compile. Thanks,
									Pavel

Signed-off-by: Pavel Machek <pavel@ucw.cz>

diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index d69b36a..c8cdde0 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -2092,12 +2092,14 @@ static void kill_all_requests(struct s3c_hsotg *hsotg,
 }
 
 #define call_gadget(_hs, _entry) \
+do { \
 	if ((_hs)->gadget.speed != USB_SPEED_UNKNOWN &&	\
 	    (_hs)->driver && (_hs)->driver->_entry) { \
 		spin_unlock(&_hs->lock); \
 		(_hs)->driver->_entry(&(_hs)->gadget); \
 		spin_lock(&_hs->lock); \
-		}
+		} \
+} while (0)
 
 /**
  * s3c_hsotg_disconnect - disconnect service
@@ -3205,7 +3207,7 @@ static int state_show(struct seq_file *seq, void *v)
 		   readl(regs + GNPTXSTS),
 		   readl(regs + GRXSTSR));
 
-	seq_printf(seq, "\nEndpoint status:\n");
+	seq_puts(seq, "\nEndpoint status:\n");
 
 	for (idx = 0; idx < 15; idx++) {
 		u32 in, out;
@@ -3222,7 +3224,7 @@ static int state_show(struct seq_file *seq, void *v)
 		seq_printf(seq, ", DIEPTSIZ=0x%08x, DOEPTSIZ=0x%08x",
 			   in, out);
 
-		seq_printf(seq, "\n");
+		seq_puts(seq, "\n");
 	}
 
 	return 0;
@@ -3256,7 +3258,7 @@ static int fifo_show(struct seq_file *seq, void *v)
 	u32 val;
 	int idx;
 
-	seq_printf(seq, "Non-periodic FIFOs:\n");
+	seq_puts(seq, "Non-periodic FIFOs:\n");
 	seq_printf(seq, "RXFIFO: Size %d\n", readl(regs + GRXFSIZ));
 
 	val = readl(regs + GNPTXFSIZ);
@@ -3264,7 +3266,7 @@ static int fifo_show(struct seq_file *seq, void *v)
 		   val >> GNPTXFSIZ_NPTxFDep_SHIFT,
 		   val & GNPTXFSIZ_NPTxFStAddr_MASK);
 
-	seq_printf(seq, "\nPeriodic TXFIFOs:\n");
+	seq_puts(seq, "\nPeriodic TXFIFOs:\n");
 
 	for (idx = 1; idx <= 15; idx++) {
 		val = readl(regs + DPTXFSIZn(idx));
@@ -3335,7 +3337,7 @@ static int ep_show(struct seq_file *seq, void *v)
 		   readl(regs + DIEPTSIZ(index)),
 		   readl(regs + DOEPTSIZ(index)));
 
-	seq_printf(seq, "\n");
+	seq_puts(seq, "\n");
 	seq_printf(seq, "mps %d\n", ep->ep.maxpacket);
 	seq_printf(seq, "total_data=%ld\n", ep->total_data);
 
@@ -3346,7 +3348,7 @@ static int ep_show(struct seq_file *seq, void *v)
 
 	list_for_each_entry(req, &ep->queue, queue) {
 		if (--show_limit < 0) {
-			seq_printf(seq, "not showing more requests...\n");
+			seq_puts(seq, "not showing more requests...\n");
 			break;
 		}
 


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: Fix style in s3c-hsotg.c

[Pavel Machek]

Hi!

> > Pavel, Felipe's remark was valid..
> > 
> > > > No, I'll not polish patch for hardware I don't have and have little
> > > > interest in.  wanted to help you, but according to your first reply,
> > > > you do not really want help.
> > > 
> > > that's your call. Now how about you stop being such a baby and go fix
> > > your mistakes to start with ? Just because I'm the maintainer of the
> > > gadget framework, doesn't mean I'm the maintainer of s3c-hsotc.c file.
> > > Maintainer != author too, btw.
> > > 
> > > Anyway, I got much better stuff to do than babysitting grown ups.
> > 
> > ..even if it could have been communicated in a gentler way.
> > 
> > Anyway s3c-hsotg.c is a driver for our hardware so I'm going to pick this
> > patch up, polish it and then re-submit it later. Thanks.
> 
> I tried a few times being nice and the response was always in the lines
> of "you should go fix the patch yourself if you don't like it" (not
> exactly these words, but same message). After some time of being nice
> and get nowhere, I just got fed up.

For the record, this is Felipe's idea of being nice:

> > > > > this is not how you send a patch, please read
> > > > > Documentation/SubmittingPatches

(What an useless reply. Yes, I know how to submit patches. I also know
that patches need to be tested, so I was not interested in "proper"
submission. I tried to explain the difference between submit patch for
inclusion and make someone look at the patch and fix the code...)

> > > > Have you considered possibility that this is how you nudge maintainer
> > > > into fix their coding style?

(Felipe)
> > > cute...
> > > 
> > > Seriously though, read that file, you're commit log has garbage in it
> > > which shouldn't go to git history.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html