Re: [PATCH] comedi: Change error return code for if statement in the function,cb_pcimdas_ai_rinsn

[Ian Abbott <abbotti@mev.co.uk>]

On 25/02/15 21:37, Nicholas Krause wrote:
>
>
> On February 25, 2015 1:03:14 PM EST, Hartley Sweeten <HartleyS@visionengravers.com> wrote:
>> On Tuesday, February 24, 2015 9:13 PM, Nicholas Krause wrote:
>>> This changes us using the incorrect error,-ETIMEOUT when checking if
>>> the channel we are allocating to on the device structure pointer
>> passed
>>> to this function is greater then the maximum available channels for
>> this
>>> device to the correct error for a channel being out of range,-ECHRNG.
>>>
>>> Signed-off-by: Nicholas Krause <xerofoify@gmail.com>
>>> ---
>>>   drivers/staging/comedi/drivers/cb_pcimdas.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/comedi/drivers/cb_pcimdas.c
>> b/drivers/staging/comedi/drivers/cb_pcimdas.c
>>> index 70dd2c9..d91a6f3 100644
>>> --- a/drivers/staging/comedi/drivers/cb_pcimdas.c
>>> +++ b/drivers/staging/comedi/drivers/cb_pcimdas.c
>>> @@ -121,7 +121,7 @@ static int cb_pcimdas_ai_rinsn(struct
>> comedi_device *dev,
>>>   		maxchans = s->n_chan;
>>>
>>>   	if (chan > (maxchans - 1))
>>> -		return -ETIMEDOUT;	/* *** Wrong error code. Fixme. */
>>> +		return -ECHRNG;
>>>
>>>   	/* configure for sw initiated read */
>>>   	d = inb(devpriv->BADR3 + 5);
>>
>> Hmm... This isn't quite right...
>>
>> The 16 single-ended / 8 differential analog input channels on this
>> board is
>> set with a switch on the PCB. The state of the switch should be read
>> when
>> the driver is attached and the subdevice initialized with the correct
>> number
>> of channels. The core will then validate the "chan" number before
>> calling
>> the (*insn_read) operation.
>>
>> Regards,
>> Hartley
> Hartley,
> If that is the case then why is the check  for max channels here.  I  can send in a v2 removing this check as it seems unneeded based on my understanding and your response to this patch.
> Nick
>

The patch is fine, but now superceded by Hartley's series of patches 
which has more extensive changes to the driver:

http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-February/065754.html

That set of patches assumes the switches remain in the same state 
through the lifetime of the device instance, which seems reasonable for 
a PCI card.

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