RE: [PATCH 05/46] staging: comedi: me4000: remove 'board' from me4000_ai_insn_read()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wednesday, August 05, 2015 4:38 AM, Ian Abbott wrote:
> On 04/08/15 18:57, H Hartley Sweeten wrote:
>> The 'board' pointer is only used in this function to verify that the
>> 'chan' is valid for an aref of AREF_DIFF. The comedi core ensures that
>> that aref is only possible if the subdevice has the SDF_DIFF subdevice_flag
>> set. If so, the maximum channel is half the subdevice 'n_chan'. Use that
>> instead and remove the 'board' variable.
>>
>> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>
>> ---
>>   drivers/staging/comedi/drivers/me4000.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/me4000.c b/drivers/staging/comedi/drivers/me4000.c
>> index d6b7adc..a1b30f9 100644
>> --- a/drivers/staging/comedi/drivers/me4000.c
>> +++ b/drivers/staging/comedi/drivers/me4000.c
>> @@ -436,10 +436,10 @@ static void me4000_reset(struct comedi_device *dev)
>>     ===========================================================================*/
>>
>>   static int me4000_ai_insn_read(struct comedi_device *dev,
>> -			       struct comedi_subdevice *subdevice,
>> -			       struct comedi_insn *insn, unsigned int *data)
>> +			       struct comedi_subdevice *s,
>> +			       struct comedi_insn *insn,
>> +			       unsigned int *data)
>>   {
>> -	const struct me4000_board *board = dev->board_ptr;
>>   	int chan = CR_CHAN(insn->chanspec);
>>   	int rang = CR_RANGE(insn->chanspec);
>>   	int aref = CR_AREF(insn->chanspec);
>> @@ -487,7 +487,7 @@ static int me4000_ai_insn_read(struct comedi_device *dev,
>>   			return -EINVAL;
>>   		}
>>
>> -		if (chan >= board->ai_diff_nchan) {
>> +		if (chan >= (s->n_chan / 2)) {
>
> You also need to check that differential input mode is supported because 
> the comedi core doesn't verify the aref flags.  Since you are removing 
> the 'board' variable, that can be done by checking 's->subdev_flags & 
> SDF_DIFF' (since patch 03 ensures that is only set if differential mode 
> supported).

Gah... Now that you mentioned it, I knew that but forgot... I think I missed
It due the removal of the (unused) aref_invalid() function in range.c.

We might want to add a comment about the aref flags not being checked.

You had some other comments that I need to address in this series. I'll
do that and post a v2 later.

Thanks,
Hartley



_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux