[OPW,kernel,v3] staging: silicom: fix space prohibited before semicolon
diff mbox

Message ID 20131002223933.GA22607@gmail.com
State Accepted
Headers show

Commit Message

Tugce Sirin Oct. 2, 2013, 10:39 p.m. UTC
Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>

Reviewed-by: Josh Triplett <josh@joshtriplett.org>

---
 drivers/staging/silicom/bp_mod.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Waskiewicz Jr, Peter P Oct. 2, 2013, 10:53 p.m. UTC | #1
On Thu, 2013-10-03 at 01:39 +0300, Tugce Sirin wrote:
> Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>
> 
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>

This looks fine to me as well.  Thanks for updating the subject line to
include versioning.

-PJ

> ---
>  drivers/staging/silicom/bp_mod.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/silicom/bp_mod.h b/drivers/staging/silicom/bp_mod.h
> index cfa1f43..8154a7b 100644
> --- a/drivers/staging/silicom/bp_mod.h
> +++ b/drivers/staging/silicom/bp_mod.h
> @@ -22,7 +22,7 @@ do {						\
>  	int  i;					\
>  	if (1) {				\
>  		for (i = 0; i < 1000; i++) {	\
> -			udelay(x) ;		\
> +			udelay(x);		\
>  		}				\
>  	} else {				\
>  		msleep(x);			\
> -- 
> 1.7.9.5
>
Greg KH Oct. 2, 2013, 11:22 p.m. UTC | #2
On Thu, Oct 03, 2013 at 01:39:57AM +0300, Tugce Sirin wrote:
> Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>
>
> Reviewed-by: Josh Triplett <josh@joshtriplett.org>

Tiniest nit, there shouldn't be an extra line between these.

Don't worry, I've fixed this and now applied it to my tree, nice job.

greg k-h
Sarah Sharp Oct. 3, 2013, 12:10 a.m. UTC | #3
Hi Tugce,

Can you also submit your application to opw-list@gnome.org, now that you
have your first patch accepted into the kernel?  I know you applied last
round, but we need you to send in an updated application.

Sarah Sharp

On Wed, Oct 02, 2013 at 10:53:00PM +0000, Waskiewicz Jr, Peter P wrote:
> On Thu, 2013-10-03 at 01:39 +0300, Tugce Sirin wrote:
> > Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>
> > 
> > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> 
> This looks fine to me as well.  Thanks for updating the subject line to
> include versioning.
> 
> -PJ
> 
> > ---
> >  drivers/staging/silicom/bp_mod.h |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/silicom/bp_mod.h b/drivers/staging/silicom/bp_mod.h
> > index cfa1f43..8154a7b 100644
> > --- a/drivers/staging/silicom/bp_mod.h
> > +++ b/drivers/staging/silicom/bp_mod.h
> > @@ -22,7 +22,7 @@ do {						\
> >  	int  i;					\
> >  	if (1) {				\
> >  		for (i = 0; i < 1000; i++) {	\
> > -			udelay(x) ;		\
> > +			udelay(x);		\
> >  		}				\
> >  	} else {				\
> >  		msleep(x);			\
> > -- 
> > 1.7.9.5
> > 
> 
> -- 
> You received this message because you are subscribed to the Google Groups "opw-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to opw-kernel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
Zach Brown Oct. 3, 2013, 5:45 a.m. UTC | #4
Hi gang!  Just joined, here goes..

> > > Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>
> > > 
> > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > 
> > This looks fine to me as well.  Thanks for updating the subject line to
> > include versioning.

Indeed, this patch looks fine to me too.

Reviewed-by: Zach Brown <zab@redhat.com>

Is it reasonable and appropriate for opw-kernel to give the review
feedback that the msec_delay_bp() macro that this patch fixes could also
be replaced with mdelay() in its callers?

- z
Greg KH Oct. 3, 2013, 6:08 a.m. UTC | #5
On Wed, Oct 02, 2013 at 10:45:13PM -0700, Zach Brown wrote:
> 
> Hi gang!  Just joined, here goes..
> 
> > > > Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>
> > > > 
> > > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > > 
> > > This looks fine to me as well.  Thanks for updating the subject line to
> > > include versioning.
> 
> Indeed, this patch looks fine to me too.
> 
> Reviewed-by: Zach Brown <zab@redhat.com>
> 
> Is it reasonable and appropriate for opw-kernel to give the review
> feedback that the msec_delay_bp() macro that this patch fixes could also
> be replaced with mdelay() in its callers?

Yes it is.  That's a great place for the original submitter to take on a
bit more of an "advanced" change than just coding style cleanups.

thanks,

greg k-h
Tugce Sirin Oct. 3, 2013, 6:51 p.m. UTC | #6
Umm, actually I couldn't follow the conversation. I understand there can be
another fix but I didn't understand if you were asking me to check on it or
not? Or suggest, perhaps. May you explain it for me, please?


Best regards,
Tugce Sirin


2013/10/3 Greg KH <gregkh@linuxfoundation.org>

> On Wed, Oct 02, 2013 at 10:45:13PM -0700, Zach Brown wrote:
> >
> > Hi gang!  Just joined, here goes..
> >
> > > > > Signed-off-by: Tugce Sirin <ztugcesirin@gmail.com>
> > > > >
> > > > > Reviewed-by: Josh Triplett <josh@joshtriplett.org>
> > > >
> > > > This looks fine to me as well.  Thanks for updating the subject line
> to
> > > > include versioning.
> >
> > Indeed, this patch looks fine to me too.
> >
> > Reviewed-by: Zach Brown <zab@redhat.com>
> >
> > Is it reasonable and appropriate for opw-kernel to give the review
> > feedback that the msec_delay_bp() macro that this patch fixes could also
> > be replaced with mdelay() in its callers?
>
> Yes it is.  That's a great place for the original submitter to take on a
> bit more of an "advanced" change than just coding style cleanups.
>
> thanks,
>
> greg k-h
>
> --
> You received this message because you are subscribed to the Google Groups
> "opw-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to opw-kernel+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
Greg KH Oct. 3, 2013, 10:12 p.m. UTC | #7
A: No.
Q: Should I include quotations after my reply?

http://daringfireball.net/2007/07/on_top

On Thu, Oct 03, 2013 at 09:51:49PM +0300, Tu?├že ?irin wrote:
> Umm, actually I couldn't follow the conversation. I understand there can be
> another fix but I didn't understand if you were asking me to check on it or
> not? Or suggest, perhaps. May you explain it for me, please?

The macro you fixed up, should be removed entirely from the driver and
the kernel function mdelay() should be used instead where that macro was
being called (after making sure the units are all correct.)

Does that help?

greg k-h

Patch
diff mbox

diff --git a/drivers/staging/silicom/bp_mod.h b/drivers/staging/silicom/bp_mod.h
index cfa1f43..8154a7b 100644
--- a/drivers/staging/silicom/bp_mod.h
+++ b/drivers/staging/silicom/bp_mod.h
@@ -22,7 +22,7 @@  do {						\
 	int  i;					\
 	if (1) {				\
 		for (i = 0; i < 1000; i++) {	\
-			udelay(x) ;		\
+			udelay(x);		\
 		}				\
 	} else {				\
 		msleep(x);			\