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. Regards, Hartley _______________________________________________ devel mailing list [email protected] http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
