[PATCH 000/108] staging: comedi: addi_apci_3120: cleanup driver
Ian Abbott
abbotti at mev.co.uk
Wed Nov 5 14:48:29 UTC 2014
On 04/11/14 17:53, H Hartley Sweeten wrote:
> Following is the big cleanup series for the ADDI-DATA APCI-3120 driver.
>
> Quick summary of the cleanup:
>
> * Removes all the CamelCase
> * Cleans up the register map defines
> * Fixes the Analog Input subdevice
> * Fixes the async command support for the Analog Input subdevice
> * Cleans up all the timer support code
> * Cleans up the DMA support code
> * Removes the included addi-data/hwdrv_apci3120.c source file
> * Removes all the cruft
>
> H Hartley Sweeten (108):
> staging: comedi: addi_apci_3120: introduce apci3120_ns_to_timer()
> staging: comedi: addi_apci_3120: rename private data 'b_DigitalOutputRegister'
> staging: comedi: addi_apci_3120: introduce apci3120_timer_write()
> staging: comedi: addi_apci_3120: introduce apci3120_timer_read()
> staging: comedi: addi_apci_3120: tidy up CTR0 register defines
> staging: comedi: addi_apci_3120: fix counter and external interrupt disable
> staging: comedi: addi_apci_3120: rename APCI3120_TIMER_VALUE
> staging: comedi: addi_apci_3120: rename private data 'b_TimerSelectMode'
> staging: comedi: addi_apci_3120: tidy up timer_mode masking
> staging: comedi: addi_apci_3120: introduce apci3120_timer_set_mode()
> staging: comedi: addi_apci_3120: move timer helpers to main driver source
> staging: comedi: addi_apci_3120: rename private data 'us_OutputRegister'
> staging: comedi: addi_apci_3120: tidy up devpriv->ctrl use
> staging: comedi: addi_apci_3120: remove APCI3120_DISABLE_TIMER[012]
> staging: comedi: addi_apci_3120: tidy up APCI3120_ENABLE_TIMER[012]
> staging: comedi: addi_apci_3120: rename APCI3120_ENABLE_EXT_TRIGGER
> staging: comedi: addi_apci_3120: tidy up apci3120_exttrig_{enable,disable}()
> staging: comedi: addi_apci_3120: introduce apci3120_timer_enable()
> staging: comedi: addi_apci_3120: fix timer 2 disable in apci3120_write_insn_timer()
> staging: comedi: addi_apci_3120: rename APCI3120_WR_ADDRESS
> staging: comedi: addi_apci_3120: move apci3120_timer_enable() to driver source
> staging: comedi: addi_apci_3120: move apci3120_exttrig_enable() to driver source
> staging: comedi: addi_apci_3120: introduce apci3120_clr_timer2_interrupt()
> staging: comedi: addi_apci_3120: remove unnecessary reset of the scan sequence
> staging: comedi: addi_apci_3120: tidy up scan chanlist programming
> staging: comedi: addi_apci_3120: remove 'check' param from apci3120_setup_chan_list()
> staging: comedi: addi_apci_3120: introduce apci3120_ai_reset_fifo()
> staging: comedi: addi_apci_3120: move ai range table to driver source
> staging: comedi: addi_apci_3120: remove APCI3120_DISABLE_ALL_INTERRUPT_WITHOUT_TIMER
> staging: comedi: addi_apci_3120: properly disable interrupts in apci3120_cancel()
> staging: comedi: addi_apci_3120: rename private data 'b_ModeSelectRegister'
> staging: comedi: addi_apci_3120: remove unnecessary devpriv->mode masking
> staging: comedi: addi_apci_3120: remove devpriv->mode '0xef' magic value
> staging: comedi: addi_apci_3120: remove APCI3120_DISABLE_TIMER_COUNTER
> staging: comedi: addi_apci_3120: remove APCI3120_DISABLE_WATCHDOG
> staging: comedi: addi_apci_3120: remove APCI3120_DISABLE_TIMER_INT
> staging: comedi: addi_apci_3120: remove APCI3120_DISABLE_EOC_INT
> staging: comedi: addi_apci_3120: remove APCI3120_DISABLE_EOS_INT
> staging: comedi: addi_apci_3120: remove APCI3120_DISABLE_SCAN
> staging: comedi: addi_apci_3120: define the "enable" bits in the mode register
> staging: comedi: addi_apci_3120: define the timer 2 operation bits
> staging: comedi: addi_apci_3120: define the timer 2 clock select bits
> staging: comedi: addi_apci_3120: rename APCI3120_WRITE_MODE_SELECT
> staging: comedi: addi_apci_3120: remove scanning from ai (*insn_read)
> staging: comedi: addi_apci_3120: remove private data 'ui_EocEosConversionTime'
> staging: comedi: addi_apci_3120: remove interrupt support from ai (*insn_read)
> staging: comedi: addi_apci_3120: remove apci3120_ai_insn_config()
> staging: comedi: addi_apci_3120: remove private data 'ui_AiReadData'
> staging: comedi: addi_apci_3120: fix apci3120_ai_insn_read()
> staging: comedi: addi_apci_3120: absorb apci3120_interrupt_handle_eos()
> staging: comedi: addi_apci_3120: remove private data 'ui_AiNbrofChannels'
> staging: comedi: addi_apci_3120: remove private data 'ui_AiChannelList'
> staging: comedi: addi_apci_3120: rename APCI3120_RD_STATUS
> staging: comedi: addi_apci_3120: define status register bits
> staging: comedi: addi_apci_3120: remove private data 'ai_running'
> staging: comedi: addi_apci_3120: move apci3120_do_insn_bits() to driver source
> staging: comedi: addi_apci_3120: move apci3120_di_insn_bits() to driver source
> staging: comedi: addi_apci_3120: move apci3120_ao_insn_write() to driver source
> staging: comedi: addi_apci_3120: move apci3120_ai_insn_read() to driver source
> staging: comedi: addi_apci_3120: remove check in apci3120_setup_chan_list()
> staging: comedi: addi_apci_3120: move apci3120_set_chanlist() to driver source
> staging: comedi: addi_apci_3120: factor DMA setup out of apci3120_cyclic_ai()
> staging: comedi: addi_apci_3120: remove APCI3120_{ENABLE,DISABLE}
> staging: comedi: addi_apci_3120: flip 'us_UseDma' test in apci3120_cyclic_ai()
> staging: comedi: addi_apci_3120: move timer 2 enable in apci3120_cyclic_ai()
> staging: comedi: addi_apci_3120: move start_src check into apci3120_cyclic_ai()
> staging: comedi: addi_apci_3120: absorb apci3120_cyclic_ai()
> staging: comedi: addi_apci_3120: tidy up timer programming in apci3120_ai_cmd()
> staging: comedi: addi_apci_3120: tidy up timer 2 programming in apci3120_ai_cmd()
> staging: comedi: addi_apci_3120: reset fifo after programming chanlist
> staging: comedi: addi_apci_3120: set scan length/start after programming chanlist
> staging: comedi: addi_apci_3120: enable chanlist scanning if needed
> staging: comedi: addi_apci_3120: tidy up devpriv->mode in apci3120_ai_cmd()
> staging: comedi: addi_apci_3120: remove private data 'b_InterruptMode'
> staging: comedi: addi_apci_3120: remove private data 'b_ExttrigEnable'
> staging: comedi: addi_apci_3120: introduce apci3120_init_dma()
> staging: comedi: addi_apci_3120: introduce apci3120_addon_write()
> staging: comedi: addi_apci_3120: use amcc_s5933.h defines
> staging: comedi: addi_apci_3120: define the Add-On registers
> staging: comedi: addi_apci_3120: move APCI3120_FIFO_ADVANCE_ON_BYTE_2
> staging: comedi: addi_apci_3120: move DMA init code to apci3120_init_dma()
> staging: comedi: addi_apci_3120: tidy up apci3120_reset()
> staging: comedi: addi_apci_3120: move apci3120_reset() to driver source
> staging: comedi: addi_apci_3120: rename private data 'us_UseDma'
> staging: comedi: addi_apci_3120: rename private data 'b_DmaDoubleBuffer'
> staging: comedi: addi_apci_3120: rename private data 'ui_DmaActualBuffer'
> staging: comedi: addi_apci_3120: don't use timer 2 to count scans
> staging: comedi: addi_apci_3120: define the AI FIFO register
> staging: comedi: addi_apci_3120: define the AI software trigger register
> staging: comedi: addi_apci_3120: fix timer (*insn_read)
> staging: comedi: addi_apci_3120: fix timer (*insn_config)
> staging: comedi: addi_apci_3120: fix cmd->convert_arg vaildation
> staging: comedi: addi_apci_3120: move AI (*do_cmdtest) to main driver
> staging: comedi: addi_apci_3120: add copyright information
> staging: comedi: addi_apci_3120: remove unnecessary include
> staging: comedi: addi_apci_3120: move apci3120_addon_write() to driver
> staging: comedi: addi_apci_3120: move apci3120_init_dma() to driver
> staging: comedi: addi_apci_3120: move apci3120_setup_dma() to driver
> staging: comedi: addi_apci_3120: move apci3120_ai_cmd() to driver
> staging: comedi: addi_apci_3120: use async->events to report hardware error
> staging: comedi: addi_apci_3120: move apci3120_cancel() to driver
> staging: comedi: addi_apci_3120: move apci3120_interrupt() to driver
> staging: comedi: addi_apci_3120: use comedi_bytes_to_samples()
> staging: comedi: addi_apci_3120: move apci3120_interrupt_dma() to driver
> staging: comedi: addi_apci_3120: change params to apci3120_interrupt_dma()
> staging: comedi: addi_apci_3120: switch DMA buffers after writing samples
> staging: comedi: addi_apci_3120: enable AI async commands
> staging: comedi: addi_apci_3120: absorb apci3120_ai_reset_fifo()
>
> .../comedi/drivers/addi-data/hwdrv_apci3120.c | 1903 --------------------
> drivers/staging/comedi/drivers/addi_apci_3120.c | 912 +++++++++-
> drivers/staging/comedi/drivers/amcc_s5933.h | 2 +
> 3 files changed, 885 insertions(+), 1932 deletions(-)
> delete mode 100644 drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c
>
You've been busy! I think I'll review this as a whole rather than
individually.
My only comments are:
1) It would be preferable to not switch on the I8254_MODE<n> constants
for the timer mode in PATCH 091 as they're pretty irrelevant here.
Rather, just add some new constants to comedi.h - perhaps the
APCI3120_TIMER_MODE<n> constants introduced in PATCH 010.
2) The "IRQ from unknown source\n" message should be removed to avoid
spamming the kernel log since this is likely to be due to an interrupt
from a different device on the same IRQ number. (That was copied over
from the original code by PATCH 102.)
3) A comedi driver header comment would be nice!
Overall, a splendid job!
Reviewed-by: Ian Abbott <abbotti at mev.co.uk>
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti at mev.co.uk> )=-
-=( Web: http://www.mev.co.uk/ )=-
More information about the devel
mailing list