[PATCH 1/3] Staging: comedi: addi-data: comment cleanup in hwdrv_apci035.c

[PATCH 1/3] Staging: comedi: addi-data: comment cleanup in hwdrv_apci035.c

[Chase Southwood]

This patch further cleans up the comments in hwdrv_apci035.c, converting
them to kernel style and removing some commented conditional statements
that are unused.

Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
---

I decided to return to the first driver I touched.  I found some more
things that could be cleaned up a little.

 .../comedi/drivers/addi-data/hwdrv_apci035.c       | 106 +++++++--------------
 1 file changed, 32 insertions(+), 74 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
index 8ce3335..7c40535 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
@@ -188,17 +188,14 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
 	outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
 	ui_Command = 0;
 	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
-	/************************/
+
 	/* Set the reload value */
-	/************************/
 	outl(data[3], devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 4);
-	/*********************/
+
 	/* Set the time unit */
-	/*********************/
 	outl(data[2], devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 8);
 	if (data[0] == ADDIDATA_TIMER) {
 
-		/******************************/
 		/* Set the mode :             */
 		/* - Disable the hardware     */
 		/* - Disable the counter mode */
@@ -206,23 +203,19 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
 		/* - Disable the reset        */
 		/* - Enable the timer mode    */
 		/* - Set the timer mode       */
-		/******************************/
 
 		ui_Command =
 			(ui_Command & 0xFFF719E2UL) | ui_Mode << 13UL | 0x10UL;
 
-	}			/* if (data[0] == ADDIDATA_TIMER) */
-	else {
+	} else {
 		if (data[0] == ADDIDATA_WATCHDOG) {
 
-			/******************************/
 			/* Set the mode :             */
 			/* - Disable the hardware     */
 			/* - Disable the counter mode */
 			/* - Disable the warning      */
 			/* - Disable the reset        */
 			/* - Disable the timer mode   */
-			/******************************/
 
 			ui_Command = ui_Command & 0xFFF819E2UL;
 
@@ -234,73 +227,61 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
 	outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
 	ui_Command = 0;
 	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
-	/********************************/
+
 	/* Disable the hardware trigger */
-	/********************************/
 	ui_Command = ui_Command & 0xFFFFF89FUL;
 	if (data[4] == ADDIDATA_ENABLE) {
-		/**********************************/
+
 		/* Set the hardware trigger level */
-		/**********************************/
 		ui_Command = ui_Command | (data[5] << 5);
 	}
 	outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
 	ui_Command = 0;
 	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
-	/*****************************/
+
 	/* Disable the hardware gate */
-	/*****************************/
 	ui_Command = ui_Command & 0xFFFFF87FUL;
 	if (data[6] == ADDIDATA_ENABLE) {
-		/*******************************/
+
 		/* Set the hardware gate level */
-		/*******************************/
 		ui_Command = ui_Command | (data[7] << 7);
 	}
 	outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
 	ui_Command = 0;
 	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
-	/*******************************/
+
 	/* Disable the hardware output */
-	/*******************************/
 	ui_Command = ui_Command & 0xFFFFF9FBUL;
-	/*********************************/
+
 	/* Set the hardware output level */
-	/*********************************/
 	ui_Command = ui_Command | (data[8] << 2);
 	outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
 	if (data[9] == ADDIDATA_ENABLE) {
-		/************************/
+
 		/* Set the reload value */
-		/************************/
 		outl(data[11],
 			devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 24);
-		/**********************/
+
 		/* Set the time unite */
-		/**********************/
 		outl(data[10],
 			devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 28);
 	}
 
 	ui_Command = 0;
 	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
-	/*******************************/
+
 	/* Disable the hardware output */
-	/*******************************/
 	ui_Command = ui_Command & 0xFFFFF9F7UL;
-	/*********************************/
+
 	/* Set the hardware output level */
-	/*********************************/
 	ui_Command = ui_Command | (data[12] << 3);
 	outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
-	/*************************************/
-	/**  Enable the watchdog interrupt  **/
-	/*************************************/
+
+	/* Enable the watchdog interrupt */
 	ui_Command = 0;
 	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
-	/*******************************/
+
 	/* Set the interrupt selection */
-	/*******************************/
 	ui_Status = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 16);
 
 	ui_Command = (ui_Command & 0xFFFFF9FDUL) | (data[13] << 1);
@@ -348,19 +329,17 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
 	if (data[0] == 1) {
 		ui_Command =
 			inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
-		/**********************/
+
 		/* Start the hardware */
-		/**********************/
 		ui_Command = (ui_Command & 0xFFFFF9FFUL) | 0x1UL;
 		outl(ui_Command,
 			devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
-	}			/*  if  (data[0]==1) */
+	}
 	if (data[0] == 2) {
 		ui_Command =
 			inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
-		/***************************/
+
 		/* Set the trigger command */
-		/***************************/
 		ui_Command = (ui_Command & 0xFFFFF9FFUL) | 0x200UL;
 		outl(ui_Command,
 			devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
@@ -375,7 +354,7 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
 		*/
 		outl(ui_Command,
 			devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
-	}			/*   if (data[1]==0) */
+	}
 	if (data[0] == 3) {
 		/* stop all Watchdogs */
 		ui_Command = 0;
@@ -463,28 +442,19 @@ static int i_APCI035_ReadTimerWatchdog(struct comedi_device *dev,
 
 	i_WatchdogNbr = insn->unused[0];
 
-	/******************/
 	/* Get the status */
-	/******************/
-
 	ui_Status = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 16);
 
-	/***********************************/
 	/* Get the software trigger status */
-	/***********************************/
-
 	data[0] = ((ui_Status >> 1) & 1);
-	/***********************************/
+
 	/* Get the hardware trigger status */
-	/***********************************/
 	data[1] = ((ui_Status >> 2) & 1);
-	/*********************************/
+
 	/* Get the software clear status */
-	/*********************************/
 	data[2] = ((ui_Status >> 3) & 1);
-	/***************************/
+
 	/* Get the overflow status */
-	/***************************/
 	data[3] = ((ui_Status >> 0) & 1);
 	if (devpriv->b_TimerSelectMode == ADDIDATA_TIMER)
 		data[4] = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 0);
@@ -525,9 +495,8 @@ static int i_APCI035_ConfigAnalogInput(struct comedi_device *dev,
 	devpriv->tsk_Current = current;
 	outl(0x200 | 0, devpriv->iobase + 128 + 0x4);
 	outl(0, devpriv->iobase + 128 + 0);
-	/********************************/
+
 	/* Initialise the warning value */
-	/********************************/
 	outl(0x300 | 0, devpriv->iobase + 128 + 0x4);
 	outl((data[0] << 8), devpriv->iobase + 128 + 0);
 	outl(0x200000UL, devpriv->iobase + 128 + 12);
@@ -564,18 +533,13 @@ static int i_APCI035_ReadAnalogInput(struct comedi_device *dev,
 	struct addi_private *devpriv = dev->private;
 	unsigned int ui_CommandRegister = 0;
 
-	/******************/
 	/*  Set the start */
-	/******************/
 	ui_CommandRegister = 0x80000;
-	/******************************/
+
 	/* Write the command register */
-	/******************************/
 	outl(ui_CommandRegister, devpriv->iobase + 128 + 8);
 
-	/***************************************/
 	/* Read the digital value of the input */
-	/***************************************/
 	data[0] = inl(devpriv->iobase + 128 + 28);
 	return insn->n;
 }
@@ -640,40 +604,34 @@ static void v_APCI035_Interrupt(int irq, void *d)
 		i_WatchdogNbr = i_Flag;
 		i_Flag = i_Flag + 1;
 	}
-	/**************************************/
+
 	/* Read the interrupt status register of temperature Warning */
-	/**************************************/
 	ui_StatusRegister1 = inl(devpriv->iobase + 128 + 16);
-	/**************************************/
-	/* Read the interrupt status register for Watchdog/timer */
-	/**************************************/
 
+	/* Read the interrupt status register for Watchdog/timer */
 	ui_StatusRegister2 =
 		inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 20);
 
 	/* Test if warning relay interrupt */
 	if ((((ui_StatusRegister1) & 0x8) == 0x8)) {
-		/**********************************/
+
 		/* Disable the temperature warning */
-		/**********************************/
 		ui_ReadCommand = inl(devpriv->iobase + 128 + 12);
 		ui_ReadCommand = ui_ReadCommand & 0xFFDF0000UL;
 		outl(ui_ReadCommand, devpriv->iobase + 128 + 12);
-		/***************************/
+
 		/* Read the channel number */
-		/***************************/
 		ui_ChannelNumber = inl(devpriv->iobase + 128 + 60);
-		/**************************************/
+
 		/* Read the digital temperature value */
-		/**************************************/
 		ui_DigitalTemperature = inl(devpriv->iobase + 128 + 60);
 		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
-	}			/* if (((ui_StatusRegister1 & 0x8) == 0x8)) */
+	}
 
 	else {
 		if ((ui_StatusRegister2 & 0x1) == 0x1)
 			send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
-	}			/* else if (((ui_StatusRegister1 & 0x8) == 0x8)) */
+	}
 
 	return;
 }
-- 
1.8.5.3

[PATCH 2/3] Staging: comedi: addi-data: cleanup conditional blocks in hwdrv_apci035.c

[Chase Southwood]

There were some conditional blocks that had an unneccesary level of
indentation in them.  We can remove this to improve code clarity.

Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
---
 .../comedi/drivers/addi-data/hwdrv_apci035.c       | 31 ++++++++++------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
index 7c40535..34e5321 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
@@ -207,23 +207,22 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
 		ui_Command =
 			(ui_Command & 0xFFF719E2UL) | ui_Mode << 13UL | 0x10UL;
 
-	} else {
-		if (data[0] == ADDIDATA_WATCHDOG) {
+	} else if (data[0] == ADDIDATA_WATCHDOG) {
 
-			/* Set the mode :             */
-			/* - Disable the hardware     */
-			/* - Disable the counter mode */
-			/* - Disable the warning      */
-			/* - Disable the reset        */
-			/* - Disable the timer mode   */
+		/* Set the mode :             */
+		/* - Disable the hardware     */
+		/* - Disable the counter mode */
+		/* - Disable the warning      */
+		/* - Disable the reset        */
+		/* - Disable the timer mode   */
 
-			ui_Command = ui_Command & 0xFFF819E2UL;
+		ui_Command = ui_Command & 0xFFF819E2UL;
 
-		} else {
-			dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
-			return -EINVAL;
-		}
+	} else {
+		dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
+		return -EINVAL;
 	}
+
 	outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
 	ui_Command = 0;
 	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
@@ -628,10 +627,8 @@ static void v_APCI035_Interrupt(int irq, void *d)
 		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
 	}
 
-	else {
-		if ((ui_StatusRegister2 & 0x1) == 0x1)
-			send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
-	}
+	else if ((ui_StatusRegister2 & 0x1) == 0x1)
+		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
 
 	return;
 }
-- 
1.8.5.3

[PATCH 3/3] Staging: comedi: addi-data: fix a couple of lines that are too long

[Chase Southwood]

There are a couple of cases where a comment being on the same line as a
statement is causing the line to be over 80 characters long.  This is an
easy fix; move these comments to the previous line.

Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
---
 drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
index 34e5321..d5c4e71 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
@@ -565,7 +565,9 @@ static int i_APCI035_Reset(struct comedi_device *dev)
 
 	for (i_Count = 1; i_Count <= 4; i_Count++) {
 		i_WatchdogNbr = i_Count;
-		outl(0x0, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 0);	/* stop all timers */
+
+		/* stop all timers */
+		outl(0x0, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 0);
 	}
 	outl(0x0, devpriv->iobase + 128 + 12);	/* Disable the warning delay */
 
@@ -624,11 +626,14 @@ static void v_APCI035_Interrupt(int irq, void *d)
 
 		/* Read the digital temperature value */
 		ui_DigitalTemperature = inl(devpriv->iobase + 128 + 60);
-		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
+
+		/*  send signal to the sample */
+		send_sig(SIGIO, devpriv->tsk_Current, 0);
 	}
 
 	else if ((ui_StatusRegister2 & 0x1) == 0x1)
-		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
+		/*  send signal to the sample */
+		send_sig(SIGIO, devpriv->tsk_Current, 0);
 
 	return;
 }
-- 
1.8.5.3

Re: [PATCH 1/3] Staging: comedi: addi-data: comment cleanup in hwdrv_apci035.c

[Ian Abbott]

On 2014-02-22 03:20, Chase Southwood wrote:
> This patch further cleans up the comments in hwdrv_apci035.c, converting
> them to kernel style and removing some commented conditional statements
> that are unused.
>
> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
> ---
>
> I decided to return to the first driver I touched.  I found some more
> things that could be cleaned up a little.

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

Re: [PATCH 2/3] Staging: comedi: addi-data: cleanup conditional blocks in hwdrv_apci035.c

[Ian Abbott]

On 2014-02-22 03:21, Chase Southwood wrote:
> There were some conditional blocks that had an unneccesary level of
> indentation in them.  We can remove this to improve code clarity.
>
> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
> ---
>   .../comedi/drivers/addi-data/hwdrv_apci035.c       | 31 ++++++++++------------
>   1 file changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> index 7c40535..34e5321 100644
> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> @@ -207,23 +207,22 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
>   		ui_Command =
>   			(ui_Command & 0xFFF719E2UL) | ui_Mode << 13UL | 0x10UL;
>
> -	} else {
> -		if (data[0] == ADDIDATA_WATCHDOG) {
> +	} else if (data[0] == ADDIDATA_WATCHDOG) {
>
> -			/* Set the mode :             */
> -			/* - Disable the hardware     */
> -			/* - Disable the counter mode */
> -			/* - Disable the warning      */
> -			/* - Disable the reset        */
> -			/* - Disable the timer mode   */
> +		/* Set the mode :             */
> +		/* - Disable the hardware     */
> +		/* - Disable the counter mode */
> +		/* - Disable the warning      */
> +		/* - Disable the reset        */
> +		/* - Disable the timer mode   */
>
> -			ui_Command = ui_Command & 0xFFF819E2UL;
> +		ui_Command = ui_Command & 0xFFF819E2UL;
>
> -		} else {
> -			dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
> -			return -EINVAL;
> -		}
> +	} else {
> +		dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
> +		return -EINVAL;
>   	}
> +
>   	outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
>   	ui_Command = 0;
>   	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
> @@ -628,10 +627,8 @@ static void v_APCI035_Interrupt(int irq, void *d)
>   		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
>   	}
>
> -	else {
> -		if ((ui_StatusRegister2 & 0x1) == 0x1)
> -			send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
> -	}
> +	else if ((ui_StatusRegister2 & 0x1) == 0x1)
> +		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */

The usual style there is to put the "else" or the "else if" on the same 
line as the preceding closing brace.

Also, if any branch in the if, else if, ..., else chain uses braces, 
they all should (see the bit that says "This does not apply if only one 
branch of a conditional statement is a single statement; in the latter 
case use braces in both branches:" in the CodingStyle).

>
>   	return;
>   }
>


-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

Re: [PATCH 3/3] Staging: comedi: addi-data: fix a couple of lines that are too long

[Ian Abbott]

On 2014-02-22 03:21, Chase Southwood wrote:
> There are a couple of cases where a comment being on the same line as a
> statement is causing the line to be over 80 characters long.  This is an
> easy fix; move these comments to the previous line.
>
> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

Re: [PATCH 2/3] Staging: comedi: addi-data: cleanup conditional blocks in hwdrv_apci035.c

[Chase Southwood]

>On Monday, February 24, 2014 8:13 AM, Ian Abbott <abbotti@mev.co.uk> wrote:

>>On 2014-02-22 03:21, Chase Southwood wrote:
>> There were some conditional blocks that had an unneccesary level of
>> indentation in them.  We can remove this to improve code clarity.
>>
>> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
>> ---
>>   .../comedi/drivers/addi-data/hwdrv_apci035.c       | 31 ++++++++++------------
>>   1 file changed, 14 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
>> index 7c40535..34e5321 100644
>> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
>> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
>> @@ -207,23 +207,22 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
>>           ui_Command =
>>               (ui_Command & 0xFFF719E2UL) | ui_Mode << 13UL | 0x10UL;
>>
>> -    } else {
>> -        if (data[0] == ADDIDATA_WATCHDOG) {
>> +    } else if (data[0] == ADDIDATA_WATCHDOG) {
>>
>> -            /* Set the mode :             */
>> -            /* - Disable the hardware     */
>> -            /* - Disable the counter mode */
>> -            /* - Disable the warning      */
>> -            /* - Disable the reset        */
>> -            /* - Disable the timer mode   */
>> +        /* Set the mode :             */
>> +        /* - Disable the hardware     */
>> +        /* - Disable the counter mode */
>> +        /* - Disable the warning      */
>> +        /* - Disable the reset        */
>> +        /* - Disable the timer mode   */
>>
>> -            ui_Command = ui_Command & 0xFFF819E2UL;
>> +        ui_Command = ui_Command & 0xFFF819E2UL;
>>
>> -        } else {
>> -            dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
>> -            return -EINVAL;
>> -        }
>> +    } else {
>> +        dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
>> +        return -EINVAL;
>>       }
>> +
>>       outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
>>       ui_Command = 0;
>>       ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
>> @@ -628,10 +627,8 @@ static void v_APCI035_Interrupt(int irq, void *d)
>>           send_sig(SIGIO, devpriv->tsk_Current, 0);    /*  send signal to the sample */
>>       }
>>
>> -    else {
>> -        if ((ui_StatusRegister2 & 0x1) == 0x1)
>> -            send_sig(SIGIO, devpriv->tsk_Current, 0);    /*  send signal to the sample */
>> -    }
>> +    else if ((ui_StatusRegister2 & 0x1) == 0x1)
>> +        send_sig(SIGIO, devpriv->tsk_Current, 0);    /*  send signal to the sample */>
>
>The usual style there is to put the "else" or the "else if" on the same 
>line as the preceding closing brace.
>
>Also, if any branch in the if, else if, ..., else chain uses braces, 
>they all should (see the bit that says "This does not apply if only one 
>branch of a conditional statement is a single statement; in the latter 
>case use braces in both branches:" in the CodingStyle).
>

Ah geez, don't know how that else if placement slipped through the cracks...my mistake.  I've got a revision coming straight away that moves it to the proper place and puts back my overzealous brace deletion.

>
>>
>>       return;>>
>>   }
>>
>
>
>-- 
>-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
>-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
>

Thanks,
Chase

[PATCH 2/3 v2] Staging: comedi: addi-data: cleanup conditional blocks in hwdrv_apci035.c

[Chase Southwood]

There were some conditional blocks that had an unnecessary level of
indentation in them.  We can remove this to improve code clarity.

Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
---
2: Moved "else if" up to the same line as closing brace of "if".  Left
braces on single line "else if" matched to an "if" requiring braces, per
CodingStyle.

 .../comedi/drivers/addi-data/hwdrv_apci035.c       | 31 ++++++++++------------
 1 file changed, 14 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
index 7c40535..ebc0587 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
@@ -207,23 +207,22 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
 		ui_Command =
 			(ui_Command & 0xFFF719E2UL) | ui_Mode << 13UL | 0x10UL;
 
-	} else {
-		if (data[0] == ADDIDATA_WATCHDOG) {
+	} else if (data[0] == ADDIDATA_WATCHDOG) {
 
-			/* Set the mode :             */
-			/* - Disable the hardware     */
-			/* - Disable the counter mode */
-			/* - Disable the warning      */
-			/* - Disable the reset        */
-			/* - Disable the timer mode   */
+		/* Set the mode :             */
+		/* - Disable the hardware     */
+		/* - Disable the counter mode */
+		/* - Disable the warning      */
+		/* - Disable the reset        */
+		/* - Disable the timer mode   */
 
-			ui_Command = ui_Command & 0xFFF819E2UL;
+		ui_Command = ui_Command & 0xFFF819E2UL;
 
-		} else {
-			dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
-			return -EINVAL;
-		}
+	} else {
+		dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
+		return -EINVAL;
 	}
+
 	outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
 	ui_Command = 0;
 	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
@@ -626,11 +625,9 @@ static void v_APCI035_Interrupt(int irq, void *d)
 		/* Read the digital temperature value */
 		ui_DigitalTemperature = inl(devpriv->iobase + 128 + 60);
 		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
-	}
 
-	else {
-		if ((ui_StatusRegister2 & 0x1) == 0x1)
-			send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
+	} else if ((ui_StatusRegister2 & 0x1) == 0x1) {
+		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
 	}
 
 	return;
-- 
1.8.5.3

[PATCH 3/3 v2] Staging: comedi: addi-data: fix a couple of lines that are too long

[Chase Southwood]

There are a couple of cases where a comment being on the same line as a
statement is causing the line to be over 80 characters long.  This is an
easy fix, move these comments to the previous line.

Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
---
2: Some changes to PATCH 2/3 v2 occurred in the immediate context of
changes in this patch.  I've redone this patch after updating PATCH 2/3
so that it still applies cleanly.

 drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
index ebc0587..6fca105 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
@@ -565,7 +565,9 @@ static int i_APCI035_Reset(struct comedi_device *dev)
 
 	for (i_Count = 1; i_Count <= 4; i_Count++) {
 		i_WatchdogNbr = i_Count;
-		outl(0x0, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 0);	/* stop all timers */
+
+		/* stop all timers */
+		outl(0x0, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 0);
 	}
 	outl(0x0, devpriv->iobase + 128 + 12);	/* Disable the warning delay */
 
@@ -624,10 +626,13 @@ static void v_APCI035_Interrupt(int irq, void *d)
 
 		/* Read the digital temperature value */
 		ui_DigitalTemperature = inl(devpriv->iobase + 128 + 60);
-		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
+
+		/*  send signal to the sample */
+		send_sig(SIGIO, devpriv->tsk_Current, 0);
 
 	} else if ((ui_StatusRegister2 & 0x1) == 0x1) {
-		send_sig(SIGIO, devpriv->tsk_Current, 0);	/*  send signal to the sample */
+		/*  send signal to the sample */
+		send_sig(SIGIO, devpriv->tsk_Current, 0);
 	}
 
 	return;
-- 
1.8.5.3

Re: [PATCH 2/3 v2] Staging: comedi: addi-data: cleanup conditional blocks in hwdrv_apci035.c

[Ian Abbott]

On 2014-02-24 16:34, Chase Southwood wrote:
> There were some conditional blocks that had an unnecessary level of
> indentation in them.  We can remove this to improve code clarity.
>
> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
> ---
> 2: Moved "else if" up to the same line as closing brace of "if".  Left
> braces on single line "else if" matched to an "if" requiring braces, per
> CodingStyle.

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

Re: [PATCH 3/3 v2] Staging: comedi: addi-data: fix a couple of lines that are too long

[Ian Abbott]

On 2014-02-24 16:35, Chase Southwood wrote:
> There are a couple of cases where a comment being on the same line as a
> statement is causing the line to be over 80 characters long.  This is an
> easy fix, move these comments to the previous line.
>
> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
> ---
> 2: Some changes to PATCH 2/3 v2 occurred in the immediate context of
> changes in this patch.  I've redone this patch after updating PATCH 2/3
> so that it still applies cleanly.

Reviewed-by: Ian Abbott <abbotti@mev.co.uk>

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

Re: [PATCH 3/3 v2] Staging: comedi: addi-data: fix a couple of lines that are too long

[Greg KH]

On Mon, Feb 24, 2014 at 10:35:09AM -0600, Chase Southwood wrote:
> There are a couple of cases where a comment being on the same line as a
> statement is causing the line to be over 80 characters long.  This is an
> easy fix, move these comments to the previous line.
> 
> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
> Reviewed-by: Ian Abbott <abbotti@mev.co.uk>
> 
> ---
> 2: Some changes to PATCH 2/3 v2 occurred in the immediate context of
> changes in this patch.  I've redone this patch after updating PATCH 2/3
> so that it still applies cleanly.

Thank you for noticing and fixing this up before I hit it, much
appreciated.

greg k-h