[PATCH] staging: comedi: usbduxsigma: usbduxfast_ai_cmdtest rounding error
'get_cmd_generic_timed' fills the cmd structure with an informed
guess and then calls 'usbduxfast_ai_cmdtest' repeatedly while
'usbduxfast_ai_cmdtest' is modifying the cmd struct until it
no longer changes. However, because of rounding errors this never
converged because 'steps = (cmd->convert_arg * 30) / 1000' and then
back to 'cmd->convert_arg = (steps * 1000) / 30' won't be the same
because of rounding errors. 'Steps' should only be converted back to
the 'convert_arg' if 'steps' has actually been modified. In addion
the case of steps being 0 wasn't checked which is also now done.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/drivers/usbduxfast.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxfast.c
b/drivers/staging/comedi/drivers/usbduxfast.c
index 04bc488385e6..4af012968cb6 100644
--- a/drivers/staging/comedi/drivers/usbduxfast.c
+++ b/drivers/staging/comedi/drivers/usbduxfast.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * Copyright (C) 2004-2014 Bernd Porr, m...@berndporr.me.uk
+ * Copyright (C) 2004-2019 Bernd Porr, m...@berndporr.me.uk
*/
/*
@@ -8,7 +8,7 @@
* Description: University of Stirling USB DAQ & INCITE Technology Limited
* Devices: [ITL] USB-DUX-FAST (usbduxfast)
* Author: Bernd Porr
- * Updated: 10 Oct 2014
+ * Updated: 16 Nov 2019
* Status: stable
*/
@@ -22,6 +22,7 @@
*
*
* Revision history:
+ * 1.0: Fixed a rounding error in usbduxfast_ai_cmdtest
* 0.9: Dropping the first data packet which seems to be from the last
transfer.
* Buffer overflows in the FX2 are handed over to comedi.
* 0.92: Dropping now 4 packets. The quad buffer has to be emptied.
@@ -350,6 +351,7 @@ static int usbduxfast_ai_cmdtest(struct comedi_device *dev,
struct comedi_cmd *cmd)
{
int err = 0;
+ int err2 = 0;
unsigned int steps;
unsigned int arg;
@@ -399,11 +401,16 @@ static int usbduxfast_ai_cmdtest(struct comedi_device
*dev,
*/
steps = (cmd->convert_arg * 30) / 1000;
if (cmd->chanlist_len != 1)
- err |= comedi_check_trigger_arg_min(&steps,
- MIN_SAMPLING_PERIOD);
- err |= comedi_check_trigger_arg_max(&steps, MAX_SAMPLING_PERIOD);
- arg = (steps * 1000) / 30;
- err |= comedi_check_trigger_arg_is(&cmd->convert_arg, arg);
+ err2 |= comedi_check_trigger_arg_min(&steps,
+MIN_SAMPLING_PERIOD);
+ else
+ err2 |= comedi_check_trigger_arg_min(&steps, 1);
+ err2 |= comedi_check_trigger_arg_max(&steps, MAX_SAMPLING_PERIOD);
+ if (err2) {
+ err |= err2;
+ arg = (steps * 1000) / 30;
+ err |= comedi_check_trigger_arg_is(&cmd->convert_arg, arg);
+ }
if (cmd->stop_src == TRIG_COUNT)
err |= comedi_check_trigger_arg_min(&cmd->stop_arg, 1);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: usbduxfast: usbduxfast_ai_cmdtest rounding error
The userspace comedilib function 'get_cmd_generic_timed' fills
the cmd structure with an informed guess and then calls the
function 'usbduxfast_ai_cmdtest' in this driver repeatedly while
'usbduxfast_ai_cmdtest' is modifying the cmd struct until it
no longer changes. However, because of rounding errors this never
converged because 'steps = (cmd->convert_arg * 30) / 1000' and then
back to 'cmd->convert_arg = (steps * 1000) / 30' won't be the same
because of rounding errors. 'Steps' should only be converted back to
the 'convert_arg' if 'steps' has actually been modified. In addition
the case of steps being 0 wasn't checked which is also now done.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/drivers/usbduxfast.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxfast.c
b/drivers/staging/comedi/drivers/usbduxfast.c
index 04bc488385e6..4af012968cb6 100644
--- a/drivers/staging/comedi/drivers/usbduxfast.c
+++ b/drivers/staging/comedi/drivers/usbduxfast.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0+
/*
- * Copyright (C) 2004-2014 Bernd Porr, m...@berndporr.me.uk
+ * Copyright (C) 2004-2019 Bernd Porr, m...@berndporr.me.uk
*/
/*
@@ -8,7 +8,7 @@
* Description: University of Stirling USB DAQ & INCITE Technology Limited
* Devices: [ITL] USB-DUX-FAST (usbduxfast)
* Author: Bernd Porr
- * Updated: 10 Oct 2014
+ * Updated: 16 Nov 2019
* Status: stable
*/
@@ -22,6 +22,7 @@
*
*
* Revision history:
+ * 1.0: Fixed a rounding error in usbduxfast_ai_cmdtest
* 0.9: Dropping the first data packet which seems to be from the last
transfer.
* Buffer overflows in the FX2 are handed over to comedi.
* 0.92: Dropping now 4 packets. The quad buffer has to be emptied.
@@ -350,6 +351,7 @@ static int usbduxfast_ai_cmdtest(struct comedi_device *dev,
struct comedi_cmd *cmd)
{
int err = 0;
+ int err2 = 0;
unsigned int steps;
unsigned int arg;
@@ -399,11 +401,16 @@ static int usbduxfast_ai_cmdtest(struct comedi_device
*dev,
*/
steps = (cmd->convert_arg * 30) / 1000;
if (cmd->chanlist_len != 1)
- err |= comedi_check_trigger_arg_min(&steps,
- MIN_SAMPLING_PERIOD);
- err |= comedi_check_trigger_arg_max(&steps, MAX_SAMPLING_PERIOD);
- arg = (steps * 1000) / 30;
- err |= comedi_check_trigger_arg_is(&cmd->convert_arg, arg);
+ err2 |= comedi_check_trigger_arg_min(&steps,
+MIN_SAMPLING_PERIOD);
+ else
+ err2 |= comedi_check_trigger_arg_min(&steps, 1);
+ err2 |= comedi_check_trigger_arg_max(&steps, MAX_SAMPLING_PERIOD);
+ if (err2) {
+ err |= err2;
+ arg = (steps * 1000) / 30;
+ err |= comedi_check_trigger_arg_is(&cmd->convert_arg, arg);
+ }
if (cmd->stop_src == TRIG_COUNT)
err |= comedi_check_trigger_arg_min(&cmd->stop_arg, 1);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: usbdux: bug fix for accessing 'ao_chanlist' in private data
Indeed! That had been unnoticed for about 8 years! Well done Harley. /Bernd Ian Abbott wrote: [Sorry for the repeated email. One of the addresses I was replying to was corrupted.] On 2014-03-28 16:20, H Hartley Sweeten wrote: In usbdux_ao_cmd(), the channels for the command are transfered from the cmd->chanlist and stored in the private data 'ao_chanlist'. The channel numbers are bit-shifted when stored so that they become the "command" that is transfered to the device. The channel to command conversion results in the 'ao_chanlist' having these values for the channels: channel 0 -> ao_chanlist = 0x00 channel 1 -> ao_chanlist = 0x40 channel 2 -> ao_chanlist = 0x80 channel 3 -> ao_chanlist = 0xc0 The problem is, the usbduxsub_ao_isoc_irq() function uses the 'chan' value from 'ao_chanlist' to access the 'ao_readback' array in the private data. So instead of accessing the array as 0, 1, 2, 3, it accesses it as 0x00, 0x40, 0x80, 0xc0. Fix this by storing the raw channel number in 'ao_chanlist' and doing the bit-shift when creating the command. Signed-off-by: H Hartley Sweeten Cc: Ian Abbott Cc: Bernd Porr Cc: Greg Kroah-Hartman Nice catch! Another one to add to my list of stable kernel comedi bugs that need fixing. Reviewed-by: Ian Abbott -- http://www.berndporr.me.uk http://www.linux-usb-daq.co.uk http://www.imdb.com/name/nm3293421/ +44 (0)7840 340069 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/6] staging: comedi: usbduxsigma: don't clobber ao_timer in command test
Reviewed-by: Bernd Porr
Ian Abbott wrote:
`devpriv->ao_timer` is used while an asynchronous command is running on
the AO subdevice. It also gets modified by the subdevice's `cmdtest`
handler for checking new asynchronous commands,
`usbduxsigma_ao_cmdtest()`, which is not correct as it's allowed to
check new commands while an old command is still running. Fix it by
moving the code which sets up `devpriv->ao_timer` into the subdevice's
`cmd` handler, `usbduxsigma_ao_cmd()`.
Note that the removed code in `usbduxsigma_ao_cmdtest()` checked that
`devpriv->ao_timer` did not end up less that 1, but that could not
happen due because `cmd->scan_begin_arg` or `cmd->convert_arg` had
already been range-checked.
Also note that we tested the `high_speed` variable in the old code, but
that is currently always 0 and means that we always use "scan" timing
(`cmd->scan_begin_src == TRIG_TIMER` and `cmd->convert_src == TRIG_NOW`)
and never "convert" (individual sample) timing (`cmd->scan_begin_src ==
TRIG_FOLLOW` and `cmd->convert_src == TRIG_TIMER`). The moved code
tests `cmd->convert_src` instead to decide whether "scan" or "convert"
timing is being used, although currently only "scan" timing is
supported.
Fixes: fb1ef622e7a3 ("staging: comedi: usbduxsigma: tidy up analog output command
support")
Signed-off-by: Ian Abbott
Cc: # 3.19 onwards
---
drivers/staging/comedi/drivers/usbduxsigma.c | 33
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index 22517de..dc0b25a 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -912,25 +912,6 @@ static int usbduxsigma_ao_cmdtest(struct comedi_device
*dev,
if (err)
return 3;
- /* Step 4: fix up any arguments */
-
- /* we count in timer steps */
- if (high_speed) {
- /* timing of the conversion itself: every 125 us */
- devpriv->ao_timer = cmd->convert_arg / 125000;
- } else {
- /*
-* timing of the scan: every 1ms
-* we get all channels at once
-*/
- devpriv->ao_timer = cmd->scan_begin_arg / 100;
- }
- if (devpriv->ao_timer < 1)
- err |= -EINVAL;
-
- if (err)
- return 4;
-
return 0;
}
@@ -943,6 +924,20 @@ static int usbduxsigma_ao_cmd(struct comedi_device *dev,
down(&devpriv->sem);
+ if (cmd->convert_src == TRIG_TIMER) {
+ /*
+* timing of the conversion itself: every 125 us
+* at high speed (not used yet)
+*/
+ devpriv->ao_timer = cmd->convert_arg / 125000;
+ } else {
+ /*
+* timing of the scan: every 1ms
+* we get all channels at once
+*/
+ devpriv->ao_timer = cmd->scan_begin_arg / 100;
+ }
+
devpriv->ao_counter = devpriv->ao_timer;
if (cmd->start_src == TRIG_NOW) {
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/6] staging: comedi: usbduxsigma: remove unused "convert" timing for AO
Reviewed-by: Bernd Porr
Ian Abbott wrote:
The `cmdtest` and `cmd` handlers for the AO subdevice
(`usbduxsigma_ao_cmdtest()` and `usbduxsigma_ao_cmd()`) support "scan"
timing of commands with all channels updated every "scan" period. There
is some disabled code to use "convert" timing in high speed mode. That
would allow channels to be updated sequentially every "convert" period.
Since that code is incomplete and currently disabled, remove it to
simplify the existing code.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/usbduxsigma.c | 58
1 file changed, 17 insertions(+), 41 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index 4655048..d97253e 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -838,28 +838,20 @@ static int usbduxsigma_ao_cmdtest(struct comedi_device
*dev,
{
struct usbduxsigma_private *devpriv = dev->private;
int err = 0;
- int high_speed;
- unsigned int flags;
-
- /* high speed conversions are not used yet */
- high_speed = 0; /* (devpriv->high_speed) */
/* Step 1 : check if triggers are trivially valid */
err |= comedi_check_trigger_src(&cmd->start_src, TRIG_NOW | TRIG_INT);
- if (high_speed) {
- /*
-* start immediately a new scan
-* the sampling rate is set by the coversion rate
-*/
- flags = TRIG_FOLLOW;
- } else {
- /* start a new scan (output at once) with a timer */
- flags = TRIG_TIMER;
- }
- err |= comedi_check_trigger_src(&cmd->scan_begin_src, flags);
-
+ /*
+* For now, always use "scan" timing with all channels updated at once
+* (cmd->scan_begin_src == TRIG_TIMER, cmd->convert_src == TRIG_NOW).
+*
+* In a future version, "convert" timing with channels updated
+* indivually may be supported in high speed mode
+* (cmd->scan_begin_src == TRIG_FOLLOW, cmd->convert_src == TRIG_TIMER).
+*/
+ err |= comedi_check_trigger_src(&cmd->scan_begin_src, TRIG_TIMER);
err |= comedi_check_trigger_src(&cmd->convert_src, TRIG_NOW);
err |= comedi_check_trigger_src(&cmd->scan_end_src, TRIG_COUNT);
err |= comedi_check_trigger_src(&cmd->stop_src, TRIG_COUNT | TRIG_NONE);
@@ -883,17 +875,7 @@ static int usbduxsigma_ao_cmdtest(struct comedi_device
*dev,
err |= comedi_check_trigger_arg_is(&cmd->start_arg, 0);
- if (cmd->scan_begin_src == TRIG_FOLLOW) /* internal trigger */
- err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
-
- if (cmd->scan_begin_src == TRIG_TIMER) {
- err |= comedi_check_trigger_arg_min(&cmd->scan_begin_arg,
- 100);
- }
-
- /* not used now, is for later use */
- if (cmd->convert_src == TRIG_TIMER)
- err |= comedi_check_trigger_arg_min(&cmd->convert_arg, 125000);
+ err |= comedi_check_trigger_arg_min(&cmd->scan_begin_arg, 100);
err |= comedi_check_trigger_arg_is(&cmd->scan_end_arg,
cmd->chanlist_len);
@@ -918,19 +900,13 @@ static int usbduxsigma_ao_cmd(struct comedi_device *dev,
down(&devpriv->sem);
- if (cmd->convert_src == TRIG_TIMER) {
- /*
-* timing of the conversion itself: every 125 us
-* at high speed (not used yet)
-*/
- devpriv->ao_timer = cmd->convert_arg / 125000;
- } else {
- /*
-* timing of the scan: every 1ms
-* we get all channels at once
-*/
- devpriv->ao_timer = cmd->scan_begin_arg / 100;
- }
+ /*
+* For now, only "scan" timing is supported. A future version may
+* support "convert" timing in high speed mode.
+*
+* Timing of the scan: every 1ms all channels updated at once.
+*/
+ devpriv->ao_timer = cmd->scan_begin_arg / 100;
devpriv->ao_counter = devpriv->ao_timer;
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 6/6] staging: comedi: usbduxsigma: round down AO scan_begin_arg at step 4.
Reviewed-by: Bernd Porr
Ian Abbott wrote:
The return value of the `cmdtest` handler for a subdevice checks the
prospective new command in various steps and returns the step number at
which any problem was detected, or 0 if no problem was detected. It is
allowed to modify the command in various ways at each step. Corrections
for out-of-range values are generally made at step 3, and minor
adjustments such as rounding are generally made at step 4.
The `cmdtest` handler for the AO subdevice (`usbduxsigma_ao_cmdtest()`)
currently range checks the timings at step 3. Since the running command
will round down the timings, add code to round them down at step 4.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/usbduxsigma.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index d97253e..e22c374 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -837,6 +837,7 @@ static int usbduxsigma_ao_cmdtest(struct comedi_device *dev,
struct comedi_cmd *cmd)
{
struct usbduxsigma_private *devpriv = dev->private;
+ unsigned int tmp;
int err = 0;
/* Step 1 : check if triggers are trivially valid */
@@ -888,6 +889,14 @@ static int usbduxsigma_ao_cmdtest(struct comedi_device
*dev,
if (err)
return 3;
+ /* Step 4: fix up any arguments */
+
+ tmp = rounddown(cmd->scan_begin_arg, 100);
+ err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, tmp);
+
+ if (err)
+ return 4;
+
return 0;
}
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/6] staging: comedi: usbduxsigma: round down AI scan_begin_arg at step 4.
Reviewed-by: Bernd Porr
Ian Abbott wrote:
The return value of the `cmdtest` handler for a subdevice checks the
prospective new command in various steps and returns the step number at
which any problem was detected, or 0 if no problem was detected. It is
allowed to modify the command in various ways at each step. Corrections
for out-of-range values are generally made at step 3, and minor
adjustments such as rounding are generally made at step 4.
The `cmdtest` handler for the AI subdevice (`usbduxsigma_ai_cmdtest()`)
currently modifies `cmd->scan_begin_arg` to bring it into range and
round it down at step 3. Move the rounding down part to step 4 to
follow the usual Comedi convention.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/usbduxsigma.c | 13 -
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index 65a0df4..4655048 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -518,17 +518,12 @@ static int usbduxsigma_ai_cmdtest(struct comedi_device
*dev,
*/
err |= comedi_check_trigger_arg_min(&cmd->scan_begin_arg,
(125000 * interval));
-
- tmp = (cmd->scan_begin_arg / 125000) * 125000;
} else {
/* full speed */
/* 1kHz scans every USB frame */
err |= comedi_check_trigger_arg_min(&cmd->scan_begin_arg,
100);
-
- tmp = (cmd->scan_begin_arg / 100) * 100;
}
- err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, tmp);
err |= comedi_check_trigger_arg_is(&cmd->scan_end_arg,
cmd->chanlist_len);
@@ -541,6 +536,14 @@ static int usbduxsigma_ai_cmdtest(struct comedi_device
*dev,
if (err)
return 3;
+ /* Step 4: fix up any arguments */
+
+ tmp = rounddown(cmd->scan_begin_arg, high_speed ? 125000 : 100);
+ err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, tmp);
+
+ if (err)
+ return 4;
+
return 0;
}
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/6] staging: comedi: usbduxsigma: remove AI scan_begin_src == TRIG_FOLLOW
Reviewed-by: Bernd Porr
Ian Abbott wrote:
The AI subdevice `cmdtest` handler `usbduxsigma_ai_cmdtest()` ensures
that `cmd->scan_begin_src == TRIG_TIMER` by the end of step 2 of the
command checking code, so assume that this is the case for step 3
onwards and remove the redundant code.
Signed-off-by: Ian Abbott
---
drivers/staging/comedi/drivers/usbduxsigma.c | 47 +++-
1 file changed, 19 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index dc0b25a..65a0df4 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -481,6 +481,7 @@ static int usbduxsigma_ai_cmdtest(struct comedi_device *dev,
struct usbduxsigma_private *devpriv = dev->private;
int high_speed = devpriv->high_speed;
int interval = usbduxsigma_chans_to_interval(cmd->chanlist_len);
+ unsigned int tmp;
int err = 0;
/* Step 1 : check if triggers are trivially valid */
@@ -508,36 +509,26 @@ static int usbduxsigma_ai_cmdtest(struct comedi_device
*dev,
err |= comedi_check_trigger_arg_is(&cmd->start_arg, 0);
- if (cmd->scan_begin_src == TRIG_FOLLOW) /* internal trigger */
- err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, 0);
+ if (high_speed) {
+ /*
+* In high speed mode microframes are possible.
+* However, during one microframe we can roughly
+* sample two channels. Thus, the more channels
+* are in the channel list the more time we need.
+*/
+ err |= comedi_check_trigger_arg_min(&cmd->scan_begin_arg,
+ (125000 * interval));
- if (cmd->scan_begin_src == TRIG_TIMER) {
- unsigned int tmp;
-
- if (high_speed) {
- /*
-* In high speed mode microframes are possible.
-* However, during one microframe we can roughly
-* sample two channels. Thus, the more channels
-* are in the channel list the more time we need.
-*/
- err |= comedi_check_trigger_arg_min(&cmd->
- scan_begin_arg,
- (100 / 8 *
-interval));
-
- tmp = (cmd->scan_begin_arg / 125000) * 125000;
- } else {
- /* full speed */
- /* 1kHz scans every USB frame */
- err |= comedi_check_trigger_arg_min(&cmd->
- scan_begin_arg,
- 100);
-
- tmp = (cmd->scan_begin_arg / 100) * 100;
- }
- err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, tmp);
+ tmp = (cmd->scan_begin_arg / 125000) * 125000;
+ } else {
+ /* full speed */
+ /* 1kHz scans every USB frame */
+ err |= comedi_check_trigger_arg_min(&cmd->scan_begin_arg,
+ 100);
+
+ tmp = (cmd->scan_begin_arg / 100) * 100;
}
+ err |= comedi_check_trigger_arg_is(&cmd->scan_begin_arg, tmp);
err |= comedi_check_trigger_arg_is(&cmd->scan_end_arg,
cmd->chanlist_len);
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/6] staging: comedi: usbduxsigma: don't clobber ai_timer in command test
Reviewed-by: Bernd Porr
Ian Abbott wrote:
`devpriv->ai_timer` is used while an asynchronous command is running on
the AI subdevice. It also gets modified by the subdevice's `cmdtest`
handler for checking new asynchronous commands
(`usbduxsigma_ai_cmdtest()`), which is not correct as it's allowed to
check new commands while an old command is still running. Fix it by
moving the code which sets up `devpriv->ai_timer` and
`devpriv->ai_interval` into the subdevice's `cmd` handler,
`usbduxsigma_ai_cmd()`.
Note that the removed code in `usbduxsigma_ai_cmdtest()` checked that
`devpriv->ai_timer` did not end up less than than 1, but that could not
happen because `cmd->scan_begin_arg` had already been checked to be at
least the minimum required value (at least when `cmd->scan_begin_src ==
TRIG_TIMER`, which had also been checked to be the case).
Fixes: b986be8527c7 ("staging: comedi: usbduxsigma: tidy up analog input
command support)
Signed-off-by: Ian Abbott
Cc: # 3.19 onwards
---
drivers/staging/comedi/drivers/usbduxsigma.c | 37
1 file changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index eaa9add..22517de 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -550,27 +550,6 @@ static int usbduxsigma_ai_cmdtest(struct comedi_device
*dev,
if (err)
return 3;
- /* Step 4: fix up any arguments */
-
- if (high_speed) {
- /*
-* every 2 channels get a time window of 125us. Thus, if we
-* sample all 16 channels we need 1ms. If we sample only one
-* channel we need only 125us
-*/
- devpriv->ai_interval = interval;
- devpriv->ai_timer = cmd->scan_begin_arg / (125000 * interval);
- } else {
- /* interval always 1ms */
- devpriv->ai_interval = 1;
- devpriv->ai_timer = cmd->scan_begin_arg / 100;
- }
- if (devpriv->ai_timer < 1)
- err |= -EINVAL;
-
- if (err)
- return 4;
-
return 0;
}
@@ -668,6 +647,22 @@ static int usbduxsigma_ai_cmd(struct comedi_device *dev,
down(&devpriv->sem);
+ if (devpriv->high_speed) {
+ /*
+* every 2 channels get a time window of 125us. Thus, if we
+* sample all 16 channels we need 1ms. If we sample only one
+* channel we need only 125us
+*/
+ unsigned int interval = usbduxsigma_chans_to_interval(len);
+
+ devpriv->ai_interval = interval;
+ devpriv->ai_timer = cmd->scan_begin_arg / (125000 * interval);
+ } else {
+ /* interval always 1ms */
+ devpriv->ai_interval = 1;
+ devpriv->ai_timer = cmd->scan_begin_arg / 100;
+ }
+
for (i = 0; i < len; i++) {
unsigned int chan = CR_CHAN(cmd->chanlist[i]);
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: drivers: usbduxsigma: Removed variables that is never used
Indeed. It can be completely removed. I was intending to speed up DIO
reads during async acquisition but I decided against it because it would
create unpredictable latencies.
Thanks Ian for flagging it!
/Bernd
Ian Abbott wrote:
On 28/01/15 22:39, Rickard Strandqvist wrote:
Variable ar assigned a value that is never used.
I have also removed all the code that thereby serves no purpose.
This was found using a static code analysis program called cppcheck
Signed-off-by: Rickard Strandqvist
---
drivers/staging/comedi/drivers/usbduxsigma.c |3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index dc19435..7b80887 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -215,7 +215,6 @@ static void usbduxsigma_ai_handle_urb(struct
comedi_device *dev,
struct usbduxsigma_private *devpriv = dev->private;
struct comedi_async *async = s->async;
struct comedi_cmd *cmd = &async->cmd;
-unsigned int dio_state;
uint32_t val;
int ret;
int i;
@@ -225,7 +224,7 @@ static void usbduxsigma_ai_handle_urb(struct
comedi_device *dev,
devpriv->ai_counter = devpriv->ai_timer;
/* get the state of the dio pins to allow external trigger */
-dio_state = be32_to_cpu(devpriv->in_buf[0]);
+be32_to_cpu(devpriv->in_buf[0]);
That line produces a compiler warning. The line can be removed as the
call to be32_to_cpu() has no side-effects. The "/* get the state ..."
comment line can also be removed as it is only associated with this line.
Perhaps Bernd intended to use dio_state for something here, but I'm not
sure what.
/* get the data from the USB bus and hand it over to comedi */
for (i = 0; i < cmd->chanlist_len; i++) {
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging: comedi: drivers: usbduxsigma: Removed variables that is never used
Ian Abbott wrote:
On 29/01/15 22:50, Rickard Strandqvist wrote:
Variable was assigned a value that was never used.
I have also removed all the code that thereby serves no purpose.
This was found using a static code analysis program called cppcheck
Signed-off-by: Rickard Strandqvist
---
drivers/staging/comedi/drivers/usbduxsigma.c |4
1 file changed, 4 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index dc19435..185f2d1 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -215,7 +215,6 @@ static void usbduxsigma_ai_handle_urb(struct
comedi_device *dev,
struct usbduxsigma_private *devpriv = dev->private;
struct comedi_async *async = s->async;
struct comedi_cmd *cmd = &async->cmd;
-unsigned int dio_state;
uint32_t val;
int ret;
int i;
@@ -224,9 +223,6 @@ static void usbduxsigma_ai_handle_urb(struct
comedi_device *dev,
if (devpriv->ai_counter == 0) {
devpriv->ai_counter = devpriv->ai_timer;
-/* get the state of the dio pins to allow external trigger */
-dio_state = be32_to_cpu(devpriv->in_buf[0]);
-
/* get the data from the USB bus and hand it over to comedi */
for (i = 0; i < cmd->chanlist_len; i++) {
/* transfer data, note first byte is the DIO state */
Reviewed-by: Ian Abbott
Reviewed-by: Bernd Porr
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
staging: comedi: USB devs not working / some comedi core reorganization
Hi all, I've just checked out after a while the newest RC kernel and the usb-auto config/attach is broken. Seems so that the driver specific usb attach is no longer called and no firmware is loaded. Hartly, can you point me to the code bits which should call the driver spcific attach or give a short summary how that should work? I had a look at the git logs but there are quite lot of subsequent changes. Can you give me some guidance because it has changed a lot since I had a look at it the last time. I don't have much time this week but can crack on next week re that and test that thoroughly. Just now none of my DUX boards work at all. They all have the same problem that there are zero subdevices and the firmware is not loaded so I assume that the driver specific attach is not called. /Bernd -- www:http://www.berndporr.me.uk/ http://www.linux-usb-daq.co.uk/ http://www.imdb.com/name/nm3293421/ Mobile: +44 (0)7840 340069 Work: +44 (0)141 330 5237 University of Glasgow School of Engineering Rankine Building, Oakfield Avenue, Glasgow, G12 8LT ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: comedi: USB devs not working / some comedi core reorganization
2.978484] [] dump_stack+0x45/0x56
[ 4972.978489] [] warn_slowpath_common+0x7d/0xa0
[ 4972.978494] [] warn_slowpath_fmt+0x4c/0x50
[ 4972.978499] [] ? sysfs_get_dirent_ns+0x4e/0x70
[ 4972.978504] [] sysfs_remove_group+0xc6/0xd0
[ 4972.978509] [] dpm_sysfs_remove+0x43/0x50
[ 4972.978513] [] device_del+0x45/0x1c0
[ 4972.978518] [] device_unregister+0x1e/0x60
[ 4972.978522] [] device_destroy+0x3c/0x50
[ 4972.978530] []
comedi_free_subdevice_minor+0x75/0xa0 [comedi]
[ 4972.978539] [] comedi_device_detach+0x48/0x160
[comedi]
[ 4972.978546] [] comedi_device_cleanup+0x33/0x90
[comedi]
[ 4972.978552] [] comedi_free_board_dev+0x36/0x50
[comedi]
[ 4972.978558] []
comedi_release_hardware_device+0x80/0x90 [comedi]
[ 4972.978565] [] comedi_auto_unconfig+0x13/0x20 [comedi]
[ 4972.978572] [] comedi_usb_auto_unconfig+0x12/0x20
[comedi]
[ 4972.978578] [] usb_unbind_interface+0x64/0x1c0
[ 4972.978583] [] __device_release_driver+0x7f/0xf0
[ 4972.978587] [] device_release_driver+0x23/0x30
[ 4972.978592] [] bus_remove_device+0x108/0x180
[ 4972.978596] [] device_del+0x129/0x1c0
[ 4972.978601] [] usb_disable_device+0xb0/0x290
[ 4972.978605] [] usb_disconnect+0xad/0x200
[ 4972.978609] [] hub_thread+0x70d/0x1750
[ 4972.978614] [] ? sched_clock_cpu+0xa8/0x100
[ 4972.978619] [] ? prepare_to_wait_event+0x100/0x100
[ 4972.978623] [] ? usb_reset_device+0x1d0/0x1d0
[ 4972.978627] [] kthread+0xd2/0xf0
[ 4972.978632] [] ? kthread_create_on_node+0x190/0x190
[ 4972.978637] [] ret_from_fork+0x7c/0xb0
[ 4972.978641] [] ? kthread_create_on_node+0x190/0x190
[ 4972.978644] ---[ end trace 69c2b5c4559cdf1b ]---
I guess that subdevice no longer exists at this point?
/Bernd
On 10/12/13 16:31, Hartley Sweeten wrote:
On Tuesday, December 10, 2013 4:48 AM, Bernd Porr wrote:
I've just checked out after a while the newest RC kernel and the
usb-auto config/attach is broken. Seems so that the driver specific usb
attach is no longer called and no firmware is loaded. Hartly, can you
point me to the code bits which should call the driver spcific attach or
give a short summary how that should work? I had a look at the git logs
but there are quite lot of subsequent changes. Can you give me some
guidance because it has changed a lot since I had a look at it the last
time. I don't have much time this week but can crack on next week re
that and test that thoroughly. Just now none of my DUX boards work at
all. They all have the same problem that there are zero subdevices and
the firmware is not loaded so I assume that the driver specific attach
is not called.
Hello Bernd,
I'm not sure why the usbdux drivers are not auto attaching correctly.
Sorry about the problems.
What version of the kernel are you testing with?
Are the USB_DEVICE() id's correct in the driver?
The comedi USB drivers should auto attach when the device is detected.
The USB driver (*probe) will call comedi_usb_auto_config() which then
calls the comedi driver (*auto_attach).
All of the usbdux drivers do the initial USB setup, buffer allocation etc., then
call comedi_load_firmware() to request and load the firmware for the device.
After that the subdevices are allocated and initialized.
Try putting a couple printk debug messages in the usb (*probe) and comedi
(*auto_attach) functions in the drivers. You might also put a printk in the
callback functions that actually upload the firmware to the device.
Please let me know what you find. Hopefully this is a minor bug and we can
resolve it quickly.
Thanks,
Hartley
--
www:http://www.berndporr.me.uk/
http://www.linux-usb-daq.co.uk/
http://www.imdb.com/name/nm3293421/
Mobile: +44 (0)7840 340069
Work: +44 (0)141 330 5237
University of Glasgow
School of Engineering
Rankine Building, Oakfield Avenue,
Glasgow, G12 8LT
>From e2e07ce084f20d0d678d3cc85cc9b941a40c1d6c Mon Sep 17 00:00:00 2001
From: Bernd Porr
Date: Tue, 10 Dec 2013 19:42:13 +
Subject: [PATCH 1/1] comedi_load_firmware returns the number of transmitted
bytes to the USB controller. The result is negative on failure. Thus, the ret
argument needs to be checked if negative.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/drivers/usbduxfast.c | 2 +-
drivers/staging/comedi/drivers/usbduxsigma.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxfast.c b/drivers/staging/comedi/drivers/usbduxfast.c
index 9707dd1..4b7f360 100644
--- a/drivers/staging/comedi/drivers/usbduxfast.c
+++ b/drivers/staging/comedi/drivers/usbduxfast.c
@@ -1092,7 +1092,7 @@ static int usbduxfast_auto_attach(struct comedi_device *dev,
ret = comedi_load_firmware(dev, &usb->dev, FIRMWARE,
usbduxfast_upload_firmware, 0);
- if (ret)
+ if (ret < 0)
return ret;
return usbduxfast_attach_common(dev);
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c b/drivers/staging/comedi/drivers/usbduxsigma.c
index a5363de..4e
Re: staging: comedi: USB devs not working / some comedi core reorganization
Good point. Nobody cares really how many bytes the firmware uploader sends to the DUX-board. /Bernd On 10/12/13 21:30, Ian Abbott wrote: On 2013-12-10 21:07, Bernd Porr wrote: Date: Tue, 10 Dec 2013 19:42:13 + Subject: [PATCH 1/1] comedi_load_firmware returns the number of transmitted bytes to the USB controller. The result is negative on failure. Thus, the ret argument needs to be checked if negative. Signed-off-by: Bernd Porr --- drivers/staging/comedi/drivers/usbduxfast.c | 2 +- drivers/staging/comedi/drivers/usbduxsigma.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/comedi/drivers/usbduxfast.c b/drivers/staging/comedi/drivers/usbduxfast.c index 9707dd1..4b7f360 100644 --- a/drivers/staging/comedi/drivers/usbduxfast.c +++ b/drivers/staging/comedi/drivers/usbduxfast.c @@ -1092,7 +1092,7 @@ static int usbduxfast_auto_attach(struct comedi_device *dev, ret = comedi_load_firmware(dev, &usb->dev, FIRMWARE, usbduxfast_upload_firmware, 0); -if (ret) +if (ret < 0) return ret; return usbduxfast_attach_common(dev); diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c b/drivers/staging/comedi/drivers/usbduxsigma.c index a5363de..4ee6271 100644 --- a/drivers/staging/comedi/drivers/usbduxsigma.c +++ b/drivers/staging/comedi/drivers/usbduxsigma.c @@ -1596,7 +1596,7 @@ static int usbduxsigma_auto_attach(struct comedi_device *dev, ret = comedi_load_firmware(dev, &usb->dev, FIRMWARE, usbduxsigma_firmware_upload, 0); -if (ret) +if (ret < 0) return ret; ret = comedi_alloc_subdevices(dev, (devpriv->high_speed) ? 4 : 3); -- 1.8.5.1 It might be better just to prevent comedi_load_firmware() returning a value greater than zero, since I can't think of any reason why it would need to. That would also work for the usbdux driver. -- www:http://www.berndporr.me.uk/ http://www.linux-usb-daq.co.uk/ http://www.imdb.com/name/nm3293421/ Mobile: +44 (0)7840 340069 Work: +44 (0)141 330 5237 University of Glasgow School of Engineering Rankine Building, Oakfield Avenue, Glasgow, G12 8LT ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: comedi: USB devs not working / some comedi core reorganization
Agree with your comments. Let's just do the
return ret < 0 ? ret : 0;
and it's sorted. Thanks for the quick reply.
/Bernd
On 10/12/13 23:24, Hartley Sweeten wrote:
On Tuesday, December 10, 2013 3:28 PM, Dan Carpenter wrote:
On Tue, Dec 10, 2013 at 09:30:43PM +, Ian Abbott wrote:
It might be better just to prevent comedi_load_firmware() returning
a value greater than zero, since I can't think of any reason why it
would need to. That would also work for the usbdux driver.
Yeah. It looks like it's usbduxfast_upload_firmware() which has the
>problem. Does this fix your problem? This bug was introduced in
161f440c8d91 ('staging: comedi: drivers: usbduxfast.c: fix for DMA
buffers on stack')
Actually I think I broke it when I introduced the comedi_upload_firmware()
helper and converted the usbdux drivers to use it.
diff --git a/drivers/staging/comedi/drivers/usbduxfast.c
b/drivers/staging/comedi/drivers/usbduxfast.c
index 9707dd1239c4..f2d8d7fb7aab 100644
--- a/drivers/staging/comedi/drivers/usbduxfast.c
+++ b/drivers/staging/comedi/drivers/usbduxfast.c
@@ -1040,6 +1040,7 @@ static int usbduxfast_upload_firmware(struct
comedi_device *dev,
EZTIMEOUT);
if (ret < 0)
dev_err(dev->class_dev, "can not start firmware\n");
+ ret = 0;
done:
kfree(tmp);
This would only fix the usbduxfast driver.
I think Ian's comment about fixing comedi_load_firmware() is the correct
one. This should do it:
From dcb9cae663fb7489e235569de20ac5757450953d Mon Sep 17 00:00:00 2001
From: H Hartley Sweeten
Date: Tue, 18 Jun 2013 13:21:13 -0700
Subject: [PATCH] staging: comedi: drivers: fix return value of
comedi_load_firmware()
Some of the callback functions that upload the firmware in the comedi
drivers return a positive value indicating the number of bytes sent
to the device. Detect this condition and just return '0' to indicate
a successful upload.
Signed-off-by: H Hartley Sweeten
Cc: Ian Abbott
Cc: Greg Kroah-Hartman
Cc: Bernd Porr
Cc: Dan Carpenter
---
drivers/staging/comedi/drivers.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index a5d03b9..9d71b4d 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -451,7 +451,7 @@ int comedi_load_firmware(struct comedi_device *dev,
release_firmware(fw);
}
- return ret;
+ return ret < 0 ? ret : 0;
}
EXPORT_SYMBOL_GPL(comedi_load_firmware);
--
www:http://www.berndporr.me.uk/
http://www.linux-usb-daq.co.uk/
http://www.imdb.com/name/nm3293421/
Mobile: +44 (0)7840 340069
Work: +44 (0)141 330 5237
University of Glasgow
School of Engineering
Rankine Building, Oakfield Avenue,
Glasgow, G12 8LT
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: comedi: USB devs not working / some comedi core reorganization
hacked cfc_check_trigger_src:
-
static inline int cfc_check_trigger_src(unsigned int *src, unsigned int
flags)
{
unsigned int orig_src = *src;
*src = orig_src & flags;
printk("cfc_check_trigger_src: orig_src=%x, *src=%x
\n",orig_src,*src );
if (*src == TRIG_INVALID || *src != orig_src)
return -EINVAL;
return 0;
}
Any ideas?
I don't know off-hand, but I'll take a look when I get the chance.
This is the first time comedi_command_test is called in generic_timed
(in comedilib / userspace). That assumes that the subdevice is
initialised, in particular the cmd_mask. Can it be that this sometimes
won't happen? The behaviour is pretty random so it might be an
uninitialised struct. Return value of generic_timed is -1 just now but
had also 1 in the past.
/Bernd
EXPORT_ALIAS_DEFAULT(_comedi_get_cmd_src_mask,comedi_get_cmd_src_mask,0.7.18);
int _comedi_get_cmd_src_mask(comedi_t *it,unsigned int subd,comedi_cmd *cmd)
{
subdevice *s;
int ret;
if(!valid_subd(it,subd))return -1;
s=it->subdevices+subd;
if(s->cmd_mask_errno){
errno = s->cmd_mask_errno;
return -1;
}
if(!s->cmd_mask){
comedi_cmd *mask;
mask = malloc(sizeof(comedi_cmd));
memset(mask,0,sizeof(*cmd));
mask->subdev = subd;
mask->flags = 0;
mask->start_src = TRIG_ANY;
mask->scan_begin_src = TRIG_ANY;
mask->convert_src = TRIG_ANY;
mask->scan_end_src = TRIG_ANY;
mask->stop_src = TRIG_ANY;
s->cmd_mask = mask;
ret = comedi_command_test(it,mask);
if(ret<0){
s->cmd_mask_errno = errno;
return -1;
}
static int __generic_timed(comedi_t *it, unsigned s,
comedi_cmd *cmd, unsigned chanlist_len, unsigned scan_period_ns)
{
int ret;
unsigned convert_period_ns;
if(chanlist_len < 1) return -EINVAL;
ret = comedi_get_cmd_src_mask(it,s,cmd);
if(ret<0)return ret;
__comedi_errno = ENOTSUPPORTED;
if(cmd->start_src&TRIG_NOW){
cmd->start_src=TRIG_NOW;
cmd->start_arg=0;
}else if(cmd->start_src&TRIG_INT){
cmd->start_src=TRIG_INT;
cmd->start_arg=0;
}else{
COMEDILIB_DEBUG(3,"can't find good start_src\n");
return -1;
}
convert_period_ns = (scan_period_ns + chanlist_len / 2) /
chanlist_len;
if((cmd->convert_src & TRIG_TIMER) &&
(cmd->scan_begin_src & TRIG_FOLLOW))
{
cmd->convert_src = TRIG_TIMER;
cmd->convert_arg = convert_period_ns;
cmd->scan_begin_src = TRIG_FOLLOW;
cmd->scan_begin_arg = 0;
}else if((cmd->convert_src & TRIG_NOW) &&
(cmd->scan_begin_src & TRIG_TIMER))
{
--
www:http://www.berndporr.me.uk/
http://www.linux-usb-daq.co.uk/
http://www.imdb.com/name/nm3293421/
Mobile: +44 (0)7840 340069
Work: +44 (0)141 330 5237
University of Glasgow
School of Engineering
Rankine Building, Oakfield Avenue,
Glasgow, G12 8LT
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: comedi: USB devs not working / some comedi core reorganization
Hi all,
I've drilled down further. After an async command has run the channel
list is kfree'd in do_become_nonbusy. The pointer is not NULL but I
guess that's been kfree'd already somewhere else and there is no need to
do it here?
The comedi_command_test is actually OK. Ignore the report here last
night. That was actually proper behaviour of comedilib. The channellist
was 20 channels long in the demo program but the usbduxsigma only
supports 16 channels. In the old version of comedilib that was ignored
but now properly checked. Now it is. So, not a bug but a feature!
/Bernd
[ 85.208807] do_become_nonbusy: freeing chanlist
[ 85.208830] BUG: unable to handle kernel paging request at
ea0001c0
[ 85.208891] IP: [] kfree+0x56/0x130
[ 85.208933] PGD 5f684067 PUD 5f683067 PMD 0
[ 85.208971] Oops: [#1] SMP
[ 85.209001] Modules linked in: radeon bnep snd_hda_codec_idt
rc_hauppauge ir_kbd_i2c snd_hda_intel rfcomm snd_hda_codec tuner msp3400
snd_hwdep snd_bt87x bluetooth usbduxsigma(O) hid_generic comedi_fc(O)
comedi(O) usbhid pcmcia snd_pcm hid bttv pcmcia_core snd_page_alloc
parport_pc snd_seq_midi snd_seq_midi_event snd_rawmidi ppdev snd_seq ttm
btcx_risc snd_seq_device tveeprom drm_kms_helper gpio_ich snd_timer
videobuf_dma_sg drm rc_core snd dcdbas v4l2_common videobuf_core psmouse
videodev soundcore serio_raw microcode i2c_algo_bit lpc_ich mac_hid lp
parport ahci libahci e100 mii
[ 85.209493] CPU: 0 PID: 1984 Comm: cmd Tainted: G O
3.13.0-rc3+ #1
[ 85.209539] Hardware name: Dell Inc. Dimension 9100
/0X8582, BIOS A01 05/25/2005
[ 85.209603] task: 88005bc8b000 ti: 88005b334000 task.ti:
88005b334000
[ 85.209652] RIP: 0010:[] []
kfree+0x56/0x130
[ 85.209703] RSP: 0018:88005b335dd8 EFLAGS: 00010286
[ 85.209739] RAX: 01c0 RBX: fff2 RCX:
0006
[ 85.209784] RDX: RSI: 59a259a0 RDI:
fff2
[ 85.209829] RBP: 88005b335df0 R08: 0082 R09:
03c4
[ 85.209875] R10: ea0001c0 R11: 6c6e61686320676e R12:
88003bf7ad00
[ 85.209921] R13: a020995b R14: 7fffe7305770 R15:
88005b335e48
[ 85.209968] FS: 7fa60b5af740() GS:88005fc0()
knlGS:
[ 85.210020] CS: 0010 DS: ES: CR0: 80050033
[ 85.210058] CR2: ea0001c0 CR3: 5b905000 CR4:
07f0
[ 85.210104] Stack:
[ 85.210118] 8800415cfc00 88003bf7ad00 fff2
88005b335e18
[ 85.210176] a020995b 88005be4ff00 88003c981900
fff2
[ 85.210231] 88005b335ec0 a020ac6d 88003bf7ad00
8800415cfc00
[ 85.210287] Call Trace:
[ 85.210317] [] do_become_nonbusy.isra.10+0xbb/0xd0
[comedi]
[ 85.210372] [] comedi_unlocked_ioctl+0xb4d/0x1280
[comedi]
[ 85.210424] [] do_vfs_ioctl+0x2e0/0x4c0
[ 85.210465] [] ? vtime_account_user+0x54/0x60
[ 85.210510] [] SyS_ioctl+0x81/0xa0
[ 85.210546] [] tracesys+0xe1/0xe6
[ 85.210579] Code: 00 00 00 80 ff 77 00 00 49 ba 00 00 00 00 00 ea ff
ff 48 01 d8 48 0f 42 15 58 6b a7 00 48 01 d0 48 c1 e8 0c 48 c1 e0 06 49
01 c2 <49> 8b 02 f6 c4 80 0f 85 be 00 00 00 49 8b 02 a8 80 0f 84 92 00
[ 85.210923] RIP [] kfree+0x56/0x130
[ 85.210960] RSP
[ 85.210983] CR2: ea0001c0
[ 85.214342] ---[ end trace efb568732351eb56 ]---
[ 210.336281] audit_printk_skb: 135 callbacks suppressed
[ 210.336289] type=1006 audit(1386754824.943:72): pid=2083 uid=0 old
auid=4294967295 new auid=1000 old ses=4294967295 new ses=1 res=1
static void do_become_nonbusy(struct comedi_device *dev,
struct comedi_subdevice *s)
{
struct comedi_async *async;
if (s == NULL)
{
printk("do_become_nonbusy: s=NULL\n");
return;
}
async = s->async;
if (async == NULL)
{
printk("do_become_nonbusy: s->async is NULL\n");
}
comedi_set_subdevice_runflags(s, SRF_RUNNING, 0);
if (async) {
comedi_buf_reset(async);
async->inttrig = NULL;
if (async->cmd.chanlist) {
printk("do_become_nonbusy: freeing chanlist\n");
kfree(async->cmd.chanlist);
async->cmd.chanlist = NULL;
}
} else {
dev_err(dev->class_dev,
"BUG: (?) do_become_nonbusy called with
async=NULL\n");
}
s->busy = NULL;
}
Ian Abbott wrote:
On 2013-12-10 21:07, Bernd Porr wrote:
Hi all,
here is the patch to fix the original bug. That was easier than I
expected. That's against the latest RC kernel.
However there are a couple other issues now.
There seems to be an issue with comedi generic timed and the commands
correcting the TRIG bit. It ANDs the r
[PATCH] staging: comedi: drivers: fix kernel oops when channel list has invalid pointer
Hi all,
I think with this patch everything seems to be running fine now. The
kernel oops was actually caused by a bug in one of my userspace programs
(forgot to set the channel list in cmd) but of course that shouldn't
have been possible. That's now properly dealt with and we get a "bad
address" back to the userspace program.
Thanks for all your hard work to tidy up the comedi code.
/Bernd
--
www:http://www.berndporr.me.uk/
http://www.linux-usb-daq.co.uk/
http://www.imdb.com/name/nm3293421/
Mobile: +44 (0)7840 340069
Work: +44 (0)141 330 5237
University of Glasgow
School of Engineering
Rankine Building, Oakfield Avenue,
Glasgow, G12 8LT
>From d83a3e0cda7559e9b91759ab4ef8a6c3eb19fbc0 Mon Sep 17 00:00:00 2001
From: Bernd Porr
Date: Wed, 11 Dec 2013 11:45:09 +
Subject: [PATCH 1/1] If the channel list is not set in userspace we get an
error at PTR_ERR(async->cmd.chanlist). However, do_become_nonbusy(dev, s)
cleans up this pointer which causes a kernel ooops. Setting the channel list
in async to NULL and checking this in do_become_nonbusy prevents the oops.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/comedi_fops.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index f3d59e2..cb546f8 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -560,8 +560,10 @@ static void do_become_nonbusy(struct comedi_device *dev,
if (async) {
comedi_buf_reset(async);
async->inttrig = NULL;
- kfree(async->cmd.chanlist);
- async->cmd.chanlist = NULL;
+ if (async->cmd.chanlist) {
+ kfree(async->cmd.chanlist);
+ async->cmd.chanlist = NULL;
+ }
} else {
dev_err(dev->class_dev,
"BUG: (?) do_become_nonbusy called with async=NULL\n");
@@ -1425,6 +1427,7 @@ static int do_cmd_ioctl(struct comedi_device *dev,
async->cmd.chanlist_len * sizeof(int));
if (IS_ERR(async->cmd.chanlist)) {
ret = PTR_ERR(async->cmd.chanlist);
+ async->cmd.chanlist = NULL;
DPRINTK("memdup_user failed with code %d\n", ret);
goto cleanup;
}
--
1.8.5.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: comedi: USB devs not working / some comedi core reorganization
Hi all, here is another patch which fixes a kernel warning where a subdevice is released which no longer exists. The subdevice was removed before the main comedi device. The functions for the main dev removal and the subdevice were in different files/modules, were called from different subsystems and I've moved them together to make sure the order is right. There is still a problem that the subdevices are sometimes not generated at boot time (cold boot) but hotplug does. I don't get any error messages though so firmware load is probably not the problem. I've put loads of the printk in it but then the error disappears (!). Seems to be a very tight race condition. /Bernd There is another issue which has to the with the subdevices. This happens when I unplug the DUX board (all boards cause this): [ 4972.978379] [ cut here ] [ 4972.978386] WARNING: CPU: 0 PID: 33 at fs/sysfs/group.c:214 sysfs_remove_group+0xc6/0xd0() [ 4972.978389] sysfs group 81caa2e0 not found for kobject 'comedi0_subd1' [ 4972.978392] Modules linked in: usbdux(O) usbduxsigma(O) comedi_fc(O) comedi(O) radeon bnep rfcomm bluetooth snd_hda_codec_idt snd_hda_intel snd_hda_codec parport_pc ppdev rc_hauppauge snd_bt87x ir_kbd_i2c snd_hwdep tuner snd_pcm msp3400 snd_page_alloc snd_seq_midi bttv snd_seq_midi_event snd_rawmidi pcmcia hid_generic snd_seq pcmcia_core ttm usbhid hid drm_kms_helper snd_seq_device drm btcx_risc snd_timer tveeprom videobuf_dma_sg rc_core snd v4l2_common psmouse videobuf_core gpio_ich videodev dcdbas microcode serio_raw lpc_ich i2c_algo_bit soundcore mac_hid lp parport e100 ahci mii libahci [last unloaded: comedi] [ 4972.978454] CPU: 0 PID: 33 Comm: khubd Tainted: GWC O 3.13.0-rc3+ #1 [ 4972.978457] Hardware name: Dell Inc. Dimension 9100 /0X8582, BIOS A01 05/25/2005 [ 4972.978460] 0009 88005c801a38 81721083 88005c801a80 [ 4972.978466] 88005c801a70 810645fd 81caa2e0 [ 4972.978472] 8800560ed810 88005a615000 88005c801ad0 [ 4972.978478] Call Trace: [ 4972.978484] [] dump_stack+0x45/0x56 [ 4972.978489] [] warn_slowpath_common+0x7d/0xa0 [ 4972.978494] [] warn_slowpath_fmt+0x4c/0x50 [ 4972.978499] [] ? sysfs_get_dirent_ns+0x4e/0x70 [ 4972.978504] [] sysfs_remove_group+0xc6/0xd0 [ 4972.978509] [] dpm_sysfs_remove+0x43/0x50 [ 4972.978513] [] device_del+0x45/0x1c0 [ 4972.978518] [] device_unregister+0x1e/0x60 [ 4972.978522] [] device_destroy+0x3c/0x50 [ 4972.978530] [] comedi_free_subdevice_minor+0x75/0xa0 [comedi] [ 4972.978539] [] comedi_device_detach+0x48/0x160 [comedi] [ 4972.978546] [] comedi_device_cleanup+0x33/0x90 [comedi] [ 4972.978552] [] comedi_free_board_dev+0x36/0x50 [comedi] [ 4972.978558] [] comedi_release_hardware_device+0x80/0x90 [comedi] [ 4972.978565] [] comedi_auto_unconfig+0x13/0x20 [comedi] [ 4972.978572] [] comedi_usb_auto_unconfig+0x12/0x20 [comedi] [ 4972.978578] [] usb_unbind_interface+0x64/0x1c0 [ 4972.978583] [] __device_release_driver+0x7f/0xf0 [ 4972.978587] [] device_release_driver+0x23/0x30 [ 4972.978592] [] bus_remove_device+0x108/0x180 [ 4972.978596] [] device_del+0x129/0x1c0 [ 4972.978601] [] usb_disable_device+0xb0/0x290 [ 4972.978605] [] usb_disconnect+0xad/0x200 [ 4972.978609] [] hub_thread+0x70d/0x1750 [ 4972.978614] [] ? sched_clock_cpu+0xa8/0x100 [ 4972.978619] [] ? prepare_to_wait_event+0x100/0x100 [ 4972.978623] [] ? usb_reset_device+0x1d0/0x1d0 [ 4972.978627] [] kthread+0xd2/0xf0 [ 4972.978632] [] ? kthread_create_on_node+0x190/0x190 [ 4972.978637] [] ret_from_fork+0x7c/0xb0 [ 4972.978641] [] ? kthread_create_on_node+0x190/0x190 [ 4972.978644] ---[ end trace 69c2b5c4559cdf1b ]--- I guess that subdevice no longer exists at this point? /Bernd I'm hoping that will be fixed by other changes I made to the comedi core recently that aren't in the RC kernel yet. -- http://www.berndporr.me.uk http://www.linux-usb-daq.co.uk http://www.imdb.com/name/nm3293421/ +44 (0)7840 340069 >From 5971245d01f25890826fc05f7bab0d2b8d6bfd63 Mon Sep 17 00:00:00 2001 From: Bernd Porr Date: Fri, 20 Dec 2013 23:32:08 + Subject: [PATCH 1/1] Moving un-registering of the subdevices and the main comedi device into one function and the module which is responsible for file operations. The kernel oops observed before was because the main device was un-registered first and then the subdevices which were then no longer valid. Signed-off-by: Bernd Porr --- drivers/staging/comedi/comedi_fops.c | 19 +++ drivers/staging/comedi/drivers.c | 18 -- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c index d7f63c4..33b4e58 100644 --- a/drivers/staging/comedi/comedi_fops.c +++ b/drivers/staging/comedi/comedi_fops.c @@ -152,7
patch to add a success/failure message to comedi autoconfig
Hi all,
I've added a success / fail message to comedi autoconfig. That's badly
needed to see which driver has been associated with which comedi dev in
udev. Also, as far as I know an error in the USB probe callback won't
cause the kernel to print any errors so tracking down failures in
autoconfig would be really tricky.
/Bernd
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
>From e516c966fb36a9a8fee3e743ec97d4732cd9285c Mon Sep 17 00:00:00 2001
From: Bernd Porr
Date: Sat, 21 Dec 2013 09:52:11 +
Subject: [PATCH 1/1] Added success message to the driver autoconfig and error
message in case it fails. A success message is required so that the user can
find out which comedi driver has been associated with which udev device. This
also makes troubleshooting much easier when more than one card is in the
computer or a mix of USB and PCI devices.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/comedi_fops.c | 1 +
drivers/staging/comedi/drivers.c | 17 +++--
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index 33b4e58..4e53c5d 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2523,6 +2523,7 @@ struct comedi_device *comedi_alloc_board_minor(struct device *hardware_device)
return ERR_PTR(-EBUSY);
}
dev->minor = i;
+
csdev = device_create(comedi_class, hardware_device,
MKDEV(COMEDI_MAJOR, i), NULL, "comedi%i", i);
if (!IS_ERR(csdev))
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 29b3480..f9be097 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -585,8 +585,12 @@ int comedi_auto_config(struct device *hardware_device,
}
dev = comedi_alloc_board_minor(hardware_device);
- if (IS_ERR(dev))
+ if (IS_ERR(dev)) {
+ dev_warn(hardware_device,
+ "comedi driver '%s' could not create device.\n",
+ driver->driver_name);
return PTR_ERR(dev);
+ }
/* Note: comedi_alloc_board_minor() locked dev->mutex. */
dev->driver = driver;
@@ -598,8 +602,17 @@ int comedi_auto_config(struct device *hardware_device,
comedi_device_detach(dev);
mutex_unlock(&dev->mutex);
- if (ret < 0)
+ if (ret < 0) {
+ dev_warn(hardware_device,
+ "comedi driver '%s' could not be auto-configured.\n",
+ driver->driver_name);
comedi_release_hardware_device(hardware_device);
+ } else {
+ /* class_dev should be set properly here after a successul auto config*/
+ dev_info(dev->class_dev,
+ "'%s' has been successfully auto-configured.\n",
+ driver->driver_name);
+ }
return ret;
}
EXPORT_SYMBOL_GPL(comedi_auto_config);
--
1.8.5.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: patch to add a success/failure message to comedi autoconfig
Hi Hartley, it's essentially what I've also written within the patch but I can make the comment in the patch a bit meatier if you want. However, I'm travelling now and won't be able to work on it till new year probably. Best, /Bernd Hartley Sweeten wrote: On Sunday, December 22, 2013 1:30 PM, Bernd Porr wrote: Hi all, I've added a success / fail message to comedi autoconfig. That's badly needed to see which driver has been associated with which comedi dev in udev. Also, as far as I know an error in the USB probe callback won't cause the kernel to print any errors so tracking down failures in autoconfig would be really tricky. Bernd, Please re-post this with the patch inline. The comment above is not required since the commit message should provide the necessary information about the patch. Thanks, Hartley -- http://www.berndporr.me.uk http://www.linux-usb-daq.co.uk http://www.imdb.com/name/nm3293421/ +44 (0)7840 340069 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: comedi: report success/failure of autoconfig
Added success message to the driver autoconfig and error
message in case it fails. A success message is required
so that the user can find out which comedi driver has been
associated with which udev device. This also makes troubleshooting
much easier when more than one card is in the computer or a
mix of USB and PCI devices.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/drivers.c | 17 +++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index d6dc58a..58c7f23 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -580,8 +580,12 @@ int comedi_auto_config(struct device *hardware_device,
}
dev = comedi_alloc_board_minor(hardware_device);
- if (IS_ERR(dev))
+ if (IS_ERR(dev)) {
+ dev_warn(hardware_device,
+"comedi driver '%s' could not create device.\n",
+driver->driver_name);
return PTR_ERR(dev);
+ }
/* Note: comedi_alloc_board_minor() locked dev->mutex. */
dev->driver = driver;
@@ -593,8 +597,17 @@ int comedi_auto_config(struct device *hardware_device,
comedi_device_detach(dev);
mutex_unlock(&dev->mutex);
- if (ret < 0)
+ if (ret < 0) {
+ dev_warn(hardware_device,
+"comedi driver '%s' could not be auto-configured.\n",
+driver->driver_name);
comedi_release_hardware_device(hardware_device);
+ } else {
+ /* class_dev should be set properly here after a successul auto
config*/
+ dev_info(dev->class_dev,
+"'%s' has been successfully auto-configured.\n",
+driver->driver_name);
+ }
return ret;
}
EXPORT_SYMBOL_GPL(comedi_auto_config);
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: comedi: fix auto-unconfig kernel error
Signed-off-by: Bernd Porr
Merging un-registering of both the subdevices and the
main comedi device into one function and the module which
actually associated with it. The kernel oops observed before
was because the main device was un-registered first and
then the subdevices which were then no longer valid.
This has been also tested with 'comedi_config -r'.
---
drivers/staging/comedi/comedi_fops.c | 19 +++
drivers/staging/comedi/drivers.c | 18 --
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index f3d59e2..2680b72 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -141,7 +141,26 @@ static struct comedi_device
*comedi_clear_board_minor(unsigned minor)
static void comedi_free_board_dev(struct comedi_device *dev)
{
+ int i;
+ struct comedi_subdevice *s;
+
if (dev) {
+ if (dev->subdevices) {
+ for (i = 0; i < dev->n_subdevices; i++) {
+ s = &dev->subdevices[i];
+ if (s->runflags & SRF_FREE_SPRIV)
+ kfree(s->private);
+ comedi_free_subdevice_minor(s);
+ if (s->async) {
+ comedi_buf_alloc(dev, s, 0);
+ kfree(s->async);
+ }
+ }
+ kfree(dev->subdevices);
+ dev->subdevices = NULL;
+ dev->n_subdevices = 0;
+ }
+
if (dev->class_dev) {
device_destroy(comedi_class,
MKDEV(COMEDI_MAJOR, dev->minor));
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 4964d2a..d6dc58a 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -97,24 +97,6 @@ EXPORT_SYMBOL_GPL(comedi_alloc_subdevices);
static void cleanup_device(struct comedi_device *dev)
{
- int i;
- struct comedi_subdevice *s;
-
- if (dev->subdevices) {
- for (i = 0; i < dev->n_subdevices; i++) {
- s = &dev->subdevices[i];
- if (s->runflags & SRF_FREE_SPRIV)
- kfree(s->private);
- comedi_free_subdevice_minor(s);
- if (s->async) {
- comedi_buf_alloc(dev, s, 0);
- kfree(s->async);
- }
- }
- kfree(dev->subdevices);
- dev->subdevices = NULL;
- dev->n_subdevices = 0;
- }
kfree(dev->private);
dev->private = NULL;
dev->driver = NULL;
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: comedi: fix auto-unconfig kernel error
"Signed-off-by" should be after the description. :)
How did I manage that? ;)
Merging un-registering of both the subdevices and the
main comedi device into one function and the module which
actually associated with it. The kernel oops observed before
was because the main device was un-registered first and
then the subdevices which were then no longer valid.
This has been also tested with 'comedi_config -r'.
It might work with 'comedi_config -r' for auto-configured devices, but I
don't think it will work for manually configured "legacy" devices (e.g.
a "comedi_test" device configured by 'comedi_config /dev/comedi0
comedi_test' when the 'comedi_num_legacy_minors' module parameter is
greater than 0). comedi_free_board_dev() wouldn't be called in that case
when the device is unconfigured later, because the legacy comedi devices
persist until the comedi module is unloaded, whereas the auto-configured
comedi devices only persist while the low-level device remains attached
(more or less - they now also persist while referenced by an unreleased
file object).
I guess there is a flag we could check for the legacy drivers? Could you
point me to it and I add it? I can do some work today in between and
then I'm off for a week till 6th Jan.
/Bernd
---
drivers/staging/comedi/comedi_fops.c | 19 +++
drivers/staging/comedi/drivers.c | 18 --
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index f3d59e2..2680b72 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -141,7 +141,26 @@ static struct comedi_device
*comedi_clear_board_minor(unsigned minor)
static void comedi_free_board_dev(struct comedi_device *dev)
{
+int i;
+struct comedi_subdevice *s;
+
if (dev) {
+if (dev->subdevices) {
+for (i = 0; i < dev->n_subdevices; i++) {
+s = &dev->subdevices[i];
+if (s->runflags & SRF_FREE_SPRIV)
+kfree(s->private);
+comedi_free_subdevice_minor(s);
+if (s->async) {
+comedi_buf_alloc(dev, s, 0);
+kfree(s->async);
+}
+}
+kfree(dev->subdevices);
+dev->subdevices = NULL;
+dev->n_subdevices = 0;
+}
+
if (dev->class_dev) {
device_destroy(comedi_class,
MKDEV(COMEDI_MAJOR, dev->minor));
diff --git a/drivers/staging/comedi/drivers.c
b/drivers/staging/comedi/drivers.c
index 4964d2a..d6dc58a 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -97,24 +97,6 @@ EXPORT_SYMBOL_GPL(comedi_alloc_subdevices);
static void cleanup_device(struct comedi_device *dev)
{
-int i;
-struct comedi_subdevice *s;
-
-if (dev->subdevices) {
-for (i = 0; i < dev->n_subdevices; i++) {
-s = &dev->subdevices[i];
-if (s->runflags & SRF_FREE_SPRIV)
-kfree(s->private);
-comedi_free_subdevice_minor(s);
-if (s->async) {
-comedi_buf_alloc(dev, s, 0);
-kfree(s->async);
-}
-}
-kfree(dev->subdevices);
-dev->subdevices = NULL;
-dev->n_subdevices = 0;
-}
kfree(dev->private);
dev->private = NULL;
dev->driver = NULL;
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] staging: comedi: fix auto-unconfig kernel error
Hi Ian,
I've just tested it with a couple of legacy minors and it works fine.
You have already taken care of it. comedi_free_board_dev(dev) is only
called for autoconfigured devices. If they are legacy devices
comedi_free_board_dev won't be called:
-comedi_fops.c--
if (arg == 0 &&
dev->minor >= comedi_num_legacy_minors) {
/* Successfully unconfigured a
dynamically
* allocated device. Try and remove it. */
if (comedi_clear_board_dev(dev)) {
mutex_unlock(&dev->mutex);
comedi_free_board_dev(dev);
return rc;
}
}
-
I've tested it now with:
modprobe comedi comedi_num_legacy_minors=5
modprobe comedi_test
comedi_config /dev/comedi0 comedi_test
comedi_config -r /dev/comedi0
I'll add this to the commit message.
/Bernd
Ian Abbott wrote:
On 27/12/13 23:49, Bernd Porr wrote:
Signed-off-by: Bernd Porr
"Signed-off-by" should be after the description. :)
Merging un-registering of both the subdevices and the
main comedi device into one function and the module which
actually associated with it. The kernel oops observed before
was because the main device was un-registered first and
then the subdevices which were then no longer valid.
This has been also tested with 'comedi_config -r'.
It might work with 'comedi_config -r' for auto-configured devices, but I
don't think it will work for manually configured "legacy" devices (e.g.
a "comedi_test" device configured by 'comedi_config /dev/comedi0
comedi_test' when the 'comedi_num_legacy_minors' module parameter is
greater than 0). comedi_free_board_dev() wouldn't be called in that case
when the device is unconfigured later, because the legacy comedi devices
persist until the comedi module is unloaded, whereas the auto-configured
comedi devices only persist while the low-level device remains attached
(more or less - they now also persist while referenced by an unreleased
file object).
---
drivers/staging/comedi/comedi_fops.c | 19 +++
drivers/staging/comedi/drivers.c | 18 --
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index f3d59e2..2680b72 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -141,7 +141,26 @@ static struct comedi_device
*comedi_clear_board_minor(unsigned minor)
static void comedi_free_board_dev(struct comedi_device *dev)
{
+int i;
+struct comedi_subdevice *s;
+
if (dev) {
+if (dev->subdevices) {
+for (i = 0; i < dev->n_subdevices; i++) {
+s = &dev->subdevices[i];
+if (s->runflags & SRF_FREE_SPRIV)
+kfree(s->private);
+comedi_free_subdevice_minor(s);
+if (s->async) {
+comedi_buf_alloc(dev, s, 0);
+kfree(s->async);
+}
+}
+kfree(dev->subdevices);
+dev->subdevices = NULL;
+dev->n_subdevices = 0;
+}
+
if (dev->class_dev) {
device_destroy(comedi_class,
MKDEV(COMEDI_MAJOR, dev->minor));
diff --git a/drivers/staging/comedi/drivers.c
b/drivers/staging/comedi/drivers.c
index 4964d2a..d6dc58a 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -97,24 +97,6 @@ EXPORT_SYMBOL_GPL(comedi_alloc_subdevices);
static void cleanup_device(struct comedi_device *dev)
{
-int i;
-struct comedi_subdevice *s;
-
-if (dev->subdevices) {
-for (i = 0; i < dev->n_subdevices; i++) {
-s = &dev->subdevices[i];
-if (s->runflags & SRF_FREE_SPRIV)
-kfree(s->private);
-comedi_free_subdevice_minor(s);
-if (s->async) {
-comedi_buf_alloc(dev, s, 0);
-kfree(s->async);
-}
-}
-kfree(dev->subdevices);
-dev->subdevices = NULL;
-dev->n_subdevices = 0;
-}
kfree(dev->private);
dev->private = NULL;
dev->driver = NULL;
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: comedi: report success/failure of autoconfig
Added success message to the driver autoconfig and error
message in case it fails. A success message is required
so that the user can find out which comedi driver has been
associated with which udev device. This also makes troubleshooting
much easier when more than one card is in the computer or a
mix of USB and PCI devices.
As Ian suggested we should report both the driver and the board
which might have different names, esp if one driver covers a
range of different boards.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/drivers.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index d6dc58a..59a8909 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -580,8 +580,12 @@ int comedi_auto_config(struct device *hardware_device,
}
dev = comedi_alloc_board_minor(hardware_device);
- if (IS_ERR(dev))
+ if (IS_ERR(dev)) {
+ dev_warn(hardware_device,
+"driver '%s' could not create device.\n",
+driver->driver_name);
return PTR_ERR(dev);
+ }
/* Note: comedi_alloc_board_minor() locked dev->mutex. */
dev->driver = driver;
@@ -593,8 +597,19 @@ int comedi_auto_config(struct device *hardware_device,
comedi_device_detach(dev);
mutex_unlock(&dev->mutex);
- if (ret < 0)
+ if (ret < 0) {
+ dev_warn(hardware_device,
+"driver '%s' faild to auto-configure device.\n",
+driver->driver_name);
comedi_release_hardware_device(hardware_device);
+ } else {
+ /* class_dev should be set properly here
+ after a successful auto config */
+ dev_info(dev->class_dev,
+"driver '%s' has successfully "
+"auto-configured '%s'.\n",
+driver->driver_name, dev->board_name);
+ }
return ret;
}
EXPORT_SYMBOL_GPL(comedi_auto_config);
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: comedi: drivers: streamlined auto attach with main comedi
Removed the word "attached" from the "ADC_zero" output which is
now reported by comedi itself at the end of the auto attach.
A negative value of the offset is an error and should be reported to
comedi auto config as an error. Output only the offset if no error
has been reported.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/drivers/usbduxsigma.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index a5363de..125eae5 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -1656,11 +1656,14 @@ static int usbduxsigma_auto_attach(struct comedi_device
*dev,
}
offset = usbduxsigma_getstatusinfo(dev, 0);
- if (offset < 0)
+ if (offset < 0) {
+ /* a neg ADC value in comedi terms is a device error */
dev_err(dev->class_dev,
- "Communication to USBDUXSIGMA failed! Check firmware
and cabling\n");
+ "Communication to USBDUXSIGMA failed! Check firmware
and cabling.\n");
+ return offset;
+ }
- dev_info(dev->class_dev, "attached, ADC_zero = %x\n", offset);
+ dev_info(dev->class_dev, "ADC_zero = %x\n", offset);
return 0;
}
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: comedi: fix auto-unconfig kernel error
Merging the un-registering of both the subdevices and the
main comedi device into one function and the module which
actually associated with it. The kernel oops observed before
was because the main device was un-registered first and
then the subdevices which were then no longer valid.
This has been also tested with 'comedi_config -r' for
both autoconfigured and legacy devices.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/comedi_fops.c | 19 +++
drivers/staging/comedi/drivers.c | 18 --
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index f3d59e2..2680b72 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -141,7 +141,26 @@ static struct comedi_device
*comedi_clear_board_minor(unsigned minor)
static void comedi_free_board_dev(struct comedi_device *dev)
{
+ int i;
+ struct comedi_subdevice *s;
+
if (dev) {
+ if (dev->subdevices) {
+ for (i = 0; i < dev->n_subdevices; i++) {
+ s = &dev->subdevices[i];
+ if (s->runflags & SRF_FREE_SPRIV)
+ kfree(s->private);
+ comedi_free_subdevice_minor(s);
+ if (s->async) {
+ comedi_buf_alloc(dev, s, 0);
+ kfree(s->async);
+ }
+ }
+ kfree(dev->subdevices);
+ dev->subdevices = NULL;
+ dev->n_subdevices = 0;
+ }
+
if (dev->class_dev) {
device_destroy(comedi_class,
MKDEV(COMEDI_MAJOR, dev->minor));
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 4964d2a..d6dc58a 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -97,24 +97,6 @@ EXPORT_SYMBOL_GPL(comedi_alloc_subdevices);
static void cleanup_device(struct comedi_device *dev)
{
- int i;
- struct comedi_subdevice *s;
-
- if (dev->subdevices) {
- for (i = 0; i < dev->n_subdevices; i++) {
- s = &dev->subdevices[i];
- if (s->runflags & SRF_FREE_SPRIV)
- kfree(s->private);
- comedi_free_subdevice_minor(s);
- if (s->async) {
- comedi_buf_alloc(dev, s, 0);
- kfree(s->async);
- }
- }
- kfree(dev->subdevices);
- dev->subdevices = NULL;
- dev->n_subdevices = 0;
- }
kfree(dev->private);
dev->private = NULL;
dev->driver = NULL;
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: comedi: USB devs not working / some comedi core reorganization
Hi Dan, see my re-submission of these patches. I did that properly with the git email this time. Hope that's now all properly formatted. Thanks also for the link. Best, /Bernd Dan Carpenter wrote: On Fri, Dec 20, 2013 at 11:48:31PM +0000, Bernd Porr wrote: >From 5971245d01f25890826fc05f7bab0d2b8d6bfd63 Mon Sep 17 00:00:00 2001 From: Bernd Porr Date: Fri, 20 Dec 2013 23:32:08 + Subject: [PATCH 1/1] Moving un-registering of the subdevices and the main comedi device into one function and the module which is responsible for file operations. The kernel oops observed before was because the main device was un-registered first and then the subdevices which were then no longer valid. When you are doing a git commit then the first line of the commit message is the subject and then put a blank line followed by the body of the commit message. [PATCH] Staging: comedi: fix remove ordering Bla blah blah explanation. Put the warning message in the commit log as well. Really patches need to be sent inline and not as an attachment. Greg deals with too many patches and has scripts to handle inline patches and has to handle attachments manually. Everyone else has review scripts for inline patches as well. http://www.freedesktop.org/wiki/Software/PulseAudio/HowToUseGitSendEmail/ regards, dan carpenter -- http://www.berndporr.me.uk http://www.linux-usb-daq.co.uk http://www.imdb.com/name/nm3293421/ +44 (0)7840 340069 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: comedi: fix auto-unconfig kernel error
Hi Ian,
will test later and fix the patches / rebase them to the latest version.
For future patches: shall I in general submit my patches against the
latest linux next or against linux next + Greg's remote staging tree?
I'm a bit confused because I thought linux next plus Gregs remote would
be the most cutting edge version? Should I just patch against linux-next?
/Bernd
Ian Abbott wrote:
On 2013-12-28 21:31, Bernd Porr wrote:
Merging the un-registering of both the subdevices and the
main comedi device into one function and the module which
actually associated with it. The kernel oops observed before
was because the main device was un-registered first and
then the subdevices which were then no longer valid.
This has been also tested with 'comedi_config -r' for
both autoconfigured and legacy devices.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/comedi_fops.c | 19 +++
drivers/staging/comedi/drivers.c | 18 --
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index f3d59e2..2680b72 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -141,7 +141,26 @@ static struct comedi_device
*comedi_clear_board_minor(unsigned minor)
static void comedi_free_board_dev(struct comedi_device *dev)
{
+int i;
+struct comedi_subdevice *s;
+
if (dev) {
+if (dev->subdevices) {
+for (i = 0; i < dev->n_subdevices; i++) {
+s = &dev->subdevices[i];
+if (s->runflags & SRF_FREE_SPRIV)
+kfree(s->private);
+comedi_free_subdevice_minor(s);
+if (s->async) {
+comedi_buf_alloc(dev, s, 0);
+kfree(s->async);
+}
+}
+kfree(dev->subdevices);
+dev->subdevices = NULL;
+dev->n_subdevices = 0;
+}
+
if (dev->class_dev) {
device_destroy(comedi_class,
MKDEV(COMEDI_MAJOR, dev->minor));
diff --git a/drivers/staging/comedi/drivers.c
b/drivers/staging/comedi/drivers.c
index 4964d2a..d6dc58a 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -97,24 +97,6 @@ EXPORT_SYMBOL_GPL(comedi_alloc_subdevices);
static void cleanup_device(struct comedi_device *dev)
{
-int i;
-struct comedi_subdevice *s;
-
-if (dev->subdevices) {
-for (i = 0; i < dev->n_subdevices; i++) {
-s = &dev->subdevices[i];
-if (s->runflags & SRF_FREE_SPRIV)
-kfree(s->private);
-comedi_free_subdevice_minor(s);
-if (s->async) {
-comedi_buf_alloc(dev, s, 0);
-kfree(s->async);
-}
-}
-kfree(dev->subdevices);
-dev->subdevices = NULL;
-dev->n_subdevices = 0;
-}
kfree(dev->private);
dev->private = NULL;
dev->driver = NULL;
This doesn't apply to linux-next any more. (For example,
cleanup_device() function was renamed amongst other stuff.) I've also
fixed a load of stuff related to this bug since this patch was
applicable, so possibly the bug has been squashed already.
Bernd, can you reproduce the problem on the latest tagged linux-next
branch?
Also, the removal of all that code from cleanup_device() (since renamed
to comedi_device_detach_cleanup()) would stop the COMEDI_DEVCONFIG ioctl
with NULL argument (e.g. via the 'comedi_config -r' command to detach a
device) cleaning up after the detachment, especially for the
preallocated "legacy" comedi devices that hang around after the
detachment, ready to be reattached via another instance of the ioctl call.
--
http://www.berndporr.me.uk
http://www.linux-usb-daq.co.uk
http://www.imdb.com/name/nm3293421/
+44 (0)7840 340069
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/3] staging: comedi: fix auto-unconfig kernel error
Ian Abbott wrote:
On 2013-12-28 21:31, Bernd Porr wrote:
Merging the un-registering of both the subdevices and the
main comedi device into one function and the module which
actually associated with it. The kernel oops observed before
was because the main device was un-registered first and
then the subdevices which were then no longer valid.
This has been also tested with 'comedi_config -r' for
both autoconfigured and legacy devices.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/comedi_fops.c | 19 +++
drivers/staging/comedi/drivers.c | 18 --
2 files changed, 19 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c
b/drivers/staging/comedi/comedi_fops.c
index f3d59e2..2680b72 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -141,7 +141,26 @@ static struct comedi_device
*comedi_clear_board_minor(unsigned minor)
static void comedi_free_board_dev(struct comedi_device *dev)
{
+int i;
+struct comedi_subdevice *s;
+
if (dev) {
+if (dev->subdevices) {
+for (i = 0; i < dev->n_subdevices; i++) {
+s = &dev->subdevices[i];
+if (s->runflags & SRF_FREE_SPRIV)
+kfree(s->private);
+comedi_free_subdevice_minor(s);
+if (s->async) {
+comedi_buf_alloc(dev, s, 0);
+kfree(s->async);
+}
+}
+kfree(dev->subdevices);
+dev->subdevices = NULL;
+dev->n_subdevices = 0;
+}
+
if (dev->class_dev) {
device_destroy(comedi_class,
MKDEV(COMEDI_MAJOR, dev->minor));
diff --git a/drivers/staging/comedi/drivers.c
b/drivers/staging/comedi/drivers.c
index 4964d2a..d6dc58a 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -97,24 +97,6 @@ EXPORT_SYMBOL_GPL(comedi_alloc_subdevices);
static void cleanup_device(struct comedi_device *dev)
{
-int i;
-struct comedi_subdevice *s;
-
-if (dev->subdevices) {
-for (i = 0; i < dev->n_subdevices; i++) {
-s = &dev->subdevices[i];
-if (s->runflags & SRF_FREE_SPRIV)
-kfree(s->private);
-comedi_free_subdevice_minor(s);
-if (s->async) {
-comedi_buf_alloc(dev, s, 0);
-kfree(s->async);
-}
-}
-kfree(dev->subdevices);
-dev->subdevices = NULL;
-dev->n_subdevices = 0;
-}
kfree(dev->private);
dev->private = NULL;
dev->driver = NULL;
This doesn't apply to linux-next any more. (For example,
cleanup_device() function was renamed amongst other stuff.) I've also
fixed a load of stuff related to this bug since this patch was
applicable, so possibly the bug has been squashed already.
Bernd, can you reproduce the problem on the latest tagged linux-next
branch?
I can. Still the same problem as before.
Linux bp1-Dimension-9100 3.13.0-rc7-next-20140106 #1 SMP Tue Jan 7
06:36:37 GMT 2014 x86_64 x86_64 x86_64 GNU/Linux
[ 74.183729] [ cut here ]
[ 74.183741] WARNING: CPU: 0 PID: 28 at fs/sysfs/group.c:216
sysfs_remove_group+0x93/0xa0()
[ 74.183744] sysfs group 81caa9e0 not found for kobject
'comedi0_subd0'
[ 74.183747] Modules linked in: radeon bnep rfcomm bluetooth
snd_hda_codec_idt snd_hda_codec_generic snd_hda_intel 6lowpan_iphc(P)
snd_hda_codec rc_hauppauge ir_kbd_i2c ttm tuner drm_kms_helper msp3400
snd_hwdep snd_bt87x drm snd_pcm bttv hid_generic gpio_ich dcdbas
microcode usbduxsigma(C) snd_page_alloc comedi_fc(C) comedi(C) usbhid
btcx_risc snd_seq_midi tveeprom videobuf_dma_sg snd_seq_midi_event
snd_rawmidi rc_core psmouse snd_seq v4l2_common pcmcia pcmcia_core hid
snd_seq_device videobuf_core snd_timer serio_raw videodev snd lpc_ich
i2c_algo_bit soundcore ahci e100 libahci mii
[ 74.183798] CPU: 0 PID: 28 Comm: khubd Tainted: PWC
3.13.0-rc7-next-20140106 #1
[ 74.183801] Hardware name: Dell Inc. Dimension 9100
/0X8582, BIOS A01 05/25/2005
[ 74.183804] 0009 88005cfb1a30 8172cf2d
88005cfb1a78
[ 74.183809] 88005cfb1a68 81066cbd
81caa9e0
[ 74.183813] 880058562010 880058413400
88005cfb1ac8
[ 74.183817] Call Trace:
[ 74.183827] [] dump_stack+0x45/0x56
[ 74.183832] [] warn_slowpath_common+0x7d/0xa0
[ 74.183836] [] warn_slowpath_fmt+0x4c/0x50
[ 74.183841] [] ? kernfs_find_and_get_ns+0x48/0x60
[ 74.183845] [] sysfs_remove_group+0x93/0xa0
[ 74.183850] [] dpm_sysfs_remove+0x43/0x50
[ 74.183854] [] device_del+0x45/0x1c0
[ 74.183857] [] device_unr
Re: [PATCH 1/3] staging: comedi: fix auto-unconfig kernel error
Ian Abbott wrote: On 2014-01-07 10:01, Bernd Porr wrote: Ian Abbott wrote: On 2013-12-28 21:31, Bernd Porr wrote: This doesn't apply to linux-next any more. (For example, cleanup_device() function was renamed amongst other stuff.) I've also fixed a load of stuff related to this bug since this patch was applicable, so possibly the bug has been squashed already. Bernd, can you reproduce the problem on the latest tagged linux-next branch? I can. Still the same problem as before. Linux bp1-Dimension-9100 3.13.0-rc7-next-20140106 #1 SMP Tue Jan 7 06:36:37 GMT 2014 x86_64 x86_64 x86_64 GNU/Linux [ 74.183729] [ cut here ] [ 74.183741] WARNING: CPU: 0 PID: 28 at fs/sysfs/group.c:216 sysfs_remove_group+0x93/0xa0() [ 74.183744] sysfs group 81caa9e0 not found for kobject 'comedi0_subd0' I'll try and reproduce it. I don't have the usbdux devices, but I should be able to reproduce it with PCI devices that support comedi asynchronous commands by removing them via the /sys/bus/pci/devices/:xx:xx.x/remove interface. (I thought I had tested it actually, but must have missed something!). > Was the device in use by a comedi application when disconnected, or was > it just idle? > Hi, it was idling and gave the kernel error above. I've just run comedirecord and pulled the plug. That works fine (gives error -1 in userspace) and after plugging it back in it works. /Bernd BTW: I've also tried to compile Greg's staging tree but that goes into busybox (no root fs) without a working keyboard / mouse. -- http://www.berndporr.me.uk http://www.linux-usb-daq.co.uk http://www.imdb.com/name/nm3293421/ +44 (0)7840 340069 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: comedi: usbduxsigma: return failure of auto attach
The function usbduxsigma_getstatusinfo() returns a negative
value in case there has been a communication error with
the board. This should always work and if this communication
fails then there is something seriously wrong with the board.
This is now returned to the caller so that it can
terminte the auto attachement. The return command also prevents
printing out the offset value in case of a fault.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/drivers/usbduxsigma.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c
b/drivers/staging/comedi/drivers/usbduxsigma.c
index 194c686..ff521b3 100644
--- a/drivers/staging/comedi/drivers/usbduxsigma.c
+++ b/drivers/staging/comedi/drivers/usbduxsigma.c
@@ -1655,9 +1655,11 @@ static int usbduxsigma_auto_attach(struct comedi_device
*dev,
}
offset = usbduxsigma_getstatusinfo(dev, 0);
- if (offset < 0)
+ if (offset < 0) {
dev_err(dev->class_dev,
- "Communication to USBDUXSIGMA failed! Check firmware
and cabling\n");
+ "Communication to USBDUXSIGMA failed! Check firmware
and cabling.\n");
+ return offset;
+ }
dev_info(dev->class_dev, "attached, ADC_zero = %x\n", offset);
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/3] staging: comedi: report success/failure of autoconfig
Added success message to the driver autoconfig and error
message in case it fails. A success message is required
so that the user can find out which comedi driver has been
associated with which udev device. This also makes troubleshooting
much easier when more than one card is in the computer or
there is a mix of USB and PCI devices.
As Ian suggested we should report both the driver and the board
which might have different names, especially if one driver covers a
range of different boards.
Signed-off-by: Bernd Porr
---
drivers/staging/comedi/drivers.c | 20 ++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 9d71b4d..950f544 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -603,8 +603,12 @@ int comedi_auto_config(struct device *hardware_device,
}
dev = comedi_alloc_board_minor(hardware_device);
- if (IS_ERR(dev))
+ if (IS_ERR(dev)) {
+ dev_warn(hardware_device,
+"driver '%s' could not create device.\n",
+driver->driver_name);
return PTR_ERR(dev);
+ }
/* Note: comedi_alloc_board_minor() locked dev->mutex. */
dev->driver = driver;
@@ -616,8 +620,20 @@ int comedi_auto_config(struct device *hardware_device,
comedi_device_detach(dev);
mutex_unlock(&dev->mutex);
- if (ret < 0)
+ if (ret < 0) {
+ dev_warn(hardware_device,
+"driver '%s' failed to auto-configure device.\n",
+driver->driver_name);
comedi_release_hardware_device(hardware_device);
+ } else {
+ /*
+* class_dev should be set properly here
+* after a successful auto config
+*/
+ dev_info(dev->class_dev,
+"driver '%s' has successfully auto-configured '%s'.\n",
+driver->driver_name, dev->board_name);
+ }
return ret;
}
EXPORT_SYMBOL_GPL(comedi_auto_config);
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: comedi: usbduxsigma: removing unneccesay attached info
Comedi core now reports that a device has been attached so that the driver itself won't need to do it any longer. The driver now just outputs the offset of the ADC converter which is a soft indicator of the health of the board and also the user can grep this value from the kernel log easier for debugging purposes. Signed-off-by: Bernd Porr --- drivers/staging/comedi/drivers/usbduxsigma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c b/drivers/staging/comedi/drivers/usbduxsigma.c index ff521b3..3beeb12 100644 --- a/drivers/staging/comedi/drivers/usbduxsigma.c +++ b/drivers/staging/comedi/drivers/usbduxsigma.c @@ -1661,7 +1661,7 @@ static int usbduxsigma_auto_attach(struct comedi_device *dev, return offset; } - dev_info(dev->class_dev, "attached, ADC_zero = %x\n", offset); + dev_info(dev->class_dev, "ADC_zero = %x\n", offset); return 0; } -- 1.8.5.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/3] staging: comedi: usbduxfast: updated address details
Updated the range of years, e-mail and added driver desription as usually done in comedi. Signed-off-by: Bernd Porr --- drivers/staging/comedi/drivers/usbduxfast.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/usbduxfast.c b/drivers/staging/comedi/drivers/usbduxfast.c index f85818d..4f57321 100644 --- a/drivers/staging/comedi/drivers/usbduxfast.c +++ b/drivers/staging/comedi/drivers/usbduxfast.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2004 Bernd Porr, bernd.p...@f2s.com + * Copyright (C) 2004-2014 Bernd Porr, m...@berndporr.me.uk * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -12,6 +12,15 @@ * GNU General Public License for more details. */ +/* + * Driver: usbduxfast + * Description: University of Stirling USB DAQ & INCITE Technology Limited + * Devices: (ITL) USB-DUX [usbduxfast] + * Author: Bernd Porr + * Updated: 10 Oct 2014 + * Status: stable + */ + /* * I must give credit here to Chris Baugher who * wrote the driver for AT-MIO-16d. I used some parts of this -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/3] staging: comedi: usbduxsigma: updated contact details and status
I've updated my contact details of the driver. I've also tested it thoroughly and it works perfectly. I've changed the status to stable. Signed-off-by: Bernd Porr --- drivers/staging/comedi/drivers/usbduxsigma.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/staging/comedi/drivers/usbduxsigma.c b/drivers/staging/comedi/drivers/usbduxsigma.c index 94a09c1..309878e 100644 --- a/drivers/staging/comedi/drivers/usbduxsigma.c +++ b/drivers/staging/comedi/drivers/usbduxsigma.c @@ -1,6 +1,6 @@ /* * usbduxsigma.c - * Copyright (C) 2011 Bernd Porr, bernd.p...@f2s.com + * Copyright (C) 2011-2014 Bernd Porr, m...@berndporr.me.uk * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -17,9 +17,9 @@ * Driver: usbduxsigma * Description: University of Stirling USB DAQ & INCITE Technology Limited * Devices: (ITL) USB-DUX [usbduxsigma] - * Author: Bernd Porr - * Updated: 8 Nov 2011 - * Status: testing + * Author: Bernd Porr + * Updated: 10 Oct 2014 + * Status: stable */ /* -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/3] staging: comedi: usbdux: updated contact details / comments
I've updated my contact details and removed obsolete comments. Signed-off-by: Bernd Porr --- drivers/staging/comedi/drivers/usbdux.c | 59 - 1 file changed, 28 insertions(+), 31 deletions(-) diff --git a/drivers/staging/comedi/drivers/usbdux.c b/drivers/staging/comedi/drivers/usbdux.c index 053bc50..24fa9f66 100644 --- a/drivers/staging/comedi/drivers/usbdux.c +++ b/drivers/staging/comedi/drivers/usbdux.c @@ -1,37 +1,34 @@ /* - comedi/drivers/usbdux.c - Copyright (C) 2003-2007 Bernd Porr, bernd.p...@f2s.com - - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 2 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. + * usbdux.c + * Copyright (C) 2003-2014 Bernd Porr, m...@berndporr.me.uk + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. */ + /* -Driver: usbdux -Description: University of Stirling USB DAQ & INCITE Technology Limited -Devices: [ITL] USB-DUX (usbdux.o) -Author: Bernd Porr -Updated: 8 Dec 2008 -Status: Stable -Configuration options: - You have to upload firmware with the -i option. The - firmware is usually installed under /usr/share/usb or - /usr/local/share/usb or /lib/firmware. - -Connection scheme for the counter at the digital port: - 0=/CLK0, 1=UP/DOWN0, 2=RESET0, 4=/CLK1, 5=UP/DOWN1, 6=RESET1. - The sampling rate of the counter is approximately 500Hz. - -Please note that under USB2.0 the length of the channel list determines -the max sampling rate. If you sample only one channel you get 8kHz -sampling rate. If you sample two channels you get 4kHz and so on. -*/ + * Driver: usbdux + * Description: University of Stirling USB DAQ & INCITE Technology Limited + * Devices: (ITL) USB-DUX [usbdux] + * Author: Bernd Porr + * Updated: 10 Oct 2014 + * Status: Stable + * Connection scheme for the counter at the digital port: + * 0=/CLK0, 1=UP/DOWN0, 2=RESET0, 4=/CLK1, 5=UP/DOWN1, 6=RESET1. + * The sampling rate of the counter is approximately 500Hz. + * + * Note that under USB2.0 the length of the channel list determines + * the max sampling rate. If you sample only one channel you get 8kHz + * sampling rate. If you sample two channels you get 4kHz and so on. + */ + /* * I must give credit here to Chris Baugher who * wrote the driver for AT-MIO-16d. I used some parts of this -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/5] staging: comedi: usbdux: introduce usbduxsub_ao_handle_urb()
On 14/10/2014 20:14, H Hartley Sweeten wrote: Factor the urb handling and resubmit out of the analog output urb completion handler and tidy it up. This allows a common exit path to be used in the completion handler to stop the async command and handle the events. Signed-off-by: H Hartley Sweeten Reviewed-by: Bernd Porr ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/5] staging: comedi: usbdux: introduce usbduxsub_ai_handle_urb()
Signed-off-by: H Hartley Sweeten Reviewed-by: Bernd Porr ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/5] staging: comedi: usbduxfast: introduce usbduxfast_ai_handle_urb()
Signed-off-by: H Hartley Sweeten Reviewed-by: Bernd Porr ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 5/5] staging: comedi: usbduxsigma: introduce usbduxsigma_ai_handle_urb()
Signed-off-by: H Hartley Sweeten Reviewed-by: Bernd Porr ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/5] staging: comedi: usbduxsigma: introduce usbduxsigma_ao_handle_urb()
Signed-off-by: H Hartley Sweeten Reviewed-by: Bernd Porr ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
