[PATCH 2/2] Staging: nvec: nvec: fixed check style issues

[PATCH 2/2] Staging: nvec: nvec: fixed check style issues

[Amir Mahdi Ghorbanian]

Fixed an endline open parenthesis issue and replaced udelay() by the
preferred usleep_range() function.

Signed-off-by: Amir Mahdi Ghorbanian <indigoomega021@gmail.com>
---
 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

Re: [PATCH 2/2] Staging: nvec: nvec: fixed check style issues

[Joe Perches]

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.

Re: [PATCH 2/2] Staging: nvec: nvec: fixed check style issues

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 3082 bytes --]

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

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67862 bytes --]

Re: [PATCH 2/2] Staging: nvec: nvec: fixed check style issues

[Greg KH]

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

Re: [PATCH 2/2] Staging: nvec: nvec: fixed check style issues

[Marc Dietrich]

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