Re: [PATCH] staging: comedi: fix spelling mistake "desination" -> "destination"
On 27/11/2018 14:23, Colin King wrote: From: Colin Ian King There is a spelling mistake in message text in the call to unittest, fix this. Signed-off-by: Colin Ian King --- drivers/staging/comedi/drivers/tests/ni_routes_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/tests/ni_routes_test.c b/drivers/staging/comedi/drivers/tests/ni_routes_test.c index a1eda035f270..c6dc18f346e8 100644 --- a/drivers/staging/comedi/drivers/tests/ni_routes_test.c +++ b/drivers/staging/comedi/drivers/tests/ni_routes_test.c @@ -372,7 +372,7 @@ void test_ni_lookup_route_register(void) unittest(ni_lookup_route_register(O(8), O(9), T) == 8, "validate last destination\n"); unittest(ni_lookup_route_register(O(10), O(9), T) == -EINVAL, -"lookup invalid desination\n"); +"lookup invalid destination\n"); unittest(ni_lookup_route_register(rgout0_src0, TRIGGER_LINE(0), T) == -EINVAL, Thanks for spotting that! Looks good. Reviewed-by: Ian Abbott -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: comedi: change do_insn*_ioctl to allow more samples
On 04/12/2018 19:07, Spencer E. Olson wrote: Changes do_insn*_ioctl functions to allow for data lengths for each comedi_insn of up to 2^16. This patch also changes these functions to only allocate as much memory as is necessary for each comedi_insn, rather than allocating a fixed-sized scratch space. In testing some user-space code for the new INSN_DEVICE_CONFIG_GET_ROUTES facility with some newer hardware, I discovered that do_insn_ioctl and do_insnlist_ioctl limited the amount of data that can be passed into the kernel for insn's to a length of 256. For some newer hardware, the number of routes can be greater than 1000. Working around the old limits (256) would complicate the user-space/kernel interaction. The new upper limit is reasonable with current memory available and does not otherwise impact the memory footprint for any current or otherwise typical configuration. Signed-off-by: Spencer E. Olson --- Implements Ian's suggestions to: 1) Provide a minimum amount of space to allocate (16*sizeof(uint)) to protect drivers that do not (yet) check the size of the instruction data (Ian has submitted several patches fixing this for other drivers already). In case insn.n does not get set, this minimum amount also avoids trying to allocate zero-length data. 2) Allocate the maximum required space needed for any of the instructions in an instruction list before executing the list of instructions (this, rather than allocating and freeing memory separately while iterating through the instruction list and executing each instruction). drivers/staging/comedi/comedi_fops.c | 48 ++-- 1 file changed, 31 insertions(+), 17 deletions(-) Looks good, thanks! (I still need to crack on and fix up the remaining drivers that ignore insn->n.) Reviewed-by: Ian Abbott -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] comedi/ni_pcidio: make all defines uppercase
On 17/12/2018 20:51, Alexander Schroth wrote: According to the Linux coding guidelines, defines should be written in uppercase. This patch converts all define-statements in the ni_pcidio.c file to uppercase, thus matching the coding style of the kernel. Signed-off-by: Alexander Schroth Signed-off-by: Marco Ammon --- Minor quibble: There should be a description of the 'v2' changes here (after the '---' line). Also, the patch "Subject:" line is missing the "staging:" tag. The usual set of tags for patches to this driver is "staging: comedi: ni_pcidio:". Apart from that, the patch seems fine. Reviewed-by: Ian Abbott drivers/staging/comedi/drivers/ni_pcidio.c | 442 +++-- 1 file changed, 222 insertions(+), 220 deletions(-) -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3] staging: comedi: ni_pcidio: make defines uppercase
On 10/01/2019 14:57, Alexander Schroth wrote: According to the Linux coding guidelines, defines should be written in uppercase. This patch converts all define-statements in the ni_pcidio.c file to uppercase, thus matching the coding style of the kernel. Signed-off-by: Alexander Schroth Signed-off-by: Marco Ammon --- v2: After feedback, renamed defines we had proposed to delete in the original patch series. v3: rephrased subject line to add correct tags --- drivers/staging/comedi/drivers/ni_pcidio.c | 444 +++-- 1 file changed, 223 insertions(+), 221 deletions(-) Thanks for resending. The patch looks fine, thanks. Reviewed-by: Ian Abbott -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: Removed not necessary braces for single block
On 15/01/2019 15:36, Jitendra Khasdev wrote:
This patch is used to remove not necessary braces for single if block.
Signed-off-by: Jitendra Khasdev
---
drivers/staging/comedi/comedi_fops.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 5d2fcbfe02af..38980fad8be4 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1605,9 +1605,9 @@ static int do_insn_ioctl(struct comedi_device *dev,
unsigned int n_data = MIN_SAMPLES;
int ret = 0;
- if (copy_from_user(&insn, arg, sizeof(insn))) {
+ if (copy_from_user(&insn, arg, sizeof(insn)))
return -EFAULT;
- }
+
n_data = max(n_data, insn.n);
The patch looks fine. Thanks for the fix!
Reviewed-by: Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: comedi: ni_660x: fix missing break in switch statement
On 12/02/2019 18:44, Gustavo A. R. Silva wrote:
Add missing break statement in order to prevent the code from falling
through to the default case and return -EINVAL every time.
This bug was found thanks to the ongoing efforts to enable
-Wimplicit-fallthrough.
Fixes: aa94f225 ("staging: comedi: ni_660x: tidy up
ni_660x_set_pfi_routing()")
Cc: sta...@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva
---
Changes in v2:
- Fix Fixes tag.
drivers/staging/comedi/drivers/ni_660x.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/staging/comedi/drivers/ni_660x.c
b/drivers/staging/comedi/drivers/ni_660x.c
index e70a461e723f..405573e927cf 100644
--- a/drivers/staging/comedi/drivers/ni_660x.c
+++ b/drivers/staging/comedi/drivers/ni_660x.c
@@ -656,6 +656,7 @@ static int ni_660x_set_pfi_routing(struct comedi_device
*dev,
case NI_660X_PFI_OUTPUT_DIO:
if (chan > 31)
return -EINVAL;
+ break;
default:
return -EINVAL;
}
Thanks for the bug fix!
Reviewed-by: Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_660x: fix missing break in switch statement
On 15/02/2019 15:48, Sasha Levin wrote:
Hi,
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag,
fixing commit: 58dd7c0a2a6e Staging: comedi: add ni_660x driver.
The bot has tested the following trees: v4.20.8, v4.19.21, v4.14.99, v4.9.156,
v4.4.174, v3.18.134.
v4.20.8: Build OK!
v4.19.21: Build OK!
v4.14.99: Build OK!
v4.9.156: Build OK!
v4.4.174: Failed to apply! Possible dependencies:
01ead0ded315 ("staging: comedi: ni_660x: cleanup the NI660X_IO_CFG register
helpers")
22acd860137a ("staging: comedi: ni_660x: change IOConfigReg() into a
macro")
41014593caeb ("staging: comedi: ni_660x: cleanup the NI660X_GLOBAL_INT_{STATUS,
CFG}")
502552e161ae ("staging: comedi: ni_660x: remove enum
clock_config_register_bits")
518d38423b48 ("staging: comedi: ni_660x: tidy up
ni_660x_select_pfi_output()")
9678b73e273a ("staging: comedi: ni_660x: tidy up ni_660x_write_register()")
aa94f225 ("staging: comedi: ni_660x: tidy up
ni_660x_set_pfi_routing()")
ad98c18cb9de ("staging: comedi: ni_660x: tidy up ni_660x_read_register()")
cded944fa90c ("staging: comedi: ni_660x: Prefer kernel type 'u64' over
'uint64_t'")
fecf4cce0021 ("staging: comedi: ni_660x: cleanup the NI660X_DMA_CFG register
helpers")
v3.18.134: Failed to apply! Possible dependencies:
01ead0ded315 ("staging: comedi: ni_660x: cleanup the NI660X_IO_CFG register
helpers")
22acd860137a ("staging: comedi: ni_660x: change IOConfigReg() into a
macro")
41014593caeb ("staging: comedi: ni_660x: cleanup the NI660X_GLOBAL_INT_{STATUS,
CFG}")
502552e161ae ("staging: comedi: ni_660x: remove enum
clock_config_register_bits")
518d38423b48 ("staging: comedi: ni_660x: tidy up
ni_660x_select_pfi_output()")
9678b73e273a ("staging: comedi: ni_660x: tidy up ni_660x_write_register()")
aa94f225 ("staging: comedi: ni_660x: tidy up
ni_660x_set_pfi_routing()")
ad98c18cb9de ("staging: comedi: ni_660x: tidy up ni_660x_read_register()")
cded944fa90c ("staging: comedi: ni_660x: Prefer kernel type 'u64' over
'uint64_t'")
fecf4cce0021 ("staging: comedi: ni_660x: cleanup the NI660X_DMA_CFG register
helpers")
How should we proceed with this patch?
Hi Sasha, the bug was introduced in v4.7 and hasn't been backported to
any earlier stable kernels, so no need to do anything for v4.4.x or v3.18.x.
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: ni_tio: Allocate shadow regs for each counter chip
The "ni_tio" module contains code to allocate, destroy and operate on a
`struct ni_gpct_device`, which represents a number of counters spread
over one or more blocks (or "chips"). `struct ni_gpct_device` includes
an array member `regs` holding shadow copies of register values.
Unfortunately, this is currently shared by each block of counters so
they interfere with each other. This is a problem for the "ni_660x"
module, which has 8 counters spread over 2 blocks. The `regs` storage
needs to be two-dimensional, indexed by block (chip) number and register
number. (It does not need to be three-dimensional because the registers
for individual counters are intermingled within the block.)
Change the `regs` member to an array pointer that can be indexed like a
two-dimensional array to access the shadow storage for each register in
each block. Allocate the storage in `ni_gpct_device_construct()` and
free it in `ni_gpct_device_destroy()`. (`ni_gpct_device_construct()`
can determine the number of blocks from the `num_counters` and
`counters_per_chip` parameters.)
Add new member `num_chips` to hold the number of chips. Use that to
check that `chip_index` value is in range in the same places that
check the register offset is in range.
Remove the `counters_per_chip` member of `struct ni_gpct_device` as it
is not needed anywhere and could be easily derived from the
`num_counters` and `num_chips` members if required.
Thanks to GitHub user "raabej" (real name unknown) for an initial
implementation of this in the out-of-tree fork of the Comedi drivers.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/ni_tio.c | 71 -
drivers/staging/comedi/drivers/ni_tio.h | 4 +-
2 files changed, 47 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_tio.c
b/drivers/staging/comedi/drivers/ni_tio.c
index 0eb388c0e1f0..048cb35723ad 100644
--- a/drivers/staging/comedi/drivers/ni_tio.c
+++ b/drivers/staging/comedi/drivers/ni_tio.c
@@ -224,13 +224,16 @@ static void ni_tio_set_bits_transient(struct ni_gpct
*counter,
unsigned int transient)
{
struct ni_gpct_device *counter_dev = counter->counter_dev;
+ unsigned int chip = counter->chip_index;
unsigned long flags;
- if (reg < NITIO_NUM_REGS) {
+ if (reg < NITIO_NUM_REGS && chip < counter_dev->num_chips) {
+ unsigned int *regs = counter_dev->regs[chip];
+
spin_lock_irqsave(&counter_dev->regs_lock, flags);
- counter_dev->regs[reg] &= ~mask;
- counter_dev->regs[reg] |= (value & mask);
- ni_tio_write(counter, counter_dev->regs[reg] | transient, reg);
+ regs[reg] &= ~mask;
+ regs[reg] |= (value & mask);
+ ni_tio_write(counter, regs[reg] | transient, reg);
mmiowb();
spin_unlock_irqrestore(&counter_dev->regs_lock, flags);
}
@@ -267,12 +270,13 @@ unsigned int ni_tio_get_soft_copy(const struct ni_gpct
*counter,
enum ni_gpct_register reg)
{
struct ni_gpct_device *counter_dev = counter->counter_dev;
+ unsigned int chip = counter->chip_index;
unsigned int value = 0;
unsigned long flags;
- if (reg < NITIO_NUM_REGS) {
+ if (reg < NITIO_NUM_REGS && chip < counter_dev->num_chips) {
spin_lock_irqsave(&counter_dev->regs_lock, flags);
- value = counter_dev->regs[reg];
+ value = counter_dev->regs[chip][reg];
spin_unlock_irqrestore(&counter_dev->regs_lock, flags);
}
return value;
@@ -302,6 +306,7 @@ static int ni_m_series_clock_src_select(const struct
ni_gpct *counter,
{
struct ni_gpct_device *counter_dev = counter->counter_dev;
unsigned int cidx = counter->counter_index;
+ unsigned int chip = counter->chip_index;
unsigned int second_gate_reg = NITIO_GATE2_REG(cidx);
unsigned int clock_source = 0;
unsigned int src;
@@ -318,7 +323,7 @@ static int ni_m_series_clock_src_select(const struct
ni_gpct *counter,
clock_source = NI_GPCT_TIMEBASE_2_CLOCK_SRC_BITS;
break;
case NI_M_TIMEBASE_3_CLK:
- if (counter_dev->regs[second_gate_reg] & GI_SRC_SUBSEL)
+ if (counter_dev->regs[chip][second_gate_reg] & GI_SRC_SUBSEL)
clock_source =
NI_GPCT_ANALOG_TRIGGER_OUT_CLOCK_SRC_BITS;
else
@@ -328,7 +333,7 @@ static int ni_m_series_clock_src_select(const struct
ni_gpct *counter,
clock_source = NI_GPCT_LOGIC_LOW_CLOCK_SRC_BITS;
break;
case NI_M_NEXT_GATE_CLK:
-
[PATCH] staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest
`ni_cdio_cmdtest()` validates Comedi asynchronous commands for the DIO
subdevice (subdevice 2) of supported National Instruments M-series
cards. It is called when handling the `COMEDI_CMD` and `COMEDI_CMDTEST`
ioctls for this subdevice. There are two causes for a possible
divide-by-zero error when validating that the `stop_arg` member of the
passed-in command is not too large.
The first cause for the divide-by-zero is that calls to
`comedi_bytes_per_scan()` are only valid once the command has been
copied to `s->async->cmd`, but that copy is only done for the
`COMEDI_CMD` ioctl. For the `COMEDI_CMDTEST` ioctl, it will use
whatever was left there by the previous `COMEDI_CMD` ioctl, if any.
(This is very likely, as it is usual for the application to use
`COMEDI_CMDTEST` before `COMEDI_CMD`.) If there has been no previous,
valid `COMEDI_CMD` for this subdevice, then `comedi_bytes_per_scan()`
will return 0, so the subsequent division in `ni_cdio_cmdtest()` of
`s->async->prealloc_bufsz / comedi_bytes_per_scan(s)` will be a
divide-by-zero error. To fix this error, call a new function
`comedi_bytes_per_scan_cmd(s, cmd)`, based on the existing
`comedi_bytes_per_scan(s)` but using a specified `struct comedi_cmd` for
its calculations. (Also refactor `comedi_bytes_per_scan()` to call the
new function.)
Once the first cause for the divide-by-zero has been fixed, the second
cause is that `comedi_bytes_per_scan_cmd()` can legitimately return 0 if
the `scan_end_arg` member of the `struct comedi_cmd` being tested is 0.
Fix it by only performing the division (and validating that `stop_arg`
is no more than the maximum value) if `comedi_bytes_per_scan_cmd()`
returns a non-zero value.
The problem was reported on the COMEDI mailing list here:
https://groups.google.com/forum/#!topic/comedi_list/4t9WlHzMhKM
Fixes: f164cbf98fa8 ("staging: comedi: ni_mio_common: add finite regeneration
to dio output")
Cc: # 4.6+
Cc: Spencer E. Olson
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedidev.h| 2 ++
drivers/staging/comedi/drivers.c | 33 ---
.../staging/comedi/drivers/ni_mio_common.c| 10 --
3 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index a7d569cfca5d..0dff1ac057cd 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -1001,6 +1001,8 @@ int comedi_dio_insn_config(struct comedi_device *dev,
unsigned int mask);
unsigned int comedi_dio_update_state(struct comedi_subdevice *s,
unsigned int *data);
+unsigned int comedi_bytes_per_scan_cmd(struct comedi_subdevice *s,
+ struct comedi_cmd *cmd);
unsigned int comedi_bytes_per_scan(struct comedi_subdevice *s);
unsigned int comedi_nscans_left(struct comedi_subdevice *s,
unsigned int nscans);
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index eefa62f42c0f..5a32b8fc000e 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -394,11 +394,13 @@ unsigned int comedi_dio_update_state(struct
comedi_subdevice *s,
EXPORT_SYMBOL_GPL(comedi_dio_update_state);
/**
- * comedi_bytes_per_scan() - Get length of asynchronous command "scan" in bytes
+ * comedi_bytes_per_scan_cmd() - Get length of asynchronous command "scan" in
+ * bytes
* @s: COMEDI subdevice.
+ * @cmd: COMEDI command.
*
* Determines the overall scan length according to the subdevice type and the
- * number of channels in the scan.
+ * number of channels in the scan for the specified command.
*
* For digital input, output or input/output subdevices, samples for
* multiple channels are assumed to be packed into one or more unsigned
@@ -408,9 +410,9 @@ EXPORT_SYMBOL_GPL(comedi_dio_update_state);
*
* Returns the overall scan length in bytes.
*/
-unsigned int comedi_bytes_per_scan(struct comedi_subdevice *s)
+unsigned int comedi_bytes_per_scan_cmd(struct comedi_subdevice *s,
+ struct comedi_cmd *cmd)
{
- struct comedi_cmd *cmd = &s->async->cmd;
unsigned int num_samples;
unsigned int bits_per_sample;
@@ -427,6 +429,29 @@ unsigned int comedi_bytes_per_scan(struct comedi_subdevice
*s)
}
return comedi_samples_to_bytes(s, num_samples);
}
+EXPORT_SYMBOL_GPL(comedi_bytes_per_scan_cmd);
+
+/**
+ * comedi_bytes_per_scan() - Get length of asynchronous command "scan" in bytes
+ * @s: COMEDI subdevice.
+ *
+ * Determines the overall scan length according to the subdevice type and the
+ * number of channels in the scan for the current command.
+ *
+ * For digital input, output or input/output subdevices, samples for
+ * multiple channels are assumed to be packed into one or more unsigned
+ * short o
Re: [PATCH] staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest
On 05/03/2019 11:10, Dan Carpenter wrote:
On Mon, Mar 04, 2019 at 02:33:54PM +, Ian Abbott wrote:
The problem was reported on the COMEDI mailing list here:
https://groups.google.com/forum/#!topic/comedi_list/4t9WlHzMhKM
Can you give Ivan a Reported-by tag? It's a public mailing list so
that shouldn't be a problem.
I can do, but I don't know his full name. Will that be a problem?
Fixes: f164cbf98fa8 ("staging: comedi: ni_mio_common: add finite regeneration to dio
output")
Cc: # 4.6+
Cc: Spencer E. Olson
Signed-off-by: Ian Abbott
---
regards,
dan carpenter
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest
On 05/03/2019 11:39, Dan Carpenter wrote: On Tue, Mar 05, 2019 at 11:32:18AM +, Ian Abbott wrote: On 05/03/2019 11:10, Dan Carpenter wrote: On Mon, Mar 04, 2019 at 02:33:54PM +, Ian Abbott wrote: The problem was reported on the COMEDI mailing list here: https://groups.google.com/forum/#!topic/comedi_list/4t9WlHzMhKM Can you give Ivan a Reported-by tag? It's a public mailing list so that shouldn't be a problem. I can do, but I don't know his full name. Will that be a problem? Nah, it's not a problem. People should fix their email headers to reflect what they want to be called. Or you could ask but I don't think I have ever asked for someone's name I've always just gone with their email header name. In this case, Ivan just signed off with that name and it didn't appear in the email headers. -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest
On 05/03/2019 23:42, Sasha Levin wrote:
Hi,
[This is an automated email]
This commit has been processed because it contains a "Fixes:" tag,
fixing commit: f164cbf98fa8 staging: comedi: ni_mio_common: add finite
regeneration to dio output.
The bot has tested the following trees: v4.20.13, v4.19.26, v4.14.104, v4.9.161.
v4.20.13: Build OK!
v4.19.26: Failed to apply! Possible dependencies:
56d0b826d39f ("staging: comedi: ni_mio_common: implement new routing for
TRIG_EXT")
v4.14.104: Failed to apply! Possible dependencies:
56d0b826d39f ("staging: comedi: ni_mio_common: implement new routing for
TRIG_EXT")
v4.9.161: Failed to apply! Possible dependencies:
56d0b826d39f ("staging: comedi: ni_mio_common: implement new routing for
TRIG_EXT")
How should we proceed with this patch?
I also noticed the same failures to apply manually with 'git am'.
However, I was able to apply the the patch to the stable kernels with
'patch -p1' (with some fuzz), so I'm hoping that it will be 'git
cherry-pick'able after it enters the mainline repo.
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_mio_common: Fix divide-by-zero for DIO cmdtest
On 04/03/2019 14:33, Ian Abbott wrote:
`ni_cdio_cmdtest()` validates Comedi asynchronous commands for the DIO
subdevice (subdevice 2) of supported National Instruments M-series
cards. It is called when handling the `COMEDI_CMD` and `COMEDI_CMDTEST`
ioctls for this subdevice. There are two causes for a possible
divide-by-zero error when validating that the `stop_arg` member of the
passed-in command is not too large.
The first cause for the divide-by-zero is that calls to
`comedi_bytes_per_scan()` are only valid once the command has been
copied to `s->async->cmd`, but that copy is only done for the
`COMEDI_CMD` ioctl. For the `COMEDI_CMDTEST` ioctl, it will use
whatever was left there by the previous `COMEDI_CMD` ioctl, if any.
(This is very likely, as it is usual for the application to use
`COMEDI_CMDTEST` before `COMEDI_CMD`.) If there has been no previous,
valid `COMEDI_CMD` for this subdevice, then `comedi_bytes_per_scan()`
will return 0, so the subsequent division in `ni_cdio_cmdtest()` of
`s->async->prealloc_bufsz / comedi_bytes_per_scan(s)` will be a
divide-by-zero error. To fix this error, call a new function
`comedi_bytes_per_scan_cmd(s, cmd)`, based on the existing
`comedi_bytes_per_scan(s)` but using a specified `struct comedi_cmd` for
its calculations. (Also refactor `comedi_bytes_per_scan()` to call the
new function.)
Once the first cause for the divide-by-zero has been fixed, the second
cause is that `comedi_bytes_per_scan_cmd()` can legitimately return 0 if
the `scan_end_arg` member of the `struct comedi_cmd` being tested is 0.
Fix it by only performing the division (and validating that `stop_arg`
is no more than the maximum value) if `comedi_bytes_per_scan_cmd()`
returns a non-zero value.
The problem was reported on the COMEDI mailing list here:
https://groups.google.com/forum/#!topic/comedi_list/4t9WlHzMhKM
Fixes: f164cbf98fa8 ("staging: comedi: ni_mio_common: add finite regeneration to dio
output")
Greg,
If it's not too late, it would be nice if the following "Reported-by:"
and "Tested-by:" lines could be added (or I can resend with these lines
included if necessary). It's no big deal if this is too late. I'll
live with it. Thanks.
Reported-by: Ivan Vasilyev
Tested-by: Ivan Vasilyev
Cc: # 4.6+
Cc: Spencer E. Olson
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedidev.h| 2 ++
drivers/staging/comedi/drivers.c | 33 ---
.../staging/comedi/drivers/ni_mio_common.c| 10 --
3 files changed, 38 insertions(+), 7 deletions(-)
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] staging: comedi: ni_mio_common: use insn->n in ni_m_series_eeprom_insn_read()
The `insn_read` handler for the EEPROM subdevice on M-series boards
(`ni_m_series_eeprom_insn_read()`) currently ignores `insn->n` (the
number of samples to read) and assumes a single sample is to be read
into `data[0]`. Fortunately, the Comedi core ensures that `data[]` has
a length of at least 16 so there is no problem with array bounds.
The usual Comedi convention for `insn_read` handlers is to read the same
channel `insn->n` times into successive elements of `data[]` so let's do
that. (Each channel corresponds to a single EEPROM address.) In this
case, the data value comes from a local copy of the EEPROM contents.
Also, follow the usual Comedi convention and return `insn->n` from the
handler to indicate success (although any non-negative value will do).
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/ni_mio_common.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 5b397ad2a604..5eef1a63e3eb 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -4524,10 +4524,12 @@ static int ni_m_series_eeprom_insn_read(struct
comedi_device *dev,
unsigned int *data)
{
struct ni_private *devpriv = dev->private;
+ unsigned int i;
- data[0] = devpriv->eeprom_buffer[CR_CHAN(insn->chanspec)];
+ for (i = 0; i < insn->n; i++)
+ data[i] = devpriv->eeprom_buffer[CR_CHAN(insn->chanspec)];
- return 1;
+ return insn->n;
}
static unsigned int ni_old_get_pfi_routing(struct comedi_device *dev,
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/4] staging: comedi: ni_mio_common: Don't assume insn->n is 1
For insn_read and insn_write handlers for the EEPROM and calibration subdevices currently assume that insn->n is 1 and access data[0] without checking insn->n. data[0] exists because the Comedi core ensures it, but in the case of the insn_write handler it will contain some random, uninitialized value. These patches change the handlers to follow the usual Comedi conventions. 1) staging: comedi: ni_mio_common: Use insn->n in ni_calib_insn_write() 2) staging: comedi: ni_mio_common: use insn->n in ni_calib_insn_read() 3) staging: comedi: ni_mio_common: use insn->n in ni_eeprom_insn_read() 4) staging: comedi: ni_mio_common: use insn->n in ni_m_series_eeprom_insn_read() drivers/staging/comedi/drivers/ni_mio_common.c | 30 +++--- 1 file changed, 22 insertions(+), 8 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/4] staging: comedi: ni_mio_common: Use insn->n in ni_calib_insn_write()
The `insn_write` handler for the calibration subdevice
(`ni_calib_insn_write()`) currently ignores `insn->n` (the number of
samples to write) and assumes a single sample is to be written, but
`insn->n` could be 0, meaning no samples should be written, in which
case `data[0]` is invalid.
Change `ni_calib_insn_write()` to only write to the calibration device
if `insn->n > 0`. There isn't much point writing all the values when
`insn->n > 1`, so just write the last one (`data[insn->n - 1]`).
Also follow the usual Comedi convention and return `insn->n` from the
handler to indicate success (although any non-negative return value will
do as far as the Comedi core is concerned).
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/ni_mio_common.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 5edf59ac6706..613b6b2abf7d 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -4394,9 +4394,13 @@ static int ni_calib_insn_write(struct comedi_device *dev,
struct comedi_insn *insn,
unsigned int *data)
{
- ni_write_caldac(dev, CR_CHAN(insn->chanspec), data[0]);
+ if (insn->n) {
+ /* only bother writing the last sample to the channel */
+ ni_write_caldac(dev, CR_CHAN(insn->chanspec),
+ data[insn->n - 1]);
+ }
- return 1;
+ return insn->n;
}
static int ni_calib_insn_read(struct comedi_device *dev,
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] staging: comedi: ni_mio_common: use insn->n in ni_calib_insn_read()
The `insn_read` handler for the calibration subdevice
(`ni_calib_insn_read()`) currently ignores `insn->n` (the number of
samples to read) and assumes a single sample is to be read into
`data[0]`. Fortunately, the Comedi core ensures that `data[]` has a
length of at least 16, so there is no problem with array bounds.
The usual Comedi convention for `insn_read` handlers is to read the same
channel `insn->n` times into successive elements of `data[]`, so let's
do that.
Also, follow the usual Comedi convention and return `insn->n` from the
handler to indicate success (although any non-negative value will do).
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/ni_mio_common.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 613b6b2abf7d..e008095436d7 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -4409,10 +4409,12 @@ static int ni_calib_insn_read(struct comedi_device *dev,
unsigned int *data)
{
struct ni_private *devpriv = dev->private;
+ unsigned int i;
- data[0] = devpriv->caldacs[CR_CHAN(insn->chanspec)];
+ for (i = 0; i < insn->n; i++)
+ data[0] = devpriv->caldacs[CR_CHAN(insn->chanspec)];
- return 1;
+ return insn->n;
}
static void caldac_setup(struct comedi_device *dev, struct comedi_subdevice *s)
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] staging: comedi: ni_mio_common: use insn->n in ni_eeprom_insn_read()
The `insn_read` handler for the EEPROM subdevice on E-series boards
(`ni_eeprom_insn_read()`) currently ignores `insn->n` (the number of
samples to read) and assumes a single sample is to be read into
`data[0]`. Fortunately, the Comedi core ensures that `data[]` has a
length of at least 16 so there is no problem with array bounds.
The usual Comedi convention for `insn_read` handlers is to read the same
channel `insn->n` times into successive elements of `data[]` so let's do
that. (Each channel number corresponds to a single EEPROM address.)
Since we do not expect the EEPROM data at a particular address to change
between readings, let's just read it once and copy the value `insn->n`
times.
Also, follow the usual Comedi convention and return `insn->n` from the
handler to indicate success (although any non-negative value will do).
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/ni_mio_common.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index e008095436d7..5b397ad2a604 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -4507,9 +4507,15 @@ static int ni_eeprom_insn_read(struct comedi_device *dev,
struct comedi_insn *insn,
unsigned int *data)
{
- data[0] = ni_read_eeprom(dev, CR_CHAN(insn->chanspec));
+ unsigned int val;
+ unsigned int i;
- return 1;
+ if (insn->n) {
+ val = ni_read_eeprom(dev, CR_CHAN(insn->chanspec));
+ for (i = 0; i < insn->n; i++)
+ data[i] = val;
+ }
+ return insn->n;
}
static int ni_m_series_eeprom_insn_read(struct comedi_device *dev,
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: comedi: ni_mio_common.c: Added blank line after declarations
On 21/03/2019 19:18, Arash Fotouhi wrote:
Added blank line after declarations.
Signed-off-by: Arash Fotouhi
---
drivers/staging/comedi/drivers/ni_mio_common.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 5edf59a..c6aff8f 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -2110,6 +2110,7 @@ static int ni_ai_cmdtest(struct comedi_device *dev,
struct comedi_subdevice *s,
if (cmd->scan_begin_src == TRIG_TIMER) {
unsigned int tmp = cmd->scan_begin_arg;
+
cmd->scan_begin_arg =
ni_timer_to_ns(dev, ni_ns_to_timer(dev,
cmd->scan_begin_arg,
@@ -2120,6 +2121,7 @@ static int ni_ai_cmdtest(struct comedi_device *dev,
struct comedi_subdevice *s,
if (cmd->convert_src == TRIG_TIMER) {
if (!devpriv->is_611x && !devpriv->is_6143) {
unsigned int tmp = cmd->convert_arg;
+
cmd->convert_arg =
ni_timer_to_ns(dev, ni_ns_to_timer(dev,
cmd->convert_arg,
Looks good, thanks.
Reviewed-by: Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: ni_tio: Use data[insn->n-1] in ni_tio_insn_write()
The `insn_write` handler for the counter subdevices
(`ni_tio_insn_write()`) writes a single data value `data[0]` to the
channel. Technically, `insn->n` specifies the number of successive
values from `data[]` to write to the channel, but when there is little
benefit in writing multiple data values, the usual Comedi convention is
to just write the last data value `data[insn->n - 1]`. Change the
function to follow that convention and use `data[insn->n - 1]` instead
of `data[0]`. (In practice, `insn->n` would normally be 1 anyway.)
Also follow the usual Comedi convention and return `insn->n` from the
handler to indicate success instead of 0 (although any non-negative
return value will do).
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/ni_tio.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_tio.c
b/drivers/staging/comedi/drivers/ni_tio.c
index 048cb35723ad..943c5177cbea 100644
--- a/drivers/staging/comedi/drivers/ni_tio.c
+++ b/drivers/staging/comedi/drivers/ni_tio.c
@@ -1682,9 +1682,11 @@ int ni_tio_insn_write(struct comedi_device *dev,
unsigned int cidx = counter->counter_index;
unsigned int chip = counter->chip_index;
unsigned int load_reg;
+ unsigned int load_val;
if (insn->n < 1)
return 0;
+ load_val = data[insn->n - 1];
switch (channel) {
case 0:
/*
@@ -1697,7 +1699,7 @@ int ni_tio_insn_write(struct comedi_device *dev,
* load register is already selected.
*/
load_reg = ni_tio_next_load_register(counter);
- ni_tio_write(counter, data[0], load_reg);
+ ni_tio_write(counter, load_val, load_reg);
ni_tio_set_bits_transient(counter, NITIO_CMD_REG(cidx),
0, 0, GI_LOAD);
/* restore load reg */
@@ -1705,17 +1707,17 @@ int ni_tio_insn_write(struct comedi_device *dev,
load_reg);
break;
case 1:
- counter_dev->regs[chip][NITIO_LOADA_REG(cidx)] = data[0];
- ni_tio_write(counter, data[0], NITIO_LOADA_REG(cidx));
+ counter_dev->regs[chip][NITIO_LOADA_REG(cidx)] = load_val;
+ ni_tio_write(counter, load_val, NITIO_LOADA_REG(cidx));
break;
case 2:
- counter_dev->regs[chip][NITIO_LOADB_REG(cidx)] = data[0];
- ni_tio_write(counter, data[0], NITIO_LOADB_REG(cidx));
+ counter_dev->regs[chip][NITIO_LOADB_REG(cidx)] = load_val;
+ ni_tio_write(counter, load_val, NITIO_LOADB_REG(cidx));
break;
default:
return -EINVAL;
}
- return 0;
+ return insn->n;
}
EXPORT_SYMBOL_GPL(ni_tio_insn_write);
--
2.20.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/13] staging: comedi: ni_routing: Add NI signal routing info
exit(1);
+ }
+
+ fprintf(fp, "%u,", dR->routes[i].src[j]);
+
+ ++j;
+ }
+ fprintf(fp, "],\n");
+
+ ++i;
+ }
+ fprintf(fp, " },\n\n");
+}
+
+int main(void)
+{
+ int n_families = sizeof(all_route_values) / sizeof(void *);
+ int n_devices = sizeof(device_routes_list) / sizeof(void *);
The more legitimate way to get the length of the array is 'sizeof(array)
/ sizeof(array[0])'. But maybe these will be changed to NULL-terminated
lists anyway?
+
+ FILE *fp = fopen("ni_values.py", "w");
+
+ /* write route register values */
+ fprintf(fp, "ni_route_values = {\n");
+ for (int i = 0; i < n_families; ++i)
+ family_write(all_route_values[i], fp);
+ fprintf(fp, "}\n\n");
+
+ /* write valid device routes */
+ fprintf(fp, "ni_device_routes = {\n");
+ for (int i = 0; i < n_devices; ++i)
+ device_write(device_routes_list[i], fp);
+ fprintf(fp, "}\n");
+
+ /* finish; close file */
+ fclose(fp);
+ return 0;
+}
[I don't really have any comments on the maintenance scripts in Python,
but I guess they'll need some minor tweaks to do the requested
constification changes to the C code.]
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_mio_common: protect register write overflow
On 19/09/18 17:17, Spencer E. Olson wrote:
Fixes two problems introduced as early as
commit 03aef4b6dc12 ("Staging: comedi: add ni_mio_common code"):
(1) Ensures that the last four bits of NISTC_RTSI_TRIGB_OUT_REG register is
not unduly overwritten on e-series devices. On e-series devices, the
first three of the last four bits are reserved. The last bit defines
the output selection of the RGOUT0 pin, otherwise known as
RTSI_Sub_Selection. For m-series devices, these last four bits are
indeed used as the output selection of the RTSI7 pin (and the
RTSI_Sub_Selection bit for the RGOUT0 pin is moved to the
RTSI_Trig_Direction register.
(2) Allows all 4 RTSI_BRD lines to be treated as valid sources for RTSI
lines.
This patch also cleans up the ni_get_rtsi_routing command for readability.
Fixes: 03aef4b6dc12 ("Staging: comedi: add ni_mio_common code")
Signed-off-by: Spencer E. Olson
---
I thought I had already submitted this patch a while ago... Whoops.
.../staging/comedi/drivers/ni_mio_common.c| 24 +--
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 4dee2fc37aed..4d0d0621780e 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -4980,7 +4980,10 @@ static int ni_valid_rtsi_output_source(struct
comedi_device *dev,
case NI_RTSI_OUTPUT_G_SRC0:
case NI_RTSI_OUTPUT_G_GATE0:
case NI_RTSI_OUTPUT_RGOUT0:
- case NI_RTSI_OUTPUT_RTSI_BRD_0:
+ case NI_RTSI_OUTPUT_RTSI_BRD(0):
+ case NI_RTSI_OUTPUT_RTSI_BRD(1):
+ case NI_RTSI_OUTPUT_RTSI_BRD(2):
+ case NI_RTSI_OUTPUT_RTSI_BRD(3):
return 1;
case NI_RTSI_OUTPUT_RTSI_OSC:
return (devpriv->is_m_series) ? 1 : 0;
@@ -5001,11 +5004,18 @@ static int ni_set_rtsi_routing(struct comedi_device
*dev,
devpriv->rtsi_trig_a_output_reg |= NISTC_RTSI_TRIG(chan, src);
ni_stc_writew(dev, devpriv->rtsi_trig_a_output_reg,
NISTC_RTSI_TRIGA_OUT_REG);
- } else if (chan < 8) {
+ } else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
devpriv->rtsi_trig_b_output_reg &= ~NISTC_RTSI_TRIG_MASK(chan);
devpriv->rtsi_trig_b_output_reg |= NISTC_RTSI_TRIG(chan, src);
ni_stc_writew(dev, devpriv->rtsi_trig_b_output_reg,
NISTC_RTSI_TRIGB_OUT_REG);
+ } else if (chan != NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
+ /* probably should never reach this, since the
+* ni_valid_rtsi_output_source above errors out if chan is too
+* high
+*/
+ dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);
+ return -EINVAL;
}
return 2;
}
@@ -5021,12 +5031,12 @@ static unsigned int ni_get_rtsi_routing(struct
comedi_device *dev,
} else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
return NISTC_RTSI_TRIG_TO_SRC(chan,
devpriv->rtsi_trig_b_output_reg);
- } else {
- if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN)
- return NI_RTSI_OUTPUT_RTSI_OSC;
- dev_err(dev->class_dev, "bug! should never get here?\n");
- return 0;
+ } else if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN)
+ return NI_RTSI_OUTPUT_RTSI_OSC;
}
+
+ dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);
+ return -EINVAL;
}
static int ni_rtsi_insn_config(struct comedi_device *dev,
Looks good, thanks!
Reviewed-by: Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/13] device-global identifiers and routes introduced
ices (counter/timers) used by these boards.
Spencer E. Olson (13):
staging: comedi: tests: add unittest framework for comedi
staging: comedi: add abstracted NI signal/terminal named constants
staging: comedi: add new device-global config interface
staging: comedi: ni_routing: Add NI signal routing info
staging: comedi: add interface to ni routing table information
staging: comedi: ni_mio_common: implement new routing for TRIG_EXT
staging: comedi: ni_mio_common: implement global pfi,rtsi routing
staging: comedi: ni_mio_common: implement output selection of
GPFO_{0,1}
staging: comedi: tio: implement global tio/ctr routing
staging: comedi: ni_mio_common: create device-global access to tio
staging: comedi: ni_660x: Add NI PCI-6608 to list of supported devices
staging: comedi: ni_660x: clean up pfi routing
staging: comedi: ni_660x: add device-global routing
It looks like a lot of effort has gone into this. I've added some
comments in my reply to your PATCH 04/13 regarding additional
constification of the data structures and other minor changes. This
will have knock on effects for the other patches and the maintenance
scripts.
The big thing I think people are going to complain about is the nested
inclusion of .c files (as opposed to linking separately compiled .c
files) so you may need to take another look at that.
I also noticed a lot of the new .c files define static inline functions.
We generally omit the 'inline' and let the compiler decide whether it
is worth inlining them or not (unless the code is especially
time-critical for some reason).
Best regards,
Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/4] Add facility to directly query subdevice timing
On 19/09/18 17:51, Spencer E. Olson wrote: This patchset adds a facility to directly query hardware speed limits of subdevices, in particular for scan_begin and convert signals. This information is generally already stored for many devices, such as analog input devices for NI hardware. This patchset makes this information available. This patchset also adds similar information for several digital devices. Spencer E. Olson (4): staging: comedi: add facility to directly query subdevice timing constraints staging: comedi: ni_mio_common: implement INSN_CONFIG_GET_CMD_TIMING_CONSTRAINTS staging: comedi: ni_pcidio: implement INSN_CONFIG_GET_CMD_TIMING_CONSTRAINTS staging: comedi: comedi_test: implement INSN_CONFIG_GET_CMD_TIMING_CONSTRAINTS drivers/staging/comedi/comedi.h | 5 ++- drivers/staging/comedi/comedi_fops.c | 4 ++ drivers/staging/comedi/drivers/comedi_test.c | 44 +++ .../staging/comedi/drivers/ni_mio_common.c| 24 ++ drivers/staging/comedi/drivers/ni_pcidio.c| 13 ++ drivers/staging/comedi/drivers/ni_pcimio.c| 21 + drivers/staging/comedi/drivers/ni_stc.h | 1 + 7 files changed, 111 insertions(+), 1 deletion(-) Those all look great, thanks! Reviewed-by: Ian Abbott -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/13] staging: comedi: ni_mio_common: implement global pfi,rtsi routing
On 19/09/18 17:38, Spencer E. Olson wrote:
Implement device-global config interface for ni_mio devices. In
particular, this patch implements:
INSN_DEVICE_CONFIG_TEST_ROUTE,
INSN_DEVICE_CONFIG_CONNECT_ROUTE,
INSN_DEVICE_CONFIG_DISCONNECT_ROUTE,
INSN_DEVICE_CONFIG_GET_ROUTES
for the ni mio devices. This means that the new abstracted signal/terminal
names can be used to define signal routing with regards to the PFI
terminals and RTSI trigger bus lines.
This also adds ability to identify PFI and RTSI channels on the PFI and
RTSI subdevices using the new device-global names. This does not change
the values that are set for channel output selections using the subdevice
interfaces--these still require direct register values.
Annotates and updates tables of register values to reflect this new
implementation status.
Signed-off-by: Spencer E. Olson
---
.../staging/comedi/drivers/ni_mio_common.c| 687 --
drivers/staging/comedi/drivers/ni_stc.h | 68 ++
2 files changed, 683 insertions(+), 72 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index b033f0be9b8a..c42d480df7ff 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
In trying an experimental application of your patches to "staging-next"
(to examine the code more carefully), this (PATCH 07/13) failed to apply
with "git am". When applying the failed patch manually with "patch
-p1", there was one failed hunk (see below).
@@ -5040,13 +5077,17 @@ static unsigned int ni_get_rtsi_routing(struct
comedi_device *dev,
{
struct ni_private *devpriv = dev->private;
+ if (chan >= TRIGGER_LINE(0))
+ /* allow new and old names of rtsi channels to work. */
+ chan -= TRIGGER_LINE(0);
+
if (chan < 4) {
return NISTC_RTSI_TRIG_TO_SRC(chan,
devpriv->rtsi_trig_a_output_reg);
} else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
return NISTC_RTSI_TRIG_TO_SRC(chan,
devpriv->rtsi_trig_b_output_reg);
- } else if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN)
+ } else if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
return NI_RTSI_OUTPUT_RTSI_OSC;
}
That is the hunk that failed to apply. The line numbers seem a bit off
the original source, and the "} else if" part at the end didn't match at
all.
Perhaps you need to rebase?
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/13] staging: comedi: add interface to ni routing table information
On 19/09/18 17:38, Spencer E. Olson wrote: Adds interface and associated unittests for accessing/looking-up/validating the new ni routing table information. Signed-off-by: Spencer E. Olson --- drivers/staging/comedi/Kconfig| 4 + drivers/staging/comedi/drivers/Makefile | 1 + drivers/staging/comedi/drivers/ni_routes.c| 523 +++ drivers/staging/comedi/drivers/ni_routes.h| 320 + drivers/staging/comedi/drivers/ni_stc.h | 4 + drivers/staging/comedi/drivers/tests/Makefile | 3 +- .../comedi/drivers/tests/ni_routes_test.c | 614 ++ 7 files changed, 1468 insertions(+), 1 deletion(-) create mode 100644 drivers/staging/comedi/drivers/ni_routes.c create mode 100644 drivers/staging/comedi/drivers/ni_routes.h create mode 100644 drivers/staging/comedi/drivers/tests/ni_routes_test.c diff --git a/drivers/staging/comedi/Kconfig b/drivers/staging/comedi/Kconfig index 583bce9bb18e..901503c3b279 100644 --- a/drivers/staging/comedi/Kconfig +++ b/drivers/staging/comedi/Kconfig @@ -1097,6 +1097,7 @@ config COMEDI_NI_TIOCMD depends on HAS_DMA select COMEDI_NI_TIO select COMEDI_MITE + select COMEDI_NI_ROUTES This 'select COMEDI_NI_ROUTES' may be in the wrong place. If you disable the COMEDI_NI_PCIMIO and COMEDI_NI_660X options, but enable the COMEDI_NI_ATMIO option (in the COMEDI_ISA_DRIVERS menu) and/or the COMEDI_NI_MIO_CS option (in the COMEDI_PCMCIA_DRIVERS menu) then the build breaks for the "ni_atmio" and/or "ni_mio_cs" modules. -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/13] staging: comedi: ni_routing: Add NI signal routing info
On 25/09/18 05:47, Spencer E. Olson wrote:
[N.B. top-posting is frowned upon on the kernel mailing lists.]
These static arrays are
 (1) not expressed with as much "const"ness as suggested
 (2) included into one compile unit
because
 - ni_device_routes.routes and ni_route_set.src are sorted at module
  load time using the kernel sort(...) routine.
 - ni_device_routes.n_route_sets and ni_route_set.n_src data
  members are changed as a result of the counting/sorting.
 - ni_device_routes.routes and ni_route_set.src are both searched at
  runtime using the kernel bsearch(...) routine.
These choices were made as a compromise between maintenance,
code-execution performance, and memory footprint. Rather than require a
large, mostly sparse table be kept for each ni hardware device, the
signal route information is divided up into one large table of register
values for each device family and smaller hardware-specific sorted lists
that can easily be searched to identify possible signal routes.
It seemed unreasonable to require a developer to maintain the proper
order of the structures to provide for best searching. It also seemed
unreasonable to require the developer to specifically instantiate the
ni_device_routes.n_route_sets and ni_route_set.n_src data members
correctly.
I never noticed that you sort the data on module load. You also have an
exported function 'ni_sort_device_routes()' called internally by
'ni_sort_all_device_routes()' when the "ni_routes" module is loaded. Do
you envision that function ever being called externally?
Your 'ni_sort_all_device_routes()' function iterates through
'device_routes_list[]' and calls 'ni_sort_device_routes()' on each of
the 'struct ni_device_routes *' values in the list. The list of
pointers itself should be 'const', but you have the 'const' in the wrong
place. You have it declared as:
static const struct ni_device_routes *device_routes_list[] = {
...
};
but I think you meant:
static struct ni_device_routes *const device_routes_list[] = {
...
};
i.e. the array of pointers is 'const', not the things they point to.
You work around that with a type cast to remove the 'const' in
'ni_sort_all_device_routes()'. That type cast should be avoided.
You also have type casts in your calls to 'sort()' and 'bsearch()' which
shouldn't be needed if you declare '_ni_sort_destcmp()',
'_ni_sort_srccmp()', '_ni_bsearch_destcmp()' and '_ni_bsearch_srccmp()'
properly.
The 'all_route_values[]' in "[...]/ni_routing/ni_route_values.c" can
still be made 'const':
static const struct family_route_values *const all_route_values[] = {
^
Because these arrays are sorted (at module load time) by ni_routes, it
seemed best to have the symbols for these tables only have static
linkage, thus ensuring that _only_ the ni_routes module accesses these.
I still think external linkage rather than .c file inclusion should be
doable. It shouldn't be that hard to link the "ni_routes" module from
several object files, and the consequent inability to use 'ARRAY_SIZE()'
on 'device_routes_list[]' and 'all_route_values[]' can be worked around
either by NULL terminating them or by storing their lengths in separate
variables.
If the likes of 'ni_660x_route_values' are declared with external
linkage, only "ni_routes" would be able to link to them if it is built
as a loadable module. If "ni_routes" is built into the kernel image
itself, then other code built into the kernel image would also be able
to link to them, but such code should be subject to review anyway.
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/13] staging: comedi: ni_mio_common: implement global pfi,rtsi routing
On 26/09/18 00:35, Spencer Olson wrote:
On Tue, Sep 25, 2018 at 4:27 AM Ian Abbott wrote:
On 19/09/18 17:38, Spencer E. Olson wrote:
Implement device-global config interface for ni_mio devices. In
particular, this patch implements:
INSN_DEVICE_CONFIG_TEST_ROUTE,
INSN_DEVICE_CONFIG_CONNECT_ROUTE,
INSN_DEVICE_CONFIG_DISCONNECT_ROUTE,
INSN_DEVICE_CONFIG_GET_ROUTES
for the ni mio devices. This means that the new abstracted signal/terminal
names can be used to define signal routing with regards to the PFI
terminals and RTSI trigger bus lines.
This also adds ability to identify PFI and RTSI channels on the PFI and
RTSI subdevices using the new device-global names. This does not change
the values that are set for channel output selections using the subdevice
interfaces--these still require direct register values.
Annotates and updates tables of register values to reflect this new
implementation status.
Signed-off-by: Spencer E. Olson
---
.../staging/comedi/drivers/ni_mio_common.c| 687 --
drivers/staging/comedi/drivers/ni_stc.h | 68 ++
2 files changed, 683 insertions(+), 72 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index b033f0be9b8a..c42d480df7ff 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
In trying an experimental application of your patches to "staging-next"
(to examine the code more carefully), this (PATCH 07/13) failed to apply
with "git am". When applying the failed patch manually with "patch
-p1", there was one failed hunk (see below).
@@ -5040,13 +5077,17 @@ static unsigned int ni_get_rtsi_routing(struct
comedi_device *dev,
{
struct ni_private *devpriv = dev->private;
+ if (chan >= TRIGGER_LINE(0))
+ /* allow new and old names of rtsi channels to work. */
+ chan -= TRIGGER_LINE(0);
+
if (chan < 4) {
return NISTC_RTSI_TRIG_TO_SRC(chan,
devpriv->rtsi_trig_a_output_reg);
} else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
return NISTC_RTSI_TRIG_TO_SRC(chan,
devpriv->rtsi_trig_b_output_reg);
- } else if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN)
+ } else if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
return NI_RTSI_OUTPUT_RTSI_OSC;
}
That is the hunk that failed to apply. The line numbers seem a bit off
the original source, and the "} else if" part at the end didn't match at
all.
Perhaps you need to rebase?
After looking at this patch, it looks like this is based on my earlier
patch that you "stamped" reviewed. The patch was titled: "staging:
comedi: ni_mio_common: protect register write overflow". Probably
should have included that one in this patch set or waited for more
time between--oh well. Any suggestion on how to handle this?
Yes, I'd forgotten about that at the time and came to the same
conclusion. It's probably not worth worrying about, as the other patch
will hopefully have been applied by Greg before you submit your next
round of patches. But if you send more than one patch series with some
dependency between them, you can note it in the patch series
introductory message, and preferably add a note after the "scissors"
line of the dependent patch.
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 05/13] staging: comedi: add interface to ni routing table information
On 26/09/18 04:47, Spencer Olson wrote: How about making it selected based on COMEDI_NI_TIO? This will impact all the *mio* (except ATMIO16D) and 660x drivers. This seems to be everything that fits into the e-series, m-series, and 660x series devices for which we know the register values. It also doesn't look like anything else depends on COMEDI_NI_TIO. That ought to work. And please don't top-post. :) -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/13] staging: comedi: ni_routing: Add NI signal routing info
On 26/09/18 02:49, Spencer Olson wrote: On Tue, Sep 25, 2018 at 6:26 AM Ian Abbott wrote: I never noticed that you sort the data on module load. You also have an exported function 'ni_sort_device_routes()' called internally by 'ni_sort_all_device_routes()' when the "ni_routes" module is loaded. Do you envision that function ever being called externally? I struggled with this one a tiny bit. The main reason why this is exported is so that I could use it inside one of the unit tests in the tests/ni_routes_test module (these were indeed invaluable for testing on a system that didn't have real hardware attached). Otherwise, I do not really imagine this being used externally. Since this function is fully self-contained (i.e. only touches local variables and no module variables), I figured that this export was at least safe. That explains it, thanks. I still think external linkage rather than .c file inclusion should be doable. It shouldn't be that hard to link the "ni_routes" module from several object files, and the consequent inability to use 'ARRAY_SIZE()' on 'device_routes_list[]' and 'all_route_values[]' can be worked around either by NULL terminating them or by storing their lengths in separate variables. If the likes of 'ni_660x_route_values' are declared with external linkage, only "ni_routes" would be able to link to them if it is built as a loadable module. If "ni_routes" is built into the kernel image itself, then other code built into the kernel image would also be able to link to them, but such code should be subject to review anyway. In my working copy, I now have the ni_routing module that is comprised of: ni_routes.o ni_routing/ni_route_values.o ni_routing/ni_route_values/ni_660x.o ni_routing/ni_route_values/ni_eseries.o ni_routing/ni_route_values/ni_mseries.o ni_routing/ni_device_routes.o (i.e. I've only broken out the components of what was originally included in ni_routing/ni_route_values.c so far) I am not very satisfied with the result--with respect to maintenance. For the ni_route_values*, this is not too much of a deal since there are not that many hardware families. For ni_device_routes*, this seems a bit much, since each new hardware device implies - a new compile unit in the Makefile ni_routing module - two new lines in ni_routing/ni_device_routes.c (extern declaration & addition to array) - and a new file for the device route list in sub-directory ni_routing/ Currently, I have ni_route_values.c declaring each of the arrays from ni_routing/ni_route_values/ni_*.o, rather than going overboard and making .h files for these to include in ni_route_values.c. If I also break out the arrays in ni_routing/ni_device_routes/*.c, I would make this approach to avoid a fourth implication for maintenance. Breaking out the ni_routing/ni_device_routes/*.c content into separate compile units results in 18 more compile units that are linked into ni_routing.ko and 18 additional lines in ni_device_routes. I'm willing to be encouraged one way or another, but would like to see your feedback/thoughts on the maintenance aspect. You could perhaps lump all the external declarations of the individual 'struct ni_device_routes' and 'struct family_routes_values' variables from the individual .c files into a couple of shared header files. It doesn't matter if the header file declares external variables that are not actually referenced by a particular compilation unit. When adding a new .c file in ni_routing/ni_device_routes/ or ni_routing/ni_route_values/, that would also need a single line addition to a shared .h file, a single line addition to ni_routing/ni_device_routes.c or ni_routing/ni_route_values.c, and a change to list of objects in the Makefile used to build ni_routing/ni_device_routes.o or ni_routing/ni_route_values.o. -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 05/13] staging: comedi: add interface to ni routing table information
On 27/09/18 20:27, Spencer E. Olson wrote:
Adds interface and associated unittests for accessing/looking-up/validating
the new ni routing table information.
Signed-off-by: Spencer E. Olson
---
Changes since last submission:
- [PATCH v2 05/13]: Tweak Makefile to build routing info for newly added
hardware in updates to [PATCH v2 04/13].
- [PATCH v2 05/13]: Fixes placement of "select COMEDI_NI_ROUTING" as per
Ian's
suggestion.
- [PATCH v2 05/13]: Removes a few inline function declarations in unit test.
drivers/staging/comedi/Kconfig| 4 +
drivers/staging/comedi/drivers/Makefile | 27 +
drivers/staging/comedi/drivers/ni_routes.c| 521 +++
drivers/staging/comedi/drivers/ni_routes.h| 329 ++
drivers/staging/comedi/drivers/ni_stc.h | 4 +
drivers/staging/comedi/drivers/tests/Makefile | 3 +-
.../comedi/drivers/tests/ni_routes_test.c | 613 ++
7 files changed, 1500 insertions(+), 1 deletion(-)
create mode 100644 drivers/staging/comedi/drivers/ni_routes.c
create mode 100644 drivers/staging/comedi/drivers/ni_routes.h
create mode 100644 drivers/staging/comedi/drivers/tests/ni_routes_test.c
[snip]
diff --git a/drivers/staging/comedi/drivers/ni_routes.c
b/drivers/staging/comedi/drivers/ni_routes.c
new file mode 100644
index ..9e999bc4f3c4
--- /dev/null
+++ b/drivers/staging/comedi/drivers/ni_routes.c
[snip]
+/* BEGIN Routes search routines */
+static int _ni_bsearch_destcmp(const int *key, const struct ni_route_set *elt)
+{
+ if (*key < elt->dest)
+ return -1;
+ else if (*key > elt->dest)
+ return 1;
+ return 0;
+}
+
+static int _ni_bsearch_srccmp(const int *key, const int *elt)
+{
+ if (*key < *elt)
+ return -1;
+ else if (*key > *elt)
+ return 1;
+ return 0;
+}
Can those be changed to use 'const void *' parameters, like you did for
for the 'sort' callbacks?
+
+/**
+ * ni_find_route_set() - Finds the proper route set with the specified
+ * destination.
+ * @destination: Destination of which to search for the route set.
+ * @valid_routes: Pointer to device routes within which to search.
+ *
+ * Return: NULL if no route_set is found with the specified @destination;
+ * otherwise, a pointer to the route_set if found.
+ */
+const struct ni_route_set *
+ni_find_route_set(const int destination,
+ const struct ni_device_routes *valid_routes)
+{
+ typedef int (*cmp_ftype)(const void *, const void *);
+
+ return bsearch(&destination, valid_routes->routes,
+ valid_routes->n_route_sets, sizeof(struct ni_route_set),
+ (cmp_ftype)_ni_bsearch_destcmp);
+}
+EXPORT_SYMBOL_GPL(ni_find_route_set);
+
+/**
+ * ni_route_set_has_source() - Determines whether the given source is in
+ *included given route_set.
+ *
+ * Return: true if found; false otherwise.
+ */
+bool ni_route_set_has_source(const struct ni_route_set *routes,
+const int source)
+{
+ typedef int (*cmp_ftype)(const void *, const void *);
+
+ if (!bsearch(&source, routes->src, routes->n_src, sizeof(int),
+(cmp_ftype)_ni_bsearch_srccmp))
+ return false;
+ return true;
+}
+EXPORT_SYMBOL_GPL(ni_route_set_has_source);
Then you wouldn't need those nasty '(cmp_ftype)' type casts.
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 04/13] staging: comedi: ni_routing: Add NI signal routing info
On 27/09/18 20:27, Spencer E. Olson wrote:
See README for a thorough discussion of this content.
Adds tables of all register values for routing various signals to various
terminals on National Instruments hardware. This information is directly
compared to and taken from register-level programming documentation and/or
register-level programming examples as provided by National Instruments.
Furthermore, this information was mostly compared (favorably) to the
register values already used in the comedi drivers for NI hardware.
Adds tables of valid routes for many devices. This information is not
consistent from device to device, nor entirely consistent within device
families. One additional major challenge is that this information does not
seem to be obtainable in any programmatic fashion, neither through the
proprietary NIDAQmx(-base) c-libraries, nor with register level
programming, _nor_ through any documentation. In fact, the only consistent
source of this information is through the proprietary NI-MAX software,
which currently only runs on Windows platforms. A further challenge is
that this information cannot be exported from NI-MAX, except by screenshot.
The collection and maintenance of this information is somewhat tedious and
requires frequent re-examination and comparison of NI-MAX and/or the
NI-MHDDK documentation (register programming information) and NI-MHDDK
examples. Tools are added with this patch to facilitate generating CSV
files from the data tables. These CSV files can be used with a spreadsheet
program to provide better visual comparision with screenshots gathered from
NI-MAX. Tools are also added to regenerate the data tables from CSV
content--this greatly enhances updating data tables with large changes
(such as when adding devices).
Signed-off-by: Spencer E. Olson
---
Changes since last submission:
- [PATCH v2 04/13]: Add routing information for PXIe-6535 and PXIe-6738
devices.
- [PATCH v2 04/13]: Implements Ian's suggestion to break up components of new
ni_routing module into multiple compile units so that .c files are not
included from .c files.
- [PATCH v2 04/13]: Fixes various function prototypes and "const" variable
declarations as per Ian's suggestions.
I'm not bothered by some of the lines being slightly over 80 columns in
the auto-generated C code (it would be a real pain for the Python
scripts to avoid that!). And the CamelCase stuff has been explained.
[snip]
diff --git a/drivers/staging/comedi/drivers/ni_routing/tools/convert_c_to_py.c
b/drivers/staging/comedi/drivers/ni_routing/tools/convert_c_to_py.c
new file mode 100644
index ..5952aba91953
--- /dev/null
+++ b/drivers/staging/comedi/drivers/ni_routing/tools/convert_c_to_py.c
@@ -0,0 +1,159 @@
[snip]
+void family_write(const struct family_route_values *rv, FILE *fp)
+{
+ fprintf(fp,
+ " \"%s\" : {\n"
+ "# dest -> {src0:val0, src1:val1, ...}\n"
+ , rv->family);
+ for (unsigned int dest = NI_NAMES_BASE;
+dest < (NI_NAMES_BASE + NI_NUM_NAMES);
+++dest) {
+ unsigned int src = NI_NAMES_BASE;
+
+ for (; src < (NI_NAMES_BASE + NI_NUM_NAMES) &&
+RVij(rv, B(src), B(dest)) == 0; ++src)
+ ;
checkpatch.pl complains here about "suspect code indent for conditional
statements". That semi-colon needs indenting.
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 00/13] device-global identifiers and routes introduced
On 27/09/18 20:27, Spencer E. Olson wrote: This patch set is the second revision of a recent patch set of the same name. Changes and notes: - [PATCH v2 02/13]: Update signal/terminal names found after adding additional devices to routing list in [PATCH v2 04/13]. - [PATCH v2 04/13]: Add routing information for PXIe-6535 and PXIe-6738 devices. - [PATCH v2 04/13]: Implements Ian's suggestion to break up components of new ni_routing module into multiple compile units so that .c files are not included from .c files. - [PATCH v2 04/13]: Fixes various function prototypes and "const" variable declarations as per Ian's suggestions. - [PATCH v2 05/13]: Tweak Makefile to build routing info for newly added hardware in updates to [PATCH v2 04/13]. - [PATCH v2 05/13]: Fixes placement of "select COMEDI_NI_ROUTING" to ensure ni_routing module is enabled for all dependent modules. - [PATCH v2 05/13]: Removes a few inline function declarations in unit test. - [PATCH v2 07/13]: This patch must be built upon an earlier patch recently submitted and in the queue for integration: "staging: comedi: ni_mio_common: protect register write overflow" I noted a minor line indentation problem detected by checkpatch.pl in my reply to patch 04. (There are various other checkpatch.pl 'CHECK' level issues that I'm not that bothered about.) I also noted a cleanup for the 'bsearch' callbacks in my reply to patch 05. Overall, I think it's very close to acceptable (from a commit point of view I mean). -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 04/13] staging: comedi: ni_routing: Add NI signal routing info
On 02/10/18 03:24, Spencer E. Olson wrote: See README for a thorough discussion of this content. Adds tables of all register values for routing various signals to various terminals on National Instruments hardware. This information is directly compared to and taken from register-level programming documentation and/or register-level programming examples as provided by National Instruments. Furthermore, this information was mostly compared (favorably) to the register values already used in the comedi drivers for NI hardware. Adds tables of valid routes for many devices. This information is not consistent from device to device, nor entirely consistent within device families. One additional major challenge is that this information does not seem to be obtainable in any programmatic fashion, neither through the proprietary NIDAQmx(-base) c-libraries, nor with register level programming, _nor_ through any documentation. In fact, the only consistent source of this information is through the proprietary NI-MAX software, which currently only runs on Windows platforms. A further challenge is that this information cannot be exported from NI-MAX, except by screenshot. The collection and maintenance of this information is somewhat tedious and requires frequent re-examination and comparison of NI-MAX and/or the NI-MHDDK documentation (register programming information) and NI-MHDDK examples. Tools are added with this patch to facilitate generating CSV files from the data tables. These CSV files can be used with a spreadsheet program to provide better visual comparision with screenshots gathered from NI-MAX. Tools are also added to regenerate the data tables from CSV content--this greatly enhances updating data tables with large changes (such as when adding devices). Signed-off-by: Spencer E. Olson --- Patch revisions: - [PATCH v3 04/13]: Minor update in indentation for support tool. - [PATCH v2 04/13]: Add routing information for PXIe-6535 and PXIe-6738 devices. - [PATCH v2 04/13]: Implements Ian's suggestion to break up components of new ni_routing module into multiple compile units so that .c files are not included from .c files. - [PATCH v2 04/13]: Fixes various function prototypes and "const" variable declarations as per Ian's suggestions. I'm not sure if this is a glitch in my email copy of the patch, but I got a "trailing whitespace error" on one line when applying this patch with "git am": Applying: staging: comedi: ni_routing: Add NI signal routing info .git/rebase-apply/patch:8112: trailing whitespace. NI_CtrGate(0), warning: 1 line adds whitespace errors. When looking at line 8212 of "drivers/staging/comedi/drivers/ni_routing/ni_device_routes/pci-6254.c", the line was terminated by CRLF instead of LF (which also caused checkpath.pl to complain about "DOS line endings"). It wasn't there in the previous patch series, and there is no reason for it to have appeared in this patch series, which is why I'm suspecting an email receiving glitch at my end. -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 00/13] device-global identifiers and routes introduced
l sources and
destinations using a consistent naming scheme, global to a driver family.
2) For NI devices, use terminal/signal naming that is consistent with (a) the
NI's user level documentation, (b) NI's user-level code, (c) the
information
as provided by the proprietary NI-MAX software, and (d) the user interface
code provided by the user-land comedilib library.
3) Make for easy maintenance of register level values that are to be used for
any particular NI device of any particular NI device family.
4) Provide a means whereby the user can query the set of signal routes that
is
valid for a particular device.
5) Provide an interface whereby the user can query the status and capability
of any signal route. The driver can provide information on whether the
route is valid for the device and whether the route is already connected.
This patch set implements various changes that keep the goals set forth here.
This patch set is in nowise complete with respect to the various NI hardware
options supported by comedi, though a large selection should be supported--all
e/m-series (ni_mio_common.c hardware) boards and 660x boards are the target of
this patch set, including the tio devices (counter/timers) used by these boards.
Spencer E. Olson (13):
staging: comedi: tests: add unittest framework for comedi
staging: comedi: add abstracted NI signal/terminal named constants
staging: comedi: add new device-global config interface
staging: comedi: ni_routing: Add NI signal routing info
staging: comedi: add interface to ni routing table information
staging: comedi: ni_mio_common: implement new routing for TRIG_EXT
staging: comedi: ni_mio_common: implement global pfi,rtsi routing
staging: comedi: ni_mio_common: implement output selection of
GPFO_{0,1}
staging: comedi: tio: implement global tio/ctr routing
staging: comedi: ni_mio_common: create device-global access to tio
staging: comedi: ni_660x: Add NI PCI-6608 to list of supported devices
staging: comedi: ni_660x: clean up pfi routing
staging: comedi: ni_660x: add device-global routing
I got a whitespace error when applying patch 04, which might be due to a
glitch in my received email copy - some random line ended up with an
extra carriage return (see my reply to patch 04). Apart from that I'm
happy with it. Thanks, Spencer!
Reviewed-by: Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_mio_common: protect register write overflow
On 03/10/18 02:24, Spencer Olson wrote:
On Tue, Oct 2, 2018 at 6:16 PM Spencer Olson wrote:
On Tue, Oct 2, 2018 at 11:32 AM Greg Kroah-Hartman
wrote:
On Wed, Sep 19, 2018 at 10:17:19AM -0600, Spencer E. Olson wrote:
[snip]
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 4dee2fc37aed..4d0d0621780e 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -4980,7 +4980,10 @@ static int ni_valid_rtsi_output_source(struct
comedi_device *dev,
case NI_RTSI_OUTPUT_G_SRC0:
case NI_RTSI_OUTPUT_G_GATE0:
case NI_RTSI_OUTPUT_RGOUT0:
- case NI_RTSI_OUTPUT_RTSI_BRD_0:
+ case NI_RTSI_OUTPUT_RTSI_BRD(0):
+ case NI_RTSI_OUTPUT_RTSI_BRD(1):
+ case NI_RTSI_OUTPUT_RTSI_BRD(2):
+ case NI_RTSI_OUTPUT_RTSI_BRD(3):
return 1;
case NI_RTSI_OUTPUT_RTSI_OSC:
return (devpriv->is_m_series) ? 1 : 0;
@@ -5001,11 +5004,18 @@ static int ni_set_rtsi_routing(struct comedi_device
*dev,
devpriv->rtsi_trig_a_output_reg |= NISTC_RTSI_TRIG(chan, src);
ni_stc_writew(dev, devpriv->rtsi_trig_a_output_reg,
NISTC_RTSI_TRIGA_OUT_REG);
- } else if (chan < 8) {
+ } else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
devpriv->rtsi_trig_b_output_reg &= ~NISTC_RTSI_TRIG_MASK(chan);
devpriv->rtsi_trig_b_output_reg |= NISTC_RTSI_TRIG(chan, src);
ni_stc_writew(dev, devpriv->rtsi_trig_b_output_reg,
NISTC_RTSI_TRIGB_OUT_REG);
+ } else if (chan != NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
+ /* probably should never reach this, since the
+ * ni_valid_rtsi_output_source above errors out if chan is too
+ * high
+ */
While you're fixing it, could that be changed to the usual block comment
format?
+ dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);
This patch breaks the build very badly. Always test-build your patches
at the very least :(
greg k-h
I have been test building this with at least a fairly recent
staging-next rebase (rebase a week or two ago). I'll rebase again and
check this out
So the problem had been that I had been compiling the entire time with
my other patch set that I recently have just submitted. When I split
this patch off from that patch set, I had neglected to compile with it
by itsself. The issue was a forgotten "{" at the beginning of the
last if statement.
Should I resubmit this patch and the entire patch set from earlier
today, each separately?
The patch set from today titled "device-global identifiers and routes
introduced" _does_ depend on this patch that was missing the
brace--this is indicated in the patch notes as requested by Ian.
Hmm yes, it appears patch 07/13 fixes this one, as well as depending on it!
Personally, I'd be inclined to combine them into a single series of 14
patches.
I did just check to make sure that I had not made the same mistake on
the other patch set submitted earlier that was titled: "Add facility
to directly query subdevice timing". That patch set is fine as is and
did not depend on any of the other patches I had been working on.
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_mio_common: protect register write overflow
On 03/10/2018 11:34, Ian Abbott wrote: On 03/10/18 02:24, Spencer Olson wrote: Should I resubmit this patch and the entire patch set from earlier today, each separately? The patch set from today titled "device-global identifiers and routes introduced" _does_ depend on this patch that was missing the brace--this is indicated in the patch notes as requested by Ian. Hmm yes, it appears patch 07/13 fixes this one, as well as depending on it! Personally, I'd be inclined to combine them into a single series of 14 patches. But do what Greg wants (separate series). :) -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: comedi: ni_mio_common: protect register write overflow
On 03/10/18 21:54, Spencer E. Olson wrote:
Fixes two problems introduced as early as
commit 03aef4b6dc12 ("Staging: comedi: add ni_mio_common code"):
(1) Ensures that the last four bits of NISTC_RTSI_TRIGB_OUT_REG register is
not unduly overwritten on e-series devices. On e-series devices, the
first three of the last four bits are reserved. The last bit defines
the output selection of the RGOUT0 pin, otherwise known as
RTSI_Sub_Selection. For m-series devices, these last four bits are
indeed used as the output selection of the RTSI7 pin (and the
RTSI_Sub_Selection bit for the RGOUT0 pin is moved to the
RTSI_Trig_Direction register.
(2) Allows all 4 RTSI_BRD lines to be treated as valid sources for RTSI
lines.
This patch also cleans up the ni_get_rtsi_routing command for readability.
Fixes: 03aef4b6dc12 ("Staging: comedi: add ni_mio_common code")
Signed-off-by: Spencer E. Olson
---
Changes:
- [PATCH v2]: Adds missing brace that had been corrected in a later patchset
but should have been correct here. This patch is now independent and
complete.
.../staging/comedi/drivers/ni_mio_common.c| 24 +--
1 file changed, 17 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 4dee2fc37aed..44fcb3790113 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -4980,7 +4980,10 @@ static int ni_valid_rtsi_output_source(struct
comedi_device *dev,
case NI_RTSI_OUTPUT_G_SRC0:
case NI_RTSI_OUTPUT_G_GATE0:
case NI_RTSI_OUTPUT_RGOUT0:
- case NI_RTSI_OUTPUT_RTSI_BRD_0:
+ case NI_RTSI_OUTPUT_RTSI_BRD(0):
+ case NI_RTSI_OUTPUT_RTSI_BRD(1):
+ case NI_RTSI_OUTPUT_RTSI_BRD(2):
+ case NI_RTSI_OUTPUT_RTSI_BRD(3):
return 1;
case NI_RTSI_OUTPUT_RTSI_OSC:
return (devpriv->is_m_series) ? 1 : 0;
@@ -5001,11 +5004,18 @@ static int ni_set_rtsi_routing(struct comedi_device
*dev,
devpriv->rtsi_trig_a_output_reg |= NISTC_RTSI_TRIG(chan, src);
ni_stc_writew(dev, devpriv->rtsi_trig_a_output_reg,
NISTC_RTSI_TRIGA_OUT_REG);
- } else if (chan < 8) {
+ } else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
devpriv->rtsi_trig_b_output_reg &= ~NISTC_RTSI_TRIG_MASK(chan);
devpriv->rtsi_trig_b_output_reg |= NISTC_RTSI_TRIG(chan, src);
ni_stc_writew(dev, devpriv->rtsi_trig_b_output_reg,
NISTC_RTSI_TRIGB_OUT_REG);
+ } else if (chan != NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
+ /* probably should never reach this, since the
+* ni_valid_rtsi_output_source above errors out if chan is too
+* high
+*/
+ dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);
+ return -EINVAL;
}
return 2;
}
@@ -5021,12 +5031,12 @@ static unsigned int ni_get_rtsi_routing(struct
comedi_device *dev,
} else if (chan < NISTC_RTSI_TRIG_NUM_CHAN(devpriv->is_m_series)) {
return NISTC_RTSI_TRIG_TO_SRC(chan,
devpriv->rtsi_trig_b_output_reg);
- } else {
- if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN)
- return NI_RTSI_OUTPUT_RTSI_OSC;
- dev_err(dev->class_dev, "bug! should never get here?\n");
- return 0;
+ } else if (chan == NISTC_RTSI_TRIG_OLD_CLK_CHAN) {
+ return NI_RTSI_OUTPUT_RTSI_OSC;
}
+
+ dev_err(dev->class_dev, "%s: unknown rtsi channel\n", __func__);
+ return -EINVAL;
I'm not really sure what it should return for an invalid channel, since
this value gets written to the data[1] of the INSN_CONFIG_GET_ROUTING
instruction which doesn't return an error.
Perhaps it would be better if INSN_CONFIG_GET_ROUTING returned an error
for an out of range channel, but that's a change for another day.
}
static int ni_rtsi_insn_config(struct comedi_device *dev,
Reviewed-by: Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 00/13] device-global identifiers and routes introduced
hese patches are:
0) Allow current code to function as is, providing backwards compatibility to
the current interface, following a suggestion by Eric Piel.
1) Provide an interface to connect routes or identify signal sources and
destinations using a consistent naming scheme, global to a driver family.
2) For NI devices, use terminal/signal naming that is consistent with (a) the
NI's user level documentation, (b) NI's user-level code, (c) the
information
as provided by the proprietary NI-MAX software, and (d) the user interface
code provided by the user-land comedilib library.
3) Make for easy maintenance of register level values that are to be used for
any particular NI device of any particular NI device family.
4) Provide a means whereby the user can query the set of signal routes that
is
valid for a particular device.
5) Provide an interface whereby the user can query the status and capability
of any signal route. The driver can provide information on whether the
route is valid for the device and whether the route is already connected.
This patch set implements various changes that keep the goals set forth here.
This patch set is in nowise complete with respect to the various NI hardware
options supported by comedi, though a large selection should be supported--all
e/m-series (ni_mio_common.c hardware) boards and 660x boards are the target of
this patch set, including the tio devices (counter/timers) used by these boards.
Spencer E. Olson (13):
staging: comedi: tests: add unittest framework for comedi
staging: comedi: add abstracted NI signal/terminal named constants
staging: comedi: add new device-global config interface
staging: comedi: ni_routing: Add NI signal routing info
staging: comedi: add interface to ni routing table information
staging: comedi: ni_mio_common: implement new routing for TRIG_EXT
staging: comedi: ni_mio_common: implement global pfi,rtsi routing
staging: comedi: ni_mio_common: implement output selection of
GPFO_{0,1}
staging: comedi: tio: implement global tio/ctr routing
staging: comedi: ni_mio_common: create device-global access to tio
staging: comedi: ni_660x: Add NI PCI-6608 to list of supported devices
staging: comedi: ni_660x: clean up pfi routing
staging: comedi: ni_660x: add device-global routing
Thanks for the new functionality.
Reviewed-by: Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: tio: fix multiple missing break in switch bugs
On 11/10/2018 20:05, Gustavo A. R. Silva wrote:
Currently, there are multiple missing break statements in two switch code
blocks. This makes the execution path to fall all the way down through
to the default cases, which makes the function ni_tio_set_gate_src() to
always return -EINVAL.
Fix this by adding the missing break statements.
Also, notice that due to the absence of the break statements,
the following pieces of code are unreachable:
1078if (ret)
1079return ret;
1080/* 3. reenable & set mode to starts things back up */
1081ni_tio_set_gate_mode(counter, src);
1098if (ret)
1099return ret;
1100/* 3. reenable & set mode to starts things back up */
1101ni_tio_set_gate2_mode(counter, src);
So, by adding the missing breaks, this patch also fixes the problem
above.
Addresses-Coverity-ID: 1474165 ("Missing break in switch")
Addresses-Coverity-ID: 1474162 ("Structurally dead code")
Fixes: 347e244884c3 ("staging: comedi: tio: implement global tio/ctr routing")
Signed-off-by: Gustavo A. R. Silva
---
drivers/staging/comedi/drivers/ni_tio.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/staging/comedi/drivers/ni_tio.c
b/drivers/staging/comedi/drivers/ni_tio.c
index 838614e..0eb388c 100644
--- a/drivers/staging/comedi/drivers/ni_tio.c
+++ b/drivers/staging/comedi/drivers/ni_tio.c
@@ -1070,8 +1070,10 @@ int ni_tio_set_gate_src(struct ni_gpct *counter,
case ni_gpct_variant_e_series:
case ni_gpct_variant_m_series:
ret = ni_m_set_gate(counter, chan);
+ break;
case ni_gpct_variant_660x:
ret = ni_660x_set_gate(counter, chan);
+ break;
default:
return -EINVAL;
}
@@ -1090,8 +1092,10 @@ int ni_tio_set_gate_src(struct ni_gpct *counter,
switch (counter_dev->variant) {
case ni_gpct_variant_m_series:
ret = ni_m_set_gate2(counter, chan);
+ break;
case ni_gpct_variant_660x:
ret = ni_660x_set_gate2(counter, chan);
+ break;
default:
return -EINVAL;
}
Thanks for catching that bug!
Reviewed-by: Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: clarify/unify macros for NI macro-defined terminals
On 24/10/18 15:33, Spencer E. Olson wrote:
Uses a single macro to define multiple macros that represent a series of
terminals for NI devices. This patch also redefines NI_MAX_COUNTERS as the
maximum number of counters possible on NI devices (instead of the maximum
index of the counters). This was a little confusing and caused a bug in
commit 347e244884c3b ("staging: comedi: tio: implement global tio/ctr routing")
when setting/reading registers for counter terminals.
Fixes: 347e244884c3b ("staging: comedi: tio: implement global tio/ctr routing")
Signed-off-by: Spencer E. Olson
---
drivers/staging/comedi/comedi.h | 39 ++---
1 file changed, 21 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
index e90b17775284..09a940066c0e 100644
--- a/drivers/staging/comedi/comedi.h
+++ b/drivers/staging/comedi/comedi.h
@@ -1005,35 +1005,38 @@ enum i8254_mode {
* and INSN_DEVICE_CONFIG_GET_ROUTES.
*/
#define NI_NAMES_BASE 0x8000u
+
+#define _TERM_N(base, n, x)((base) + ((x) & ((n) - 1)))
+
/*
* not necessarily all allowed 64 PFIs are valid--certainly not for all
devices
*/
-#define NI_PFI(x) (NI_NAMES_BASE+ ((x) & 0x3f))
+#define NI_PFI(x) _TERM_N(NI_NAMES_BASE, 64, x)
/* 8 trigger lines by standard, Some devices cannot talk to all eight. */
-#define TRIGGER_LINE(x)(NI_PFI(-1) + 1 + ((x) & 0x7))
+#define TRIGGER_LINE(x)_TERM_N(NI_PFI(-1) + 1, 8, x)
/* 4 RTSI shared MUXes to route signals to/from TRIGGER_LINES on NI hardware
*/
-#define NI_RTSI_BRD(x) (TRIGGER_LINE(-1) + 1 + ((x) & 0x3))
+#define NI_RTSI_BRD(x) _TERM_N(TRIGGER_LINE(-1) + 1, 4, x)
/* *** Counter/timer names : 8 counters max *** */
-#define NI_COUNTER_NAMES_BASE (NI_RTSI_BRD(-1) + 1)
-#define NI_MAX_COUNTERS 7
-#define NI_CtrSource(x) (NI_COUNTER_NAMES_BASE + ((x) &
NI_MAX_COUNTERS))
+#define NI_MAX_COUNTERS8
+#define NI_COUNTER_NAMES_BASE (NI_RTSI_BRD(-1) + 1)
+#define NI_CtrSource(x) _TERM_N(NI_COUNTER_NAMES_BASE,
NI_MAX_COUNTERS, x)
/* Gate, Aux, A,B,Z are all treated, at times as gates */
-#define NI_GATES_NAMES_BASE(NI_CtrSource(-1) + 1)
-#define NI_CtrGate(x) (NI_GATES_NAMES_BASE + ((x) & NI_MAX_COUNTERS))
-#define NI_CtrAux(x) (NI_CtrGate(-1) + 1 + ((x) & NI_MAX_COUNTERS))
-#define NI_CtrA(x)(NI_CtrAux(-1)+ 1 + ((x) & NI_MAX_COUNTERS))
-#define NI_CtrB(x)(NI_CtrA(-1) + 1 + ((x) & NI_MAX_COUNTERS))
-#define NI_CtrZ(x)(NI_CtrB(-1) + 1 + ((x) & NI_MAX_COUNTERS))
-#define NI_GATES_NAMES_MAX NI_CtrZ(-1)
-#define NI_CtrArmStartTrigger(x) (NI_CtrZ(-1)+ 1 + ((x) &
NI_MAX_COUNTERS))
+#define NI_GATES_NAMES_BASE(NI_CtrSource(-1) + 1)
+#define NI_CtrGate(x) _TERM_N(NI_GATES_NAMES_BASE, NI_MAX_COUNTERS, x)
+#define NI_CtrAux(x) _TERM_N(NI_CtrGate(-1) + 1, NI_MAX_COUNTERS, x)
+#define NI_CtrA(x) _TERM_N(NI_CtrAux(-1) + 1, NI_MAX_COUNTERS, x)
+#define NI_CtrB(x) _TERM_N(NI_CtrA(-1) + 1, NI_MAX_COUNTERS, x)
+#define NI_CtrZ(x) _TERM_N(NI_CtrB(-1) + 1, NI_MAX_COUNTERS, x)
+#define NI_GATES_NAMES_MAX NI_CtrZ(-1)
+#define NI_CtrArmStartTrigger(x) _TERM_N(NI_CtrZ(-1)+ 1, NI_MAX_COUNTERS,
x)
#define NI_CtrInternalOutput(x) \
-(NI_CtrArmStartTrigger(-1) + 1 + ((x) & NI_MAX_COUNTERS))
+ _TERM_N(NI_CtrArmStartTrigger(-1) + 1, NI_MAX_COUNTERS, x)
/** external pin(s) labeled conveniently as CtrOut. */
-#define NI_CtrOut(x) (NI_CtrInternalOutput(-1) + 1 + ((x) &
NI_MAX_COUNTERS))
+#define NI_CtrOut(x) _TERM_N(NI_CtrInternalOutput(-1) + 1, NI_MAX_COUNTERS,
x)
/** For Buffered sampling of ctr -- x series capability. */
-#define NI_CtrSampleClock(x) (NI_CtrOut(-1) + 1 + ((x) & NI_MAX_COUNTERS))
-#define NI_COUNTER_NAMES_MAX NI_CtrSampleClock(-1)
+#define NI_CtrSampleClock(x) _TERM_N(NI_CtrOut(-1) + 1, NI_MAX_COUNTERS, x)
+#define NI_COUNTER_NAMES_MAX NI_CtrSampleClock(-1)
enum ni_common_signal_names {
/* PXI_Star: this is a non-NI-specific signal */
It's no more ugly than the original, although I pity the fool who has to
make sense of the preprocessor output!
Reviewed-by: Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: comedi: ni_mio_common: scale ao INSN_CONFIG_GET_CMD_TIMING_CONSTRAINTS
On 24/10/18 15:46, Spencer E. Olson wrote:
Fixes implementation of INSN_CONFIG_GET_CMD_TIMING_CONSTRAINTS for
ni_mio devices. The previous patch should have used the channel
information passed in to scale the result by the number of channels being
used.
Fixes: 51fd36738383 ("staging: comedi: ni_mio_common: implement
INSN_CONFIG_GET_CMD_TIMING_CONSTRAINTS")
Signed-off-by: Spencer E. Olson
---
drivers/staging/comedi/drivers/ni_mio_common.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 2d1e0325d04d..5edf59ac6706 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -2843,7 +2843,8 @@ static int ni_ao_insn_config(struct comedi_device *dev,
return ni_ao_arm(dev, s);
case INSN_CONFIG_GET_CMD_TIMING_CONSTRAINTS:
/* we don't care about actual channels */
- data[1] = board->ao_speed;
+ /* data[3] : chanlist_len */
+ data[1] = board->ao_speed * data[3];
data[2] = 0;
return 0;
default:
Thanks. Hopefully this and your other patch will make it into the next
version of the kernel (4.20? 5.0?). If not, they should be backported.
Reviewed-by: Ian Abbott
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: change do_insn*_ioctl to allow more samples
valid. In ni_mio_common.c: * ni_calib_insn_read(), ni_eeprom_insn_read(), ni_m_series_eeprom_insn_read(), ni_calib_insn_write() all assume insn->n is 1 and that data[0] is valid or accessible. In s526.c: * s526_gpct_winsn() uses data[0] and sometimes data[1] but doesn't check insn->n. * s526_gpct_insn_config() accesses up to data[3] but doesn't check the length (and check_insn_config_length() in comedi_fops.c doesn't check the lengths for the INSN_CONFIG instructions handled here). -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: comedi: cb_pcidas64: Use insn->n in AO insn_write handler
The `insn_write` handler for the AO subdevice (`ao_winsn()` currently
ignores `insn->n` (the number of samples to write) and assumes a single
sample is to be written. But `insn->n` could be 0, meaning no samples
should be written, in which case `data[0]` is invalid.
Follow the usual Comedi guidelines and change `ao_winsn()` to write the
specified number of samples. This fixes the assumption that `data[0]`
is valid.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/cb_pcidas64.c | 32
1 file changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/comedi/drivers/cb_pcidas64.c
b/drivers/staging/comedi/drivers/cb_pcidas64.c
index 631a703b345d..44e5aaf8bae5 100644
--- a/drivers/staging/comedi/drivers/cb_pcidas64.c
+++ b/drivers/staging/comedi/drivers/cb_pcidas64.c
@@ -3097,8 +3097,10 @@ static int ao_winsn(struct comedi_device *dev, struct
comedi_subdevice *s,
{
const struct pcidas64_board *board = dev->board_ptr;
struct pcidas64_private *devpriv = dev->private;
- int chan = CR_CHAN(insn->chanspec);
- int range = CR_RANGE(insn->chanspec);
+ unsigned int chan = CR_CHAN(insn->chanspec);
+ unsigned int range = CR_RANGE(insn->chanspec);
+ unsigned int val = s->readback[chan];
+ unsigned int i;
/* do some initializing */
writew(0, devpriv->main_iobase + DAC_CONTROL0_REG);
@@ -3108,20 +3110,24 @@ static int ao_winsn(struct comedi_device *dev, struct
comedi_subdevice *s,
writew(devpriv->dac_control1_bits,
devpriv->main_iobase + DAC_CONTROL1_REG);
- /* write to channel */
- if (board->layout == LAYOUT_4020) {
- writew(data[0] & 0xff,
- devpriv->main_iobase + dac_lsb_4020_reg(chan));
- writew((data[0] >> 8) & 0xf,
- devpriv->main_iobase + dac_msb_4020_reg(chan));
- } else {
- writew(data[0], devpriv->main_iobase + dac_convert_reg(chan));
+ for (i = 0; i < insn->n; i++) {
+ /* write to channel */
+ val = data[i];
+ if (board->layout == LAYOUT_4020) {
+ writew(val & 0xff,
+ devpriv->main_iobase + dac_lsb_4020_reg(chan));
+ writew((val >> 8) & 0xf,
+ devpriv->main_iobase + dac_msb_4020_reg(chan));
+ } else {
+ writew(val,
+ devpriv->main_iobase + dac_convert_reg(chan));
+ }
}
- /* remember output value */
- s->readback[chan] = data[0];
+ /* remember last output value */
+ s->readback[chan] = val;
- return 1;
+ return insn->n;
}
static void set_dac_control0_reg(struct comedi_device *dev,
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/3] staging: comedi: cb_pcidas64: Fix insn->n assumptions
Fix some assumptions about the length of Comedi instructions in the "cb_pcidas64" driver. 1) staging: comedi: cb_pcidas64: Use insn->n in AO insn_write handler 2) staging: comedi: cb_pcidas64: Use insn->n in EEPROM insn_read handler 3) staging: comedi: Check length of INSN_CONFIG_TIMER_1 instruction drivers/staging/comedi/comedi_fops.c | 1 + drivers/staging/comedi/drivers/cb_pcidas64.c | 44 ++-- 2 files changed, 30 insertions(+), 15 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: comedi: Check length of INSN_CONFIG_TIMER_1 instruction
The contents of the Comedi configuration instruction `INSN_CONFIG_TIMER_1` instruction are not very well defined, but the one driver that uses it (the "cb_pcidas64" driver for the PCI-DAS4020/12 card) assumes its `insn->n` is 5. Add a check in `check_insn_config_length()` to verify that `insn->n` is correct for this configuration instruction. Signed-off-by: Ian Abbott --- drivers/staging/comedi/comedi_fops.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index c1c6b2b4ab91..ceb6ba5dd57c 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -1209,6 +1209,7 @@ static int check_insn_config_length(struct comedi_insn *insn, break; case INSN_CONFIG_PWM_OUTPUT: case INSN_CONFIG_ANALOG_TRIG: + case INSN_CONFIG_TIMER_1: if (insn->n == 5) return 0; break; -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: comedi: cb_pcidas64: Use insn->n in EEPROM insn_read handler
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;
}
/* Allocate and initialize the subdevice structures. */
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: cb_pcidda: Use insn->n in AO insn_write handler
The `insn_write` handler for the AO subdevice (`cb_pcidda_ao_insn_write()`) currently ignores `insn->n` (the number of samples to write) and assumes a single sample is to be written. But `insn->n` could be 0, meaning no samples should be written, in which case `data[0]` is invalid. Follow the usual Comedi guidelines and change `cb_pcidda_ao_insn_write()` to write the specified number of samples. This fixes the assumption that `data[0]` is valid. Signed-off-by: Ian Abbott --- drivers/staging/comedi/drivers/cb_pcidda.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/cb_pcidda.c b/drivers/staging/comedi/drivers/cb_pcidda.c index 807a31e75883..1d09dd265ab7 100644 --- a/drivers/staging/comedi/drivers/cb_pcidda.c +++ b/drivers/staging/comedi/drivers/cb_pcidda.c @@ -291,6 +291,7 @@ static int cb_pcidda_ao_insn_write(struct comedi_device *dev, unsigned int channel = CR_CHAN(insn->chanspec); unsigned int range = CR_RANGE(insn->chanspec); unsigned int ctrl; + unsigned int i; if (range != devpriv->ao_range[channel]) cb_pcidda_calibrate(dev, channel, range); @@ -317,7 +318,8 @@ static int cb_pcidda_ao_insn_write(struct comedi_device *dev, outw(ctrl, devpriv->daqio + CB_DDA_DA_CTRL_REG); - outw(data[0], devpriv->daqio + CB_DDA_DA_DATA_REG(channel)); + for (i = 0; i < insn->n; i++) + outw(data[i], devpriv->daqio + CB_DDA_DA_DATA_REG(channel)); return insn->n; } -- 2.19.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] staging: comedi: cb_pcidas64: Use insn->n in EEPROM insn_read handler
On 30/10/18 15:34, Hartley Sweeten wrote:
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
I'd rather keep it consistent and use the "normal" Comedi meaning. We
already have some insn_write handlers for EEPROM that just write the
final sample from the data (data[insn->n - 1]) to the location specified
by insn->chanspec (see labpc_eeprom_insn_write() in
.../comedi/drivers/ni_labpc_common.c), so it would be inconsistent with
that.
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: ni_labpc_common: Use insn->n in AO insn_write handler
The `insn_write` handler for the AO subdevice (`labpc_ao_insn_write()`)
currently ignores `insn->n` (the number of samples to write) and assumes
a single sample is to be written. But `insn->n` could be 0, meaning no
samples should be written, in which case `data[0]` is invalid.
Follow the usual Comedi guidelines and change `labpc_ao_insn_write()` to
write the specified number of samples. This fixes the assumption that
`data[0]` is valid.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/ni_labpc_common.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_labpc_common.c
b/drivers/staging/comedi/drivers/ni_labpc_common.c
index 7fa2d39562db..406952f5521d 100644
--- a/drivers/staging/comedi/drivers/ni_labpc_common.c
+++ b/drivers/staging/comedi/drivers/ni_labpc_common.c
@@ -906,7 +906,9 @@ static int labpc_ao_insn_write(struct comedi_device *dev,
{
const struct labpc_boardinfo *board = dev->board_ptr;
struct labpc_private *devpriv = dev->private;
- int channel, range;
+ unsigned int channel;
+ unsigned int range;
+ unsigned int i;
unsigned long flags;
channel = CR_CHAN(insn->chanspec);
@@ -932,9 +934,10 @@ static int labpc_ao_insn_write(struct comedi_device *dev,
devpriv->write_byte(dev, devpriv->cmd6, CMD6_REG);
}
/* send data */
- labpc_ao_write(dev, s, channel, data[0]);
+ for (i = 0; i < insn->n; i++)
+ labpc_ao_write(dev, s, channel, data[i]);
- return 1;
+ return insn->n;
}
/* lowlevel write to eeprom/dac */
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: change do_insn*_ioctl to allow more samples
On 30/10/18 22:27, Spencer Olson wrote: On Tue, Oct 30, 2018 at 6:21 AM Ian Abbott wrote: On 25/10/18 02:36, Spencer E. Olson wrote: Changes do_insn*_ioctl functions to allow for data lengths for each comedi_insn of up to 2^16. This patch also changes these functions to only allocate as much memory as is necessary for each comedi_insn, rather than allocating a fixed-sized scratch space. In testing some user-space code for the new INSN_DEVICE_CONFIG_GET_ROUTES facility with some newer hardware, I discovered that do_insn_ioctl and do_insnlist_ioctl limited the amount of data that can be passed into the kernel for insn's to a length of 256. For some newer hardware, the number of routes can be greater than 1000. Working around the old limits (256) would complicate the user-space/kernel interaction. The new upper limit is reasonable with current memory available and does not otherwise impact the memory footprint for any current or otherwise typical configuration. [snip] There are still some broken drivers that do not check insn->n and assume the data has some minimum length, so we will need to make the data array some minimum length (say 16 * sizeof(unsigned int)) until those drivers have been fixed. I don't think that would be much of a problem, since 16*sizeof(uint) is pretty negligible. How certain are you that we would not have to make that higher, as the previous default was a length of 256*sizeof(uint)? It'll take me a day or two to resubmit (have other things pressing on my time). ai_config_master_clock() in cb_pcidas64.c is the worst offender of using unchecked data[] length, and that only uses a data[] length of 5. I've checked all the insn_read, insn_write, and insn_config handlers now and found a couple more drivers with the problem of potentially using data[] members beyond insn->n-1. These are "s626.c" and "addi_apci_3501.c". (The previously found files with the problem are "cb_pcidas64.c", "cb_pcidda.c", "ni_labpc_common.c", "ni_mio_common.c", and "s526.c". I've submitted patches for some of those, and will submit patches for the others sometime in the next few days.) -- -=( Ian Abbott || Web: www.mev.co.uk )=- -=( MEV Ltd. is a company registered in England & Wales. )=- -=( Registered number: 02862268. Registered address:)=- -=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: change do_insn*_ioctl to allow more samples
On 25/10/18 02:36, Spencer E. Olson wrote:
Changes do_insn*_ioctl functions to allow for data lengths for each
comedi_insn of up to 2^16. This patch also changes these functions to only
allocate as much memory as is necessary for each comedi_insn, rather than
allocating a fixed-sized scratch space.
In testing some user-space code for the new INSN_DEVICE_CONFIG_GET_ROUTES
facility with some newer hardware, I discovered that do_insn_ioctl and
do_insnlist_ioctl limited the amount of data that can be passed into the
kernel for insn's to a length of 256. For some newer hardware, the number
of routes can be greater than 1000. Working around the old limits (256)
would complicate the user-space/kernel interaction.
The new upper limit is reasonable with current memory available and does
not otherwise impact the memory footprint for any current or otherwise
typical configuration.
Signed-off-by: Spencer E. Olson
---
drivers/staging/comedi/comedi_fops.c | 37 +---
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index c1c6b2b4ab91..a163ec2df872 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1500,7 +1500,7 @@ static int parse_insn(struct comedi_device *dev, struct
comedi_insn *insn,
*data (for reads) to insns[].data pointers
*/
/* arbitrary limits */
-#define MAX_SAMPLES 256
+#define MAX_SAMPLES 65536
static int do_insnlist_ioctl(struct comedi_device *dev,
struct comedi_insnlist __user *arg, void *file)
{
@@ -1513,12 +1513,6 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
if (copy_from_user(&insnlist, arg, sizeof(insnlist)))
return -EFAULT;
- data = kmalloc_array(MAX_SAMPLES, sizeof(unsigned int), GFP_KERNEL);
- if (!data) {
- ret = -ENOMEM;
- goto error;
- }
-
insns = kcalloc(insnlist.n_insns, sizeof(*insns), GFP_KERNEL);
if (!insns) {
ret = -ENOMEM;
@@ -1539,6 +1533,14 @@ static int do_insnlist_ioctl(struct comedi_device *dev,
ret = -EINVAL;
goto error;
}
+
+ data = kmalloc_array(insns[i].n, sizeof(unsigned int),
+GFP_KERNEL);
+ if (!data) {
+ ret = -ENOMEM;
+ goto error;
+ }
+
Something you could do here is work out which insn->n in the instruction
list is the largest, and allocate the 'data' once outside the
instruction list handling loop instead of allocating it inside the loop.
--
-=( Ian Abbott || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address:)=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: addi_apci_3501: Use insn->n in EEPROM insn_read handler
The `insn_read` handler for the EEPROM subdevice
(`apci3501_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.)
Following 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/addi_apci_3501.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/addi_apci_3501.c
b/drivers/staging/comedi/drivers/addi_apci_3501.c
index a38267928e5e..b4aa588975df 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3501.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3501.c
@@ -258,8 +258,15 @@ static int apci3501_eeprom_insn_read(struct comedi_device
*dev,
{
struct apci3501_private *devpriv = dev->private;
unsigned short addr = CR_CHAN(insn->chanspec);
+ unsigned int val;
+ unsigned int i;
- data[0] = apci3501_eeprom_readw(devpriv->amcc, 2 * addr);
+ if (insn->n) {
+ /* No point reading the same EEPROM location more than once. */
+ val = apci3501_eeprom_readw(devpriv->amcc, 2 * addr);
+ for (i = 0; i < insn->n; i++)
+ data[i] = val;
+ }
return insn->n;
}
--
2.19.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: comedi: add ioctls to set per-file read and write subdevice
Now that Comedi has the structures in place to support setting the
current "read" and/or "write" subdevice on a per-file object basis, add
new ioctls to set them. The newly chosen "read" ("write") subdevice
needs to support "read" ("write") commands, and the file cannot be busy
handling a "read" ("write") command on the previous subdevice (if any).
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi.h | 2 +
drivers/staging/comedi/comedi_compat32.c | 2 +
drivers/staging/comedi/comedi_fops.c | 90
3 files changed, 94 insertions(+)
diff --git a/drivers/staging/comedi/comedi.h b/drivers/staging/comedi/comedi.h
index f302ce6..7455740 100644
--- a/drivers/staging/comedi/comedi.h
+++ b/drivers/staging/comedi/comedi.h
@@ -367,6 +367,8 @@ enum comedi_support_level {
#define COMEDI_BUFCONFIG _IOR(CIO, 13, struct comedi_bufconfig)
#define COMEDI_BUFINFO _IOWR(CIO, 14, struct comedi_bufinfo)
#define COMEDI_POLL _IO(CIO, 15)
+#define COMEDI_SETRSUBD _IO(CIO, 16)
+#define COMEDI_SETWSUBD _IO(CIO, 17)
/* structures */
diff --git a/drivers/staging/comedi/comedi_compat32.c
b/drivers/staging/comedi/comedi_compat32.c
index 9b6f96f..5a4c74f 100644
--- a/drivers/staging/comedi/comedi_compat32.c
+++ b/drivers/staging/comedi/comedi_compat32.c
@@ -416,6 +416,8 @@ static inline int raw_ioctl(struct file *file, unsigned int
cmd,
case COMEDI_UNLOCK:
case COMEDI_CANCEL:
case COMEDI_POLL:
+ case COMEDI_SETRSUBD:
+ case COMEDI_SETWSUBD:
/* No translation needed. */
rc = translated_ioctl(file, cmd, arg);
break;
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 79b852c..f143cb6 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1847,6 +1847,90 @@ static int do_poll_ioctl(struct comedi_device *dev,
unsigned long arg,
return -EINVAL;
}
+/*
+ * COMEDI_SETRSUBD ioctl
+ * sets the current "read" subdevice on a per-file basis
+ *
+ * arg:
+ * subdevice number
+ *
+ * reads:
+ * nothing
+ *
+ * writes:
+ * nothing
+ */
+static int do_setrsubd_ioctl(struct comedi_device *dev, unsigned long arg,
+struct file *file)
+{
+ struct comedi_file *cfp = file->private_data;
+ struct comedi_subdevice *s_old, *s_new;
+
+ if (arg >= dev->n_subdevices)
+ return -EINVAL;
+
+ s_new = &dev->subdevices[arg];
+ s_old = comedi_file_read_subdevice(file);
+ if (s_old == s_new)
+ return 0; /* no change */
+
+ if (!(s_new->subdev_flags & SDF_CMD_READ))
+ return -EINVAL;
+
+ /*
+* Check the file isn't still busy handling a "read" command on the
+* old subdevice (if any).
+*/
+ if (s_old && s_old->busy == file && s_old->async &&
+ !(s_old->async->cmd.flags & CMDF_WRITE))
+ return -EBUSY;
+
+ ACCESS_ONCE(cfp->read_subdev) = s_new;
+ return 0;
+}
+
+/*
+ * COMEDI_SETWSUBD ioctl
+ * sets the current "write" subdevice on a per-file basis
+ *
+ * arg:
+ * subdevice number
+ *
+ * reads:
+ * nothing
+ *
+ * writes:
+ * nothing
+ */
+static int do_setwsubd_ioctl(struct comedi_device *dev, unsigned long arg,
+struct file *file)
+{
+ struct comedi_file *cfp = file->private_data;
+ struct comedi_subdevice *s_old, *s_new;
+
+ if (arg >= dev->n_subdevices)
+ return -EINVAL;
+
+ s_new = &dev->subdevices[arg];
+ s_old = comedi_file_write_subdevice(file);
+ if (s_old == s_new)
+ return 0; /* no change */
+
+ if (!(s_new->subdev_flags & SDF_CMD_WRITE))
+ return -EINVAL;
+
+ /*
+* Check the file isn't still busy handling a "write" command on the
+* old subdevice (if any).
+*/
+ if (s_old && s_old->busy == file && s_old->async &&
+ (s_old->async->cmd.flags & CMDF_WRITE))
+ return -EBUSY;
+
+ ACCESS_ONCE(cfp->write_subdev) = s_new;
+ return 0;
+}
+
static long comedi_unlocked_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
{
@@ -1941,6 +2025,12 @@ static long comedi_unlocked_ioctl(struct file *file,
unsigned int cmd,
case COMEDI_POLL:
rc = do_poll_ioctl(dev, arg, file);
break;
+ case COMEDI_SETRSUBD:
+ rc = do_setrsubd_ioctl(dev, arg, file);
+ break;
+ case COMEDI_SETWSUBD:
+ rc = do_setwsubd_ioctl(dev, arg, file);
+ break;
default:
rc = -ENOTTY;
break;
--
2.1.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 0/2] staging: comedi: per-file read/write subdevice choice
This series of patches adds a couple of ioctl codes to the Comedi core to allow the current "read" and "write" subdevice to be changed after opening the comedi device. The current read and write subdevice information is stored in file private data allocated for the lifetime of the file object, so the notion of current "read" and "write" subdevice is local to the file object and does not alter anything in the main control structure for the comedi device. An extra level of indirection is now required to access the main control structure. I've tested the multiple "read" subdevice case using a modified version of the "comedi_test" module (modifying it to clone the existing AI subdevice), and a modified version of the "comedi_test" application (part of the "comedilib" installation) modified to use the new ioctls. There isn't any support in comedilib itself yet. 1) staging: comedi: prepare support for per-file read and write subdevices 2) staging: comedi: add ioctls to set per-file read and write subdevice drivers/staging/comedi/comedi.h | 2 + drivers/staging/comedi/comedi_compat32.c | 2 + drivers/staging/comedi/comedi_fops.c | 217 +++ 3 files changed, 196 insertions(+), 25 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: comedi: prepare support for per-file read and write subdevices
Comedi devices may have several subdevices that support "read" and/or
"write" asynchronous commands that use the "read" or "write" file
operations for data transfer. The low-level Comedi drivers may nominate
a default "read" subdevice and/or a default "write" subdevice, but it
may have other subdevices that support asynchronous commands.
The Comedi core provides a somewhat clunky mechanism to provide access
to the asynchronous command support of the non-default subdevices. When
a low-level device is "attached" to a core Comedi device, it dynamically
allocates a minor device number for each of the subdevices that support
asynchrounous commands and associates them with files created in SysFS
named "comediX_subdY", where "X" is the minor device number of the main
comedi device, and "Y" is the subdevice number. An application can open
these subdevice-specific files and they behave like the regular
"comediX" files except that the "read" and/or "write" subdevice may be
different to the default chosen by the low-level driver.
This patch adds a layer of indirection between the file object and the
comedi device object to allow the current "read" and/or "write"
subdevice to be altered after opening the Comedi device, on a per-file
object basis. The advantage is that an application only needs to open
the main Comedi device file and can then choose which subdevice it wants
to "read" or "write". The main Comedi device file can be opened more
than once, and each file object can choose the "read" and "write"
subdevices independently.
The new `struct comedi_file` is created on "open" and freed on
"release". It includes pointers to the main Comedi device structure,
and to the current "read" and "write" subdevice structures (which may be
NULL). It also has information to keep track of when a low-level device
has been attached or detached since the previous time the file object
was used. In that case, the current "read" and "write" subdevices in
the `struct comedi_file` will be changed to the new defaults (or set to
NULL). (The change to new defaults is done by `comedi_file_reset()`.
The checking for attach/detach is done by `comedi_file_check()` which
will call `comedi_file_reset()` if there have been any attach/detach
operations since the previous call.)
A subsequent patch will add the ioctls to change the current "read" and
"write" subdevices.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/comedi_fops.c | 127 ---
1 file changed, 102 insertions(+), 25 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index 65894fd..79b852c 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -43,6 +43,22 @@
#include "comedi_internal.h"
+/**
+ * struct comedi_file - per-file private data for comedi device
+ * @dev: comedi_device struct
+ * @read_subdev: current "read" subdevice
+ * @write_subdev: current "write" subdevice
+ * @last_detach_count: last known detach count
+ * @last_attached: last known attached/detached state
+ */
+struct comedi_file {
+ struct comedi_device *dev;
+ struct comedi_subdevice *read_subdev;
+ struct comedi_subdevice *write_subdev;
+ unsigned int last_detach_count;
+ bool last_attached:1;
+};
+
#define COMEDI_NUM_MINORS 0x100
#define COMEDI_NUM_SUBDEVICE_MINORS\
(COMEDI_NUM_MINORS - COMEDI_NUM_BOARD_MINORS)
@@ -239,6 +255,54 @@ comedi_write_subdevice(const struct comedi_device *dev,
unsigned int minor)
return dev->write_subdev;
}
+static void comedi_file_reset(struct file *file)
+{
+ struct comedi_file *cfp = file->private_data;
+ struct comedi_device *dev = cfp->dev;
+ struct comedi_subdevice *s, *read_s, *write_s;
+ unsigned int minor = iminor(file_inode(file));
+
+ read_s = dev->read_subdev;
+ write_s = dev->write_subdev;
+ if (minor >= COMEDI_NUM_BOARD_MINORS) {
+ s = comedi_subdevice_from_minor(dev, minor);
+ if (s == NULL || s->subdev_flags & SDF_CMD_READ)
+ read_s = s;
+ if (s == NULL || s->subdev_flags & SDF_CMD_WRITE)
+ write_s = s;
+ }
+ cfp->last_attached = dev->attached;
+ cfp->last_detach_count = dev->detach_count;
+ ACCESS_ONCE(cfp->read_subdev) = read_s;
+ ACCESS_ONCE(cfp->write_subdev) = write_s;
+}
+
+static void comedi_file_check(struct file *file)
+{
+ struct comedi_file *cfp = file->private_data;
+ struct comedi_device *dev = cfp->dev;
+
+ if (cfp->last_at
Re: [PATCH 0/2] staging: comedi: per-file read/write subdevice choice
On 04/11/2014 18:44, Hartley Sweeten wrote: On Tuesday, November 04, 2014 11:09 AM, Ian Abbott wrote: This series of patches adds a couple of ioctl codes to the Comedi core to allow the current "read" and "write" subdevice to be changed after opening the comedi device. The current read and write subdevice information is stored in file private data allocated for the lifetime of the file object, so the notion of current "read" and "write" subdevice is local to the file object and does not alter anything in the main control structure for the comedi device. An extra level of indirection is now required to access the main control structure. I've tested the multiple "read" subdevice case using a modified version of the "comedi_test" module (modifying it to clone the existing AI subdevice), and a modified version of the "comedi_test" application (part of the "comedilib" installation) modified to use the new ioctls. There isn't any support in comedilib itself yet. 1) staging: comedi: prepare support for per-file read and write subdevices 2) staging: comedi: add ioctls to set per-file read and write subdevice drivers/staging/comedi/comedi.h | 2 + drivers/staging/comedi/comedi_compat32.c | 2 + drivers/staging/comedi/comedi_fops.c | 217 +++ 3 files changed, 196 insertions(+), 25 deletions(-) Ian, If I understand this correctly, the user can now open the "comediX" device which will give then access to the default dev->read_subdev and dev->write_subdev. They can then use the new ioctls to change the read/write subdevice and use them without having the open the "comediX_subdY" devices. If this is correct it seems like a good idea. That's correct. Reviewed-by: H Hartley Sweeten Thanks for the review! Also, does this mean we can get rid of comedi_alloc_subdevice_minor() and the "comediX_sudbY" stuff? That would be a bit premature at this stage. Maybe in a year or two. -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 12/18] staging: comedi: s626: use comedi_async 'scans_done' to detect EOA
On 04/11/14 16:50, H Hartley Sweeten wrote:
Remove the private data member 'ai_sample_count' and use the comedi_async
'scans_done' member to detect the end-of-acquisition.
Also, remove the unnecessary COMEDI_CB_EOS event. The core automatically
detects and adds that event.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/s626.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/comedi/drivers/s626.c
b/drivers/staging/comedi/drivers/s626.c
index 6976bb0..842b7db 100644
--- a/drivers/staging/comedi/drivers/s626.c
+++ b/drivers/staging/comedi/drivers/s626.c
@@ -78,7 +78,6 @@ struct s626_buffer_dma {
struct s626_private {
uint8_t ai_cmd_running; /* ai_cmd is running */
- int ai_sample_count;/* number of samples to acquire */
unsigned int ai_sample_timer; /* time between samples in
* units of the timer */
int ai_convert_count; /* conversion counter */
@@ -1496,16 +1495,8 @@ static bool s626_handle_eos_interrupt(struct
comedi_device *dev)
comedi_buf_write_samples(s, &tempdata, 1);
}
- /* end of scan occurs */
- async->events |= COMEDI_CB_EOS;
-
- if (cmd->stop_src == TRIG_COUNT) {
- devpriv->ai_sample_count--;
- if (devpriv->ai_sample_count <= 0) {
- devpriv->ai_cmd_running = 0;
- async->events |= COMEDI_CB_EOA;
- }
- }
+ if (cmd->stop_src == TRIG_COUNT && async->scans_done >= cmd->stop_arg)
+ async->events |= COMEDI_CB_EOA;
I think you still need to set devpriv->ai_cmd_running to 0 here, don't you?
if (devpriv->ai_cmd_running && cmd->scan_begin_src == TRIG_EXT)
s626_dio_set_irq(dev, cmd->scan_begin_arg);
@@ -2091,8 +2082,6 @@ static int s626_ai_cmd(struct comedi_device *dev, struct
comedi_subdevice *s)
break;
}
- devpriv->ai_sample_count = cmd->stop_arg;
-
s626_reset_adc(dev, ppl);
switch (cmd->start_src) {
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 14/18] staging: comedi: usbdux: use comedi_async 'scans_done' to detect AI EOA
On 04/11/14 16:50, H Hartley Sweeten wrote:
Remove the private data member 'ai_sample_count' and use the comedi_async
'scans_done' member to detect the analog input end-of-acquisition.
Move the EOA check so it happens after adding the samples from the current
urb to the async buffer. This prevents the unnecessary resubmit of the urb
when the EOA occurs.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/usbdux.c | 22 --
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbdux.c
b/drivers/staging/comedi/drivers/usbdux.c
index 7782015..4737dbf 100644
--- a/drivers/staging/comedi/drivers/usbdux.c
+++ b/drivers/staging/comedi/drivers/usbdux.c
@@ -202,8 +202,6 @@ struct usbdux_private {
unsigned int ao_cmd_running:1;
unsigned int pwm_cmd_running:1;
- /* number of samples to acquire */
- int ai_sample_count;
/* time between samples in units of the timer */
unsigned int ai_timer;
unsigned int ao_timer;
@@ -263,14 +261,6 @@ static void usbduxsub_ai_handle_urb(struct comedi_device
*dev,
if (devpriv->ai_counter == 0) {
devpriv->ai_counter = devpriv->ai_timer;
- if (cmd->stop_src == TRIG_COUNT) {
- devpriv->ai_sample_count--;
- if (devpriv->ai_sample_count < 0) {
- async->events |= COMEDI_CB_EOA;
- return;
- }
- }
-
/* get the data from the USB bus and hand it over to comedi */
for (i = 0; i < cmd->chanlist_len; i++) {
unsigned int range = CR_RANGE(cmd->chanlist[i]);
@@ -284,6 +274,10 @@ static void usbduxsub_ai_handle_urb(struct comedi_device
*dev,
if (!comedi_buf_write_samples(s, &val, 1))
return;
}
+
+ if (cmd->stop_src == TRIG_COUNT &&
+ async->scans_done >= cmd->stop_arg)
+ async->events |= COMEDI_CB_EOA;
You could return early here, but it doesn't really matter as the
following code does nothing if COMEDI_CB_EOA is set.
}
/* if command is still running, resubmit urb */
@@ -732,14 +726,6 @@ static int usbdux_ai_cmd(struct comedi_device *dev, struct
comedi_subdevice *s)
devpriv->ai_counter = devpriv->ai_timer;
- if (cmd->stop_src == TRIG_COUNT) {
- /* data arrives as one packet */
- devpriv->ai_sample_count = cmd->stop_arg;
- } else {
- /* continous acquisition */
- devpriv->ai_sample_count = 0;
- }
-
if (cmd->start_src == TRIG_NOW) {
/* enable this acquisition operation */
devpriv->ai_cmd_running = 1;
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/18] staging: comedi: comedidev.h: add 'scans_done' member to comedi_async
On 04/11/14 16:50, H Hartley Sweeten wrote:
Introduce a new member to comedi_async to count the number of scans completed.
This member is cleared by comedi_buf_reset() along with the other comedi_async
members. It is incremented in comedi_inc_scan_progress() when the end of scan
is detected.
This member will be used to clean up the scan counting in the comedi drivers.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/comedi_buf.c | 1 +
drivers/staging/comedi/comedidev.h | 2 ++
drivers/staging/comedi/drivers.c| 1 +
3 files changed, 4 insertions(+)
diff --git a/drivers/staging/comedi/comedi_buf.c
b/drivers/staging/comedi/comedi_buf.c
index eb3fecf..19e7b22 100644
--- a/drivers/staging/comedi/comedi_buf.c
+++ b/drivers/staging/comedi/comedi_buf.c
@@ -236,6 +236,7 @@ void comedi_buf_reset(struct comedi_subdevice *s)
async->buf_read_ptr = 0;
async->cur_chan = 0;
+ async->scans_done = 0;
async->scan_progress = 0;
async->munge_chan = 0;
async->munge_count = 0;
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index 90af11a..bff5a58 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -121,6 +121,7 @@ struct comedi_buf_map {
* @buf_read_ptr: buffer position for reader
* @cur_chan: current position in chanlist for scan (for those
*drivers that use it)
+ * @scans_done:the number of scans completed (COMEDI_CB_EOS)
* @scan_progress:amount received or sent for current scan (in bytes)
* @munge_chan: current position in chanlist for "munging"
* @munge_count: "munge" count (in bytes, modulo 2**32)
@@ -201,6 +202,7 @@ struct comedi_async {
unsigned int buf_write_ptr;
unsigned int buf_read_ptr;
unsigned int cur_chan;
+ unsigned int scans_done;
unsigned int scan_progress;
unsigned int munge_chan;
unsigned int munge_count;
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 9a8c5fc..4fc992b 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -352,6 +352,7 @@ void comedi_inc_scan_progress(struct comedi_subdevice *s,
async->scan_progress += num_bytes;
if (async->scan_progress >= scan_length) {
+ async->scans_done += async->scan_progress / scan_length;
async->scan_progress %= scan_length;
async->events |= COMEDI_CB_EOS;
}
The patch is fine, but as a follow-up patch, async->scans_done could be
clamped to UINT_MAX (or possibly async->cmd.stop_arg, but UINT_MAX is
easier), e.g.:
unsigned int scans = async->scan_progress / scan_length;
if (async->scans_done < UINT_MAX - scans)
async->scans_done += scans;
else
async->scans_done = UINT_MAX;
async->scan_progress -= scans * scan_length;
async->events |= COMEDI_CB_EOS;
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 16/18] staging: comedi: usbduxsigma: use comedi_async 'scans_done' to detect AI EOA
On 04/11/14 16:50, H Hartley Sweeten wrote:
Remove the private data member 'ai_sample_count' and use the comedi_async
'scans_done' member to detect the analog input end-of-acquisition.
Move the EOA check so it happens after adding the samples from the current
urb to the async buffer. This prevents the unnecessary resubmit of the urb
when the EOA occurs.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/usbduxsigma.c | 22 --
1 file changed, 4 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index 31b2806..dc19435 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -164,8 +164,6 @@ struct usbduxsigma_private {
unsigned ao_cmd_running:1;
unsigned pwm_cmd_running:1;
- /* number of samples to acquire */
- int ai_sample_count;
/* time between samples in units of the timer */
unsigned int ai_timer;
unsigned int ao_timer;
@@ -226,14 +224,6 @@ static void usbduxsigma_ai_handle_urb(struct comedi_device
*dev,
if (devpriv->ai_counter == 0) {
devpriv->ai_counter = devpriv->ai_timer;
- if (cmd->stop_src == TRIG_COUNT) {
- devpriv->ai_sample_count--;
- if (devpriv->ai_sample_count < 0) {
- async->events |= COMEDI_CB_EOA;
- return;
- }
- }
-
/* get the state of the dio pins to allow external trigger */
dio_state = be32_to_cpu(devpriv->in_buf[0]);
@@ -247,6 +237,10 @@ static void usbduxsigma_ai_handle_urb(struct comedi_device
*dev,
if (!comedi_buf_write_samples(s, &val, 1))
return;
}
+
+ if (cmd->stop_src == TRIG_COUNT &&
+ async->scans_done >= cmd->stop_arg)
+ async->events |= COMEDI_CB_EOA;
Again, you could return early here, but it doesn't really matter.
}
/* if command is still running, resubmit urb */
@@ -578,14 +572,6 @@ static int usbduxsigma_ai_cmdtest(struct comedi_device
*dev,
if (devpriv->ai_timer < 1)
err |= -EINVAL;
- if (cmd->stop_src == TRIG_COUNT) {
- /* data arrives as one packet */
- devpriv->ai_sample_count = cmd->stop_arg;
- } else {
- /* continuous acquisition */
- devpriv->ai_sample_count = 0;
- }
-
if (err)
return 4;
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/13] staging: comedi: drivers: introduce comedi_nscans_left()
On 04/11/14 17:00, H Hartley Sweeten wrote:
Introduce a helper function to determine the number of scans left in
the async command.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/comedidev.h | 2 ++
drivers/staging/comedi/drivers.c | 39 ++
2 files changed, 41 insertions(+)
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index bff5a58..e3ef5cc 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -516,6 +516,8 @@ int comedi_dio_insn_config(struct comedi_device *, struct
comedi_subdevice *,
unsigned int comedi_dio_update_state(struct comedi_subdevice *,
unsigned int *data);
unsigned int comedi_bytes_per_scan(struct comedi_subdevice *s);
+unsigned int comedi_nscans_left(struct comedi_subdevice *s,
+ unsigned int nscans);
void comedi_inc_scan_progress(struct comedi_subdevice *s,
unsigned int num_bytes);
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 4fc992b..a0bd48e 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -328,6 +328,45 @@ unsigned int comedi_bytes_per_scan(struct comedi_subdevice
*s)
EXPORT_SYMBOL_GPL(comedi_bytes_per_scan);
/**
+ * comedi_nscans_left - return the number of scans left in the command
+ * @s: comedi_subdevice struct
+ * @nscans: the expected number of scans
+ *
+ * If nscans is 0, the number of scans available in the async buffer will be
+ * used. Otherwise the expected number of scans will be used.
+ *
+ * If the async command has a stop_src of TRIG_COUNT, the nscans will be
+ * checked against the number of scans left in the command.
+ *
+ * The return value will then be either the expected number of scans or the
+ * number of scans remaining in the command.
+ */
+unsigned int comedi_nscans_left(struct comedi_subdevice *s,
+ unsigned int nscans)
+{
+ struct comedi_async *async = s->async;
+ struct comedi_cmd *cmd = &async->cmd;
+
+ if (nscans == 0) {
+ unsigned int nbytes = comedi_buf_read_n_available(s);
+
+ nscans = comedi_bytes_to_samples(s, nbytes);
That's the number of samples. I think you want this:
nscans = nbytes / comedi_bytes_per_scan(s);
+ }
+
+ if (cmd->stop_src == TRIG_COUNT) {
+ unsigned int scans_left = 0;
+
+ if (async->scans_done < cmd->stop_arg)
+ scans_left = cmd->stop_arg - async->scans_done;
+
+ if (nscans > scans_left)
+ nscans = scans_left;
+ }
+ return nscans;
+}
+EXPORT_SYMBOL_GPL(comedi_nscans_left);
+
+/**
* comedi_inc_scan_progress - update scan progress in asynchronous command
* @s: comedi_subdevice struct
* @num_bytes: amount of data in bytes to increment scan progress
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/13] staging: comedi: convert more drivers to use 'scans_done'
On 04/11/14 17:00, H Hartley Sweeten wrote: This series converts the remaining comedi drivers to use the comedi_async 'scans_done' member to count the number of scans completed when counting scans with a cmd->stop_src of TRIG_COUNT. H Hartley Sweeten (13): staging: comedi: drivers: introduce comedi_nscans_left() staging: comedi: amplc_pci224: use comedi_async 'scans_done' to detect EOA staging: comedi: comedi_test: use comedi_async 'scans_done' to detect EOA staging: comedi: amplc_pci230: use comedi_async 'scans_done' to detect AO EOA staging: comedi: drivers: introduce comedi_nsamples_left() staging: comedi: amplc_pci230: use comedi_async 'scans_done' to detect AI EOA staging: comedi: usbduxfast: use comedi_async 'scans_done' to detect AI EOA staging: comedi: cb_pcidas: use comedi_async 'scans_done' to detect AI EOA staging: comedi: cb_pcidas: use comedi_async 'scans_done' to detect AO EOA staging: comedi: cb_pcidas64: use comedi_async 'scans_done' to detect AI EOA staging: comedi: cb_pcidas64: use comedi_async 'scans_done' to detect AO EOA staging: comedi: quatech_daqp_cs: use comedi_async 'scans_done' to detect EOA staging: comedi: das1800: use comedi_async 'scans_done' to detect EOA drivers/staging/comedi/comedidev.h | 4 + drivers/staging/comedi/drivers.c | 71 +++ drivers/staging/comedi/drivers/amplc_pci224.c| 43 +++-- drivers/staging/comedi/drivers/amplc_pci230.c| 110 --- drivers/staging/comedi/drivers/cb_pcidas.c | 43 +++-- drivers/staging/comedi/drivers/cb_pcidas64.c | 95 +--- drivers/staging/comedi/drivers/comedi_test.c | 17 +--- drivers/staging/comedi/drivers/das1800.c | 52 --- drivers/staging/comedi/drivers/quatech_daqp_cs.c | 27 +++--- drivers/staging/comedi/drivers/usbduxfast.c | 19 ++-- 10 files changed, 197 insertions(+), 284 deletions(-) Looks fine apart from a glaring bug in PATCH 01. For patches 02 to 13... Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/18] staging: comedi: make core track the number of scans
On 04/11/14 16:50, H Hartley Sweeten wrote: The comedi drivers that support async commands with a cmd->stop_src of TRIG_COUNT currently have a private data member that is used to count the number of scans completed in order to detect the end of acquisition for the command. This series addes a new member, 'scans_done', to comedi_async and makes the core automatically count the completed scans. The comedi drivers that do simple scan counting are then converted to use the new member. H Hartley Sweeten (18): staging: comedi: comedidev.h: add 'scans_done' member to comedi_async staging: comedi: addi_apci_2032: use comedi_async 'scans_done' to detect EOA staging: comedi: amplc_dio200_common: use comedi_async 'scans_done' to detect EOA staging: comedi: pcl711: use comedi_async 'scans_done' to detect EOA staging: comedi: pcl812: use comedi_async 'scans_done' to detect EOA staging: comedi: pcl816: use comedi_async 'scans_done' to detect EOA staging: comedi: pcl818: remove private data member 'ai_act_chan' staging: comedi: pcl818: use comedi_async 'scans_done' to detect EOA staging: comedi: adv_pci1710: use comedi_async 'scans_done' to detect EOA staging: comedi: pcmmio: use comedi_async 'scans_done' to detect EOA staging: comedi: pcmuio: use comedi_async 'scans_done' to detect EOA staging: comedi: s626: use comedi_async 'scans_done' to detect EOA staging: comedi: usbdux: use comedi_async 'scans_done' to detect AO EOA staging: comedi: usbdux: use comedi_async 'scans_done' to detect AI EOA staging: comedi: usbduxsigma: use comedi_async 'scans_done' to detect AO EOA staging: comedi: usbduxsigma: use comedi_async 'scans_done' to detect AI EOA staging: comedi: addi_apci_3120: use comedi_async 'scans_done' to detect EOA staging: comedi: das800: use comedi_async 'scans_done' to detect EOA drivers/staging/comedi/comedi_buf.c| 1 + drivers/staging/comedi/comedidev.h | 2 + drivers/staging/comedi/drivers.c | 1 + .../comedi/drivers/addi-data/hwdrv_apci3120.c | 34 +++--- drivers/staging/comedi/drivers/addi_apci_2032.c| 17 +++ drivers/staging/comedi/drivers/addi_apci_3120.c| 1 - drivers/staging/comedi/drivers/adv_pci1710.c | 26 +++ .../staging/comedi/drivers/amplc_dio200_common.c | 14 ++ drivers/staging/comedi/drivers/das800.c| 31 + drivers/staging/comedi/drivers/pcl711.c| 16 +++ drivers/staging/comedi/drivers/pcl812.c| 9 +--- drivers/staging/comedi/drivers/pcl816.c| 9 +--- drivers/staging/comedi/drivers/pcl818.c| 17 +++ drivers/staging/comedi/drivers/pcmmio.c| 12 ++--- drivers/staging/comedi/drivers/pcmuio.c| 14 ++ drivers/staging/comedi/drivers/s626.c | 15 +-- drivers/staging/comedi/drivers/usbdux.c| 51 - drivers/staging/comedi/drivers/usbduxsigma.c | 52 ------ 18 files changed, 73 insertions(+), 249 deletions(-) I think there's a bug in PATCH 12, but the rest are okay. For patches 01 to 11, 13 to 18... Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 000/108] staging: comedi: addi_apci_3120: cleanup driver
3120: move apci3120_ai_insn_read() to driver
source
staging: comedi: addi_apci_3120: remove check in apci3120_setup_chan_list()
staging: comedi: addi_apci_3120: move apci3120_set_chanlist() to driver
source
staging: comedi: addi_apci_3120: factor DMA setup out of apci3120_cyclic_ai()
staging: comedi: addi_apci_3120: remove APCI3120_{ENABLE,DISABLE}
staging: comedi: addi_apci_3120: flip 'us_UseDma' test in
apci3120_cyclic_ai()
staging: comedi: addi_apci_3120: move timer 2 enable in apci3120_cyclic_ai()
staging: comedi: addi_apci_3120: move start_src check into
apci3120_cyclic_ai()
staging: comedi: addi_apci_3120: absorb apci3120_cyclic_ai()
staging: comedi: addi_apci_3120: tidy up timer programming in
apci3120_ai_cmd()
staging: comedi: addi_apci_3120: tidy up timer 2 programming in
apci3120_ai_cmd()
staging: comedi: addi_apci_3120: reset fifo after programming chanlist
staging: comedi: addi_apci_3120: set scan length/start after programming
chanlist
staging: comedi: addi_apci_3120: enable chanlist scanning if needed
staging: comedi: addi_apci_3120: tidy up devpriv->mode in apci3120_ai_cmd()
staging: comedi: addi_apci_3120: remove private data 'b_InterruptMode'
staging: comedi: addi_apci_3120: remove private data 'b_ExttrigEnable'
staging: comedi: addi_apci_3120: introduce apci3120_init_dma()
staging: comedi: addi_apci_3120: introduce apci3120_addon_write()
staging: comedi: addi_apci_3120: use amcc_s5933.h defines
staging: comedi: addi_apci_3120: define the Add-On registers
staging: comedi: addi_apci_3120: move APCI3120_FIFO_ADVANCE_ON_BYTE_2
staging: comedi: addi_apci_3120: move DMA init code to apci3120_init_dma()
staging: comedi: addi_apci_3120: tidy up apci3120_reset()
staging: comedi: addi_apci_3120: move apci3120_reset() to driver source
staging: comedi: addi_apci_3120: rename private data 'us_UseDma'
staging: comedi: addi_apci_3120: rename private data 'b_DmaDoubleBuffer'
staging: comedi: addi_apci_3120: rename private data 'ui_DmaActualBuffer'
staging: comedi: addi_apci_3120: don't use timer 2 to count scans
staging: comedi: addi_apci_3120: define the AI FIFO register
staging: comedi: addi_apci_3120: define the AI software trigger register
staging: comedi: addi_apci_3120: fix timer (*insn_read)
staging: comedi: addi_apci_3120: fix timer (*insn_config)
staging: comedi: addi_apci_3120: fix cmd->convert_arg vaildation
staging: comedi: addi_apci_3120: move AI (*do_cmdtest) to main driver
staging: comedi: addi_apci_3120: add copyright information
staging: comedi: addi_apci_3120: remove unnecessary include
staging: comedi: addi_apci_3120: move apci3120_addon_write() to driver
staging: comedi: addi_apci_3120: move apci3120_init_dma() to driver
staging: comedi: addi_apci_3120: move apci3120_setup_dma() to driver
staging: comedi: addi_apci_3120: move apci3120_ai_cmd() to driver
staging: comedi: addi_apci_3120: use async->events to report hardware error
staging: comedi: addi_apci_3120: move apci3120_cancel() to driver
staging: comedi: addi_apci_3120: move apci3120_interrupt() to driver
staging: comedi: addi_apci_3120: use comedi_bytes_to_samples()
staging: comedi: addi_apci_3120: move apci3120_interrupt_dma() to driver
staging: comedi: addi_apci_3120: change params to apci3120_interrupt_dma()
staging: comedi: addi_apci_3120: switch DMA buffers after writing samples
staging: comedi: addi_apci_3120: enable AI async commands
staging: comedi: addi_apci_3120: absorb apci3120_ai_reset_fifo()
.../comedi/drivers/addi-data/hwdrv_apci3120.c | 1903
drivers/staging/comedi/drivers/addi_apci_3120.c| 912 +-
drivers/staging/comedi/drivers/amcc_s5933.h|2 +
3 files changed, 885 insertions(+), 1932 deletions(-)
delete mode 100644 drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c
You've been busy! I think I'll review this as a whole rather than
individually.
My only comments are:
1) It would be preferable to not switch on the I8254_MODE constants
for the timer mode in PATCH 091 as they're pretty irrelevant here.
Rather, just add some new constants to comedi.h - perhaps the
APCI3120_TIMER_MODE constants introduced in PATCH 010.
2) The "IRQ from unknown source\n" message should be removed to avoid
spamming the kernel log since this is likely to be due to an interrupt
from a different device on the same IRQ number. (That was copied over
from the original code by PATCH 102.)
3) A comedi driver header comment would be nice!
Overall, a splendid job!
Reviewed-by: Ian Abbott
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 00/18] staging: comedi: make core track the number of scans
On 05/11/2014 17:20, H Hartley Sweeten wrote: The comedi drivers that support async commands with a cmd->stop_src of TRIG_COUNT currently have a private data member that is used to count the number of scans completed in order to detect the end of acquisition for the command. This series addes a new member, 'scans_done', to comedi_async and makes the core automatically count the completed scans. The comedi drivers that do simple scan counting are then converted to use the new member. v2: update series per suggestions from Ian Abbott PATCH 01/18 - clamp scans_done to UINT_MAX PATCH 12/18 - clear devpriv->ai_cmd_running flag correctly H Hartley Sweeten (18): staging: comedi: comedidev.h: add 'scans_done' member to comedi_async staging: comedi: addi_apci_2032: use comedi_async 'scans_done' to detect EOA staging: comedi: amplc_dio200_common: use comedi_async 'scans_done' to detect EOA staging: comedi: pcl711: use comedi_async 'scans_done' to detect EOA staging: comedi: pcl812: use comedi_async 'scans_done' to detect EOA staging: comedi: pcl816: use comedi_async 'scans_done' to detect EOA staging: comedi: pcl818: remove private data member 'ai_act_chan' staging: comedi: pcl818: use comedi_async 'scans_done' to detect EOA staging: comedi: adv_pci1710: use comedi_async 'scans_done' to detect EOA staging: comedi: pcmmio: use comedi_async 'scans_done' to detect EOA staging: comedi: pcmuio: use comedi_async 'scans_done' to detect EOA staging: comedi: s626: use comedi_async 'scans_done' to detect EOA staging: comedi: usbdux: use comedi_async 'scans_done' to detect AO EOA staging: comedi: usbdux: use comedi_async 'scans_done' to detect AI EOA staging: comedi: usbduxsigma: use comedi_async 'scans_done' to detect AO EOA staging: comedi: usbduxsigma: use comedi_async 'scans_done' to detect AI EOA staging: comedi: addi_apci_3120: use comedi_async 'scans_done' to detect EOA staging: comedi: das800: use comedi_async 'scans_done' to detect EOA drivers/staging/comedi/comedi_buf.c| 1 + drivers/staging/comedi/comedidev.h | 2 + drivers/staging/comedi/drivers.c | 7 +++ .../comedi/drivers/addi-data/hwdrv_apci3120.c | 34 +++--- drivers/staging/comedi/drivers/addi_apci_2032.c| 17 +++ drivers/staging/comedi/drivers/addi_apci_3120.c| 1 - drivers/staging/comedi/drivers/adv_pci1710.c | 26 +++ .../staging/comedi/drivers/amplc_dio200_common.c | 14 ++ drivers/staging/comedi/drivers/das800.c| 31 + drivers/staging/comedi/drivers/pcl711.c| 16 +++ drivers/staging/comedi/drivers/pcl812.c| 9 +--- drivers/staging/comedi/drivers/pcl816.c| 9 +--- drivers/staging/comedi/drivers/pcl818.c| 17 +++ drivers/staging/comedi/drivers/pcmmio.c| 12 ++--- drivers/staging/comedi/drivers/pcmuio.c| 14 ++ drivers/staging/comedi/drivers/s626.c | 16 ++- drivers/staging/comedi/drivers/usbdux.c | 51 ++++- drivers/staging/comedi/drivers/usbduxsigma.c | 52 -- 18 files changed, 81 insertions(+), 248 deletions(-) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 01/13] staging: comedi: drivers: introduce comedi_nscans_left()
On 05/11/14 17:31, H Hartley Sweeten wrote: Introduce a helper function to determine the number of scans left in the async command. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/comedidev.h | 2 ++ drivers/staging/comedi/drivers.c | 39 ++ 2 files changed, 41 insertions(+) Reviewed-by: Ian Abbott (patches 02 to 13 already reviewed) -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: adl_pci9111: use comedi_async 'scans_done' to detect EOA
On 11/11/14 00:28, H Hartley Sweeten wrote: The comedi core now counts the number of samples added to the async buffer and detects the end-of-scan and increments the comedi_async 'scans_done' counter. Remove the private data member 'stop_counter' and use the 'scans_done' member to detect the end-of-acquisition. This fixes a possible interger overflow when calculating the value of the 'stop_counter' and removes the need to accumulate the 'total' number of samples added to the async buffer in pci9111_handle_fifo_half_full(). Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/adl_pci9111.c | 28 1 file changed, 4 insertions(+), 24 deletions(-) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/4] staging: comedi: adl_pci9118: switch DMA buffers after writing samples
On 11/11/14 00:57, H Hartley Sweeten wrote: Currently the DMA buffers are switched before writing the current samples to the async buffer. This works but when the EOA event happens we end up with an outstanding DMA operation in progress that gets terminated by the (*cancel). Avoid the outstanding DMA operation by switching the DMA buffers after writing the samples. The driver will detect the EOA event and not retart the DMA. I think the DMA buffer is switched early to try and avoid overruns in the hardware while the other buffer is being processed (by defragment_dma_buffer() and comedi_buf_write_samples()), so this patch is likely to reduce the maximum error-free throughput. The combination of defragment_dma_buffer() and comedi_buf_write_samples() is a bit inefficient anyway, so perhaps we can drop this patch for now, and do a "no switch needed" test as part of another set of changes. (I can work on that if you want.) -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/15] staging: comedi: addi_apci_1564: fix board I/O mapping
On 10/11/14 23:20, H Hartley Sweeten wrote: The APCI-1564 boards do not actually have an AMCC PCI controller. According to ADDI-DATA, the boards have always used an FPGA with a PCI core. Unfortunately, there are two major revisions of the FPGA and they use different I/O mappings. Fix the driver so that both I/O map revisions work correctly. The counter/timer code in hwdrv_apci1564.c is still broken due to comedi API violations. H Hartley Sweeten (15): staging: comedi: addi_apci_1564: board does not use an AMCC PCI controller staging: comedi: addi_apci_1564: use correct I/O base for APCI1564_DI_INT_STATUS_REG staging: comedi: addi_apci_1564: store PCI BAR 1 base address in private data staging: comedi: addi_apci_1564: use dev->iobase for main registers staging: comedi: addi_apci_1564: detect PLD revision for I/O mapping staging: comedi: addi_apci_1564: fix dev->iobase for all PLD revisions staging: comedi: addi_apci_1564: fix timer iobase for all PLD revisions staging: comedi: addi_apci_1564: fix counter code in main driver source staging: comedi: addi_apci_1564: move counter register defines to driver staging: comedi: addi_apci_1564: split timer and counter subdevices staging: comedi: addi_tcw.h: provide generic defines for the ADDI-DATA TCW staging: comedi: addi_apci_1564: use addi_tcw.h defines for timer staging: comedi: addi_apci_1564: use addi_tcw.h defines for counters staging: comedi: addi_apci_1564: enable support for PLD Rev 1.0 I/O mapping staging: comedi: addi_watchdog: use addi_tcw.h defines for watchdog .../comedi/drivers/addi-data/hwdrv_apci1564.c | 371 + drivers/staging/comedi/drivers/addi_apci_1564.c| 260 ++- drivers/staging/comedi/drivers/addi_tcw.h | 56 drivers/staging/comedi/drivers/addi_watchdog.c | 30 +- 4 files changed, 404 insertions(+), 313 deletions(-) create mode 100644 drivers/staging/comedi/drivers/addi_tcw.h Not quite there yet, but hopefully the driver can be bent to comedi's will eventually. Thanks for resolving the ambiguities surrounding the register mappings! Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: comedi: adl_pci9118: use comedi_async 'scans_done' to detect EOA
On 11/11/14 00:57, H Hartley Sweeten wrote: The comedi core now counts the number of samples added to the async buffer and detects the end-of-scan and increments the comedi_async 'scans_done' counter. Remove the private data member 'ai_act_scan' and use the 'scans_done' member to detect the end-of-acquisition. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/adl_pci9118.c | 22 -- 1 file changed, 4 insertions(+), 18 deletions(-) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/4] staging: comedi: adl_pci9118: use comedi_bytes_to_samples()
On 11/11/14 00:57, H Hartley Sweeten wrote: Remove the assumption of the sample size by using the comedi_bytes_to_samples() helper function to convert the number of bytes to the number of samples. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/adl_pci9118.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: comedi: adl_pci9118: absorb move_block_from_dma()
On 11/11/14 00:57, H Hartley Sweeten wrote: Absorb this simple helper function into interrupt_pci9118_ai_dma(). Remove the unnecessary local variables 'sampls' and 'm'. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers/adl_pci9118.c | 24 +++- 1 file changed, 7 insertions(+), 17 deletions(-) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 12/30] staging: comedi: dmm32at: use 8255 module for Digital I/O subdevice
On 11/11/14 23:55, H Hartley Sweeten wrote: The Dimond-MM-32-AT board uses an internal 82C55-type digital I/O circuit to provide the 24 digital I/O lines. The only quirk is the need to set the page selection bits in the control register to select page 1 addresses. Instead of duplicating the 8255 code, provide an (*io) callback and use the 8255 module to support this subdevice. This also removes the need for the private data in this driver. The patch is fine, but there's a bug in the original code which means we might need the private data back (unless the DMM32AT_CNTRL register is read-write rather than read-only). The bug is that the ISR routine clobbers the page selection bits in the register. To avoid that, the current page would either need to be stored in private data or read from the register (if possible) to avoid clobbering it, and updates to the register would need a spin-lock. -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/30] staging: comedi: dmm32at: cleanup driver
On 11/11/14 23:55, H Hartley Sweeten wrote:
Here's the big cleanup series for the dmm32at comedi driver.
H Hartley Sweeten (30):
staging: comedi: dmm32at: make AI (*cancel) actually cancel async command
staging: comedi: dmm32at: use comedi_async 'scans_done' to detect EOA
staging: comedi: dmm32at: introduce dmm32_ai_get_sample()
staging: comedi: dmm32at: tidy up dmm32at_ai_rinsn()
staging: comedi: dmm32at: introduce dmm32at_reset()
staging: comedi: dmm32at: tidy up subdevice initialization
staging: comedi: dmm32at: tidy up cmd->scan_begin_{src,arg} validation
staging: comedi: dmm32at: tidy up cmd->convert_{src,arg} validation
staging: comedi: dmm32at: remove dmm32at_ns_to_timer()
staging: comedi: dmm32at: remove unused members of the private data
staging: comedi: dmm32at: introduce dmm32at_ai_set_chanspec()
staging: comedi: dmm32at: use 8255 module for Digital I/O subdevice
staging: comedi: dmm32at: rename DMM32AT_CONV
staging: comedi: dmm32at: rename DMM32AT_AI[LM]SB
staging: comedi: dmm32at: rename DMM32AT_AUXDOUT
staging: comedi: dmm32at: rename DMM32AT_AI{LOW,HIGH}
staging: comedi: dmm32at: rename DMM32AT_DAC[LM]SB
staging: comedi: dmm32at: rename DMM32AT_DACSTAT
staging: comedi: dmm32at: rename DMM32AT_DACMSB_CHAN
staging: comedi: dmm32at: define the FIFO Depth register
staging: comedi: dmm32at: rename DMM32AT_FIFOCNTRL
staging: comedi: dmm32at: rename DMM32AT_FIFOSTAT
staging: comedi: dmm32at: rename DMM32AT_CNTRL
staging: comedi: dmm32at: rename DMM32AT_AISTAT
staging: comedi: dmm32at: rename DMM32AT_INTCLOCK
staging: comedi: dmm32at: rename DMM32AT_CNTRDIO
staging: comedi: dmm32at: rename DMM32AT_AICONF
staging: comedi: dmm32at: rename DMM32AT_AIRBACK
staging: comedi: dmm32at: tidy up multi-line comments
staging: comedi: dmm32at: update the MODULE_DESCRIPTION
drivers/staging/comedi/Kconfig | 1 +
drivers/staging/comedi/drivers/dmm32at.c | 700 ---
2 files changed, 275 insertions(+), 426 deletions(-)
Reviewed-by: Ian Abbott
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: fix scan_end_arg == chanlist_len assumption
Some comedi drivers allow the `scan_end_arg` value of an asynchronous
command to be a multiple (> 1) of the `chanlist_len` although most
require them to be the same value.
`comedi_bytes_per_scan()` is incorrectly using `chanlist_len` as the
length of the scan. Change it to use `scan_end_arg`.
`comedi_nsamples_left()` is incorrectly using `cur_chan` as the current
sample position in the scan (it is actually the current position in the
channel list). Change it to use the actual sample position in the scan.
(Unfortunately we only have the current scan position in bytes currently,
so convert that to a sample position.)
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 82ac845..e516ed9 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -317,10 +317,10 @@ unsigned int comedi_bytes_per_scan(struct
comedi_subdevice *s)
case COMEDI_SUBD_DO:
case COMEDI_SUBD_DIO:
bits_per_sample = 8 * comedi_bytes_per_sample(s);
- num_samples = DIV_ROUND_UP(cmd->chanlist_len, bits_per_sample);
+ num_samples = DIV_ROUND_UP(cmd->scan_end_arg, bits_per_sample);
break;
default:
- num_samples = cmd->chanlist_len;
+ num_samples = cmd->scan_end_arg;
break;
}
return comedi_samples_to_bytes(s, num_samples);
@@ -384,11 +384,13 @@ unsigned int comedi_nsamples_left(struct comedi_subdevice
*s,
/* +1 to force comedi_nscans_left() to return the scans left */
unsigned int nscans = (nsamples / cmd->scan_end_arg) + 1;
unsigned int scans_left = comedi_nscans_left(s, nscans);
+ unsigned int scan_pos =
+ comedi_bytes_to_samples(s, async->scan_progress);
unsigned long long samples_left = 0;
if (scans_left) {
samples_left = ((unsigned long long)scans_left *
- cmd->scan_end_arg) - async->cur_chan;
+ cmd->scan_end_arg) - scan_pos;
}
if (samples_left < nsamples)
--
2.1.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 12/30] staging: comedi: dmm32at: use 8255 module for Digital I/O subdevice
On 12/11/14 16:07, Hartley Sweeten wrote: On Wednesday, November 12, 2014 3:12 AM, Ian Abbott wrote: On 11/11/14 23:55, H Hartley Sweeten wrote: The Dimond-MM-32-AT board uses an internal 82C55-type digital I/O circuit to provide the 24 digital I/O lines. The only quirk is the need to set the page selection bits in the control register to select page 1 addresses. Instead of duplicating the 8255 code, provide an (*io) callback and use the 8255 module to support this subdevice. This also removes the need for the private data in this driver. The patch is fine, but there's a bug in the original code which means we might need the private data back (unless the DMM32AT_CNTRL register is read-write rather than read-only). The bug is that the ISR routine clobbers the page selection bits in the register. To avoid that, the current page would either need to be stored in private data or read from the register (if possible) to avoid clobbering it, and updates to the register would need a spin-lock. Ian, Thanks for the review. The write to the Miscellaneous Control register (DMM32AT_CTRL_REG) in the ISR routine is actually safe. According to the user manual: INTRST Writing a 1 to this location resets the interrupt request circuit on the board. The programmer must write a 1 to this bit during the interrupt service routine, or further interrupts will not occur. Writing a 1 to this bit does not disturb the values of the PAGE bits. Oh right, so the other bits in the written value get ignored if that bit is set. The only other writes to the DMM32AT_CTRL_REG are in: dmm32at_reset() - sets the RESETA bit to cause a full reset of the board dmm32at_ai_cmd() - sets INTRST to reset the interrupt (this is probably not necessary, maybe it should be moved to the (*cancel)). dmm32at_setaitimer() - selects the 8254 page dmm32at_8255_io() - selects the 8255 page The only place the protection is needed is in the dmm32at_setaitimer() and dmm32at_8255_io() functions. The PAGE bits are readable in the FIFO Status register (DMM32AT_FIFO_STATUS_REG) but a spin-lock is probably safer. Could we use the comedi_device 'spinlock' for this? The core does not seem to use it. Is this just ageneral purpose spin-lock for the drivers? It really needs a comment of some sort. Yes, it's general purpose for the driver and the comedi core doesn't use it itself. I don't think the spin-lock is strictly necessary in the dmm32at_setaitimer() and dmm32at_8255_io() functions as they currently only get called while dev->mutex is locked. -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/4] staging: comedi: das6402: add basic (*do_cmdtest) for AI async commands
On 12/11/14 23:00, H Hartley Sweeten wrote:
Currently the async command support in this driver consists of just the
stubbed in functions.
Flesh out the (*do_cmdtest) function for basic support of timed analog
input acquisitions.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
drivers/staging/comedi/drivers/das6402.c | 105 ++-
1 file changed, 104 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/das6402.c
b/drivers/staging/comedi/drivers/das6402.c
index f3909f3..511b6b3 100644
--- a/drivers/staging/comedi/drivers/das6402.c
+++ b/drivers/staging/comedi/drivers/das6402.c
@@ -35,6 +35,7 @@
#include
#include "../comedidev.h"
+#include "comedi_fc.h"
#include "8253.h"
/*
@@ -207,11 +208,113 @@ static int das6402_ai_cmd(struct comedi_device *dev,
return -EINVAL;
}
+static int das6402_ai_check_chanlist(struct comedi_device *dev,
+struct comedi_subdevice *s,
+struct comedi_cmd *cmd)
+{
+ unsigned int chan0 = CR_CHAN(cmd->chanlist[0]);
+ unsigned int range0 = CR_RANGE(cmd->chanlist[0]);
+ unsigned int aref0 = CR_AREF(cmd->chanlist[0]);
+ int i;
+
+ for (i = 1; i < cmd->chanlist_len; i++) {
+ unsigned int chan = CR_CHAN(cmd->chanlist[i]);
+ unsigned int range = CR_RANGE(cmd->chanlist[i]);
+ unsigned int aref = CR_AREF(cmd->chanlist[i]);
+
+ if (chan != chan0 + i) {
+ dev_dbg(dev->class_dev,
+ "chanlist must be consecutive\n");
+ return -EINVAL;
+ }
+
+ if (range != range0) {
+ dev_dbg(dev->class_dev,
+ "chanlist must have the same range\n");
+ return -EINVAL;
+ }
+
+ if (aref != aref0) {
+ dev_dbg(dev->class_dev,
+ "chanlist must have the reference\n");
Shouldn't that say the _same_ reference?
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/4] staging: comedi: das6402: add basic AI async command support
On 12/11/14 23:00, H Hartley Sweeten wrote: This driver was _really_ broken and had to be rewritten. During that rewrite the async command support was stubbed in but not completed. This series adds support for basic AI async commands. H Hartley Sweeten (4): staging: comedi: das6402: add basic (*do_cmdtest) for AI async commands staging: comedi: das6402: introduce das6402_ai_set_mode() staging: comedi: das6402: read analog input samples in interrupt handler staging: comedi: das6402: add (*do_cmd) for AI async commands drivers/staging/comedi/drivers/das6402.c | 198 +++ 1 file changed, 178 insertions(+), 20 deletions(-) Just a minor niggle in one of the debug messages in patch 1. Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 12/30] staging: comedi: dmm32at: use 8255 module for Digital I/O subdevice
On 12/11/14 16:46, Hartley Sweeten wrote: OK, so no change needed. Side-note... Currently the board reset also probes the board to try and "verify" that the board is actually a dmm32at. Do you think this is actually necessary? Not strictly necessary, but may help if the user configures it incorrectly. I feel the board reset just needs to set the RESETA bit to reset the board. This causes a full reset of all features of the board, including the DACs, the FIFO, the digital I/O, and all internal registers. The other writes to the board registers are not needed since they are only setup the registers for the verify check. The udelay() calls can also probably be removed since there is no mention of a delay needed after the reset in the user manual. Without knowing for sure (it might be something needed that was neglected from the user manual), I'd leave it in or change it to use usleep_range(), e.g. usleep_range(1000, 1). There is still the issue of the SD1/0 bits in the AI status register. These provide feedback of how the jumpers are set to configure the AI channels for single-ended or differential input. This could be worked out so the differential mode actually works. The all single-ended and all-differential cases should be straightforward. The "half and half" (or rather "third and two-thirds") cases would be tricky as you may end up with a gap in the channel numbers, and the support for subdevices with channel-specific flags (SDF_FLAGS) is currently disabled in the comedi core. -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: me4000: Fixed code style issue
On 15/11/14 15:55, Marcus Hufvudsson wrote: Fixed checkpatch.pl error message. Space prohibited before that ',' Signed-off-by: Marcus Hufvudsson --- drivers/staging/comedi/drivers/me4000.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/me4000.c b/drivers/staging/comedi/drivers/me4000.c index ae6ac49..fc67419 100644 --- a/drivers/staging/comedi/drivers/me4000.c +++ b/drivers/staging/comedi/drivers/me4000.c @@ -416,7 +416,7 @@ static void me4000_reset(struct comedi_device *dev) val |= PLX9052_CNTRL_PCI_RESET; outl(val, info->plx_regbase + PLX9052_CNTRL); val &= ~PLX9052_CNTRL_PCI_RESET; - outl(val , info->plx_regbase + PLX9052_CNTRL); + outl(val, info->plx_regbase + PLX9052_CNTRL); /* 0x8000 to the DACs means an output voltage of 0V */ for (chan = 0; chan < 4; chan++) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/19] staging: comedi: drivers: have core hook up default (*insn_read) for readback
On 20/11/14 22:07, H Hartley Sweeten wrote: Most of the comedi drivers that provide readback for write only subdevices now use the comedi core comedi_alloc_subdev_readback() helper to allocate the subdevice 'reaback' member instead of using some member in their private data. These drivers also hook up the (*insn_read) callback to the comedi_readback_insn_read() helper to provide the readback. Have the core automatically hook up the (*insn_read) callback after allocating the memory. For the drivers that use a private callback, hook it up after the readback bas been allocated and add a comment about the override of the default. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- drivers/staging/comedi/drivers.c | 3 +++ drivers/staging/comedi/drivers/addi_apci_3120.c | 1 - drivers/staging/comedi/drivers/addi_apci_3501.c | 1 - drivers/staging/comedi/drivers/addi_apci_3xxx.c | 1 - drivers/staging/comedi/drivers/adl_pci6208.c | 1 - drivers/staging/comedi/drivers/adl_pci9111.c | 1 - drivers/staging/comedi/drivers/adl_pci9118.c | 1 - drivers/staging/comedi/drivers/aio_aio12_8.c | 1 - drivers/staging/comedi/drivers/amplc_pci224.c| 1 - drivers/staging/comedi/drivers/amplc_pci230.c| 1 - drivers/staging/comedi/drivers/cb_das16_cs.c | 1 - drivers/staging/comedi/drivers/cb_pcidas.c | 1 - drivers/staging/comedi/drivers/cb_pcidas64.c | 1 - drivers/staging/comedi/drivers/cb_pcimdas.c | 1 - drivers/staging/comedi/drivers/cb_pcimdda.c | 3 ++- drivers/staging/comedi/drivers/dac02.c | 1 - drivers/staging/comedi/drivers/daqboard2000.c| 1 - drivers/staging/comedi/drivers/das08.c | 1 - drivers/staging/comedi/drivers/das16.c | 1 - drivers/staging/comedi/drivers/das6402.c | 3 ++- drivers/staging/comedi/drivers/dmm32at.c | 1 - drivers/staging/comedi/drivers/dt2801.c | 1 - drivers/staging/comedi/drivers/dt2811.c | 1 - drivers/staging/comedi/drivers/dt282x.c | 1 - drivers/staging/comedi/drivers/dt3000.c | 1 - drivers/staging/comedi/drivers/dt9812.c | 3 ++- drivers/staging/comedi/drivers/fl512.c | 1 - drivers/staging/comedi/drivers/icp_multi.c | 1 - drivers/staging/comedi/drivers/ii_pci20kc.c | 1 - drivers/staging/comedi/drivers/me4000.c | 1 - drivers/staging/comedi/drivers/me_daq.c | 1 - drivers/staging/comedi/drivers/mf6x4.c | 1 - drivers/staging/comedi/drivers/multiq3.c | 1 - drivers/staging/comedi/drivers/ni_670x.c | 1 - drivers/staging/comedi/drivers/ni_at_ao.c| 1 - drivers/staging/comedi/drivers/ni_atmio16d.c | 1 - drivers/staging/comedi/drivers/ni_mio_common.c | 1 - drivers/staging/comedi/drivers/pcl711.c | 1 - drivers/staging/comedi/drivers/pcl726.c | 1 - drivers/staging/comedi/drivers/pcl812.c | 1 - drivers/staging/comedi/drivers/pcl818.c | 1 - drivers/staging/comedi/drivers/pcmda12.c | 3 ++- drivers/staging/comedi/drivers/pcmmio.c | 1 - drivers/staging/comedi/drivers/quatech_daqp_cs.c | 1 - drivers/staging/comedi/drivers/rtd520.c | 1 - drivers/staging/comedi/drivers/rti800.c | 1 - drivers/staging/comedi/drivers/rti802.c | 1 - drivers/staging/comedi/drivers/s526.c| 1 - drivers/staging/comedi/drivers/s626.c| 1 - drivers/staging/comedi/drivers/usbdux.c | 3 ++- drivers/staging/comedi/drivers/usbduxsigma.c | 3 ++- 51 files changed, 15 insertions(+), 50 deletions(-) diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c index e516ed9..051f004 100644 --- a/drivers/staging/comedi/drivers.c +++ b/drivers/staging/comedi/drivers.c @@ -109,6 +109,9 @@ int comedi_alloc_subdev_readback(struct comedi_subdevice *s) s->readback = kcalloc(s->n_chan, sizeof(*s->readback), GFP_KERNEL); if (!s->readback) return -ENOMEM; + + s->insn_read = comedi_readback_insn_read; + Maybe it should only do that if s->insn_read is NULL. Then it wouldn't matter if a low-level driver overrides it before or after calling comedi_alloc_subdev_readback(). -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/19] staging: comedi: more comedi_subdevice 'readback' cleanup
staging/comedi/drivers/rti802.c | 1 - drivers/staging/comedi/drivers/s526.c| 1 - drivers/staging/comedi/drivers/s626.c| 1 - drivers/staging/comedi/drivers/usbdux.c | 3 +- drivers/staging/comedi/drivers/usbduxsigma.c | 3 +- 53 files changed, 230 insertions(+), 341 deletions(-) I suggested a minor change to patch 01, but it's not that important. Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/19] staging: comedi: adv_pci1724: cleanup driver
On 20/11/14 22:10, H Hartley Sweeten wrote: Following is the big cleanup for the adv_pci1724 driver. H Hartley Sweeten (19): staging: comedi: adv_pci1724: use subdevice readback for 'ao_value' staging: comedi: adv_pci1724: use subdevice readback for 'offset_value' staging: comedi: adv_pci1724: use subdevice readback for 'gain_value' staging: comedi: adv_pci1724: remove NUM_AO_CHANNELS define staging: comedi: adv_pci1724: introduce adv_pci1724_insn_write() staging: comedi: adv_pci1724: use comedi_timeout() to wait for DAC idle state staging: comedi: adv_pci1724: absorb set_dac() staging: comedi: adv_pci1724: remove PCI_VENDOR_ID_ADVANTECH define staging: comedi: adv_pci1724: tidy up the register I/O map staging: comedi: adv_pci1724: define the dac control register bits staging: comedi: adv_pci1724: define the sync output control/status reg staging: comedi: adv_pci1724: remove enum sync_output_trigger_contents staging: comedi: adv_pci1724: define the board id register bits staging: comedi: adv_pci1724: absorb setup_subdevices() staging: comedi: adv_pci1724: remove unnecessary dev_info() staging: comedi: adv_pci1724: tidy up the pci_driver declaration staging: comedi: adv_pci1724: tidy up the comedi_driver declaration staging: comedi: adv_pci1724: tidy up multi-line comments staging: comedi: adv_pci1724: rename ao_ranges_1724 drivers/staging/comedi/drivers/adv_pci1724.c | 471 +-- 1 file changed, 145 insertions(+), 326 deletions(-) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 01/19] staging: comedi: drivers: have core hook up default (*insn_read) for readback
On 21/11/2014 17:19, H Hartley Sweeten wrote: Most of the comedi drivers that provide readback for write only subdevices now use the comedi core comedi_alloc_subdev_readback() helper to allocate the subdevice 'reaback' member instead of using some member in their private data. These drivers also hook up the (*insn_read) callback to the comedi_readback_insn_read() helper to provide the readback. Have the core automatically hook up the (*insn_read) callback after allocating the memory if the driver has not already hooked it up to a private function. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Greg Kroah-Hartman --- v2: Only hook up the default (*insn_read) if the driver has not already hooked it to a private function (suggested by Ian Abbott). The rest of this series is unchanged. drivers/staging/comedi/drivers.c | 4 drivers/staging/comedi/drivers/addi_apci_3120.c | 1 - drivers/staging/comedi/drivers/addi_apci_3501.c | 1 - drivers/staging/comedi/drivers/addi_apci_3xxx.c | 1 - drivers/staging/comedi/drivers/adl_pci6208.c | 1 - drivers/staging/comedi/drivers/adl_pci9111.c | 1 - drivers/staging/comedi/drivers/adl_pci9118.c | 1 - drivers/staging/comedi/drivers/aio_aio12_8.c | 1 - drivers/staging/comedi/drivers/amplc_pci224.c| 1 - drivers/staging/comedi/drivers/amplc_pci230.c| 1 - drivers/staging/comedi/drivers/cb_das16_cs.c | 1 - drivers/staging/comedi/drivers/cb_pcidas.c | 1 - drivers/staging/comedi/drivers/cb_pcidas64.c | 1 - drivers/staging/comedi/drivers/cb_pcimdas.c | 1 - drivers/staging/comedi/drivers/dac02.c | 1 - drivers/staging/comedi/drivers/daqboard2000.c| 1 - drivers/staging/comedi/drivers/das08.c | 1 - drivers/staging/comedi/drivers/das16.c | 1 - drivers/staging/comedi/drivers/dmm32at.c | 1 - drivers/staging/comedi/drivers/dt2801.c | 1 - drivers/staging/comedi/drivers/dt2811.c | 1 - drivers/staging/comedi/drivers/dt282x.c | 1 - drivers/staging/comedi/drivers/dt3000.c | 1 - drivers/staging/comedi/drivers/fl512.c | 1 - drivers/staging/comedi/drivers/icp_multi.c | 1 - drivers/staging/comedi/drivers/ii_pci20kc.c | 1 - drivers/staging/comedi/drivers/me4000.c | 1 - drivers/staging/comedi/drivers/me_daq.c | 1 - drivers/staging/comedi/drivers/mf6x4.c | 1 - drivers/staging/comedi/drivers/multiq3.c | 1 - drivers/staging/comedi/drivers/ni_670x.c | 1 - drivers/staging/comedi/drivers/ni_at_ao.c| 1 - drivers/staging/comedi/drivers/ni_atmio16d.c | 1 - drivers/staging/comedi/drivers/ni_mio_common.c | 1 - drivers/staging/comedi/drivers/pcl711.c | 1 - drivers/staging/comedi/drivers/pcl726.c | 1 - drivers/staging/comedi/drivers/pcl812.c | 1 - drivers/staging/comedi/drivers/pcl818.c | 1 - drivers/staging/comedi/drivers/pcmmio.c | 1 - drivers/staging/comedi/drivers/quatech_daqp_cs.c | 1 - drivers/staging/comedi/drivers/rtd520.c | 1 - drivers/staging/comedi/drivers/rti800.c | 1 - drivers/staging/comedi/drivers/rti802.c | 1 - drivers/staging/comedi/drivers/s526.c| 1 - drivers/staging/comedi/drivers/s626.c| 1 - 45 files changed, 4 insertions(+), 44 deletions(-) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/11] staging: comedi: adv_pci1723: cleanup driver
On 21/11/14 21:22, H Hartley Sweeten wrote: Following is the big cleanup for the adv_pci1723 driver. H Hartley Sweeten (11): staging: comedi: adv_pci1723: tidy up register map staging: comedi: adv_pci1723: remove private data 'da_range' staging: comedi: adv_pci1723: remove board reset during (*detach) staging: comedi: adv_pci1723: absorb pci1723_reset() staging: comedi: adv_pci1723: use comedi_subdevice readback for 'ao_data' staging: comedi: adv_pci1723: tidy up pci1723_dio_insn_config() staging: comedi: adv_pci1723: tidy up DIO io_bits initialization staging: comedi: adv_pci1723: remove subdevice 'len_chanlist' initialization staging: comedi: adv_pci1723: update the MODULE_DESCRIPTION staging: comedi: adv_pci1723: tidy up multi-line comments staging: comedi: adv_pci1723: remove comedi_device 'write_subdev' init drivers/staging/comedi/drivers/adv_pci1723.c | 336 ++- 1 file changed, 124 insertions(+), 212 deletions(-) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: comedi: adl_pci9118: try and avoid unnecessary DMA restart
`interrupt_pci9118_ai_dma()` is called on interrupt to transfer data
from DMA buffers into the comedi async data buffer. Currently it always
restarts DMA. If double buffering, it restarts DMA on the next DMA
buffer before processing the current DMA buffer, otherwise it restarts
DMA on the same DMA buffer after it has been processed.
For single buffering we can avoid restarting the DMA transfer by
checking the async event flags after the current buffer has been
processed, which is easy.
For double buffering, we need to know how many valid samples there are
in the current buffer before it has been processed and determine whether
there is enough to complete the acquisition. Call new function
`valid_samples_in_act_dma_buf()` to determine the number of valid
samples in the current DMA buffer, and compare that with the result of
`comedi_nsamples_left()` to determine if DMA needs to be restarted.
(`comedi_nsamples_left()` needs an upper bound to clamp to, so use the
number of valid samples in the DMA buffer plus one for our test.)
It is still possible for DMA to be restarted unnecessarily in the double
buffer case if a `COMEDI_CB_OVERFLOW` event occurs while copying to the
comedi async buffer, but it doesn't really matter. The ongoing DMA
operation will get disabled when the subdevice's `cancel()` handler is
called when the events are handled later in the interrupt service
routine (as it does currently).
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/adl_pci9118.c | 78 +---
1 file changed, 71 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c
b/drivers/staging/comedi/drivers/adl_pci9118.c
index aecfae8..c0ea733 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -446,6 +446,62 @@ static void interrupt_pci9118_ai_mode4_switch(struct
comedi_device *dev,
outl(devpriv->ai_cfg, dev->iobase + PCI9118_AI_CFG_REG);
}
+static unsigned int valid_samples_in_act_dma_buf(struct comedi_device *dev,
+struct comedi_subdevice *s,
+unsigned int n_raw_samples)
+{
+ struct pci9118_private *devpriv = dev->private;
+ struct comedi_cmd *cmd = &s->async->cmd;
+ unsigned int start_pos = devpriv->ai_add_front;
+ unsigned int stop_pos = start_pos + cmd->chanlist_len;
+ unsigned int span_len = stop_pos + devpriv->ai_add_back;
+ unsigned int dma_pos = devpriv->ai_act_dmapos;
+ unsigned int whole_spans, n_samples, x;
+
+ if (span_len == cmd->chanlist_len)
+ return n_raw_samples; /* use all samples */
+
+ /*
+* Not all samples are to be used. Buffer contents consist of a
+* possibly non-whole number of spans and a region of each span
+* is to be used.
+*
+* Account for samples in whole number of spans.
+*/
+ whole_spans = n_raw_samples / span_len;
+ n_samples = whole_spans * cmd->chanlist_len;
+ n_raw_samples -= whole_spans * span_len;
+
+ /*
+* Deal with remaining samples which could overlap up to two spans.
+*/
+ while (n_raw_samples) {
+ if (dma_pos < start_pos) {
+ /* Skip samples before start position. */
+ x = start_pos - dma_pos;
+ if (x > n_raw_samples)
+ x = n_raw_samples;
+ dma_pos += x;
+ n_raw_samples -= x;
+ if (!n_raw_samples)
+ break;
+ }
+ if (dma_pos < stop_pos) {
+ /* Include samples before stop position. */
+ x = stop_pos - dma_pos;
+ if (x > n_raw_samples)
+ x = n_raw_samples;
+ n_samples += x;
+ dma_pos += x;
+ n_raw_samples -= x;
+ }
+ /* Advance to next span. */
+ start_pos += span_len;
+ stop_pos += span_len;
+ }
+ return n_samples;
+}
+
static unsigned int defragment_dma_buffer(struct comedi_device *dev,
struct comedi_subdevice *s,
unsigned short *dma_buffer,
@@ -607,10 +663,16 @@ static void interrupt_pci9118_ai_dma(struct comedi_device
*dev,
struct pci9118_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
struct pci9118_dmabuf *dmabuf = &devpriv->dmabuf[devpriv->dma_actbuf];
- unsigned int nsamples = comedi_bytes_to_samples(s, dmabuf->use_size);
+ unsigned int n_all = comedi_bytes_to_samples(s, dmabuf->use_size);
+
[PATCH 0/3] staging: comedi: adl_pci9118: some dma transfer changes
For streaming acquisition on the analog input subdevice, this driver normally uses DMA double buffering into two internal DMA buffers, so it can switch buffers early after a DMA transfer has completed, while it processes the completed DMA buffer. PATCH 1 is just a bit of tidy up. PATCH 2 avoids switching the DMA double buffer at the end of acquisition. For DMA single buffering, it avoids restarting the DMA transfer if acquisition has ended normally or due to a comedi buffer overflow error. (However, for double buffering, the DMA transfer has already been started on the other DMA buffer and will be stopped soon after.) PATCH 3 eliminates a possible defragmentation step in the DMA buffer contents before they are transferred to the comedi buffer. Instead, the DMA buffer fragments are copied directly to the comedi buffer, or is copied all in one go if the data is not fragmented. This work is in response to Hartley's "PATCH 4/4] staging: comedi: adl_pci9118: switch DMA buffers after writing samples", dated Mon, 10 Nov 2014 1959:17 -0500, message ID <1415667477-28403-5-git-send-email-hswee...@visionengravers.com>, which was not applied. These patches are against Greg's "staging-testing" branch. 1) staging: comedi: adl_pci9118: simplify interrupt_pci9118_ai_dma() a bit 2) staging: comedi: adl_pci9118: try and avoid unnecessary DMA restart 3) staging: comedi: adl_pci9118: eliminate DMA buffer defragmentation step drivers/staging/comedi/drivers/adl_pci9118.c | 157 +-- 1 file changed, 121 insertions(+), 36 deletions(-) ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: comedi: adl_pci9118: simplify interrupt_pci9118_ai_dma() a bit
Eliminate the `next_dma_buf` variable in `interrupt_pci9118_ai_dma()`.
It holds the next value of `devpriv->dma_actbuf` when double buffering
is used, but we can just set that to the next value directly at the
point where the buffers are switched as the old value is not used
anywhere else.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/adl_pci9118.c | 24 ++--
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c
b/drivers/staging/comedi/drivers/adl_pci9118.c
index 5e0ff9d..aecfae8 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -608,16 +608,15 @@ static void interrupt_pci9118_ai_dma(struct comedi_device
*dev,
struct comedi_cmd *cmd = &s->async->cmd;
struct pci9118_dmabuf *dmabuf = &devpriv->dmabuf[devpriv->dma_actbuf];
unsigned int nsamples = comedi_bytes_to_samples(s, dmabuf->use_size);
- unsigned int next_dma_buf;
- if (devpriv->dma_doublebuf) { /*
-* switch DMA buffers if is used
-* double buffering
-*/
- next_dma_buf = 1 - devpriv->dma_actbuf;
- pci9118_amcc_setup_dma(dev, next_dma_buf);
- if (devpriv->ai_do == 4)
- interrupt_pci9118_ai_mode4_switch(dev, next_dma_buf);
+ /* switch DMA buffers and restart DMA if double buffering */
+ if (devpriv->dma_doublebuf) {
+ devpriv->dma_actbuf = 1 - devpriv->dma_actbuf;
+ pci9118_amcc_setup_dma(dev, devpriv->dma_actbuf);
+ if (devpriv->ai_do == 4) {
+ interrupt_pci9118_ai_mode4_switch(dev,
+ devpriv->dma_actbuf);
+ }
}
if (nsamples) {
@@ -631,11 +630,8 @@ static void interrupt_pci9118_ai_dma(struct comedi_device
*dev,
s->async->events |= COMEDI_CB_EOA;
}
- if (devpriv->dma_doublebuf) {
- /* switch dma buffers */
- devpriv->dma_actbuf = 1 - devpriv->dma_actbuf;
- } else {
- /* restart DMA if is not used double buffering */
+ /* restart DMA if not double buffering */
+ if (!devpriv->dma_doublebuf) {
pci9118_amcc_setup_dma(dev, 0);
if (devpriv->ai_do == 4)
interrupt_pci9118_ai_mode4_switch(dev, 0);
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: comedi: adl_pci9118: eliminate DMA buffer defragmentation step
The DMA operations used by the driver may have been set up to acquire
data from unwanted channels in addition to the wanted channels.
Currently, `interrupt_pci9118_ai_dma()` calls `defragment_dma_buffer()`
to move all the wanted data to the start of the DMA buffer and then
calls `comedi_buf_write_samples()` to copy it all to the comedi async
buffer. Those two functions used to be called from
`move_block_from_dma()` which was absorbed into
`interrupt_pci9118_ai_dma()`.
Reinstate `move_block_from_dma()` but rewrite it to copy data directly
from the wanted fragments of the DMA buffer to the comedi async buffer
without defragmenting the buffer first.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/adl_pci9118.c | 67 +++-
1 file changed, 46 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/comedi/drivers/adl_pci9118.c
b/drivers/staging/comedi/drivers/adl_pci9118.c
index c0ea733..2660358 100644
--- a/drivers/staging/comedi/drivers/adl_pci9118.c
+++ b/drivers/staging/comedi/drivers/adl_pci9118.c
@@ -502,29 +502,56 @@ static unsigned int valid_samples_in_act_dma_buf(struct
comedi_device *dev,
return n_samples;
}
-static unsigned int defragment_dma_buffer(struct comedi_device *dev,
- struct comedi_subdevice *s,
- unsigned short *dma_buffer,
- unsigned int num_samples)
+static void move_block_from_dma(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ unsigned short *dma_buffer,
+ unsigned int n_raw_samples)
{
struct pci9118_private *devpriv = dev->private;
struct comedi_cmd *cmd = &s->async->cmd;
- unsigned int i = 0, j = 0;
- unsigned int start_pos = devpriv->ai_add_front,
- stop_pos = devpriv->ai_add_front + cmd->chanlist_len;
- unsigned int raw_scanlen = devpriv->ai_add_front + cmd->chanlist_len +
- devpriv->ai_add_back;
+ unsigned int start_pos = devpriv->ai_add_front;
+ unsigned int stop_pos = start_pos + cmd->chanlist_len;
+ unsigned int span_len = stop_pos + devpriv->ai_add_back;
+ unsigned int dma_pos = devpriv->ai_act_dmapos;
+ unsigned int x;
- for (i = 0; i < num_samples; i++) {
- if (devpriv->ai_act_dmapos >= start_pos &&
- devpriv->ai_act_dmapos < stop_pos) {
- dma_buffer[j++] = dma_buffer[i];
+ if (span_len == cmd->chanlist_len) {
+ /* All samples are to be copied. */
+ comedi_buf_write_samples(s, dma_buffer, n_raw_samples);
+ dma_pos += n_raw_samples;
+ } else {
+ /*
+* Not all samples are to be copied. Buffer contents consist
+* of a possibly non-whole number of spans and a region of
+* each span is to be copied.
+*/
+ while (n_raw_samples) {
+ if (dma_pos < start_pos) {
+ /* Skip samples before start position. */
+ x = start_pos - dma_pos;
+ if (x > n_raw_samples)
+ x = n_raw_samples;
+ dma_pos += x;
+ n_raw_samples -= x;
+ if (!n_raw_samples)
+ break;
+ }
+ if (dma_pos < stop_pos) {
+ /* Copy samples before stop position. */
+ x = stop_pos - dma_pos;
+ if (x > n_raw_samples)
+ x = n_raw_samples;
+ comedi_buf_write_samples(s, dma_buffer, x);
+ dma_pos += x;
+ n_raw_samples -= x;
+ }
+ /* Advance to next span. */
+ start_pos += span_len;
+ stop_pos += span_len;
}
- devpriv->ai_act_dmapos++;
- devpriv->ai_act_dmapos %= raw_scanlen;
}
-
- return j;
+ /* Update position in span for next time. */
+ devpriv->ai_act_dmapos = dma_pos % span_len;
}
static void pci9118_exttrg_enable(struct comedi_device *dev, bool enable)
@@ -681,10 +708,8 @@ static void interrupt_pci9118_ai_dma(struct comedi_device
*dev,
}
}
- if (n_all) {
- n_valid = defragment_dma_buffer(dev, s, dmabuf->virt, n_all);
- comedi_buf_write_samples(s, dmabuf->virt, n_valid);
- }
+ if (n_all)
+
Re: [PATCH] staging: comedi: change some printk calls to pr_err
On 01/12/14 08:47, Jeremiah Mahler wrote:
Chase,
On Sun, Nov 30, 2014 at 11:05:56PM -0600, Chase Southwood wrote:
There are a handful of calls to printk in ni_stc.h without specified log
levels, as well as one in ni_mio_common.c. This patch converts these
calls to pr_err() instead, so that they are now explicitly log level
ERR.
Signed-off-by: Chase Southwood
---
I tacked the change to ni_mio_common.c on to this patch since it's the same
exact change and it's just one line, so I think a single patch is justified
here.
drivers/staging/comedi/drivers/ni_mio_common.c | 2 +-
drivers/staging/comedi/drivers/ni_stc.h| 14 +++---
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c
b/drivers/staging/comedi/drivers/ni_mio_common.c
index 353c17b..11e7017 100644
--- a/drivers/staging/comedi/drivers/ni_mio_common.c
+++ b/drivers/staging/comedi/drivers/ni_mio_common.c
@@ -3945,7 +3945,7 @@ static unsigned ni_gpct_to_stc_register(enum
ni_gpct_register reg)
stc_register = Interrupt_B_Enable_Register;
break;
default:
- printk("%s: unhandled register 0x%x in switch.\n",
+ pr_err("%s: unhandled register 0x%x in switch.\n",
__func__, reg);
BUG();
return 0;
[...]
On my system the default log level is 4 which corresponds to
KERN_WARNING. So switching to pr_err() would change the log level
of these messages. Using pr_warn() might be a better choice.
I wouldn't worry about the log level in this case as those printks would
only be reached due to driver bugs.
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: change some printk calls to pr_err
On 01/12/14 05:05, Chase Southwood wrote: There are a handful of calls to printk in ni_stc.h without specified log levels, as well as one in ni_mio_common.c. This patch converts these calls to pr_err() instead, so that they are now explicitly log level ERR. Signed-off-by: Chase Southwood --- I tacked the change to ni_mio_common.c on to this patch since it's the same exact change and it's just one line, so I think a single patch is justified here. drivers/staging/comedi/drivers/ni_mio_common.c | 2 +- drivers/staging/comedi/drivers/ni_stc.h| 14 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/11] staging: comedi: hwdrv_apci1500: introduce z8536_read()
On 02/12/14 17:19, H Hartley Sweeten wrote:
The Z8536 CIO registers are indirectly read by writing the register value
I think you mean the register offset. (That also applies to patch 2.)
to the control register then reading the control register. Introduce a helper
function to read the Z8536 CIO registers.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
.../comedi/drivers/addi-data/hwdrv_apci1500.c | 216 +
1 file changed, 52 insertions(+), 164 deletions(-)
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1500.c
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1500.c
index bfa9228..5d4d35b 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1500.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1500.c
@@ -129,6 +129,14 @@ static int i_InputChannel;
static int i_TimerCounter1Enabled, i_TimerCounter2Enabled,
i_WatchdogCounter3Enabled;
+static unsigned int z8536_read(struct comedi_device *dev, unsigned int reg)
+{
+ struct apci1500_private *devpriv = dev->private;
+
+ outb(reg, devpriv->iobase + APCI1500_Z8536_CONTROL_REGISTER);
+ return inb(devpriv->iobase + APCI1500_Z8536_CONTROL_REGISTER);
+}
+
/*
* An event can be generated for each port. The first event is related to the
* first 8 channels (port 1) and the second to the following 6 channels (port
2)
@@ -311,14 +319,8 @@ static int apci1500_di_config(struct comedi_device *dev,
devpriv->iobase +
APCI1500_Z8536_CONTROL_REGISTER);
- /* Selects the mode specification mask*/
- /* register of port 1 */
- outb(APCI1500_RW_PORT_A_SPECIFICATION,
- devpriv->iobase +
- APCI1500_Z8536_CONTROL_REGISTER);
- i_RegValue =
- inb(devpriv->iobase +
- APCI1500_Z8536_CONTROL_REGISTER);
+ i_RegValue = z8536_read(dev,
+ APCI1500_RW_PORT_A_SPECIFICATION);
/* Selects the mode specification mask*/
/* register of port 1 */
@@ -366,14 +368,8 @@ static int apci1500_di_config(struct comedi_device *dev,
outb(0x74,
devpriv->iobase +
APCI1500_Z8536_CONTROL_REGISTER);
- /* Selects the mode specification mask */
- /* register of port B */
- outb(APCI1500_RW_PORT_B_SPECIFICATION,
- devpriv->iobase +
- APCI1500_Z8536_CONTROL_REGISTER);
- i_RegValue =
- inb(devpriv->iobase +
- APCI1500_Z8536_CONTROL_REGISTER);
+ i_RegValue = z8536_read(dev,
+ APCI1500_RW_PORT_B_SPECIFICATION);
/* Selects the mode specification mask*/
/* register of port B */
@@ -416,14 +412,8 @@ static int apci1500_di_config(struct comedi_device *dev,
devpriv->iobase +
APCI1500_Z8536_CONTROL_REGISTER);
- /* Selects the mode specification mask*/
- /* register of port 2 */
- outb(APCI1500_RW_PORT_B_SPECIFICATION,
- devpriv->iobase +
- APCI1500_Z8536_CONTROL_REGISTER);
- i_RegValue =
- inb(devpriv->iobase +
- APCI1500_Z8536_CONTROL_REGISTER);
+ i_RegValue = z8536_read(dev,
+ APCI1500_RW_PORT_B_SPECIFICATION);
/* Selects the mode specification mask*/
/* register of port 2 */
outb(APCI1500_RW_PORT_B_SPECIFICATION,
@@ -500,12 +490,8 @@ static int apci1500_di_write(struct comedi_device *dev,
devpriv->iobase +
APCI1500_Z8536_CONTROL_REGISTER);
i_Event1InterruptStatus = 1;
- outb(APCI1500_RW_PORT_A_SPECIFICATION,
- devpriv->iobase +
-
APCI1500_Z8536_CONTROL_REGISTER);
- i_RegValue =
-
Re: [PATCH 01/11] staging: comedi: hwdrv_apci1500: introduce z8536_read()
On 02/12/14 17:19, H Hartley Sweeten wrote:
The Z8536 CIO registers are indirectly read by writing the register value
to the control register then reading the control register. Introduce a helper
function to read the Z8536 CIO registers.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
---
.../comedi/drivers/addi-data/hwdrv_apci1500.c | 216 +
1 file changed, 52 insertions(+), 164 deletions(-)
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1500.c
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1500.c
index bfa9228..5d4d35b 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1500.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1500.c
@@ -129,6 +129,14 @@ static int i_InputChannel;
static int i_TimerCounter1Enabled, i_TimerCounter2Enabled,
i_WatchdogCounter3Enabled;
+static unsigned int z8536_read(struct comedi_device *dev, unsigned int reg)
+{
+ struct apci1500_private *devpriv = dev->private;
+
+ outb(reg, devpriv->iobase + APCI1500_Z8536_CONTROL_REGISTER);
+ return inb(devpriv->iobase + APCI1500_Z8536_CONTROL_REGISTER);
+}
+
Since this is used by both the interrupt handler and "normal" code, it
should use a spin-lock (e.g. dev->spinlock could be used). That also
applies to z8536_write() in patch 2 and probably the "reset" sequence at
the start of z8536_reset() in patch 3.
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 00/11] staging: comedi: addi_apci_1500: initial cleanup
On 03/12/14 18:25, H Hartley Sweeten wrote: This driver is pretty broken but in the current state its difficult to figure out where to fix it. Introduce a couple helper functions to handle the read/write and reset of the Z8536 CIO chip. This removes quite a bit of lines of code and makes the driver a _bit_ easier to follow. Tidy up the register map defines. Fix the interrupt handler so that the IRQ is properly shared. v2: spinlock protect the Z8536 indirect register access as suggested by Ian Abbott H Hartley Sweeten (11): staging: comedi: hwdrv_apci1500: introduce z8536_read() staging: comedi: hwdrv_apci1500: introduce z8536_write() staging: comedi: hwdrv_apci1500: introduce z8536_reset() staging: comedi: addi_apci_1500: tidy up PCI Bar 1 register map staging: comedi: addi_apci_1500: remove private data 'iobase' staging: comedi: addi_apci_1500: remove private data 'i_IobaseReserved' staging: comedi: addi_apci_1500: use amcc_s5933.h defines staging: comedi: addi_apci_1500: rename private data 'i_IobaseAddon' staging: comedi: addi_apci_1500: tidy up PCI Bar 2 register map staging: comedi: addi_apci_1500: remove APCI1500_ADDRESS_RANGE staging: comedi: addi_apci_1500: handle shared interrupt .../comedi/drivers/addi-data/hwdrv_apci1500.c | 1505 +--- drivers/staging/comedi/drivers/addi_apci_1500.c| 32 +- 2 files changed, 404 insertions(+), 1133 deletions(-) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: comedi: dmm32at: fix style issues
On 25/12/14 20:28, David Decotigny wrote: Before: 1 ERROR: code indent should use tabs where possible 1 WARNING: please, no spaces at the start of a line After: (none) Signed-off-by: David Decotigny --- drivers/staging/comedi/drivers/dmm32at.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/dmm32at.c b/drivers/staging/comedi/drivers/dmm32at.c index 6df298a..31919b8 100644 --- a/drivers/staging/comedi/drivers/dmm32at.c +++ b/drivers/staging/comedi/drivers/dmm32at.c @@ -365,7 +365,7 @@ static void dmm32at_setaitimer(struct comedi_device *dev, unsigned int nansec) /* enable the ai conversion interrupt and the clock to start scans */ outb(DMM32AT_INTCLK_ADINT | DMM32AT_INTCLK_CLKEN | DMM32AT_INTCLK_CLKSEL, - dev->iobase + DMM32AT_INTCLK_REG); +dev->iobase + DMM32AT_INTCLK_REG); } static int dmm32at_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s) Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: comedi: drivers: fix space coding style issue in s626.c
On 29/12/14 09:12, jitendra kumar khasdev wrote:
This is a patch to the s626.c file that fixes up a space error
found by the checkpatch.pl tool
Signed-off-by: Jitendra Kumar Khasdev
---
drivers/staging/comedi/drivers/s626.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/s626.c
b/drivers/staging/comedi/drivers/s626.c
index 14932c5..fc497dd 100644
--- a/drivers/staging/comedi/drivers/s626.c
+++ b/drivers/staging/comedi/drivers/s626.c
@@ -118,7 +118,7 @@ static void s626_mc_enable(struct comedi_device *dev,
static void s626_mc_disable(struct comedi_device *dev,
unsigned int cmd, unsigned int reg)
{
- writel(cmd << 16 , dev->mmio + reg);
+ writel(cmd << 16, dev->mmio + reg);
mmiowb();
}
Presumably this is PATCH 1/2?
Reviewed-by: Ian Abbott
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/2] Staging: comedi: driver : fix max 80 character coding style issue in s626.c
On 29/12/14 09:27, jitendra kumar khasdev wrote:
This is a patch to the s626.c file that fixes up a maximum 80 character limit
warning found by the checkpatch.pl tool
Signed-off-by: Jitendra Kumar Khasdev
---
drivers/staging/comedi/drivers/s626.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers/s626.c
b/drivers/staging/comedi/drivers/s626.c
index fc497dd..77f715b 100644
--- a/drivers/staging/comedi/drivers/s626.c
+++ b/drivers/staging/comedi/drivers/s626.c
@@ -2534,7 +2534,8 @@ static int s626_initialize(struct comedi_device *dev)
for (i = 0; i < 2; i++) {
writel(S626_I2C_CLKSEL, dev->mmio + S626_P_I2CSTAT);
s626_mc_enable(dev, S626_MC2_UPLD_IIC, S626_P_MC2);
- ret = comedi_timeout(dev, NULL, NULL, s626_i2c_handshake_eoc,
0);
+ ret = comedi_timeout(dev, NULL, NULL, s626_i2c_handshake_eoc,
+ 0);
Technically it meets the requirements, but our comedi house style is to
indent the continuation of the statement just past the opening
bracket/parenthesis like so:
ret = comedi_timeout(dev, NULL, NULL, s626_i2c_handshake_eoc,
0);
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] comedi: checkpatch error: tabs instead of spaces
On 28/12/14 18:56, Andreas Siegling wrote: This patch fixes all occurrences of 8 whitespaces instead of a tab. It will remove the 8 whitespaces and replace them with a tab, thereby the checkpatch-error: ERROR: code indent should use tabs where possible is removed from all files in drivers/staging/comedi/ It also brings some consistency of indentation in the comments. Signed-off-by: Andreas Siegling Signed-off-by: Zhutao Lu --- drivers/staging/comedi/drivers/daqboard2000.c | 62 +-- drivers/staging/comedi/drivers/das16m1.c | 4 +- drivers/staging/comedi/drivers/dmm32at.c | 2 +- drivers/staging/comedi/drivers/pcl818.c | 36 4 files changed, 52 insertions(+), 52 deletions(-) Most of these aren't worth doing as they only affect multi-line comments that should be reformatted to the usual block comment style anyway. diff --git a/drivers/staging/comedi/drivers/dmm32at.c b/drivers/staging/comedi/drivers/dmm32at.c index 6df298a..31919b8 100644 --- a/drivers/staging/comedi/drivers/dmm32at.c +++ b/drivers/staging/comedi/drivers/dmm32at.c @@ -365,7 +365,7 @@ static void dmm32at_setaitimer(struct comedi_device *dev, unsigned int nansec) /* enable the ai conversion interrupt and the clock to start scans */ outb(DMM32AT_INTCLK_ADINT | DMM32AT_INTCLK_CLKEN | DMM32AT_INTCLK_CLKSEL, - dev->iobase + DMM32AT_INTCLK_REG); +dev->iobase + DMM32AT_INTCLK_REG); } That bit of the patch is fine, but has already been tackled in a patch by David Decotigny. -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Web: http://www.mev.co.uk/ )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/3] comedi: checkpatch error/warning: spaces
On 28/12/14 18:56, Andreas Siegling wrote:
This patch will fix the following checkpatch errors/warnings in
drivers/staging/comedi/ which are brought by wrong spacing:
ERROR: trailing whitespace
ERROR: space prohibited before that ',' (ctx:WxW)
WARNING: please, no space before tabs
Signed-off-by: Andreas Siegling
Signed-off-by: Zhutao Lu
---
drivers/staging/comedi/drivers/daqboard2000.c | 2 +-
drivers/staging/comedi/drivers/das16m1.c | 2 +-
drivers/staging/comedi/drivers/ni_at_a2150.c | 2 +-
drivers/staging/comedi/drivers/ni_stc.h | 4 ++--
drivers/staging/comedi/drivers/s626.c | 2 +-
5 files changed, 6 insertions(+), 6 deletions(-)
Those should really be split into separate patches, but as for your
PATCH 1/3 it's not worth fixing up the comments this way as they need
reformatting anyway.
diff --git a/drivers/staging/comedi/drivers/ni_at_a2150.c
b/drivers/staging/comedi/drivers/ni_at_a2150.c
index 69e543a..bff4852 100644
--- a/drivers/staging/comedi/drivers/ni_at_a2150.c
+++ b/drivers/staging/comedi/drivers/ni_at_a2150.c
@@ -108,7 +108,7 @@ TRIG_WAKE_EOS
#define IRQ_LVL_BITS(x) (((x) & 0xf) << 4)/* sets irq
level */
#define FIFO_INTR_EN_BIT0x100 /* enable fifo interrupts */
#define FIFO_INTR_FHF_BIT 0x200 /* interrupt fifo half full */
-#define DMA_INTR_EN_BIT 0x800 /* enable interrupt on dma
terminal count */
+#define DMA_INTR_EN_BIT 0x800 /* enable interrupt on dma
terminal count */
#define DMA_DEM_EN_BIT 0x1000 /* enables demand mode dma */
#define I8253_BASE_REG0x14
#define I8253_MODE_REG0x17
diff --git a/drivers/staging/comedi/drivers/ni_stc.h
b/drivers/staging/comedi/drivers/ni_stc.h
index bd69c3f..bb76fe1 100644
--- a/drivers/staging/comedi/drivers/ni_stc.h
+++ b/drivers/staging/comedi/drivers/ni_stc.h
@@ -166,7 +166,7 @@ enum Interrupt_B_Ack_Bits {
#define AI_SCAN_IN_PROG_Pulse _bit4
#define AI_EXTMUX_CLK_Pulse _bit3
#define AI_LOCALMUX_CLK_Pulse _bit2
-#define AI_SC_TC_Pulse _bit1
+#define AI_SC_TC_Pulse _bit1
#define AI_CONVERT_Pulse _bit0
#define AO_Command_1_Register 9
@@ -642,7 +642,7 @@ static unsigned AO_UPDATE_Output_Select(enum
ao_update_output_selection
#define G_Load_Source_Select _bit7
#define G_Reload_Source_Switching _bit15
#define G_Loading_On_Gate _bit14
-#define G_Gate_Polarity_bit13
+#define G_Gate_Polarity_bit13
#define G_Counting_Once(a)(((a)&0x03)<<10)
#define G_Stop_Mode(a)(((a)&0x03)<<5)
Those parts of the patch are okay.
diff --git a/drivers/staging/comedi/drivers/s626.c
b/drivers/staging/comedi/drivers/s626.c
index 14932c5..fc497dd 100644
--- a/drivers/staging/comedi/drivers/s626.c
+++ b/drivers/staging/comedi/drivers/s626.c
@@ -118,7 +118,7 @@ static void s626_mc_enable(struct comedi_device *dev,
static void s626_mc_disable(struct comedi_device *dev,
unsigned int cmd, unsigned int reg)
{
- writel(cmd << 16 , dev->mmio + reg);
+ writel(cmd << 16, dev->mmio + reg);
mmiowb();
}
That part of the patch has also been tackled in a separate patch by
jitendra kumar khasdev. Your patch predates it, but jitendra's patch is
split better.
--
-=( Ian Abbott @ MEV Ltd.E-mail: )=-
-=( Web: http://www.mev.co.uk/ )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
