* [PATCH] staging: comedi: das1800: remove unused variable
@ 2016-04-05 14:23 Sudip Mukherjee
2016-04-06 1:21 ` Hartley Sweeten
0 siblings, 1 reply; 6+ messages in thread
From: Sudip Mukherjee @ 2016-04-05 14:23 UTC (permalink / raw)
To: Ian Abbott, H Hartley Sweeten, Greg Kroah-Hartman
Cc: linux-kernel, devel, Sudip Mukherjee
The variable unipolar was never used.
Signed-off-by: Sudip Mukherjee <sudip.mukherjee@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.
drivers/staging/comedi/drivers/das1800.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/staging/comedi/drivers/das1800.c b/drivers/staging/comedi/drivers/das1800.c
index 94078118..76cf2cd 100644
--- a/drivers/staging/comedi/drivers/das1800.c
+++ b/drivers/staging/comedi/drivers/das1800.c
@@ -482,9 +482,6 @@ static void das1800_handle_fifo_not_empty(struct comedi_device *dev,
{
struct comedi_cmd *cmd = &s->async->cmd;
unsigned short dpnt;
- int unipolar;
-
- unipolar = inb(dev->iobase + DAS1800_CONTROL_C) & UB;
while (inb(dev->iobase + DAS1800_STATUS) & FNE) {
dpnt = inw(dev->iobase + DAS1800_FIFO);
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] staging: comedi: das1800: remove unused variable
2016-04-05 14:23 [PATCH] staging: comedi: das1800: remove unused variable Sudip Mukherjee
@ 2016-04-06 1:21 ` Hartley Sweeten
2016-04-06 9:41 ` Ian Abbott
0 siblings, 1 reply; 6+ messages in thread
From: Hartley Sweeten @ 2016-04-06 1:21 UTC (permalink / raw)
To: Sudip Mukherjee, Ian Abbott, Greg Kroah-Hartman; +Cc: linux-kernel, devel
On Tuesday, April 05, 2016 7:23 AM, Sudip Mukherjee wrote:
> The variable unipolar was never used.
>
> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@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.
Thanks,
Hartley
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: comedi: das1800: remove unused variable
2016-04-06 1:21 ` Hartley Sweeten
@ 2016-04-06 9:41 ` Ian Abbott
2016-04-06 10:40 ` Ian Abbott
2016-04-06 16:19 ` Hartley Sweeten
0 siblings, 2 replies; 6+ messages in thread
From: Ian Abbott @ 2016-04-06 9:41 UTC (permalink / raw)
To: Hartley Sweeten, Sudip Mukherjee, Greg Kroah-Hartman; +Cc: linux-kernel, devel
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.mukherjee@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.
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@mev.co.uk> )=-
-=( Web: http://www.mev.co.uk/ )=-
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] staging: comedi: das1800: remove unused variable
2016-04-06 9:41 ` Ian Abbott
@ 2016-04-06 10:40 ` Ian Abbott
2016-04-06 17:55 ` Hartley Sweeten
2016-04-06 16:19 ` Hartley Sweeten
1 sibling, 1 reply; 6+ messages in thread
From: Ian Abbott @ 2016-04-06 10:40 UTC (permalink / raw)
To: Hartley Sweeten, Sudip Mukherjee, Greg Kroah-Hartman; +Cc: linux-kernel, devel
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.mukherjee@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.
>
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!
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@mev.co.uk> )=-
-=( Web: http://www.mev.co.uk/ )=-
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] staging: comedi: das1800: remove unused variable
2016-04-06 9:41 ` Ian Abbott
2016-04-06 10:40 ` Ian Abbott
@ 2016-04-06 16:19 ` Hartley Sweeten
1 sibling, 0 replies; 6+ messages in thread
From: Hartley Sweeten @ 2016-04-06 16:19 UTC (permalink / raw)
To: Ian Abbott, Sudip Mukherjee, Greg Kroah-Hartman; +Cc: linux-kernel, devel
On Wednesday, April 06, 2016 2:41 AM, 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.mukherjee@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?
I'll split it out as a separate patch and post it shortly.
Thanks,
Hartley
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] staging: comedi: das1800: remove unused variable
2016-04-06 10:40 ` Ian Abbott
@ 2016-04-06 17:55 ` Hartley Sweeten
0 siblings, 0 replies; 6+ messages in thread
From: Hartley Sweeten @ 2016-04-06 17:55 UTC (permalink / raw)
To: Ian Abbott, Sudip Mukherjee, Greg Kroah-Hartman; +Cc: linux-kernel, devel
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.mukherjee@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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-04-06 18:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-05 14:23 [PATCH] staging: comedi: das1800: remove unused variable Sudip Mukherjee
2016-04-06 1:21 ` Hartley Sweeten
2016-04-06 9:41 ` Ian Abbott
2016-04-06 10:40 ` Ian Abbott
2016-04-06 17:55 ` Hartley Sweeten
2016-04-06 16:19 ` Hartley Sweeten
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).