On 24/04/14 18:35, Hartley Sweeten wrote:
On Thursday, April 24, 2014 4:06 AM, Ian Abbott wrote:
On 2014-04-24 00:07, H Hartley Sweeten wrote:
The chanlist is checked in Step 5 of the (*do_cmdtest) there is no
reason to check it again in the (*do_cmd). The only reasonm its done
is to get the actual 'seglen', the non-repeating length of the chanlist.
Save the 'seglen' found by pci171x_ai_check_chanlist() in the private
data and use that in the (*do_cmd).
Since do_cmdtest can be done while a command is running, one needs to be
careful to make sure that any private data modified by do_cmdtest is not
used by the running command, and only used by do_cmd when setting up the
next command.
Ian,
I think the "live" command used for a running command and the "test"
command are already handled properly by the core.
The COMEDI_CMDTEST ioctl is handled by do_cmdtest_ioctl(). This function
copies the users command to a local variable that is then passed to the
subdevice (*do_cmdtest) function, this is the "test" command. Any
modifications that function does to the command are then passed back
to the user. Since the comedi_cmd is a local variable it does not affect the
running command that is stored in the async->cmd.
The COMEDI_CMD ioctl is handled by do_cmd_ioctl(). This function also
copies the users command to a local variable. It then checks the subdevice
lock and busy members to make sure that the command can actually be
performed. If so it then sets the async->cmd to the comedi_command
copied from the user, this is now the "live" command. The async->cmd
is then passed to the subdevice (*do_cmdtest) function, just like with the
COMEDI_CMDTEST ioctl. If the (*do_cmdtest) fails the "live" command
is dropped by do_become_nonbusy(). If it succeeds the subdevice is
set as busy and the command is executed by the (*do_cmd).
So:
1a) If the user does a COMEDI_CMDTEST when a command is not running
we don't have any issues.
1b) If the user does a COMEDI_CMDTEST while a command is running it will
not affect the "live" command in async->cmd since it is using a local variable
for the comedi_cmd.
2a) If the user does a COMEDI_CMD when a command is not running we
don't have any issues.
2b) If the user does a COMEDI_CMD while a command is running it will not
succeed because the subdevice is busy.
Have I missed something?
The patch is fine (as I mentioned in the second paragraph of my reply)
although I thought devpriv->act_chanlist_len was now misnamed as it's
used for setting up the next command and there may be an active command
already running.
The first paragraph of my reply was just a comment about being careful
when changing the private data in the do_cmdtest routine. Hope that
clears up any misunderstanding!
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@xxxxxxxxx> )=-
-=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
