On Mon, Mar 25, 2013 at 11:35:12AM -0500, H Hartley Sweeten wrote: > On Monday, March 25, 2013 3:50 AM, Ian Abbott wrote: > > On 2013-03-22 16:58, H Hartley Sweeten wrote: > >> The comedi core expects the (*insn_write) operations to write insn->n > >> values and return the number of values actually wrote. > >> > >> Make this function work like the core expects. > >> > >> Also, remove the noise about invalid channels. > >> > >> Signed-off-by: H Hartley Sweeten <[email protected]> > >> Cc: Ian Abbott <[email protected]> > >> Cc: Greg Kroah-Hartman <[email protected]> > >> --- > >> drivers/staging/comedi/drivers/ni_labpc.c | 20 ++++++++++---------- > >> 1 file changed, 10 insertions(+), 10 deletions(-) > >> > >> diff --git a/drivers/staging/comedi/drivers/ni_labpc.c > >> b/drivers/staging/comedi/drivers/ni_labpc.c > >> index 0afcede..6461a46 100644 > >> --- a/drivers/staging/comedi/drivers/ni_labpc.c > >> +++ b/drivers/staging/comedi/drivers/ni_labpc.c > >> @@ -1568,21 +1568,21 @@ static int labpc_eeprom_insn_write(struct > >> comedi_device *dev, > >> struct comedi_insn *insn, > >> unsigned int *data) > >> { > >> - int channel = CR_CHAN(insn->chanspec); > >> + unsigned int chan = CR_CHAN(insn->chanspec); > >> int ret; > >> + int i; > >> > >> - /* only allow writes to user area of eeprom */ > >> - if (channel < 16 || channel > 127) { > >> - dev_dbg(dev->class_dev, > >> - "eeprom writes are only allowed to channels 16 through > >> 127 (the pointer and user areas)\n"); > >> + /* only allow writes to user area of eeprom */ > >> + if (chan < 16 || chan > 127) > >> return -EINVAL; > >> - } > >> > >> - ret = labpc_eeprom_write(dev, channel, data[0]); > >> - if (ret < 0) > >> - return ret; > >> + for (i = 0; i < insn->n; i++) { > >> + ret = labpc_eeprom_write(dev, chan, data[i]); > >> + if (ret) > >> + return ret; > >> + } > >> > >> - return 1; > >> + return insn->n; > >> } > > > > This might not actually work if insn->n > 0 as the second call to > > labpc_eeprom_write() might time out (it starts off reading a status > > register up to 10000 times waiting for a previous call to > > labpc_eeprom_write() to complete, but the only delay in the loop is the > > register access itself). > > > > It's probably safer to just write the last data value as follows, as the > > preceding data would be overwritten anyway: > > > > if (insn->n > 0) { > > ret = labpc_eeprom_write(dev, chan, data[insn->n - 1]); > > if (ret) > > return ret; > > } > > > > return insn->n; > > > > Good point. I'll fix this in v2.
This was v2 already, right? I'll apply the patches up to this one for now. thanks, greg k-h _______________________________________________ devel mailing list [email protected] http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
