2014-02-22 03:20:47

by Chase Southwood

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

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 <[email protected]>
---

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


2014-02-22 03:22:08

by Chase Southwood

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

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 <[email protected]>
---
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

2014-02-22 03:22:35

by Chase Southwood

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

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 <[email protected]>
---
.../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

2014-02-24 14:09:30

by Ian Abbott

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

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 <[email protected]>
> ---
>
> 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 <[email protected]>

--
-=( Ian Abbott @ MEV Ltd. E-mail: <[email protected]> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-

2014-02-24 14:13:54

by Ian Abbott

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

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 <[email protected]>
> ---
> .../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: <[email protected]> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-

2014-02-24 14:14:35

by Ian Abbott

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

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 <[email protected]>

Reviewed-by: Ian Abbott <[email protected]>

--
-=( Ian Abbott @ MEV Ltd. E-mail: <[email protected]> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-

2014-02-24 16:02:04

by Chase Southwood

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

>On Monday, February 24, 2014 8:13 AM, Ian Abbott <[email protected]> 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 <[email protected]>
>> ---
>> ? .../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: <[email protected]>? ? ? ? )=-
>-=( Tel: +44 (0)161 477 1898? FAX: +44 (0)161 718 3587? ? ? ? )=-
>

Thanks,
Chase

2014-02-24 16:34:24

by Chase Southwood

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

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 <[email protected]>
---
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

2014-02-24 16:35:15

by Chase Southwood

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

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 <[email protected]>
---
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

2014-02-24 16:41:27

by Ian Abbott

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

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 <[email protected]>
> ---
> 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 <[email protected]>

--
-=( Ian Abbott @ MEV Ltd. E-mail: <[email protected]> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-

2014-02-24 16:41:05

by Ian Abbott

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

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 <[email protected]>
> ---
> 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 <[email protected]>

--
-=( Ian Abbott @ MEV Ltd. E-mail: <[email protected]> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-

2014-02-25 00:59:08

by Greg Kroah-Hartman

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

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 <[email protected]>
> Reviewed-by: Ian Abbott <[email protected]>
>
> ---
> 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