All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julia Lawall <julia.lawall@lip6.fr>
To: SIMRAN SINGHAL <singhalsimran0@gmail.com>
Cc: devel@driverdev.osuosl.org, gregkh@linuxfoundation.org,
	linux-kernel@vger.kernel.org,
	outreachy-kernel <outreachy-kernel@googlegroups.com>,
	linux-tegra@vger.kernel.org, ac100@lists.launchpad.net
Subject: Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks
Date: Thu, 2 Mar 2017 16:01:12 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.20.1703021557230.3414@hadrien> (raw)
In-Reply-To: <b90bc602-cf06-4abb-bea2-6386d4976864@googlegroups.com>

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



On Thu, 2 Mar 2017, SIMRAN SINGHAL wrote:

>
>
> On Thursday, March 2, 2017 at 8:06:40 PM UTC+5:30, Julia Lawall wrote:
>
>
>       On Thu, 2 Mar 2017, simran singhal wrote:
>
>       > Resolve strict checkpatch USLEEP_RANGE checks by converting
>       delays and
>       > sleeps as described in
>       ./Documentation/timers/timers-howto.txt.
>       >
>       > CHECK: usleep_range is preferred over udelay; see
>       Documentation/
>       > timers/timers-howto.txt
>       >
>       > Signed-off-by: simran singhal <singhal...@gmail.com>
>       > ---
>       >  drivers/staging/nvec/nvec.c | 4 ++--
>       >  1 file changed, 2 insertions(+), 2 deletions(-)
>       >
>       > diff --git a/drivers/staging/nvec/nvec.c
>       b/drivers/staging/nvec/nvec.c
>       > index c1feccf..cd35e64 100644
>       > --- a/drivers/staging/nvec/nvec.c
>       > +++ b/drivers/staging/nvec/nvec.c
>       > @@ -631,7 +631,7 @@ static irqreturn_t nvec_interrupt(int irq,
>       void *dev)
>       >                  break;
>       >          case 2:                /* first byte after command */
>       >                  if (status == (I2C_SL_IRQ | RNW | RCVD)) {
>       > -                        udelay(33);
>       > +                        usleep_range(33, 100);
>
>       How did you choose the upper limit.
>
>       I believe that Greg previously suggested not to make these
>       changes if you
>       have no way to test them.
>
> Julia, After going through the reply given by Nicholas Mc Guire 
> https://www.mail-archive.com/kernelnewbies@kernelnewbies.org/msg16464.html
> in this reply he has mentioned that even the range of 10 microsecond is
> enough,
> so I prefer to take 100 as upper limit.  

Than you for the link.

It looks like he suggests to change 33 to 30-40, not to 33-100.  In any
case, you have three choices for this kind of issue:
1. Don't make the change, because you can't test the result
2. Make the change, and explain the commit log what your rationale is
3. Make the change, and explain below the --- that you have no idea what
you are doing, and you are just proposing the patch as something concrete
to start a discussion.

But your preference is not a suitable justification.  The hardware does
something, and the choice can only really be made by the person who knows
what it does.

julia

>  
> Simran
>
>       julia
>
>
>       >                          if (nvec->rx->data[0] != 0x01) {
>       >                                  dev_err(nvec->dev,
>       >                                          "Read without prior
>       read command\n");
>       > @@ -718,7 +718,7 @@ static irqreturn_t nvec_interrupt(int irq,
>       void *dev)
>       >           * We experience less incomplete messages with this
>       delay than without
>       >           * it, but we don't know why. Help is appreciated.
>       >           */
>       > -        udelay(100);
>       > +        usleep_range(100, 200);
>       >
>       >          return IRQ_HANDLED;
>       >  }
>       > --
>       > 2.7.4
>       >
>       > --
>       > You received this message because you are subscribed to the
>       Google Groups "outreachy-kernel" group.
>       > To unsubscribe from this group and stop receiving emails from
>       it, send an email to outreachy-kern...@googlegroups.com.
>       > To post to this group, send email to
>       outreach...@googlegroups.com.
>       > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/20170302142418.GA16773%4
>       0singhal-Inspiron-5558.
>       > For more options, visit https://groups.google.com/d/optout.
>       >
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/b90bc602-cf06-4abb-bea2-
> 6386d4976864%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

[-- Attachment #2: Type: text/plain, Size: 169 bytes --]

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

WARNING: multiple messages have this Message-ID (diff)
From: Julia Lawall <julia.lawall@lip6.fr>
To: SIMRAN SINGHAL <singhalsimran0@gmail.com>
Cc: outreachy-kernel <outreachy-kernel@googlegroups.com>,
	marvin24@gmx.de,  gregkh@linuxfoundation.org,
	ac100@lists.launchpad.net,  linux-tegra@vger.kernel.org,
	devel@driverdev.osuosl.org,  linux-kernel@vger.kernel.org
Subject: Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks
Date: Thu, 2 Mar 2017 16:01:12 +0100 (CET)	[thread overview]
Message-ID: <alpine.DEB.2.20.1703021557230.3414@hadrien> (raw)
In-Reply-To: <b90bc602-cf06-4abb-bea2-6386d4976864@googlegroups.com>

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



On Thu, 2 Mar 2017, SIMRAN SINGHAL wrote:

>
>
> On Thursday, March 2, 2017 at 8:06:40 PM UTC+5:30, Julia Lawall wrote:
>
>
>       On Thu, 2 Mar 2017, simran singhal wrote:
>
>       > Resolve strict checkpatch USLEEP_RANGE checks by converting
>       delays and
>       > sleeps as described in
>       ./Documentation/timers/timers-howto.txt.
>       >
>       > CHECK: usleep_range is preferred over udelay; see
>       Documentation/
>       > timers/timers-howto.txt
>       >
>       > Signed-off-by: simran singhal <singhal...@gmail.com>
>       > ---
>       >  drivers/staging/nvec/nvec.c | 4 ++--
>       >  1 file changed, 2 insertions(+), 2 deletions(-)
>       >
>       > diff --git a/drivers/staging/nvec/nvec.c
>       b/drivers/staging/nvec/nvec.c
>       > index c1feccf..cd35e64 100644
>       > --- a/drivers/staging/nvec/nvec.c
>       > +++ b/drivers/staging/nvec/nvec.c
>       > @@ -631,7 +631,7 @@ static irqreturn_t nvec_interrupt(int irq,
>       void *dev)
>       >                  break;
>       >          case 2:                /* first byte after command */
>       >                  if (status == (I2C_SL_IRQ | RNW | RCVD)) {
>       > -                        udelay(33);
>       > +                        usleep_range(33, 100);
>
>       How did you choose the upper limit.
>
>       I believe that Greg previously suggested not to make these
>       changes if you
>       have no way to test them.
>
> Julia, After going through the reply given by Nicholas Mc Guire 
> https://www.mail-archive.com/kernelnewbies@kernelnewbies.org/msg16464.html
> in this reply he has mentioned that even the range of 10 microsecond is
> enough,
> so I prefer to take 100 as upper limit.  

Than you for the link.

It looks like he suggests to change 33 to 30-40, not to 33-100.  In any
case, you have three choices for this kind of issue:
1. Don't make the change, because you can't test the result
2. Make the change, and explain the commit log what your rationale is
3. Make the change, and explain below the --- that you have no idea what
you are doing, and you are just proposing the patch as something concrete
to start a discussion.

But your preference is not a suitable justification.  The hardware does
something, and the choice can only really be made by the person who knows
what it does.

julia

>  
> Simran
>
>       julia
>
>
>       >                          if (nvec->rx->data[0] != 0x01) {
>       >                                  dev_err(nvec->dev,
>       >                                          "Read without prior
>       read command\n");
>       > @@ -718,7 +718,7 @@ static irqreturn_t nvec_interrupt(int irq,
>       void *dev)
>       >           * We experience less incomplete messages with this
>       delay than without
>       >           * it, but we don't know why. Help is appreciated.
>       >           */
>       > -        udelay(100);
>       > +        usleep_range(100, 200);
>       >
>       >          return IRQ_HANDLED;
>       >  }
>       > --
>       > 2.7.4
>       >
>       > --
>       > You received this message because you are subscribed to the
>       Google Groups "outreachy-kernel" group.
>       > To unsubscribe from this group and stop receiving emails from
>       it, send an email to outreachy-kern...@googlegroups.com.
>       > To post to this group, send email to
>       outreach...@googlegroups.com.
>       > To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/20170302142418.GA16773%4
>       0singhal-Inspiron-5558.
>       > For more options, visit https://groups.google.com/d/optout.
>       >
>
> --
> You received this message because you are subscribed to the Google Groups
> "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/b90bc602-cf06-4abb-bea2-
> 6386d4976864%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
>

  parent reply	other threads:[~2017-03-02 15:01 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-02 14:24 [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks simran singhal
2017-03-02 14:24 ` simran singhal
2017-03-02 14:36 ` [Outreachy kernel] " Julia Lawall
2017-03-02 14:36   ` Julia Lawall
2017-03-02 14:48   ` SIMRAN SINGHAL
2017-03-02 14:57     ` [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGEcheckpatch checks Marc Dietrich
2017-03-02 14:57       ` Marc Dietrich
2017-03-02 15:28       ` SIMRAN SINGHAL
2017-03-08 12:46       ` Thierry Reding
2017-03-08 12:46         ` Thierry Reding
2017-03-02 15:01     ` Julia Lawall [this message]
2017-03-02 15:01       ` [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks Julia Lawall
2017-03-02 15:17       ` SIMRAN SINGHAL
2017-03-02 15:18         ` Julia Lawall
2017-03-02 15:18           ` Julia Lawall
2017-03-02 15:18           ` Julia Lawall

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.20.1703021557230.3414@hadrien \
    --to=julia.lawall@lip6.fr \
    --cc=ac100@lists.launchpad.net \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=outreachy-kernel@googlegroups.com \
    --cc=singhalsimran0@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.