RE: [PATCH 2/3] staging: comedi: cb_pcidas64: Use insn->n in EEPROM insn_read handler

[Hartley Sweeten]

On Tuesday, October 30, 2018 7:17 AM, Ian Abbott wrote:
> The `insn_read` handler for the EEPROM subdevice (`eeprom_insn_read()`) 
> currently
> ignores `insn->n` (the number of samples to be read) and assumes a single 
> sample is
> to be read.  But `insn->n` could be 0, meaning no samples should be read, in 
> which
> case `data[0]` ought not to be written.  (The comedi core at least ensures 
> that
> `data[0]` exists, but we should not rely on that.)
>
> Follow the usual Comedi guidelines and interpret `insn->n` as the number of 
> samples
> to be read, but only read the EEPROM location once and make `insn->n` copies, 
> as we
> don't expect the contents of the EEPROM location to change between readings.
>
> Signed-off-by: Ian Abbott 
> ---
> drivers/staging/comedi/drivers/cb_pcidas64.c | 12 ++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c 
> b/drivers/staging/comedi/drivers/cb_pcidas64.c
> index 44e5aaf8bae5..e1774e09a320 100644
> --- a/drivers/staging/comedi/drivers/cb_pcidas64.c
> +++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
> @@ -3768,9 +3768,17 @@ static int eeprom_read_insn(struct comedi_device *dev,
>   struct comedi_subdevice *s,
>   struct comedi_insn *insn, unsigned int *data)  {
> - data[0] = read_eeprom(dev, CR_CHAN(insn->chanspec));
> + unsigned int val;
> + unsigned int i;
> 
> - return 1;
> + if (insn->n) {
> + /* No point reading the same EEPROM location more than once. */
> + val = read_eeprom(dev, CR_CHAN(insn->chanspec));
> + for (i = 0; i < insn->n; i++)
> + data[i] = val;
> + }
> +
> + return insn->n;
> }
 
Hi Ian,

I realize it's not the "normal" Comedi use, but with the EEPROM I would think 
that if the user
wanted to read more than one sample they would expect to read a block of data 
from the
EEPROM.

Maybe the EEPROM read should be something like:

+   if (insn->n) {
+   unsigned int address = CR_CHAN(insn->chanspec);
+   for (i = 0; i < insn->n; i++) {
+   val = read_eeprom(dev, address);
+   data[i] = val;
+   address++;
+   if (address >= s->n_chan)
+   address = 0;
+   }
+   }

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] Staging: comedi: das16: Fixed a const struct coding style issue

[Hartley Sweeten]

On Monday, November 27, 2017 3:28 AM, Ian Abbott wrote:
> On 26/11/17 01:50, Alex Frappier Lachapelle wrote:
>> +const struct comedi_lrange *lrange;
>
> NAK.  The following lines of source code allocate memory pointed to by 
> 'lrange' and modify it, so 'const' is not appropriate here.

Ian,

Wonder if it's worth putting a comment about this in the code. This has come up 
a
couple times.

Regards,
Hartley
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] mtd: nand: Rename nand.h into rawnand.h

[Hartley Sweeten]

On Friday, August 04, 2017 8:29 AM, Boris Brezillon wrote:
> We are planning to share more code between different NAND based
> devices (SPI NAND, OneNAND and raw NANDs), but before doing that
> we need to move the existing include/linux/mtd/nand.h file into
> include/linux/mtd/rawnand.h so we can later create a nand.h header
> containing all common structure and function prototypes.

For ep93xx,

Acked-by: H Hartley Sweeten <hswee...@visionengravers.com>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 00/14] staging: comedi: daqboard2000: some clean-up

[Hartley Sweeten]

On Wednesday, January 04, 2017 3:56 AM, Ian Abbott wrote:
> I've had these clean-up patches for the daqboard2000 driver sitting
> around for a few months, with a view to adding support for more boards
> in the same series, and adding support for extra subdevice types.  I
> never got around to doing that, but I did manage to add support for a
> couple of extra DAC channels on one of the two currently supported card
> types.

I'm still not positive that this driver works correctly. The adc/dac
initialization code appears to be incomplete and the ai (*insn_read)
is pretty odd.

But, thanks for cleaning this one up!

Reviewed-by: H Hartley Sweeten <hswee...@visionengravers.com>

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 00/12] staging: comedi: add identifiers to function parameters

[Hartley Sweeten]

On Thursday, December 15, 2016 6:20 AM, Ian Abbott wrote:
> Fix checkpatch.pl warnings of the form "function definition argument
> 'foo' should also have an identifier name" in various comedi header
> files.
>
> I omitted comedi_pci.h because Piotr Gregor already submitted a patch
> for that one.
>
> 01) staging: comedi: addi_watchdog.h: add identifiers to function
> parameters
> 02) staging: comedi: comedi_8254.h: add identifiers to function
> parameters
> 03) staging: comedi: comedi_isadma.h: add identifiers to function
> parameters
> 04) staging: comedi: mite.h: add identifiers to function parameters
> 05) staging: comedi: ni_labpc.h: add identifiers to function parameters
> 06) staging: comedi: ni_tio.h: add identifiers to function parameters
> 07) staging: comedi: ni_tio_internal.h: add identifiers to function
> parameters
> 08) staging: comedi: comedi_compat32.h: add identifiers to function
> parameters
> 09) staging: comedi: comedi_internal.h: add identifiers to function
> parameters
> 10) staging: comedi: comedi_pcmcia.[ch]: add identifiers to function
> parameters
> 11) staging: comedi: comedi_usb.h: add identifiers to function
> parameters
> 12) staging: comedi: comedidev.h: add identifiers to function
> parameters

Reviewed-by: H Hartley Sweeten <hswee...@visionengravers.com>

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] drivers: staging: comedi: fix function prototypes

[Hartley Sweeten]

On Thursday, December 15, 2016 4:45 AM, Ian Abbott wrote:
> On 14/12/16 13:42, Piotr Gregor wrote:
>> Add names of parameters to function prototypes in comedi PCI.
>> Checkpatch reports now no errors.
>>
>> Signed-off-by: Piotr Gregor <piotrgre...@rsyncme.org>
>
> Looks good, thanks!
>
> Reviewed-by: Ian Abbott <abbo...@mev.co.uk>

Reviewed-by: H Hartley Sweeten <hswee...@visionengravers.com>

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] drivers: staging: comedi: fix function prototypes

[Hartley Sweeten]

On Thursday, December 15, 2016 4:47 AM, Ian Abbott wrote:
> On 14/12/16 16:14, Hartley Sweeten wrote:
>> On December 14, 2016 6:42 AM, Piotr Gregor wrote:
>>> -struct pci_dev *comedi_to_pci_dev(struct comedi_device *);
>>> +struct pci_dev *comedi_to_pci_dev(struct comedi_device *dev);
>>
>> For the function prototypes I prefer no names for the "pointer" parameters.
>>
>> The "struct foo *" declaration is just as clear as "struct foo *bar".
>
> Maybe, but checkpatch.pl doesn't agree (not since commit 
> ca0d8929e75ab1f860f61208d46955f280a1b05e anyway)!

Hmm.. Missed seeing that one go in.

I still think it's silly to name struct pointers in function arguments.
Especially since that normally leads to stuff like 'struct foo *foo' where
the parameter name is the same as the struct name.

Void pointers and generic types are a different matter. Naming those
makes sense for clarity.

Oh well... Just my 2 cents...

Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] drivers: staging: comedi: fix function prototypes

[Hartley Sweeten]

On December 14, 2016 6:42 AM, Piotr Gregor wrote:
> Add names of parameters to function prototypes in comedi PCI.
> Checkpatch reports now no errors.
>
> Signed-off-by: Piotr Gregor 
> ---
>  drivers/staging/comedi/comedi_pci.h | 18 ++
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/comedi/comedi_pci.h 
> b/drivers/staging/comedi/comedi_pci.h
> index 4005cc9..7dfd892 100644
> --- a/drivers/staging/comedi/comedi_pci.h
> +++ b/drivers/staging/comedi/comedi_pci.h
> @@ -34,18 +34,20 @@
>  #define PCI_VENDOR_ID_RTD0x1435
>  #define PCI_VENDOR_ID_HUMUSOFT   0x186c
>  
> -struct pci_dev *comedi_to_pci_dev(struct comedi_device *);
> +struct pci_dev *comedi_to_pci_dev(struct comedi_device *dev);
 
For the function prototypes I prefer no names for the "pointer" parameters.

The "struct foo *" declaration is just as clear as "struct foo *bar".

Thanks,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH] staging: comedi: ni_mio_common: fix AO inttrig backwards compatibility

[Hartley Sweeten]

On Tuesday, July 19, 2016 4:18 AM, Ian Abbott wrote:
> Commit ebb657babfa9 ("staging: comedi: ni_mio_common: clarify the
> cmd->start_arg validation and use") introduced a backwards compatibility
> issue in the use of asynchronous commands on the AO subdevice when
> `start_src` is `TRIG_EXT`.  Valid values for `start_src` are `TRIG_INT`
> (for internal, software trigger), and `TRIG_EXT` (for external trigger).
> When set to `TRIG_EXT`.  In both cases, the driver relies on an
> internal, software trigger to set things up (allowing the user
> application to write sufficient samples to the data buffer before the
> trigger), so it acts as a software "pre-trigger" in the `TRIG_EXT` case.
> The software trigger is handled by `ni_ao_inttrig()`.
>
> Prior to the above change, when `start_src` was `TRIG_INT`, `start_arg`
> was required to be 0, and `ni_ao_inttrig()` checked that the software
> trigger number was also 0.  After the above change, when `start_src` was
> `TRIG_INT`, any value was allowed for `start_arg`, and `ni_ao_inttrig()`
> checked that the software trigger number matched this `start_arg` value.
> The backwards compatibility issue is that the internal trigger number
> now has to match `start_arg` when `start_src` is `TRIG_EXT` when it
> previously had to be 0.
>
> Fix the backwards compatibility issue in `ni_ao_inttrig()` by always
> allowing software trigger number 0 when `start_src` is something other
> than `TRIG_INT`.
>
> Thanks to Spencer Olson for reporting the issue.
>
> Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
> Reported-by: Spencer Olson <olso...@umich.edu>
> Fixes: ebb657babfa9 ("staging: comedi: ni_mio_common: clarify the
>   cmd->start_arg validation and use")
> ---
>  drivers/staging/comedi/drivers/ni_mio_common.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c 
> b/drivers/staging/comedi/drivers/ni_mio_common.c
> index 8dabb19..9f4036f 100644
> --- a/drivers/staging/comedi/drivers/ni_mio_common.c
> +++ b/drivers/staging/comedi/drivers/ni_mio_common.c
> @@ -2772,7 +2772,15 @@ static int ni_ao_inttrig(struct comedi_device *dev,
>   int i;
>   static const int timeout = 1000;
>  
> - if (trig_num != cmd->start_arg)
> + /*
> +  * Require trig_num == cmd->start_arg when cmd->start_src == TRIG_INT.
> +  * For backwards compatibility, also allow trig_num == 0 when
> +  * cmd->start_src != TRIG_INT (i.e. when cmd->start_src == TRIG_EXT);
> +  * in that case, the internal trigger is being used as a pre-trigger
> +  * before the external trigger.
> +  */
> + if (!(trig_num == cmd->start_arg ||
> +   (trig_num == 0 && cmd->start_src != TRIG_INT)))
>   return -EINVAL;

Ian,

I think this test is a bit clearer:

+   /*
+* Require trig_num == cmd->start_arg when cmd->start_src == TRIG_INT.
+* For backwards compatibility, any trig_num is valid when
+* cmd->start_src != TRIG_INT (i.e. when cmd->start_src == TRIG_EXT);
+* in that case, the internal trigger is being used as a pre-trigger
+* before the external trigger.
+*/
+   if (cmd->start_src == TRIG_INT && trig_num != cmd->start_arg)
return -EINVAL;

But, either way:

Reviewed-by: H Hartley Sweeten <hswee...@visionengravers.com>

Thanks!

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/9] staging: comedi: ni_pcimio: fix block comments

[H Hartley Sweeten]

Fix the checkpatch.pl issues:
WARNING: Block comments use * on subsequent lines

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/ni_pcimio.c | 216 ++---
 1 file changed, 106 insertions(+), 110 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_pcimio.c 
b/drivers/staging/comedi/drivers/ni_pcimio.c
index d891739..f13a2f7 100644
--- a/drivers/staging/comedi/drivers/ni_pcimio.c
+++ b/drivers/staging/comedi/drivers/ni_pcimio.c
@@ -1,111 +1,106 @@
 /*
-comedi/drivers/ni_pcimio.c
-Hardware driver for NI PCI-MIO E series cards
+ * Comedi driver for NI PCI-MIO E series cards
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 1997-8 David A. Schleef <d...@schleef.org>
+ *
+ * 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 - Linux Control and Measurement Device Interface
-Copyright (C) 1997-8 David A. Schleef <d...@schleef.org>
-
-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: ni_pcimio
-Description: National Instruments PCI-MIO-E series and M series (all boards)
-Author: ds, John Hallen, Frank Mori Hess, Rolf Mueller, Herbert Peremans,
-  Herman Bruyninckx, Terry Barnaby
-Status: works
-Devices: [National Instruments] PCI-MIO-16XE-50 (ni_pcimio),
-  PCI-MIO-16XE-10, PXI-6030E, PCI-MIO-16E-1, PCI-MIO-16E-4, PCI-6014, 
PCI-6040E,
-  PXI-6040E, PCI-6030E, PCI-6031E, PCI-6032E, PCI-6033E, PCI-6071E, PCI-6023E,
-  PCI-6024E, PCI-6025E, PXI-6025E, PCI-6034E, PCI-6035E, PCI-6052E,
-  PCI-6110, PCI-6111, PCI-6220, PCI-6221, PCI-6224, PXI-6224,
-  PCI-6225, PXI-6225, PCI-6229, PCI-6250,
-  PCI-6251, PXI-6251, PCIe-6251, PXIe-6251,
-  PCI-6254, PCI-6259, PCIe-6259,
-  PCI-6280, PCI-6281, PXI-6281, PCI-6284, PCI-6289,
-  PCI-6711, PXI-6711, PCI-6713, PXI-6713,
-  PXI-6071E, PCI-6070E, PXI-6070E,
-  PXI-6052E, PCI-6036E, PCI-6731, PCI-6733, PXI-6733,
-  PCI-6143, PXI-6143
-Updated: Mon, 09 Jan 2012 14:52:48 +
-
-These boards are almost identical to the AT-MIO E series, except that
-they use the PCI bus instead of ISA (i.e., AT).  See the notes for
-the ni_atmio.o driver for additional information about these boards.
-
-Autocalibration is supported on many of the devices, using the
-comedi_calibrate (or comedi_soft_calibrate for m-series) utility.
-M-Series boards do analog input and analog output calibration entirely
-in software. The software calibration corrects
-the analog input for offset, gain and
-nonlinearity.  The analog outputs are corrected for offset and gain.
-See the comedilib documentation on comedi_get_softcal_converter() for
-more information.
-
-By default, the driver uses DMA to transfer analog input data to
-memory.  When DMA is enabled, not all triggering features are
-supported.
-
-Digital I/O may not work on 673x.
-
-Note that the PCI-6143 is a simultaineous sampling device with 8 convertors.
-With this board all of the convertors perform one simultaineous sample during
-a scan interval. The period for a scan is used for the convert time in a
-Comedi cmd. The convert trigger source is normally set to TRIG_NOW by default.
-
-The RTSI trigger bus is supported on these cards on
-subdevice 10. See the comedilib documentation for details.
-
-Information (number of channels, bits, etc.) for some devices may be
-incorrect.  Please check this and submit a bug if there are problems
-for your device.
-
-SCXI is probably broken for m-series boards.
-
-Bugs:
- - When DMA is enabled, COMEDI_EV_CONVERT does
-   not work correctly.
+ * Driver: ni_pcimio
+ * Description: National Instruments PCI-MIO-E series and M series (all boards)
+ * Author: ds, John Hallen, Frank Mori Hess, Rolf Mueller, Herbert Peremans,
+ *   Herman Bruyninckx, Terry Barnaby
+ * Status: works
+ * Devices: [National Instruments] PCI-MIO-16XE-50 (ni_pcimio),
+ *   PCI-MIO-16XE-10, PXI-6030E, PCI-MIO-16E-1, PCI-MIO-16E-4, PCI-6014,
+ *   PCI-6040E, PXI-60

[PATCH 9/9] staging: comedi: ni_at_a2150: fix block comments

[H Hartley Sweeten]

Fix the checkpatch.pl issues:
WARNING: Block comments use * on subsequent lines
WARNING: Block comments use a trailing */ on a separate line

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/ni_at_a2150.c | 103 ---
 1 file changed, 45 insertions(+), 58 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_at_a2150.c 
b/drivers/staging/comedi/drivers/ni_at_a2150.c
index 9b444f8..957fb9f 100644
--- a/drivers/staging/comedi/drivers/ni_at_a2150.c
+++ b/drivers/staging/comedi/drivers/ni_at_a2150.c
@@ -1,62 +1,47 @@
 /*
-comedi/drivers/ni_at_a2150.c
-Driver for National Instruments AT-A2150 boards
-Copyright (C) 2001, 2002 Frank Mori Hess <fmh...@users.sourceforge.net>
-
-COMEDI - Linux Control and Measurement Device Interface
-Copyright (C) 2000 David A. Schleef <d...@schleef.org>
-
-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: ni_at_a2150
-Description: National Instruments AT-A2150
-Author: Frank Mori Hess
-Status: works
-Devices: [National Instruments] AT-A2150C (at_a2150c), AT-2150S (at_a2150s)
-
-If you want to ac couple the board's inputs, use AREF_OTHER.
-
-Configuration options:
-  [0] - I/O port base address
-  [1] - IRQ (optional, required for timed conversions)
-  [2] - DMA (optional, required for timed conversions)
+ * Comedi driver for National Instruments AT-A2150 boards
+ * Copyright (C) 2001, 2002 Frank Mori Hess <fmh...@users.sourceforge.net>
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 2000 David A. Schleef <d...@schleef.org>
+ *
+ * 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.
+ */
 
-*/
 /*
-Yet another driver for obsolete hardware brought to you by Frank Hess.
-Testing and debugging help provided by Dave Andruczyk.
-
-This driver supports the boards:
-
-AT-A2150C
-AT-A2150S
-
-The only difference is their master clock frequencies.
-
-Options:
-   [0] - base io address
-   [1] - irq
-   [2] - dma channel
-
-References (from ftp://ftp.natinst.com/support/manuals):
-
-  320360.pdf  AT-A2150 User Manual
-
-TODO:
-
-analog level triggering
-TRIG_WAKE_EOS
-
-*/
+ * Driver: ni_at_a2150
+ * Description: National Instruments AT-A2150
+ * Author: Frank Mori Hess
+ * Status: works
+ * Devices: [National Instruments] AT-A2150C (at_a2150c), AT-2150S (at_a2150s)
+ *
+ * Configuration options:
+ *   [0] - I/O port base address
+ *   [1] - IRQ (optional, required for timed conversions)
+ *   [2] - DMA (optional, required for timed conversions)
+ *
+ * Yet another driver for obsolete hardware brought to you by Frank Hess.
+ * Testing and debugging help provided by Dave Andruczyk.
+ *
+ * If you want to ac couple the board's inputs, use AREF_OTHER.
+ *
+ * The only difference in the boards is their master clock frequencies.
+ *
+ * References (from ftp://ftp.natinst.com/support/manuals):
+ *   320360.pdf  AT-A2150 User Manual
+ *
+ * TODO:
+ * - analog level triggering
+ * - TRIG_WAKE_EOS
+ */
 
 #include 
 #include 
@@ -536,8 +521,10 @@ static int a2150_ai_cmd(struct comedi_device *dev, struct 
comedi_subdevice *s)
 
comedi_isadma_program(desc);
 
-   /* clear dma interrupt before enabling it, to try and get rid of that
-* one spurious interrupt that has been happening */
+   /*
+* Clear dma interrupt before enabling it, to try and get rid of
+* that one spurious interrupt that has been happening.
+*/
outw(0x00, dev->iobase + DMA_TC_CLEAR_REG);
 
/*  enable dma on card */
-- 
2.9.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 6/9] staging: comedi: ni_atmio: fix block comments

[H Hartley Sweeten]

Fix the checkpatch.pl issues:
WARNING: Block comments use * on subsequent lines

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/ni_atmio.c | 165 ++
 1 file changed, 78 insertions(+), 87 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_atmio.c 
b/drivers/staging/comedi/drivers/ni_atmio.c
index 95435b8..162a000 100644
--- a/drivers/staging/comedi/drivers/ni_atmio.c
+++ b/drivers/staging/comedi/drivers/ni_atmio.c
@@ -1,93 +1,84 @@
 /*
-comedi/drivers/ni_atmio.c
-Hardware driver for NI AT-MIO E series cards
-
-COMEDI - Linux Control and Measurement Device Interface
-Copyright (C) 1997-2001 David A. Schleef <d...@schleef.org>
-
-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: ni_atmio
-Description: National Instruments AT-MIO-E series
-Author: ds
-Devices: [National Instruments] AT-MIO-16E-1 (ni_atmio),
-  AT-MIO-16E-2, AT-MIO-16E-10, AT-MIO-16DE-10, AT-MIO-64E-3,
-  AT-MIO-16XE-50, AT-MIO-16XE-10, AT-AI-16XE-10
-Status: works
-Updated: Thu May  1 20:03:02 CDT 2003
-
-The driver has 2.6 kernel isapnp support, and
-will automatically probe for a supported board if the
-I/O base is left unspecified with comedi_config.
-However, many of
-the isapnp id numbers are unknown.  If your board is not
-recognized, please send the output of 'cat /proc/isapnp'
-(you may need to modprobe the isa-pnp module for
-/proc/isapnp to exist) so the
-id numbers for your board can be added to the driver.
-
-Otherwise, you can use the isapnptools package to configure
-your board.  Use isapnp to
-configure the I/O base and IRQ for the board, and then pass
-the same values as
-parameters in comedi_config.  A sample isapnp.conf file is included
-in the etc/ directory of Comedilib.
-
-Comedilib includes a utility to autocalibrate these boards.  The
-boards seem to boot into a state where the all calibration DACs
-are at one extreme of their range, thus the default calibration
-is terrible.  Calibration at boot is strongly encouraged.
-
-To use the extended digital I/O on some of the boards, enable the
-8255 driver when configuring the Comedi source tree.
-
-External triggering is supported for some events.  The channel index
-(scan_begin_arg, etc.) maps to PFI0 - PFI9.
-
-Some of the more esoteric triggering possibilities of these boards
-are not supported.
-*/
-/*
-   The real guts of the driver is in ni_mio_common.c, which is included
-   both here and in ni_pcimio.c
-
-   Interrupt support added by Truxton Fulton <t...@truxton.com>
-
-   References for specifications:
-
-  340747b.pdf  Register Level Programmer Manual (obsolete)
-  340747c.pdf  Register Level Programmer Manual (new)
-  DAQ-STC reference manual
-
-   Other possibly relevant info:
-
-  320517c.pdf  User manual (obsolete)
-  320517f.pdf  User manual (new)
-  320889a.pdf  delete
-  320906c.pdf  maximum signal ratings
-  321066a.pdf  about 16x
-  321791a.pdf  discontinuation of at-mio-16e-10 rev. c
-  321808a.pdf  about at-mio-16e-10 rev P
-  321837a.pdf  discontinuation of at-mio-16de-10 rev d
-  321838a.pdf  about at-mio-16de-10 rev N
-
-   ISSUES:
-
-   need to deal with external reference for DAC, and other DAC
-   properties in board properties
+ * Comedi driver for NI AT-MIO E series cards
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 1997-2001 David A. Schleef <d...@schleef.org>
+ *
+ * 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.
+ */
 
-   deal with at-mio-16de-10 revision D to N changes, etc.
+/*
+ * Driver: ni_atmio
+ * Description: National Instruments AT-MIO-E series
+ * Author: ds
+ * Devices: [National Instruments] AT-MIO-16E-1 (ni_atmio),
+ *   AT-MIO-16E-2, AT-MIO-16E-10, AT-MIO-16DE-10, AT-MIO-64E-3,
+ *   AT-MIO-16XE-50, AT-MIO-16XE

[PATCH 7/9] staging: comedi: ni_atmio16d: fix block comments

[H Hartley Sweeten]

Fix the checkpatch.pl issues:
WARNING: Block comments use * on subsequent lines

Move the configuration options comment into the comedi driver comment
block. That's were they typically are listed.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/ni_atmio16d.c | 106 ---
 1 file changed, 47 insertions(+), 59 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_atmio16d.c 
b/drivers/staging/comedi/drivers/ni_atmio16d.c
index c3eb546..fb59b0f 100644
--- a/drivers/staging/comedi/drivers/ni_atmio16d.c
+++ b/drivers/staging/comedi/drivers/ni_atmio16d.c
@@ -1,25 +1,41 @@
 /*
-   comedi/drivers/ni_atmio16d.c
-   Hardware driver for National Instruments AT-MIO16D board
-   Copyright (C) 2000 Chris R. Baugher <baug...@enteract.com>
-
-   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 driver for National Instruments AT-MIO16D board
+ * Copyright (C) 2000 Chris R. Baugher <baug...@enteract.com>
+ *
+ * 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: ni_atmio16d
-Description: National Instruments AT-MIO-16D
-Author: Chris R. Baugher <baug...@enteract.com>
-Status: unknown
-Devices: [National Instruments] AT-MIO-16 (atmio16), AT-MIO-16D (atmio16d)
-*/
+ * Driver: ni_atmio16d
+ * Description: National Instruments AT-MIO-16D
+ * Author: Chris R. Baugher <baug...@enteract.com>
+ * Status: unknown
+ * Devices: [National Instruments] AT-MIO-16 (atmio16), AT-MIO-16D (atmio16d)
+ *
+ * Configuration options:
+ *   [0] - I/O port
+ *   [1] - MIO irq (0 == no irq; or 3,4,5,6,7,9,10,11,12,14,15)
+ *   [2] - DIO irq (0 == no irq; or 3,4,5,6,7,9)
+ *   [3] - DMA1 channel (0 == no DMA; or 5,6,7)
+ *   [4] - DMA2 channel (0 == no DMA; or 5,6,7)
+ *   [5] - a/d mux (0=differential; 1=single)
+ *   [6] - a/d range (0=bipolar10; 1=bipolar5; 2=unipolar10)
+ *   [7] - dac0 range (0=bipolar; 1=unipolar)
+ *   [8] - dac0 reference (0=internal; 1=external)
+ *   [9] - dac0 coding (0=2's comp; 1=straight binary)
+ *   [10] - dac1 range (same as dac0 options)
+ *   [11] - dac1 reference (same as dac0 options)
+ *   [12] - dac1 coding (same as dac0 options)
+ */
+
 /*
  * I must give credit here to Michal Dobes <do...@tesnet.cz> who
  * wrote the driver for Advantec's pcl812 boards. I used the interrupt
@@ -295,8 +311,10 @@ static int atmio16d_ai_cmd(struct comedi_device *dev,
unsigned int sample_count, tmp, chan, gain;
int i;
 
-   /* This is slowly becoming a working command interface. *
-* It is still uber-experimental */
+   /*
+* This is slowly becoming a working command interface.
+* It is still uber-experimental
+*/
 
reset_counters(dev);
 
@@ -322,9 +340,10 @@ static int atmio16d_ai_cmd(struct comedi_device *dev,
outw(tmp, dev->iobase + MUX_GAIN_REG);
}
 
-   /* Now program the sample interval timer */
-   /* Figure out which clock to use then get an
-* appropriate timer value */
+   /*
+* Now program the sample interval timer.
+* Figure out which clock to use then get an appropriate timer value.
+*/
if (cmd->convert_arg < 65536000) {
base_clock = CLOCK_1_MHZ;
timer = cmd->convert_arg / 1000;
@@ -386,9 +405,10 @@ static int atmio16d_ai_cmd(struct comedi_device *dev,
outw(devpriv->com_reg_1_state, dev->iobase + COM_REG_1);
}
 
-   /* Program the scan interval timer ONLY IF SCANNING IS ENABLED */
-   /* Figure out which clock to use then get an
-* appropriate timer value */
+   /*
+* Program the scan interval timer ONLY IF SCANNING IS ENABLED.
+* Figure out which clock to use then get an appropriate timer value.
+*/
if (cmd->chanlist_len > 1) {
if (cmd->scan_begin_arg < 65536000) {
   

[PATCH 2/9] staging: comedi: ni_670x: fix block comment issues

[H Hartley Sweeten]

Fix the checkpatch.pl issues:
WARNING: Block comments use * on subsequent lines

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/ni_670x.c | 62 +++-
 1 file changed, 28 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_670x.c 
b/drivers/staging/comedi/drivers/ni_670x.c
index 3e7271880..74911db 100644
--- a/drivers/staging/comedi/drivers/ni_670x.c
+++ b/drivers/staging/comedi/drivers/ni_670x.c
@@ -1,40 +1,34 @@
 /*
-comedi/drivers/ni_670x.c
-Hardware driver for NI 670x devices
-
-COMEDI - Linux Control and Measurement Device Interface
-Copyright (C) 1997-2001 David A. Schleef <d...@schleef.org>
-
-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: ni_670x
-Description: National Instruments 670x
-Author: Bart Joris <bjo...@advalvas.be>
-Updated: Wed, 11 Dec 2002 18:25:35 -0800
-Devices: [National Instruments] PCI-6703 (ni_670x), PCI-6704
-Status: unknown
-
-Commands are not supported.
-*/
+ * Comedi driver for NI 670x devices
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 1997-2001 David A. Schleef <d...@schleef.org>
+ *
+ * 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.
+ */
 
 /*
-   Bart Joris <bjo...@advalvas.be> Last updated on 20/08/2001
-
-   Manuals:
-
-   322110a.pdf PCI/PXI-6704 User Manual
-   322110b.pdf PCI/PXI-6703/6704 User Manual
-
-*/
+ * Driver: ni_670x
+ * Description: National Instruments 670x
+ * Author: Bart Joris <bjo...@advalvas.be>
+ * Updated: Wed, 11 Dec 2002 18:25:35 -0800
+ * Devices: [National Instruments] PCI-6703 (ni_670x), PCI-6704
+ * Status: unknown
+ *
+ * Commands are not supported.
+ *
+ * Manuals:
+ *   322110a.pdf   PCI/PXI-6704 User Manual
+ *   322110b.pdf   PCI/PXI-6703/6704 User Manual
+ */
 
 #include 
 #include 
-- 
2.9.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/9] staging: comedi: ni_mio_cs: fix block comments

[H Hartley Sweeten]

Fix the checkpatch.pl issues:
WARNING: Block comments use * on subsequent lines

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/ni_mio_cs.c | 67 +++---
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/drivers/staging/comedi/drivers/ni_mio_cs.c 
b/drivers/staging/comedi/drivers/ni_mio_cs.c
index e3d821b..21f8231 100644
--- a/drivers/staging/comedi/drivers/ni_mio_cs.c
+++ b/drivers/staging/comedi/drivers/ni_mio_cs.c
@@ -1,40 +1,39 @@
 /*
-comedi/drivers/ni_mio_cs.c
-Hardware driver for NI PCMCIA MIO E series cards
-
-COMEDI - Linux Control and Measurement Device Interface
-Copyright (C) 1997-2000 David A. Schleef <d...@schleef.org>
-
-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: ni_mio_cs
-Description: National Instruments DAQCard E series
-Author: ds
-Status: works
-Devices: [National Instruments] DAQCard-AI-16XE-50 (ni_mio_cs),
-  DAQCard-AI-16E-4, DAQCard-6062E, DAQCard-6024E, DAQCard-6036E
-Updated: Thu Oct 23 19:43:17 CDT 2003
-
-See the notes in the ni_atmio.o driver.
-*/
-/*
-   The real guts of the driver is in ni_mio_common.c, which is
-   included by all the E series drivers.
-
-   References for specifications:
+ * Comedi driver for NI PCMCIA MIO E series cards
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * Copyright (C) 1997-2000 David A. Schleef <d...@schleef.org>
+ *
+ * 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.
+ */
 
-  341080a.pdf  DAQCard E Series Register Level Programmer Manual
+/*
+ * Driver: ni_mio_cs
+ * Description: National Instruments DAQCard E series
+ * Author: ds
+ * Status: works
+ * Devices: [National Instruments] DAQCard-AI-16XE-50 (ni_mio_cs),
+ *   DAQCard-AI-16E-4, DAQCard-6062E, DAQCard-6024E, DAQCard-6036E
+ * Updated: Thu Oct 23 19:43:17 CDT 2003
+ *
+ * See the notes in the ni_atmio.o driver.
+ */
 
-*/
+/*
+ * The real guts of the driver is in ni_mio_common.c, which is
+ * included by all the E series drivers.
+ *
+ * References for specifications:
+ * 341080a.pdf  DAQCard E Series Register Level Programmer Manual
+ */
 
 #include 
 #include 
-- 
2.9.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] staging: comedi: addi_apci_3501: add a comedi driver comment block

[H Hartley Sweeten]

This comment block is used to automatically generate documentation in
Comedi and Comedilib.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/addi_apci_3501.c | 27 +
 1 file changed, 27 insertions(+)

diff --git a/drivers/staging/comedi/drivers/addi_apci_3501.c 
b/drivers/staging/comedi/drivers/addi_apci_3501.c
index fc03f4a..57f0f46 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3501.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3501.c
@@ -22,6 +22,33 @@
  * more details.
  */
 
+/*
+ * Driver: addi_apci_3501
+ * Description: ADDI-DATA APCI-3501 Analog output board
+ * Devices: [ADDI-DATA] APCI-3501 (addi_apci_3501)
+ * Author: H Hartley Sweeten <hswee...@visionengravers.com>
+ * Updated: Mon, 20 Jun 2016 10:57:01 -0700
+ * Status: untested
+ *
+ * Configuration Options: not applicable, uses comedi PCI auto config
+ *
+ * This board has the following features:
+ *   - 4 or 8 analog output channels
+ *   - 2 optically isolated digital inputs
+ *   - 2 optically isolated digital outputs
+ *   - 1 12-bit watchdog/timer
+ *
+ * There are 2 versions of the APCI-3501:
+ *   - APCI-3501-4  4 analog output channels
+ *   - APCI-3501-8  8 analog output channels
+ *
+ * These boards use the same PCI Vendor/Device IDs. The number of output
+ * channels used by this driver is determined by reading the EEPROM on
+ * the board.
+ *
+ * The watchdog/timer subdevice is not currently supported.
+ */
+
 #include 
 
 #include "../comedi_pci.h"
-- 
2.9.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] staging: comedi: addi_apci_3501: remove timer/counter subdevice support

[H Hartley Sweeten]

This driver is for a simple 4/8 channel analog output board with 2 isolated
digital inputs and 2 isolated digital outputs. Support for these subdevices
is provided by the driver.

The boards also has a watchdog timer that can be used to reset the analog
outputs. It can also be used as a general purpose 12-bit timer when the
watchdog function is not necessary. The current support code for this
subdevice is broken. It does not follow the comedi API and requires some
out-of-tree patches in order to even work.

Remove the subdevice support. If a proper register map can be located for
this board we can add support back later.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 .../comedi/drivers/addi-data/hwdrv_apci3501.c  | 141 -
 drivers/staging/comedi/drivers/addi_apci_3501.c|  58 +
 2 files changed, 2 insertions(+), 197 deletions(-)
 delete mode 100644 drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c 
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c
deleted file mode 100644
index 3757074..000
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c
+++ /dev/null
@@ -1,141 +0,0 @@
-/* Watchdog Related Defines */
-
-#define ADDIDATA_TIMER 0
-#define ADDIDATA_WATCHDOG  2
-
-/*
- * (*insn_config) for the timer subdevice
- *
- * Configures The Timer, Counter or Watchdog
- * Data Pointer contains configuration parameters as below
- * data[0] : 0 Configure As Timer
- *   1 Configure As Counter
- *   2 Configure As Watchdog
- * data[1] : 1 Enable  Interrupt
- *   0 Disable Interrupt
- * data[2] : Time Unit
- * data[3] : Reload Value
- */
-static int apci3501_config_insn_timer(struct comedi_device *dev,
- struct comedi_subdevice *s,
- struct comedi_insn *insn,
- unsigned int *data)
-{
-   struct apci3501_private *devpriv = dev->private;
-   unsigned int ctrl;
-
-   if (data[0] != ADDIDATA_WATCHDOG &&
-   data[0] != ADDIDATA_TIMER)
-   return -EINVAL;
-
-   devpriv->tsk_Current = current;
-
-   devpriv->timer_mode = data[0];
-
-   /* first, disable the watchdog or stop the timer */
-   if (devpriv->timer_mode == ADDIDATA_WATCHDOG) {
-   ctrl = 0;
-   } else {
-   ctrl = inl(devpriv->tcw + ADDI_TCW_CTRL_REG);
-   ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
- ADDI_TCW_CTRL_ENA);
-   }
-   outl(ctrl, devpriv->tcw + ADDI_TCW_CTRL_REG);
-
-   /* enable/disable the timer interrupt */
-   ctrl = (data[1] == 1) ? ADDI_TCW_CTRL_IRQ_ENA : 0;
-   outl(ctrl, devpriv->tcw + ADDI_TCW_CTRL_REG);
-
-   outl(data[2], devpriv->tcw + ADDI_TCW_TIMEBASE_REG);
-   outl(data[3], devpriv->tcw + ADDI_TCW_RELOAD_REG);
-
-   ctrl = inl(devpriv->tcw + ADDI_TCW_CTRL_REG);
-   if (devpriv->timer_mode == ADDIDATA_WATCHDOG) {
-   /* Set the mode (e2->e0) NOTE: this doesn't look correct */
-   ctrl |= ~(ADDI_TCW_CTRL_CNT_UP | ADDI_TCW_CTRL_EXT_CLK_MASK |
- ADDI_TCW_CTRL_MODE_MASK | ADDI_TCW_CTRL_GATE |
- ADDI_TCW_CTRL_TRIG | ADDI_TCW_CTRL_TIMER_ENA |
- ADDI_TCW_CTRL_RESET_ENA | ADDI_TCW_CTRL_WARN_ENA |
- ADDI_TCW_CTRL_IRQ_ENA | ADDI_TCW_CTRL_ENA);
-   } else {
-   /* mode 2 */
-   ctrl &= ~(ADDI_TCW_CTRL_CNTR_ENA | ADDI_TCW_CTRL_MODE_MASK |
- ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
- ADDI_TCW_CTRL_TIMER_ENA | ADDI_TCW_CTRL_RESET_ENA |
- ADDI_TCW_CTRL_WARN_ENA | ADDI_TCW_CTRL_ENA);
-   ctrl |= ADDI_TCW_CTRL_MODE(2) | ADDI_TCW_CTRL_TIMER_ENA;
-   }
-   outl(ctrl, devpriv->tcw + ADDI_TCW_CTRL_REG);
-
-   return insn->n;
-}
-
-/*
- * (*insn_write) for the timer subdevice
- *
- * Start / Stop The Selected Timer , Counter or Watchdog
- * Data Pointer contains configuration parameters as below
- * data[0] : 0 Timer
- *   1 Counter
- *   2 Watchdog
- * data[1] : 1 Start
- *   0 Stop
- *   2 Trigger
- */
-static int apci3501_write_insn_timer(struct comedi_device *dev,
-struct comedi_subdevice *s,
-struct comedi_insn *insn,
-unsigned int *data)
-{
-   struct apci3501_private *devpriv = dev->private;
-   unsigned int ctrl;
-
-   if (devpriv->timer_mode == ADDIDATA_WATCHDOG ||
- 

[PATCH 0/2] staging: comedi: addi_apci_3501: cleanup driver

[H Hartley Sweeten]

The timer/counter subdevice in this driver is broken and it really
doesn't add much use if it did work.

Remove the timer/counter support and add the missing comedi driver
comment block.

H Hartley Sweeten (2):
  staging: comedi: addi_apci_3501: remove timer/counter subdevice support
  staging: comedi: addi_apci_3501: add a comedi driver comment block

 .../comedi/drivers/addi-data/hwdrv_apci3501.c  | 141 -
 drivers/staging/comedi/drivers/addi_apci_3501.c|  85 +
 2 files changed, 29 insertions(+), 197 deletions(-)
 delete mode 100644 drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c

-- 
2.9.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 2649/2649] Staging: comedi : fixed a camel case style issue

[Hartley Sweeten]

On Friday, June 17, 2016 9:02 PM, Greg KH wrote:
>> The patch looks fine (although this odd-ball Comedi driver shouldn't really
>> be sending signals to a user-space task!).
>
> Yeah, that's really odd, fixing that would be nice...

This is the last addi-data driver that does this. It's on my list to fix.

Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 4/6] staging: comedi: addi_apci_1564: rewrite the timer subdevice support

[H Hartley Sweeten]

The support functions for the timer subdevice are broken.

1) The (*insn_write) assumes that insn->n is always 2 (data[1] is used)
2) The (*insn_read) assumes that insn->n is always 2 (data can be returned in
   data[0] and data[1]).
3) The (*insn_config) does not follow the API. It assumes insn->n is always 4
   (data[1], data[2] and data[3] are used). It also doesn't use data[0] to
   determine what the config "instruction" is.

Rewrite the code to follow the comedi API and add the missing comedi driver
comment block.

The new implementation is based on the (minimal) datasheet I have from
ADDI-DATA.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 .../comedi/drivers/addi-data/hwdrv_apci1564.c  |  91 ---
 drivers/staging/comedi/drivers/addi_apci_1564.c| 129 +
 2 files changed, 129 insertions(+), 91 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c 
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index d6b2880..a1df66d 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -1,94 +1,3 @@
-static int apci1564_timer_insn_config(struct comedi_device *dev,
- struct comedi_subdevice *s,
- struct comedi_insn *insn,
- unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-   unsigned int ctrl;
-
-   /* Stop the timer */
-   ctrl = inl(devpriv->timer + ADDI_TCW_CTRL_REG);
-   ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
- ADDI_TCW_CTRL_ENA);
-   outl(ctrl, devpriv->timer + ADDI_TCW_CTRL_REG);
-
-   if (data[1] == 1) {
-   /* Enable timer int & disable all the other int sources */
-   outl(ADDI_TCW_CTRL_IRQ_ENA,
-devpriv->timer + ADDI_TCW_CTRL_REG);
-   outl(0x0, dev->iobase + APCI1564_DI_IRQ_REG);
-   outl(0x0, dev->iobase + APCI1564_DO_IRQ_REG);
-   outl(0x0, dev->iobase + APCI1564_WDOG_IRQ_REG);
-   if (devpriv->counters) {
-   unsigned long iobase;
-
-   iobase = devpriv->counters + ADDI_TCW_IRQ_REG;
-   outl(0x0, iobase + APCI1564_COUNTER(0));
-   outl(0x0, iobase + APCI1564_COUNTER(1));
-   outl(0x0, iobase + APCI1564_COUNTER(2));
-   }
-   } else {
-   /* disable Timer interrupt */
-   outl(0x0, devpriv->timer + ADDI_TCW_CTRL_REG);
-   }
-
-   /* Loading Timebase */
-   outl(data[2], devpriv->timer + ADDI_TCW_TIMEBASE_REG);
-
-   /* Loading the Reload value */
-   outl(data[3], devpriv->timer + ADDI_TCW_RELOAD_REG);
-
-   ctrl = inl(devpriv->timer + ADDI_TCW_CTRL_REG);
-   ctrl &= ~(ADDI_TCW_CTRL_CNTR_ENA | ADDI_TCW_CTRL_MODE_MASK |
- ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
- ADDI_TCW_CTRL_TIMER_ENA | ADDI_TCW_CTRL_RESET_ENA |
- ADDI_TCW_CTRL_WARN_ENA | ADDI_TCW_CTRL_ENA);
-   ctrl |= ADDI_TCW_CTRL_MODE(2) | ADDI_TCW_CTRL_TIMER_ENA;
-   outl(ctrl, devpriv->timer + ADDI_TCW_CTRL_REG);
-
-   return insn->n;
-}
-
-static int apci1564_timer_insn_write(struct comedi_device *dev,
-struct comedi_subdevice *s,
-struct comedi_insn *insn,
-unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-   unsigned int ctrl;
-
-   ctrl = inl(devpriv->timer + ADDI_TCW_CTRL_REG);
-   ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG);
-   switch (data[1]) {
-   case 0: /* Stop The Timer */
-   ctrl &= ~ADDI_TCW_CTRL_ENA;
-   break;
-   case 1: /* Enable the Timer */
-   ctrl |= ADDI_TCW_CTRL_ENA;
-   break;
-   }
-   outl(ctrl, devpriv->timer + ADDI_TCW_CTRL_REG);
-
-   return insn->n;
-}
-
-static int apci1564_timer_insn_read(struct comedi_device *dev,
-   struct comedi_subdevice *s,
-   struct comedi_insn *insn,
-   unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-
-   /* Stores the status of the Timer */
-   data[0] = inl(devpriv->timer + ADDI_TCW_STATUS_REG) &
- ADDI_TCW_STATUS_OVERFLOW;
-
-   /* Stores the Actual value of the Timer */
-   data[1] = inl(devpriv->timer + ADDI_TCW_VAL_REG);
-
-   return insn->n;
-}
-
 sta

[PATCH v2 1/6] staging: comedi: addi_apci_1564: clarify change-of-state interrupt support

[H Hartley Sweeten]

This board supports change-of-state interrupts on digital inputs 4 to 19
not 0 to 15.

The current code "works" but it could set inappropriate bits in the mode1
and mode2 registers that setup which channels are enabled. It also doesn't
return the status of the upper 4 channels (19 to 16).

Fix the comment and mask the mode1/mode2 values so that only the interrupt
capable channels can be enabled.

Add the SDF_LSAMPL flag to the subdevice so that 32-bit samples are used
instead of 16-bit ones. This allows returning the upper 4 channels. Use
the remaining bits in the sample to return "event" flags to the user.

The timer and counter subdevices can also generate interrupts and are a bit
hacked. They don't currently follow the comedi API and they use send_sig()
to let the task that know that the interrupt occured. The "event" flags will
be used instead when these subdevices are fixed.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 41 +
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index f1ccfbd..9350f59 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -77,6 +77,7 @@
 #define APCI1564_DI_REG0x00
 #define APCI1564_DI_INT_MODE1_REG  0x04
 #define APCI1564_DI_INT_MODE2_REG  0x08
+#define APCI1564_DI_INT_MODE_MASK  0x0000 /* chans [19:4] */
 #define APCI1564_DI_INT_STATUS_REG 0x0c
 #define APCI1564_DI_IRQ_REG0x10
 #define APCI1564_DI_IRQ_ENABIT(2)
@@ -111,11 +112,18 @@
  */
 #define APCI1564_COUNTER(x)((x) * 0x20)
 
+/*
+ * The dev->read_subdev is used to return the interrupt events along with
+ * the state of the interrupt capable inputs.
+ */
+#define APCI1564_EVENT_COS BIT(31)
+#define APCI1564_EVENT_MASK0xffff /* all but [19:4] */
+
 struct apci1564_private {
unsigned long eeprom;   /* base address of EEPROM register */
unsigned long timer;/* base address of 12-bit timer */
unsigned long counters; /* base address of 32-bit counters */
-   unsigned int mode1; /* riding-edge/high level channels */
+   unsigned int mode1; /* rising-edge/high level channels */
unsigned int mode2; /* falling-edge/low level channels */
unsigned int ctrl;  /* interrupt mode OR (edge) . AND (level) */
struct task_struct *tsk_current;
@@ -165,18 +173,18 @@ static irqreturn_t apci1564_interrupt(int irq, void *d)
unsigned int ctrl;
unsigned int chan;
 
+   s->state &= ~APCI1564_EVENT_MASK;
+
status = inl(dev->iobase + APCI1564_DI_IRQ_REG);
if (status & APCI1564_DI_IRQ_ENA) {
-   /* disable the interrupt */
+   /* get the COS interrupt state and set the event flag */
+   s->state = inl(dev->iobase + APCI1564_DI_INT_STATUS_REG);
+   s->state &= APCI1564_DI_INT_MODE_MASK;
+   s->state |= APCI1564_EVENT_COS;
+
+   /* clear the interrupt */
outl(status & ~APCI1564_DI_IRQ_ENA,
 dev->iobase + APCI1564_DI_IRQ_REG);
-
-   s->state = inl(dev->iobase + APCI1564_DI_INT_STATUS_REG) &
-  0x;
-   comedi_buf_write_samples(s, >state, 1);
-   comedi_handle_events(dev, s);
-
-   /* enable the interrupt */
outl(status, dev->iobase + APCI1564_DI_IRQ_REG);
}
 
@@ -214,6 +222,11 @@ static irqreturn_t apci1564_interrupt(int irq, void *d)
}
}
 
+   if (s->state & APCI1564_EVENT_MASK) {
+   comedi_buf_write_samples(s, >state, 1);
+   comedi_handle_events(dev, s);
+   }
+
return IRQ_HANDLED;
 }
 
@@ -255,7 +268,7 @@ static int apci1564_diag_insn_bits(struct comedi_device 
*dev,
 /*
  * Change-Of-State (COS) interrupt configuration
  *
- * Channels 0 to 15 are interruptible. These channels can be configured
+ * Channels 4 to 19 are interruptible. These channels can be configured
  * to generate interrupts based on AND/OR logic for the desired channels.
  *
  * OR logic
@@ -343,6 +356,10 @@ static int apci1564_cos_insn_config(struct comedi_device 
*dev,
default:
return -EINVAL;
}
+
+   /* ensure the mode bits are in-range for channels [19:4] */
+   devpriv->mode1 &= APCI1564_DI_INT_MODE_MASK;
+   devpriv->mode2 &= A

[PATCH v2 6/6] staging: comedi: addi_apci_1564: remove unnecessary wdog register defines

[H Hartley Sweeten]

The watchdog subdevice is supported using the addi_watchdog module and
it uses the register defines from addi_tcw.h. The only register define
needed it the iobase offset to the register block.

Remove the unnecessary defines and rename the iobase define.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 5813de5..9bfb79c 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -144,14 +144,7 @@
 #define APCI1564_DO_INT_STATUS_VCC BIT(0)
 #define APCI1564_DO_IRQ_REG0x20
 #define APCI1564_DO_IRQ_INTR   BIT(0)
-#define APCI1564_WDOG_REG  0x24
-#define APCI1564_WDOG_RELOAD_REG   0x28
-#define APCI1564_WDOG_TIMEBASE_REG 0x2c
-#define APCI1564_WDOG_CTRL_REG 0x30
-#define APCI1564_WDOG_STATUS_REG   0x34
-#define APCI1564_WDOG_IRQ_REG  0x38
-#define APCI1564_WDOG_WARN_TIMEVAL_REG 0x3c
-#define APCI1564_WDOG_WARN_TIMEBASE_REG0x40
+#define APCI1564_WDOG_IOBASE   0x24
 
 /*
  * devpriv->timer Register Map (see addi_tcw.h for register/bit defines)
@@ -198,7 +191,7 @@ static int apci1564_reset(struct comedi_device *dev)
outl(0x0, dev->iobase + APCI1564_DO_INT_CTRL_REG);
 
/* Reset the watchdog registers */
-   addi_watchdog_reset(dev->iobase + APCI1564_WDOG_REG);
+   addi_watchdog_reset(dev->iobase + APCI1564_WDOG_IOBASE);
 
/* Reset the timer registers */
outl(0x0, devpriv->timer + ADDI_TCW_CTRL_REG);
@@ -773,7 +766,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
/* Initialize the watchdog subdevice */
s = >subdevices[5];
-   ret = addi_watchdog_init(s, dev->iobase + APCI1564_WDOG_REG);
+   ret = addi_watchdog_init(s, dev->iobase + APCI1564_WDOG_IOBASE);
if (ret)
return ret;
 
-- 
2.8.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 2/6] staging: comedi: addi_apci_1564: use comedi_handle_event() for timer

[H Hartley Sweeten]

The timer subdevice can generate an interrupt. Currently send_sig() is used
to let the task know when the interrupt occurs. Use the dev->read_subdev
and comedi_handle_events() instead.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 9350f59..45c1558 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -117,6 +117,7 @@
  * the state of the interrupt capable inputs.
  */
 #define APCI1564_EVENT_COS BIT(31)
+#define APCI1564_EVENT_TIMER   BIT(30)
 #define APCI1564_EVENT_MASK0xffff /* all but [19:4] */
 
 struct apci1564_private {
@@ -189,15 +190,12 @@ static irqreturn_t apci1564_interrupt(int irq, void *d)
}
 
status = inl(devpriv->timer + ADDI_TCW_IRQ_REG);
-   if (status & 0x01) {
-   /*  Disable Timer Interrupt */
+   if (status & ADDI_TCW_IRQ) {
+   s->state |= APCI1564_EVENT_TIMER;
+
+   /* clear the interrupt */
ctrl = inl(devpriv->timer + ADDI_TCW_CTRL_REG);
outl(0x0, devpriv->timer + ADDI_TCW_CTRL_REG);
-
-   /* Send a signal to from kernel to user space */
-   send_sig(SIGIO, devpriv->tsk_current, 0);
-
-   /*  Enable Timer Interrupt */
outl(ctrl, devpriv->timer + ADDI_TCW_CTRL_REG);
}
 
-- 
2.8.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 0/6] staging: comedi: addi_apci_1564: cleanup driver

[H Hartley Sweeten]

This driver still uses some of old broken code in hwdrv_apci1564.c
for the timer and counter subdevices. This code does not follow the
comedi API and does not work without additional patches to the core.

Tidy up the change-of-state support so that the dev->read_subdev
can be used to handle the interrupt events from the timer and counter
subdevices.

Rewrite the timer and counter subdevice support so that it follows
the comedi API.

NOTE: The datasheet I have from ADDI-DATA just describes the registers.
It does not have any functional information on the operation of the
timer or counter. The new code might not work _correctly_ but it at
least makes the subdevices work with the comedi API so that additional
testing can be done.

v2: mask the change-of-state input status in patch 1/6 at the request
of Ian Abbott

H Hartley Sweeten (6):
  staging: comedi: addi_apci_1564: clarify change-of-state interrupt support
  staging: comedi: addi_apci_1564: use comedi_handle_event() for timer
  staging: comedi: addi_apci_1564: use comedi_handle_event() for counters
  staging: comedi: addi_apci_1564: rewrite the timer subdevice support
  staging: comedi: addi_apci_1564: rewrite the counter subdevice support
  staging: comedi: addi_apci_1564: remove unnecessary wdog register defines

 .../comedi/drivers/addi-data/hwdrv_apci1564.c  | 187 -
 drivers/staging/comedi/drivers/addi_apci_1564.c| 305 ++---
 2 files changed, 264 insertions(+), 228 deletions(-)
 delete mode 100644 drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c

-- 
2.8.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 5/6] staging: comedi: addi_apci_1564: rewrite the counter subdevice support

[H Hartley Sweeten]

Like the timer, the support functions for the counter subdevice are broken.

Rewrite the code to follow the comedi API.

The new implementation is based on the (minimal) datasheet I have from
ADDI-DATA.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 .../comedi/drivers/addi-data/hwdrv_apci1564.c  | 92 -
 drivers/staging/comedi/drivers/addi_apci_1564.c| 94 +-
 2 files changed, 92 insertions(+), 94 deletions(-)
 delete mode 100644 drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c 
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
deleted file mode 100644
index a1df66d..000
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ /dev/null
@@ -1,92 +0,0 @@
-static int apci1564_counter_insn_config(struct comedi_device *dev,
-   struct comedi_subdevice *s,
-   struct comedi_insn *insn,
-   unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-   unsigned int chan = CR_CHAN(insn->chanspec);
-   unsigned long iobase = devpriv->counters + APCI1564_COUNTER(chan);
-   unsigned int ctrl;
-
-   /* Stop The Timer */
-   ctrl = inl(iobase + ADDI_TCW_CTRL_REG);
-   ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
- ADDI_TCW_CTRL_ENA);
-   outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
-
-   /* Set the reload value */
-   outl(data[3], iobase + ADDI_TCW_RELOAD_REG);
-
-   /* Set the mode */
-   ctrl &= ~(ADDI_TCW_CTRL_EXT_CLK_MASK | ADDI_TCW_CTRL_MODE_MASK |
- ADDI_TCW_CTRL_TIMER_ENA | ADDI_TCW_CTRL_RESET_ENA |
- ADDI_TCW_CTRL_WARN_ENA);
-   ctrl |= ADDI_TCW_CTRL_CNTR_ENA | ADDI_TCW_CTRL_MODE(data[4]);
-   outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
-
-   /* Enable or Disable Interrupt */
-   if (data[1])
-   ctrl |= ADDI_TCW_CTRL_IRQ_ENA;
-   else
-   ctrl &= ~ADDI_TCW_CTRL_IRQ_ENA;
-   outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
-
-   /* Set the Up/Down selection */
-   if (data[6])
-   ctrl |= ADDI_TCW_CTRL_CNT_UP;
-   else
-   ctrl &= ~ADDI_TCW_CTRL_CNT_UP;
-   outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
-
-   return insn->n;
-}
-
-static int apci1564_counter_insn_write(struct comedi_device *dev,
-  struct comedi_subdevice *s,
-  struct comedi_insn *insn,
-  unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-   unsigned int chan = CR_CHAN(insn->chanspec);
-   unsigned long iobase = devpriv->counters + APCI1564_COUNTER(chan);
-   unsigned int ctrl;
-
-   ctrl = inl(iobase + ADDI_TCW_CTRL_REG);
-   ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG);
-   switch (data[1]) {
-   case 0: /* Stops the Counter subdevice */
-   ctrl = 0;
-   break;
-   case 1: /* Start the Counter subdevice */
-   ctrl |= ADDI_TCW_CTRL_ENA;
-   break;
-   case 2: /* Clears the Counter subdevice */
-   ctrl |= ADDI_TCW_CTRL_GATE;
-   break;
-   }
-   outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
-
-   return insn->n;
-}
-
-static int apci1564_counter_insn_read(struct comedi_device *dev,
- struct comedi_subdevice *s,
- struct comedi_insn *insn,
- unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-   unsigned int chan = CR_CHAN(insn->chanspec);
-   unsigned long iobase = devpriv->counters + APCI1564_COUNTER(chan);
-   unsigned int status;
-
-   /* Read the Counter Actual Value. */
-   data[0] = inl(iobase + ADDI_TCW_VAL_REG);
-
-   status = inl(iobase + ADDI_TCW_STATUS_REG);
-   data[1] = (status & ADDI_TCW_STATUS_SOFT_TRIG) ? 1 : 0;
-   data[2] = (status & ADDI_TCW_STATUS_HARDWARE_TRIG) ? 1 : 0;
-   data[3] = (status & ADDI_TCW_STATUS_SOFT_CLR) ? 1 : 0;
-   data[4] = (status & ADDI_TCW_STATUS_OVERFLOW) ? 1 : 0;
-
-   return insn->n;
-}
diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 3e8ac67..5813de5 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -67,6 +67,12 @@
  * the raw data[1] to this register along with the raw data[2] value to the
  * ADDI_TCW_RELOAD_REG. If anyone tests this and can determine the actual
  * timebase/reload oper

[PATCH v2 3/6] staging: comedi: addi_apci_1564: use comedi_handle_event() for counters

[H Hartley Sweeten]

The counter subdevice can generate an interrupt. Currently send_sig() is used
to let the task know when the interrupt occurs. Use the dev->read_subdev and
comedi_handle_events() instead.

Remove the, now unused, 'tsk_current' member from the private data and the
unnecessary include of .

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 .../staging/comedi/drivers/addi-data/hwdrv_apci1564.c|  4 
 drivers/staging/comedi/drivers/addi_apci_1564.c  | 16 ++--
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c 
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index f0c0d58..d6b2880 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -6,8 +6,6 @@ static int apci1564_timer_insn_config(struct comedi_device *dev,
struct apci1564_private *devpriv = dev->private;
unsigned int ctrl;
 
-   devpriv->tsk_current = current;
-
/* Stop the timer */
ctrl = inl(devpriv->timer + ADDI_TCW_CTRL_REG);
ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
@@ -101,8 +99,6 @@ static int apci1564_counter_insn_config(struct comedi_device 
*dev,
unsigned long iobase = devpriv->counters + APCI1564_COUNTER(chan);
unsigned int ctrl;
 
-   devpriv->tsk_current = current;
-
/* Stop The Timer */
ctrl = inl(iobase + ADDI_TCW_CTRL_REG);
ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 45c1558..4af45d8 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -23,7 +23,6 @@
 
 #include 
 #include 
-#include 
 
 #include "../comedi_pci.h"
 #include "addi_tcw.h"
@@ -118,6 +117,7 @@
  */
 #define APCI1564_EVENT_COS BIT(31)
 #define APCI1564_EVENT_TIMER   BIT(30)
+#define APCI1564_EVENT_COUNTER(x)  BIT(27 + (x)) /* counter 0-2 */
 #define APCI1564_EVENT_MASK0xffff /* all but [19:4] */
 
 struct apci1564_private {
@@ -127,7 +127,6 @@ struct apci1564_private {
unsigned int mode1; /* rising-edge/high level channels */
unsigned int mode2; /* falling-edge/low level channels */
unsigned int ctrl;  /* interrupt mode OR (edge) . AND (level) */
-   struct task_struct *tsk_current;
 };
 
 #include "addi-data/hwdrv_apci1564.c"
@@ -200,21 +199,18 @@ static irqreturn_t apci1564_interrupt(int irq, void *d)
}
 
if (devpriv->counters) {
-   for (chan = 0; chan < 4; chan++) {
+   for (chan = 0; chan < 3; chan++) {
unsigned long iobase;
 
iobase = devpriv->counters + APCI1564_COUNTER(chan);
 
status = inl(iobase + ADDI_TCW_IRQ_REG);
-   if (status & 0x01) {
-   /*  Disable Counter Interrupt */
+   if (status & ADDI_TCW_IRQ) {
+   s->state |= APCI1564_EVENT_COUNTER(chan);
+
+   /* clear the interrupt */
ctrl = inl(iobase + ADDI_TCW_CTRL_REG);
outl(0x0, iobase + ADDI_TCW_CTRL_REG);
-
-   /* Send a signal to from kernel to user space */
-   send_sig(SIGIO, devpriv->tsk_current, 0);
-
-   /*  Enable Counter Interrupt */
outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
}
}
-- 
2.8.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/6] staging: comedi: addi_apci_1564: clarify change-of-state interrupt support

[Hartley Sweeten]

On Wednesday, June 08, 2016 2:34 AM, Ian Abbott wrote:
> On 06/06/16 20:51, Hartley Sweeten wrote:
>>  From that I assume that the other bits will always return 0.
>
> Since it doesn't explicitly sat the other bits return 0, and until 
> someone can confirm, I think it's better not to assume that.

OK. I'll update the patch and repost the series.

Thanks,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 5/5] Staging: comedi: Prefer using the BIT macro issue in dmm32at.c

[Hartley Sweeten]

On Tuesday, June 07, 2016 4:51 AM, Ravishankar Karkala Mallikarjunayya wrote:
> Subject: [PATCH 5/5] Staging: comedi: Prefer using the BIT macro issue in 
> dmm32at.c

Please don't add random patches to a series. You started this series with 1/4, 
2/4,
3/4 and 4/4 then added a 5/5. 

Also, please fix the subject lines to include the driver name. This makes thing
clearer when multiple patches do similar things. The subject lines should look
something like this:

staging: comedi: : 

> This patch Replace all occurences of (1< to get rid of checkpatch.pl "CHECK" output "Prefer using the BIT macro"

There is no need to state that this is a patch. That is already known.

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/6] staging: comedi: addi_apci_1564: clarify change-of-state interrupt support

[Hartley Sweeten]

On Monday, June 06, 2016 12:33 PM, Ian Abbott wrote:
> Okay, I think I must have got something inverted in my head at the time 
> I wrote my reply.
>
> Follow-up question: when you do:
>
>   s->state = inl(dev->iobase + APCI1564_DI_INT_STATUS_REG);
>
> should that be ANDed with something (possibly 
> APCI1564_DI_INT_MODE_MASK), or are the other bits (including the new 
> "event" bits) guaranteed to read zero?

I guess it could be ANDed with APCI1564_DI_INT_MODE_MASK for
completeness.

The datasheet from ADDI-DATA shows this information for the register:

Address + 0x10
  Write: Nicht benutzt  (English translation: unused)
  Read: Status Register 4 to 19
Interrupt Status 4 to 19
0: No Interrupt
1: Interrupt generated

>From that I assume that the other bits will always return 0.

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 1/6] staging: comedi: addi_apci_1564: clarify change-of-state interrupt support

[Hartley Sweeten]

On Friday, June 03, 2016 2:58 AM, Ian Abbott wrote:
> On 02/06/16 22:58, H Hartley Sweeten wrote:
>> This board supports change-of-state interrupts on digital inputs 4 to 19
>> not 0 to 15.
>>
>> The current code "works" but it could set inappropriate bits in the mode1
>> and mode2 registers that setup which channels are enabled. It also doesn't
>> return the status of the upper 4 channels (19 to 16).
>>
>> Fix the comment and mask the mode1/mode2 values so that only the interrupt
>> capable channels can be enabled.
>>
>> Add the SDF_LSAMPL flag to the subdevice so that 32-bit samples are used
>> instead of 16-bit ones. This allows returning the upper 4 channels. Use
>> the remaining bits in the sample to return "event" flags to the user.
>>
>> The timer and counter subdevices can also generate interrupts and are a bit
>> hacked. They don't currently follow the comedi API and they use send_sig()
>> to let the task that know that the interrupt occured. The "event" flags will
>> be used instead when these subdevices are fixed.
>>
>> Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
>> Cc: Ian Abbott <abbo...@mev.co.uk>
>> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
>> ---
>>   drivers/staging/comedi/drivers/addi_apci_1564.c | 37 
>> +
>>   1 file changed, 26 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
>> b/drivers/staging/comedi/drivers/addi_apci_1564.c
>> index f1ccfbd..37e18b3 100644



>> +#define APCI1564_DI_INT_MODE_MASK   0x0000 /* chans [19:4] */



>> +#define APCI1564_EVENT_MASK 0xffff /* all but [19:4] */



>> +s->state &= ~APCI1564_EVENT_MASK;
>> +
>   status = inl(dev->iobase + APCI1564_DI_IRQ_REG);
>   if (status & APCI1564_DI_IRQ_ENA) {
> - /* disable the interrupt */
> + s->state = inl(dev->iobase + APCI1564_DI_INT_STATUS_REG);
> + s->state |= APCI1564_EVENT_COS;



> + if (s->state & APCI1564_EVENT_MASK) {
> + comedi_buf_write_samples(s, >state, 1);
> + comedi_handle_events(dev, s);
> + }
> +
>   return IRQ_HANDLED;
>   }

> I'm struggling to see how that works.  It looks like once 
> APCI1564_EVENT_COS has been set in s->state, it never gets cleared (or 
> at least it only gets cleared momentarily), and after that, the `if 
> (s->state & APCI1564_EVENT_MASK)` test will always be true, and so it 
> will write a sample to the buffer on every interrupt.

Ian,

1) When an interrupt occurs the last "events" are masked off
(leaving the last state of the digital inputs).
2) If the digital inputs caused the interrupt, the state of the
digital inputs is read and the event bit APCI1564_EVENT_COS
is set.
3) Similarly, the other patches add support for the timer and
   counter subdevices and set the APCI1564_EVENT_TIMER or
APCI1564_EVENT_COUNTER(chan) bits as appropriate.
4) At the end of the interrupt if any of the event bits are set
the sample will be written to the buffer and the event handled.

This seemed like the best way to handle the timer and counter
interrupts without adding unnecessary async command support.

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 4/6] staging: comedi: addi_apci_1564: rewrite the timer subdevice support

[H Hartley Sweeten]

The support functions for the timer subdevice are broken.

1) The (*insn_write) assumes that insn->n is always 2 (data[1] is used)
2) The (*insn_read) assumes that insn->n is always 2 (data can be returned in
   data[0] and data[1]).
3) The (*insn_config) does not follow the API. It assumes insn->n is always 4
   (data[1], data[2] and data[3] are used). It also doesn't use data[0] to
   determine what the config "instruction" is.

Rewrite the code to follow the comedi API and add the missing comedi driver
comment block.

The new implementation is based on the (minimal) datasheet I have from
ADDI-DATA.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 .../comedi/drivers/addi-data/hwdrv_apci1564.c  |  91 ---
 drivers/staging/comedi/drivers/addi_apci_1564.c| 129 +
 2 files changed, 129 insertions(+), 91 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c 
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index d6b2880..a1df66d 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -1,94 +1,3 @@
-static int apci1564_timer_insn_config(struct comedi_device *dev,
- struct comedi_subdevice *s,
- struct comedi_insn *insn,
- unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-   unsigned int ctrl;
-
-   /* Stop the timer */
-   ctrl = inl(devpriv->timer + ADDI_TCW_CTRL_REG);
-   ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
- ADDI_TCW_CTRL_ENA);
-   outl(ctrl, devpriv->timer + ADDI_TCW_CTRL_REG);
-
-   if (data[1] == 1) {
-   /* Enable timer int & disable all the other int sources */
-   outl(ADDI_TCW_CTRL_IRQ_ENA,
-devpriv->timer + ADDI_TCW_CTRL_REG);
-   outl(0x0, dev->iobase + APCI1564_DI_IRQ_REG);
-   outl(0x0, dev->iobase + APCI1564_DO_IRQ_REG);
-   outl(0x0, dev->iobase + APCI1564_WDOG_IRQ_REG);
-   if (devpriv->counters) {
-   unsigned long iobase;
-
-   iobase = devpriv->counters + ADDI_TCW_IRQ_REG;
-   outl(0x0, iobase + APCI1564_COUNTER(0));
-   outl(0x0, iobase + APCI1564_COUNTER(1));
-   outl(0x0, iobase + APCI1564_COUNTER(2));
-   }
-   } else {
-   /* disable Timer interrupt */
-   outl(0x0, devpriv->timer + ADDI_TCW_CTRL_REG);
-   }
-
-   /* Loading Timebase */
-   outl(data[2], devpriv->timer + ADDI_TCW_TIMEBASE_REG);
-
-   /* Loading the Reload value */
-   outl(data[3], devpriv->timer + ADDI_TCW_RELOAD_REG);
-
-   ctrl = inl(devpriv->timer + ADDI_TCW_CTRL_REG);
-   ctrl &= ~(ADDI_TCW_CTRL_CNTR_ENA | ADDI_TCW_CTRL_MODE_MASK |
- ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
- ADDI_TCW_CTRL_TIMER_ENA | ADDI_TCW_CTRL_RESET_ENA |
- ADDI_TCW_CTRL_WARN_ENA | ADDI_TCW_CTRL_ENA);
-   ctrl |= ADDI_TCW_CTRL_MODE(2) | ADDI_TCW_CTRL_TIMER_ENA;
-   outl(ctrl, devpriv->timer + ADDI_TCW_CTRL_REG);
-
-   return insn->n;
-}
-
-static int apci1564_timer_insn_write(struct comedi_device *dev,
-struct comedi_subdevice *s,
-struct comedi_insn *insn,
-unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-   unsigned int ctrl;
-
-   ctrl = inl(devpriv->timer + ADDI_TCW_CTRL_REG);
-   ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG);
-   switch (data[1]) {
-   case 0: /* Stop The Timer */
-   ctrl &= ~ADDI_TCW_CTRL_ENA;
-   break;
-   case 1: /* Enable the Timer */
-   ctrl |= ADDI_TCW_CTRL_ENA;
-   break;
-   }
-   outl(ctrl, devpriv->timer + ADDI_TCW_CTRL_REG);
-
-   return insn->n;
-}
-
-static int apci1564_timer_insn_read(struct comedi_device *dev,
-   struct comedi_subdevice *s,
-   struct comedi_insn *insn,
-   unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-
-   /* Stores the status of the Timer */
-   data[0] = inl(devpriv->timer + ADDI_TCW_STATUS_REG) &
- ADDI_TCW_STATUS_OVERFLOW;
-
-   /* Stores the Actual value of the Timer */
-   data[1] = inl(devpriv->timer + ADDI_TCW_VAL_REG);
-
-   return insn->n;
-}
-
 sta

[PATCH 3/6] staging: comedi: addi_apci_1564: use comedi_handle_event() for counters

[H Hartley Sweeten]

The counter subdevice can generate an interrupt. Currently send_sig() is used
to let the task know when the interrupt occurs. Use the dev->read_subdev and
comedi_handle_events() instead.

Remove the, now unused, 'tsk_current' member from the private data and the
unnecessary include of .

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 .../staging/comedi/drivers/addi-data/hwdrv_apci1564.c|  4 
 drivers/staging/comedi/drivers/addi_apci_1564.c  | 16 ++--
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c 
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index f0c0d58..d6b2880 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -6,8 +6,6 @@ static int apci1564_timer_insn_config(struct comedi_device *dev,
struct apci1564_private *devpriv = dev->private;
unsigned int ctrl;
 
-   devpriv->tsk_current = current;
-
/* Stop the timer */
ctrl = inl(devpriv->timer + ADDI_TCW_CTRL_REG);
ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
@@ -101,8 +99,6 @@ static int apci1564_counter_insn_config(struct comedi_device 
*dev,
unsigned long iobase = devpriv->counters + APCI1564_COUNTER(chan);
unsigned int ctrl;
 
-   devpriv->tsk_current = current;
-
/* Stop The Timer */
ctrl = inl(iobase + ADDI_TCW_CTRL_REG);
ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index d7b920e..259d249 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -23,7 +23,6 @@
 
 #include 
 #include 
-#include 
 
 #include "../comedi_pci.h"
 #include "addi_tcw.h"
@@ -118,6 +117,7 @@
  */
 #define APCI1564_EVENT_COS BIT(31)
 #define APCI1564_EVENT_TIMER   BIT(30)
+#define APCI1564_EVENT_COUNTER(x)  BIT(27 + (x)) /* counter 0-2 */
 #define APCI1564_EVENT_MASK0xffff /* all but [19:4] */
 
 struct apci1564_private {
@@ -127,7 +127,6 @@ struct apci1564_private {
unsigned int mode1; /* riding-edge/high level channels */
unsigned int mode2; /* falling-edge/low level channels */
unsigned int ctrl;  /* interrupt mode OR (edge) . AND (level) */
-   struct task_struct *tsk_current;
 };
 
 #include "addi-data/hwdrv_apci1564.c"
@@ -198,21 +197,18 @@ static irqreturn_t apci1564_interrupt(int irq, void *d)
}
 
if (devpriv->counters) {
-   for (chan = 0; chan < 4; chan++) {
+   for (chan = 0; chan < 3; chan++) {
unsigned long iobase;
 
iobase = devpriv->counters + APCI1564_COUNTER(chan);
 
status = inl(iobase + ADDI_TCW_IRQ_REG);
-   if (status & 0x01) {
-   /*  Disable Counter Interrupt */
+   if (status & ADDI_TCW_IRQ) {
+   s->state |= APCI1564_EVENT_COUNTER(chan);
+
+   /* clear the interrupt */
ctrl = inl(iobase + ADDI_TCW_CTRL_REG);
outl(0x0, iobase + ADDI_TCW_CTRL_REG);
-
-   /* Send a signal to from kernel to user space */
-   send_sig(SIGIO, devpriv->tsk_current, 0);
-
-   /*  Enable Counter Interrupt */
outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
}
}
-- 
2.8.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 5/6] staging: comedi: addi_apci_1564: rewrite the counter subdevice support

[H Hartley Sweeten]

Like the timer, the support functions for the counter subdevice are broken.

Rewrite the code to follow the comedi API.

The new implementation is based on the (minimal) datasheet I have from
ADDI-DATA.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 .../comedi/drivers/addi-data/hwdrv_apci1564.c  | 92 -
 drivers/staging/comedi/drivers/addi_apci_1564.c| 94 +-
 2 files changed, 92 insertions(+), 94 deletions(-)
 delete mode 100644 drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c

diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c 
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
deleted file mode 100644
index a1df66d..000
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ /dev/null
@@ -1,92 +0,0 @@
-static int apci1564_counter_insn_config(struct comedi_device *dev,
-   struct comedi_subdevice *s,
-   struct comedi_insn *insn,
-   unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-   unsigned int chan = CR_CHAN(insn->chanspec);
-   unsigned long iobase = devpriv->counters + APCI1564_COUNTER(chan);
-   unsigned int ctrl;
-
-   /* Stop The Timer */
-   ctrl = inl(iobase + ADDI_TCW_CTRL_REG);
-   ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG |
- ADDI_TCW_CTRL_ENA);
-   outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
-
-   /* Set the reload value */
-   outl(data[3], iobase + ADDI_TCW_RELOAD_REG);
-
-   /* Set the mode */
-   ctrl &= ~(ADDI_TCW_CTRL_EXT_CLK_MASK | ADDI_TCW_CTRL_MODE_MASK |
- ADDI_TCW_CTRL_TIMER_ENA | ADDI_TCW_CTRL_RESET_ENA |
- ADDI_TCW_CTRL_WARN_ENA);
-   ctrl |= ADDI_TCW_CTRL_CNTR_ENA | ADDI_TCW_CTRL_MODE(data[4]);
-   outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
-
-   /* Enable or Disable Interrupt */
-   if (data[1])
-   ctrl |= ADDI_TCW_CTRL_IRQ_ENA;
-   else
-   ctrl &= ~ADDI_TCW_CTRL_IRQ_ENA;
-   outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
-
-   /* Set the Up/Down selection */
-   if (data[6])
-   ctrl |= ADDI_TCW_CTRL_CNT_UP;
-   else
-   ctrl &= ~ADDI_TCW_CTRL_CNT_UP;
-   outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
-
-   return insn->n;
-}
-
-static int apci1564_counter_insn_write(struct comedi_device *dev,
-  struct comedi_subdevice *s,
-  struct comedi_insn *insn,
-  unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-   unsigned int chan = CR_CHAN(insn->chanspec);
-   unsigned long iobase = devpriv->counters + APCI1564_COUNTER(chan);
-   unsigned int ctrl;
-
-   ctrl = inl(iobase + ADDI_TCW_CTRL_REG);
-   ctrl &= ~(ADDI_TCW_CTRL_GATE | ADDI_TCW_CTRL_TRIG);
-   switch (data[1]) {
-   case 0: /* Stops the Counter subdevice */
-   ctrl = 0;
-   break;
-   case 1: /* Start the Counter subdevice */
-   ctrl |= ADDI_TCW_CTRL_ENA;
-   break;
-   case 2: /* Clears the Counter subdevice */
-   ctrl |= ADDI_TCW_CTRL_GATE;
-   break;
-   }
-   outl(ctrl, iobase + ADDI_TCW_CTRL_REG);
-
-   return insn->n;
-}
-
-static int apci1564_counter_insn_read(struct comedi_device *dev,
- struct comedi_subdevice *s,
- struct comedi_insn *insn,
- unsigned int *data)
-{
-   struct apci1564_private *devpriv = dev->private;
-   unsigned int chan = CR_CHAN(insn->chanspec);
-   unsigned long iobase = devpriv->counters + APCI1564_COUNTER(chan);
-   unsigned int status;
-
-   /* Read the Counter Actual Value. */
-   data[0] = inl(iobase + ADDI_TCW_VAL_REG);
-
-   status = inl(iobase + ADDI_TCW_STATUS_REG);
-   data[1] = (status & ADDI_TCW_STATUS_SOFT_TRIG) ? 1 : 0;
-   data[2] = (status & ADDI_TCW_STATUS_HARDWARE_TRIG) ? 1 : 0;
-   data[3] = (status & ADDI_TCW_STATUS_SOFT_CLR) ? 1 : 0;
-   data[4] = (status & ADDI_TCW_STATUS_OVERFLOW) ? 1 : 0;
-
-   return insn->n;
-}
diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 32b2596..e59af41 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -67,6 +67,12 @@
  * the raw data[1] to this register along with the raw data[2] value to the
  * ADDI_TCW_RELOAD_REG. If anyone tests this and can determine the actual
  * timebase/reload oper

[PATCH 6/6] staging: comedi: addi_apci_1564: remove unnecessary wdog register defines

[H Hartley Sweeten]

The watchdog subdevice is supported using the addi_watchdog module and
it uses the register defines from addi_tcw.h. The only register define
needed it the iobase offset to the register block.

Remove the unnecessary defines and rename the iobase define.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 13 +++--
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index e59af41..0f853b1 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -144,14 +144,7 @@
 #define APCI1564_DO_INT_STATUS_VCC BIT(0)
 #define APCI1564_DO_IRQ_REG0x20
 #define APCI1564_DO_IRQ_INTR   BIT(0)
-#define APCI1564_WDOG_REG  0x24
-#define APCI1564_WDOG_RELOAD_REG   0x28
-#define APCI1564_WDOG_TIMEBASE_REG 0x2c
-#define APCI1564_WDOG_CTRL_REG 0x30
-#define APCI1564_WDOG_STATUS_REG   0x34
-#define APCI1564_WDOG_IRQ_REG  0x38
-#define APCI1564_WDOG_WARN_TIMEVAL_REG 0x3c
-#define APCI1564_WDOG_WARN_TIMEBASE_REG0x40
+#define APCI1564_WDOG_IOBASE   0x24
 
 /*
  * devpriv->timer Register Map (see addi_tcw.h for register/bit defines)
@@ -198,7 +191,7 @@ static int apci1564_reset(struct comedi_device *dev)
outl(0x0, dev->iobase + APCI1564_DO_INT_CTRL_REG);
 
/* Reset the watchdog registers */
-   addi_watchdog_reset(dev->iobase + APCI1564_WDOG_REG);
+   addi_watchdog_reset(dev->iobase + APCI1564_WDOG_IOBASE);
 
/* Reset the timer registers */
outl(0x0, devpriv->timer + ADDI_TCW_CTRL_REG);
@@ -771,7 +764,7 @@ static int apci1564_auto_attach(struct comedi_device *dev,
 
/* Initialize the watchdog subdevice */
s = >subdevices[5];
-   ret = addi_watchdog_init(s, dev->iobase + APCI1564_WDOG_REG);
+   ret = addi_watchdog_init(s, dev->iobase + APCI1564_WDOG_IOBASE);
if (ret)
return ret;
 
-- 
2.8.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/6] staging: comedi: addi_apci_1564: clarify change-of-state interrupt support

[H Hartley Sweeten]

This board supports change-of-state interrupts on digital inputs 4 to 19
not 0 to 15.

The current code "works" but it could set inappropriate bits in the mode1
and mode2 registers that setup which channels are enabled. It also doesn't
return the status of the upper 4 channels (19 to 16).

Fix the comment and mask the mode1/mode2 values so that only the interrupt
capable channels can be enabled.

Add the SDF_LSAMPL flag to the subdevice so that 32-bit samples are used
instead of 16-bit ones. This allows returning the upper 4 channels. Use
the remaining bits in the sample to return "event" flags to the user.

The timer and counter subdevices can also generate interrupts and are a bit
hacked. They don't currently follow the comedi API and they use send_sig()
to let the task that know that the interrupt occured. The "event" flags will
be used instead when these subdevices are fixed.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 37 +
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index f1ccfbd..37e18b3 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -77,6 +77,7 @@
 #define APCI1564_DI_REG0x00
 #define APCI1564_DI_INT_MODE1_REG  0x04
 #define APCI1564_DI_INT_MODE2_REG  0x08
+#define APCI1564_DI_INT_MODE_MASK  0x0000 /* chans [19:4] */
 #define APCI1564_DI_INT_STATUS_REG 0x0c
 #define APCI1564_DI_IRQ_REG0x10
 #define APCI1564_DI_IRQ_ENABIT(2)
@@ -111,6 +112,13 @@
  */
 #define APCI1564_COUNTER(x)((x) * 0x20)
 
+/*
+ * The dev->read_subdev is used to return the interrupt events along with
+ * the state of the interrupt capable inputs.
+ */
+#define APCI1564_EVENT_COS BIT(31)
+#define APCI1564_EVENT_MASK0xffff /* all but [19:4] */
+
 struct apci1564_private {
unsigned long eeprom;   /* base address of EEPROM register */
unsigned long timer;/* base address of 12-bit timer */
@@ -165,18 +173,16 @@ static irqreturn_t apci1564_interrupt(int irq, void *d)
unsigned int ctrl;
unsigned int chan;
 
+   s->state &= ~APCI1564_EVENT_MASK;
+
status = inl(dev->iobase + APCI1564_DI_IRQ_REG);
if (status & APCI1564_DI_IRQ_ENA) {
-   /* disable the interrupt */
+   s->state = inl(dev->iobase + APCI1564_DI_INT_STATUS_REG);
+   s->state |= APCI1564_EVENT_COS;
+
+   /* clear the interrupt */
outl(status & ~APCI1564_DI_IRQ_ENA,
 dev->iobase + APCI1564_DI_IRQ_REG);
-
-   s->state = inl(dev->iobase + APCI1564_DI_INT_STATUS_REG) &
-  0x;
-   comedi_buf_write_samples(s, >state, 1);
-   comedi_handle_events(dev, s);
-
-   /* enable the interrupt */
outl(status, dev->iobase + APCI1564_DI_IRQ_REG);
}
 
@@ -214,6 +220,11 @@ static irqreturn_t apci1564_interrupt(int irq, void *d)
}
}
 
+   if (s->state & APCI1564_EVENT_MASK) {
+   comedi_buf_write_samples(s, >state, 1);
+   comedi_handle_events(dev, s);
+   }
+
return IRQ_HANDLED;
 }
 
@@ -255,7 +266,7 @@ static int apci1564_diag_insn_bits(struct comedi_device 
*dev,
 /*
  * Change-Of-State (COS) interrupt configuration
  *
- * Channels 0 to 15 are interruptible. These channels can be configured
+ * Channels 4 to 19 are interruptible. These channels can be configured
  * to generate interrupts based on AND/OR logic for the desired channels.
  *
  * OR logic
@@ -343,6 +354,10 @@ static int apci1564_cos_insn_config(struct comedi_device 
*dev,
default:
return -EINVAL;
}
+
+   /* ensure the mode bits are in-range for channels [19:4] */
+   devpriv->mode1 &= APCI1564_DI_INT_MODE_MASK;
+   devpriv->mode2 &= APCI1564_DI_INT_MODE_MASK;
break;
default:
return -EINVAL;
@@ -409,7 +424,7 @@ static int apci1564_cos_cmd(struct comedi_device *dev,
 {
struct apci1564_private *devpriv = dev->private;
 
-   if (!devpriv->ctrl) {
+   if (!devpriv->ctrl && !(devpriv->mode1 || devpriv->mode2)) {
dev_warn(dev->class_dev,
 "Interrupts disabled due to mode configuration!\n");
return -EINVAL;
@@ -501

[PATCH 0/6] staging: comedi: addi_apci_1564: cleanup driver

[H Hartley Sweeten]

This driver still uses some of old broken code in hwdrv_apci1564.c
for the timer and counter subdevices. This code does not follow the
comedi API and does not work without additional patches to the core.

Tidy up the change-of-state support so that the dev->read_subdev
can be used to handle the interrupt events from the timer and counter
subdevices.

Rewrite the timer and counter subdevice support so that it follows
the comedi API.

NOTE: The datasheet I have from ADDI-DATA just describes the registers.
It does not have any functional information on the operation of the
timer or counter. The new code might not work _correctly_ but it at
least makes the subdevices work with the comedi API so that additional
testing can be done.

H Hartley Sweeten (6):
  staging: comedi: addi_apci_1564: clarify change-of-state interrupt support
  staging: comedi: addi_apci_1564: use comedi_handle_event() for timer
  staging: comedi: addi_apci_1564: use comedi_handle_event() for counters
  staging: comedi: addi_apci_1564: rewrite the timer subdevice support
  staging: comedi: addi_apci_1564: rewrite the counter subdevice support
  staging: comedi: addi_apci_1564: remove unnecessary wdog register defines

 .../comedi/drivers/addi-data/hwdrv_apci1564.c  | 187 -
 drivers/staging/comedi/drivers/addi_apci_1564.c| 301 ++---
 2 files changed, 261 insertions(+), 227 deletions(-)
 delete mode 100644 drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c

-- 
2.8.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/6] staging: comedi: addi_apci_1564: use comedi_handle_event() for timer

[H Hartley Sweeten]

The timer subdevice can generate an interrupt. Currently send_sig() is used
to let the task know when the interrupt occurs. Use the dev->read_subdev
and comedi_handle_events() instead.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/addi_apci_1564.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c 
b/drivers/staging/comedi/drivers/addi_apci_1564.c
index 37e18b3..d7b920e 100644
--- a/drivers/staging/comedi/drivers/addi_apci_1564.c
+++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
@@ -117,6 +117,7 @@
  * the state of the interrupt capable inputs.
  */
 #define APCI1564_EVENT_COS BIT(31)
+#define APCI1564_EVENT_TIMER   BIT(30)
 #define APCI1564_EVENT_MASK0xffff /* all but [19:4] */
 
 struct apci1564_private {
@@ -187,15 +188,12 @@ static irqreturn_t apci1564_interrupt(int irq, void *d)
}
 
status = inl(devpriv->timer + ADDI_TCW_IRQ_REG);
-   if (status & 0x01) {
-   /*  Disable Timer Interrupt */
+   if (status & ADDI_TCW_IRQ) {
+   s->state |= APCI1564_EVENT_TIMER;
+
+   /* clear the interrupt */
ctrl = inl(devpriv->timer + ADDI_TCW_CTRL_REG);
outl(0x0, devpriv->timer + ADDI_TCW_CTRL_REG);
-
-   /* Send a signal to from kernel to user space */
-   send_sig(SIGIO, devpriv->tsk_current, 0);
-
-   /*  Enable Timer Interrupt */
outl(ctrl, devpriv->timer + ADDI_TCW_CTRL_REG);
}
 
-- 
2.8.2

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 04/20] staging: comedi: drivers: re-do macros for PLX PCI 9080 LASxRR values

[Hartley Sweeten]

On Friday, May 20, 2016 10:18 AM, Ian Abbott wrote:
> On 20/05/16 17:37, Hartley Sweeten wrote:
>> On Friday, May 20, 2016 6:49 AM, Ian Abbott wrote:
>>> Rename the macros for the PLX PCI 9080 LAS0RR and LAS1RR registers in
>>> "plx9080.h", using the prefix `PLX_LASRR_`.  Make use of the `BIT(x)`
>>> and `GENMASK(h,l)` macros to define the values.
>>>
>>> Define a macro `PLX_LASRR_PREFETCH` for the "prefetchable memory" bit in
>>> this register, and define a macro `PLX_LASRR_MLOC_MASK` to mask the PCI
>>> memory location control bits.

[snip]

>>> +#define PLX_LASRR_IO   BIT(0)  /* Map to: 1=I/O, 0=Mem 
>>> */
>>> +#define PLX_LASRR_ANY32(BIT(1) * 0)/* Locate anywhere in 
>>> 32 bit */
>>> +#define PLX_LASRR_LT1MB(BIT(1) * 1)/* Locate in 1st meg */
>>> +#define PLX_LASRR_ANY64(BIT(1) * 2)/* Locate anywhere in 
>>> 64 bit */
>>
>> The (BIT(n) * x) looks ugly.
>
> You won't like the remaining patches then!

You are correct... ;-)

> FWIW, all the constants end up with the same type (unsigned long) this way.

That's probably good but it sure makes the defines look ugly, and a bit hard to
understand imoh. You also don't know what the 'max' value for the bit-field
is without further digging.

I applied your whole series to see what the final header looks like. To me it
actually looks worse than the original.

The original had a number of whitespace issues that made it hard to follow and
the defines were lacking namespace. Personally I also don't can for all the 
enums
since the symbols are not actually used as enums just as raw values. But the 
'bit'
usage of the registers was fairly clear.

With your series applied the whtespace and namespace issues are addressed.
You also converted all the enums to defines which is great. But the 'bit' usage
now is a bit muddled.  I really don't care for the (BIT(n) * (x)) stuff. There 
are
also the various, unused and unnecessary, _SHIFT defines. Those just
add additional cruft.

I'm also not sure if all the bits require a comment. They seem to clutter the
header. Datasheets for the PLX-9080 are easy to find. Maybe just have a
comment for each register and remove all the bit comments.

> I have been looking for a solution to the problem where random people 
> change something like this:
>
> #define MY_COOLREG_VAL_FOO(0 << 5)
> #define MY_COOLREG_VAL_BAR(1 << 5)
> #define MY_COOLREG_VAL_BAZ(2 << 5)
>
> to:
>
> #define MY_COOLREG_VAL_FOO(0 << 5)
> #define MY_COOLREG_VAL_BARBIT(5)
> #define MY_COOLREG_VAL_BAZ(2 << 5)
>
> and this seemed like one way to do it.

Like I stated previously, I prefer something like this for the multi-bit
fields of a register.

>> #define PLX_LASSR_MLOC(x)(((x) & 0x3) << 1)
>> #define PLX_LASSR_MLOC_ANY32 PLX_LASSR_MLOC(0)
>> #define PLX_LASSR_MLOC_LT1MB PLX_LASSR_MLOC(1)
>> #define PLX_LASSR_MLOC_ANY64 PLX_LASSR_MLOC(2)
>> #define PLX_LASSR_MLOC_MASK  PLX_LASSR_MLOC(3)
>
> It is handy when matching it up with the data sheet though.  I have a 
> field that occupies bits 2 and 1.  It also doesn't expose a fairly 
> useless PLX_LASRR_MLOC() macro to the user of the header file.

The (BIT(n) * (x)) just looks odd.

The GENMASK() for a multi-bit field also makes it more difficult to
figure out what the maximum value for the field is when there are
more than just a few bits and the lower bit is not 0.

Anyway.. Technically it looks like your series doesn't  break anything
I just don't feel that it adds much clarity.

I'm still looking it over... Maybe I'll change my mind... ;-)

Regards,
Hartley


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 03/20] staging: comedi: drivers: rename PLX PCI 9080 register offsets

[Hartley Sweeten]

On Friday, May 20, 2016 9:31 AM, Ian Abbott wrote:
> On 20/05/16 17:21, Hartley Sweeten wrote:
>> Just an comment on your renaming.
>>
>> You also renamed the registers in the daqbook2000 driver in the
>> _REG_ format. Personally I prefer the format to be
>> __REG. That way a grep for _ will
>> return both the register and bit uses.
>
> It makes it easier to distinguish the register offsets from the register 
> values, imho.

I guess. It's just not as common in comedi right now and it does make
grep'ing for all the register/bit uses a bit more cumbersome.

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 04/20] staging: comedi: drivers: re-do macros for PLX PCI 9080 LASxRR values

[Hartley Sweeten]

On Friday, May 20, 2016 6:49 AM, Ian Abbott wrote:
> Rename the macros for the PLX PCI 9080 LAS0RR and LAS1RR registers in
> "plx9080.h", using the prefix `PLX_LASRR_`.  Make use of the `BIT(x)`
> and `GENMASK(h,l)` macros to define the values.
>
> Define a macro `PLX_LASRR_PREFETCH` for the "prefetchable memory" bit in
> this register, and define a macro `PLX_LASRR_MLOC_MASK` to mask the PCI
> memory location control bits.
>
> Signed-off-by: Ian Abbott 
> ---

[snip]

> diff --git a/drivers/staging/comedi/drivers/plx9080.h 
> b/drivers/staging/comedi/drivers/plx9080.h
> index 92d2480..8788117 100644
> --- a/drivers/staging/comedi/drivers/plx9080.h
> +++ b/drivers/staging/comedi/drivers/plx9080.h
> @@ -54,14 +54,16 @@ struct plx_dma_desc {
>  /* Local Address Space 1 Range Register */
>  #define PLX_REG_LAS1RR   0x00f0
>  
> -#define  LRNG_IO   0x0001/* Map to: 1=I/O, 0=Mem */
> -#define  LRNG_ANY320x/* Locate anywhere in 32 bit */
> -#define  LRNG_LT1MB0x0002/* Locate in 1st meg */
> -#define  LRNG_ANY640x0004/* Locate anywhere in 64 bit */
> -/*  bits that specify range for memory io */
> -#define  LRNG_MEM_MASK 0xfff0
> -/*  bits that specify range for normal io */
> -#define  LRNG_IO_MASK 0xfffc
> +#define PLX_LASRR_IO BIT(0)  /* Map to: 1=I/O, 0=Mem */
> +#define PLX_LASRR_ANY32  (BIT(1) * 0)/* Locate anywhere in 
> 32 bit */
> +#define PLX_LASRR_LT1MB  (BIT(1) * 1)/* Locate in 1st meg */
> +#define PLX_LASRR_ANY64  (BIT(1) * 2)/* Locate anywhere in 
> 64 bit */

The (BIT(n) * x) looks ugly.

These bit define the memory space encoding. I would prefer something
like this:

#define PLX_LASSR_MLOC(x)   (((x) & 0x3) << 1)
#define PLX_LASSR_MLOC_ANY32PLX_LASSR_MLOC(0)
#define PLX_LASSR_MLOC_LT1MBPLX_LASSR_MLOC(1)
#define PLX_LASSR_MLOC_ANY64PLX_LASSR_MLOC(2)

> +#define PLX_LASRR_MLOC_MASK  GENMASK(2, 1)   /* Memory location bits */

I guess the GENMASK() macro is common but it's currently
not used by any of the comedi code.

Using the macro above, the 'mask' would be:

#define PLX_LASSR_MLOC_MASK PLX_LASSR_MLOC(3)

> +#define PLX_LASRR_PREFETCH   BIT(3)  /* Memory is prefetchable */
> +/* bits that specify range for memory space decode bits */
> +#define PLX_LASRR_MEM_MASK   GENMASK(31, 4)
> +/* bits that specify range for i/o space decode bits */
> +#define PLX_LASRR_IO_MASKGENMASK(31, 2)
 
I suppose the GENMASK() use makes sense for these.

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 03/20] staging: comedi: drivers: rename PLX PCI 9080 register offsets

[Hartley Sweeten]

On Friday, May 20, 2016 6:49 AM, Ian Abbott wrote:
> Rename the macros in "plx9080.h" that define the offsets of registers,
> following the pattern `PLX_REG_`, where `` is the register
> name from the PLX PCI 9080 Data Book.
>
> Add defines for the "Mailbox" registers, and add parameterized macros
> for the mailbox registers and the DMA control registers.  Make use of
> the parameterized versions of the macros where it seems appropriate.
>
> The registers for supporting the I2O (Intelligent Input/Output) feature
> are largely left undefined, just defining enough to allow the I2O
> feature to be disabled.
>
> Signed-off-by: Ian Abbott 

Ian,

Just an comment on your renaming.

You also renamed the registers in the daqbook2000 driver in the
_REG_ format. Personally I prefer the format to be
__REG. That way a grep for _ will
return both the register and bit uses.

Currently the _REG_ form is not as common in comedi.
Looks like it's only used in amcc_35933.h and will be in daqboard2000.c.

The __REG form is used a lot.

Not a big deal, just a comment.

Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v3 06/14] staging: comedi: daqboard2000: rename register offset macros

[Hartley Sweeten]

On Thursday, May 19, 2016 2:56 AM, Ian Abbott wrote:
> Rename the macros defining register offsets to avoid CamelCase, and to
> use namespace associated with the driver.
>
> Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
> Reviewed-by: H Hartley Sweeten <hswee...@visionengravers.com>
> ---
> Other CamelCase issues in this patch will be dealt with by later
> patches in the series.
>
> v2: Shortened prefix from `DAQBOARD2000_` to `DB2K_`.
> v3: Removed a space before tab that slipped in in v2.

Ian,

Your [PATCH v2 07/14: ...] does not apply after this one.

It's probably better if Greg drops the previous patches and you
post a full series.

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v2 06/14] staging: comedi: daqboard2000: rename register offset macros

[Hartley Sweeten]

On Wednesday, May 18, 2016 5:37 AM, Ian Abbott wrote:
> Rename the macros defining register offsets to avoid CamelCase, and to
> use namespace associated with the driver.
>
> Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
> Reviewed-by: H Hartley Sweeten <hswee...@visionengravers.com>
> ---
> Other CamelCase issues in this patch will be dealt with by later
> patches in the series.
>
> v2: Shortened prefix from `DAQBOARD2000_` to `DB2K_`.
> ---
>  drivers/staging/comedi/drivers/daqboard2000.c | 112 
> ++
>  1 file changed, 61 insertions(+), 51 deletions(-)

[snip]

+#define DB2K_REG_DIO_P2_EXP_IO_16_BIT(x)   (0xc0 + (x) * 2) /* s16 */

You slipped an extra space in here:
WARNING: please, no space before tabs
#184: FILE: drivers/staging/comedi/drivers/daqboard2000.c:184:
+#define DB2K_REG_DIO_P2_EXP_IO_16_BIT(x) ^I(0xc0 + (x) * 2) /* s16 */$

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH v2 12/14] staging: comedi: daqboard2000: rename reference DACs register macros

[Hartley Sweeten]

On Wednesday, May 18, 2016 5:37 AM, Ian Abbott wrote:
> Rename the macros that define values for the reference DACs register to
> avoid CamelCase, and to make it clearer which register they are
> associated with.  Add a macro `DAQBOARD2000_REF_DACS_SET` for the value
> `0x80` that triggers setting one of the references.

[snip]

+#define DB2K_REF_DACS_SET  0x0080

Minor issue... Typo in the commit. Not sure if it's worth a v3.

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 14/14] staging: comedi: daqboard2000: prefer usleep_range()

[Hartley Sweeten]

On Tuesday, May 17, 2016 2:53 AM, Ian Abbott wrote:
> The checkpatch.pl warns about two `udelay(x)` calls, one of 100
> microseconds, and one of 10 microseconds.  The 100 microseconds one is
> used when waiting for FPGA to become ready to accept firmware, and is
> not that critical, so replace it with a call to `usleep_range(100,
> 1000)`.  The 10 microseconds one is called as each 16-bit word of
> firmware data is written.  A longer sleep would slow down firmware
> loading, so leave it alone.

The firmware blob in comedi-nonfree-firmware/daqboard2000 is
41236 bytes or 20618 words. With the 10 microsecond delay for
each word to total delay time is only 0.0206 seconds. I don't think a
small usleep_range() would slow down the firmware loading by much.
How about usleep_range(10, 20)?

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 06/14] staging: comedi: daqboard2000: rename register offset macros

[Hartley Sweeten]

On Tuesday, May 17, 2016 2:53 AM, Ian Abbott wrote:
> Rename the macros defining register offsets to avoid CamelCase, and to
> use namespace associated with the driver.
>
> Signed-off-by: Ian Abbott 
> ---
> Other CamelCase issues in this patch will be dealt with by later
> patches in the series.
> ---
>  drivers/staging/comedi/drivers/daqboard2000.c | 112 
> ++
>  1 file changed, 61 insertions(+), 51 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/daqboard2000.c 
> b/drivers/staging/comedi/drivers/daqboard2000.c
> index b3b68e8..92ff8f4 100644
> --- a/drivers/staging/comedi/drivers/daqboard2000.c
> +++ b/drivers/staging/comedi/drivers/daqboard2000.c
> @@ -151,35 +151,35 @@ static const struct comedi_lrange range_daqboard2000_ai 
> = {
>  /*
>   * Register Memory Map
>   */
> -#define acqControl   0x00/* u16 */
> -#define acqScanListFIFO  0x02/* u16 */
> -#define acqPacerClockDivLow  0x04/* u32 */
> -#define acqScanCounter   0x08/* u16 */
> -#define acqPacerClockDivHigh 0x0a/* u16 */
> -#define acqTriggerCount  0x0c/* u16 */
> -#define acqResultsFIFO   0x10/* u16 */
> -#define acqResultsShadow 0x14/* u16 */
> -#define acqAdcResult 0x18/* u16 */
> -#define dacScanCounter   0x1c/* u16 */
> -#define dacControl   0x20/* u16 */
> -#define dacFIFO  0x24/* s16 */
> -#define dacPacerClockDiv 0x2a/* u16 */
> -#define refDacs  0x2c/* u16 */
> -#define dioControl   0x30/* u16 */
> -#define dioP3hsioData0x32/* s16 */
> -#define dioP3Control 0x34/* u16 */
> -#define calEepromControl 0x36/* u16 */
> -#define dacSetting(x)(0x38 + (x) * 2) /* s16 */
> -#define dioP2ExpansionIO8Bit 0x40/* s16 */
> -#define ctrTmrControl0x80/* u16 */
> -#define ctrInput(x)  (0x88 + (x) * 2) /* s16 */
> -#define timerDivisor(x)  (0xa0 + (x) * 2) /* u16 */
> -#define dmaControl   0xb0/* u16 */
> -#define trigControl  0xb2/* u16 */
> -#define calEeprom0xb8/* u16 */
> -#define acqDigitalMark   0xba/* u16 */
> -#define trigDacs 0xbc/* u16 */
> -#define dioP2ExpansionIO16Bit(x) (0xc0 + (x) * 2) /* s16 */
> +#define DAQBOARD2000_REG_ACQ_CONTROL 0x00 /* u16 */
> +#define DAQBOARD2000_REG_ACQ_SCAN_LIST_FIFO  0x02 /* u16 */
> +#define DAQBOARD2000_REG_ACQ_PACER_CLOCK_DIV_LOW 0x04 /* u32 */
> +#define DAQBOARD2000_REG_ACQ_SCAN_COUNTER0x08 /* u16 */
> +#define DAQBOARD2000_REG_ACQ_PACER_CLOCK_DIV_HIGH0x0a /* u16 */
> +#define DAQBOARD2000_REG_ACQ_TRIGGER_COUNT   0x0c /* u16 */
> +#define DAQBOARD2000_REG_ACQ_RESULTS_FIFO0x10 /* u16 */
> +#define DAQBOARD2000_REG_ACQ_RESULTS_SHADOW  0x14 /* u16 */
> +#define DAQBOARD2000_REG_ACQ_ADC_RESULT  0x18 /* u16 */
> +#define DAQBOARD2000_REG_DAC_SCAN_COUNTER0x1c /* u16 */
> +#define DAQBOARD2000_REG_DAC_CONTROL 0x20 /* u16 */
> +#define DAQBOARD2000_REG_DAC_FIFO0x24 /* s16 */
> +#define DAQBOARD2000_REG_DAC_PACER_CLOCK_DIV 0x2a /* u16 */
> +#define DAQBOARD2000_REG_REF_DACS0x2c /* u16 */
> +#define DAQBOARD2000_REG_DIO_CONTROL 0x30 /* u16 */
> +#define DAQBOARD2000_REG_P3_HSIO_DATA0x32 /* s16 */
> +#define DAQBOARD2000_REG_P3_CONTROL  0x34 /* u16 */
> +#define DAQBOARD2000_REG_CAL_EEPROM_CONTROL  0x36 /* u16 */
> +#define DAQBOARD2000_REG_DAC_SETTING(x)  (0x38 + (x) * 2) /* s16 
> */
> +#define DAQBOARD2000_REG_DIO_P2_EXP_IO_8_BIT 0x40 /* s16 */
> +#define DAQBOARD2000_REG_COUNTER_TIMER_CONTROL   0x80 /* u16 */
> +#define DAQBOARD2000_REG_COUNTER_INPUT(x)(0x88 + (x) * 2) /* s16 */
> +#define DAQBOARD2000_REG_TIMER_DIV(x)(0xa0 + (x) * 2) /* u16 
> */
> +#define DAQBOARD2000_REG_DMA_CONTROL 0xb0 /* u16 */
> +#define DAQBOARD2000_REG_TRIG_CONTROL0xb2 /* u16 */
> +#define DAQBOARD2000_REG_CAL_EEPROM  0xb8 /* u16 */
> +#define DAQBOARD2000_REG_ACQ_DIGITAL_MARK0xba /* u16 */
> +#define DAQBOARD2000_REG_TRIG_DACS   0xbc /* u16 */
> +#define DAQBOARD2000_REG_DIO_P2_EXP_IO_16_BIT(x) (0xc0 + (x) * 2) /* s16 */

Any 

RE: [PATCH 05/14] staging: comedi: daqboard2000: rename serial EEPROM register macros

[Hartley Sweeten]

On Tuesday, May 17, 2016 2:53 AM, Ian Abbott wrote:
> Rename the macros defining values for the Serial EEPROM Control Register
> to avoid CamelCase.
>
> Signed-off-by: Ian Abbott 
> ---
>  drivers/staging/comedi/drivers/daqboard2000.c | 26 +-
>  1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/daqboard2000.c 
> b/drivers/staging/comedi/drivers/daqboard2000.c
> index fde0924..b3b68e8 100644
> --- a/drivers/staging/comedi/drivers/daqboard2000.c
> +++ b/drivers/staging/comedi/drivers/daqboard2000.c
> @@ -116,12 +116,12 @@
>  #define DAQBOARD2000_SUBSYSTEM_IDS4  0x0004  /* Daqboard/2000 - 4 Dacs */

Ian,

This board uses a PLX-9080 chip for the PCI interface.

If would be better to include  and use the register/bit defines
to remove the magic numbers.

The only PLX register used is PLX_CONTROL_REG (0x6c).

>  /* Initialization bits for the Serial EEPROM Control Register */
> -#define DAQBOARD2000_SECRProgPinHi  0x8001767e
> -#define DAQBOARD2000_SECRProgPinLo  0x8000767e
> -#define DAQBOARD2000_SECRLocalBusHi 0xc000767e
> -#define DAQBOARD2000_SECRLocalBusLo 0x8000767e
> -#define DAQBOARD2000_SECRReloadHi   0xa000767e
> -#define DAQBOARD2000_SECRReloadLo   0x8000767e
> +#define DAQBOARD2000_SECR_PROG_PIN_HI0x8001767e
> +#define DAQBOARD2000_SECR_PROG_PIN_LO0x8000767e
> +#define DAQBOARD2000_SECR_LOCAL_BUS_HI   0xc000767e
> +#define DAQBOARD2000_SECR_LOCAL_BUS_LO   0x8000767e
> +#define DAQBOARD2000_SECR_RELOAD_HI  0xa000767e
> +#define DAQBOARD2000_SECR_RELOAD_LO  0x8000767e

These "Initialization bits" are just various combinations of the
PLX_CONTROL_REG bit defines (CTL_*) to toggle the various
EEPROM bits.
 
>  /* SECR status bits */
>  #define DAQBOARD2000_EEPROM_PRESENT 0x1000
> @@ -438,9 +438,9 @@ static void daqboard2000_resetLocalBus(struct 
> comedi_device *dev)
>  {
>   struct daqboard2000_private *devpriv = dev->private;
>  
> - writel(DAQBOARD2000_SECRLocalBusHi, devpriv->plx + 0x6c);
> + writel(DAQBOARD2000_SECR_LOCAL_BUS_HI, devpriv->plx + 0x6c);
>   mdelay(10);
> - writel(DAQBOARD2000_SECRLocalBusLo, devpriv->plx + 0x6c);
> + writel(DAQBOARD2000_SECR_LOCAL_BUS_LO, devpriv->plx + 0x6c);
>   mdelay(10);
>  }
>  
> @@ -448,11 +448,11 @@ static void daqboard2000_reloadPLX(struct comedi_device 
> *dev)
>  {
>   struct daqboard2000_private *devpriv = dev->private;
>  
> - writel(DAQBOARD2000_SECRReloadLo, devpriv->plx + 0x6c);
> + writel(DAQBOARD2000_SECR_RELOAD_LO, devpriv->plx + 0x6c);
>   mdelay(10);
> - writel(DAQBOARD2000_SECRReloadHi, devpriv->plx + 0x6c);
> + writel(DAQBOARD2000_SECR_RELOAD_HI, devpriv->plx + 0x6c);
>   mdelay(10);
> - writel(DAQBOARD2000_SECRReloadLo, devpriv->plx + 0x6c);
> + writel(DAQBOARD2000_SECR_RELOAD_LO, devpriv->plx + 0x6c);
>   mdelay(10);
>  }
 >
> @@ -460,9 +460,9 @@ static void daqboard2000_pulseProgPin(struct 
> comedi_device *dev)
>  {
>   struct daqboard2000_private *devpriv = dev->private;
>  
> - writel(DAQBOARD2000_SECRProgPinHi, devpriv->plx + 0x6c);
> + writel(DAQBOARD2000_SECR_PROG_PIN_HI, devpriv->plx + 0x6c);
>   mdelay(10);
> - writel(DAQBOARD2000_SECRProgPinLo, devpriv->plx + 0x6c);
> + writel(DAQBOARD2000_SECR_PROG_PIN_LO, devpriv->plx + 0x6c);
>   mdelay(10); /* Not in the original code, but I like symmetry... */
>  }

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 01/14] staging: comedi: daqboard2000: remove commented out code

[Hartley Sweeten]

On Tuesday, May 17, 2016 2:53 AM, Ian Abbott wrote:
> Remove some commented out code.  Some of it uses constructs that don't
> exist in the driver, and probably come from the source code for the MS
> Windows driver.
>
> Signed-off-by: Ian Abbott 
> ---
>  drivers/staging/comedi/drivers/daqboard2000.c | 6 --
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/daqboard2000.c 
> b/drivers/staging/comedi/drivers/daqboard2000.c
> index 57ab668..8c9a024 100644
> --- a/drivers/staging/comedi/drivers/daqboard2000.c
> +++ b/drivers/staging/comedi/drivers/daqboard2000.c
> @@ -278,9 +278,7 @@ struct daqboard2000_private {
>  
>  static void writeAcqScanListEntry(struct comedi_device *dev, u16 entry)
>  {
> - /* udelay(4); */
>   writew(entry & 0x00ff, dev->mmio + acqScanListFIFO);
> - /* udelay(4); */
>   writew((entry >> 8) & 0x00ff, dev->mmio + acqScanListFIFO);
>  }
>  
> @@ -315,10 +313,6 @@ static void setup_sampling(struct comedi_device *dev, 
> int chan, int gain)
>   word3 = 0;
>   break;
>   }
> -/*
> -  dev->eeprom.correctionDACSE[i][j][k].offset = 0x800;
> -  dev->eeprom.correctionDACSE[i][j][k].gain = 0xc00;
> -*/
>   /* These should be read from EEPROM */
>   word2 |= 0x0800;
>   word3 |= 0xc000;

It might be a good idea to add a comment about the magic 0x0800 and 0xc000 
values:

>   word2 |= 0x0800;/* offset */
>   word3 |= 0xc000;/* gain */

Wish I could find a register map for this board. I sent an email to Measurement 
Computing
to see if one is available.

Regards,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: comedi: dt2811: add async command support for AI subdevice

[H Hartley Sweeten]

The interrupt support available on this board is pretty limited but its
simple enough to give basic async command support.

This allows reading a single channel continuously using either the internal
or an external clock to trigger each conversion. The command can also use
the external trigger input to start the command if the external clock is
being used for conversions.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
This is a follow up to: [PATCH v2 00/19] staging: comedi: dt2811: cleanup driver

According to Data Translation's website, this ISA board _just_ went EOL on
May 1, 2016. Who knows, there might actually be someone out there that is
using it.

 drivers/staging/comedi/drivers/dt2811.c | 371 +---
 1 file changed, 340 insertions(+), 31 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 415512b..904f6377 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -2,11 +2,7 @@
  * Comedi driver for Data Translation DT2811
  *
  * COMEDI - Linux Control and Measurement Device Interface
- * History:
- * Base Version  - David A. Schleef <d...@schleef.org>
- * December 1998 - Updated to work.  David does not have a DT2811
- * board any longer so this was suffering from bitrot.
- * Updated performed by ...
+ * Copyright (C) David A. Schleef <d...@schleef.org>
  *
  * 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
@@ -28,7 +24,7 @@
  *
  * Configuration options:
  *   [0] - I/O port base address
- *   [1] - IRQ, although this is currently unused
+ *   [1] - IRQ (optional, needed for async command support)
  *   [2] - A/D reference (# of analog inputs)
  *0 = single-ended (16 channels)
  *1 = differential (8 channels)
@@ -51,6 +47,9 @@
  */
 
 #include 
+#include 
+#include 
+
 #include "../comedidev.h"
 
 /*
@@ -61,16 +60,9 @@
 #define DT2811_ADCSR_ADERROR   BIT(6)  /* r  1=A/D error */
 #define DT2811_ADCSR_ADBUSYBIT(5)  /* r  1=A/D busy */
 #define DT2811_ADCSR_CLRERROR  BIT(4)
-#define DT2811_ADCSR_INTENBBIT(2)  /* r/w1=interupts ena */
+#define DT2811_ADCSR_DMAENBBIT(3)  /* r/w1=dma ena */
+#define DT2811_ADCSR_INTENBBIT(2)  /* r/w1=interupts ena */
 #define DT2811_ADCSR_ADMODE(x) (((x) & 0x3) << 0)
-/* single conversion on ADGCR load */
-#define DT2811_ADCSR_ADMODE_SINGLE DT2811_ADCSR_ADMODE(0)
-/* continuous conversion, internal clock, (clock enabled on ADGCR load) */
-#define DT2811_ADCSR_ADMODE_CONT   DT2811_ADCSR_ADMODE(1)
-/* continuous conversion, internal clock, external trigger */
-#define DT2811_ADCSR_ADMODE_EXT_TRIG   DT2811_ADCSR_ADMODE(2)
-/* continuous conversion, external clock, external trigger */
-#define DT2811_ADCSR_ADMODE_EXTDT2811_ADCSR_ADMODE(3)
 
 #define DT2811_ADGCR_REG   0x01/* r/w  A/D Gain/Channel */
 #define DT2811_ADGCR_GAIN(x)   (((x) & 0x3) << 6)
@@ -85,6 +77,12 @@
 #define DT2811_DI_REG  0x06/* r   Digital Input Port 0 */
 #define DT2811_DO_REG  0x06/* w   Digital Output Port 1 */
 
+#define DT2811_TMRCTR_REG  0x07/* r/w  Timer/Counter */
+#define DT2811_TMRCTR_MANTISSA(x)  (((x) & 0x7) << 3)
+#define DT2811_TMRCTR_EXPONENT(x)  (((x) & 0x7) << 0)
+
+#define DT2811_OSC_BASE1666/* 600 kHz = 
1666.6667ns */
+
 /*
  * Timer frequency control:
  *   DT2811_TMRCTR_MANTISSADT2811_TMRCTR_EXPONENT
@@ -98,9 +96,13 @@
  *6  6  100 kHz 6   100
  *7 12   50 kHz 7   1000
  */
-#define DT2811_TMRCTR_REG  0x07/* r/w  Timer/Counter */
-#define DT2811_TMRCTR_MANTISSA(x)  (((x) & 0x7) << 3)
-#define DT2811_TMRCTR_EXPONENT(x)  (((x) & 0x7) << 0)
+const unsigned int dt2811_clk_dividers[] = {
+   1, 10, 2, 3, 4, 5, 6, 12
+};
+
+const unsigned int dt2811_clk_multipliers[] = {
+   1, 10, 100, 1000, 1, 10, 100, 1000
+};
 
 /*
  * The Analog Input range is set using jumpers on the board.
@@ -182,6 +184,287 @@ static const struct dt2811_board dt2811_boards[] = {
},
 };
 
+struct dt2811_private {
+   unsigned int ai_divisor;
+};
+
+static unsigned int dt2811_ai_read_sample(struct comedi_device *dev,
+ struct comedi_subdevice *s)
+{
+   unsigned int val;
+
+   val = inb(dev->iobase + DT2811_ADDATA_LO_REG) |
+ (inb(dev->iobase + DT2811_ADDATA_HI_REG) << 8);
+
+   return val & s->maxdata;
+}
+
+static irqreturn_t dt2811_interrupt(int irq, v

[PATCH v2 08/19] staging: comedi: dt2811: tidy up Digital Input/Output register defines

[H Hartley Sweeten]

The digital input and outputs are separate ports even though they share the
same register offset. For aesthetics, define then separately and remove the
redundant information in the comment.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 1717281..61aa323 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -73,6 +73,9 @@
 #define DT2811_DADATA_LO_REG(x)(0x02 + ((x) * 2)) /* w D/A 
Data low */
 #define DT2811_DADATA_HI_REG(x)(0x03 + ((x) * 2)) /* w D/A 
Data high */
 
+#define DT2811_DI_REG  0x06/* r   Digital Input Port 0 */
+#define DT2811_DO_REG  0x06/* w   Digital Output Port 1 */
+
 static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
4, {
UNI_RANGE(5),
@@ -128,9 +131,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 };
 
 /*
-   0x06 (R) DIO0 Digital Input Port 0
-   (W) DIO1 Digital Output Port 1
-
0x07 TMRCTR (R/W) Timer/Counter Register
bits 6,7 - reserved
bits 5-3 - Timer frequency control (mantissa)
@@ -158,7 +158,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 #define TIMEOUT 1
 
-#define DT2811_DIO 6
 #define DT2811_TMRCTR 7
 
 struct dt2811_board {
@@ -254,7 +253,7 @@ static int dt2811_di_insn_bits(struct comedi_device *dev,
   struct comedi_subdevice *s,
   struct comedi_insn *insn, unsigned int *data)
 {
-   data[1] = inb(dev->iobase + DT2811_DIO);
+   data[1] = inb(dev->iobase + DT2811_DI_REG);
 
return insn->n;
 }
@@ -265,7 +264,7 @@ static int dt2811_do_insn_bits(struct comedi_device *dev,
   unsigned int *data)
 {
if (comedi_dio_update_state(s, data))
-   outb(s->state, dev->iobase + DT2811_DIO);
+   outb(s->state, dev->iobase + DT2811_DO_REG);
 
data[1] = s->state;
 
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 05/19] staging: comedi: dt2811: tidy up A/D Gain/Channel register defines

[H Hartley Sweeten]

Cleanup the defines for this register and its bits and remove the
redundant information in the comment.

Make the (*insn_read) use the range to set the gain bits correctly.
Currently the gain is always set to 1.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 433cfd1..f72d6c8 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -63,6 +63,10 @@
 /* continuous conversion, external clock, external trigger */
 #define DT2811_ADCSR_ADMODE_EXTDT2811_ADCSR_ADMODE(3)
 
+#define DT2811_ADGCR_REG   0x01/* r/w  A/D Gain/Channel */
+#define DT2811_ADGCR_GAIN(x)   (((x) & 0x3) << 6)
+#define DT2811_ADGCR_CHAN(x)   (((x) & 0xf) << 0)
+
 static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
4, {
UNI_RANGE(5),
@@ -119,16 +123,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 /*
 
-   0x01ADGCR R/W A/D Gain/Channel Register
-   bit 6,7 - (R/W) gain select
-   00  gain=1, both PGH, PGL models
-   01  gain=2 PGH, 10 PGL
-   10  gain=4 PGH, 100 PGL
-   11  gain=8 PGH, 500 PGL
-   bit 4,5 - reserved
-   bit 3-0 - (R/W) channel select
-   channel number from 0-15
-
0x02,0x03 (R) ADDAT A/D Data Register
(W) DADAT0 D/A Data Register 0
0x02 low byte
@@ -166,7 +160,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 #define TIMEOUT 1
 
-#define DT2811_ADGCR 1
 #define DT2811_ADDATLO 2
 #define DT2811_ADDATHI 3
 #define DT2811_DADAT0LO 2
@@ -219,12 +212,15 @@ static int dt2811_ai_eoc(struct comedi_device *dev,
 static int dt2811_ai_insn(struct comedi_device *dev, struct comedi_subdevice 
*s,
  struct comedi_insn *insn, unsigned int *data)
 {
-   int chan = CR_CHAN(insn->chanspec);
+   unsigned int chan = CR_CHAN(insn->chanspec);
+   unsigned int range = CR_RANGE(insn->chanspec);
int ret;
int i;
 
for (i = 0; i < insn->n; i++) {
-   outb(chan, dev->iobase + DT2811_ADGCR);
+   /* select channel/gain and trigger conversion */
+   outb(DT2811_ADGCR_CHAN(chan) | DT2811_ADGCR_GAIN(range),
+dev->iobase + DT2811_ADGCR_REG);
 
ret = comedi_timeout(dev, s, insn, dt2811_ai_eoc, 0);
if (ret)
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 11/19] staging: comedi: dt2811: simplify analog output range options

[H Hartley Sweeten]

The D/A ranges are not programmable. Currently this driver uses some
configuration options to select the comedi_lrange for each channel.
This is a bit messy and it requires the user to make sure the correct
option value is used.

The range information isn't used by the driver. The user space library
uses it to convert between raw data values and physical units. If the
user passed an incorrect option when attaching the driver the conversion
will be incorrect.

Simplify the analog output ranges by providing a range_table that
contains all the possible output ranges. The user can then select the
correct range when converting values.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 69 +
 1 file changed, 26 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 0512d70..6850eb8 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -37,8 +37,14 @@
  *0 = [-5, 5]
  *1 = [-2.5, 2.5]
  *2 = [0, 5]
- *   [4] - D/A 0 range (same as A/D range)
- *   [5] - D/A 1 range (same as A/D range)
+ *   [4] - D/A 0 range (deprecated, see below)
+ *   [5] - D/A 1 range (deprecated, see below)
+ *
+ * Notes:
+ *   - D/A ranges are not programmable. The AO subdevice has a range_table
+ * containing all the possible analog output ranges. Use the range
+ * that matches your board configuration to convert between data
+ * values and physical units.
  */
 
 #include 
@@ -147,6 +153,23 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
}
 };
 
+/*
+ * The Analog Output range is set per-channel using jumpers on the board.
+ *
+ * DAC0 JumpersDAC1 Jumpers
+ * Output RangeW5  W6  W7  W8  W1  W2  W3  W4
+ * -5V to +5V  In  Out In  Out In  Out In  Out
+ * -2.5V to +2.5V  In  Out Out In  In  Out Out In
+ * 0 to +5VOut In  Out In  Out In  Out In
+ */
+static const struct comedi_lrange dt2811_ao_ranges = {
+   3, {
+   BIP_RANGE(5),   /* default setting from factory */
+   BIP_RANGE(2.5),
+   UNI_RANGE(5)
+   }
+};
+
 #define TIMEOUT 1
 
 struct dt2811_board {
@@ -164,16 +187,6 @@ struct dt2811_private {
enum {
adc_singleended, adc_diff, adc_pseudo_diff
} adc_mux;
-   enum {
-   dac_bipolar_5, dac_bipolar_2_5, dac_unipolar_5
-   } dac_range[2];
-   const struct comedi_lrange *range_type_list[2];
-};
-
-static const struct comedi_lrange *dac_range_types[] = {
-   _bipolar5,
-   _bipolar2_5,
-   _unipolar5
 };
 
 static int dt2811_ai_eoc(struct comedi_device *dev,
@@ -294,34 +307,6 @@ static int dt2811_attach(struct comedi_device *dev, struct 
comedi_devconfig *it)
devpriv->adc_mux = adc_singleended;
break;
}
-   switch (it->options[4]) {
-   case 0:
-   devpriv->dac_range[0] = dac_bipolar_5;
-   break;
-   case 1:
-   devpriv->dac_range[0] = dac_bipolar_2_5;
-   break;
-   case 2:
-   devpriv->dac_range[0] = dac_unipolar_5;
-   break;
-   default:
-   devpriv->dac_range[0] = dac_bipolar_5;
-   break;
-   }
-   switch (it->options[5]) {
-   case 0:
-   devpriv->dac_range[1] = dac_bipolar_5;
-   break;
-   case 1:
-   devpriv->dac_range[1] = dac_bipolar_2_5;
-   break;
-   case 2:
-   devpriv->dac_range[1] = dac_unipolar_5;
-   break;
-   default:
-   devpriv->dac_range[1] = dac_bipolar_5;
-   break;
-   }
 
s = >subdevices[0];
/* initialize the ADC subdevice */
@@ -349,9 +334,7 @@ static int dt2811_attach(struct comedi_device *dev, struct 
comedi_devconfig *it)
s->subdev_flags = SDF_WRITABLE;
s->n_chan = 2;
s->maxdata = 0xfff;
-   s->range_table_list = devpriv->range_type_list;
-   devpriv->range_type_list[0] = dac_range_types[devpriv->dac_range[0]];
-   devpriv->range_type_list[1] = dac_range_types[devpriv->dac_range[1]];
+   s->range_table = _ao_ranges;
s->insn_write = dt2811_ao_insn_write;
 
ret = comedi_alloc_subdev_readback(s);
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 16/19] staging: comedi: dt2811: tidy up analog input subdevice init

[H Hartley Sweeten]

Add some whitespace to the analog output subdevice initialization
and rename the (*insn_read) function.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index f0b3c5e..44afa78 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -197,8 +197,10 @@ static int dt2811_ai_eoc(struct comedi_device *dev,
return -EBUSY;
 }
 
-static int dt2811_ai_insn(struct comedi_device *dev, struct comedi_subdevice 
*s,
- struct comedi_insn *insn, unsigned int *data)
+static int dt2811_ai_insn_read(struct comedi_device *dev,
+  struct comedi_subdevice *s,
+  struct comedi_insn *insn,
+  unsigned int *data)
 {
unsigned int chan = CR_CHAN(insn->chanspec);
unsigned int range = CR_RANGE(insn->chanspec);
@@ -283,17 +285,17 @@ static int dt2811_attach(struct comedi_device *dev, 
struct comedi_devconfig *it)
if (ret)
return ret;
 
+   /* Analog Input subdevice */
s = >subdevices[0];
-   /* initialize the ADC subdevice */
-   s->type = COMEDI_SUBD_AI;
-   s->subdev_flags = SDF_READABLE |
+   s->type = COMEDI_SUBD_AI;
+   s->subdev_flags = SDF_READABLE |
  (it->options[2] == 1) ? SDF_DIFF :
  (it->options[2] == 2) ? SDF_COMMON : SDF_GROUND;
-   s->n_chan = (it->options[2] == 1) ? 8 : 16;
-   s->insn_read = dt2811_ai_insn;
-   s->maxdata = 0xfff;
+   s->n_chan   = (it->options[2] == 1) ? 8 : 16;
+   s->maxdata  = 0x0fff;
s->range_table  = board->is_pgh ? _pgh_ai_ranges
: _pgl_ai_ranges;
+   s->insn_read= dt2811_ai_insn_read;
 
/* Analog Output subdevice */
s = >subdevices[1];
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 01/19] staging: comedi: dt2811: tidy up copyright and comedi comments

[H Hartley Sweeten]

Fix the checkpatch.pl issue:
WARNING: Block comments use * on subsequent lines

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 78 -
 1 file changed, 39 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index a807732..499637b 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -1,45 +1,45 @@
 /*
-   comedi/drivers/dt2811.c
-   Hardware driver for Data Translation DT2811
-
-   COMEDI - Linux Control and Measurement Device Interface
-   History:
-   Base Version  - David A. Schleef <d...@schleef.org>
-   December 1998 - Updated to work.  David does not have a DT2811
-   board any longer so this was suffering from bitrot.
-   Updated performed by ...
-
-   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 driver for Data Translation DT2811
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * History:
+ * Base Version  - David A. Schleef <d...@schleef.org>
+ * December 1998 - Updated to work.  David does not have a DT2811
+ * board any longer so this was suffering from bitrot.
+ * Updated performed by ...
+ *
+ * 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: dt2811
-Description: Data Translation DT2811
-Author: ds
-Devices: [Data Translation] DT2811-PGL (dt2811-pgl), DT2811-PGH (dt2811-pgh)
-Status: works
-
-Configuration options:
-  [0] - I/O port base address
-  [1] - IRQ, although this is currently unused
-  [2] - A/D reference
- 0 = signle-ended
- 1 = differential
- 2 = pseudo-differential (common reference)
-  [3] - A/D range
- 0 = [-5, 5]
- 1 = [-2.5, 2.5]
- 2 = [0, 5]
-  [4] - D/A 0 range (same choices)
-  [4] - D/A 1 range (same choices)
-*/
+ * Driver: dt2811
+ * Description: Data Translation DT2811
+ * Author: ds
+ * Devices: [Data Translation] DT2811-PGL (dt2811-pgl), DT2811-PGH (dt2811-pgh)
+ * Status: works
+ *
+ * Configuration options:
+ *   [0] - I/O port base address
+ *   [1] - IRQ, although this is currently unused
+ *   [2] - A/D reference
+ *0 = single-ended
+ *1 = differential
+ *2 = pseudo-differential (common reference)
+ *   [3] - A/D range
+ *0 = [-5, 5]
+ *1 = [-2.5, 2.5]
+ *2 = [0, 5]
+ *   [4] - D/A 0 range (same as A/D range)
+ *   [5] - D/A 1 range (same as A/D range)
+ */
 
 #include 
 #include "../comedidev.h"
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 17/19] staging: comedi: dt2811: remove unused define

[H Hartley Sweeten]

This define is not used by the driver. Remove it.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 44afa78..b1926e7 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -168,8 +168,6 @@ static const struct comedi_lrange dt2811_ao_ranges = {
}
 };
 
-#define TIMEOUT 1
-
 struct dt2811_board {
const char *name;
unsigned int is_pgh:1;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 18/19] staging: comedi: dt2811: rename 'boardtypes'

[H Hartley Sweeten]

Rename this array so it has namespace associated with the driver.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index b1926e7..312403f 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -173,7 +173,7 @@ struct dt2811_board {
unsigned int is_pgh:1;
 };
 
-static const struct dt2811_board boardtypes[] = {
+static const struct dt2811_board dt2811_boards[] = {
{
.name   = "dt2811-pgh",
.is_pgh = 1,
@@ -334,8 +334,8 @@ static struct comedi_driver dt2811_driver = {
.module = THIS_MODULE,
.attach = dt2811_attach,
.detach = comedi_legacy_detach,
-   .board_name = [0].name,
-   .num_names  = ARRAY_SIZE(boardtypes),
+   .board_name = _boards[0].name,
+   .num_names  = ARRAY_SIZE(dt2811_boards),
.offset = sizeof(struct dt2811_board),
 };
 module_comedi_driver(dt2811_driver);
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 10/19] staging: comedi: dt2811: tidy up the digital subdevices

[H Hartley Sweeten]

Add some whitespace to the digital input and output subdevice
initialization. Reorder the initialization a bit.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 34 -
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 6ea1ad4..0512d70 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -240,7 +240,8 @@ static int dt2811_ao_insn_write(struct comedi_device *dev,
 
 static int dt2811_di_insn_bits(struct comedi_device *dev,
   struct comedi_subdevice *s,
-  struct comedi_insn *insn, unsigned int *data)
+  struct comedi_insn *insn,
+  unsigned int *data)
 {
data[1] = inb(dev->iobase + DT2811_DI_REG);
 
@@ -357,24 +358,23 @@ static int dt2811_attach(struct comedi_device *dev, 
struct comedi_devconfig *it)
if (ret)
return ret;
 
+   /* Digital Input subdevice */
s = >subdevices[2];
-   /* di subdevice */
-   s->type = COMEDI_SUBD_DI;
-   s->subdev_flags = SDF_READABLE;
-   s->n_chan = 8;
-   s->insn_bits = dt2811_di_insn_bits;
-   s->maxdata = 1;
-   s->range_table = _digital;
-
+   s->type = COMEDI_SUBD_DI;
+   s->subdev_flags = SDF_READABLE;
+   s->n_chan   = 8;
+   s->maxdata  = 1;
+   s->range_table  = _digital;
+   s->insn_bits= dt2811_di_insn_bits;
+
+   /* Digital Output subdevice */
s = >subdevices[3];
-   /* do subdevice */
-   s->type = COMEDI_SUBD_DO;
-   s->subdev_flags = SDF_WRITABLE;
-   s->n_chan = 8;
-   s->insn_bits = dt2811_do_insn_bits;
-   s->maxdata = 1;
-   s->state = 0;
-   s->range_table = _digital;
+   s->type = COMEDI_SUBD_DO;
+   s->subdev_flags = SDF_WRITABLE;
+   s->n_chan   = 8;
+   s->maxdata  = 1;
+   s->range_table  = _digital;
+   s->insn_bits= dt2811_do_insn_bits;
 
return 0;
 }
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 19/19] staging: comedi: dt2811: update the MODULE_DESCRIPTION

[H Hartley Sweeten]

Change the MODULE_DESCRIPTION to something more useful than the generic
"Comedi low-level driver".

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 312403f..415512b 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -341,5 +341,5 @@ static struct comedi_driver dt2811_driver = {
 module_comedi_driver(dt2811_driver);
 
 MODULE_AUTHOR("Comedi http://www.comedi.org;);
-MODULE_DESCRIPTION("Comedi low-level driver");
+MODULE_DESCRIPTION("Comedi driver for Data Translation DT2811 series boards");
 MODULE_LICENSE("GPL");
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 15/19] staging: comedi: dt2811: simplify analog input range options

[H Hartley Sweeten]

The A/D ranges are not programmable but the gain is. Currently this driver
uses a configuration option to select the comedi_lrange that will be used
for the analog input subdevice. This requires that the user makes sure the
correct option value is used.

The user space library uses the range information to convert between raw
values and physical units. If the user passed an incorrect option when
attaching the driver the conversion will be incorrect.

A previous patch allowed the gain to be set based on the chanspec range.
Prior to that the gain was always set to 1 so any conversion with a gain
that is not 1 would be incorrect anyway.

Simplify the analog input ranges by providing a range_table for the pgh and
pgl boards that contain all the possible range/gain options. The user can
then select the correct range (and gain) when converting values or reading
the analog inputs.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 143 ++--
 1 file changed, 61 insertions(+), 82 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 379b7d5..f0b3c5e 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -33,14 +33,17 @@
  *0 = single-ended (16 channels)
  *1 = differential (8 channels)
  *2 = pseudo-differential (16 channels)
- *   [3] - A/D range
- *0 = [-5, 5]
- *1 = [-2.5, 2.5]
- *2 = [0, 5]
+ *   [3] - A/D range (deprecated, see below)
  *   [4] - D/A 0 range (deprecated, see below)
  *   [5] - D/A 1 range (deprecated, see below)
  *
  * Notes:
+ *   - A/D ranges are not programmable but the gain is. The AI subdevice has
+ * a range_table containing all the possible analog input range/gain
+ * options for the dt2811-pgh or dt2811-pgl. Use the range that matches
+ * your board configuration and the desired gain to correctly convert
+ * between data values and physical units and to set the correct output
+ * gain.
  *   - D/A ranges are not programmable. The AO subdevice has a range_table
  * containing all the possible analog output ranges. Use the range
  * that matches your board configuration to convert between data
@@ -99,57 +102,52 @@
 #define DT2811_TMRCTR_MANTISSA(x)  (((x) & 0x7) << 3)
 #define DT2811_TMRCTR_EXPONENT(x)  (((x) & 0x7) << 0)
 
-static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
-   4, {
-   UNI_RANGE(5),
-   UNI_RANGE(2.5),
-   UNI_RANGE(1.25),
-   UNI_RANGE(0.625)
-   }
-};
-
-static const struct comedi_lrange range_dt2811_pgh_ai_2_5_bipolar = {
-   4, {
-   BIP_RANGE(2.5),
-   BIP_RANGE(1.25),
-   BIP_RANGE(0.625),
-   BIP_RANGE(0.3125)
-   }
-};
-
-static const struct comedi_lrange range_dt2811_pgh_ai_5_bipolar = {
-   4, {
-   BIP_RANGE(5),
-   BIP_RANGE(2.5),
-   BIP_RANGE(1.25),
-   BIP_RANGE(0.625)
-   }
-};
-
-static const struct comedi_lrange range_dt2811_pgl_ai_5_unipolar = {
-   4, {
-   UNI_RANGE(5),
-   UNI_RANGE(0.5),
-   UNI_RANGE(0.05),
-   UNI_RANGE(0.01)
-   }
-};
-
-static const struct comedi_lrange range_dt2811_pgl_ai_2_5_bipolar = {
-   4, {
-   BIP_RANGE(2.5),
-   BIP_RANGE(0.25),
-   BIP_RANGE(0.025),
-   BIP_RANGE(0.005)
+/*
+ * The Analog Input range is set using jumpers on the board.
+ *
+ * Input Range W9  W10
+ * -5V to +5V  In  Out
+ * -2.5V to +2.5V  In  In
+ * 0V to +5V   Out In
+ *
+ * The gain may be set to 1, 2, 4, or 8 (on the dt2811-pgh) or to
+ * 1, 10, 100, 500 (on the dt2811-pgl).
+ */
+static const struct comedi_lrange dt2811_pgh_ai_ranges = {
+   12, {
+   BIP_RANGE(5),   /* range 0: gain=1 */
+   BIP_RANGE(2.5), /* range 1: gain=2 */
+   BIP_RANGE(1.25),/* range 2: gain=4 */
+   BIP_RANGE(0.625),   /* range 3: gain=8 */
+
+   BIP_RANGE(2.5), /* range 0+4: gain=1 */
+   BIP_RANGE(1.25),/* range 1+4: gain=2 */
+   BIP_RANGE(0.625),   /* range 2+4: gain=4 */
+   BIP_RANGE(0.3125),  /* range 3+4: gain=8 */
+
+   UNI_RANGE(5),   /* range 0+8: gain=1 */
+   UNI_RANGE(2.5), /* range 1+8: gain=2 */
+   UNI_RANGE(1.25),/* range 2+8: gain=4 */
+   UNI_RANGE(0.625)/* range 3+8: gain=8 */
}
 };
 
-static const struct comedi_lrange range_dt2811_pgl_ai_5_bipolar = {
-   4, {
-   BIP_R

[PATCH v2 14/19] staging: comedi: dt2811: remove private data

[H Hartley Sweeten]

The remaining members of the private data are not used by the driver.
Remove it and the allocation.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 1fe657c..379b7d5 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -181,11 +181,6 @@ struct dt2811_board {
 
 enum { card_2811_pgh, card_2811_pgl };
 
-struct dt2811_private {
-   int ntrig;
-   int curadchan;
-};
-
 static int dt2811_ai_eoc(struct comedi_device *dev,
 struct comedi_subdevice *s,
 struct comedi_insn *insn,
@@ -274,7 +269,6 @@ static int dt2811_do_insn_bits(struct comedi_device *dev,
 static int dt2811_attach(struct comedi_device *dev, struct comedi_devconfig 
*it)
 {
const struct dt2811_board *board = dev->board_ptr;
-   struct dt2811_private *devpriv;
struct comedi_subdevice *s;
int ret;
 
@@ -286,10 +280,6 @@ static int dt2811_attach(struct comedi_device *dev, struct 
comedi_devconfig *it)
if (ret)
return ret;
 
-   devpriv = comedi_alloc_devpriv(dev, sizeof(*devpriv));
-   if (!devpriv)
-   return -ENOMEM;
-
s = >subdevices[0];
/* initialize the ADC subdevice */
s->type = COMEDI_SUBD_AI;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 07/19] staging: comedi: dt2811: tidy up D/A Data register defines

[H Hartley Sweeten]

Cleanup the defines for these registers and and remove the redundant
information in the comment.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 19 +--
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 0c0b7b7..1717281 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -70,6 +70,9 @@
 #define DT2811_ADDATA_LO_REG   0x02/* r   A/D Data low byte */
 #define DT2811_ADDATA_HI_REG   0x03/* r   A/D Data high byte */
 
+#define DT2811_DADATA_LO_REG(x)(0x02 + ((x) * 2)) /* w D/A 
Data low */
+#define DT2811_DADATA_HI_REG(x)(0x03 + ((x) * 2)) /* w D/A 
Data high */
+
 static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
4, {
UNI_RANGE(5),
@@ -125,14 +128,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 };
 
 /*
-
-   0x02,0x03
-   (W) DADAT0 D/A Data Register 0
-   0x02 low byte
-   0x03 high byte
-
-   0x04,0x05 (W) DADAT0 D/A Data Register 1
-
0x06 (R) DIO0 Digital Input Port 0
(W) DIO1 Digital Output Port 1
 
@@ -163,10 +158,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 #define TIMEOUT 1
 
-#define DT2811_DADAT0LO 2
-#define DT2811_DADAT0HI 3
-#define DT2811_DADAT1LO 4
-#define DT2811_DADAT1HI 5
 #define DT2811_DIO 6
 #define DT2811_TMRCTR 7
 
@@ -250,9 +241,9 @@ static int dt2811_ao_insn_write(struct comedi_device *dev,
 
for (i = 0; i < insn->n; i++) {
val = data[i];
-   outb(val & 0xff, dev->iobase + DT2811_DADAT0LO + 2 * chan);
+   outb(val & 0xff, dev->iobase + DT2811_DADATA_LO_REG(chan));
outb((val >> 8) & 0xff,
-dev->iobase + DT2811_DADAT0HI + 2 * chan);
+dev->iobase + DT2811_DADATA_HI_REG(chan));
}
s->readback[chan] = val;
 
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 12/19] staging: comedi: dt2811: tidy up analog output subdevice init

[H Hartley Sweeten]

Add some whitespace to the analog output subdevice initialization.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 6850eb8..b60ee74 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -328,14 +328,14 @@ static int dt2811_attach(struct comedi_device *dev, 
struct comedi_devconfig *it)
break;
}
 
+   /* Analog Output subdevice */
s = >subdevices[1];
-   /* ao subdevice */
-   s->type = COMEDI_SUBD_AO;
-   s->subdev_flags = SDF_WRITABLE;
-   s->n_chan = 2;
-   s->maxdata = 0xfff;
-   s->range_table = _ao_ranges;
-   s->insn_write = dt2811_ao_insn_write;
+   s->type = COMEDI_SUBD_AO;
+   s->subdev_flags = SDF_WRITABLE;
+   s->n_chan   = 2;
+   s->maxdata  = 0x0fff;
+   s->range_table  = _ao_ranges;
+   s->insn_write   = dt2811_ao_insn_write;
 
ret = comedi_alloc_subdev_readback(s);
if (ret)
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 00/19] staging: comedi: dt2811: cleanup driver

[H Hartley Sweeten]

Fix all the checkpatch.pl isses and tidy up the driver.

v2: sort the analog input range table to match the old configuration
option scheme, as requested by Ian Abbott, in patch 15/19.

H Hartley Sweeten (19):
  staging: comedi: dt2811: tidy up copyright and comedi comments
  staging: comedi: dt2811: remove redundant block comment
  staging: comedi: dt2811: remove disabled code
  staging: comedi: dt2811: tidy up A/D Control/Status register defines
  staging: comedi: dt2811: tidy up A/D Gain/Channel register defines
  staging: comedi: dt2811: tidy up A/D Data register defines
  staging: comedi: dt2811: tidy up D/A Data register defines
  staging: comedi: dt2811: tidy up Digital Input/Output register defines
  staging: comedi: dt2811: tidy up Timer/Counter register defines
  staging: comedi: dt2811: tidy up the digital subdevices
  staging: comedi: dt2811: simplify analog output range options
  staging: comedi: dt2811: tidy up analog output subdevice init
  staging: comedi: dt2811: simplify A/D reference configuration
  staging: comedi: dt2811: remove private data
  staging: comedi: dt2811: simplify analog input range options
  staging: comedi: dt2811: tidy up analog input subdevice init
  staging: comedi: dt2811: remove unused define
  staging: comedi: dt2811: rename 'boardtypes'
  staging: comedi: dt2811: update the MODULE_DESCRIPTION

 drivers/staging/comedi/drivers/dt2811.c | 569 
 1 file changed, 220 insertions(+), 349 deletions(-)

-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 09/19] staging: comedi: dt2811: tidy up Timer/Counter register defines

[H Hartley Sweeten]

This register currently is not being used. For aesthetics, cleanup the
define and the comment about the frequency control.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 45 +
 1 file changed, 17 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 61aa323..6ea1ad4 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -76,6 +76,23 @@
 #define DT2811_DI_REG  0x06/* r   Digital Input Port 0 */
 #define DT2811_DO_REG  0x06/* w   Digital Output Port 1 */
 
+/*
+ * Timer frequency control:
+ *   DT2811_TMRCTR_MANTISSADT2811_TMRCTR_EXPONENT
+ *   val  divisor  frequency   val  multiply divisor/divide frequency by
+ *0  1  600 kHz 0   1
+ *1 10   60 kHz 1   10
+ *2  2  300 kHz 2   100
+ *3  3  200 kHz 3   1000
+ *4  4  150 kHz 4   1
+ *5  5  120 kHz 5   10
+ *6  6  100 kHz 6   100
+ *7 12   50 kHz 7   1000
+ */
+#define DT2811_TMRCTR_REG  0x07/* r/w  Timer/Counter */
+#define DT2811_TMRCTR_MANTISSA(x)  (((x) & 0x7) << 3)
+#define DT2811_TMRCTR_EXPONENT(x)  (((x) & 0x7) << 0)
+
 static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
4, {
UNI_RANGE(5),
@@ -130,36 +147,8 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
}
 };
 
-/*
-   0x07 TMRCTR (R/W) Timer/Counter Register
-   bits 6,7 - reserved
-   bits 5-3 - Timer frequency control (mantissa)
-   543  divisor  freqency (kHz)
-   000  1600
-   001  10   60
-   010  2300
-   011  3200
-   100  4150
-   101  5120
-   110  6100
-   111  12   50
-   bits 2-0 - Timer frequency control (exponent)
-   210  multiply divisor/divide frequency by
-   000  1
-   001  10
-   010  100
-   011  1000
-   100  1
-   101  10
-   110  100
-   111  1000
-
- */
-
 #define TIMEOUT 1
 
-#define DT2811_TMRCTR 7
-
 struct dt2811_board {
const char *name;
const struct comedi_lrange *bip_5;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 13/19] staging: comedi: dt2811: simplify A/D reference configuration

[H Hartley Sweeten]

The analog inputs are confgured with jumpers on the board to be:
  * 16 single-ended inputs
  * 8 differential inputs
  * 16 pseudo-differential inputs (common ground)

Simplify the handling of this configuration option and properly set
the subdev_flags based on the selected input mode.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 32 
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index b60ee74..1fe657c 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -29,10 +29,10 @@
  * Configuration options:
  *   [0] - I/O port base address
  *   [1] - IRQ, although this is currently unused
- *   [2] - A/D reference
- *0 = single-ended
- *1 = differential
- *2 = pseudo-differential (common reference)
+ *   [2] - A/D reference (# of analog inputs)
+ *0 = single-ended (16 channels)
+ *1 = differential (8 channels)
+ *2 = pseudo-differential (16 channels)
  *   [3] - A/D range
  *0 = [-5, 5]
  *1 = [-2.5, 2.5]
@@ -184,9 +184,6 @@ enum { card_2811_pgh, card_2811_pgl };
 struct dt2811_private {
int ntrig;
int curadchan;
-   enum {
-   adc_singleended, adc_diff, adc_pseudo_diff
-   } adc_mux;
 };
 
 static int dt2811_ai_eoc(struct comedi_device *dev,
@@ -293,26 +290,13 @@ static int dt2811_attach(struct comedi_device *dev, 
struct comedi_devconfig *it)
if (!devpriv)
return -ENOMEM;
 
-   switch (it->options[2]) {
-   case 0:
-   devpriv->adc_mux = adc_singleended;
-   break;
-   case 1:
-   devpriv->adc_mux = adc_diff;
-   break;
-   case 2:
-   devpriv->adc_mux = adc_pseudo_diff;
-   break;
-   default:
-   devpriv->adc_mux = adc_singleended;
-   break;
-   }
-
s = >subdevices[0];
/* initialize the ADC subdevice */
s->type = COMEDI_SUBD_AI;
-   s->subdev_flags = SDF_READABLE | SDF_GROUND;
-   s->n_chan = devpriv->adc_mux == adc_diff ? 8 : 16;
+   s->subdev_flags = SDF_READABLE |
+ (it->options[2] == 1) ? SDF_DIFF :
+ (it->options[2] == 2) ? SDF_COMMON : SDF_GROUND;
+   s->n_chan = (it->options[2] == 1) ? 8 : 16;
s->insn_read = dt2811_ai_insn;
s->maxdata = 0xfff;
switch (it->options[3]) {
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 02/19] staging: comedi: dt2811: remove redundant block comment

[H Hartley Sweeten]

The configuration options are listed in the comedi comment block.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 20 
 1 file changed, 20 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 499637b..5d550d1 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -297,26 +297,6 @@ static int dt2811_do_insn_bits(struct comedi_device *dev,
return insn->n;
 }
 
-/*
-  options[0]   Board base address
-  options[1]   IRQ
-  options[2]   Input configuration
-0 == single-ended
-1 == differential
-2 == pseudo-differential
-  options[3]   Analog input range configuration
-0 == bipolar 5  (-5V -- +5V)
-1 == bipolar 2.5V  (-2.5V -- +2.5V)
-2 == unipolar 5V  (0V -- +5V)
-  options[4]   Analog output 0 range configuration
-0 == bipolar 5  (-5V -- +5V)
-1 == bipolar 2.5V  (-2.5V -- +2.5V)
-2 == unipolar 5V  (0V -- +5V)
-  options[5]   Analog output 1 range configuration
-0 == bipolar 5  (-5V -- +5V)
-1 == bipolar 2.5V  (-2.5V -- +2.5V)
-2 == unipolar 5V  (0V -- +5V)
-*/
 static int dt2811_attach(struct comedi_device *dev, struct comedi_devconfig 
*it)
 {
/* int i; */
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 03/19] staging: comedi: dt2811: remove disabled code

[H Hartley Sweeten]

There is no reason the (*attach) should be trying to read an analog
input sample. Remove this disabled code.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 5d550d1..3cd9fe5 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -299,23 +299,15 @@ static int dt2811_do_insn_bits(struct comedi_device *dev,
 
 static int dt2811_attach(struct comedi_device *dev, struct comedi_devconfig 
*it)
 {
-   /* int i; */
const struct dt2811_board *board = dev->board_ptr;
struct dt2811_private *devpriv;
-   int ret;
struct comedi_subdevice *s;
+   int ret;
 
ret = comedi_request_region(dev, it->options[0], 0x8);
if (ret)
return ret;
 
-#if 0
-   outb(0, dev->iobase + DT2811_ADCSR);
-   udelay(100);
-   i = inb(dev->iobase + DT2811_ADDATLO);
-   i = inb(dev->iobase + DT2811_ADDATHI);
-#endif
-
ret = comedi_alloc_subdevices(dev, 4);
if (ret)
return ret;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v2 06/19] staging: comedi: dt2811: tidy up A/D Data register defines

[H Hartley Sweeten]

Cleanup the defines for these registers and and remove the redundant
information in the comment.

Tidy up the reading of the data registers in the (*insn_read).

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index f72d6c8..0c0b7b7 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -67,6 +67,9 @@
 #define DT2811_ADGCR_GAIN(x)   (((x) & 0x3) << 6)
 #define DT2811_ADGCR_CHAN(x)   (((x) & 0xf) << 0)
 
+#define DT2811_ADDATA_LO_REG   0x02/* r   A/D Data low byte */
+#define DT2811_ADDATA_HI_REG   0x03/* r   A/D Data high byte */
+
 static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
4, {
UNI_RANGE(5),
@@ -123,7 +126,7 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 /*
 
-   0x02,0x03 (R) ADDAT A/D Data Register
+   0x02,0x03
(W) DADAT0 D/A Data Register 0
0x02 low byte
0x03 high byte
@@ -160,8 +163,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 #define TIMEOUT 1
 
-#define DT2811_ADDATLO 2
-#define DT2811_ADDATHI 3
 #define DT2811_DADAT0LO 2
 #define DT2811_DADAT0HI 3
 #define DT2811_DADAT1LO 4
@@ -218,6 +219,8 @@ static int dt2811_ai_insn(struct comedi_device *dev, struct 
comedi_subdevice *s,
int i;
 
for (i = 0; i < insn->n; i++) {
+   unsigned int val;
+
/* select channel/gain and trigger conversion */
outb(DT2811_ADGCR_CHAN(chan) | DT2811_ADGCR_GAIN(range),
 dev->iobase + DT2811_ADGCR_REG);
@@ -226,9 +229,11 @@ static int dt2811_ai_insn(struct comedi_device *dev, 
struct comedi_subdevice *s,
if (ret)
return ret;
 
-   data[i] = inb(dev->iobase + DT2811_ADDATLO);
-   data[i] |= inb(dev->iobase + DT2811_ADDATHI) << 8;
-   data[i] &= 0xfff;
+   val = inb(dev->iobase + DT2811_ADDATA_LO_REG) |
+ (inb(dev->iobase + DT2811_ADDATA_HI_REG) << 8);
+   val &= s->maxdata;
+
+   data[i] = val;
}
 
return i;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

RE: [PATCH 15/19] staging: comedi: dt2811: simplify analog input range options

[Hartley Sweeten]

On Wednesday, May 04, 2016 9:01 AM, Ian Abbott wrote:
> On 04/05/16 01:25, H Hartley Sweeten wrote:
>> The A/D ranges are not programmable but the gain is. Currently this driver
>> uses a configuration option to select the comedi_lrange that will be used
>> for the analog input subdevice. This requires that the user makes sure the
>> correct option value is used.
>>
>> The user space library uses the range information to convert between raw
>> values and physical units. If the user passed an incorrect option when
>> attaching the driver the conversion will be incorrect.
>>
>> A previous patch allowed the gain to be set based on the chanspec range.
>> Prior to that the gain was always set to 1 so any conversion with a gain
>> that is not 1 would be incorrect anyway.
>>
>> Simplify the analog input ranges by providing a range_table for the pgh and
>> pgl boards that contain all the possible range/gain options. The user can
>> then select the correct range (and gain) when converting values or reading
>> the analog inputs.
>>
>> Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
>> Cc: Ian Abbott <abbo...@mev.co.uk>
>> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>

[snip]

> I think it will be better to put the UNI_RANGE entries at the bottom. 
> Then, if the user previously defaulted option[3] to 0, the first four 
> ranges in the table would be the same as they were before (not that the 
> ranges worked properly before anyway!).

The manual I found for the dt2811 (actually it's for a PC-74 board that is
register compatible), lists the "default factory" settings for the D/A jumpers
but not for the A/D jumpers so I created the combined table in the order
the manual describes the input ranges. But changing it to match how the
configuration options were listed is not a problem.

I'll fix this up and repost the series shortly.

Thanks,
Hartley

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 00/19] staging: comedi: dt2811: cleanup driver

[H Hartley Sweeten]

Fix all the checkpatch.pl issues and tidy up the driver.

H Hartley Sweeten (19):
  staging: comedi: dt2811: tidy up copyright and comedi comments
  staging: comedi: dt2811: remove redundant block comment
  staging: comedi: dt2811: remove disabled code
  staging: comedi: dt2811: tidy up A/D Control/Status register defines
  staging: comedi: dt2811: tidy up A/D Gain/Channel register defines
  staging: comedi: dt2811: tidy up A/D Data register defines
  staging: comedi: dt2811: tidy up D/A Data register defines
  staging: comedi: dt2811: tidy up Digital Input/Output register defines
  staging: comedi: dt2811: tidy up Timer/Counter register defines
  staging: comedi: dt2811: tidy up the digital subdevices
  staging: comedi: dt2811: simplify analog output range options
  staging: comedi: dt2811: tidy up analog output subdevice init
  staging: comedi: dt2811: simplify A/D reference configuration
  staging: comedi: dt2811: remove private data
  staging: comedi: dt2811: simplify analog input range options
  staging: comedi: dt2811: tidy up analog input subdevice init
  staging: comedi: dt2811: remove unused define
  staging: comedi: dt2811: rename 'boardtypes'
  staging: comedi: dt2811: update the MODULE_DESCRIPTION

 drivers/staging/comedi/drivers/dt2811.c | 569 
 1 file changed, 220 insertions(+), 349 deletions(-)

-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 05/19] staging: comedi: dt2811: tidy up A/D Gain/Channel register defines

[H Hartley Sweeten]

Cleanup the defines for this register and its bits and remove the
redundant information in the comment.

Make the (*insn_read) use the range to set the gain bits correctly.
Currently the gain is always set to 1.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 22 +-
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 433cfd1..f72d6c8 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -63,6 +63,10 @@
 /* continuous conversion, external clock, external trigger */
 #define DT2811_ADCSR_ADMODE_EXTDT2811_ADCSR_ADMODE(3)
 
+#define DT2811_ADGCR_REG   0x01/* r/w  A/D Gain/Channel */
+#define DT2811_ADGCR_GAIN(x)   (((x) & 0x3) << 6)
+#define DT2811_ADGCR_CHAN(x)   (((x) & 0xf) << 0)
+
 static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
4, {
UNI_RANGE(5),
@@ -119,16 +123,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 /*
 
-   0x01ADGCR R/W A/D Gain/Channel Register
-   bit 6,7 - (R/W) gain select
-   00  gain=1, both PGH, PGL models
-   01  gain=2 PGH, 10 PGL
-   10  gain=4 PGH, 100 PGL
-   11  gain=8 PGH, 500 PGL
-   bit 4,5 - reserved
-   bit 3-0 - (R/W) channel select
-   channel number from 0-15
-
0x02,0x03 (R) ADDAT A/D Data Register
(W) DADAT0 D/A Data Register 0
0x02 low byte
@@ -166,7 +160,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 #define TIMEOUT 1
 
-#define DT2811_ADGCR 1
 #define DT2811_ADDATLO 2
 #define DT2811_ADDATHI 3
 #define DT2811_DADAT0LO 2
@@ -219,12 +212,15 @@ static int dt2811_ai_eoc(struct comedi_device *dev,
 static int dt2811_ai_insn(struct comedi_device *dev, struct comedi_subdevice 
*s,
  struct comedi_insn *insn, unsigned int *data)
 {
-   int chan = CR_CHAN(insn->chanspec);
+   unsigned int chan = CR_CHAN(insn->chanspec);
+   unsigned int range = CR_RANGE(insn->chanspec);
int ret;
int i;
 
for (i = 0; i < insn->n; i++) {
-   outb(chan, dev->iobase + DT2811_ADGCR);
+   /* select channel/gain and trigger conversion */
+   outb(DT2811_ADGCR_CHAN(chan) | DT2811_ADGCR_GAIN(range),
+dev->iobase + DT2811_ADGCR_REG);
 
ret = comedi_timeout(dev, s, insn, dt2811_ai_eoc, 0);
if (ret)
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 19/19] staging: comedi: dt2811: update the MODULE_DESCRIPTION

[H Hartley Sweeten]

Change the MODULE_DESCRIPTION to something more useful than the generic
"Comedi low-level driver".

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index ca768dc..8f35493 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -341,5 +341,5 @@ static struct comedi_driver dt2811_driver = {
 module_comedi_driver(dt2811_driver);
 
 MODULE_AUTHOR("Comedi http://www.comedi.org;);
-MODULE_DESCRIPTION("Comedi low-level driver");
+MODULE_DESCRIPTION("Comedi driver for Data Translation DT2811 series boards");
 MODULE_LICENSE("GPL");
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 17/19] staging: comedi: dt2811: remove unused define

[H Hartley Sweeten]

This define is not used by the driver. Remove it.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 9a74964..6beebc2 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -168,8 +168,6 @@ static const struct comedi_lrange dt2811_ao_ranges = {
}
 };
 
-#define TIMEOUT 1
-
 struct dt2811_board {
const char *name;
unsigned int is_pgh:1;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 10/19] staging: comedi: dt2811: tidy up the digital subdevices

[H Hartley Sweeten]

Add some whitespace to the digital input and output subdevice
initialization. Reorder the initialization a bit.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 34 -
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 6ea1ad4..0512d70 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -240,7 +240,8 @@ static int dt2811_ao_insn_write(struct comedi_device *dev,
 
 static int dt2811_di_insn_bits(struct comedi_device *dev,
   struct comedi_subdevice *s,
-  struct comedi_insn *insn, unsigned int *data)
+  struct comedi_insn *insn,
+  unsigned int *data)
 {
data[1] = inb(dev->iobase + DT2811_DI_REG);
 
@@ -357,24 +358,23 @@ static int dt2811_attach(struct comedi_device *dev, 
struct comedi_devconfig *it)
if (ret)
return ret;
 
+   /* Digital Input subdevice */
s = >subdevices[2];
-   /* di subdevice */
-   s->type = COMEDI_SUBD_DI;
-   s->subdev_flags = SDF_READABLE;
-   s->n_chan = 8;
-   s->insn_bits = dt2811_di_insn_bits;
-   s->maxdata = 1;
-   s->range_table = _digital;
-
+   s->type = COMEDI_SUBD_DI;
+   s->subdev_flags = SDF_READABLE;
+   s->n_chan   = 8;
+   s->maxdata  = 1;
+   s->range_table  = _digital;
+   s->insn_bits= dt2811_di_insn_bits;
+
+   /* Digital Output subdevice */
s = >subdevices[3];
-   /* do subdevice */
-   s->type = COMEDI_SUBD_DO;
-   s->subdev_flags = SDF_WRITABLE;
-   s->n_chan = 8;
-   s->insn_bits = dt2811_do_insn_bits;
-   s->maxdata = 1;
-   s->state = 0;
-   s->range_table = _digital;
+   s->type = COMEDI_SUBD_DO;
+   s->subdev_flags = SDF_WRITABLE;
+   s->n_chan   = 8;
+   s->maxdata  = 1;
+   s->range_table  = _digital;
+   s->insn_bits= dt2811_do_insn_bits;
 
return 0;
 }
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 04/19] staging: comedi: dt2811: tidy up A/D Control/Status register defines

[H Hartley Sweeten]

Cleanup the defines for this register and its bits and remove the
redundant information in the comment.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 59 -
 1 file changed, 21 insertions(+), 38 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 3cd9fe5..433cfd1 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -44,6 +44,25 @@
 #include 
 #include "../comedidev.h"
 
+/*
+ * Register I/O map
+ */
+#define DT2811_ADCSR_REG   0x00/* r/w  A/D Control/Status */
+#define DT2811_ADCSR_ADDONEBIT(7)  /* r  1=A/D conv done */
+#define DT2811_ADCSR_ADERROR   BIT(6)  /* r  1=A/D error */
+#define DT2811_ADCSR_ADBUSYBIT(5)  /* r  1=A/D busy */
+#define DT2811_ADCSR_CLRERROR  BIT(4)
+#define DT2811_ADCSR_INTENBBIT(2)  /* r/w1=interupts ena */
+#define DT2811_ADCSR_ADMODE(x) (((x) & 0x3) << 0)
+/* single conversion on ADGCR load */
+#define DT2811_ADCSR_ADMODE_SINGLE DT2811_ADCSR_ADMODE(0)
+/* continuous conversion, internal clock, (clock enabled on ADGCR load) */
+#define DT2811_ADCSR_ADMODE_CONT   DT2811_ADCSR_ADMODE(1)
+/* continuous conversion, internal clock, external trigger */
+#define DT2811_ADCSR_ADMODE_EXT_TRIG   DT2811_ADCSR_ADMODE(2)
+/* continuous conversion, external clock, external trigger */
+#define DT2811_ADCSR_ADMODE_EXTDT2811_ADCSR_ADMODE(3)
+
 static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
4, {
UNI_RANGE(5),
@@ -100,28 +119,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 /*
 
-   0x00ADCSR R/W  A/D Control/Status Register
-   bit 7 - (R) 1 indicates A/D conversion done
-   reading ADDAT clears bit
-   (W) ignored
-   bit 6 - (R) 1 indicates A/D error
-   (W) ignored
-   bit 5 - (R) 1 indicates A/D busy, cleared at end
-   of conversion
-   (W) ignored
-   bit 4 - (R) 0
-   (W)
-   bit 3 - (R) 0
-   bit 2 - (R/W) 1 indicates interrupts enabled
-   bits 1,0 - (R/W) mode bits
-   00  single conversion on ADGCR load
-   01  continuous conversion, internal clock,
-   (clock enabled on ADGCR load)
-   10  continuous conversion, internal clock,
-   external trigger
-   11  continuous conversion, external clock,
-   external trigger
-
0x01ADGCR R/W A/D Gain/Channel Register
bit 6,7 - (R/W) gain select
00  gain=1, both PGH, PGL models
@@ -169,7 +166,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 #define TIMEOUT 1
 
-#define DT2811_ADCSR 0
 #define DT2811_ADGCR 1
 #define DT2811_ADDATLO 2
 #define DT2811_ADDATHI 3
@@ -180,19 +176,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 #define DT2811_DIO 6
 #define DT2811_TMRCTR 7
 
-/*
- * flags
- */
-
-/* ADCSR */
-
-#define DT2811_ADDONE   0x80
-#define DT2811_ADERROR  0x40
-#define DT2811_ADBUSY   0x20
-#define DT2811_CLRERROR 0x10
-#define DT2811_INTENB   0x04
-#define DT2811_ADMODE   0x03
-
 struct dt2811_board {
const char *name;
const struct comedi_lrange *bip_5;
@@ -227,8 +210,8 @@ static int dt2811_ai_eoc(struct comedi_device *dev,
 {
unsigned int status;
 
-   status = inb(dev->iobase + DT2811_ADCSR);
-   if ((status & DT2811_ADBUSY) == 0)
+   status = inb(dev->iobase + DT2811_ADCSR_REG);
+   if ((status & DT2811_ADCSR_ADBUSY) == 0)
return 0;
return -EBUSY;
 }
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 11/19] staging: comedi: dt2811: simplify analog output range options

[H Hartley Sweeten]

The D/A ranges are not programmable. Currently this driver uses some
configuration options to select the comedi_lrange for each channel.
This is a bit messy and it requires the user to make sure the correct
option value is used.

The range information isn't used by the driver. The user space library
uses it to convert between raw data values and physical units. If the
user passed an incorrect option when attaching the driver the conversion
will be incorrect.

Simplify the analog output ranges by providing a range_table that
contains all the possible output ranges. The user can then select the
correct range when converting values.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 69 +
 1 file changed, 26 insertions(+), 43 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 0512d70..6850eb8 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -37,8 +37,14 @@
  *0 = [-5, 5]
  *1 = [-2.5, 2.5]
  *2 = [0, 5]
- *   [4] - D/A 0 range (same as A/D range)
- *   [5] - D/A 1 range (same as A/D range)
+ *   [4] - D/A 0 range (deprecated, see below)
+ *   [5] - D/A 1 range (deprecated, see below)
+ *
+ * Notes:
+ *   - D/A ranges are not programmable. The AO subdevice has a range_table
+ * containing all the possible analog output ranges. Use the range
+ * that matches your board configuration to convert between data
+ * values and physical units.
  */
 
 #include 
@@ -147,6 +153,23 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
}
 };
 
+/*
+ * The Analog Output range is set per-channel using jumpers on the board.
+ *
+ * DAC0 JumpersDAC1 Jumpers
+ * Output RangeW5  W6  W7  W8  W1  W2  W3  W4
+ * -5V to +5V  In  Out In  Out In  Out In  Out
+ * -2.5V to +2.5V  In  Out Out In  In  Out Out In
+ * 0 to +5VOut In  Out In  Out In  Out In
+ */
+static const struct comedi_lrange dt2811_ao_ranges = {
+   3, {
+   BIP_RANGE(5),   /* default setting from factory */
+   BIP_RANGE(2.5),
+   UNI_RANGE(5)
+   }
+};
+
 #define TIMEOUT 1
 
 struct dt2811_board {
@@ -164,16 +187,6 @@ struct dt2811_private {
enum {
adc_singleended, adc_diff, adc_pseudo_diff
} adc_mux;
-   enum {
-   dac_bipolar_5, dac_bipolar_2_5, dac_unipolar_5
-   } dac_range[2];
-   const struct comedi_lrange *range_type_list[2];
-};
-
-static const struct comedi_lrange *dac_range_types[] = {
-   _bipolar5,
-   _bipolar2_5,
-   _unipolar5
 };
 
 static int dt2811_ai_eoc(struct comedi_device *dev,
@@ -294,34 +307,6 @@ static int dt2811_attach(struct comedi_device *dev, struct 
comedi_devconfig *it)
devpriv->adc_mux = adc_singleended;
break;
}
-   switch (it->options[4]) {
-   case 0:
-   devpriv->dac_range[0] = dac_bipolar_5;
-   break;
-   case 1:
-   devpriv->dac_range[0] = dac_bipolar_2_5;
-   break;
-   case 2:
-   devpriv->dac_range[0] = dac_unipolar_5;
-   break;
-   default:
-   devpriv->dac_range[0] = dac_bipolar_5;
-   break;
-   }
-   switch (it->options[5]) {
-   case 0:
-   devpriv->dac_range[1] = dac_bipolar_5;
-   break;
-   case 1:
-   devpriv->dac_range[1] = dac_bipolar_2_5;
-   break;
-   case 2:
-   devpriv->dac_range[1] = dac_unipolar_5;
-   break;
-   default:
-   devpriv->dac_range[1] = dac_bipolar_5;
-   break;
-   }
 
s = >subdevices[0];
/* initialize the ADC subdevice */
@@ -349,9 +334,7 @@ static int dt2811_attach(struct comedi_device *dev, struct 
comedi_devconfig *it)
s->subdev_flags = SDF_WRITABLE;
s->n_chan = 2;
s->maxdata = 0xfff;
-   s->range_table_list = devpriv->range_type_list;
-   devpriv->range_type_list[0] = dac_range_types[devpriv->dac_range[0]];
-   devpriv->range_type_list[1] = dac_range_types[devpriv->dac_range[1]];
+   s->range_table = _ao_ranges;
s->insn_write = dt2811_ao_insn_write;
 
ret = comedi_alloc_subdev_readback(s);
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 14/19] staging: comedi: dt2811: remove private data

[H Hartley Sweeten]

The remaining members of the private data are not used by the driver.
Remove it and the allocation.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 10 --
 1 file changed, 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 1fe657c..379b7d5 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -181,11 +181,6 @@ struct dt2811_board {
 
 enum { card_2811_pgh, card_2811_pgl };
 
-struct dt2811_private {
-   int ntrig;
-   int curadchan;
-};
-
 static int dt2811_ai_eoc(struct comedi_device *dev,
 struct comedi_subdevice *s,
 struct comedi_insn *insn,
@@ -274,7 +269,6 @@ static int dt2811_do_insn_bits(struct comedi_device *dev,
 static int dt2811_attach(struct comedi_device *dev, struct comedi_devconfig 
*it)
 {
const struct dt2811_board *board = dev->board_ptr;
-   struct dt2811_private *devpriv;
struct comedi_subdevice *s;
int ret;
 
@@ -286,10 +280,6 @@ static int dt2811_attach(struct comedi_device *dev, struct 
comedi_devconfig *it)
if (ret)
return ret;
 
-   devpriv = comedi_alloc_devpriv(dev, sizeof(*devpriv));
-   if (!devpriv)
-   return -ENOMEM;
-
s = >subdevices[0];
/* initialize the ADC subdevice */
s->type = COMEDI_SUBD_AI;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 01/19] staging: comedi: dt2811: tidy up copyright and comedi comments

[H Hartley Sweeten]

Fix the checkpatch.pl issue:
WARNING: Block comments use * on subsequent lines

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 78 -
 1 file changed, 39 insertions(+), 39 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index a807732..499637b 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -1,45 +1,45 @@
 /*
-   comedi/drivers/dt2811.c
-   Hardware driver for Data Translation DT2811
-
-   COMEDI - Linux Control and Measurement Device Interface
-   History:
-   Base Version  - David A. Schleef <d...@schleef.org>
-   December 1998 - Updated to work.  David does not have a DT2811
-   board any longer so this was suffering from bitrot.
-   Updated performed by ...
-
-   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 driver for Data Translation DT2811
+ *
+ * COMEDI - Linux Control and Measurement Device Interface
+ * History:
+ * Base Version  - David A. Schleef <d...@schleef.org>
+ * December 1998 - Updated to work.  David does not have a DT2811
+ * board any longer so this was suffering from bitrot.
+ * Updated performed by ...
+ *
+ * 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: dt2811
-Description: Data Translation DT2811
-Author: ds
-Devices: [Data Translation] DT2811-PGL (dt2811-pgl), DT2811-PGH (dt2811-pgh)
-Status: works
-
-Configuration options:
-  [0] - I/O port base address
-  [1] - IRQ, although this is currently unused
-  [2] - A/D reference
- 0 = signle-ended
- 1 = differential
- 2 = pseudo-differential (common reference)
-  [3] - A/D range
- 0 = [-5, 5]
- 1 = [-2.5, 2.5]
- 2 = [0, 5]
-  [4] - D/A 0 range (same choices)
-  [4] - D/A 1 range (same choices)
-*/
+ * Driver: dt2811
+ * Description: Data Translation DT2811
+ * Author: ds
+ * Devices: [Data Translation] DT2811-PGL (dt2811-pgl), DT2811-PGH (dt2811-pgh)
+ * Status: works
+ *
+ * Configuration options:
+ *   [0] - I/O port base address
+ *   [1] - IRQ, although this is currently unused
+ *   [2] - A/D reference
+ *0 = single-ended
+ *1 = differential
+ *2 = pseudo-differential (common reference)
+ *   [3] - A/D range
+ *0 = [-5, 5]
+ *1 = [-2.5, 2.5]
+ *2 = [0, 5]
+ *   [4] - D/A 0 range (same as A/D range)
+ *   [5] - D/A 1 range (same as A/D range)
+ */
 
 #include 
 #include "../comedidev.h"
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 09/19] staging: comedi: dt2811: tidy up Timer/Counter register defines

[H Hartley Sweeten]

This register currently is not being used. For aesthetics, cleanup the
define and the comment about the frequency control.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 45 +
 1 file changed, 17 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 61aa323..6ea1ad4 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -76,6 +76,23 @@
 #define DT2811_DI_REG  0x06/* r   Digital Input Port 0 */
 #define DT2811_DO_REG  0x06/* w   Digital Output Port 1 */
 
+/*
+ * Timer frequency control:
+ *   DT2811_TMRCTR_MANTISSADT2811_TMRCTR_EXPONENT
+ *   val  divisor  frequency   val  multiply divisor/divide frequency by
+ *0  1  600 kHz 0   1
+ *1 10   60 kHz 1   10
+ *2  2  300 kHz 2   100
+ *3  3  200 kHz 3   1000
+ *4  4  150 kHz 4   1
+ *5  5  120 kHz 5   10
+ *6  6  100 kHz 6   100
+ *7 12   50 kHz 7   1000
+ */
+#define DT2811_TMRCTR_REG  0x07/* r/w  Timer/Counter */
+#define DT2811_TMRCTR_MANTISSA(x)  (((x) & 0x7) << 3)
+#define DT2811_TMRCTR_EXPONENT(x)  (((x) & 0x7) << 0)
+
 static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
4, {
UNI_RANGE(5),
@@ -130,36 +147,8 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
}
 };
 
-/*
-   0x07 TMRCTR (R/W) Timer/Counter Register
-   bits 6,7 - reserved
-   bits 5-3 - Timer frequency control (mantissa)
-   543  divisor  freqency (kHz)
-   000  1600
-   001  10   60
-   010  2300
-   011  3200
-   100  4150
-   101  5120
-   110  6100
-   111  12   50
-   bits 2-0 - Timer frequency control (exponent)
-   210  multiply divisor/divide frequency by
-   000  1
-   001  10
-   010  100
-   011  1000
-   100  1
-   101  10
-   110  100
-   111  1000
-
- */
-
 #define TIMEOUT 1
 
-#define DT2811_TMRCTR 7
-
 struct dt2811_board {
const char *name;
const struct comedi_lrange *bip_5;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 16/19] staging: comedi: dt2811: tidy up analog input subdevice init

[H Hartley Sweeten]

Add some whitespace to the analog output subdevice initialization
and rename the (*insn_read) function.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index acc9db8..9a74964 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -197,8 +197,10 @@ static int dt2811_ai_eoc(struct comedi_device *dev,
return -EBUSY;
 }
 
-static int dt2811_ai_insn(struct comedi_device *dev, struct comedi_subdevice 
*s,
- struct comedi_insn *insn, unsigned int *data)
+static int dt2811_ai_insn_read(struct comedi_device *dev,
+  struct comedi_subdevice *s,
+  struct comedi_insn *insn,
+  unsigned int *data)
 {
unsigned int chan = CR_CHAN(insn->chanspec);
unsigned int range = CR_RANGE(insn->chanspec);
@@ -283,17 +285,17 @@ static int dt2811_attach(struct comedi_device *dev, 
struct comedi_devconfig *it)
if (ret)
return ret;
 
+   /* Analog Input subdevice */
s = >subdevices[0];
-   /* initialize the ADC subdevice */
-   s->type = COMEDI_SUBD_AI;
-   s->subdev_flags = SDF_READABLE |
+   s->type = COMEDI_SUBD_AI;
+   s->subdev_flags = SDF_READABLE |
  (it->options[2] == 1) ? SDF_DIFF :
  (it->options[2] == 2) ? SDF_COMMON : SDF_GROUND;
-   s->n_chan = (it->options[2] == 1) ? 8 : 16;
-   s->insn_read = dt2811_ai_insn;
-   s->maxdata = 0xfff;
+   s->n_chan   = (it->options[2] == 1) ? 8 : 16;
+   s->maxdata  = 0x0fff;
s->range_table  = board->is_pgh ? _pgh_ai_ranges
: _pgl_ai_ranges;
+   s->insn_read= dt2811_ai_insn_read;
 
/* Analog Output subdevice */
s = >subdevices[1];
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 02/19] staging: comedi: dt2811: remove redundant block comment

[H Hartley Sweeten]

The configuration options are listed in the comedi comment block.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 20 
 1 file changed, 20 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 499637b..5d550d1 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -297,26 +297,6 @@ static int dt2811_do_insn_bits(struct comedi_device *dev,
return insn->n;
 }
 
-/*
-  options[0]   Board base address
-  options[1]   IRQ
-  options[2]   Input configuration
-0 == single-ended
-1 == differential
-2 == pseudo-differential
-  options[3]   Analog input range configuration
-0 == bipolar 5  (-5V -- +5V)
-1 == bipolar 2.5V  (-2.5V -- +2.5V)
-2 == unipolar 5V  (0V -- +5V)
-  options[4]   Analog output 0 range configuration
-0 == bipolar 5  (-5V -- +5V)
-1 == bipolar 2.5V  (-2.5V -- +2.5V)
-2 == unipolar 5V  (0V -- +5V)
-  options[5]   Analog output 1 range configuration
-0 == bipolar 5  (-5V -- +5V)
-1 == bipolar 2.5V  (-2.5V -- +2.5V)
-2 == unipolar 5V  (0V -- +5V)
-*/
 static int dt2811_attach(struct comedi_device *dev, struct comedi_devconfig 
*it)
 {
/* int i; */
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 07/19] staging: comedi: dt2811: tidy up D/A Data register defines

[H Hartley Sweeten]

Cleanup the defines for these registers and and remove the redundant
information in the comment.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 19 +--
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 0c0b7b7..1717281 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -70,6 +70,9 @@
 #define DT2811_ADDATA_LO_REG   0x02/* r   A/D Data low byte */
 #define DT2811_ADDATA_HI_REG   0x03/* r   A/D Data high byte */
 
+#define DT2811_DADATA_LO_REG(x)(0x02 + ((x) * 2)) /* w D/A 
Data low */
+#define DT2811_DADATA_HI_REG(x)(0x03 + ((x) * 2)) /* w D/A 
Data high */
+
 static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
4, {
UNI_RANGE(5),
@@ -125,14 +128,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 };
 
 /*
-
-   0x02,0x03
-   (W) DADAT0 D/A Data Register 0
-   0x02 low byte
-   0x03 high byte
-
-   0x04,0x05 (W) DADAT0 D/A Data Register 1
-
0x06 (R) DIO0 Digital Input Port 0
(W) DIO1 Digital Output Port 1
 
@@ -163,10 +158,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 #define TIMEOUT 1
 
-#define DT2811_DADAT0LO 2
-#define DT2811_DADAT0HI 3
-#define DT2811_DADAT1LO 4
-#define DT2811_DADAT1HI 5
 #define DT2811_DIO 6
 #define DT2811_TMRCTR 7
 
@@ -250,9 +241,9 @@ static int dt2811_ao_insn_write(struct comedi_device *dev,
 
for (i = 0; i < insn->n; i++) {
val = data[i];
-   outb(val & 0xff, dev->iobase + DT2811_DADAT0LO + 2 * chan);
+   outb(val & 0xff, dev->iobase + DT2811_DADATA_LO_REG(chan));
outb((val >> 8) & 0xff,
-dev->iobase + DT2811_DADAT0HI + 2 * chan);
+dev->iobase + DT2811_DADATA_HI_REG(chan));
}
s->readback[chan] = val;
 
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 15/19] staging: comedi: dt2811: simplify analog input range options

[H Hartley Sweeten]

The A/D ranges are not programmable but the gain is. Currently this driver
uses a configuration option to select the comedi_lrange that will be used
for the analog input subdevice. This requires that the user makes sure the
correct option value is used.

The user space library uses the range information to convert between raw
values and physical units. If the user passed an incorrect option when
attaching the driver the conversion will be incorrect.

A previous patch allowed the gain to be set based on the chanspec range.
Prior to that the gain was always set to 1 so any conversion with a gain
that is not 1 would be incorrect anyway.

Simplify the analog input ranges by providing a range_table for the pgh and
pgl boards that contain all the possible range/gain options. The user can
then select the correct range (and gain) when converting values or reading
the analog inputs.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 143 ++--
 1 file changed, 61 insertions(+), 82 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 379b7d5..acc9db8 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -33,14 +33,17 @@
  *0 = single-ended (16 channels)
  *1 = differential (8 channels)
  *2 = pseudo-differential (16 channels)
- *   [3] - A/D range
- *0 = [-5, 5]
- *1 = [-2.5, 2.5]
- *2 = [0, 5]
+ *   [3] - A/D range (deprecated, see below)
  *   [4] - D/A 0 range (deprecated, see below)
  *   [5] - D/A 1 range (deprecated, see below)
  *
  * Notes:
+ *   - A/D ranges are not programmable but the gain is. The AI subdevice has
+ * a range_table containing all the possible analog input range/gain
+ * options for the dt2811-pgh or dt2811-pgl. Use the range that matches
+ * your board configuration and the desired gain to correctly convert
+ * between data values and physical units and to set the correct output
+ * gain.
  *   - D/A ranges are not programmable. The AO subdevice has a range_table
  * containing all the possible analog output ranges. Use the range
  * that matches your board configuration to convert between data
@@ -99,57 +102,52 @@
 #define DT2811_TMRCTR_MANTISSA(x)  (((x) & 0x7) << 3)
 #define DT2811_TMRCTR_EXPONENT(x)  (((x) & 0x7) << 0)
 
-static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
-   4, {
-   UNI_RANGE(5),
-   UNI_RANGE(2.5),
-   UNI_RANGE(1.25),
-   UNI_RANGE(0.625)
-   }
-};
-
-static const struct comedi_lrange range_dt2811_pgh_ai_2_5_bipolar = {
-   4, {
-   BIP_RANGE(2.5),
-   BIP_RANGE(1.25),
-   BIP_RANGE(0.625),
-   BIP_RANGE(0.3125)
-   }
-};
-
-static const struct comedi_lrange range_dt2811_pgh_ai_5_bipolar = {
-   4, {
-   BIP_RANGE(5),
-   BIP_RANGE(2.5),
-   BIP_RANGE(1.25),
-   BIP_RANGE(0.625)
-   }
-};
-
-static const struct comedi_lrange range_dt2811_pgl_ai_5_unipolar = {
-   4, {
-   UNI_RANGE(5),
-   UNI_RANGE(0.5),
-   UNI_RANGE(0.05),
-   UNI_RANGE(0.01)
-   }
-};
-
-static const struct comedi_lrange range_dt2811_pgl_ai_2_5_bipolar = {
-   4, {
-   BIP_RANGE(2.5),
-   BIP_RANGE(0.25),
-   BIP_RANGE(0.025),
-   BIP_RANGE(0.005)
+/*
+ * The Analog Input range is set using jumpers on the board.
+ *
+ * Input Range W9  W10
+ * 0V to +5V   Out In
+ * -5V to +5V  In  Out
+ * -2.5V to +2.5V  In  In
+ *
+ * The gain may be set to 1, 2, 4, or 8 (on the dt2811-pgh) or to
+ * 1, 10, 100, 500 (on the dt2811-pgl).
+ */
+static const struct comedi_lrange dt2811_pgh_ai_ranges = {
+   12, {
+   UNI_RANGE(5),   /* range 0: gain=1 */
+   UNI_RANGE(2.5), /* range 1: gain=2 */
+   UNI_RANGE(1.25),/* range 2: gain=4 */
+   UNI_RANGE(0.625),   /* range 3: gain=8 */
+
+   BIP_RANGE(5),   /* range 0+4: gain=1 */
+   BIP_RANGE(2.5), /* range 1+4: gain=2 */
+   BIP_RANGE(1.25),/* range 2+4: gain=4 */
+   BIP_RANGE(0.625),   /* range 3+4: gain=8 */
+
+   BIP_RANGE(2.5), /* range 0+8: gain=1 */
+   BIP_RANGE(1.25),/* range 1+8: gain=2 */
+   BIP_RANGE(0.625),   /* range 2+8: gain=4 */
+   BIP_RANGE(0.3125)   /* range 3+8: gain=8 */
}
 };
 
-static const struct comedi_lrange range_dt2811_pgl_ai_5_bipolar = {
-   4, {
-   BIP_R

[PATCH 08/19] staging: comedi: dt2811: tidy up Digital Input/Output register defines

[H Hartley Sweeten]

The digital input and outputs are separate ports even though they share the
same register offset. For aesthetics, define then separately and remove the
redundant information in the comment.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/dt2811.c | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/dt2811.c 
b/drivers/staging/comedi/drivers/dt2811.c
index 1717281..61aa323 100644
--- a/drivers/staging/comedi/drivers/dt2811.c
+++ b/drivers/staging/comedi/drivers/dt2811.c
@@ -73,6 +73,9 @@
 #define DT2811_DADATA_LO_REG(x)(0x02 + ((x) * 2)) /* w D/A 
Data low */
 #define DT2811_DADATA_HI_REG(x)(0x03 + ((x) * 2)) /* w D/A 
Data high */
 
+#define DT2811_DI_REG  0x06/* r   Digital Input Port 0 */
+#define DT2811_DO_REG  0x06/* w   Digital Output Port 1 */
+
 static const struct comedi_lrange range_dt2811_pgh_ai_5_unipolar = {
4, {
UNI_RANGE(5),
@@ -128,9 +131,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 };
 
 /*
-   0x06 (R) DIO0 Digital Input Port 0
-   (W) DIO1 Digital Output Port 1
-
0x07 TMRCTR (R/W) Timer/Counter Register
bits 6,7 - reserved
bits 5-3 - Timer frequency control (mantissa)
@@ -158,7 +158,6 @@ static const struct comedi_lrange 
range_dt2811_pgl_ai_5_bipolar = {
 
 #define TIMEOUT 1
 
-#define DT2811_DIO 6
 #define DT2811_TMRCTR 7
 
 struct dt2811_board {
@@ -254,7 +253,7 @@ static int dt2811_di_insn_bits(struct comedi_device *dev,
   struct comedi_subdevice *s,
   struct comedi_insn *insn, unsigned int *data)
 {
-   data[1] = inb(dev->iobase + DT2811_DIO);
+   data[1] = inb(dev->iobase + DT2811_DI_REG);
 
return insn->n;
 }
@@ -265,7 +264,7 @@ static int dt2811_do_insn_bits(struct comedi_device *dev,
   unsigned int *data)
 {
if (comedi_dio_update_state(s, data))
-   outb(s->state, dev->iobase + DT2811_DIO);
+   outb(s->state, dev->iobase + DT2811_DO_REG);
 
data[1] = s->state;
 
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 19/25] staging: comedi: das16m1: tidy up misc. defines

[H Hartley Sweeten]

For aesthetics, move these after the register map defines and rename
the FIFO_SIZE define.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index 824cc94..6797da1 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -56,10 +56,6 @@
 #include "8255.h"
 #include "comedi_8254.h"
 
-#define DAS16M1_SIZE2 8
-
-#define FIFO_SIZE 1024 /*  1024 sample fifo */
-
 /*
  * Register map (dev->iobase)
  */
@@ -89,6 +85,10 @@
 #define DAS16M1_8255_IOBASE0x400
 #define DAS16M1_8254_IOBASE3   0x404
 
+#define DAS16M1_SIZE2  0x08
+
+#define DAS16M1_AI_FIFO_SZ 1024/* # samples */
+
 static const struct comedi_lrange range_das16m1 = {
9, {
BIP_RANGE(5),
@@ -108,7 +108,7 @@ struct das16m1_private_struct {
unsigned int intr_ctrl;
unsigned int adc_count;
u16 initial_hw_count;
-   unsigned short ai_buffer[FIFO_SIZE];
+   unsigned short ai_buffer[DAS16M1_AI_FIFO_SZ];
unsigned long extra_iobase;
 };
 
@@ -411,8 +411,8 @@ static void das16m1_handler(struct comedi_device *dev, 
unsigned int status)
num_samples = cmd->stop_arg * cmd->chanlist_len;
}
/*  make sure we dont try to get too many points if fifo has overrun */
-   if (num_samples > FIFO_SIZE)
-   num_samples = FIFO_SIZE;
+   if (num_samples > DAS16M1_AI_FIFO_SZ)
+   num_samples = DAS16M1_AI_FIFO_SZ;
insw(dev->iobase, devpriv->ai_buffer, num_samples);
munge_sample_array(devpriv->ai_buffer, num_samples);
comedi_buf_write_samples(s, devpriv->ai_buffer, num_samples);
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 00/25] staging: comedi: das16m1: cleanup driver

[H Hartley Sweeten]

Fixe all the checkpatch.pl issues and tidy up the driver.

H Hartley Sweeten (25):
  staging: comedi: das16m1: tidy up copyright and comedi comments
  staging: comedi: das16m1: tidy up comments in das16m1_handler()
  staging: comedi: das16m1: tidy up comment in das16m1_cmd_exec()
  staging: comedi: das16m1: remove unnecessary private data comments
  staging: comedi: das16m1: tidy up register map comment
  staging: comedi: das16m1: tidy up analog input data register defines
  staging: comedi: das16m1: tidy up control/status register defines
  staging: comedi: das16m1: tidy up digital input/output register defines
  staging: comedi: das16m1: tidy up clear interrupt register define
  staging: comedi: das16m1: remove unnecessary ai 'cancel' operations
  staging: comedi: das16m1: tidy up interrupt control register defines
  staging: comedi: das16m1: tidy up queue register defines
  staging: comedi: das16m1: introduce das16m1_ai_set_queue()
  staging: comedi: das16m1: tidy up analog input subdevice init
  staging: comedi: das16m1: tidy up digital input subdevice init
  staging: comedi: das16m1: tidy up das16m1_di_insn_bits()
  staging: comedi: das16m1: tidy up digital output subdevice init
  staging: comedi: das16m1: tidy up 8254/8255 register defines
  staging: comedi: das16m1: tidy up misc. defines
  staging: comedi: das16m1: remove an unnecessery comment
  staging: comedi: das16m1: init local variables when declared
  staging: comedi: das16m1: minor cleanup to das16m1_ai_insn_read()
  staging: comedi: das16m1: convert munge_sample_array() into a subdevice 
(*munge)
  staging: comedi: das16m1: rename struct das16m1_private_struct
  staging: comedi: das16m1: update the MODULE_DESCRIPTION

 drivers/staging/comedi/drivers/das16m1.c | 482 +++
 1 file changed, 233 insertions(+), 249 deletions(-)

-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 15/25] staging: comedi: das16m1: tidy up digital input subdevice init

[H Hartley Sweeten]

Add some whitespace to the subdevice init and rename the support
function.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index b19715c..f51da80 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -347,9 +347,10 @@ static int das16m1_ai_insn_read(struct comedi_device *dev,
return n;
 }
 
-static int das16m1_di_rbits(struct comedi_device *dev,
-   struct comedi_subdevice *s,
-   struct comedi_insn *insn, unsigned int *data)
+static int das16m1_di_insn_bits(struct comedi_device *dev,
+   struct comedi_subdevice *s,
+   struct comedi_insn *insn,
+   unsigned int *data)
 {
unsigned int bits;
 
@@ -574,14 +575,14 @@ static int das16m1_attach(struct comedi_device *dev,
s->poll = das16m1_ai_poll;
}
 
+   /* Digital Input subdevice */
s = >subdevices[1];
-   /* di */
-   s->type = COMEDI_SUBD_DI;
-   s->subdev_flags = SDF_READABLE;
-   s->n_chan = 4;
-   s->maxdata = 1;
-   s->range_table = _digital;
-   s->insn_bits = das16m1_di_rbits;
+   s->type = COMEDI_SUBD_DI;
+   s->subdev_flags = SDF_READABLE;
+   s->n_chan   = 4;
+   s->maxdata  = 1;
+   s->range_table  = _digital;
+   s->insn_bits= das16m1_di_insn_bits;
 
s = >subdevices[2];
/* do */
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 09/25] staging: comedi: das16m1: tidy up clear interrupt register define

[H Hartley Sweeten]

Rename this define and use a consistent comment throughout the code.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index e85d989..dcec4e2 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -72,7 +72,7 @@
 #define DAS16M1_CS_IRQDATA BIT(7)
 #define DAS16M1_DI_REG 0x03
 #define DAS16M1_DO_REG 0x03
-#define DAS16M1_CLEAR_INTR 4
+#define DAS16M1_CLR_INTR_REG   0x04
 #define DAS16M1_INTR_CONTROL   5
 #define   EXT_PACER  0x2
 #define   INT_PACER  0x3
@@ -276,8 +276,9 @@ static int das16m1_cmd_exec(struct comedi_device *dev,
byte |= DAS16M1_CS_EXT_TRIG;
 
outb(byte, dev->iobase + DAS16M1_CS_REG);
-   /* clear interrupt bit */
-   outb(0, dev->iobase + DAS16M1_CLEAR_INTR);
+
+   /* clear interrupt */
+   outb(0, dev->iobase + DAS16M1_CLR_INTR_REG);
 
devpriv->control_state |= INTE;
outb(devpriv->control_state, dev->iobase + DAS16M1_INTR_CONTROL);
@@ -330,8 +331,8 @@ static int das16m1_ai_rinsn(struct comedi_device *dev,
for (n = 0; n < insn->n; n++) {
unsigned short val;
 
-   /* clear DAS16M1_CS_IRQDATA bit */
-   outb(0, dev->iobase + DAS16M1_CLEAR_INTR);
+   /* clear interrupt */
+   outb(0, dev->iobase + DAS16M1_CLR_INTR_REG);
/* trigger conversion */
outb(0, dev->iobase + DAS16M1_AI_REG);
 
@@ -476,7 +477,7 @@ static irqreturn_t das16m1_interrupt(int irq, void *d)
das16m1_handler(dev, status);
 
/* clear interrupt */
-   outb(0, dev->iobase + DAS16M1_CLEAR_INTR);
+   outb(0, dev->iobase + DAS16M1_CLR_INTR_REG);
 
spin_unlock(>spinlock);
return IRQ_HANDLED;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 21/25] staging: comedi: das16m1: init local variables when declared

[H Hartley Sweeten]

For aesthetics, init the comedi_subdevice, comedi_async, and comedi_cmd
pointers when the local variables are declared.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index 3a62f5a..d4b1b92 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -373,16 +373,12 @@ static int das16m1_do_insn_bits(struct comedi_device *dev,
 static void das16m1_handler(struct comedi_device *dev, unsigned int status)
 {
struct das16m1_private_struct *devpriv = dev->private;
-   struct comedi_subdevice *s;
-   struct comedi_async *async;
-   struct comedi_cmd *cmd;
+   struct comedi_subdevice *s = dev->read_subdev;
+   struct comedi_async *async = s->async;
+   struct comedi_cmd *cmd = >cmd;
u16 num_samples;
u16 hw_counter;
 
-   s = dev->read_subdev;
-   async = s->async;
-   cmd = >cmd;
-
/* figure out how many samples are in fifo */
hw_counter = comedi_8254_read(devpriv->counter, 1);
/*
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 03/25] staging: comedi: das16m1: tidy up comment in das16m1_cmd_exec()

[H Hartley Sweeten]

Fix the checkpatch.pl issue:
WARNING: Block comments use a trailing */ on a separate line

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index 75e096a..05ceb13 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -291,8 +291,10 @@ static int das16m1_cmd_exec(struct comedi_device *dev,
 
/*  set control & status register */
byte = 0;
-   /* if we are using external start trigger (also board dislikes having
-* both start and conversion triggers external simultaneously) */
+   /*
+* If we are using external start trigger (also board dislikes having
+* both start and conversion triggers external simultaneously).
+*/
if (cmd->start_src == TRIG_EXT && cmd->convert_src != TRIG_EXT)
byte |= EXT_TRIG_BIT;
 
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 02/25] staging: comedi: das16m1: tidy up comments in das16m1_handler()

[H Hartley Sweeten]

Fix the checkpatch.pl issue:
WARNING: Block comments use a trailing */ on a separate line

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 18 --
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index 90d3869..75e096a 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -405,20 +405,24 @@ static void das16m1_handler(struct comedi_device *dev, 
unsigned int status)
 
/* figure out how many samples are in fifo */
hw_counter = comedi_8254_read(devpriv->counter, 1);
-   /* make sure hardware counter reading is not bogus due to initial value
-* not having been loaded yet */
+   /*
+* Make sure hardware counter reading is not bogus due to initial
+* value not having been loaded yet.
+*/
if (devpriv->adc_count == 0 &&
hw_counter == devpriv->initial_hw_count) {
num_samples = 0;
} else {
-   /* The calculation of num_samples looks odd, but it uses the
+   /*
+* The calculation of num_samples looks odd, but it uses the
 * following facts. 16 bit hardware counter is initialized with
 * value of zero (which really means 0x1000).  The counter
 * decrements by one on each conversion (when the counter
 * decrements from zero it goes to 0x).  num_samples is a
 * 16 bit variable, so it will roll over in a similar fashion
 * to the hardware counter.  Work it out, and this is what you
-* get. */
+* get.
+*/
num_samples = -hw_counter - devpriv->adc_count;
}
/*  check if we only need some of the points */
@@ -441,8 +445,10 @@ static void das16m1_handler(struct comedi_device *dev, 
unsigned int status)
}
}
 
-   /* this probably won't catch overruns since the card doesn't generate
-* overrun interrupts, but we might as well try */
+   /*
+* This probably won't catch overruns since the card doesn't generate
+* overrun interrupts, but we might as well try.
+*/
if (status & OVRUN) {
async->events |= COMEDI_CB_ERROR;
dev_err(dev->class_dev, "fifo overflow\n");
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 22/25] staging: comedi: das16m1: minor cleanup to das16m1_ai_insn_read()

[H Hartley Sweeten]

The (*insn_read) functions return the number of data values read. The 'n'
value is correct but for clarity change the return to 'insn->n'.

For aesthetics, change the 'n' loop variable name to 'i'. That's more common
in comedi drivers.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index d4b1b92..c0f2796 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -324,11 +324,11 @@ static int das16m1_ai_insn_read(struct comedi_device *dev,
unsigned int *data)
 {
int ret;
-   int n;
+   int i;
 
das16m1_ai_set_queue(dev, >chanspec, 1);
 
-   for (n = 0; n < insn->n; n++) {
+   for (i = 0; i < insn->n; i++) {
unsigned short val;
 
/* clear interrupt */
@@ -341,10 +341,10 @@ static int das16m1_ai_insn_read(struct comedi_device *dev,
return ret;
 
val = inw(dev->iobase + DAS16M1_AI_REG);
-   data[n] = DAS16M1_AI_TO_SAMPLE(val);
+   data[i] = DAS16M1_AI_TO_SAMPLE(val);
}
 
-   return n;
+   return insn->n;
 }
 
 static int das16m1_di_insn_bits(struct comedi_device *dev,
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 11/25] staging: comedi: das16m1: tidy up interrupt control register defines

[H Hartley Sweeten]

Rename these defines and use the BIT macro to define the bits. Define
some macros for the multi-bit fields in the register.

Rename the private data 'control_state' member to better match the
register name.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 34 ++--
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index ce788ae..5b1d1fc 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -73,11 +73,13 @@
 #define DAS16M1_DI_REG 0x03
 #define DAS16M1_DO_REG 0x03
 #define DAS16M1_CLR_INTR_REG   0x04
-#define DAS16M1_INTR_CONTROL   5
-#define   EXT_PACER  0x2
-#define   INT_PACER  0x3
-#define   PACER_MASK 0x3
-#define   INTE   0x80
+#define DAS16M1_INTR_CTRL_REG  0x05
+#define DAS16M1_INTR_CTRL_PACER(x) (((x) & 0x3) << 0)
+#define DAS16M1_INTR_CTRL_PACER_EXTDAS16M1_INTR_CTRL_PACER(2)
+#define DAS16M1_INTR_CTRL_PACER_INTDAS16M1_INTR_CTRL_PACER(3)
+#define DAS16M1_INTR_CTRL_PACER_MASK   DAS16M1_INTR_CTRL_PACER(3)
+#define DAS16M1_INTR_CTRL_IRQ(x)   (((x) & 0x7) << 4)
+#define DAS16M1_INTR_CTRL_INTE BIT(7)
 #define DAS16M1_QUEUE_ADDR 6
 #define DAS16M1_QUEUE_DATA 7
 #define   Q_CHAN(x)  ((x) & 0x7)
@@ -104,7 +106,7 @@ static const struct comedi_lrange range_das16m1 = {
 
 struct das16m1_private_struct {
struct comedi_8254 *counter;
-   unsigned int control_state;
+   unsigned int intr_ctrl;
unsigned int adc_count;
u16 initial_hw_count;
unsigned short ai_buffer[FIFO_SIZE];
@@ -253,13 +255,13 @@ static int das16m1_cmd_exec(struct comedi_device *dev,
}
 
/* enable interrupts and set internal pacer counter mode and counts */
-   devpriv->control_state &= ~PACER_MASK;
+   devpriv->intr_ctrl &= ~DAS16M1_INTR_CTRL_PACER_MASK;
if (cmd->convert_src == TRIG_TIMER) {
comedi_8254_update_divisors(dev->pacer);
comedi_8254_pacer_enable(dev->pacer, 1, 2, true);
-   devpriv->control_state |= INT_PACER;
+   devpriv->intr_ctrl |= DAS16M1_INTR_CTRL_PACER_INT;
} else {/* TRIG_EXT */
-   devpriv->control_state |= EXT_PACER;
+   devpriv->intr_ctrl |= DAS16M1_INTR_CTRL_PACER_EXT;
}
 
/*  set control & status register */
@@ -276,8 +278,8 @@ static int das16m1_cmd_exec(struct comedi_device *dev,
/* clear interrupt */
outb(0, dev->iobase + DAS16M1_CLR_INTR_REG);
 
-   devpriv->control_state |= INTE;
-   outb(devpriv->control_state, dev->iobase + DAS16M1_INTR_CONTROL);
+   devpriv->intr_ctrl |= DAS16M1_INTR_CTRL_INTE;
+   outb(devpriv->intr_ctrl, dev->iobase + DAS16M1_INTR_CTRL_REG);
 
return 0;
 }
@@ -286,8 +288,10 @@ static int das16m1_cancel(struct comedi_device *dev, 
struct comedi_subdevice *s)
 {
struct das16m1_private_struct *devpriv = dev->private;
 
-   devpriv->control_state &= ~INTE & ~PACER_MASK;
-   outb(devpriv->control_state, dev->iobase + DAS16M1_INTR_CONTROL);
+   /* disable interrupts and pacer */
+   devpriv->intr_ctrl &= ~(DAS16M1_INTR_CTRL_INTE |
+   DAS16M1_INTR_CTRL_PACER_MASK);
+   outb(devpriv->intr_ctrl, dev->iobase + DAS16M1_INTR_CTRL_REG);
 
return 0;
 }
@@ -592,8 +596,8 @@ static int das16m1_attach(struct comedi_device *dev,
outb(0, dev->iobase + DAS16M1_DO_REG);
 
/* set the interrupt level */
-   devpriv->control_state = das16m1_irq_bits(dev->irq) << 4;
-   outb(devpriv->control_state, dev->iobase + DAS16M1_INTR_CONTROL);
+   devpriv->intr_ctrl = DAS16M1_INTR_CTRL_IRQ(das16m1_irq_bits(dev->irq));
+   outb(devpriv->intr_ctrl, dev->iobase + DAS16M1_INTR_CTRL_REG);
 
return 0;
 }
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 06/25] staging: comedi: das16m1: tidy up analog input data register defines

[H Hartley Sweeten]

Convert the inline helper munge_sample() into a macro and rename the
defines for the analog input data register/bits. Use the register define
when accessing this register instead of just dev->iobase.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 19 +--
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index 6638a9a..2c7c86c 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -63,8 +63,9 @@
 /*
  * Register map (dev->iobase)
  */
-#define DAS16M1_AI 0   /*  16-bit wide register */
-#define   AI_CHAN(x) ((x) & 0xf)
+#define DAS16M1_AI_REG 0x00/* 16-bit register */
+#define DAS16M1_AI_TO_CHAN(x)  (((x) >> 0) & 0xf)
+#define DAS16M1_AI_TO_SAMPLE(x)(((x) >> 4) & 0xfff)
 #define DAS16M1_CS 2
 #define   EXT_TRIG_BIT   0x1
 #define   OVRUN  0x20
@@ -109,17 +110,12 @@ struct das16m1_private_struct {
unsigned long extra_iobase;
 };
 
-static inline unsigned short munge_sample(unsigned short data)
-{
-   return (data >> 4) & 0xfff;
-}
-
 static void munge_sample_array(unsigned short *array, unsigned int 
num_elements)
 {
unsigned int i;
 
for (i = 0; i < num_elements; i++)
-   array[i] = munge_sample(array[i]);
+   array[i] = DAS16M1_AI_TO_SAMPLE(array[i]);
 }
 
 static int das16m1_ai_check_chanlist(struct comedi_device *dev,
@@ -331,16 +327,19 @@ static int das16m1_ai_rinsn(struct comedi_device *dev,
outb(byte, dev->iobase + DAS16M1_QUEUE_DATA);
 
for (n = 0; n < insn->n; n++) {
+   unsigned short val;
+
/* clear IRQDATA bit */
outb(0, dev->iobase + DAS16M1_CLEAR_INTR);
/* trigger conversion */
-   outb(0, dev->iobase);
+   outb(0, dev->iobase + DAS16M1_AI_REG);
 
ret = comedi_timeout(dev, s, insn, das16m1_ai_eoc, 0);
if (ret)
return ret;
 
-   data[n] = munge_sample(inw(dev->iobase));
+   val = inw(dev->iobase + DAS16M1_AI_REG);
+   data[n] = DAS16M1_AI_TO_SAMPLE(val);
}
 
return n;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 23/25] staging: comedi: das16m1: convert munge_sample_array() into a subdevice (*munge)

[H Hartley Sweeten]

For aesthetics, convert this function into a subdevice (*munge) function and let
the comedi core handle the munging.

Add a comment about why the data needs to be munged.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index c0f2796..a803032 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -127,11 +127,21 @@ static void das16m1_ai_set_queue(struct comedi_device 
*dev,
}
 }
 
-static void munge_sample_array(unsigned short *array, unsigned int 
num_elements)
+static void das16m1_ai_munge(struct comedi_device *dev,
+struct comedi_subdevice *s,
+void *data, unsigned int num_bytes,
+unsigned int start_chan_index)
 {
+   unsigned short *array = data;
+   unsigned int nsamples = comedi_bytes_to_samples(s, num_bytes);
unsigned int i;
 
-   for (i = 0; i < num_elements; i++)
+   /*
+* The fifo values have the channel number in the lower 4-bits and
+* the sample in the upper 12-bits. This just shifts the values
+* to remove the channel numbers.
+*/
+   for (i = 0; i < nsamples; i++)
array[i] = DAS16M1_AI_TO_SAMPLE(array[i]);
 }
 
@@ -410,7 +420,6 @@ static void das16m1_handler(struct comedi_device *dev, 
unsigned int status)
if (num_samples > DAS16M1_AI_FIFO_SZ)
num_samples = DAS16M1_AI_FIFO_SZ;
insw(dev->iobase, devpriv->ai_buffer, num_samples);
-   munge_sample_array(devpriv->ai_buffer, num_samples);
comedi_buf_write_samples(s, devpriv->ai_buffer, num_samples);
devpriv->adc_count += num_samples;
 
@@ -560,6 +569,7 @@ static int das16m1_attach(struct comedi_device *dev,
s->do_cmd   = das16m1_ai_cmd;
s->cancel   = das16m1_ai_cancel;
s->poll = das16m1_ai_poll;
+   s->munge= das16m1_ai_munge;
}
 
/* Digital Input subdevice */
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 05/25] staging: comedi: das16m1: tidy up register map comment

[H Hartley Sweeten]

The comment about all the registers is not neccessary. The defines
give the same information. This also fixes a checkpatch.pl issue:
WARNING: Block comments use * on subsequent lines

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 21 ++---
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index 078b26e..6638a9a 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -61,25 +61,8 @@
 #define FIFO_SIZE 1024 /*  1024 sample fifo */
 
 /*
-CIO-DAS16_M1.pdf
-
-"cio-das16/m1"
-
-  0a/d bits 0-3, mux   start 12 bit
-  1a/d bits 4-11   unused
-  2status  control
-  3di 4 bitdo 4 bit
-  4unused  clear interrupt
-  5interrupt, pacer
-  6channel/gain queue address
-  7channel/gain queue data
-  89ab 8254
-  cdef 8254
-  400  8255
-  404-407  8254
-
-*/
-
+ * Register map (dev->iobase)
+ */
 #define DAS16M1_AI 0   /*  16-bit wide register */
 #define   AI_CHAN(x) ((x) & 0xf)
 #define DAS16M1_CS 2
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 13/25] staging: comedi: das16m1: introduce das16m1_ai_set_queue()

[H Hartley Sweeten]

Introduce a helper function to handle writing the channel/gain data to
the queue for single channel reads, (*insn_read), and multi-channel
scans, (*do_cmd).

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 33 +---
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index 1a8cf92..b941dcf 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -112,6 +112,21 @@ struct das16m1_private_struct {
unsigned long extra_iobase;
 };
 
+static void das16m1_ai_set_queue(struct comedi_device *dev,
+unsigned int *chanspec, unsigned int len)
+{
+   unsigned int i;
+
+   for (i = 0; i < len; i++) {
+   unsigned int chan = CR_CHAN(chanspec[i]);
+   unsigned int range = CR_RANGE(chanspec[i]);
+
+   outb(i, dev->iobase + DAS16M1_Q_ADDR_REG);
+   outb(DAS16M1_Q_CHAN(chan) | DAS16M1_Q_RANGE(range),
+dev->iobase + DAS16M1_Q_REG);
+   }
+}
+
 static void munge_sample_array(unsigned short *array, unsigned int 
num_elements)
 {
unsigned int i;
@@ -225,7 +240,7 @@ static int das16m1_cmd_exec(struct comedi_device *dev,
struct das16m1_private_struct *devpriv = dev->private;
struct comedi_async *async = s->async;
struct comedi_cmd *cmd = >cmd;
-   unsigned int byte, i;
+   unsigned int byte;
 
/*  set software count */
devpriv->adc_count = 0;
@@ -244,14 +259,7 @@ static int das16m1_cmd_exec(struct comedi_device *dev,
 */
devpriv->initial_hw_count = comedi_8254_read(devpriv->counter, 1);
 
-   /* setup channel/gain queue */
-   for (i = 0; i < cmd->chanlist_len; i++) {
-   outb(i, dev->iobase + DAS16M1_Q_ADDR_REG);
-   byte =
-   DAS16M1_Q_CHAN(CR_CHAN(cmd->chanlist[i])) |
-   DAS16M1_Q_RANGE(CR_RANGE(cmd->chanlist[i]));
-   outb(byte, dev->iobase + DAS16M1_Q_REG);
-   }
+   das16m1_ai_set_queue(dev, cmd->chanlist, cmd->chanlist_len);
 
/* enable interrupts and set internal pacer counter mode and counts */
devpriv->intr_ctrl &= ~DAS16M1_INTR_CTRL_PACER_MASK;
@@ -314,13 +322,8 @@ static int das16m1_ai_rinsn(struct comedi_device *dev,
 {
int ret;
int n;
-   int byte;
 
-   /* setup channel/gain queue */
-   outb(0, dev->iobase + DAS16M1_Q_ADDR_REG);
-   byte = DAS16M1_Q_CHAN(CR_CHAN(insn->chanspec)) |
-  DAS16M1_Q_RANGE(CR_RANGE(insn->chanspec));
-   outb(byte, dev->iobase + DAS16M1_Q_REG);
+   das16m1_ai_set_queue(dev, >chanspec, 1);
 
for (n = 0; n < insn->n; n++) {
unsigned short val;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 12/25] staging: comedi: das16m1: tidy up queue register defines

[H Hartley Sweeten]

Rename these defines.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index 5b1d1fc..1a8cf92 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -80,11 +80,10 @@
 #define DAS16M1_INTR_CTRL_PACER_MASK   DAS16M1_INTR_CTRL_PACER(3)
 #define DAS16M1_INTR_CTRL_IRQ(x)   (((x) & 0x7) << 4)
 #define DAS16M1_INTR_CTRL_INTE BIT(7)
-#define DAS16M1_QUEUE_ADDR 6
-#define DAS16M1_QUEUE_DATA 7
-#define   Q_CHAN(x)  ((x) & 0x7)
-#define   Q_RANGE(x) (((x) & 0xf) << 4)
-#define   UNIPOLAR   0x40
+#define DAS16M1_Q_ADDR_REG 0x06
+#define DAS16M1_Q_REG  0x07
+#define DAS16M1_Q_CHAN(x)  (((x) & 0x7) << 0)
+#define DAS16M1_Q_RANGE(x) (((x) & 0xf) << 4)
 #define DAS16M1_8254_FIRST 0x8
 #define DAS16M1_8254_SECOND0xc
 #define DAS16M1_82C55  0x400
@@ -247,11 +246,11 @@ static int das16m1_cmd_exec(struct comedi_device *dev,
 
/* setup channel/gain queue */
for (i = 0; i < cmd->chanlist_len; i++) {
-   outb(i, dev->iobase + DAS16M1_QUEUE_ADDR);
+   outb(i, dev->iobase + DAS16M1_Q_ADDR_REG);
byte =
-   Q_CHAN(CR_CHAN(cmd->chanlist[i])) |
-   Q_RANGE(CR_RANGE(cmd->chanlist[i]));
-   outb(byte, dev->iobase + DAS16M1_QUEUE_DATA);
+   DAS16M1_Q_CHAN(CR_CHAN(cmd->chanlist[i])) |
+   DAS16M1_Q_RANGE(CR_RANGE(cmd->chanlist[i]));
+   outb(byte, dev->iobase + DAS16M1_Q_REG);
}
 
/* enable interrupts and set internal pacer counter mode and counts */
@@ -318,10 +317,10 @@ static int das16m1_ai_rinsn(struct comedi_device *dev,
int byte;
 
/* setup channel/gain queue */
-   outb(0, dev->iobase + DAS16M1_QUEUE_ADDR);
-   byte =
-   Q_CHAN(CR_CHAN(insn->chanspec)) | Q_RANGE(CR_RANGE(insn->chanspec));
-   outb(byte, dev->iobase + DAS16M1_QUEUE_DATA);
+   outb(0, dev->iobase + DAS16M1_Q_ADDR_REG);
+   byte = DAS16M1_Q_CHAN(CR_CHAN(insn->chanspec)) |
+  DAS16M1_Q_RANGE(CR_RANGE(insn->chanspec));
+   outb(byte, dev->iobase + DAS16M1_Q_REG);
 
for (n = 0; n < insn->n; n++) {
unsigned short val;
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 10/25] staging: comedi: das16m1: remove unnecessary ai 'cancel' operations

[H Hartley Sweeten]

The comedi core will only call the (*insn_read) and (*do_cmd) functions
if the subdevice is not "busy". All async commands are terminated by
a (*cancel) operation which clears the INTE and PACER_MASK bits in the
interrupt control register. These bits are also cleared when the driver
first attaches. There is no need for the (*insn_read) or (*do_cmd) to
duplicate the cancel.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 9 -
 1 file changed, 9 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index dcec4e2..ce788ae 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -226,10 +226,6 @@ static int das16m1_cmd_exec(struct comedi_device *dev,
struct comedi_cmd *cmd = >cmd;
unsigned int byte, i;
 
-   /* disable interrupts and internal pacer */
-   devpriv->control_state &= ~INTE & ~PACER_MASK;
-   outb(devpriv->control_state, dev->iobase + DAS16M1_INTR_CONTROL);
-
/*  set software count */
devpriv->adc_count = 0;
 
@@ -313,15 +309,10 @@ static int das16m1_ai_rinsn(struct comedi_device *dev,
struct comedi_subdevice *s,
struct comedi_insn *insn, unsigned int *data)
 {
-   struct das16m1_private_struct *devpriv = dev->private;
int ret;
int n;
int byte;
 
-   /* disable interrupts and internal pacer */
-   devpriv->control_state &= ~INTE & ~PACER_MASK;
-   outb(devpriv->control_state, dev->iobase + DAS16M1_INTR_CONTROL);
-
/* setup channel/gain queue */
outb(0, dev->iobase + DAS16M1_QUEUE_ADDR);
byte =
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 14/25] staging: comedi: das16m1: tidy up analog input subdevice init

[H Hartley Sweeten]

Add some whitespace to the subdevice init and rename the support
functions.

Signed-off-by: H Hartley Sweeten <hswee...@visionengravers.com>
Cc: Ian Abbott <abbo...@mev.co.uk>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
 drivers/staging/comedi/drivers/das16m1.c | 48 +---
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/drivers/staging/comedi/drivers/das16m1.c 
b/drivers/staging/comedi/drivers/das16m1.c
index b941dcf..b19715c 100644
--- a/drivers/staging/comedi/drivers/das16m1.c
+++ b/drivers/staging/comedi/drivers/das16m1.c
@@ -163,8 +163,9 @@ static int das16m1_ai_check_chanlist(struct comedi_device 
*dev,
return 0;
 }
 
-static int das16m1_cmd_test(struct comedi_device *dev,
-   struct comedi_subdevice *s, struct comedi_cmd *cmd)
+static int das16m1_ai_cmdtest(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ struct comedi_cmd *cmd)
 {
int err = 0;
 
@@ -234,8 +235,8 @@ static int das16m1_cmd_test(struct comedi_device *dev,
return 0;
 }
 
-static int das16m1_cmd_exec(struct comedi_device *dev,
-   struct comedi_subdevice *s)
+static int das16m1_ai_cmd(struct comedi_device *dev,
+ struct comedi_subdevice *s)
 {
struct das16m1_private_struct *devpriv = dev->private;
struct comedi_async *async = s->async;
@@ -291,7 +292,8 @@ static int das16m1_cmd_exec(struct comedi_device *dev,
return 0;
 }
 
-static int das16m1_cancel(struct comedi_device *dev, struct comedi_subdevice 
*s)
+static int das16m1_ai_cancel(struct comedi_device *dev,
+struct comedi_subdevice *s)
 {
struct das16m1_private_struct *devpriv = dev->private;
 
@@ -316,9 +318,10 @@ static int das16m1_ai_eoc(struct comedi_device *dev,
return -EBUSY;
 }
 
-static int das16m1_ai_rinsn(struct comedi_device *dev,
-   struct comedi_subdevice *s,
-   struct comedi_insn *insn, unsigned int *data)
+static int das16m1_ai_insn_read(struct comedi_device *dev,
+   struct comedi_subdevice *s,
+   struct comedi_insn *insn,
+   unsigned int *data)
 {
int ret;
int n;
@@ -437,7 +440,8 @@ static void das16m1_handler(struct comedi_device *dev, 
unsigned int status)
comedi_handle_events(dev, s);
 }
 
-static int das16m1_poll(struct comedi_device *dev, struct comedi_subdevice *s)
+static int das16m1_ai_poll(struct comedi_device *dev,
+  struct comedi_subdevice *s)
 {
unsigned long flags;
unsigned int status;
@@ -552,22 +556,22 @@ static int das16m1_attach(struct comedi_device *dev,
if (ret)
return ret;
 
+   /* Analog Input subdevice */
s = >subdevices[0];
-   /* ai */
-   s->type = COMEDI_SUBD_AI;
-   s->subdev_flags = SDF_READABLE | SDF_DIFF;
-   s->n_chan = 8;
-   s->maxdata = (1 << 12) - 1;
-   s->range_table = _das16m1;
-   s->insn_read = das16m1_ai_rinsn;
+   s->type = COMEDI_SUBD_AI;
+   s->subdev_flags = SDF_READABLE | SDF_DIFF;
+   s->n_chan   = 8;
+   s->maxdata  = 0x0fff;
+   s->range_table  = _das16m1;
+   s->insn_read= das16m1_ai_insn_read;
if (dev->irq) {
dev->read_subdev = s;
-   s->subdev_flags |= SDF_CMD_READ;
-   s->len_chanlist = 256;
-   s->do_cmdtest = das16m1_cmd_test;
-   s->do_cmd = das16m1_cmd_exec;
-   s->cancel = das16m1_cancel;
-   s->poll = das16m1_poll;
+   s->subdev_flags |= SDF_CMD_READ;
+   s->len_chanlist = 256;
+   s->do_cmdtest   = das16m1_ai_cmdtest;
+   s->do_cmd   = das16m1_ai_cmd;
+   s->cancel   = das16m1_ai_cancel;
+   s->poll = das16m1_ai_poll;
}
 
s = >subdevices[1];
-- 
2.6.3

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel