On Wednesday, April 06, 2016 3:41 AM, Ian Abbott wrote:
> On 06/04/16 10:41, Ian Abbott wrote:
>> On 06/04/16 02:21, Hartley Sweeten wrote:
>>> On Tuesday, April 05, 2016 7:23 AM, Sudip Mukherjee wrote:
>>>> The variable unipolar was never used.
>>>>
>>>> Signed-off-by: Sudip Mukherjee <sudip.mukher...@codethink.co.uk>
>>>> ---
>>>>
>>>> There may be a chance that reading from DAS1800_CONTROL_C is necessary
>>>> before reading from DAS1800_STATUS. If that is true then please discard
>>>> this patch.
>>>
>>> Actually the driver has a bug here.
>>>
>>> The analog input samples should be munged if the inputs are
>>> configured for bipolar mode.
>>>
>>> I have a series almost ready that cleans up this driver and fixes the
>>> bug.
>>
>> Hi Hartley, can the bug fix be placed at the top of your patch series?
>> Thanks.
>>
Just posted the patch. It's a bit more involved than your fix in the comedi.org
code but I think it's more complete.
> The bug has been there forever (even in the "out-of-tree" version from
> comedi.org, where I have just fixed it). There have been patches applied
> to reformat and remove the incorrect bits of code, including
> a142785d7c9d ("Staging: comedi: das1800: fixed multiple brace coding
> style issues and pointer declaration style errors") and 82d28561b7e0
> ("staging: comedi: Remove if condition."). The latter patch,
> 82d28561b7e0, mainly served to hide the bug further!
Looks like both of those patches might have originated from one of the
outreachy programs that Greg deals with.
If you think this needs to be fixed in any of the stable trees, I can split it
to
match your comedi.org fix (to fix the stable trees) and a second patch for
the additional cleanup. This is an old legacy ISA board and, as you stated,
the bug has been there forever so I'm not sure if the backport is worth it.
Regards,
Hartley
