Re: [PATCH v2 2/3] staging: dgnc: driver.c and tty.c: replaces dgnc_driver_kzmalloc with kzalloc
On Wed, Aug 28, 2013 at 08:05:14PM -0400, Lidza Louina wrote: Also, can I put your name in the Reported-by: section of these patches? It feels like cheating to boost your Reported-by count with style comments. :P I'd be caught up to Fengguang. Everyone likes credit though. I know the docs say to ask for permission but you only need to ask if it's something where they might want the report or their email secret. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: dgap: Fix build errors
Fixes the following compilation errors: drivers/staging/dgap/dgap_driver.c:423:3: error: implicit declaration of function ‘kfree’ [-Werror=implicit-function-declaration] kfree(dgap_config_buf); ^ drivers/staging/dgap/dgap_driver.c: In function ‘dgap_driver_kzmalloc’: drivers/staging/dgap/dgap_driver.c:940:3: error: implicit declaration of function ‘kmalloc’ [-Werror=implicit-function-declaration] Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- drivers/staging/dgap/dgap_driver.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/dgap/dgap_driver.c b/drivers/staging/dgap/dgap_driver.c index 9f777e4..724a685 100644 --- a/drivers/staging/dgap/dgap_driver.c +++ b/drivers/staging/dgap/dgap_driver.c @@ -36,6 +36,7 @@ #include linux/module.h #include linux/pci.h #include linux/delay.h /* For udelay */ +#include linux/slab.h #include asm/uaccess.h /* For copy_from_user/copy_to_user */ #if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,39) -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: dgap: Remove unnecessary version check
Code should be for the kernel version it is merged in. Version check is not necessary. Signed-off-by: Sachin Kamat sachin.ka...@linaro.org --- Compile tested only. --- drivers/staging/dgap/dgap_driver.c |4 drivers/staging/dgap/dgap_kcompat.h | 29 - 2 files changed, 33 deletions(-) diff --git a/drivers/staging/dgap/dgap_driver.c b/drivers/staging/dgap/dgap_driver.c index 724a685..65d7ee0 100644 --- a/drivers/staging/dgap/dgap_driver.c +++ b/drivers/staging/dgap/dgap_driver.c @@ -32,16 +32,12 @@ #include linux/kernel.h -#include linux/version.h #include linux/module.h #include linux/pci.h #include linux/delay.h /* For udelay */ #include linux/slab.h #include asm/uaccess.h /* For copy_from_user/copy_to_user */ - -#if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,39) #include linux/sched.h -#endif #include dgap_driver.h #include dgap_pci.h diff --git a/drivers/staging/dgap/dgap_kcompat.h b/drivers/staging/dgap/dgap_kcompat.h index 8ebf4b7..0dc2404 100644 --- a/drivers/staging/dgap/dgap_kcompat.h +++ b/drivers/staging/dgap/dgap_kcompat.h @@ -28,11 +28,6 @@ #ifndef __DGAP_KCOMPAT_H #define __DGAP_KCOMPAT_H -# ifndef KERNEL_VERSION -# define KERNEL_VERSION(a,b,c) (((a) 16) + ((b) 8) + (c)) -# endif - - #if !defined(TTY_FLIPBUF_SIZE) # define TTY_FLIPBUF_SIZE 512 #endif @@ -66,28 +61,4 @@ module_param(VAR, long, PERM); \ MODULE_PARM_DESC(VAR, DESC); - - - - -#if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,27) - - - - -/* NOTHING YET */ - - - - -# else - - - -# error this driver does not support anything below the 2.6.27 kernel series. - - - -# endif - #endif /* ! __DGAP_KCOMPAT_H */ -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/11] staging: comedi: core: introduce comedi_dio_insn_bits()
On 2013-08-28 21:26, H Hartley Sweeten wrote:
The (*insn_bits) functions for DIO and DO subdevices typically use
the subdevice 's-state' to hold the current state of the output
channels. The 'insn' passed to these functions, INSN_BITS, specifies
two parameters passed in the 'data'.
data[0] = 'mask', the channels to update
data[1] = 'bits', the new state for the channels
Introduce a helper function to handle this boilerplate.
The function returns:
0 if there are no changes in the state
1 if the driver needs to update the hardware to a new state
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/staging/comedi/comedidev.h | 2 ++
drivers/staging/comedi/drivers.c | 25 +
2 files changed, 27 insertions(+)
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index 2e19f65..de36499 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -345,6 +345,8 @@ void comedi_buf_memcpy_from(struct comedi_async *async,
unsigned int offset,
int comedi_dio_insn_config(struct comedi_device *, struct comedi_subdevice *,
struct comedi_insn *, unsigned int *data,
unsigned int mask);
+int comedi_dio_insn_bits(struct comedi_device *, struct comedi_subdevice *,
+struct comedi_insn *, unsigned int *data);
void *comedi_alloc_devpriv(struct comedi_device *, size_t);
int comedi_alloc_subdevices(struct comedi_device *, int);
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 317a821..2bbd9d0 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -190,6 +190,31 @@ int comedi_dio_insn_config(struct comedi_device *dev,
}
EXPORT_SYMBOL_GPL(comedi_dio_insn_config);
+/**
+ * comedi_dio_insn_bits() - boilerplate (*insn_bits) for DIO and DO subdevices.
+ * @dev: comedi_device struct
+ * @s: comedi_subdevice struct
+ * @insn: comedi_insn struct
+ * @data: parameters for the @insn
+ */
+int comedi_dio_insn_bits(struct comedi_device *dev,
+struct comedi_subdevice *s,
+struct comedi_insn *insn,
+unsigned int *data)
+{
+ unsigned int mask = data[0];
+ unsigned int bits = data[1];
+
+ if (mask) {
+ s-state = ~mask;
+ s-state |= (bits mask);
+
+ return 1;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(comedi_dio_insn_bits);
+
static int insn_rw_emulate_bits(struct comedi_device *dev,
struct comedi_subdevice *s,
struct comedi_insn *insn, unsigned int *data)
I'm not convinced this achieves much. It doesn't use the insn parameter
at all.
--
-=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 04/11] staging: comedi: drivers: use comedi_dio_insn_bits()
On 2013-08-28 21:28, H Hartley Sweeten wrote:
Use comedi_dio_insn_bits() to handle the boilerplate code to update
the subdevice s-state for DIO and DO subdevices.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
[snip]
diff --git a/drivers/staging/comedi/drivers/8255.c
b/drivers/staging/comedi/drivers/8255.c
index 2f070fd..d29f404 100644
--- a/drivers/staging/comedi/drivers/8255.c
+++ b/drivers/staging/comedi/drivers/8255.c
@@ -126,30 +126,17 @@ EXPORT_SYMBOL_GPL(subdev_8255_interrupt);
static int subdev_8255_insn(struct comedi_device *dev,
struct comedi_subdevice *s,
- struct comedi_insn *insn, unsigned int *data)
+ struct comedi_insn *insn,
+ unsigned int *data)
{
struct subdev_8255_private *spriv = s-private;
unsigned long iobase = spriv-iobase;
- unsigned int mask;
- unsigned int bits;
unsigned int v;
- mask = data[0];
- bits = data[1];
-
- if (mask) {
- v = s-state;
- v = ~mask;
- v |= (bits mask);
-
- if (mask 0xff)
- spriv-io(1, _8255_DATA, v 0xff, iobase);
- if (mask 0xff00)
- spriv-io(1, _8255_DATA + 1, (v 8) 0xff, iobase);
- if (mask 0xff)
- spriv-io(1, _8255_DATA + 2, (v 16) 0xff, iobase);
-
- s-state = v;
+ if (comedi_dio_insn_bits(dev, s, insn, data)) {
+ spriv-io(1, _8255_DATA, s-state 0xff, iobase);
+ spriv-io(1, _8255_DATA + 1, (s-state 8) 0xff, iobase);
+ spriv-io(1, _8255_DATA + 2, (s-state 16) 0xff, iobase);
}
There's extra register access overhead here that wasn't in the original.
Quite often, the mask only has one bit set (when insn_bits is being
used to emulate insn_write, for example), which would only write one
register. For PC I/O space, register writes typically take a
microsecond. Or at least they used to, but I think it might be less on
newer hardware.
For 8255, this change may also result in the output pin values being
different than they were before due to a quirk of the 8255. Following
an INSN_CONFIG_DIO_INPUT or INSN_CONFIG_DIO_OUTPUT, all output pins are
set to 0 by the chip, although comedi's s-state value is not affected.
Then a following INSN_BITS with a mask will set _all_ the output pins
to the new s-state, rather than setting _some of_ the output pins to
the new s-state. Maybe that's a good thing, although it's a change to
the existing behaviour.
Probably best to play it safe and write to the output registers
selectively according to the mask for the 8255 module.
Just a thought - perhaps comedi_dio_insn_bits() could return the mask
value to allow the caller to selectively write to registers based on the
mask if it wants to?
[snip]
diff --git a/drivers/staging/comedi/drivers/dt2817.c
b/drivers/staging/comedi/drivers/dt2817.c
index f4a8529..5de1330 100644
--- a/drivers/staging/comedi/drivers/dt2817.c
+++ b/drivers/staging/comedi/drivers/dt2817.c
@@ -80,36 +80,24 @@ static int dt2817_dio_insn_config(struct comedi_device *dev,
static int dt2817_dio_insn_bits(struct comedi_device *dev,
struct comedi_subdevice *s,
- struct comedi_insn *insn, unsigned int *data)
+ struct comedi_insn *insn,
+ unsigned int *data)
{
- unsigned int changed;
-
- /* It's questionable whether it is more important in
-* a driver like this to be deterministic or fast.
-* We choose fast. */
-
- if (data[0]) {
- changed = s-state;
- s-state = ~data[0];
- s-state |= (data[0] data[1]);
- changed ^= s-state;
- changed = s-io_bits;
- if (changed 0x00ff)
- outb(s-state 0xff, dev-iobase + DT2817_DATA + 0);
- if (changed 0xff00)
- outb((s-state 8) 0xff,
-dev-iobase + DT2817_DATA + 1);
- if (changed 0x00ff)
- outb((s-state 16) 0xff,
-dev-iobase + DT2817_DATA + 2);
- if (changed 0xff00)
- outb((s-state 24) 0xff,
-dev-iobase + DT2817_DATA + 3);
+ unsigned int val;
+
+ if (comedi_dio_insn_bits(dev, s, insn, data)) {
+ outb(s-state 0xff, dev-iobase + DT2817_DATA + 0);
+ outb((s-state 8) 0xff, dev-iobase + DT2817_DATA + 1);
+ outb((s-state 16) 0xff, dev-iobase + DT2817_DATA + 2);
+ outb((s-state 24) 0xff, dev-iobase +
Re: [PATCH 05/11] staging: comedi: core: initialize subdevice s-io_bits in postconfig
On 2013-08-28 21:29, H Hartley Sweeten wrote:
The subdevice 'io_bits' is a bit mask of the i/o configuration for
digital subdevices. '0' values indicate that a channel is configured
as an input and '1' values that the channel is an output. Since the
subdevice data is kzalloc()'d, all channels default as inputs.
Modify __comedi_device_postconfig() so that the 'io_bits' are correctly
initialized for Digital Output subdevices.
Remove all the unnecessary initializations of 's-io_bits' from the
drivers. Also, remove the unnecessary initialization of the 's-state'.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/staging/comedi/drivers.c | 7 +++
drivers/staging/comedi/drivers/8255.c | 3 ---
drivers/staging/comedi/drivers/addi-data/addi_common.c | 2 --
drivers/staging/comedi/drivers/addi_apci_3120.c| 2 --
drivers/staging/comedi/drivers/adl_pci6208.c | 1 -
drivers/staging/comedi/drivers/adl_pci9118.c | 2 --
drivers/staging/comedi/drivers/adv_pci1710.c | 4
drivers/staging/comedi/drivers/amplc_dio200_common.c | 2 --
drivers/staging/comedi/drivers/icp_multi.c | 3 ---
drivers/staging/comedi/drivers/me_daq.c| 1 -
drivers/staging/comedi/drivers/ni_660x.c | 1 -
drivers/staging/comedi/drivers/ni_daq_700.c| 1 -
12 files changed, 7 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 2bbd9d0..7477514 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -310,6 +310,13 @@ static int __comedi_device_postconfig(struct comedi_device
*dev)
if (s-type == COMEDI_SUBD_UNUSED)
continue;
+ if (s-type == COMEDI_SUBD_DO) {
+ if (s-n_chan 32)
+ s-io_bits = (1 s-n_chan) - 1;
+ else
+ s-io_bits = 0x;
+ }
+
if (s-len_chanlist == 0)
s-len_chanlist = 1;
You don't really need to test s-n_chan 32. For s-n_chan = 32,
s-io_bits will end up set to 0x anyway (and for s-n_chan 32,
the low-level drivers shouldn't really be using s-state and s-io_bits
anyway).
[snip]
--
-=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 07/11] staging: comedi: drivers: use comedi_dio_insn_bits() for DIO subdevices
On 2013-08-28 21:29, H Hartley Sweeten wrote:
Use comedi_dio_insn_bits() to handle the boilerplate code to update
the subdevice s-state for DIO subdevices.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
[snip]
diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
index 40e8d56..f3b8514 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
@@ -711,20 +711,11 @@ static int apci3xxx_dio_insn_bits(struct comedi_device
*dev,
struct comedi_insn *insn,
unsigned int *data)
{
- unsigned int mask = data[0];
- unsigned int bits = data[1];
unsigned int val;
- /* only update output channels */
- mask = s-io_bits;
- if (mask) {
- s-state = ~mask;
- s-state |= (bits mask);
-
- if (mask 0xff)
- outl(s-state 0xff, dev-iobase + 80);
- if (mask 0xff)
- outl((s-state 16) 0xff, dev-iobase + 112);
+ if (comedi_dio_insn_bits(dev, s, insn, data)) {
+ outl(s-state 0xff, dev-iobase + 80);
+ outl((s-state 16) 0xff, dev-iobase + 112);
}
Since I was noting drivers that filtered register writes depending on
the mask in the earlier patches, addi_apci_3xxx is one of those.
[snip]
diff --git a/drivers/staging/comedi/drivers/ii_pci20kc.c
b/drivers/staging/comedi/drivers/ii_pci20kc.c
index 5c3a318..a2e38fe 100644
--- a/drivers/staging/comedi/drivers/ii_pci20kc.c
+++ b/drivers/staging/comedi/drivers/ii_pci20kc.c
@@ -378,31 +378,22 @@ static int ii20k_dio_insn_bits(struct comedi_device *dev,
unsigned int *data)
{
struct ii20k_private *devpriv = dev-private;
- unsigned int mask = data[0] s-io_bits;/* outputs only */
- unsigned int bits = data[1];
-
- if (mask) {
- s-state = ~mask;
- s-state |= (bits mask);
-
- if (mask 0x00ff)
- writeb((s-state 0) 0xff,
- devpriv-ioaddr + II20K_DIO0_REG);
- if (mask 0xff00)
- writeb((s-state 8) 0xff,
- devpriv-ioaddr + II20K_DIO1_REG);
- if (mask 0x00ff)
- writeb((s-state 16) 0xff,
- devpriv-ioaddr + II20K_DIO2_REG);
- if (mask 0xff00)
- writeb((s-state 24) 0xff,
- devpriv-ioaddr + II20K_DIO3_REG);
+ void __iomem *ioaddr = devpriv-ioaddr;
+ unsigned int val;
+
+ if (comedi_dio_insn_bits(dev, s, insn, data)) {
+ writeb((s-state 0) 0xff, ioaddr + II20K_DIO0_REG);
+ writeb((s-state 8) 0xff, ioaddr + II20K_DIO1_REG);
+ writeb((s-state 16) 0xff, ioaddr + II20K_DIO2_REG);
+ writeb((s-state 24) 0xff, ioaddr + II20K_DIO3_REG);
}
ii_pci20k is another.
[snip]
diff --git a/drivers/staging/comedi/drivers/me4000.c
b/drivers/staging/comedi/drivers/me4000.c
index 8f4afad..1c385e3 100644
--- a/drivers/staging/comedi/drivers/me4000.c
+++ b/drivers/staging/comedi/drivers/me4000.c
@@ -1313,45 +1313,27 @@ static int me4000_ao_insn_read(struct comedi_device
*dev,
return 1;
}
-/*=
- Digital I/O section
- ===*/
-
static int me4000_dio_insn_bits(struct comedi_device *dev,
struct comedi_subdevice *s,
- struct comedi_insn *insn, unsigned int *data)
+ struct comedi_insn *insn,
+ unsigned int *data)
{
- /*
-* The insn data consists of a mask in data[0] and the new data
-* in data[1]. The mask defines which bits we are concerning about.
-* The new data must be anded with the mask.
-* Each channel corresponds to a bit.
-*/
- if (data[0]) {
- /* Check if requested ports are configured for output */
- if ((s-io_bits data[0]) != data[0])
- return -EIO;
-
- s-state = ~data[0];
- s-state |= data[0] data[1];
-
- /* Write out the new digital output lines */
- outl((s-state 0) 0xFF,
- dev-iobase + ME4000_DIO_PORT_0_REG);
- outl((s-state 8) 0xFF,
- dev-iobase + ME4000_DIO_PORT_1_REG);
- outl((s-state 16) 0xFF,
- dev-iobase +
Re: [PATCH 08/11] staging: comedi: drivers: more users of comedi_dio_insn_bits()
On 2013-08-28 21:30, H Hartley Sweeten wrote:
Convert a couple more comedi drivers to use comedi_dio_insn_bits() to
handle the boilerplate code to update the subdevice s-state.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
.../staging/comedi/drivers/amplc_dio200_common.c | 22 +-
drivers/staging/comedi/drivers/cb_pcidas64.c | 14 +++---
drivers/staging/comedi/drivers/ni_mio_common.c | 20 +---
3 files changed, 21 insertions(+), 35 deletions(-)
diff --git a/drivers/staging/comedi/drivers/amplc_dio200_common.c
b/drivers/staging/comedi/drivers/amplc_dio200_common.c
index 8c6fa1e..0fc0081 100644
--- a/drivers/staging/comedi/drivers/amplc_dio200_common.c
+++ b/drivers/staging/comedi/drivers/amplc_dio200_common.c
@@ -946,26 +946,22 @@ static void dio200_subdev_8255_set_dir(struct
comedi_device *dev,
*/
static int dio200_subdev_8255_bits(struct comedi_device *dev,
struct comedi_subdevice *s,
- struct comedi_insn *insn, unsigned int *data)
+ struct comedi_insn *insn,
+ unsigned int *data)
{
struct dio200_subdev_8255 *subpriv = s-private;
- if (data[0]) {
- s-state = ~data[0];
- s-state |= (data[0] data[1]);
- if (data[0] 0xff)
- dio200_write8(dev, subpriv-ofs, s-state 0xff);
- if (data[0] 0xff00)
- dio200_write8(dev, subpriv-ofs + 1,
- (s-state 8) 0xff);
- if (data[0] 0xff)
- dio200_write8(dev, subpriv-ofs + 2,
- (s-state 16) 0xff);
+ if (comedi_dio_insn_bits(dev, s, insn, data)) {
+ dio200_write8(dev, subpriv-ofs, s-state 0xff);
+ dio200_write8(dev, subpriv-ofs + 1, (s-state 8) 0xff);
+ dio200_write8(dev, subpriv-ofs + 2, (s-state 16) 0xff);
}
+
data[1] = dio200_read8(dev, subpriv-ofs);
data[1] |= dio200_read8(dev, subpriv-ofs + 1) 8;
data[1] |= dio200_read8(dev, subpriv-ofs + 2) 16;
- return 2;
+
+ return insn-n;
}
/*
amplc_dio200_common is another one of those that filtered register
writes according to the mask. As for the 8255 module, this change may
change the state of more output pins than the original code due to the
side effects of I/O direction configuration on the 8255 chip.
(It could be argued that following an INSN_CONFIG_DIO_INPUT or
INSN_CONFIG_DIO_OUTPUT, drivers operating on 8255 chips should either
reset s-state to 0 to reflect the physical side-effects of the chip, or
rewrite s-state to the outputs. This is outside the scope of this
patch though.)
[snip]
--
-=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 06/15] staging: comedi: comedi_parport: tidy up parport_insn_c()
On 2013-08-28 21:57, H Hartley Sweeten wrote:
Rename this function to better describe it's use.
Use comedi_dio_insn_bits() to handle the boilerplate code to update
the subdevice s-state.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/staging/comedi/drivers/comedi_parport.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/comedi/drivers/comedi_parport.c
b/drivers/staging/comedi/drivers/comedi_parport.c
index 04feec7..7180b0c 100644
--- a/drivers/staging/comedi/drivers/comedi_parport.c
+++ b/drivers/staging/comedi/drivers/comedi_parport.c
@@ -140,20 +140,20 @@ static int parport_status_reg_insn_bits(struct
comedi_device *dev,
return insn-n;
}
-static int parport_insn_c(struct comedi_device *dev, struct comedi_subdevice
*s,
- struct comedi_insn *insn, unsigned int *data)
+static int parport_ctrl_reg_insn_bits(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ struct comedi_insn *insn,
+ unsigned int *data)
{
struct parport_private *devpriv = dev-private;
- data[0] = 0x0f;
- if (data[0]) {
- devpriv-c_data = ~data[0];
- devpriv-c_data |= (data[0] data[1]);
-
+ if (comedi_dio_insn_bits(dev, s, insn, data)) {
+ devpriv-c_data = ~((1 s-n_chan) - 1);
+ devpriv-c_data |= s-state;
outb(devpriv-c_data, dev-iobase + PARPORT_CTRL_REG);
}
- data[1] = devpriv-c_data 0xf;
+ data[1] = s-state;
return insn-n;
}
@@ -304,7 +304,7 @@ static int parport_attach(struct comedi_device *dev,
s-n_chan = 4;
s-maxdata = 1;
s-range_table = range_digital;
- s-insn_bits = parport_insn_c;
+ s-insn_bits = parport_ctrl_reg_insn_bits;
s = dev-subdevices[3];
if (irq) {
I'll just note that this patch depends on PATCH 06/11 of your
comedi_dio_insn_bits patch series.
--
-=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/15] staging: comedi: comedi_parport: cleanup driver
On 2013-08-28 21:55, H Hartley Sweeten wrote: Use the new comedi_dio_insn_bits() and comedi_dio_insn_config() helpers to tidy up the digital subdevices in this driver. Remove the need for the kzalloc'd private data. H Hartley Sweeten (15): staging: comedi: comedi_parport: tidy up the register map staging: comedi: comedi_parport: remove 'a_data' from private data staging: comedi: comedi_parport: tidy up parport_insn_a() staging: comedi: comedi_parport: fix parport_insn_config_a() staging: comedi: comedi_parport: tidy up parport_insn_b() staging: comedi: comedi_parport: tidy up parport_insn_c() staging: comedi: comedi_parport: remove 'c_data' from private data staging: comedi: comedi_parport: remove 'enable_irq' from private data staging: comedi: comedi_parport: don't fail attach if irq is not available staging: comedi: comedi_parport: tidy up parport_attach() staging: comedi: comedi_parport: tidy up multi-line comments staging: comedi: comedi_parport: reorder #include's staging: comedi: comedi_parport: rename parport_intr_insn() staging: comedi: comedi_parport: use dev-read_subdev in interrupt handler staging: comedi: comedi_parport: change MODULE_DESCRIPTION drivers/staging/comedi/drivers/comedi_parport.c | 380 +++- 1 file changed, 177 insertions(+), 203 deletions(-) Reviewed-by: Ian Abbott abbo...@mev.co.uk Looks okay, but it depends on the staging: comedi: tidy up digital output (*insn_bits) patch series that I had a few issues with. -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: ni_labpc: use comedi_range_is_unipolar()
On 2013-08-28 23:01, H Hartley Sweeten wrote:
Use the core provided helper function instead of duplicating it as
a private function.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/staging/comedi/drivers/ni_labpc.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_labpc.c
b/drivers/staging/comedi/drivers/ni_labpc.c
index 1add114..27daccb 100644
--- a/drivers/staging/comedi/drivers/ni_labpc.c
+++ b/drivers/staging/comedi/drivers/ni_labpc.c
@@ -201,12 +201,6 @@ static int labpc_counter_set_mode(struct comedi_device
*dev,
return i8254_set_mode(base_address, 0, counter_number, mode);
}
-static bool labpc_range_is_unipolar(struct comedi_subdevice *s,
- unsigned int range)
-{
- return s-range_table-range[range].min = 0;
-}
-
static int labpc_cancel(struct comedi_device *dev, struct comedi_subdevice *s)
{
struct labpc_private *devpriv = dev-private;
@@ -272,7 +266,7 @@ static void labpc_setup_cmd6_reg(struct comedi_device *dev,
devpriv-cmd6 = ~CMD6_NRSE;
/* bipolar or unipolar range? */
- if (labpc_range_is_unipolar(s, range))
+ if (comedi_range_is_unipolar(s, range))
devpriv-cmd6 |= CMD6_ADCUNI;
else
devpriv-cmd6 = ~CMD6_ADCUNI;
@@ -1046,7 +1040,7 @@ static int labpc_ao_insn_write(struct comedi_device *dev,
/* set range */
if (board-is_labpc1200) {
range = CR_RANGE(insn-chanspec);
- if (labpc_range_is_unipolar(s, range))
+ if (comedi_range_is_unipolar(s, range))
devpriv-cmd6 |= CMD6_DACUNI(channel);
else
devpriv-cmd6 = ~CMD6_DACUNI(channel);
Reviewed-by: Ian Abbott abbo...@mev.co.uk
--
-=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: comedi: pcmad: use comedi_range_is_bipolar()
On 2013-08-28 22:59, H Hartley Sweeten wrote:
Use the core provided helper function instead of duplicating it as
a private function.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/staging/comedi/drivers/pcmad.c | 8 +---
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/drivers/staging/comedi/drivers/pcmad.c
b/drivers/staging/comedi/drivers/pcmad.c
index 423f236..fe482fd 100644
--- a/drivers/staging/comedi/drivers/pcmad.c
+++ b/drivers/staging/comedi/drivers/pcmad.c
@@ -75,12 +75,6 @@ static int pcmad_ai_wait_for_eoc(struct comedi_device *dev,
return -ETIME;
}
-static bool pcmad_range_is_bipolar(struct comedi_subdevice *s,
- unsigned int range)
-{
- return s-range_table-range[range].min 0;
-}
-
static int pcmad_ai_insn_read(struct comedi_device *dev,
struct comedi_subdevice *s,
struct comedi_insn *insn,
@@ -106,7 +100,7 @@ static int pcmad_ai_insn_read(struct comedi_device *dev,
if (s-maxdata == 0x0fff)
val = 4;
- if (pcmad_range_is_bipolar(s, range)) {
+ if (comedi_range_is_bipolar(s, range)) {
/* munge the two's complement value */
val ^= ((s-maxdata + 1) 1);
}
Reviewed-by: Ian Abbott abbo...@mev.co.uk
--
-=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: crystalhd: Resolve sparse 'different base types' warnings.
The result from crystalhd_get_sgle_paddr and crystalhd_get_sgle_len are later used in calculations, so the result should be in CPU byte ordering. Signed-off-by: Shaun Laing sh...@xresource.ca --- Assumes sg_dma_address and sg_dma_len return values in CPU byte ordering. drivers/staging/crystalhd/crystalhd_misc.h |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/staging/crystalhd/crystalhd_misc.h b/drivers/staging/crystalhd/crystalhd_misc.h index 4dae3a7..aa736c8 100644 --- a/drivers/staging/crystalhd/crystalhd_misc.h +++ b/drivers/staging/crystalhd/crystalhd_misc.h @@ -177,8 +177,8 @@ extern enum BC_STATUS crystalhd_map_dio(struct crystalhd_adp *, void *, extern enum BC_STATUS crystalhd_unmap_dio(struct crystalhd_adp *, struct crystalhd_dio_req*); -#define crystalhd_get_sgle_paddr(_dio, _ix) (cpu_to_le64(sg_dma_address(_dio-sg[_ix]))) -#define crystalhd_get_sgle_len(_dio, _ix) (cpu_to_le32(sg_dma_len(_dio-sg[_ix]))) +#define crystalhd_get_sgle_paddr(_dio, _ix) (sg_dma_address(_dio-sg[_ix])) +#define crystalhd_get_sgle_len(_dio, _ix) (sg_dma_len(_dio-sg[_ix])) /* General Purpose Queues ==*/ extern enum BC_STATUS crystalhd_create_dioq(struct crystalhd_adp *, -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 01/11] staging: comedi: core: introduce comedi_dio_insn_bits()
On Thursday, August 29, 2013 4:14 AM, Ian Abbott wrote:
On 2013-08-28 21:26, H Hartley Sweeten wrote:
The (*insn_bits) functions for DIO and DO subdevices typically use
the subdevice 's-state' to hold the current state of the output
channels. The 'insn' passed to these functions, INSN_BITS, specifies
two parameters passed in the 'data'.
data[0] = 'mask', the channels to update
data[1] = 'bits', the new state for the channels
Introduce a helper function to handle this boilerplate.
The function returns:
0 if there are no changes in the state
1 if the driver needs to update the hardware to a new state
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/staging/comedi/comedidev.h | 2 ++
drivers/staging/comedi/drivers.c | 25 +
2 files changed, 27 insertions(+)
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index 2e19f65..de36499 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -345,6 +345,8 @@ void comedi_buf_memcpy_from(struct comedi_async *async,
unsigned int offset,
int comedi_dio_insn_config(struct comedi_device *, struct comedi_subdevice
*,
struct comedi_insn *, unsigned int *data,
unsigned int mask);
+int comedi_dio_insn_bits(struct comedi_device *, struct comedi_subdevice *,
+ struct comedi_insn *, unsigned int *data);
void *comedi_alloc_devpriv(struct comedi_device *, size_t);
int comedi_alloc_subdevices(struct comedi_device *, int);
diff --git a/drivers/staging/comedi/drivers.c
b/drivers/staging/comedi/drivers.c
index 317a821..2bbd9d0 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -190,6 +190,31 @@ int comedi_dio_insn_config(struct comedi_device *dev,
}
EXPORT_SYMBOL_GPL(comedi_dio_insn_config);
+/**
+ * comedi_dio_insn_bits() - boilerplate (*insn_bits) for DIO and DO
subdevices.
+ * @dev: comedi_device struct
+ * @s: comedi_subdevice struct
+ * @insn: comedi_insn struct
+ * @data: parameters for the @insn
+ */
+int comedi_dio_insn_bits(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ struct comedi_insn *insn,
+ unsigned int *data)
+{
+unsigned int mask = data[0];
+unsigned int bits = data[1];
+
+if (mask) {
+s-state = ~mask;
+s-state |= (bits mask);
+
+return 1;
+}
+return 0;
+}
+EXPORT_SYMBOL_GPL(comedi_dio_insn_bits);
+
I'm not convinced this achieves much. It doesn't use the insn parameter
at all.
Well it does remove 302 lines of boilerplate code.
The parameters for the function could be reduced to just.
int comedi_dio_insn_bits(struct comedi_subdevice *s,
unsigned int mask,
unsigned int bits)
Then the callers could do:
If (comedi_dio_insn_bits(s, data[0], data[1]) {
/* Update the hardware */
}
But, I think just passing all the (*insn_bits) parameters is a bit cleaner.
To address your thought in PATCH 04/11, the return could be either the
raw 'mask' (data[0]) or the filtered mask (data[0] s-io_bits).
Regards,
Hartley
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 01/11] staging: comedi: core: introduce comedi_dio_insn_bits()
On 2013-08-29 17:20, Hartley Sweeten wrote:
On Thursday, August 29, 2013 4:14 AM, Ian Abbott wrote:
On 2013-08-28 21:26, H Hartley Sweeten wrote:
The (*insn_bits) functions for DIO and DO subdevices typically use
the subdevice 's-state' to hold the current state of the output
channels. The 'insn' passed to these functions, INSN_BITS, specifies
two parameters passed in the 'data'.
data[0] = 'mask', the channels to update
data[1] = 'bits', the new state for the channels
Introduce a helper function to handle this boilerplate.
The function returns:
0 if there are no changes in the state
1 if the driver needs to update the hardware to a new state
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/staging/comedi/comedidev.h | 2 ++
drivers/staging/comedi/drivers.c | 25 +
2 files changed, 27 insertions(+)
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index 2e19f65..de36499 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -345,6 +345,8 @@ void comedi_buf_memcpy_from(struct comedi_async *async,
unsigned int offset,
int comedi_dio_insn_config(struct comedi_device *, struct comedi_subdevice *,
struct comedi_insn *, unsigned int *data,
unsigned int mask);
+int comedi_dio_insn_bits(struct comedi_device *, struct comedi_subdevice *,
+struct comedi_insn *, unsigned int *data);
void *comedi_alloc_devpriv(struct comedi_device *, size_t);
int comedi_alloc_subdevices(struct comedi_device *, int);
diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
index 317a821..2bbd9d0 100644
--- a/drivers/staging/comedi/drivers.c
+++ b/drivers/staging/comedi/drivers.c
@@ -190,6 +190,31 @@ int comedi_dio_insn_config(struct comedi_device *dev,
}
EXPORT_SYMBOL_GPL(comedi_dio_insn_config);
+/**
+ * comedi_dio_insn_bits() - boilerplate (*insn_bits) for DIO and DO subdevices.
+ * @dev: comedi_device struct
+ * @s: comedi_subdevice struct
+ * @insn: comedi_insn struct
+ * @data: parameters for the @insn
+ */
+int comedi_dio_insn_bits(struct comedi_device *dev,
+struct comedi_subdevice *s,
+struct comedi_insn *insn,
+unsigned int *data)
+{
+ unsigned int mask = data[0];
+ unsigned int bits = data[1];
+
+ if (mask) {
+ s-state = ~mask;
+ s-state |= (bits mask);
+
+ return 1;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(comedi_dio_insn_bits);
+
I'm not convinced this achieves much. It doesn't use the insn parameter
at all.
Well it does remove 302 lines of boilerplate code.
The parameters for the function could be reduced to just.
int comedi_dio_insn_bits(struct comedi_subdevice *s,
unsigned int mask,
unsigned int bits)
Then the callers could do:
If (comedi_dio_insn_bits(s, data[0], data[1]) {
/* Update the hardware */
}
But, I think just passing all the (*insn_bits) parameters is a bit cleaner.
It just seemed a waste passing a parameter that is never used. You
could still pass the data[] array as a pointer with the assumption that
it's 2 elements long. (Or you could declare it as `unsigned int
data[2]` in the prototype. I'm a little wary of using array syntax in
function prototypes but I suppose it allows some bounds checking during
static analysis.)
To address your thought in PATCH 04/11, the return could be either the
raw 'mask' (data[0]) or the filtered mask (data[0] s-io_bits).
I suppose the filtered mask makes more sense, as that is what has been
used to mask the s-state changes.
--
-=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 04/11] staging: comedi: drivers: use comedi_dio_insn_bits()
On Thursday, August 29, 2013 5:32 AM, Ian Abbott wrote:
On 2013-08-28 21:28, H Hartley Sweeten wrote:
Use comedi_dio_insn_bits() to handle the boilerplate code to update
the subdevice s-state for DIO and DO subdevices.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
[snip]
diff --git a/drivers/staging/comedi/drivers/8255.c
b/drivers/staging/comedi/drivers/8255.c
index 2f070fd..d29f404 100644
--- a/drivers/staging/comedi/drivers/8255.c
+++ b/drivers/staging/comedi/drivers/8255.c
@@ -126,30 +126,17 @@ EXPORT_SYMBOL_GPL(subdev_8255_interrupt);
static int subdev_8255_insn(struct comedi_device *dev,
struct comedi_subdevice *s,
- struct comedi_insn *insn, unsigned int *data)
+ struct comedi_insn *insn,
+ unsigned int *data)
{
struct subdev_8255_private *spriv = s-private;
unsigned long iobase = spriv-iobase;
- unsigned int mask;
- unsigned int bits;
unsigned int v;
- mask = data[0];
- bits = data[1];
-
- if (mask) {
- v = s-state;
- v = ~mask;
- v |= (bits mask);
-
- if (mask 0xff)
- spriv-io(1, _8255_DATA, v 0xff, iobase);
- if (mask 0xff00)
- spriv-io(1, _8255_DATA + 1, (v 8) 0xff,
iobase);
- if (mask 0xff)
- spriv-io(1, _8255_DATA + 2, (v 16) 0xff,
iobase);
-
- s-state = v;
+ if (comedi_dio_insn_bits(dev, s, insn, data)) {
+ spriv-io(1, _8255_DATA, s-state 0xff, iobase);
+ spriv-io(1, _8255_DATA + 1, (s-state 8) 0xff, iobase);
+ spriv-io(1, _8255_DATA + 2, (s-state 16) 0xff,
iobase);
}
There's extra register access overhead here that wasn't in the original.
Quite often, the mask only has one bit set (when insn_bits is being
used to emulate insn_write, for example), which would only write one
register. For PC I/O space, register writes typically take a
microsecond. Or at least they used to, but I think it might be less on
newer hardware.
I'm not sure if the I/O access adds any significant more time than the
test to see if the I/O is needed.
For simplicity, and consistency, I removed all these checks of the mask
in this patch. If you think they are needed I will refresh the patch and
leave them all in.
For 8255, this change may also result in the output pin values being
different than they were before due to a quirk of the 8255. Following
an INSN_CONFIG_DIO_INPUT or INSN_CONFIG_DIO_OUTPUT, all output pins are
set to 0 by the chip, although comedi's s-state value is not affected.
Then a following INSN_BITS with a mask will set _all_ the output pins
to the new s-state, rather than setting _some of_ the output pins to
the new s-state. Maybe that's a good thing, although it's a change to
the existing behaviour.
That sounds like a bug in the driver. Or, if this is expected, maybe the core
should automatically clear the effected states when a channel is changed
from input to output.
Probably best to play it safe and write to the output registers
selectively according to the mask for the 8255 module.
Just a thought - perhaps comedi_dio_insn_bits() could return the mask
value to allow the caller to selectively write to registers based on the
mask if it wants to?
I'll wait for your reply on this and PATCH 01/11 before I refresh the series.
[snip]
diff --git a/drivers/staging/comedi/drivers/dt2817.c
b/drivers/staging/comedi/drivers/dt2817.c
index f4a8529..5de1330 100644
--- a/drivers/staging/comedi/drivers/dt2817.c
+++ b/drivers/staging/comedi/drivers/dt2817.c
@@ -80,36 +80,24 @@ static int dt2817_dio_insn_config(struct comedi_device
*dev,
static int dt2817_dio_insn_bits(struct comedi_device *dev,
struct comedi_subdevice *s,
- struct comedi_insn *insn, unsigned int *data)
+ struct comedi_insn *insn,
+ unsigned int *data)
{
- unsigned int changed;
-
- /* It's questionable whether it is more important in
-* a driver like this to be deterministic or fast.
-* We choose fast. */
-
- if (data[0]) {
- changed = s-state;
- s-state = ~data[0];
- s-state |= (data[0] data[1]);
- changed ^= s-state;
- changed = s-io_bits;
- if (changed 0x00ff)
- outb(s-state 0xff, dev-iobase + DT2817_DATA + 0);
- if (changed 0xff00)
- outb((s-state 8) 0xff,
-dev-iobase +
RE: [PATCH 07/11] staging: comedi: drivers: use comedi_dio_insn_bits() for DIO subdevices
On Thursday, August 29, 2013 5:52 AM, Ian Abbott wrote: On 2013-08-28 21:29, H Hartley Sweeten wrote: Use comedi_dio_insn_bits() to handle the boilerplate code to update the subdevice s-state for DIO subdevices. Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com Cc: Ian Abbott abbo...@mev.co.uk Cc: Greg Kroah-Hartman gre...@linuxfoundation.org [snip] diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c b/drivers/staging/comedi/drivers/addi_apci_3xxx.c index 40e8d56..f3b8514 100644 [snip] Since I was noting drivers that filtered register writes depending on the mask in the earlier patches, addi_apci_3xxx is one of those. Noted. [snip] diff --git a/drivers/staging/comedi/drivers/ii_pci20kc.c b/drivers/staging/comedi/drivers/ii_pci20kc.c index 5c3a318..a2e38fe 100644 [snip] ii_pci20k is another. [snip] diff --git a/drivers/staging/comedi/drivers/me4000.c b/drivers/staging/comedi/drivers/me4000.c index 8f4afad..1c385e3 100644 [snip] me4000 is another. The error returned when attempting to write to lines configured as inputs was a little unusual! I though the error was a bit odd also. I don't think the (*insn_bits) functions should fail for this situation. If the user attempts to change the output state of a channel configured as an input it should just be ignored. [snip] diff --git a/drivers/staging/comedi/drivers/me_daq.c b/drivers/staging/comedi/drivers/me_daq.c index 00ebf4d..d29416b 100644 [snip[ me_daq is another. Ok. So far these have been identified as doing conditional updates of the state based on the mask: 8255 dt2817 ni_6527 pcl711 pcl726 addi_apci_3xxx ii_pci20kc me4000 me_daq It could a argued that some of these only have the conditional update because the driver was based on a previous driver that also had it. There is a lot of cut-and-paste in the comedi drivers. Regards, Hartley ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 01/11] staging: comedi: core: introduce comedi_dio_insn_bits()
On Thursday, August 29, 2013 9:37 AM, Ian Abbott wrote:
On 2013-08-29 17:20, Hartley Sweeten wrote:
On Thursday, August 29, 2013 4:14 AM, Ian Abbott wrote:
On 2013-08-28 21:26, H Hartley Sweeten wrote:
The (*insn_bits) functions for DIO and DO subdevices typically use
the subdevice 's-state' to hold the current state of the output
channels. The 'insn' passed to these functions, INSN_BITS, specifies
two parameters passed in the 'data'.
data[0] = 'mask', the channels to update
data[1] = 'bits', the new state for the channels
Introduce a helper function to handle this boilerplate.
The function returns:
0 if there are no changes in the state
1 if the driver needs to update the hardware to a new state
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
[snip]
+/**
+ * comedi_dio_insn_bits() - boilerplate (*insn_bits) for DIO and DO
subdevices.
+ * @dev: comedi_device struct
+ * @s: comedi_subdevice struct
+ * @insn: comedi_insn struct
+ * @data: parameters for the @insn
+ */
+int comedi_dio_insn_bits(struct comedi_device *dev,
+ struct comedi_subdevice *s,
+ struct comedi_insn *insn,
+ unsigned int *data)
+{
+ unsigned int mask = data[0];
+ unsigned int bits = data[1];
+
+ if (mask) {
+ s-state = ~mask;
+ s-state |= (bits mask);
+
+ return 1;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(comedi_dio_insn_bits);
+
I'm not convinced this achieves much. It doesn't use the insn parameter
at all.
Well it does remove 302 lines of boilerplate code.
The parameters for the function could be reduced to just.
int comedi_dio_insn_bits(struct comedi_subdevice *s,
unsigned int mask,
unsigned int bits)
Then the callers could do:
If (comedi_dio_insn_bits(s, data[0], data[1]) {
/* Update the hardware */
}
But, I think just passing all the (*insn_bits) parameters is a bit cleaner.
It just seemed a waste passing a parameter that is never used. You
could still pass the data[] array as a pointer with the assumption that
it's 2 elements long. (Or you could declare it as `unsigned int
data[2]` in the prototype. I'm a little wary of using array syntax in
function prototypes but I suppose it allows some bounds checking during
static analysis.)
I could also add a sanity check of (insn-n) but it adds that overhead to every
call.
Actually, the only place in the core that calls the (*insn_bits) is in
comedi_fops.c
parse_insn(). In that function the sanity check of insn-n is already done.
To reduce the number of parameters I'll modify the patch to the reduced
form above.
To address your thought in PATCH 04/11, the return could be either the
raw 'mask' (data[0]) or the filtered mask (data[0] s-io_bits).
I suppose the filtered mask makes more sense, as that is what has been
used to mask the s-state changes.
I'll also return the filtered mask and leave all the conditional updates in
the drivers.
Hopefully I will be able to get the updated series posted later today.
Thanks,
Hartley
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH -next] staging: dgap: Add missing #include linux/slab.h
drivers/staging/dgap/dgap_driver.c: In function ‘dgap_cleanup_module’: drivers/staging/dgap/dgap_driver.c:423: error: implicit declaration of function ‘kfree’ drivers/staging/dgap/dgap_driver.c: In function ‘dgap_driver_kzmalloc’: drivers/staging/dgap/dgap_driver.c:940: error: implicit declaration of function ‘kmalloc’ drivers/staging/dgap/dgap_driver.c:940: warning: initialization makes pointer from integer without a cast Signed-off-by: Geert Uytterhoeven ge...@linux-m68k.org --- http://kisskb.ellerman.id.au/kisskb/buildresult/9422860/ http://kisskb.ellerman.id.au/kisskb/buildresult/9419277/ drivers/staging/dgap/dgap_driver.c |1 + 1 file changed, 1 insertion(+) diff --git a/drivers/staging/dgap/dgap_driver.c b/drivers/staging/dgap/dgap_driver.c index 9f777e4..724a685 100644 --- a/drivers/staging/dgap/dgap_driver.c +++ b/drivers/staging/dgap/dgap_driver.c @@ -36,6 +36,7 @@ #include linux/module.h #include linux/pci.h #include linux/delay.h /* For udelay */ +#include linux/slab.h #include asm/uaccess.h /* For copy_from_user/copy_to_user */ #if LINUX_VERSION_CODE = KERNEL_VERSION(2,6,39) -- 1.7.9.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch] staging: r8188eu: off by one bugs
These should be instead of =. Also we can use the ARRAY_SIZE()
macro.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/staging/rtl8188eu/core/rtw_cmd.c
b/drivers/staging/rtl8188eu/core/rtw_cmd.c
index 859fc19..9632ef4 100644
--- a/drivers/staging/rtl8188eu/core/rtw_cmd.c
+++ b/drivers/staging/rtl8188eu/core/rtw_cmd.c
@@ -367,7 +367,7 @@ _next:
memcpy(pcmdbuf, pcmd-parmbuf, pcmd-cmdsz);
- if (pcmd-cmdcode = (sizeof(wlancmds) / sizeof(struct
cmd_hdl))) {
+ if (pcmd-cmdcode ARRAY_SIZE(wlancmds)) {
cmd_hdl = wlancmds[pcmd-cmdcode].h2cfuns;
if (cmd_hdl) {
@@ -385,7 +385,7 @@ _next:
post_process:
/* call callback function for post-processed */
- if (pcmd-cmdcode = (sizeof(rtw_cmd_callback) / sizeof(struct
_cmd_callback))) {
+ if (pcmd-cmdcode ARRAY_SIZE(rtw_cmd_callback)) {
pcmd_callback =
rtw_cmd_callback[pcmd-cmdcode].callback;
if (pcmd_callback == NULL) {
RT_TRACE(_module_rtl871x_cmd_c_, _drv_info_,
(mlme_cmd_hdl(): pcmd_callback = 0x%p, cmdcode = 0x%x\n, pcmd_callback,
pcmd-cmdcode));
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch 2/2] staging: rtl8188eu: || vs typo
Obviously it's impossible for -KeyLength to be both 5 and 13. I assume
that was intended here.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
index 5fab477..193f641 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
@@ -743,7 +743,7 @@ _func_enter_;
/* Check key length for WEP. For NDTEST, 2005.01.27, by
rcnjko. */
if ((encryptionalgo == _WEP40_ || encryptionalgo == _WEP104_)
- (key-KeyLength != 5 || key-KeyLength != 13)) {
+ (key-KeyLength != 5 key-KeyLength != 13)) {
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_err_, (WEP
KeyLength:0x%x != 5 or 13\n, key-KeyLength));
ret = _FAIL;
goto exit;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch] staging: r8188eu: copying one byte too much
There is a copy and paste bug here so we copy 4 bytes instead of 3.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index a43fc88..013ea48 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -1592,7 +1592,7 @@ void update_bmc_sta_support_rate(struct adapter
*padapter, u32 mac_id)
/* Only B, B/G, and B/G/N AP could use CCK rate */
memcpy((pmlmeinfo-FW_sta_info[mac_id].SupportedRates),
rtw_basic_rate_cck, 4);
} else {
- memcpy((pmlmeinfo-FW_sta_info[mac_id].SupportedRates),
rtw_basic_rate_ofdm, 4);
+ memcpy((pmlmeinfo-FW_sta_info[mac_id].SupportedRates),
rtw_basic_rate_ofdm, 3);
}
}
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
re: staging: r8188eu: Add files for new driver - part 4
Hello Larry Finger,
The patch 7b464c9fa5cc: staging: r8188eu: Add files for new driver -
part 4 from Aug 21, 2013, leads to the following Smatch warning:
drivers/staging/rtl8188eu/core/rtw_mlme_ext.c:8328 mlme_evt_hdl()
error: buffer overflow 'wlanevents' 24 = 24
8321 /* checking if event code is valid */
8322 if (evt_code = MAX_C2HEVT) {
^^
This limit is slightly larger than the number of elements in the
wlanevents[] array.
8323 RT_TRACE(_module_rtl871x_cmd_c_, _drv_err_, (\nEvent
Code(%d) mismatch!\n, evt_code));
8324 goto _abort_event_;
8325 }
8326
8327 /* checking if event size match the event parm size */
8328 if ((wlanevents[evt_code].parmsize != 0)
Off by one.
8329 (wlanevents[evt_code].parmsize != evt_sz)) {
8330 RT_TRACE(_module_rtl871x_cmd_c_, _drv_err_,
8331 (\nEvent(%d) Parm Size mismatch (%d vs
%d)!\n,
8332 evt_code, wlanevents[evt_code].parmsize,
evt_sz));
8333 goto _abort_event_;
8334 }
It's not clear to me what the fix is.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[patch 1/2] staging: rtl8188eu: off by one in rtw_set_802_11_add_wep()
keyid is used as an offset into the -dot11DefKey[] array. The array
has 4 elements.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
diff --git a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
index 9e12774..5fab477 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ioctl_set.c
@@ -566,7 +566,7 @@ _func_enter_;
keyid = wep-KeyIndex 0x3fff;
- if (keyid 4) {
+ if (keyid = 4) {
RT_TRACE(_module_rtl871x_ioctl_set_c_, _drv_err_,
(MgntActrtw_set_802_11_add_wep:keyid4 =fail\n));
ret = false;
goto exit;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
re: staging: r8188eu: Add files for new driver - part 7
Hello Larry Finger,
The patch d6846af679e0: staging: r8188eu: Add files for new driver -
part 7 from Aug 21, 2013, leads to the following Smatch warning:
drivers/staging/rtl8188eu/core/rtw_xmit.c:1570
dequeue_one_xmitframe() info: ignoring unreachable code.
1559 while (!rtw_end_of_queue_search(xmitframe_phead,
xmitframe_plist)) {
1560 pxmitframe = LIST_CONTAINOR(xmitframe_plist, struct
xmit_frame, list);
1561
1562 xmitframe_plist = get_next(xmitframe_plist);
1563
1564 rtw_list_delete(pxmitframe-list);
1565
1566 ptxservq-qcnt--;
1567
1568 break;
^
Is there supposed to be an if statement with this break? This is a loop
that doesn't loop.
1569
1570 pxmitframe = NULL;
^
Is this bit important?
1571 }
1572
1573 return pxmitframe;
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
re: staging: r8188eu: Add files for new driver - part 14
Hello Larry Finger,
The patch 615a4d12e556: staging: r8188eu: Add files for new driver -
part 14 from Aug 21, 2013, leads to the following warning:
drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c:2008
Hal_ReadPowerValueFromPROM_8188E()
error: buffer overflow 'pwrInfo24G-IndexBW40_Base[rfPath]' 5 = 5
drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
2005 /* 2.4G default value */
2006 for (group = 0; group MAX_CHNL_GROUP_24G;
group++) {
2007
pwrInfo24G-IndexCCK_Base[rfPath][group] = EEPROM_DEFAULT_24G_INDEX;
2008
pwrInfo24G-IndexBW40_Base[rfPath][group] = EEPROM_DEFAULT_24G_INDEX;
^^^
-IndexCCK_Base[] has MAX_CHNL_GROUP_24G elements but -IndexBW40_Base[]
has MAX_CHNL_GROUP_24G - 1 elements. I guess this should be two loops.
2009 }
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: dgnc: adds TODO
This patchs adds a TODO for the driver. Signed-off-by: Lidza Louina lidza.lou...@gmail.com --- drivers/staging/dgnc/TODO | 17 + 1 file changed, 17 insertions(+) create mode 100644 drivers/staging/dgnc/TODO diff --git a/drivers/staging/dgnc/TODO b/drivers/staging/dgnc/TODO new file mode 100644 index 000..1ff2d18 --- /dev/null +++ b/drivers/staging/dgnc/TODO @@ -0,0 +1,17 @@ +* remove kzalloc casts +* checkpatch fixes +* sparse fixes +* fix use of sizeof(). Example replace sizeof(struct board_t) + with sizeof(*brd) and remove sizeof(char) +* change name of board_t to dgnc_board +* split two assignments into the two assignments on two lines; + don't use two equals signs +* remove unecessary comments +* remove unecessary error messages. Example kzalloc() has its + own error message. Adding an extra one is useless. +* use goto statements for error handling when appropriate +* there is a lot of unecessary code in the driver. It was + originally a standalone driver. Remove uneeded code. + +Please send patches to Greg Kroah-Hartman g...@kroah.com and +Cc: Lidza Louina lidza.lou...@gmail.com -- 1.8.1.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 01/17] staging: comedi: initialize subdevice s-io_bits in postconfig
The subdevice 'io_bits' is a bit mask of the i/o configuration for digital subdevices. '0' values indicate that a channel is configured as an input and '1' values that the channel is an output. Since the subdevice data is kzalloc()'d, all channels default as inputs. Modify __comedi_device_postconfig() so that the 'io_bits' are correctly initialized for Digital Output subdevices. Remove all the unnecessary initializations of 's-io_bits' from the drivers. Also, remove the unnecessary initialization of the 's-state'. Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com Cc: Ian Abbott abbo...@mev.co.uk Cc: Greg Kroah-Hartman gre...@linuxfoundation.org --- drivers/staging/comedi/drivers.c | 3 +++ drivers/staging/comedi/drivers/8255.c | 3 --- drivers/staging/comedi/drivers/addi-data/addi_common.c | 2 -- drivers/staging/comedi/drivers/addi_apci_3120.c| 2 -- drivers/staging/comedi/drivers/adl_pci6208.c | 1 - drivers/staging/comedi/drivers/adl_pci9118.c | 2 -- drivers/staging/comedi/drivers/adv_pci1710.c | 4 drivers/staging/comedi/drivers/amplc_dio200_common.c | 2 -- drivers/staging/comedi/drivers/icp_multi.c | 3 --- drivers/staging/comedi/drivers/me_daq.c| 1 - drivers/staging/comedi/drivers/ni_660x.c | 1 - drivers/staging/comedi/drivers/ni_daq_700.c| 1 - 12 files changed, 3 insertions(+), 22 deletions(-) diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c index 317a821..1c81d65 100644 --- a/drivers/staging/comedi/drivers.c +++ b/drivers/staging/comedi/drivers.c @@ -285,6 +285,9 @@ static int __comedi_device_postconfig(struct comedi_device *dev) if (s-type == COMEDI_SUBD_UNUSED) continue; + if (s-type == COMEDI_SUBD_DO) + s-io_bits = (1 s-n_chan) - 1; + if (s-len_chanlist == 0) s-len_chanlist = 1; diff --git a/drivers/staging/comedi/drivers/8255.c b/drivers/staging/comedi/drivers/8255.c index 2f070fd..5283bb5 100644 --- a/drivers/staging/comedi/drivers/8255.c +++ b/drivers/staging/comedi/drivers/8255.c @@ -288,9 +288,6 @@ int subdev_8255_init(struct comedi_device *dev, struct comedi_subdevice *s, s-insn_bits= subdev_8255_insn; s-insn_config = subdev_8255_insn_config; - s-state= 0; - s-io_bits = 0; - subdev_8255_do_config(dev, s); return 0; diff --git a/drivers/staging/comedi/drivers/addi-data/addi_common.c b/drivers/staging/comedi/drivers/addi-data/addi_common.c index 63dff77..dc87df0 100644 --- a/drivers/staging/comedi/drivers/addi-data/addi_common.c +++ b/drivers/staging/comedi/drivers/addi-data/addi_common.c @@ -204,7 +204,6 @@ static int addi_auto_attach(struct comedi_device *dev, s-len_chanlist = devpriv-s_EeParameters.i_NbrDiChannel; s-range_table = range_digital; - s-io_bits = 0; /* all bits input */ s-insn_config = this_board-di_config; s-insn_read = this_board-di_read; s-insn_write = this_board-di_write; @@ -223,7 +222,6 @@ static int addi_auto_attach(struct comedi_device *dev, s-len_chanlist = devpriv-s_EeParameters.i_NbrDoChannel; s-range_table = range_digital; - s-io_bits = 0xf; /* all bits output */ /* insn_config - for digital output memory */ s-insn_config = this_board-do_config; diff --git a/drivers/staging/comedi/drivers/addi_apci_3120.c b/drivers/staging/comedi/drivers/addi_apci_3120.c index d804957..67d09e8 100644 --- a/drivers/staging/comedi/drivers/addi_apci_3120.c +++ b/drivers/staging/comedi/drivers/addi_apci_3120.c @@ -164,7 +164,6 @@ static int apci3120_auto_attach(struct comedi_device *dev, s-maxdata = 1; s-len_chanlist = this_board-i_NbrDiChannel; s-range_table = range_digital; - s-io_bits = 0; /* all bits input */ s-insn_bits = apci3120_di_insn_bits; /* Allocate and Initialise DO Subdevice Structures */ @@ -176,7 +175,6 @@ static int apci3120_auto_attach(struct comedi_device *dev, s-maxdata = this_board-i_DoMaxdata; s-len_chanlist = this_board-i_NbrDoChannel; s-range_table = range_digital; - s-io_bits = 0xf; /* all bits output */ s-insn_bits = apci3120_do_insn_bits; /* Allocate and Initialise Timer Subdevice Structures */ diff --git a/drivers/staging/comedi/drivers/adl_pci6208.c b/drivers/staging/comedi/drivers/adl_pci6208.c index a67ad57..2897506 100644 --- a/drivers/staging/comedi/drivers/adl_pci6208.c +++ b/drivers/staging/comedi/drivers/adl_pci6208.c @@ -221,7 +221,6 @@ static int pci6208_auto_attach(struct comedi_device *dev, val = inw(dev-iobase + PCI6208_DIO);
[PATCH v2 05/17] staging: comedi: drivers: use comedi_dio_update_state() for simple cases
Use comedi_dio_update_state() to handle the boilerplate code to update
the subdevice s-state for simple cases where the hardware is updated
when any channel is modified.
Also, fix a bug in the amplc_pc263 and amplc_pci263 drivers where the
current state is not returned in data[1].
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
.../comedi/drivers/addi-data/hwdrv_apci1564.c | 7 +
.../comedi/drivers/addi-data/hwdrv_apci3200.c | 7 +
drivers/staging/comedi/drivers/addi_apci_1516.c| 8 +-
drivers/staging/comedi/drivers/addi_apci_2032.c| 8 +-
drivers/staging/comedi/drivers/addi_apci_2200.c| 8 +-
drivers/staging/comedi/drivers/addi_apci_3501.c| 8 +-
drivers/staging/comedi/drivers/addi_apci_3xxx.c| 8 +-
drivers/staging/comedi/drivers/adl_pci6208.c | 9 +-
drivers/staging/comedi/drivers/adl_pci7x3x.c | 13 +
drivers/staging/comedi/drivers/adl_pci9111.c | 9 +-
drivers/staging/comedi/drivers/adl_pci9118.c | 9 +++---
drivers/staging/comedi/drivers/adv_pci1710.c | 12 +++-
drivers/staging/comedi/drivers/adv_pci1723.c | 13 -
drivers/staging/comedi/drivers/adv_pci_dio.c | 33 --
drivers/staging/comedi/drivers/aio_iiro_16.c | 4 +--
drivers/staging/comedi/drivers/amplc_pc263.c | 17 ++-
drivers/staging/comedi/drivers/amplc_pci263.c | 17 ++-
drivers/staging/comedi/drivers/cb_das16_cs.c | 9 ++
drivers/staging/comedi/drivers/cb_pcidas64.c | 25
drivers/staging/comedi/drivers/contec_pci_dio.c| 12 ++--
drivers/staging/comedi/drivers/das16.c | 9 +-
drivers/staging/comedi/drivers/das800.c| 6 +---
drivers/staging/comedi/drivers/dt2801.c| 18 ++--
drivers/staging/comedi/drivers/dt2811.c| 8 +++---
drivers/staging/comedi/drivers/dt282x.c| 10 +++
drivers/staging/comedi/drivers/dt3000.c| 9 +++---
drivers/staging/comedi/drivers/dt9812.c| 9 +-
drivers/staging/comedi/drivers/dyna_pci10xx.c | 20 -
drivers/staging/comedi/drivers/icp_multi.c | 11 ++--
drivers/staging/comedi/drivers/multiq3.c | 8 +++---
drivers/staging/comedi/drivers/ni_670x.c | 11 ++--
drivers/staging/comedi/drivers/ni_at_ao.c | 8 ++
drivers/staging/comedi/drivers/ni_atmio16d.c | 9 +++---
drivers/staging/comedi/drivers/ni_pcidio.c | 9 +++---
drivers/staging/comedi/drivers/pcl812.c| 11 +++-
drivers/staging/comedi/drivers/pcl818.c| 18
drivers/staging/comedi/drivers/quatech_daqp_cs.c | 8 +-
drivers/staging/comedi/drivers/rtd520.c| 8 +-
drivers/staging/comedi/drivers/rti800.c| 8 +-
drivers/staging/comedi/drivers/s526.c | 9 ++
40 files changed, 135 insertions(+), 308 deletions(-)
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
index e3cc429..8466854 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
@@ -260,18 +260,13 @@ static int apci1564_do_insn_bits(struct comedi_device
*dev,
unsigned int *data)
{
struct addi_private *devpriv = dev-private;
- unsigned int mask = data[0];
- unsigned int bits = data[1];
s-state = inl(devpriv-i_IobaseAmcc + APCI1564_DIGITAL_OP +
APCI1564_DIGITAL_OP_RW);
- if (mask) {
- s-state = ~mask;
- s-state |= (bits mask);
+ if (comedi_dio_update_state(s, data))
outl(s-state, devpriv-i_IobaseAmcc + APCI1564_DIGITAL_OP +
APCI1564_DIGITAL_OP_RW);
- }
data[1] = s-state;
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3200.c
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3200.c
index 32dce03..dc73d4d 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3200.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3200.c
@@ -623,16 +623,11 @@ static int apci3200_do_insn_bits(struct comedi_device
*dev,
unsigned int *data)
{
struct addi_private *devpriv = dev-private;
- unsigned int mask = data[0];
- unsigned int bits = data[1];
s-state = inl(devpriv-i_IobaseAddon) 0xf;
- if (mask) {
- s-state = ~mask;
- s-state |= (bits mask);
+ if (comedi_dio_update_state(s, data))
outl(s-state, devpriv-i_IobaseAddon);
- }
data[1] = s-state;
diff --git
[PATCH v2 11/17] staging: comedi: das1800: remove do_bits from private data
Use the subdevice 'state' variable instead of carrying the state of
the output channels in the private data.
Use comedi_dio_update_state() to handle the boilerplate code to update
the subdevice s-state.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/staging/comedi/drivers/das1800.c | 22 ++
1 file changed, 6 insertions(+), 16 deletions(-)
diff --git a/drivers/staging/comedi/drivers/das1800.c
b/drivers/staging/comedi/drivers/das1800.c
index 5b30029..8b9a0a6 100644
--- a/drivers/staging/comedi/drivers/das1800.c
+++ b/drivers/staging/comedi/drivers/das1800.c
@@ -427,7 +427,6 @@ struct das1800_private {
volatile unsigned int count;/* number of data points left to be
taken */
unsigned int divisor1; /* value to load into board's counter 1 for
timed conversions */
unsigned int divisor2; /* value to load into board's counter 2 for
timed conversions */
- int do_bits;/* digital output bits */
int irq_dma_bits; /* bits for control register b */
/* dma bits for control register b, stored so that dma can be
* turned on and off */
@@ -1319,24 +1318,15 @@ static int das1800_di_rbits(struct comedi_device *dev,
return insn-n;
}
-/* writes to digital output channels */
static int das1800_do_wbits(struct comedi_device *dev,
struct comedi_subdevice *s,
- struct comedi_insn *insn, unsigned int *data)
+ struct comedi_insn *insn,
+ unsigned int *data)
{
- struct das1800_private *devpriv = dev-private;
- unsigned int wbits;
-
- /* only set bits that have been masked */
- data[0] = (1 s-n_chan) - 1;
- wbits = devpriv-do_bits;
- wbits = ~data[0];
- wbits |= data[0] data[1];
- devpriv-do_bits = wbits;
-
- outb(devpriv-do_bits, dev-iobase + DAS1800_DIGITAL);
+ if (comedi_dio_update_state(s, data))
+ outb(s-state, dev-iobase + DAS1800_DIGITAL);
- data[1] = devpriv-do_bits;
+ data[1] = s-state;
return insn-n;
}
@@ -1644,7 +1634,7 @@ static int das1800_attach(struct comedi_device *dev,
das1800_cancel(dev, dev-read_subdev);
/* initialize digital out channels */
- outb(devpriv-do_bits, dev-iobase + DAS1800_DIGITAL);
+ outb(0, dev-iobase + DAS1800_DIGITAL);
/* initialize analog out channels */
if (thisboard-ao_ability == 1) {
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 14/17] staging: comedi: ssv_dnp: use comedi_dio_update_state()
Use comedi_dio_update_state() to handle the boilerplate code to update
the subdevice s-state.
Also, fix a bug where the state of the channels is returned in data[0].
The comedi core expects it to be returned in data[1].
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/staging/comedi/drivers/ssv_dnp.c | 48 +---
1 file changed, 20 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ssv_dnp.c
b/drivers/staging/comedi/drivers/ssv_dnp.c
index 11758a5..df22a78 100644
--- a/drivers/staging/comedi/drivers/ssv_dnp.c
+++ b/drivers/staging/comedi/drivers/ssv_dnp.c
@@ -46,51 +46,43 @@ Status: unknown
#define PCMR 0xa3 /* Port C Mode Register */
#define PCDR 0xa7 /* Port C Data Register */
-/* - */
-/* The insn_bits interface allows packed reading/writing of DIO channels.*/
-/* The comedi core can convert between insn_bits and insn_read/write, so you */
-/* are able to use these instructions as well. */
-/* - */
-
static int dnp_dio_insn_bits(struct comedi_device *dev,
struct comedi_subdevice *s,
-struct comedi_insn *insn, unsigned int *data)
+struct comedi_insn *insn,
+unsigned int *data)
{
- /* The insn data is a mask in data[0] and the new data in data[1], */
- /* each channel cooresponding to a bit. */
-
- /* Ports A and B are straight forward: each bit corresponds to an*/
- /* output pin with the same order. Port C is different: bits 0...3 */
- /* correspond to bits 4...7 of the output register (PCDR). */
+ unsigned int mask;
+ unsigned int val;
- if (data[0]) {
+ /*
+* Ports A and B are straight forward: each bit corresponds to an
+* output pin with the same order. Port C is different: bits 0...3
+* correspond to bits 4...7 of the output register (PCDR).
+*/
+ mask = comedi_dio_update_state(s, data);
+ if (mask) {
outb(PADR, CSCIR);
- outb((inb(CSCDR)
- ~(u8) (data[0] 0xFF))
-| (u8) (data[1] 0xFF), CSCDR);
+ outb(s-state 0xff, CSCDR);
outb(PBDR, CSCIR);
- outb((inb(CSCDR)
- ~(u8) ((data[0] 0x00FF00) 8))
-| (u8) ((data[1] 0x00FF00) 8), CSCDR);
+ outb((s-state 8) 0xff, CSCDR);
outb(PCDR, CSCIR);
- outb((inb(CSCDR)
- ~(u8) ((data[0] 0x0F) 12))
-| (u8) ((data[1] 0x0F) 12), CSCDR);
+ val = inb(CSCDR) 0x0f;
+ outb(((s-state 12) 0xf0) | val, CSCDR);
}
- /* on return, data[1] contains the value of the digital input lines. */
outb(PADR, CSCIR);
- data[0] = inb(CSCDR);
+ val = inb(CSCDR);
outb(PBDR, CSCIR);
- data[0] += inb(CSCDR) 8;
+ val |= (inb(CSCDR) 8);
outb(PCDR, CSCIR);
- data[0] += ((inb(CSCDR) 0xF0) 12);
+ val |= ((inb(CSCDR) 0xf0) 12);
- return insn-n;
+ data[1] = val;
+ return insn-n;
}
static int dnp_dio_insn_config(struct comedi_device *dev,
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 15/17] staging: comedi: hwdrv_apci3120: use comedi_dio_update_state()
Use comedi_dio_update_state() to handle the boilerplate code to update
the subdevice s-state.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
.../staging/comedi/drivers/addi-data/hwdrv_apci3120.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c
b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c
index 1449b92..ac6e75d 100644
--- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c
+++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3120.c
@@ -2175,21 +2175,16 @@ static int apci3120_do_insn_bits(struct comedi_device
*dev,
unsigned int *data)
{
struct addi_private *devpriv = dev-private;
- unsigned int mask = data[0];
- unsigned int bits = data[1];
- unsigned int val;
- /* The do channels are bits 7:4 of the do register */
- val = devpriv-b_DigitalOutputRegister 4;
- if (mask) {
- val = ~mask;
- val |= (bits mask);
- devpriv-b_DigitalOutputRegister = val 4;
+ if (comedi_dio_update_state(s, data)) {
+ /* The do channels are bits 7:4 of the do register */
+ devpriv-b_DigitalOutputRegister = s-state 4;
- outb(val 4, devpriv-iobase + APCI3120_DIGITAL_OUTPUT);
+ outb(devpriv-b_DigitalOutputRegister,
+devpriv-iobase + APCI3120_DIGITAL_OUTPUT);
}
- data[1] = val;
+ data[1] = s-state;
return insn-n;
}
--
1.8.3.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 17/17] staging: comedi: drivers: convert remaining drivers to comedi_dio_update_state()
Use comedi_dio_update_state() to handle the boilerplate code to update
the subdevice s-state.
Signed-off-by: H Hartley Sweeten hswee...@visionengravers.com
Cc: Ian Abbott abbo...@mev.co.uk
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
---
drivers/staging/comedi/drivers/addi_apci_16xx.c | 12 +++-
drivers/staging/comedi/drivers/addi_apci_3xxx.c | 9 ++---
drivers/staging/comedi/drivers/ii_pci20kc.c | 7 ++-
drivers/staging/comedi/drivers/me4000.c | 26 +
drivers/staging/comedi/drivers/me_daq.c | 8 ++--
drivers/staging/comedi/drivers/pcmuio.c | 10 --
drivers/staging/comedi/drivers/s626.c | 15 --
7 files changed, 22 insertions(+), 65 deletions(-)
diff --git a/drivers/staging/comedi/drivers/addi_apci_16xx.c
b/drivers/staging/comedi/drivers/addi_apci_16xx.c
index 9652374..3469676 100644
--- a/drivers/staging/comedi/drivers/addi_apci_16xx.c
+++ b/drivers/staging/comedi/drivers/addi_apci_16xx.c
@@ -87,17 +87,11 @@ static int apci16xx_dio_insn_bits(struct comedi_device *dev,
struct comedi_insn *insn,
unsigned int *data)
{
- unsigned int mask = data[0];
- unsigned int bits = data[1];
-
- /* Only update the channels configured as outputs */
- mask = s-io_bits;
- if (mask) {
- s-state = ~mask;
- s-state |= (bits mask);
+ unsigned int mask;
+ mask = comedi_dio_update_state(s, data);
+ if (mask)
outl(s-state, dev-iobase + APCI16XX_OUT_REG(s-index));
- }
data[1] = inl(dev-iobase + APCI16XX_IN_REG(s-index));
diff --git a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
index fcec604..761cbf8 100644
--- a/drivers/staging/comedi/drivers/addi_apci_3xxx.c
+++ b/drivers/staging/comedi/drivers/addi_apci_3xxx.c
@@ -711,16 +711,11 @@ static int apci3xxx_dio_insn_bits(struct comedi_device
*dev,
struct comedi_insn *insn,
unsigned int *data)
{
- unsigned int mask = data[0];
- unsigned int bits = data[1];
+ unsigned int mask;
unsigned int val;
- /* only update output channels */
- mask = s-io_bits;
+ mask = comedi_dio_update_state(s, data);
if (mask) {
- s-state = ~mask;
- s-state |= (bits mask);
-
if (mask 0xff)
outl(s-state 0xff, dev-iobase + 80);
if (mask 0xff)
diff --git a/drivers/staging/comedi/drivers/ii_pci20kc.c
b/drivers/staging/comedi/drivers/ii_pci20kc.c
index 5c3a318..858 100644
--- a/drivers/staging/comedi/drivers/ii_pci20kc.c
+++ b/drivers/staging/comedi/drivers/ii_pci20kc.c
@@ -378,13 +378,10 @@ static int ii20k_dio_insn_bits(struct comedi_device *dev,
unsigned int *data)
{
struct ii20k_private *devpriv = dev-private;
- unsigned int mask = data[0] s-io_bits; /* outputs only */
- unsigned int bits = data[1];
+ unsigned int mask;
+ mask = comedi_dio_update_state(s, data);
if (mask) {
- s-state = ~mask;
- s-state |= (bits mask);
-
if (mask 0x00ff)
writeb((s-state 0) 0xff,
devpriv-ioaddr + II20K_DIO0_REG);
diff --git a/drivers/staging/comedi/drivers/me4000.c
b/drivers/staging/comedi/drivers/me4000.c
index 8f4afad..be6c5d5 100644
--- a/drivers/staging/comedi/drivers/me4000.c
+++ b/drivers/staging/comedi/drivers/me4000.c
@@ -1313,29 +1313,15 @@ static int me4000_ao_insn_read(struct comedi_device
*dev,
return 1;
}
-/*=
- Digital I/O section
- ===*/
-
static int me4000_dio_insn_bits(struct comedi_device *dev,
struct comedi_subdevice *s,
- struct comedi_insn *insn, unsigned int *data)
+ struct comedi_insn *insn,
+ unsigned int *data)
{
- /*
-* The insn data consists of a mask in data[0] and the new data
-* in data[1]. The mask defines which bits we are concerning about.
-* The new data must be anded with the mask.
-* Each channel corresponds to a bit.
-*/
- if (data[0]) {
- /* Check if requested ports are configured for output */
- if ((s-io_bits data[0]) != data[0])
- return -EIO;
-
- s-state = ~data[0];
- s-state |= data[0] data[1];
+ unsigned int mask;
- /* Write out the new digital output lines */
+ mask = comedi_dio_update_state(s, data);
+
