On 09/11/18 23:44, Ben Hutchings wrote:

On Tue, 2018-07-10 at 18:02 +0100, Ian Abbott wrote:

...

From: Dan Carpenter dan.carpenter@oracle.com

[ Upstream commit 1376b0a2160319125c3a2822e8c09bd283cd8141 ]

There is a '>' vs '<' typo so this loop is a no-op.

Thanks, but this driver seems to have lots of other bugs in 3.16, like it only ever accesses data[0] in this loop. I don't think there's much point in applying just this one. But if you think it's worth fixing then I can apply all the necessary fixes.

Ben.

It restores some basic functionality for daqp_ao_insn_write() that was broken. The most common case to be handled is insn->n == 1, as that is what the comedi_data_write() function in the user-space Comedilib library sets.

However, it may be worth applying e024181b02ed ("staging: comedi: quatech_daqp_cs: fix bug in daqp_ao_insn_write()") and (with a small amount of backporting) e031642eccc0 ("staging: comedi: quatech_daqp_cs: use comedi_timeout() in ao (*insn_write)") before this one. I'll append a patch series as replies to this email.

...

Fixes: d35dcc89fc93 ("staging: comedi: quatech_daqp_cs: fix daqp_ao_insn_write()") Signed-off-by: Dan Carpenter dan.carpenter@oracle.com Reviewed-by: Ian Abbott abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org

---

drivers/staging/comedi/drivers/quatech_daqp_cs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/comedi/drivers/quatech_daqp_cs.c b/drivers/staging/comedi/drivers/quatech_daqp_cs.c index b3bbec0a0d23..f89a863ea04c 100644 --- a/drivers/staging/comedi/drivers/quatech_daqp_cs.c +++ b/drivers/staging/comedi/drivers/quatech_daqp_cs.c @@ -649,7 +649,7 @@ static int daqp_ao_insn_write(struct comedi_device *dev, /* Make sure D/A update mode is direct update */ outb(0, dev->iobase + DAQP_AUX);

• for (i = 0; i > insn->n; i++) {

• for (i = 0; i < insn->n; i++) { val = data[0]; val &= 0x0fff; val ^= 0x0800; /* Flip the sign */

From: H Hartley Sweeten hsweeten@visionengravers.com

commit e031642eccc040648b09cfc7d632e2e8d0b6f94f upstream.

The data link between the D/A data port and the D/A converter is a serial link. The serial link requires about 8ms to complete a transfer. Use the comedi_timeout() helper to ensure that there is not a previous transfer still happening before trying to write new data to the channel.

Signed-off-by: H Hartley Sweeten hsweeten@visionengravers.com Reviewed-by: Ian Abbott abbotti@mev.co.uk Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org [ija: Backported to 3.16: No 'readback' member in subdevice.] Signed-off-by: Ian Abbott abbotti@mev.co.uk --- .../staging/comedi/drivers/quatech_daqp_cs.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)

diff --git a/drivers/staging/comedi/drivers/quatech_daqp_cs.c b/drivers/staging/comedi/drivers/quatech_daqp_cs.c index dde91c0c0418..870751db03de 100644 --- a/drivers/staging/comedi/drivers/quatech_daqp_cs.c +++ b/drivers/staging/comedi/drivers/quatech_daqp_cs.c @@ -633,6 +633,19 @@ static int daqp_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s) return 0; }

+static int daqp_ao_empty(struct comedi_device *dev, + struct comedi_subdevice *s, + struct comedi_insn *insn, + unsigned long context) +{ + unsigned int status; + + status = inb(dev->iobase + DAQP_AUX); + if ((status & DAQP_AUX_DA_BUFFER) == 0) + return 0; + return -EBUSY; +} + static int daqp_ao_insn_write(struct comedi_device *dev, struct comedi_subdevice *s, struct comedi_insn *insn, @@ -650,6 +663,12 @@ static int daqp_ao_insn_write(struct comedi_device *dev,

for (i = 0; i > insn->n; i++) { unsigned val = data[i]; + int ret; + + /* D/A transfer rate is about 8ms */ + ret = comedi_timeout(dev, s, insn, daqp_ao_empty, 0); + if (ret) + return ret;

val &= 0x0fff; val ^= 0x0800; /* Flip the sign */