All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues in hwdrv_apci035.c
@ 2014-02-16  8:40 Chase Southwood
  2014-02-16  8:41 ` [PATCH 2/4] Staging: comedi: addi-data: cleanup comments " Chase Southwood
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Chase Southwood @ 2014-02-16  8:40 UTC (permalink / raw)
  To: gregkh; +Cc: abbotti, hsweeten, devel, linux-kernel, Chase Southwood

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 <chase.southwood@yahoo.com>
---
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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/4] Staging: comedi: addi-data: cleanup comments in hwdrv_apci035.c
  2014-02-16  8:40 [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues in hwdrv_apci035.c Chase Southwood
@ 2014-02-16  8:41 ` Chase Southwood
  2014-02-17 13:16   ` Ian Abbott
  2014-02-16  8:41 ` [PATCH 3/4] Staging: comedi: addi-data: convert printk() to dev_err() Chase Southwood
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Chase Southwood @ 2014-02-16  8:41 UTC (permalink / raw)
  To: gregkh; +Cc: abbotti, hsweeten, devel, linux-kernel, Chase Southwood

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 <chase.southwood@yahoo.com>
---
 .../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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/4] Staging: comedi: addi-data: convert printk() to dev_err()
  2014-02-16  8:40 [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues in hwdrv_apci035.c Chase Southwood
  2014-02-16  8:41 ` [PATCH 2/4] Staging: comedi: addi-data: cleanup comments " Chase Southwood
@ 2014-02-16  8:41 ` Chase Southwood
  2014-02-17 13:17   ` Ian Abbott
  2014-02-18  6:34   ` [PATCH 3/4 v2] " Chase Southwood
  2014-02-16  8:41 ` [PATCH 4/4] Staging: comedi: addi-data: do not initialize statics to 0 in hwdrv_apci035.c Chase Southwood
  2014-02-17 13:16 ` [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues " Ian Abbott
  3 siblings, 2 replies; 14+ messages in thread
From: Chase Southwood @ 2014-02-16  8:41 UTC (permalink / raw)
  To: gregkh; +Cc: abbotti, hsweeten, devel, linux-kernel, Chase Southwood

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 <chase.southwood@yahoo.com>
---
 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/4] Staging: comedi: addi-data: do not initialize statics to 0 in hwdrv_apci035.c
  2014-02-16  8:40 [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues in hwdrv_apci035.c Chase Southwood
  2014-02-16  8:41 ` [PATCH 2/4] Staging: comedi: addi-data: cleanup comments " Chase Southwood
  2014-02-16  8:41 ` [PATCH 3/4] Staging: comedi: addi-data: convert printk() to dev_err() Chase Southwood
@ 2014-02-16  8:41 ` Chase Southwood
  2014-02-17 13:22   ` Ian Abbott
  2014-02-17 13:16 ` [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues " Ian Abbott
  3 siblings, 1 reply; 14+ messages in thread
From: Chase Southwood @ 2014-02-16  8:41 UTC (permalink / raw)
  To: gregkh; +Cc: abbotti, hsweeten, devel, linux-kernel, Chase Southwood

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 <chase.southwood@yahoo.com>
---
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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues in hwdrv_apci035.c
  2014-02-16  8:40 [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues in hwdrv_apci035.c Chase Southwood
                   ` (2 preceding siblings ...)
  2014-02-16  8:41 ` [PATCH 4/4] Staging: comedi: addi-data: do not initialize statics to 0 in hwdrv_apci035.c Chase Southwood
@ 2014-02-17 13:16 ` Ian Abbott
  2014-02-17 20:45   ` Chase Southwood
  3 siblings, 1 reply; 14+ messages in thread
From: Ian Abbott @ 2014-02-17 13:16 UTC (permalink / raw)
  To: Chase Southwood, gregkh; +Cc: hsweeten, devel, linux-kernel

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 <chase.southwood@yahoo.com>
> ---
> 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 <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         )=-

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/4] Staging: comedi: addi-data: cleanup comments in hwdrv_apci035.c
  2014-02-16  8:41 ` [PATCH 2/4] Staging: comedi: addi-data: cleanup comments " Chase Southwood
@ 2014-02-17 13:16   ` Ian Abbott
  0 siblings, 0 replies; 14+ messages in thread
From: Ian Abbott @ 2014-02-17 13:16 UTC (permalink / raw)
  To: Chase Southwood, gregkh; +Cc: hsweeten, devel, linux-kernel

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 <chase.southwood@yahoo.com>
> ---
>   .../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 <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         )=-

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] Staging: comedi: addi-data: convert printk() to dev_err()
  2014-02-16  8:41 ` [PATCH 3/4] Staging: comedi: addi-data: convert printk() to dev_err() Chase Southwood
@ 2014-02-17 13:17   ` Ian Abbott
  2014-02-17 20:47     ` Chase Southwood
  2014-02-18  6:34   ` [PATCH 3/4 v2] " Chase Southwood
  1 sibling, 1 reply; 14+ messages in thread
From: Ian Abbott @ 2014-02-17 13:17 UTC (permalink / raw)
  To: Chase Southwood, gregkh; +Cc: hsweeten, devel, linux-kernel

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 <chase.southwood@yahoo.com>
> ---
>   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: <abbotti@mev.co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/4] Staging: comedi: addi-data: do not initialize statics to 0 in hwdrv_apci035.c
  2014-02-16  8:41 ` [PATCH 4/4] Staging: comedi: addi-data: do not initialize statics to 0 in hwdrv_apci035.c Chase Southwood
@ 2014-02-17 13:22   ` Ian Abbott
  0 siblings, 0 replies; 14+ messages in thread
From: Ian Abbott @ 2014-02-17 13:22 UTC (permalink / raw)
  To: Chase Southwood, gregkh; +Cc: hsweeten, devel, linux-kernel

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 <chase.southwood@yahoo.com>
> ---
> 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 <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         )=-

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues in hwdrv_apci035.c
  2014-02-17 13:16 ` [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues " Ian Abbott
@ 2014-02-17 20:45   ` Chase Southwood
  2014-02-18 10:59     ` Dan Carpenter
  0 siblings, 1 reply; 14+ messages in thread
From: Chase Southwood @ 2014-02-17 20:45 UTC (permalink / raw)
  To: Ian Abbott, gregkh; +Cc: hsweeten, devel, linux-kernel

>On Monday, February 17, 2014 7:16 AM, Ian Abbott <abbotti@mev.co.uk> 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 <chase.southwood@yahoo.com>
>> ---
>> 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 <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         )=-
>

Thanks,
Chase


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] Staging: comedi: addi-data: convert printk() to dev_err()
  2014-02-17 13:17   ` Ian Abbott
@ 2014-02-17 20:47     ` Chase Southwood
  0 siblings, 0 replies; 14+ messages in thread
From: Chase Southwood @ 2014-02-17 20:47 UTC (permalink / raw)
  To: Ian Abbott, gregkh; +Cc: hsweeten, devel, linux-kernel

>On Monday, February 17, 2014 7:18 AM, Ian Abbott <abbotti@mev.co.uk> 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 <chase.southwood@yahoo.com>
>> ---
>>   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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/4 v2] Staging: comedi: addi-data: convert printk() to dev_err()
  2014-02-16  8:41 ` [PATCH 3/4] Staging: comedi: addi-data: convert printk() to dev_err() Chase Southwood
  2014-02-17 13:17   ` Ian Abbott
@ 2014-02-18  6:34   ` Chase Southwood
  2014-02-18 10:52     ` Ian Abbott
  1 sibling, 1 reply; 14+ messages in thread
From: Chase Southwood @ 2014-02-18  6:34 UTC (permalink / raw)
  To: gregkh; +Cc: abbotti, hsweeten, devel, linux-kernel, Chase Southwood

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 <chase.southwood@yahoo.com>
---
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


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4 v2] Staging: comedi: addi-data: convert printk() to dev_err()
  2014-02-18  6:34   ` [PATCH 3/4 v2] " Chase Southwood
@ 2014-02-18 10:52     ` Ian Abbott
  0 siblings, 0 replies; 14+ messages in thread
From: Ian Abbott @ 2014-02-18 10:52 UTC (permalink / raw)
  To: Chase Southwood, gregkh; +Cc: hsweeten, devel, linux-kernel

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 <chase.southwood@yahoo.com>
> ---
> 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 <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         )=-

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues in hwdrv_apci035.c
  2014-02-17 20:45   ` Chase Southwood
@ 2014-02-18 10:59     ` Dan Carpenter
  2014-02-19  1:25       ` Chase Southwood
  0 siblings, 1 reply; 14+ messages in thread
From: Dan Carpenter @ 2014-02-18 10:59 UTC (permalink / raw)
  To: Chase Southwood; +Cc: Ian Abbott, gregkh, devel, linux-kernel

On Mon, Feb 17, 2014 at 12:45:07PM -0800, Chase Southwood wrote:
> >On Monday, February 17, 2014 7:16 AM, Ian Abbott <abbotti@mev.co.uk> 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 <chase.southwood@yahoo.com>
> >> ---
> >> 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues in hwdrv_apci035.c
  2014-02-18 10:59     ` Dan Carpenter
@ 2014-02-19  1:25       ` Chase Southwood
  0 siblings, 0 replies; 14+ messages in thread
From: Chase Southwood @ 2014-02-19  1:25 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Ian Abbott, gregkh, devel, linux-kernel



>On Tuesday, February 18, 2014 5:00 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:

>>On Mon, Feb 17, 2014 at 12:45:07PM -0800, Chase Southwood wrote:
>>>On Monday, February 17, 2014 7:16 AM, Ian Abbott <abbotti@mev.co.uk> 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 <chase.southwood@yahoo.com>
>>>> ---
>>>> 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2014-02-19  1:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-16  8:40 [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues in hwdrv_apci035.c Chase Southwood
2014-02-16  8:41 ` [PATCH 2/4] Staging: comedi: addi-data: cleanup comments " Chase Southwood
2014-02-17 13:16   ` Ian Abbott
2014-02-16  8:41 ` [PATCH 3/4] Staging: comedi: addi-data: convert printk() to dev_err() Chase Southwood
2014-02-17 13:17   ` Ian Abbott
2014-02-17 20:47     ` Chase Southwood
2014-02-18  6:34   ` [PATCH 3/4 v2] " Chase Southwood
2014-02-18 10:52     ` Ian Abbott
2014-02-16  8:41 ` [PATCH 4/4] Staging: comedi: addi-data: do not initialize statics to 0 in hwdrv_apci035.c Chase Southwood
2014-02-17 13:22   ` Ian Abbott
2014-02-17 13:16 ` [PATCH 1/4] Staging: comedi: addi-data: fix brace-related coding style issues " Ian Abbott
2014-02-17 20:45   ` Chase Southwood
2014-02-18 10:59     ` Dan Carpenter
2014-02-19  1:25       ` Chase Southwood

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.