staging: comedi: cb_pcidas64.c: Fix checkpatch warning
cover staging: comedi: cb_pcidas64.c: Fix checkpatch warning
Fix checkpath.pl warning:
line over 80 characters
Block comments use * on subsequent lines
Block comments use a trailing */ on a separate line Statements should start on a tabstop
Comparisons should place the constant on the right side of the test
Anson Jacob (6):
staging: comedi: cb_pcidas64.c: Fix checkpatch warning
staging: comedi: jr3_pci.h: Fix checkpatch warning
staging: comedi: ni_atmio.c: Fix checkpatch warning
staging: comedi: s626.h: Fix checkpatch warning
staging: comedi: jr3_pci.c: Fix checkpatch warning
staging: comedi: ni_at_a2150.c: Fix checkpatch warning
drivers/staging/comedi/drivers/cb_pcidas64.c | 105 ++++++++++++++------------- drivers/staging/comedi/drivers/jr3_pci.c | 3 +- drivers/staging/comedi/drivers/jr3_pci.h | 13 ++-- drivers/staging/comedi/drivers/ni_at_a2150.c | 82 ++++++++++++--------- drivers/staging/comedi/drivers/ni_atmio.c | 4 +- drivers/staging/comedi/drivers/s626.h | 2 +- 6 files changed, 113 insertions(+), 96 deletions(-)
Signature · 1 line
2.7.4
PATCH 1/6 staging: comedi: cb_pcidas64.c: Fix checkpatch warning
Fix checkpatch.pl warning:
Block comments use * on subsequent lines Block comments use a trailing */ on a separate line
--- drivers/staging/comedi/drivers/cb_pcidas64.c | 105 ++++++++++++++------------- 1 file changed, 55 insertions(+), 50 deletions(-)
drivers/staging/comedi/drivers/cb_pcidas64.c +55 -50
@@ -1,34 +1,34 @@/* -comedi/drivers/cb_pcidas64.c -This is a driver for the ComputerBoards/MeasurementComputing PCI-DAS -64xx, 60xx, and 4020 cards. - -Author: Frank Mori Hess <[email protected]> -Copyright (C) 2001, 2002 Frank Mori Hess - -Thanks also go to the following people: - -Steve Rosenbluth, for providing the source code for -his pci-das6402 driver, and source code for working QNX pci-6402 -drivers by Greg Laird and Mariusz Bogacz. None of the code was -used directly here, but it was useful as an additional source of -documentation on how to program the boards. - -John Sims, for much testing and feedback on pcidas-4020 support. - -COMEDI - Linux Control and Measurement Device Interface -Copyright (C) 1997-8 David A. Schleef <[email protected]> - -This program is free software; you can redistribute it and/or modify -it under the terms of the GNU General Public License as published by -the Free Software Foundation; either version 2 of the License, or -(at your option) any later version. - -This program is distributed in the hope that it will be useful, -but WITHOUT ANY WARRANTY; without even the implied warranty of -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -GNU General Public License for more details. -*/ +* comedi/drivers/cb_pcidas64.c +* This is a driver for the ComputerBoards/MeasurementComputing PCI-DAS +* 64xx, 60xx, and 4020 cards. +* +* Author: Frank Mori Hess <[email protected]> +* Copyright (C) 2001, 2002 Frank Mori Hess +* +* Thanks also go to the following people: +* +* Steve Rosenbluth, for providing the source code for +* his pci-das6402 driver, and source code for working QNX pci-6402 +* drivers by Greg Laird and Mariusz Bogacz. None of the code was +* used directly here, but it was useful as an additional source of +* documentation on how to program the boards. +* +* John Sims, for much testing and feedback on pcidas-4020 support. +* +* COMEDI - Linux Control and Measurement Device Interface +* Copyright (C) 1997-8 David A. Schleef <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +*/ /* * Driver: cb_pcidas64
@@ -66,19 +66,18 @@*/ /* - -TODO: -make it return error if user attempts an ai command that uses the -external queue, and an ao command simultaneously user counter subdevice -there are a number of boards this driver will support when they are -fully released, but does not yet since the pci device id numbers -are not yet available. - -support prescaled 100khz clock for slow pacing (not available on 6000 -series?) - -make ao fifo size adjustable like ai fifo -*/ +* TODO: +* make it return error if user attempts an ai command that uses the +* external queue, and an ao command simultaneously user counter subdevice +* there are a number of boards this driver will support when they are +* fully released, but does not yet since the pci device id numbers +* are not yet available. +* +* support prescaled 100khz clock for slow pacing (not available on 6000 +* series?) +* +* make ao fifo size adjustable like ai fifo +*/ #include <linux/module.h> #include <linux/delay.h>
@@ -91,11 +90,12 @@ TODO:#define TIMER_BASE 25/* 40MHz master clock */ /* 100kHz 'prescaled' clock for slow acquisition, -* maybe I'll support this someday */ +* maybe I'll support this someday +*/ #define PRESCALED_TIMER_BASE 10000#define DMA_BUFFER_SIZE 0x1000 -/* maximum value that can be loaded into board's 24-bit counters*/ +/* maximum value that can be loaded into board's 24-bit counters */ static const int max_counter_value =0xffffff ;/* PCI-DAS64xxx base addresses */
@@ -216,7 +216,8 @@ enum hw_config_contents {/* use 225 nanosec strobe when loading dac instead of 50 nanosec */ SLOW_DAC_BIT =0x400 ,/* bit with unknown function yet given as default value in pci-das64 -* manual */ +* manual +*/ HW_CONFIG_DUMMY_BITS =0x2000 ,/* bit selects channels 1/0 for analog input/output, otherwise 0/1 */ DMA_CH_SELECT_BIT = 0x8000,
@@ -1138,12 +1139,14 @@ struct pcidas64_private {/* physical addresses of ai dma buffers */ dma_addr_t ai_buffer_bus_addr[MAX_AI_DMA_RING_COUNT ];/* array of ai dma descriptors read by plx9080, -* allocated to get proper alignment */ +* allocated to get proper alignment +*/ struct plx_dma_desc *ai_dma_desc;/* physical address of ai dma descriptor array */ dma_addr_t ai_dma_desc_bus_addr;/* index of the ai dma descriptor/buffer -* that is currently being used */ +* that is currently being used +*/ unsigned int ai_dma_index;/* dma buffers for analog output */ uint16_t *ao_buffer[AO_DMA_RING_COUNT ];
@@ -1314,10 +1317,12 @@ static void init_plx9080(struct comedi_device *dev)/* enable dma chaining */ bits |=PLX_DMAMODE_CHAINEN ;/* enable interrupt on dma done -* (probably don't need this, since chain never finishes) */ +* (probably don't need this, since chain never finishes) +*/ bits |=PLX_DMAMODE_DONEIEN ;/* don't increment local address during transfers -* (we are transferring from a fixed fifo register) */ +* (we are transferring from a fixed fifo register) +*/ bits |=PLX_DMAMODE_LACONST ;/* route dma interrupt to pci bus */ bits |=PLX_DMAMODE_INTRPCI ;
Signature · 1 line
2.7.4
PATCH 2/6 staging: comedi: jr3_pci.h: Fix checkpatch warning
In reply to Anson JacobFix checkpatch.pl warning:
Block comments use * on subsequent lines Block comments use a trailing */ on a separate line
--- drivers/staging/comedi/drivers/jr3_pci.h | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
drivers/staging/comedi/drivers/jr3_pci.h +7 -6
@@ -421,12 +421,13 @@ struct jr3_channel {*/ struct force_array filter[7 ];/* offset 0x0090, -offset 0x0098, -offset 0x00a0, -offset 0x00a8, -offset 0x00b0, -offset 0x00b8 , -offset 0x00c0 */ +* offset 0x0098, +* offset 0x00a0, +* offset 0x00a8, +* offset 0x00b0, +* offset 0x00b8, +* offset 0x00c0 +*/ /* Rate_data is the calculated rate data. It is a first derivative * calculation. It is calculated at a frequency specified by the
Signature · 1 line
2.7.4
PATCH 3/6 staging: comedi: ni_atmio.c: Fix checkpatch warning
In reply to Anson JacobFix checkpatch.pl warning for 'Statements should start on a tabstop'
--- drivers/staging/comedi/drivers/ni_atmio.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/staging/comedi/drivers/ni_atmio.c +2 -2
@@ -278,10 +278,10 @@ static const struct ni_board_struct *ni_atmio_probe(struct comedi_device *dev) }if (device_id ==255 )dev_err (dev->class_dev,"can't find board\n" ); -else if (device_id ==0 ) +else if (device_id ==0 )dev_err (dev->class_dev,"EEPROM read error (?) or device not found\n" ); -else +else dev_err (dev->class_dev,"unknown device ID %d -- contact author\n" , device_id);
Signature · 1 line
2.7.4
PATCH 4/6 staging: comedi: s626.h: Fix checkpatch warning
In reply to Anson JacobFix checkpatch.pl warning for
Comparisons should place the constant on the right side of the test
--- drivers/staging/comedi/drivers/s626.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/comedi/drivers/s626.h +1 -1
@@ -329,7 +329,7 @@* WS1-WS4 = CS* outputs. */ -#if S626_PLATFORM ==S626_INTEL /* +#if (S626_PLATFORM ==S626_INTEL )/* * Base ACON1 config: always run * A1 based on TSL1. */
Signature · 1 line
2.7.4
PATCH 5/6 staging: comedi: jr3_pci.c: Fix checkpatch warning
In reply to Anson JacobFix checkpatch.pl warning 'line over 80 characters'
--- drivers/staging/comedi/drivers/jr3_pci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/staging/comedi/drivers/jr3_pci.c +2 -1
@@ -448,7 +448,8 @@ static int jr3_download_firmware(struct comedi_device *dev,return 0 ; } -static struct jr3_pci_poll_delay jr3_pci_poll_subdevice (struct comedi_subdevice *s) +static struct jr3_pci_poll_delay +jr3_pci_poll_subdevice (struct comedi_subdevice *s) {struct jr3_pci_subdev_private *spriv = s->private;struct jr3_pci_poll_delay result =poll_delay_min_max (1000 ,2000 );
Signature · 1 line
2.7.4
PATCH 6/6 staging: comedi: ni_at_a2150.c: Fix checkpatch warning
In reply to Anson JacobFix checkpatch.pl warning 'line over 80 characters'
--- drivers/staging/comedi/drivers/ni_at_a2150.c | 82 ++++++++++++++++------------ 1 file changed, 46 insertions(+), 36 deletions(-)
drivers/staging/comedi/drivers/ni_at_a2150.c +46 -36
@@ -58,41 +58,49 @@/* Registers and bits */ #define CONFIG_REG 0x0 -#define CHANNEL_BITS (x) ((x) & 0x7) -#define CHANNEL_MASK 0x7 -#define CLOCK_SELECT_BITS (x) (((x) & 0x3) << 3) -#define CLOCK_DIVISOR_BITS (x) (((x) & 0x3) << 5) -#define CLOCK_MASK (0xf << 3) -#define ENABLE0_BIT 0x80/* enable (don't internally ground) channels 0 and 1 */ -#define ENABLE1_BIT 0x100/* enable (don't internally ground) channels 2 and 3 */ -#define AC0_BIT 0x200/* ac couple channels 0,1 */ -#define AC1_BIT 0x400/* ac couple channels 2,3 */ -#define APD_BIT 0x800/* analog power down */ -#define DPD_BIT 0x1000/* digital power down */ -#define TRIGGER_REG 0x2/* trigger config register */ -#define POST_TRIGGER_BITS 0x2 -#define DELAY_TRIGGER_BITS 0x3 -#define HW_TRIG_EN 0x10/* enable hardware trigger */ -#define FIFO_START_REG 0x6/* software start aquistion trigger */ -#define FIFO_RESET_REG 0x8/* clears fifo + fifo flags */ -#define FIFO_DATA_REG 0xa/* read data */ -#define DMA_TC_CLEAR_REG 0xe/* clear dma terminal count interrupt */ -#define STATUS_REG 0x12/* read only */ -#define FNE_BIT 0x1/* fifo not empty */ -#define OVFL_BIT 0x8/* fifo overflow */ -#define EDAQ_BIT 0x10/* end of acquisition interrupt */ -#define DCAL_BIT 0x20/* offset calibration in progress */ -#define INTR_BIT 0x40/* interrupt has occurred */ -#define DMA_TC_BIT 0x80/* dma terminal count interrupt has occurred */ -#define ID_BITS (x) (((x) >> 8) & 0x3) -#define IRQ_DMA_CNTRL_REG 0x12/* write only */ -#define DMA_CHAN_BITS (x) ((x) & 0x7)/* sets dma channel */ -#define DMA_EN_BIT 0x8/* enables dma */ -#define IRQ_LVL_BITS (x) (((x) & 0xf) << 4)/* sets irq level */ -#define FIFO_INTR_EN_BIT 0x100/* enable fifo interrupts */ -#define FIFO_INTR_FHF_BIT 0x200/* interrupt fifo half full */ -#define DMA_INTR_EN_BIT 0x800/* enable interrupt on dma terminal count */ -#define DMA_DEM_EN_BIT 0x1000/* enables demand mode dma */ +#define CHANNEL_BITS (x) ((x) & 0x7) +#define CHANNEL_MASK 0x7 +#define CLOCK_SELECT_BITS (x) (((x) & 0x3) << 3) +#define CLOCK_DIVISOR_BITS (x) (((x) & 0x3) << 5) +#define CLOCK_MASK (0xf << 3) +#define ENABLE0_BIT 0x80/* enable (don't internally ground) +* channels 0 and 1 +*/ +#define ENABLE1_BIT 0x100/* enable (don't internally ground) +* channels 2 and 3 +*/ +#define AC0_BIT 0x200/* ac couple channels 0,1 */ +#define AC1_BIT 0x400/* ac couple channels 2,3 */ +#define APD_BIT 0x800/* analog power down */ +#define DPD_BIT 0x1000/* digital power down */ +#define TRIGGER_REG 0x2/* trigger config register */ +#define POST_TRIGGER_BITS 0x2 +#define DELAY_TRIGGER_BITS 0x3 +#define HW_TRIG_EN 0x10/* enable hardware trigger */ +#define FIFO_START_REG 0x6/* software start aquistion trigger */ +#define FIFO_RESET_REG 0x8/* clears fifo + fifo flags */ +#define FIFO_DATA_REG 0xa/* read data */ +#define DMA_TC_CLEAR_REG 0xe/* clear dma terminal count interrupt */ +#define STATUS_REG 0x12/* read only */ +#define FNE_BIT 0x1/* fifo not empty */ +#define OVFL_BIT 0x8/* fifo overflow */ +#define EDAQ_BIT 0x10/* end of acquisition interrupt */ +#define DCAL_BIT 0x20/* offset calibration in progress */ +#define INTR_BIT 0x40/* interrupt has occurred */ +#define DMA_TC_BIT 0x80/* dma terminal count interrupt has +* occurred +*/ +#define ID_BITS (x) (((x) >> 8) & 0x3) +#define IRQ_DMA_CNTRL_REG 0x12/* write only */ +#define DMA_CHAN_BITS (x) ((x) & 0x7)/* sets dma channel */ +#define DMA_EN_BIT 0x8/* enables dma */ +#define IRQ_LVL_BITS (x) (((x) & 0xf) << 4)/* sets irq level */ +#define FIFO_INTR_EN_BIT 0x100/* enable fifo interrupts */ +#define FIFO_INTR_FHF_BIT 0x200/* interrupt fifo half full */ +#define DMA_INTR_EN_BIT 0x800/* enable interrupt on dma terminal +* count +*/ +#define DMA_DEM_EN_BIT 0x1000/* enables demand mode dma */ #define I8253_BASE_REG 0x14struct a2150_board {
@@ -550,7 +558,9 @@ static int a2150_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s)if (cmd->start_src ==TRIG_EXT ) { trigger_bits |=HW_TRIG_EN ; }else if (cmd->start_src ==TRIG_OTHER ) { -/* XXX add support for level/slope start trigger using TRIG_OTHER */ +/* XXX add support for level/slope start trigger +* using TRIG_OTHER +*/ dev_err (dev->class_dev,"you shouldn't see this?\n" ); }/* send trigger config bits */
Signature · 1 line
2.7.4
Re: [PATCH 6/6] staging: comedi: ni_at_a2150.c: Fix checkpatch warning
Quoted 12 lines · Anson Jacob
Fix checkpatch.pl warning 'line over 80 characters'
Signed-off-by Anson Jacob <[email protected]>--- drivers/staging/comedi/drivers/ni_at_a2150.c | 82 ++++++++++++++++------------ 1 file changed, 46 insertions(+), 36 deletions(-)drivers/staging/comedi/drivers/ni_at_a2150.c +0
-0
drivers/staging/comedi/drivers/ni_at_a2150.c line 63
Quoted 6 lines
drivers/staging/comedi/drivers/ni_at_a2150.c +0
-3@@ -58,41 +58,49 @@/* Registers and bits */ #define CONFIG_REG 0x0 -#define CHANNEL_BITS (x) ((x) & 0x7) -#define CHANNEL_MASK 0x7 -#define CLOCK_SELECT_BITS (x) (((x) & 0x3) << 3)
You have lost this treeish look which carried information about thing being a register or a field.
drivers/staging/comedi/drivers/ni_at_a2150.c line 62
Quoted 2 lines
drivers/staging/comedi/drivers/ni_at_a2150.c +2
-0+#define CHANNEL_BITS (x) ((x) & 0x7) +#define CHANNEL_MASK 0x7
No uniform alignment. Please get everything in a row.
If it's hard or this part of driver is expected to have a lot of changes in near future, then I'd remove any whitespace over single space between name and value.
drivers/staging/comedi/drivers/ni_at_a2150.c line 71
Quoted 6 lines
drivers/staging/comedi/drivers/ni_at_a2150.c +6
-0+#define ENABLE0_BIT 0x80/* enable (don't internally ground) +* channels 0 and 1 +*/ +#define ENABLE1_BIT 0x100/* enable (don't internally ground) +* channels 2 and 3 +*/
This commenting style is discouraged (Linus has even stated explicitly that he dislikes it, even for pieces of code which historically had this style everywhere). Opening "/*" should not be followed by text.
Also, In my personal experience, it is more stable to have these comments on previous line, not at end of the line. This way you always have enough space for both a comment and a macro.
Re: [PATCH 6/6] staging: comedi: ni_at_a2150.c: Fix checkpatch warning
Thank you for your comments.
Quoted 9 lines · Andrey Utkin
drivers/staging/comedi/drivers/ni_at_a2150.c line 62
Quoted 3 lines
drivers/staging/comedi/drivers/ni_at_a2150.c +2
-0+#define CHANNEL_BITS (x) ((x) & 0x7) +#define CHANNEL_MASK 0x7No uniform alignment. Please get everything in a row.
If it's hard or this part of driver is expected to have a lot of changes in near future, then I'd remove any whitespace over single space between name and value.
I didn't get your point in this case. Could you explain it.
Re: [PATCH 6/6] staging: comedi: ni_at_a2150.c: Fix checkpatch warning
Quoted 11 lines · Anson Jacob
Quoted 9 lines · Andrey Utkin
drivers/staging/comedi/drivers/ni_at_a2150.c line 62
Quoted 3 lines
drivers/staging/comedi/drivers/ni_at_a2150.c +2
-0+#define CHANNEL_BITS (x) ((x) & 0x7) +#define CHANNEL_MASK 0x7No uniform alignment. Please get everything in a row.
If it's hard or this part of driver is expected to have a lot of changes in near future, then I'd remove any whitespace over single space between name and value.
Quoted 1 line
I didn't get your point in this case. Could you explain it.
Reviewing actual source code with the patch applied, it looks well. But if you look at the patch email itself, or in "git show <commit>, the alignment is broken. This is an illustration why using tab characters in the middle of lines is bad idea. However this is what I see in lots of kernel code.
So feel free to ignore this point.