This patch for hwdrv_apci035 removes some unneeded braces, and moves some
improperly placed braces to the correct position, as found by checkpatch.
It also removes a commented out if-statement that I found whilst cleaning
braces that is identical to another un-commented if-statement directly
above it, so it is just added clutter and so we can delete it to clean up
further.
Signed-off-by: Chase Southwood <[email protected]>
---
So I decided to venture into addi-data today and found that most of the
files in there are very messy from a style standpoint. This is the first
(of probably a few) patchsets to try and clean those files up a bit. I
hope that this will be helpful!
.../comedi/drivers/addi-data/hwdrv_apci035.c | 49 ++++++++++------------
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
index 1128c22..584a1d5 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
@@ -177,11 +177,11 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
devpriv->tsk_Current = current;
devpriv->b_TimerSelectMode = data[0];
i_WatchdogNbr = data[1];
- if (data[0] == 0) {
+ if (data[0] == 0)
ui_Mode = 2;
- } else {
+ else
ui_Mode = 0;
- }
+
/* ui_Command = inl(devpriv->iobase+((i_WatchdogNbr-1)*32)+12); */
ui_Command = 0;
/* ui_Command = ui_Command & 0xFFFFF9FEUL; */
@@ -366,8 +366,7 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
}
- if (data[0] == 0) /* Stop The Watchdog */
- {
+ if (data[0] == 0) {
/* Stop The Watchdog */
ui_Command = 0;
/*
@@ -377,15 +376,15 @@ 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 */
- {
+ if (data[0] == 3) {
+ /* stop all Watchdogs */
ui_Command = 0;
for (i_Count = 1; i_Count <= 4; i_Count++) {
- if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
+ if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG)
ui_Command = 0x2UL;
- } else {
+ else
ui_Command = 0x10UL;
- }
+
i_WatchdogNbr = i_Count;
outl(ui_Command,
devpriv->iobase + ((i_WatchdogNbr - 1) * 32) +
@@ -393,30 +392,29 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
}
}
- if (data[0] == 4) /* start all Watchdogs */
- {
+ if (data[0] == 4) {
+ /* start all Watchdogs */
ui_Command = 0;
for (i_Count = 1; i_Count <= 4; i_Count++) {
- if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
+ if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG)
ui_Command = 0x1UL;
- } else {
+ else
ui_Command = 0x8UL;
- }
+
i_WatchdogNbr = i_Count;
outl(ui_Command,
devpriv->iobase + ((i_WatchdogNbr - 1) * 32) +
0);
}
}
- if (data[0] == 5) /* trigger all Watchdogs */
- {
+ if (data[0] == 5) {
+ /* trigger all Watchdogs */
ui_Command = 0;
for (i_Count = 1; i_Count <= 4; i_Count++) {
- if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
+ if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG)
ui_Command = 0x4UL;
- } else {
+ else
ui_Command = 0x20UL;
- }
i_WatchdogNbr = i_Count;
outl(ui_Command,
@@ -488,11 +486,9 @@ static int i_APCI035_ReadTimerWatchdog(struct comedi_device *dev,
/* Get the overflow status */
/***************************/
data[3] = ((ui_Status >> 0) & 1);
- if (devpriv->b_TimerSelectMode == ADDIDATA_TIMER) {
+ if (devpriv->b_TimerSelectMode == ADDIDATA_TIMER)
data[4] = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 0);
- } /* if (devpriv->b_TimerSelectMode==ADDIDATA_TIMER) */
-
return insn->n;
}
@@ -655,8 +651,8 @@ static void v_APCI035_Interrupt(int irq, void *d)
ui_StatusRegister2 =
inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 20);
- if ((((ui_StatusRegister1) & 0x8) == 0x8)) /* Test if warning relay interrupt */
- {
+ /* Test if warning relay interrupt */
+ if ((((ui_StatusRegister1) & 0x8) == 0x8)) {
/**********************************/
/* Disable the temperature warning */
/**********************************/
@@ -675,9 +671,8 @@ static void v_APCI035_Interrupt(int irq, void *d)
} /* if (((ui_StatusRegister1 & 0x8) == 0x8)) */
else {
- if ((ui_StatusRegister2 & 0x1) == 0x1) {
+ 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
This patch for hwdrv_apci035.c aligns comment blocks and makes indentation
of comments consistent. Removed all "spaces before tabs" in comment
indentation as well.
Signed-off-by: Chase Southwood <[email protected]>
---
.../comedi/drivers/addi-data/hwdrv_apci035.c | 140 ++++++++++-----------
1 file changed, 70 insertions(+), 70 deletions(-)
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
index 584a1d5..90d5801 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
@@ -188,17 +188,17 @@ 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 */
-/************************/
+ /************************/
+ /* Set the reload value */
+ /************************/
outl(data[3], devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 4);
-/*********************/
-/* Set the time unit */
-/*********************/
+ /*********************/
+ /* 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,7 +206,7 @@ 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;
@@ -215,14 +215,14 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
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 +234,73 @@ 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 */
-/********************************/
+ /********************************/
+ /* 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 */
-/*****************************/
+ /*****************************/
+ /* Disable the hardware gate */
+ /*****************************/
ui_Command = ui_Command & 0xFFFFF87FUL;
if (data[6] == ADDIDATA_ENABLE) {
-/*******************************/
-/* Set the hardware gate level */
-/*******************************/
+ /*******************************/
+ /* 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 */
-/*******************************/
+ /*******************************/
+ /* Disable the hardware output */
+ /*******************************/
ui_Command = ui_Command & 0xFFFFF9FBUL;
-/*********************************/
-/* Set the hardware output level */
-/*********************************/
+ /*********************************/
+ /* 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 */
-/*******************************/
+ /*******************************/
+ /* Set the interrupt selection */
+ /*******************************/
ui_Status = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 16);
ui_Command = (ui_Command & 0xFFFFF9FDUL) | (data[13] << 1);
@@ -348,9 +348,9 @@ 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);
@@ -358,9 +358,9 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
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);
@@ -369,10 +369,10 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
if (data[0] == 0) {
/* Stop The Watchdog */
ui_Command = 0;
-/*
-* ui_Command = inl(devpriv->iobase+((i_WatchdogNbr-1)*32)+12);
-* ui_Command = ui_Command & 0xFFFFF9FEUL;
-*/
+ /*
+ * ui_Command = inl(devpriv->iobase+((i_WatchdogNbr-1)*32)+12);
+ * ui_Command = ui_Command & 0xFFFFF9FEUL;
+ */
outl(ui_Command,
devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
} /* if (data[1]==0) */
@@ -525,9 +525,9 @@ 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 */
-/********************************/
+ /********************************/
+ /* 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 +564,18 @@ static int i_APCI035_ReadAnalogInput(struct comedi_device *dev,
struct addi_private *devpriv = dev->private;
unsigned int ui_CommandRegister = 0;
-/******************/
-/* Set the start */
-/******************/
+ /******************/
+ /* Set the start */
+ /******************/
ui_CommandRegister = 0x80000;
- /******************************/
+ /******************************/
/* Write the command register */
- /******************************/
+ /******************************/
outl(ui_CommandRegister, devpriv->iobase + 128 + 8);
-/***************************************/
-/* Read the digital value of the input */
-/***************************************/
+ /***************************************/
+ /* Read the digital value of the input */
+ /***************************************/
data[0] = inl(devpriv->iobase + 128 + 28);
return insn->n;
}
@@ -640,32 +640,32 @@ 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 */
- /**************************************/
+ /**************************************/
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)) */
--
1.8.5.3
This patch for hwdrv_apci035.c changes a printk() call to a dev_err call
since this is generally preferred.
Signed-off-by: Chase Southwood <[email protected]>
---
drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
index 90d5801..f02b714 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
@@ -227,7 +227,7 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
ui_Command = ui_Command & 0xFFF819E2UL;
} else {
- printk("\n The parameter for Timer/watchdog selection is in error\n");
+ dev_err(dev->class_dev, "\n The parameter for Timer/watchdog selection is in error\n");
return -EINVAL;
}
}
--
1.8.5.3
This patch for hwdrv_apci035.c removes a zero initialization from two
static variables. Static variables are initialized to zero by default,
so doing so explicitly is not necessary.
Signed-off-by: Chase Southwood <[email protected]>
---
I purposely made this patch the last in the series, because while it is
technically true that static variables need not be initialized to zero,
and checkpatch doesn't much like the practice, I didn't know if, for
readability's sake, you wanted these to stay in as-is. Since this is the
last patch, feel free to drop it if you don't like it.
drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
index f02b714..10e02e5 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
@@ -106,8 +106,8 @@ static struct comedi_lrange range_apci035_ai = {
}
};
-static int i_WatchdogNbr = 0;
-static int i_Temp = 0;
+static int i_WatchdogNbr;
+static int i_Temp;
static int i_Flag = 1;
/*
+----------------------------------------------------------------------------+
--
1.8.5.3
On 2014-02-16 08:40, Chase Southwood wrote:
> This patch for hwdrv_apci035 removes some unneeded braces, and moves some
> improperly placed braces to the correct position, as found by checkpatch.
> It also removes a commented out if-statement that I found whilst cleaning
> braces that is identical to another un-commented if-statement directly
> above it, so it is just added clutter and so we can delete it to clean up
> further.
>
> Signed-off-by: Chase Southwood <[email protected]>
> ---
> So I decided to venture into addi-data today and found that most of the
> files in there are very messy from a style standpoint. This is the first
> (of probably a few) patchsets to try and clean those files up a bit. I
> hope that this will be helpful!
Quite a few have been cleaned up extensively by Hartley, but they needed
more extensive changes than clean-ups due to them trying to handle
things differently to the normal comedi way of doing things.
>
> .../comedi/drivers/addi-data/hwdrv_apci035.c | 49 ++++++++++------------
> 1 file changed, 22 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> index 1128c22..584a1d5 100644
> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> @@ -177,11 +177,11 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
> devpriv->tsk_Current = current;
> devpriv->b_TimerSelectMode = data[0];
> i_WatchdogNbr = data[1];
> - if (data[0] == 0) {
> + if (data[0] == 0)
> ui_Mode = 2;
> - } else {
> + else
> ui_Mode = 0;
> - }
> +
> /* ui_Command = inl(devpriv->iobase+((i_WatchdogNbr-1)*32)+12); */
> ui_Command = 0;
> /* ui_Command = ui_Command & 0xFFFFF9FEUL; */
> @@ -366,8 +366,7 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
> devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
> }
>
> - if (data[0] == 0) /* Stop The Watchdog */
> - {
> + if (data[0] == 0) {
> /* Stop The Watchdog */
> ui_Command = 0;
> /*
> @@ -377,15 +376,15 @@ 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 */
> - {
> + if (data[0] == 3) {
> + /* stop all Watchdogs */
> ui_Command = 0;
> for (i_Count = 1; i_Count <= 4; i_Count++) {
> - if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
> + if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG)
> ui_Command = 0x2UL;
> - } else {
> + else
> ui_Command = 0x10UL;
> - }
> +
> i_WatchdogNbr = i_Count;
> outl(ui_Command,
> devpriv->iobase + ((i_WatchdogNbr - 1) * 32) +
> @@ -393,30 +392,29 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
> }
>
> }
> - if (data[0] == 4) /* start all Watchdogs */
> - {
> + if (data[0] == 4) {
> + /* start all Watchdogs */
> ui_Command = 0;
> for (i_Count = 1; i_Count <= 4; i_Count++) {
> - if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
> + if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG)
> ui_Command = 0x1UL;
> - } else {
> + else
> ui_Command = 0x8UL;
> - }
> +
> i_WatchdogNbr = i_Count;
> outl(ui_Command,
> devpriv->iobase + ((i_WatchdogNbr - 1) * 32) +
> 0);
> }
> }
> - if (data[0] == 5) /* trigger all Watchdogs */
> - {
> + if (data[0] == 5) {
> + /* trigger all Watchdogs */
> ui_Command = 0;
> for (i_Count = 1; i_Count <= 4; i_Count++) {
> - if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
> + if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG)
> ui_Command = 0x4UL;
> - } else {
> + else
> ui_Command = 0x20UL;
> - }
>
> i_WatchdogNbr = i_Count;
> outl(ui_Command,
> @@ -488,11 +486,9 @@ static int i_APCI035_ReadTimerWatchdog(struct comedi_device *dev,
> /* Get the overflow status */
> /***************************/
> data[3] = ((ui_Status >> 0) & 1);
> - if (devpriv->b_TimerSelectMode == ADDIDATA_TIMER) {
> + if (devpriv->b_TimerSelectMode == ADDIDATA_TIMER)
> data[4] = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 0);
>
> - } /* if (devpriv->b_TimerSelectMode==ADDIDATA_TIMER) */
> -
> return insn->n;
> }
>
> @@ -655,8 +651,8 @@ static void v_APCI035_Interrupt(int irq, void *d)
> ui_StatusRegister2 =
> inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 20);
>
> - if ((((ui_StatusRegister1) & 0x8) == 0x8)) /* Test if warning relay interrupt */
> - {
> + /* Test if warning relay interrupt */
> + if ((((ui_StatusRegister1) & 0x8) == 0x8)) {
> /**********************************/
> /* Disable the temperature warning */
> /**********************************/
> @@ -675,9 +671,8 @@ static void v_APCI035_Interrupt(int irq, void *d)
> } /* if (((ui_StatusRegister1 & 0x8) == 0x8)) */
>
> else {
> - if ((ui_StatusRegister2 & 0x1) == 0x1) {
> + if ((ui_StatusRegister2 & 0x1) == 0x1)
> send_sig(SIGIO, devpriv->tsk_Current, 0); /* send signal to the sample */
> - }
> } /* else if (((ui_StatusRegister1 & 0x8) == 0x8)) */
>
> return;
>
Looks good.
Signed-off-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 )=-
On 2014-02-16 08:41, Chase Southwood wrote:
> This patch for hwdrv_apci035.c aligns comment blocks and makes indentation
> of comments consistent. Removed all "spaces before tabs" in comment
> indentation as well.
>
> Signed-off-by: Chase Southwood <[email protected]>
> ---
> .../comedi/drivers/addi-data/hwdrv_apci035.c | 140 ++++++++++-----------
> 1 file changed, 70 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> index 584a1d5..90d5801 100644
> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> @@ -188,17 +188,17 @@ 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 */
> -/************************/
> + /************************/
> + /* Set the reload value */
> + /************************/
> outl(data[3], devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 4);
> -/*********************/
> -/* Set the time unit */
> -/*********************/
> + /*********************/
> + /* 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,7 +206,7 @@ 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;
> @@ -215,14 +215,14 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
> 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 +234,73 @@ 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 */
> -/********************************/
> + /********************************/
> + /* 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 */
> -/*****************************/
> + /*****************************/
> + /* Disable the hardware gate */
> + /*****************************/
> ui_Command = ui_Command & 0xFFFFF87FUL;
> if (data[6] == ADDIDATA_ENABLE) {
> -/*******************************/
> -/* Set the hardware gate level */
> -/*******************************/
> + /*******************************/
> + /* 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 */
> -/*******************************/
> + /*******************************/
> + /* Disable the hardware output */
> + /*******************************/
> ui_Command = ui_Command & 0xFFFFF9FBUL;
> -/*********************************/
> -/* Set the hardware output level */
> -/*********************************/
> + /*********************************/
> + /* 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 */
> -/*******************************/
> + /*******************************/
> + /* Set the interrupt selection */
> + /*******************************/
> ui_Status = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 16);
>
> ui_Command = (ui_Command & 0xFFFFF9FDUL) | (data[13] << 1);
> @@ -348,9 +348,9 @@ 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);
> @@ -358,9 +358,9 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
> 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);
> @@ -369,10 +369,10 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
> if (data[0] == 0) {
> /* Stop The Watchdog */
> ui_Command = 0;
> -/*
> -* ui_Command = inl(devpriv->iobase+((i_WatchdogNbr-1)*32)+12);
> -* ui_Command = ui_Command & 0xFFFFF9FEUL;
> -*/
> + /*
> + * ui_Command = inl(devpriv->iobase+((i_WatchdogNbr-1)*32)+12);
> + * ui_Command = ui_Command & 0xFFFFF9FEUL;
> + */
> outl(ui_Command,
> devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
> } /* if (data[1]==0) */
> @@ -525,9 +525,9 @@ 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 */
> -/********************************/
> + /********************************/
> + /* 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 +564,18 @@ static int i_APCI035_ReadAnalogInput(struct comedi_device *dev,
> struct addi_private *devpriv = dev->private;
> unsigned int ui_CommandRegister = 0;
>
> -/******************/
> -/* Set the start */
> -/******************/
> + /******************/
> + /* Set the start */
> + /******************/
> ui_CommandRegister = 0x80000;
> - /******************************/
> + /******************************/
> /* Write the command register */
> - /******************************/
> + /******************************/
> outl(ui_CommandRegister, devpriv->iobase + 128 + 8);
>
> -/***************************************/
> -/* Read the digital value of the input */
> -/***************************************/
> + /***************************************/
> + /* Read the digital value of the input */
> + /***************************************/
> data[0] = inl(devpriv->iobase + 128 + 28);
> return insn->n;
> }
> @@ -640,32 +640,32 @@ 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 */
> - /**************************************/
> + /**************************************/
>
> 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)) */
>
Looks good.
Signed-off-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 )=-
On 2014-02-16 08:41, Chase Southwood wrote:
> This patch for hwdrv_apci035.c changes a printk() call to a dev_err call
> since this is generally preferred.
>
> Signed-off-by: Chase Southwood <[email protected]>
> ---
> drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> index 90d5801..f02b714 100644
> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> @@ -227,7 +227,7 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
> ui_Command = ui_Command & 0xFFF819E2UL;
>
> } else {
> - printk("\n The parameter for Timer/watchdog selection is in error\n");
> + dev_err(dev->class_dev, "\n The parameter for Timer/watchdog selection is in error\n");
> return -EINVAL;
> }
> }
>
It'd be great if you could get rid of the initial "\n " in the error
string too.
--
-=( Ian Abbott @ MEV Ltd. E-mail: <[email protected]> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
On 2014-02-16 08:41, Chase Southwood wrote:
> This patch for hwdrv_apci035.c removes a zero initialization from two
> static variables. Static variables are initialized to zero by default,
> so doing so explicitly is not necessary.
>
> Signed-off-by: Chase Southwood <[email protected]>
> ---
> I purposely made this patch the last in the series, because while it is
> technically true that static variables need not be initialized to zero,
> and checkpatch doesn't much like the practice, I didn't know if, for
> readability's sake, you wanted these to stay in as-is. Since this is the
> last patch, feel free to drop it if you don't like it.
>
> drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> index f02b714..10e02e5 100644
> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> @@ -106,8 +106,8 @@ static struct comedi_lrange range_apci035_ai = {
> }
> };
>
> -static int i_WatchdogNbr = 0;
> -static int i_Temp = 0;
> +static int i_WatchdogNbr;
> +static int i_Temp;
> static int i_Flag = 1;
> /*
> +----------------------------------------------------------------------------+
>
God knows why it's using these variables anyway. The addi-data drivers
that haven't been redone by Hartley are a complete mess!
FWIW, there's no extra harm in applying this patch!
Signed-off-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 )=-
>On Monday, February 17, 2014 7:16 AM, Ian Abbott <[email protected]> wrote:
>>On 2014-02-16 08:40, Chase Southwood wrote:
>> This patch for hwdrv_apci035 removes some unneeded braces, and moves some
>> improperly placed braces to the correct position, as found by checkpatch.
>> It also removes a commented out if-statement that I found whilst cleaning
>> braces that is identical to another un-commented if-statement directly
>> above it, so it is just added clutter and so we can delete it to clean up
>> further.
>>
>> Signed-off-by: Chase Southwood <[email protected]>
>> ---
>> So I decided to venture into addi-data today and found that most of the
>> files in there are very messy from a style standpoint.? This is the first
>> (of probably a few) patchsets to try and clean those files up a bit.? I
>> hope that this will be helpful!
>
>Quite a few have been cleaned up extensively by Hartley, but they needed
>more extensive changes than clean-ups due to them trying to handle
>things differently to the normal comedi way of doing things.
Oh, I see.? Makes perfect sense.? Well like I said, if cleaning them up a bit will help make reworking them any easier or seems like useful work, I'd be happy to continue making them a little easier on the eyes!
>>
>>? .../comedi/drivers/addi-data/hwdrv_apci035.c? ? ? | 49 ++++++++++------------
>>? 1 file changed, 22 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
>> index 1128c22..584a1d5 100644
>> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
>> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
>> @@ -177,11 +177,11 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
>>? ??? devpriv->tsk_Current = current;
>>? ??? devpriv->b_TimerSelectMode = data[0];
>>? ??? i_WatchdogNbr = data[1];
>> -??? if (data[0] == 0) {
>> +??? if (data[0] == 0)
>>? ??? ??? ui_Mode = 2;
>> -??? } else {
>> +??? else
>>? ??? ??? ui_Mode = 0;
>> -??? }
>> +
>>? /* ui_Command = inl(devpriv->iobase+((i_WatchdogNbr-1)*32)+12); */
>>? ??? ui_Command = 0;
>>? /* ui_Command = ui_Command & 0xFFFFF9FEUL; */
>> @@ -366,8 +366,7 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
>>? ??? ??? ??? devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 12);
>>? ??? }
>>
>> -??? if (data[0] == 0)??? /* Stop The Watchdog */
>> -??? {
>> +??? if (data[0] == 0) {
>>? ??? ??? /* Stop The Watchdog */
>>? ??? ??? ui_Command = 0;
>>? /*
>> @@ -377,15 +376,15 @@ 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 */
>> -??? {
>> +??? if (data[0] == 3) {
>> +??? ??? /* stop all Watchdogs */
>>? ??? ??? ui_Command = 0;
>>? ??? ??? for (i_Count = 1; i_Count <= 4; i_Count++) {
>> -??? ??? ??? if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
>> +??? ??? ??? if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG)
>>? ??? ??? ??? ??? ui_Command = 0x2UL;
>> -??? ??? ??? } else {
>> +??? ??? ??? else
>>? ??? ??? ??? ??? ui_Command = 0x10UL;
>> -??? ??? ??? }
>> +
>>? ??? ??? ??? i_WatchdogNbr = i_Count;
>>? ??? ??? ??? outl(ui_Command,
>>? ??? ??? ??? ??? devpriv->iobase + ((i_WatchdogNbr - 1) * 32) +
>> @@ -393,30 +392,29 @@ static int i_APCI035_StartStopWriteTimerWatchdog(struct comedi_device *dev,
>>? ??? ??? }
>>
>>? ??? }
>> -??? if (data[0] == 4)??? /* start all Watchdogs */
>> -??? {
>> +??? if (data[0] == 4) {
>> +??? ??? /* start all Watchdogs */
>>? ??? ??? ui_Command = 0;
>>? ??? ??? for (i_Count = 1; i_Count <= 4; i_Count++) {
>> -??? ??? ??? if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
>> +??? ??? ??? if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG)
>>? ??? ??? ??? ??? ui_Command = 0x1UL;
>> -??? ??? ??? } else {
>> +??? ??? ??? else
>>? ??? ??? ??? ??? ui_Command = 0x8UL;
>> -??? ??? ??? }
>> +
>>? ??? ??? ??? i_WatchdogNbr = i_Count;
>>? ??? ??? ??? outl(ui_Command,
>>? ??? ??? ??? ??? devpriv->iobase + ((i_WatchdogNbr - 1) * 32) +
>>? ??? ??? ??? ??? 0);
>>? ??? ??? }
>>? ??? }
>> -??? if (data[0] == 5)??? /* trigger all Watchdogs */
>> -??? {
>> +??? if (data[0] == 5) {
>> +??? ??? /* trigger all Watchdogs */
>>? ??? ??? ui_Command = 0;
>>? ??? ??? for (i_Count = 1; i_Count <= 4; i_Count++) {
>> -??? ??? ??? if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG) {
>> +??? ??? ??? if (devpriv->b_TimerSelectMode == ADDIDATA_WATCHDOG)
>>? ??? ??? ??? ??? ui_Command = 0x4UL;
>> -??? ??? ??? } else {
>> +??? ??? ??? else
>>? ??? ??? ??? ??? ui_Command = 0x20UL;
>> -??? ??? ??? }
>>
>>? ??? ??? ??? i_WatchdogNbr = i_Count;
>>? ??? ??? ??? outl(ui_Command,
>> @@ -488,11 +486,9 @@ static int i_APCI035_ReadTimerWatchdog(struct comedi_device *dev,
>>? ??? /* Get the overflow status */
>>? ??? /***************************/
>>? ??? data[3] = ((ui_Status >> 0) & 1);
>> -??? if (devpriv->b_TimerSelectMode == ADDIDATA_TIMER) {
>> +??? if (devpriv->b_TimerSelectMode == ADDIDATA_TIMER)
>>? ??? ??? data[4] = inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 0);
>>
>> -??? }??? ??? ??? /*? if? (devpriv->b_TimerSelectMode==ADDIDATA_TIMER) */
>> -
>>? ??? return insn->n;
>>? }
>>
>> @@ -655,8 +651,8 @@ static void v_APCI035_Interrupt(int irq, void *d)
>>? ??? ui_StatusRegister2 =
>>? ??? ??? inl(devpriv->iobase + ((i_WatchdogNbr - 1) * 32) + 20);
>>
>> -??? if ((((ui_StatusRegister1) & 0x8) == 0x8))??? /* Test if warning relay interrupt */
>> -??? {
>> +??? /* Test if warning relay interrupt */
>> +??? if ((((ui_StatusRegister1) & 0x8) == 0x8)) {
>>? ??? /**********************************/
>>? ??? ??? /* Disable the temperature warning */
>>? ??? /**********************************/
>> @@ -675,9 +671,8 @@ static void v_APCI035_Interrupt(int irq, void *d)
>>? ??? }??? ??? ??? /* if (((ui_StatusRegister1 & 0x8) == 0x8)) */
>>
>>? ??? else {
>> -??? ??? if ((ui_StatusRegister2 & 0x1) == 0x1) {
>> +??? ??? if ((ui_StatusRegister2 & 0x1) == 0x1)
>>? ??? ??? ??? send_sig(SIGIO, devpriv->tsk_Current, 0);??? /*? send signal to the sample */
>> -??? ??? }
>>? ??? }??? ??? ??? /* else if (((ui_StatusRegister1 & 0x8) == 0x8)) */
>>
>>? ??? return;
>>
>
>Looks good.
>
>Signed-off-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? ? ? ? )=-
>
Thanks,
Chase
>On Monday, February 17, 2014 7:18 AM, Ian Abbott <[email protected]> wrote:
>>On 2014-02-16 08:41, Chase Southwood wrote:
>>
>> This patch for hwdrv_apci035.c changes a printk() call to a dev_err call
>> since this is generally preferred.
>>
>> Signed-off-by: Chase Southwood <[email protected]>
>> ---
>>? drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c | 2 +-
>>? 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
>> index 90d5801..f02b714 100644
>> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
>> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
>> @@ -227,7 +227,7 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
>>? ??? ??? ??? ui_Command = ui_Command & 0xFFF819E2UL;
>>
>>? ??? ??? } else {
>> -??? ??? ??? printk("\n The parameter for Timer/watchdog selection is in error\n");
>> +??? ??? ??? dev_err(dev->class_dev, "\n The parameter for Timer/watchdog selection is in error\n");
>>? ??? ??? ??? return -EINVAL;>>
>>? ??? ??? }
>>? ??? }
>>
>
>It'd be great if you could get rid of the initial "\n " in the error
>string too.
>
Excellent, thanks for the feedback.? I'll resend without that newline!
Thanks,
Chase
This patch for hwdrv_apci035.c changes a printk() call to a dev_err() call
since this is generally preferred. It also removes a newline from the start
of the error message.
Signed-off-by: Chase Southwood <[email protected]>
---
2: Removed leading newline, per Ian's request.
drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
index 90d5801..ff64540 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
@@ -227,7 +227,7 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
ui_Command = ui_Command & 0xFFF819E2UL;
} else {
- printk("\n The parameter for Timer/watchdog selection is in error\n");
+ dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
return -EINVAL;
}
}
--
1.8.5.3
On 2014-02-18 06:34, Chase Southwood wrote:
> This patch for hwdrv_apci035.c changes a printk() call to a dev_err() call
> since this is generally preferred. It also removes a newline from the start
> of the error message.
>
> Signed-off-by: Chase Southwood <[email protected]>
> ---
> 2: Removed leading newline, per Ian's request.
>
> drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> index 90d5801..ff64540 100644
> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci035.c
> @@ -227,7 +227,7 @@ static int i_APCI035_ConfigTimerWatchdog(struct comedi_device *dev,
> ui_Command = ui_Command & 0xFFF819E2UL;
>
> } else {
> - printk("\n The parameter for Timer/watchdog selection is in error\n");
> + dev_err(dev->class_dev, "The parameter for Timer/watchdog selection is in error\n");
> return -EINVAL;
> }
> }
>
That's better!
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 )=-
On Mon, Feb 17, 2014 at 12:45:07PM -0800, Chase Southwood wrote:
> >On Monday, February 17, 2014 7:16 AM, Ian Abbott <[email protected]> wrote:
>
> >>On 2014-02-16 08:40, Chase Southwood wrote:
> >> This patch for hwdrv_apci035 removes some unneeded braces, and moves some
> >> improperly placed braces to the correct position, as found by checkpatch.
> >> It also removes a commented out if-statement that I found whilst cleaning
> >> braces that is identical to another un-commented if-statement directly
> >> above it, so it is just added clutter and so we can delete it to clean up
> >> further.
> >>
> >> Signed-off-by: Chase Southwood <[email protected]>
> >> ---
> >> So I decided to venture into addi-data today and found that most of the
> >> files in there are very messy from a style standpoint.? This is the first
> >> (of probably a few) patchsets to try and clean those files up a bit.? I
> >> hope that this will be helpful!
> >
> >Quite a few have been cleaned up extensively by Hartley, but they needed
> >more extensive changes than clean-ups due to them trying to handle
> >things differently to the normal comedi way of doing things.
>
> Oh, I see.? Makes perfect sense.? Well like I said, if cleaning them
> up a bit will help make reworking them any easier or seems like useful
> work, I'd be happy to continue making them a little easier on the
> eyes!
Ian acked your patch already... If you want to clean these up then go
for it. There is no reason Hartley should have to do all the work.
But consider doing more "extensive" cleanups. Instead of just shifting
the comments over in [patch 2/4] you could change the format to kernel
style. Newbies are too timid. Look through the work that Hartley has
done and try copy it.
regards,
dan carpenter
>On Tuesday, February 18, 2014 5:00 AM, Dan Carpenter <[email protected]> wrote:
>>On Mon, Feb 17, 2014 at 12:45:07PM -0800, Chase Southwood wrote:
>>>On Monday, February 17, 2014 7:16 AM, Ian Abbott <[email protected]> wrote:
>>>>
>>>>On 2014-02-16 08:40, Chase Southwood wrote:
>>>> This patch for hwdrv_apci035 removes some unneeded braces, and moves some
>>>> improperly placed braces to the correct position, as found by checkpatch.
>>>> It also removes a commented out if-statement that I found whilst cleaning
>>>> braces that is identical to another un-commented if-statement directly
>>>> above it, so it is just added clutter and so we can delete it to clean up
>>>> further.
>>>>
>>>> Signed-off-by: Chase Southwood <[email protected]>
>>>> ---
>>>> So I decided to venture into addi-data today and found that most of the
>>>> files in there are very messy from a style standpoint.? This is the first
>>>> (of probably a few) patchsets to try and clean those files up a bit.? I
>>>> hope that this will be helpful!
>>>>
>>>Quite a few have been cleaned up extensively by Hartley, but they needed
>>>more extensive changes than clean-ups due to them trying to handle
>>>things differently to the normal comedi way of doing things.
>>
>>Oh, I see.? Makes perfect sense.? Well like I said, if cleaning them
>>up a bit will help make reworking them any easier or seems like useful
>>work, I'd be happy to continue making them a little easier on the
>>eyes!
>
>Ian acked your patch already...? If you want to clean these up then go
>for it.? There is no reason Hartley should have to do all the work.
>
>But consider doing more "extensive" cleanups.? Instead of just shifting
>the comments over in [patch 2/4] you could change the format to kernel
>style.? Newbies are too timid.? Look through the work that Hartley has
>done and try copy it.
>
>regards,
>dan carpenter
Dan,
I appreciate the feedback on my contributions.? I will try to do more extensive cleaning because I want to help out as best as I can.
Thanks,
Chase