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

[Ian Abbott]

On 25/02/15 21:37, Nicholas Krause wrote:
> On February 25, 2015 1:03:14 PM EST, Hartley Sweeten <HartleyS@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> > > ---
> > >   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@xxxxxxxxx> )=-
-=(                          Web: http://www.mev.co.uk/  )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/