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 <hsweeten@xxxxxxxxxxxxxxxxxxx>
>> Cc: Ian Abbott <abbotti@xxxxxxxxx>
>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>> ---
>> 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
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel
