Re: [PATCH 2/2] Staging: nvec: nvec: fixed check style issues
Hi,
On Mon, 17 Dec 2018, Greg KH wrote:
On Sun, Dec 16, 2018 at 08:57:43AM -0800, Amir Mahdi Ghorbanian wrote:
@@ -626,7 +628,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(0, 33);
Why is this a valid range to sleep for for this device? Have you been
able to verify/test this?
oh no, not again. Why does this come up again every half year? This udelay
is a workaround for a hw bug which only seldom triggers (if it triggers at
all). Secondly, this is in interrupt context, so *sleep timers are no go,
afaik.
Marc
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] Staging: nvec: nvec: fixed check style issues
On Sun, Dec 16, 2018 at 08:57:43AM -0800, Amir Mahdi Ghorbanian wrote:
> @@ -626,7 +628,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(0, 33);
Why is this a valid range to sleep for for this device? Have you been
able to verify/test this?
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] Staging: nvec: nvec: fixed check style issues
Hi Amir,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on staging/staging-testing]
[also build test ERROR on v4.20-rc7 next-20181214]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Amir-Mahdi-Ghorbanian/Staging-comedi-cb_pcidas-fixed-a-spelling-mistake-coding-style-issue/20181217-032106
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.2.0 make.cross ARCH=arm
All errors (new ones prefixed by >>):
drivers/staging/nvec/nvec.c: In function 'nvec_request_master':
>> drivers/staging/nvec/nvec.c:385:3: error: 'done' undeclared (first use in
>> this function); did you mean 'zone'?
done = >ec_transfer;
^~~~
zone
drivers/staging/nvec/nvec.c:385:3: note: each undeclared identifier is
reported only once for each function it appears in
>> drivers/staging/nvec/nvec.c:386:3: error: 'timeout' undeclared (first use in
>> this function); did you mean 'time_t'?
timeout = msecs_to_jiffies(5000);
^~~
time_t
vim +385 drivers/staging/nvec/nvec.c
364
365 /**
366 * nvec_request_master - Process outgoing messages
367 * @work: A work_struct (the tx_worker member of
nvec_chip)
368 *
369 * Processes all outgoing requests by sending the request and awaiting
the
370 * response, then continuing with the next request. Once a request has a
371 * matching response, it will be freed and removed from the list.
372 */
373 static void nvec_request_master(struct work_struct *work)
374 {
375 struct nvec_chip *nvec = container_of(work, struct nvec_chip,
tx_work);
376 unsigned long flags;
377 long err;
378 struct nvec_msg *msg;
379
380 spin_lock_irqsave(>tx_lock, flags);
381 while (!list_empty(>tx_data)) {
382 msg = list_first_entry(>tx_data, struct nvec_msg,
node);
383 spin_unlock_irqrestore(>tx_lock, flags);
384 nvec_gpio_set_value(nvec, 0);
> 385 done = >ec_transfer;
> 386 timeout = msecs_to_jiffies(5000);
387 err = wait_for_completion_interruptible_timeout(done,
388
timeout);
389
390 if (err == 0) {
391 dev_warn(nvec->dev, "timeout waiting for ec
transfer\n");
392 nvec_gpio_set_value(nvec, 1);
393 msg->pos = 0;
394 }
395
396 spin_lock_irqsave(>tx_lock, flags);
397
398 if (err > 0) {
399 list_del_init(>node);
400 nvec_msg_free(nvec, msg);
401 }
402 }
403 spin_unlock_irqrestore(>tx_lock, flags);
404 }
405
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] Staging: nvec: nvec: fixed check style issues
On Sun, 2018-12-16 at 08:57 -0800, Amir Mahdi Ghorbanian wrote: > Fixed an endline open parenthesis issue and replaced udelay() by the > preferred usleep_range() function. Not all checkpatch bleats need to be fixed. Function names with 40+ character length identifiers makes fitting within an 80 column limit nearly impossible. > diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c [] > @@ -382,8 +382,10 @@ static void nvec_request_master(struct work_struct *work) > msg = list_first_entry(>tx_data, struct nvec_msg, node); > spin_unlock_irqrestore(>tx_lock, flags); > nvec_gpio_set_value(nvec, 0); > - err = wait_for_completion_interruptible_timeout( > - >ec_transfer, msecs_to_jiffies(5000)); > + done = >ec_transfer; > + timeout = msecs_to_jiffies(5000); > + err = wait_for_completion_interruptible_timeout(done, > + timeout); This was easier to read without the temporaries. And where are done and timeout declared? Please compile your patches before submitting them. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] Staging: nvec: nvec: fixed check style issues
Fixed an endline open parenthesis issue and replaced udelay() by the
preferred usleep_range() function.
Signed-off-by: Amir Mahdi Ghorbanian
---
drivers/staging/nvec/nvec.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/nvec/nvec.c b/drivers/staging/nvec/nvec.c
index 08027a3..69eef7d 100644
--- a/drivers/staging/nvec/nvec.c
+++ b/drivers/staging/nvec/nvec.c
@@ -382,8 +382,10 @@ static void nvec_request_master(struct work_struct *work)
msg = list_first_entry(>tx_data, struct nvec_msg, node);
spin_unlock_irqrestore(>tx_lock, flags);
nvec_gpio_set_value(nvec, 0);
- err = wait_for_completion_interruptible_timeout(
- >ec_transfer, msecs_to_jiffies(5000));
+ done = >ec_transfer;
+ timeout = msecs_to_jiffies(5000);
+ err = wait_for_completion_interruptible_timeout(done,
+ timeout);
if (err == 0) {
dev_warn(nvec->dev, "timeout waiting for ec
transfer\n");
@@ -626,7 +628,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(0, 33);
if (nvec->rx->data[0] != 0x01) {
dev_err(nvec->dev,
"Read without prior read command\n");
@@ -713,7 +715,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(0, 100);
return IRQ_HANDLED;
}
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
