Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks

[Julia Lawall]

On Thu, 2 Mar 2017, SIMRAN SINGHAL wrote:

>
>
> On Thursday, March 2, 2017 at 8:31:23 PM UTC+5:30, Julia Lawall wrote:
>
>
>       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...@xxxxxxxxx>
>       >       > ---
>       >       >  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@xxxxxxxxxxxxxxxxx/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.
>
>  
> Thanks, 
> Julia I'll keep this in mind from next time.
>
> I choose the range from 33 to 100 for being on more safer side.
> Should I make it 30-40 and send v2.

I thought that the maintainer already responded saying that one shouldnot
change this code?

julia

>
>       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...@xxxxxxxxxxxxxxxxx
>       >       > To post to this group, send email to
>       >       outreach...@xxxxxxxxxxxxxxxxx
>       >       > To view this discussion on the webvisithttps://groups.google.com/d/msgid/outreachy-kernel/20170302142418.GA16
>       773%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-kern...@xxxxxxxxxxxxxxxxx
>       > To post to this group, send email to
>       outreach...@xxxxxxxxxxxxxxxxx
>       > To view this discussion on the webvisithttps://groups.google.com/d/msgid/outreachy-kernel/b90bc602-cf06-4abb-
>       bea2-
>       > 6386d4976864%40googlegroups.com.
>       > 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@xxxxxxxxxxxxxxxxx
> To post to this group, send email to outreachy-kernel@xxxxxxxxxxxxxxxxx
> To view this discussion on the web visithttps://groups.google.com/d/msgid/outreachy-kernel/4958c8a8-4b50-4567-91d8-
> 9554e9bdf7f6%40googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
>