[PATCH 46/55] staging: comedi: drivers: handle SDF_PACKED in comedi_inc_scan_progress()

Ian Abbott abbotti at mev.co.uk
Fri Oct 24 08:49:17 UTC 2014


On 23/10/14 17:35, Hartley Sweeten wrote:
> On Thursday, October 23, 2014 4:28 AM, Ian Abbott wrote:
>> On 22/10/14 23:37, H Hartley Sweeten wrote:
>>> Subdevices that set the SDF_PACKED flag return all the scan data in a single
>>> sample. The cmd->chanlist_len is used to pass the DIO channel information to
>>> the command and does not indicate the length (cmd->scan_end_arg) of the scan.
>>
>> In general, the scan data could be packed into more than one "sample",
>> but you can fit 8*bytes_per_sample() 1-bit samples in each "sample".  In
>> practice, currently only the ni_pcidio driver sets the SDF_PACKED flag
>>   (although some other drivers ought to set it) and the whole scan will
>> fit into a single "sample" in that case.
>
> Patches 47, 48, 50, and 51 add the SDF_PACKED flag to the addi_apci_2032,
> amplc_dio200_common, pcmmio, and pcmuio drivers.

I appreciate that.  It was something I was hoping to get around to, 
especially as pcmmio and pcmuio had endian issues.

>>> Currently this flag is not handled in the comedi core files. Modify
>>> comedi_inc_scan_progress() to automatically set the COMEDI_CB_EOS event if
>>> the SDF_PACKED flag is set. This makes the flag work and we can remove the
>>> events from the drivers.
>>>
>>> Signed-off-by: H Hartley Sweeten <hsweeten at visionengravers.com>
>>> Cc: Ian Abbott <abbotti at mev.co.uk>
>>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>>> ---
>>>    drivers/staging/comedi/drivers.c | 19 +++++++++++++++----
>>>    1 file changed, 15 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/staging/comedi/drivers.c b/drivers/staging/comedi/drivers.c
>>> index ff2df85..dd8f5db 100644
>>> --- a/drivers/staging/comedi/drivers.c
>>> +++ b/drivers/staging/comedi/drivers.c
>>> @@ -341,12 +341,23 @@ void comedi_inc_scan_progress(struct comedi_subdevice *s,
>>>    			      unsigned int num_bytes)
>>>    {
>>>    	struct comedi_async *async = s->async;
>>> -	unsigned int scan_length = comedi_bytes_per_scan(s);
>>>
>>> -	async->scan_progress += num_bytes;
>>> -	if (async->scan_progress >= scan_length) {
>>> -		async->scan_progress %= scan_length;
>>> +	/*
>>> +	 * The SDF_PACKED flag indicates that the subdevice does "packed DIO".
>>> +	 *
>>> +	 * This means that all the scan data is returned in a single sample
>>> +	 * and an "end of scan" occurs for every sample.
>>> +	 */
>>> +	if (s->subdev_flags & SDF_PACKED) {
>>>    		async->events |= COMEDI_CB_EOS;
>>> +	} else {
>>> +		unsigned int scan_length = comedi_bytes_per_scan(s);
>>> +
>>> +		async->scan_progress += num_bytes;
>>> +		if (async->scan_progress >= scan_length) {
>>> +			async->scan_progress %= scan_length;
>>> +			async->events |= COMEDI_CB_EOS;
>>> +		}
>>>    	}
>>>    }
>>>    EXPORT_SYMBOL_GPL(comedi_inc_scan_progress);
>>>
>>
>> Is this patch even necessary?  comedi_bytes_per_scan() assumes digital
>> samples will be packed into a whole number of "samples" and calculates
>> the scan length in bytes accordingly.
>
> I guess it's not really "necessary" but it adds clarification to what
> the SDF_PACKED flag does. Right now the only documentation
> for that flag is:
>
> #define SDF_PACKED	0x20000000	/* subdevice can do packed DIO */
>
> But nothing in the core uses it. I don't think comedilib
> references the flag either.

Applications could see it, though they would probably ignore it as well, 
as any application using async commands for digital I/O probably needs 
to be hardware specific as the order of data in the scan data doesn't 
match the order of channels in the chanlist for all drivers. (Some just 
return all channels in channel number order.)

> It took me a bit of head scratching to figure out why the
> ni_pcidio driver would be setting the flag. This seemed
> like the best way to make the core use the flag for
> something.
>
> As you pointed out, this will not work if a driver packs a scan
> into more than one "sample" (32-bits if the SDF_LSAMPL flag
> is set, 16-bits if not). But, it works for all the current comedi
> drivers. If necessary, a separate patch could be made to make
> it work if a driver does use more than one "sample" to pack the
> scan.

But the original already works in that case!

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk> )=-
-=(                          Web: http://www.mev.co.uk/  )=-


More information about the devel mailing list