Re: [PATCH] staging: comedi: cb_pcidas: reinstate delay removed from trimpot setting

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Wed, Nov 04, 2020 at 10:49:18AM +0000, Ian Abbott wrote:
> On 02/11/2020 11:16, Ian Abbott wrote:
> > On 02/11/2020 10:25, Ian Abbott wrote:
> > > On 29/10/2020 14:18, Ian Abbott wrote:
> > > > Commit eddd2a4c675c ("staging: comedi: cb_pcidas: refactor
> > > > write_calibration_bitstream()") inadvertently removed one of the
> > > > `udelay(1)` calls when writing to the calibration register in
> > > > `cb_pcidas_calib_write()`.  Reinstate the delay.  It may seem strange
> > > > that the delay is placed before the register write, but this function is
> > > > called in a loop so the extra delay can make a difference.
> > > > 
> > > > This _might_ solve reported issues reading analog inputs on a
> > > > PCIe-DAS1602/16 card where the analog input values "were scaled in a
> > > > strange way that didn't make sense".  On the same hardware running a
> > > > system with a 3.13 kernel, and then a system with a 4.4 kernel, but with
> > > > the same application software, the system with the 3.13 kernel was fine,
> > > > but the one with the 4.4 kernel exhibited the problem.  Of the 90
> > > > changes to the driver between those kernel versions, this change looked
> > > > like the most likely culprit.
> > > 
> > > Actually, I've realized that this patch will have no effect on the
> > > PCIe-DAS1602/16 card because it uses a different driver -
> > > cb_pcimdas, not cb_pcidas.
> > 
> > But that's also confusing because PCIe-DAS1602/16 was not supported
> > until the 3.19 kernel!  I know the reported has both PCI-DAS1602/16 and
> > PCIe-DAS1602/16 cards (supported by cb_pcidas and cb_pcimdas
> > respectively), so there could have been some mix-up in the reporting.
> 
> Mystery solved.  The reporter had a mixture of PCIe-DAS1602/16 and
> PCIM-DAS1602/16 cards (not PCI-DAS1602/16).  Both of those are supported by
> the "cb_pcimdas" driver (not "cb_pcidas"), although the PCIe card was not
> supported until the 3.19 kernel (by commit 4e3d14af1286).  Testing with the
> 3.13 kernel was done with the PCIM card.
> 
> The "strange scaling" was due to a change in the ranges reported for the
> analog input subdevice in the 4.1 kernel (by commit c7549d770a27). Before
> then, it just reported a single dummy range [0, 1000000] with no units
> (converted to [0.0, 1.0] with no units by comedilib).  Afterwards, it
> reported four different voltage ranges (either unipolar or bipolar,
> depending in a status bit tied to a physical switch).  The reporter's
> application code was using the reported range to scale the raw values to a
> voltage (using comedilib functions), but because the reported range was
> bogus, the application code was performing additional scaling (outside of
> comedilib).  The application code can be changed to check whether the device
> is reporting a proper voltage range or the old, bogus range, and behave
> accordingly.
> 
> > > Greg, you might as well drop this patch if you haven't already
> > > applied it, since it was only a hunch that it fixed a problem.
> 
> That's still the case, although it won't do any harm if applied (apart from
> the incorrect patch description).

I'll leave it dropped :)

thanks,

greg k-h
_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel