2018-12-16 17:01:03

by Amir Mahdi Ghorbanian

[permalink] [raw]
Subject: [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 <[email protected]>
---
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(&nvec->tx_data, struct nvec_msg, node);
spin_unlock_irqrestore(&nvec->tx_lock, flags);
nvec_gpio_set_value(nvec, 0);
- err = wait_for_completion_interruptible_timeout(
- &nvec->ec_transfer, msecs_to_jiffies(5000));
+ done = &nvec->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



2018-12-16 17:58:37

by Joe Perches

[permalink] [raw]
Subject: 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(&nvec->tx_data, struct nvec_msg, node);
> spin_unlock_irqrestore(&nvec->tx_lock, flags);
> nvec_gpio_set_value(nvec, 0);
> - err = wait_for_completion_interruptible_timeout(
> - &nvec->ec_transfer, msecs_to_jiffies(5000));
> + done = &nvec->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.



2018-12-17 01:44:41

by kernel test robot

[permalink] [raw]
Subject: 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 = &nvec->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 &struct work_struct (the tx_worker member of &struct 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(&nvec->tx_lock, flags);
381 while (!list_empty(&nvec->tx_data)) {
382 msg = list_first_entry(&nvec->tx_data, struct nvec_msg, node);
383 spin_unlock_irqrestore(&nvec->tx_lock, flags);
384 nvec_gpio_set_value(nvec, 0);
> 385 done = &nvec->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(&nvec->tx_lock, flags);
397
398 if (err > 0) {
399 list_del_init(&msg->node);
400 nvec_msg_free(nvec, msg);
401 }
402 }
403 spin_unlock_irqrestore(&nvec->tx_lock, flags);
404 }
405

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation


Attachments:
(No filename) (3.09 kB)
.config.gz (66.27 kB)
Download all attachments

2018-12-17 08:21:46

by Greg Kroah-Hartman

[permalink] [raw]
Subject: 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

2018-12-17 20:45:52

by Marc Dietrich

[permalink] [raw]
Subject: 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