[PATCH] Staging: comedi: addi-data: clean-up variable use in hwdrv_apci035.c

[PATCH] Staging: comedi: addi-data: clean-up variable use in hwdrv_apci035.c

[Chase Southwood]

The static variable ui_Command is as of right now being cleared to a
value of zero between everytime that it writes to a port and then takes a
new value from a port.  Seems like this zeroing is unnecessary, so we can
just remove these lines.

Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
---
This sort of thing seems like a copy/paste sort of error to me, but there
could as easily be some reason here that I'm missing that this is needed
here.  My first impression, however, was that this extra clearing is
useless.

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

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
index 6fca105..9041fdf 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
@@ -182,11 +182,9 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
 	else
 		ui_Mode = 0;
 
-/* ui_Command = inl(devpriv->iobase+((i_WatchdogNbr-1)*32)+12); */
 	ui_Command = 0;
-/* ui_Command = ui_Command & 0xFFFFF9FEUL; */
 	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 */
@@ -224,7 +222,7 @@ 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 */
@@ -235,7 +233,7 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
 		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 */
@@ -246,7 +244,7 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
 		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 */
@@ -266,7 +264,6 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
 			devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 28);
 	}
 
-	ui_Command = 0;
 	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
 
 	/* Disable the hardware output */
@@ -277,7 +274,6 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
 	outl(ui_Command, devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
 
 	/* Enable the watchdog interrupt */
-	ui_Command = 0;
 	ui_Command = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
 
 	/* Set the interrupt selection */
-- 
1.8.5.3

Re: [PATCH] Staging: comedi: addi-data: clean-up variable use in hwdrv_apci035.c

[Ian Abbott]

On 2014-02-25 08:15, Chase Southwood wrote:
> The static variable ui_Command is as of right now being cleared to a
> value of zero between everytime that it writes to a port and then takes a
> new value from a port.  Seems like this zeroing is unnecessary, so we can
> just remove these lines.

The description is slightly wrong as the variable isn't static storage 
class.

>
> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
> ---
> This sort of thing seems like a copy/paste sort of error to me, but there
> could as easily be some reason here that I'm missing that this is needed
> here.  My first impression, however, was that this extra clearing is
> useless.

Yes, the extra clearing was useless.  There are also some useless 
initializers for this variable and others.

Fine, apart from the description.

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] Staging: comedi: addi-data: clean-up variable use in hwdrv_apci035.c

[Chase Southwood]

>On Tuesday, February 25, 2014 3:56 AM, Ian Abbott <abbotti@mev.co.uk> wrote:

>>On 2014-02-25 08:15, Chase Southwood wrote:
>> The static variable ui_Command is as of right now being cleared to a
>> value of zero between everytime that it writes to a port and then takes a
>> new value from a port.  Seems like this zeroing is unnecessary, so we can
>> just remove these lines.
>
>The description is slightly wrong as the variable isn't static storage 
>class.
>

Oh, shoot, you're exactly right.  I was looking at some other variables which
are static when I was making this up and I got storage classes all mixed up.
I'll fix up the description and send it off to Greg again as it hasn't been
applied yet.

>>
>> Signed-off-by: Chase Southwood <chase.southwood@yahoo.com>
>> ---
>> This sort of thing seems like a copy/paste sort of error to me, but there
>> could as easily be some reason here that I'm missing that this is needed
>> here.  My first impression, however, was that this extra clearing is
>> useless.
>
>Yes, the extra clearing was useless.  There are also some useless 
>initializers for this variable and others.
>
>Fine, apart from the description.
>
>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        )=-