Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1371667 > unrolled thread

[PATCH] staging: comedi: das1800: remove unused variable

Back to article view | Back to linux.kernel

Contents

  [PATCH] staging: comedi: das1800: remove unused variable Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2016-04-05 16:30 +0200
    RE: [PATCH] staging: comedi: das1800: remove unused variable Hartley Sweeten <HartleyS@visionengravers.com> - 2016-04-06 03:30 +0200
      Re: [PATCH] staging: comedi: das1800: remove unused variable Ian Abbott <abbotti@mev.co.uk> - 2016-04-06 11:50 +0200
        Re: [PATCH] staging: comedi: das1800: remove unused variable Ian Abbott <abbotti@mev.co.uk> - 2016-04-06 12:50 +0200
          RE: [PATCH] staging: comedi: das1800: remove unused variable Hartley Sweeten <HartleyS@visionengravers.com> - 2016-04-06 20:20 +0200
        RE: [PATCH] staging: comedi: das1800: remove unused variable Hartley Sweeten <HartleyS@visionengravers.com> - 2016-04-06 18:40 +0200

#1371667 — [PATCH] staging: comedi: das1800: remove unused variable

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

[toc] | [next] | [standalone]

#1372125

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

[toc] | [prev] | [next] | [standalone]

#1372342

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/  )=-

[toc] | [prev] | [next] | [standalone]

#1372396

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/  )=-

[toc] | [prev] | [next] | [standalone]

#1372718

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

[toc] | [prev] | [next] | [standalone]

#1372656

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web