* [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks @ 2017-03-02 14:24 ` simran singhal 0 siblings, 0 replies; 16+ messages in thread From: simran singhal @ 2017-03-02 14:24 UTC (permalink / raw) To: marvin24 Cc: devel, gregkh, linux-kernel, outreachy-kernel, linux-tegra, ac100 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 <singhalsimran0@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); 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks @ 2017-03-02 14:24 ` simran singhal 0 siblings, 0 replies; 16+ messages in thread From: simran singhal @ 2017-03-02 14:24 UTC (permalink / raw) To: marvin24 Cc: gregkh, ac100, linux-tegra, devel, linux-kernel, outreachy-kernel 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 <singhalsimran0@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); 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 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks 2017-03-02 14:24 ` simran singhal @ 2017-03-02 14:36 ` Julia Lawall -1 siblings, 0 replies; 16+ messages in thread From: Julia Lawall @ 2017-03-02 14:36 UTC (permalink / raw) To: simran singhal Cc: devel, gregkh, linux-kernel, outreachy-kernel, linux-tegra, ac100 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 <singhalsimran0@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 > 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-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170302142418.GA16773%40singhal-Inspiron-5558. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks @ 2017-03-02 14:36 ` Julia Lawall 0 siblings, 0 replies; 16+ messages in thread From: Julia Lawall @ 2017-03-02 14:36 UTC (permalink / raw) To: simran singhal Cc: marvin24, gregkh, ac100, linux-tegra, devel, linux-kernel, outreachy-kernel 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 <singhalsimran0@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 > 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-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170302142418.GA16773%40singhal-Inspiron-5558. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks 2017-03-02 14:36 ` Julia Lawall (?) @ 2017-03-02 14:48 ` SIMRAN SINGHAL 2017-03-02 14:57 ` Marc Dietrich 2017-03-02 15:01 ` Julia Lawall -1 siblings, 2 replies; 16+ messages in thread From: SIMRAN SINGHAL @ 2017-03-02 14:48 UTC (permalink / raw) To: outreachy-kernel Cc: singhalsimran0, marvin24, gregkh, ac100, linux-tegra, devel, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 2692 bytes --] 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 <javascript:>> > > --- > > 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. 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 <javascript:>. > > To post to this group, send email to outreach...@googlegroups.com > <javascript:>. > > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/20170302142418.GA16773%40singhal-Inspiron-5558. > > > For more options, visit https://groups.google.com/d/optout. > > > [-- Attachment #1.2: Type: text/html, Size: 4894 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGEcheckpatch checks 2017-03-02 14:48 ` SIMRAN SINGHAL @ 2017-03-02 14:57 ` Marc Dietrich 2017-03-02 15:01 ` Julia Lawall 1 sibling, 0 replies; 16+ messages in thread From: Marc Dietrich @ 2017-03-02 14:57 UTC (permalink / raw) To: SIMRAN SINGHAL, linux-tegra; +Cc: devel, outreachy-kernel, linux-kernel, gregkh [-- Attachment #1.1: Type: text/plain, Size: 3289 bytes --] Hi Simran, Am Donnerstag, 2. März 2017, 15:48:13 CET schrieb SIMRAN SINGHAL: > 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 <javascript:>> I prefer not to change this. The whole interrupt routine is very wonky, and changing some delays might break the communication with the i2c master. Also this is in interrupt context, so a change to usleep_range may not by justified. Marc > > > --- > > > > > > 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. > > 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; > > > > > > } > > > > Groups "outreachy-kernel" group. > > > > > To unsubscribe from this group and stop receiving emails from it, send > > > > an email to outreachy-kern...@googlegroups.com <javascript:>. > > > > > To post to this group, send email to outreach...@googlegroups.com > > > > <javascript:>. > > > > > To view this discussion on the web visit > > > > https://groups.google.com/d/msgid/outreachy-kernel/20170302142418.GA16773% > > 40singhal-Inspiron-5558.> > > > For more options, visit https://groups.google.com/d/optout. [-- Attachment #1.2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 488 bytes --] [-- Attachment #2: Type: text/plain, Size: 169 bytes --] _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGEcheckpatch checks @ 2017-03-02 14:57 ` Marc Dietrich 0 siblings, 0 replies; 16+ messages in thread From: Marc Dietrich @ 2017-03-02 14:57 UTC (permalink / raw) To: SIMRAN SINGHAL, linux-tegra; +Cc: outreachy-kernel, gregkh, devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 3289 bytes --] Hi Simran, Am Donnerstag, 2. März 2017, 15:48:13 CET schrieb SIMRAN SINGHAL: > 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 <javascript:>> I prefer not to change this. The whole interrupt routine is very wonky, and changing some delays might break the communication with the i2c master. Also this is in interrupt context, so a change to usleep_range may not by justified. Marc > > > --- > > > > > > 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. > > 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; > > > > > > } > > > > Groups "outreachy-kernel" group. > > > > > To unsubscribe from this group and stop receiving emails from it, send > > > > an email to outreachy-kern...@googlegroups.com <javascript:>. > > > > > To post to this group, send email to outreach...@googlegroups.com > > > > <javascript:>. > > > > > To view this discussion on the web visit > > > > https://groups.google.com/d/msgid/outreachy-kernel/20170302142418.GA16773% > > 40singhal-Inspiron-5558.> > > > For more options, visit https://groups.google.com/d/optout. [-- Attachment #2: This is a digitally signed message part. --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGEcheckpatch checks 2017-03-02 14:57 ` Marc Dietrich (?) @ 2017-03-02 15:28 ` SIMRAN SINGHAL -1 siblings, 0 replies; 16+ messages in thread From: SIMRAN SINGHAL @ 2017-03-02 15:28 UTC (permalink / raw) To: outreachy-kernel Cc: singhalsimran0, linux-tegra, gregkh, devel, linux-kernel, marvin24 [-- Attachment #1.1: Type: text/plain, Size: 3713 bytes --] On Thursday, March 2, 2017 at 8:31:53 PM UTC+5:30, Marc Dietrich wrote: > > Hi Simran, > > Am Donnerstag, 2. März 2017, 15:48:13 CET schrieb SIMRAN SINGHAL: > > 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 <javascript:>> > > I prefer not to change this. The whole interrupt routine is very wonky, > and > changing some delays might break the communication with the i2c master. > Also > this is in interrupt context, so a change to usleep_range may not by > justified. > > Thanks, for the explination. Simran > Marc > > > > > > > --- > > > > > > > > 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. > > > > 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; > > > > > > > > } > > > > > > Groups "outreachy-kernel" group. > > > > > > > To unsubscribe from this group and stop receiving emails from it, > send > > > > > > an email to outreachy-kern...@googlegroups.com <javascript:>. > > > > > > > To post to this group, send email to outreach...@googlegroups.com > > > > > > <javascript:>. > > > > > > > To view this discussion on the web visit > > > > > > > https://groups.google.com/d/msgid/outreachy-kernel/20170302142418.GA16773% > > > 40singhal-Inspiron-5558.> > > > > For more options, visit https://groups.google.com/d/optout. > > [-- Attachment #1.2: Type: text/html, Size: 6250 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGEcheckpatch checks 2017-03-02 14:57 ` Marc Dietrich @ 2017-03-08 12:46 ` Thierry Reding -1 siblings, 0 replies; 16+ messages in thread From: Thierry Reding @ 2017-03-08 12:46 UTC (permalink / raw) To: Marc Dietrich Cc: devel, gregkh, linux-kernel, outreachy-kernel, linux-tegra, SIMRAN SINGHAL [-- Attachment #1.1: Type: text/plain, Size: 1121 bytes --] On Thu, Mar 02, 2017 at 03:57:01PM +0100, Marc Dietrich wrote: > Hi Simran, > > Am Donnerstag, 2. März 2017, 15:48:13 CET schrieb SIMRAN SINGHAL: > > 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 <javascript:>> > > I prefer not to change this. The whole interrupt routine is very wonky, and > changing some delays might break the communication with the i2c master. Also > this is in interrupt context, so a change to usleep_range may not by > justified. Yeah, I think this is going to trigger a WARN_ON from somewhere in the scheduler because of the interrupt context. I suppose checkpatch could be made smarter about this, though I doubt my perl skills would be up to it. Thierry [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] [-- Attachment #2: Type: text/plain, Size: 169 bytes --] _______________________________________________ devel mailing list devel@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGEcheckpatch checks @ 2017-03-08 12:46 ` Thierry Reding 0 siblings, 0 replies; 16+ messages in thread From: Thierry Reding @ 2017-03-08 12:46 UTC (permalink / raw) To: Marc Dietrich Cc: SIMRAN SINGHAL, linux-tegra, outreachy-kernel, gregkh, devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 1121 bytes --] On Thu, Mar 02, 2017 at 03:57:01PM +0100, Marc Dietrich wrote: > Hi Simran, > > Am Donnerstag, 2. März 2017, 15:48:13 CET schrieb SIMRAN SINGHAL: > > 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 <javascript:>> > > I prefer not to change this. The whole interrupt routine is very wonky, and > changing some delays might break the communication with the i2c master. Also > this is in interrupt context, so a change to usleep_range may not by > justified. Yeah, I think this is going to trigger a WARN_ON from somewhere in the scheduler because of the interrupt context. I suppose checkpatch could be made smarter about this, though I doubt my perl skills would be up to it. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks 2017-03-02 14:48 ` SIMRAN SINGHAL @ 2017-03-02 15:01 ` Julia Lawall 2017-03-02 15:01 ` Julia Lawall 1 sibling, 0 replies; 16+ messages in thread From: Julia Lawall @ 2017-03-02 15:01 UTC (permalink / raw) To: SIMRAN SINGHAL Cc: devel, gregkh, linux-kernel, outreachy-kernel, linux-tegra, ac100 [-- 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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks @ 2017-03-02 15:01 ` Julia Lawall 0 siblings, 0 replies; 16+ messages in thread From: Julia Lawall @ 2017-03-02 15:01 UTC (permalink / raw) To: SIMRAN SINGHAL Cc: outreachy-kernel, marvin24, gregkh, ac100, linux-tegra, devel, linux-kernel [-- 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. > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks 2017-03-02 15:01 ` Julia Lawall (?) @ 2017-03-02 15:17 ` SIMRAN SINGHAL 2017-03-02 15:18 ` Julia Lawall -1 siblings, 1 reply; 16+ messages in thread From: SIMRAN SINGHAL @ 2017-03-02 15:17 UTC (permalink / raw) To: outreachy-kernel Cc: singhalsimran0, marvin24, gregkh, ac100, linux-tegra, devel, linux-kernel [-- Attachment #1.1: Type: text/plain, Size: 4770 bytes --] 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...@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. > > 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. 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-kern...@googlegroups.com <javascript:>. > > To post to this group, send email to outreach...@googlegroups.com > <javascript:>. > > 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 #1.2: Type: text/html, Size: 8971 bytes --] ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks 2017-03-02 15:17 ` SIMRAN SINGHAL 2017-03-02 15:18 ` Julia Lawall @ 2017-03-02 15:18 ` Julia Lawall 0 siblings, 0 replies; 16+ messages in thread From: Julia Lawall @ 2017-03-02 15:18 UTC (permalink / raw) To: SIMRAN SINGHAL Cc: devel, gregkh, linux-kernel, outreachy-kernel, linux-tegra, ac100 [-- Attachment #1: Type: text/plain, Size: 6170 bytes --] 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...@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. > > > 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...@googlegroups.com. > > > To post to this group, send email to > > outreach...@googlegroups.com. > > > 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...@googlegroups.com. > > To post to this group, send email to > outreach...@googlegroups.com. > > 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@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/4958c8a8-4b50-4567-91d8- > 9554e9bdf7f6%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 ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks @ 2017-03-02 15:18 ` Julia Lawall 0 siblings, 0 replies; 16+ messages in thread From: Julia Lawall @ 2017-03-02 15:18 UTC (permalink / raw) To: SIMRAN SINGHAL Cc: outreachy-kernel, marvin24, gregkh, ac100, linux-tegra, devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 7074 bytes --] 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...@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. > > ￜ > 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...@googlegroups.com. > > ￜ ￜ ￜ > To post to this group, send email to > > ￜ ￜ ￜ outreach...@googlegroups.com. > > ￜ ￜ ￜ > 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...@googlegroups.com. > > To post to this group, send email to > outreach...@googlegroups.com. > > 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@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/4958c8a8-4b50-4567-91d8- > 9554e9bdf7f6%40googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks @ 2017-03-02 15:18 ` Julia Lawall 0 siblings, 0 replies; 16+ messages in thread From: Julia Lawall @ 2017-03-02 15:18 UTC (permalink / raw) To: SIMRAN SINGHAL Cc: outreachy-kernel, marvin24, gregkh, ac100, linux-tegra, devel, linux-kernel [-- Attachment #1: Type: text/plain, Size: 6170 bytes --] 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...@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. > > > 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...@googlegroups.com. > > > To post to this group, send email to > > outreach...@googlegroups.com. > > > 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...@googlegroups.com. > > To post to this group, send email to > outreach...@googlegroups.com. > > 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@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/4958c8a8-4b50-4567-91d8- > 9554e9bdf7f6%40googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > > ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-03-08 12:54 UTC | newest] Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 ` [Outreachy kernel] [PATCH] staging: nvec: cleanup USLEEP_RANGE checkpatch checks Julia Lawall 2017-03-02 15:01 ` 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
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.