2014-02-28 07:31:27

by Chase Southwood

[permalink] [raw]
Subject: [PATCH 1/2] Staging: comedi: addi-data: fix lines that are over 80 characters

hwdrv_apci1564.c had numerous lines over the column limit. This patch
splits all such lines to bring them in compliance with coding style.

Signed-off-by: Chase Southwood <[email protected]>
---
.../comedi/drivers/addi-data/hwdrv_apci1564.c | 50 ++++++++++++++++------
1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index 2b47fa1..77030c5 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -324,11 +324,15 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
inl(devpriv->i_IobaseAmcc + APCI1564_TIMER +
APCI1564_TCW_PROG);
ul_Command1 = ul_Command1 & 0xFFFFF9FEUL;
- outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* Stop The Timer */
+ /* Stop The Timer */
+ outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER +
+ APCI1564_TCW_PROG);

devpriv->b_TimerSelectMode = ADDIDATA_TIMER;
if (data[1] == 1) {
- outl(0x02, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* Enable TIMER int & DISABLE ALL THE OTHER int SOURCES */
+ /* Enable TIMER int & DISABLE ALL THE OTHER int SOURCES */
+ outl(0x02, devpriv->i_IobaseAmcc + APCI1564_TIMER +
+ APCI1564_TCW_PROG);
outl(0x0,
devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP +
APCI1564_DIGITAL_IP_IRQ);
@@ -352,7 +356,9 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
devpriv->iobase + APCI1564_COUNTER4 +
APCI1564_TCW_IRQ);
} else {
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* disable Timer interrupt */
+ /* disable Timer interrupt */
+ outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER +
+ APCI1564_TCW_PROG);
}

/* Loading Timebase */
@@ -370,7 +376,9 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
APCI1564_TCW_PROG);
ul_Command1 =
(ul_Command1 & 0xFFF719E2UL) | 2UL << 13UL | 0x10UL;
- outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* mode 2 */
+ /* mode 2 */
+ outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER +
+ APCI1564_TCW_PROG);
} else if (data[0] == ADDIDATA_COUNTER) {
devpriv->b_TimerSelectMode = ADDIDATA_COUNTER;
devpriv->b_ModeSelectRegister = data[5];
@@ -380,7 +388,9 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
inl(devpriv->iobase + ((data[5] - 1) * 0x20) +
APCI1564_TCW_PROG);
ul_Command1 = ul_Command1 & 0xFFFFF9FEUL;
- outl(ul_Command1, devpriv->iobase + ((data[5] - 1) * 0x20) + APCI1564_TCW_PROG); /* Stop The Timer */
+ /* Stop The Timer */
+ outl(ul_Command1, devpriv->iobase + ((data[5] - 1) * 0x20) +
+ APCI1564_TCW_PROG);

/* Set the reload value */
outl(data[3],
@@ -457,7 +467,9 @@ static int i_APCI1564_StartStopWriteTimerCounterWatchdog(struct comedi_device *d
if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
switch (data[1]) {
case 0: /* stop the watchdog */
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP_WATCHDOG + APCI1564_TCW_PROG); /* disable the watchdog */
+ /* disable the watchdog */
+ outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP_WATCHDOG +
+ APCI1564_TCW_PROG);
break;
case 1: /* start the watchdog */
outl(0x0001,
@@ -678,13 +690,18 @@ static void v_APCI1564_Interrupt(int irq, void *d)
inl(devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP +
APCI1564_DIGITAL_IP_INTERRUPT_STATUS);
ui_InterruptStatus_1564 = ui_InterruptStatus_1564 & 0X000FFFF0;
- send_sig(SIGIO, devpriv->tsk_Current, 0); /* send signal to the sample */
- outl(ui_DI, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP + APCI1564_DIGITAL_IP_IRQ); /* enable the interrupt */
+ /* send signal to the sample */
+ send_sig(SIGIO, devpriv->tsk_Current, 0);
+ /* enable the interrupt */
+ outl(ui_DI, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP +
+ APCI1564_DIGITAL_IP_IRQ);
return;
}

if (ui_DO == 1) {
- /* Check for Digital Output interrupt Type - 1: Vcc interrupt 2: CC interrupt. */
+ /* Check for Digital Output interrupt Type */
+ /* 1: Vcc interrupt */
+ /* 2: CC interrupt */
ui_Type =
inl(devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP +
APCI1564_DIGITAL_OP_INTERRUPT_STATUS) & 0x3;
@@ -829,14 +846,19 @@ static int i_APCI1564_Reset(struct comedi_device *dev)
{
struct addi_private *devpriv = dev->private;

- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_IRQ); /* disable the interrupts */
- inl(devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_STATUS); /* Reset the interrupt status register */
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_MODE1); /* Disable the and/or interrupt */
+ /* disable the interrupts */
+ outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_IRQ);
+ /* Reset the interrupt status register */
+ inl(devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_STATUS);
+ /* Disable the and/or interrupt */
+ outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_MODE1);
outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_MODE2);
devpriv->b_DigitalOutputRegister = 0;
ui_Type = 0;
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP); /* Resets the output channels */
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP_INTERRUPT); /* Disables the interrupt. */
+ /* Resets the output channels */
+ outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP);
+ /* Disables the interrupt. */
+ outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP_INTERRUPT);
outl(0x0,
devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP_WATCHDOG +
APCI1564_TCW_RELOAD_VALUE);
--
1.8.5.3


2014-02-28 07:32:49

by Chase Southwood

[permalink] [raw]
Subject: [PATCH 2/2] Staging: comedi: addi-data: remove unnecessary variable initializations in hwdrv_apci1564.c

A handful of variables here were being initialized to 0 upon declaration,
however they are always then set to another value before their first use,
so initialization here is useless and we can remove it.

Signed-off-by: Chase Southwood <[email protected]>
---
drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index 968e26c..83e4a41 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -304,7 +304,7 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
unsigned int *data)
{
struct addi_private *devpriv = dev->private;
- unsigned int ul_Command1 = 0;
+ unsigned int ul_Command1;

devpriv->tsk_Current = current;
if (data[0] == ADDIDATA_WATCHDOG) {
@@ -462,7 +462,7 @@ static int i_APCI1564_StartStopWriteTimerCounterWatchdog(struct comedi_device *d
unsigned int *data)
{
struct addi_private *devpriv = dev->private;
- unsigned int ul_Command1 = 0;
+ unsigned int ul_Command1;

if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
switch (data[1]) {
@@ -560,7 +560,7 @@ static int i_APCI1564_ReadTimerCounterWatchdog(struct comedi_device *dev,
unsigned int *data)
{
struct addi_private *devpriv = dev->private;
- unsigned int ul_Command1 = 0;
+ unsigned int ul_Command1;

if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
/* Stores the status of the Watchdog */
@@ -658,7 +658,7 @@ static void v_APCI1564_Interrupt(int irq, void *d)
unsigned int ui_DO, ui_DI;
unsigned int ui_Timer;
unsigned int ui_C1, ui_C2, ui_C3, ui_C4;
- unsigned int ul_Command2 = 0;
+ unsigned int ul_Command2;

ui_DI = inl(devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP +
APCI1564_DIGITAL_IP_IRQ) & 0x01;
--
1.8.5.3

2014-02-28 07:53:37

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 1/2] Staging: comedi: addi-data: fix lines that are over 80 characters

On Fri, Feb 28, 2014 at 01:31:20AM -0600, Chase Southwood wrote:
> hwdrv_apci1564.c had numerous lines over the column limit. This patch
> splits all such lines to bring them in compliance with coding style.
>
> Signed-off-by: Chase Southwood <[email protected]>
> ---
> .../comedi/drivers/addi-data/hwdrv_apci1564.c | 50 ++++++++++++++++------
> 1 file changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> index 2b47fa1..77030c5 100644
> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> @@ -324,11 +324,15 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
> inl(devpriv->i_IobaseAmcc + APCI1564_TIMER +
> APCI1564_TCW_PROG);
> ul_Command1 = ul_Command1 & 0xFFFFF9FEUL;
> - outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* Stop The Timer */
> + /* Stop The Timer */
> + outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER +
> + APCI1564_TCW_PROG);

Just make a helper function so that you can call it like this:

static void outl_1564_timer(struct addi_private *devpriv, unsigned int cmd,
unsigned int reg)
{
outl(cmd, devpriv->i_IobaseAmcc + APCI1564_TIMER, reg);
}

Then the caller becomes:

outl_1564_timer(devpriv, cmd, APCI1564_TCW_PROG);

regards,
dan carpenter

2014-02-28 07:57:16

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 1/2] Staging: comedi: addi-data: fix lines that are over 80 characters

On Fri, Feb 28, 2014 at 10:52:32AM +0300, Dan Carpenter wrote:
> On Fri, Feb 28, 2014 at 01:31:20AM -0600, Chase Southwood wrote:
> > hwdrv_apci1564.c had numerous lines over the column limit. This patch
> > splits all such lines to bring them in compliance with coding style.
> >
> > Signed-off-by: Chase Southwood <[email protected]>
> > ---
> > .../comedi/drivers/addi-data/hwdrv_apci1564.c | 50 ++++++++++++++++------
> > 1 file changed, 36 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> > index 2b47fa1..77030c5 100644
> > --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> > +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> > @@ -324,11 +324,15 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
> > inl(devpriv->i_IobaseAmcc + APCI1564_TIMER +
> > APCI1564_TCW_PROG);
> > ul_Command1 = ul_Command1 & 0xFFFFF9FEUL;
> > - outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* Stop The Timer */
> > + /* Stop The Timer */
> > + outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER +
> > + APCI1564_TCW_PROG);
>
> Just make a helper function so that you can call it like this:
>
> static void outl_1564_timer(struct addi_private *devpriv, unsigned int cmd,
> unsigned int reg)
> {
> outl(cmd, devpriv->i_IobaseAmcc + APCI1564_TIMER, reg);
^^^
Should be "devpriv->i_IobaseAmcc + APCI1564_TIMER + reg" obviously.

regards,
dan carpenter

2014-02-28 08:04:02

by Chase Southwood

[permalink] [raw]
Subject: Re: [PATCH 1/2] Staging: comedi: addi-data: fix lines that are over 80 characters

>On Friday, February 28, 2014 1:57 AM, Dan Carpenter <[email protected]> wrote:

>>On Fri, Feb 28, 2014 at 10:52:32AM +0300, Dan Carpenter wrote:
>>> On Fri, Feb 28, 2014 at 01:31:20AM -0600, Chase Southwood wrote:
>> > hwdrv_apci1564.c had numerous lines over the column limit.? This patch
>> > splits all such lines to bring them in compliance with coding style.
>> >
>> > Signed-off-by: Chase Southwood <[email protected]>
>> > ---
>> >? .../comedi/drivers/addi-data/hwdrv_apci1564.c? ? ? | 50 ++++++++++++++++------
>> >? 1 file changed, 36 insertions(+), 14 deletions(-)
>> >
>> > diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
>> > index 2b47fa1..77030c5 100644
>> > --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
>> > +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
>> > @@ -324,11 +324,15 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
>> >? ??? ??? ??? inl(devpriv->i_IobaseAmcc + APCI1564_TIMER +
>> >? ??? ??? ??? APCI1564_TCW_PROG);
>> >? ??? ??? ul_Command1 = ul_Command1 & 0xFFFFF9FEUL;
>> > -??? ??? outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG);??? /* Stop The Timer */
>> > +??? ??? /* Stop The Timer */
>> > +??? ??? outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER +
>> > +??? ??? ??? APCI1564_TCW_PROG);
>>
>> Just make a helper function so that you can call it like this:
>>
>> static void outl_1564_timer(struct addi_private *devpriv, unsigned int cmd,
>> ??? ??? ??? ? ? unsigned int reg)
>> {
>> ??? outl(cmd, devpriv->i_IobaseAmcc + APCI1564_TIMER, reg);
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?^^^
>Should be "devpriv->i_IobaseAmcc + APCI1564_TIMER + reg" obviously.
>
>
>regards,
>dan carpenter

You got it, I'll do it this way and resend.

Thanks,
Chase

2014-02-28 09:15:59

by Chase Southwood

[permalink] [raw]
Subject: [PATCH 1/2 v2] Staging: comedi: fix lines that are over 80 characters

This patch introduces a simple helper function, outl_1564_timer(), to
allow several lines which violate the character limit to be shortened.
A handful of other lines that are too long are appropriately split as
well.

Cc: Dan Carpenter <[email protected]>
Signed-off-by: Chase Southwood <[email protected]>
---
2: introduced outl_1564_timer() at the suggestion of Dan.
.../comedi/drivers/addi-data/hwdrv_apci1564.c | 83 +++++++++++++---------
1 file changed, 49 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index 2b47fa1..d958d3c 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -99,6 +99,9 @@ This program is distributed in the hope that it will be useful, but WITHOUT ANY
#define APCI1564_TCW_WARN_TIMEVAL 24
#define APCI1564_TCW_WARN_TIMEBASE 28

+/* Helper functions */
+static void outl_1564_timer(struct addi_private *, unsigned int, unsigned int);
+
/* Global variables */
static unsigned int ui_InterruptStatus_1564;
static unsigned int ui_InterruptData, ui_Type;
@@ -324,11 +327,13 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
inl(devpriv->i_IobaseAmcc + APCI1564_TIMER +
APCI1564_TCW_PROG);
ul_Command1 = ul_Command1 & 0xFFFFF9FEUL;
- outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* Stop The Timer */
+ /* Stop The Timer */
+ outl_1564_timer(devpriv, ul_Command1, APCI1564_TCW_PROG);

devpriv->b_TimerSelectMode = ADDIDATA_TIMER;
if (data[1] == 1) {
- outl(0x02, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* Enable TIMER int & DISABLE ALL THE OTHER int SOURCES */
+ /* Enable TIMER int & DISABLE ALL THE OTHER int SOURCES */
+ outl_1564_timer(devpriv, 0x02, APCI1564_TCW_PROG);
outl(0x0,
devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP +
APCI1564_DIGITAL_IP_IRQ);
@@ -352,25 +357,23 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
devpriv->iobase + APCI1564_COUNTER4 +
APCI1564_TCW_IRQ);
} else {
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* disable Timer interrupt */
+ /* disable Timer interrupt */
+ outl_1564_timer(devpriv, 0x0, APCI1564_TCW_PROG);
}

/* Loading Timebase */
- outl(data[2],
- devpriv->i_IobaseAmcc + APCI1564_TIMER +
- APCI1564_TCW_TIMEBASE);
+ outl_1564_timer(devpriv, data[2], APCI1564_TCW_TIMEBASE);

/* Loading the Reload value */
- outl(data[3],
- devpriv->i_IobaseAmcc + APCI1564_TIMER +
- APCI1564_TCW_RELOAD_VALUE);
+ outl_1564_timer(devpriv, data[3], APCI1564_TCW_RELOAD_VALUE);

ul_Command1 =
inl(devpriv->i_IobaseAmcc + APCI1564_TIMER +
APCI1564_TCW_PROG);
ul_Command1 =
(ul_Command1 & 0xFFF719E2UL) | 2UL << 13UL | 0x10UL;
- outl(ul_Command1, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG); /* mode 2 */
+ /* mode 2 */
+ outl_1564_timer(devpriv, ul_Command1, APCI1564_TCW_PROG);
} else if (data[0] == ADDIDATA_COUNTER) {
devpriv->b_TimerSelectMode = ADDIDATA_COUNTER;
devpriv->b_ModeSelectRegister = data[5];
@@ -380,7 +383,9 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
inl(devpriv->iobase + ((data[5] - 1) * 0x20) +
APCI1564_TCW_PROG);
ul_Command1 = ul_Command1 & 0xFFFFF9FEUL;
- outl(ul_Command1, devpriv->iobase + ((data[5] - 1) * 0x20) + APCI1564_TCW_PROG); /* Stop The Timer */
+ /* Stop The Timer */
+ outl(ul_Command1, devpriv->iobase + ((data[5] - 1) * 0x20) +
+ APCI1564_TCW_PROG);

/* Set the reload value */
outl(data[3],
@@ -457,7 +462,9 @@ static int i_APCI1564_StartStopWriteTimerCounterWatchdog(struct comedi_device *d
if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
switch (data[1]) {
case 0: /* stop the watchdog */
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP_WATCHDOG + APCI1564_TCW_PROG); /* disable the watchdog */
+ /* disable the watchdog */
+ outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP_WATCHDOG +
+ APCI1564_TCW_PROG);
break;
case 1: /* start the watchdog */
outl(0x0001,
@@ -484,9 +491,7 @@ static int i_APCI1564_StartStopWriteTimerCounterWatchdog(struct comedi_device *d
ul_Command1 = (ul_Command1 & 0xFFFFF9FFUL) | 0x1UL;

/* Enable the Timer */
- outl(ul_Command1,
- devpriv->i_IobaseAmcc + APCI1564_TIMER +
- APCI1564_TCW_PROG);
+ outl_1564_timer(devpriv, ul_Command1, APCI1564_TCW_PROG);
} else if (data[1] == 0) {
/* Stop The Timer */

@@ -494,9 +499,7 @@ static int i_APCI1564_StartStopWriteTimerCounterWatchdog(struct comedi_device *d
inl(devpriv->i_IobaseAmcc + APCI1564_TIMER +
APCI1564_TCW_PROG);
ul_Command1 = ul_Command1 & 0xFFFFF9FEUL;
- outl(ul_Command1,
- devpriv->i_IobaseAmcc + APCI1564_TIMER +
- APCI1564_TCW_PROG);
+ outl_1564_timer(devpriv, ul_Command1, APCI1564_TCW_PROG);
}
}
if (devpriv->b_TimerSelectMode == ADDIDATA_COUNTER) {
@@ -678,13 +681,18 @@ static void v_APCI1564_Interrupt(int irq, void *d)
inl(devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP +
APCI1564_DIGITAL_IP_INTERRUPT_STATUS);
ui_InterruptStatus_1564 = ui_InterruptStatus_1564 & 0X000FFFF0;
- send_sig(SIGIO, devpriv->tsk_Current, 0); /* send signal to the sample */
- outl(ui_DI, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP + APCI1564_DIGITAL_IP_IRQ); /* enable the interrupt */
+ /* send signal to the sample */
+ send_sig(SIGIO, devpriv->tsk_Current, 0);
+ /* enable the interrupt */
+ outl(ui_DI, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP +
+ APCI1564_DIGITAL_IP_IRQ);
return;
}

if (ui_DO == 1) {
- /* Check for Digital Output interrupt Type - 1: Vcc interrupt 2: CC interrupt. */
+ /* Check for Digital Output interrupt Type */
+ /* 1: Vcc interrupt */
+ /* 2: CC interrupt */
ui_Type =
inl(devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP +
APCI1564_DIGITAL_OP_INTERRUPT_STATUS) & 0x3;
@@ -705,18 +713,14 @@ static void v_APCI1564_Interrupt(int irq, void *d)
ul_Command2 =
inl(devpriv->i_IobaseAmcc + APCI1564_TIMER +
APCI1564_TCW_PROG);
- outl(0x0,
- devpriv->i_IobaseAmcc + APCI1564_TIMER +
- APCI1564_TCW_PROG);
+ outl_1564_timer(devpriv, 0x0, APCI1564_TCW_PROG);

/* Send a signal to from kernel to user space */
send_sig(SIGIO, devpriv->tsk_Current, 0);

/* Enable Timer Interrupt */

- outl(ul_Command2,
- devpriv->i_IobaseAmcc + APCI1564_TIMER +
- APCI1564_TCW_PROG);
+ outl_1564_timer(devpriv, ul_Command2, APCI1564_TCW_PROG);
}
}

@@ -829,19 +833,24 @@ static int i_APCI1564_Reset(struct comedi_device *dev)
{
struct addi_private *devpriv = dev->private;

- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_IRQ); /* disable the interrupts */
- inl(devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_STATUS); /* Reset the interrupt status register */
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_MODE1); /* Disable the and/or interrupt */
+ /* disable the interrupts */
+ outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_IRQ);
+ /* Reset the interrupt status register */
+ inl(devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_STATUS);
+ /* Disable the and/or interrupt */
+ outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_MODE1);
outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_MODE2);
devpriv->b_DigitalOutputRegister = 0;
ui_Type = 0;
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP); /* Resets the output channels */
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP_INTERRUPT); /* Disables the interrupt. */
+ /* Resets the output channels */
+ outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP);
+ /* Disables the interrupt. */
+ outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP_INTERRUPT);
outl(0x0,
devpriv->i_IobaseAmcc + APCI1564_DIGITAL_OP_WATCHDOG +
APCI1564_TCW_RELOAD_VALUE);
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER);
- outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER + APCI1564_TCW_PROG);
+ outl_1564_timer(devpriv, 0x0, 0x0);
+ outl_1564_timer(devpriv, 0x0, APCI1564_TCW_PROG);

outl(0x0, devpriv->iobase + APCI1564_COUNTER1 + APCI1564_TCW_PROG);
outl(0x0, devpriv->iobase + APCI1564_COUNTER2 + APCI1564_TCW_PROG);
@@ -849,3 +858,9 @@ static int i_APCI1564_Reset(struct comedi_device *dev)
outl(0x0, devpriv->iobase + APCI1564_COUNTER4 + APCI1564_TCW_PROG);
return 0;
}
+
+static void outl_1564_timer(struct addi_private *devpriv, unsigned int cmd,
+ unsigned int reg)
+{
+ outl(cmd, devpriv->i_IobaseAmcc + APCI1564_TIMER + reg);
+}
--
1.8.5.3

2014-02-28 09:16:30

by Chase Southwood

[permalink] [raw]
Subject: [PATCH 2/2 v2] Staging: comedi: addi-data: remove unnecessary variable initializations in hwdrv_apci1564.c

A handful of variables here were being initialized to 0 upon declaration,
however they are always then set to another value before their first use,
so initialization here is useless and we can remove it.

Signed-off-by: Chase Southwood <[email protected]>
---
2: no content change; redone and resent after PATCH 1/2 changed to ensure
clean applying.

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

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index d958d3c..c6e2d7e 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -307,7 +307,7 @@ static int i_APCI1564_ConfigTimerCounterWatchdog(struct comedi_device *dev,
unsigned int *data)
{
struct addi_private *devpriv = dev->private;
- unsigned int ul_Command1 = 0;
+ unsigned int ul_Command1;

devpriv->tsk_Current = current;
if (data[0] == ADDIDATA_WATCHDOG) {
@@ -457,7 +457,7 @@ static int i_APCI1564_StartStopWriteTimerCounterWatchdog(struct comedi_device *d
unsigned int *data)
{
struct addi_private *devpriv = dev->private;
- unsigned int ul_Command1 = 0;
+ unsigned int ul_Command1;

if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
switch (data[1]) {
@@ -551,7 +551,7 @@ static int i_APCI1564_ReadTimerCounterWatchdog(struct comedi_device *dev,
unsigned int *data)
{
struct addi_private *devpriv = dev->private;
- unsigned int ul_Command1 = 0;
+ unsigned int ul_Command1;

if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
/* Stores the status of the Watchdog */
@@ -649,7 +649,7 @@ static void v_APCI1564_Interrupt(int irq, void *d)
unsigned int ui_DO, ui_DI;
unsigned int ui_Timer;
unsigned int ui_C1, ui_C2, ui_C3, ui_C4;
- unsigned int ul_Command2 = 0;
+ unsigned int ul_Command2;

ui_DI = inl(devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP +
APCI1564_DIGITAL_IP_IRQ) & 0x01;
--
1.8.5.3

2014-02-28 09:42:42

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 1/2 v2] Staging: comedi: fix lines that are over 80 characters

On Fri, Feb 28, 2014 at 03:15:45AM -0600, Chase Southwood wrote:
> This patch introduces a simple helper function, outl_1564_timer(), to
> allow several lines which violate the character limit to be shortened.
> A handful of other lines that are too long are appropriately split as
> well.
>
> Cc: Dan Carpenter <[email protected]>
> Signed-off-by: Chase Southwood <[email protected]>
> ---
> 2: introduced outl_1564_timer() at the suggestion of Dan.
> .../comedi/drivers/addi-data/hwdrv_apci1564.c | 83 +++++++++++++---------
> 1 file changed, 49 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> index 2b47fa1..d958d3c 100644
> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> @@ -99,6 +99,9 @@ This program is distributed in the hope that it will be useful, but WITHOUT ANY
> #define APCI1564_TCW_WARN_TIMEVAL 24
> #define APCI1564_TCW_WARN_TIMEBASE 28
>
> +/* Helper functions */

Obvious comment is too obvious.

> +static void outl_1564_timer(struct addi_private *, unsigned int, unsigned int);
> +

Avoid forward declarations. Put the function itself here.

Do a function for APCI1564_DIGITAL_IP as well.

outl_1564_digital_ip()
inl_1564_digital_ip()

To be honest, I'm not sure how much the 1564 adds to the name. The
names should match of course.

I wonder if this line is buggy?:
outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_MODE1); /* Disable the and/or interrupt */
Should it be?
outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP + APCI1564_DIGITAL_IP_INTERRUPT_MODE1);

You would need to read the specs to be sure.

Anyway, take some more time with this and play with different ways to
clean it up.

regards,
dan carpenter

2014-02-28 09:53:51

by Chase Southwood

[permalink] [raw]
Subject: Re: [PATCH 1/2 v2] Staging: comedi: fix lines that are over 80 characters

>On Friday, February 28, 2014 3:42 AM, Dan Carpenter <[email protected]> wrote:

>>On Fri, Feb 28, 2014 at 03:15:45AM -0600, Chase Southwood wrote:
>> This patch introduces a simple helper function, outl_1564_timer(), to
>> allow several lines which violate the character limit to be shortened.
>> A handful of other lines that are too long are appropriately split as
>> well.
>>?
>> Cc: Dan Carpenter <[email protected]>
>> Signed-off-by: Chase Southwood <[email protected]>
>> ---
>> 2: introduced outl_1564_timer() at the suggestion of Dan.
>> ?.../comedi/drivers/addi-data/hwdrv_apci1564.c? ? ? | 83 +++++++++++++---------
>> ?1 file changed, 49 insertions(+), 34 deletions(-)
>>?
>> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
>> index 2b47fa1..d958d3c 100644
>> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
>> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
>> @@ -99,6 +99,9 @@ This program is distributed in the hope that it will be useful, but WITHOUT ANY
>> ?#define APCI1564_TCW_WARN_TIMEVAL??? ??? ??? 24
>> ?#define APCI1564_TCW_WARN_TIMEBASE??? ??? ??? 28
>> ?
>> +/* Helper functions */
>
>Obvious comment is too obvious.
>
>> +static void outl_1564_timer(struct addi_private *, unsigned int, unsigned int);
>> +
>
>Avoid forward declarations.? Put the function itself here.
>
>Do a function for APCI1564_DIGITAL_IP as well.
>
>outl_1564_digital_ip()
>inl_1564_digital_ip()
>
>To be honest, I'm not sure how much the 1564 adds to the name.? The
>names should match of course.
>
>I wonder if this line is buggy?:
>outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP_INTERRUPT_MODE1); /* Disable the >and/or interrupt */
>Should it be?
>outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DIGITAL_IP + >APCI1564_DIGITAL_IP_INTERRUPT_MODE1);
>
>You would need to read the specs to be sure.
>
>Anyway, take some more time with this and play with different ways to
>clean it up.
>
>regards,
>
>dan carpenter

Hey Dan!

Thanks a lot, I really appreciate all of the really good feedback. ?It's getting late now, but tomorrow I'll work on tidying everything up based on all of these comments and double check that hardware spec as well.

Thanks,
Chase

2014-02-28 22:30:51

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 1/2 v2] Staging: comedi: fix lines that are over 80 characters

On Fri, Feb 28, 2014 at 03:15:45AM -0600, Chase Southwood wrote:
> This patch introduces a simple helper function, outl_1564_timer(), to
> allow several lines which violate the character limit to be shortened.
> A handful of other lines that are too long are appropriately split as
> well.
>
> Cc: Dan Carpenter <[email protected]>
> Signed-off-by: Chase Southwood <[email protected]>
> ---
> 2: introduced outl_1564_timer() at the suggestion of Dan.
> .../comedi/drivers/addi-data/hwdrv_apci1564.c | 83 +++++++++++++---------
> 1 file changed, 49 insertions(+), 34 deletions(-)

The Subject: doesn't match the patch content :(