linux-kernel - Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-ID: <alpine.DEB.2.20.1703021557230.3414@hadrien>
Date:   Thu, 2 Mar 2017 16:01:12 +0100 (CET)
From:   Julia Lawall <julia.lawall@...6.fr>
To:     SIMRAN SINGHAL <singhalsimran0@...il.com>
cc:     outreachy-kernel <outreachy-kernel@...glegroups.com>,
        marvin24@....de, gregkh@...uxfoundation.org,
        ac100@...ts.launchpad.net, linux-tegra@...r.kernel.org,
        devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE
 checkpatch checks



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...@...il.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...@...glegroups.com.
>       > To post to this group, send email to
>       outreach...@...glegroups.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@...glegroups.com.
> To post to this group, send email to outreachy-kernel@...glegroups.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.
>
>