[PATCH 01/15] staging: comedi: hwdrv_apci3501: remove useless read/mask to stop watchdog

Hartley Sweeten HartleyS at visionengravers.com
Fri Aug 14 17:15:28 UTC 2015


On Thursday, August 13, 2015 2:42 AM, Ian Abbott wrote:
> On 12/08/15 21:25, H Hartley Sweeten wrote:
>> The watchdog is stopped in apci3501_write_insn_timer() by writing a 0 to
>> the timer control register. There is no need to read the register first
>> and mask it (as done when the timer is used as a timer).
>>
>> Reported-by: coverity (CID 1227052)
>> Signed-off-by: H Hartley Sweeten <hsweeten at visionengravers.com>
>> Cc: Ian Abbott <abbotti at mev.co.uk>
>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>> ---
>>   drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c
>> index 1f2f781..e12b2bc 100644
>> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c
>> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci3501.c
>> @@ -102,8 +102,6 @@ static int apci3501_write_insn_timer(struct comedi_device *dev,
>>   			/* Enable the Watchdog */
>>   			outl(ul_Command1, dev->iobase + APCI3501_TIMER_CTRL_REG);
>>   		} else if (data[1] == 0) { /* Stop The Watchdog */
>> -			ul_Command1 = inl(dev->iobase + APCI3501_TIMER_CTRL_REG);
>> -			ul_Command1 = ul_Command1 & 0xFFFFF9FEUL;
>>   			outl(0x0, dev->iobase + APCI3501_TIMER_CTRL_REG);
>>   		} else if (data[1] == 2) {
>>   			ul_Command1 = inl(dev->iobase + APCI3501_TIMER_CTRL_REG);
>>
>
> I wonder if that was a bug in the original code.  Shouldn't it have just 
> cleared bit 0 to stop the watchdog?

I'm not really sure. All of the ADDI-DATA drivers were a pretty big mess originally.

Looking at the original code, it appears the tried to take their out of tree drivers and
shoehorn them into comedi. Those drivers appear to have started from a base driver
and just got cut-pasted to add/remove features that were present on a particular
board. This could have introduced a number of bugs, especially in the TCW (timer/
counter/watchdog). Looking at the I/O maps it the TCW hardware for all the ADDI-DATA
boards is based on the same IP but some implementation have various features
removed.

Unfortunately I have only been able to get the I/O maps from ADDI-DATA. These have
little if any descriptions about the TCW. Mostly they just have the register offsets and
some of the bit defines. They don't appear to have an actual datasheet on the operation
of the TCW. All we can do is try to derive the operation based on their out-of-tree drivers
and the original comedi drivers.

Some actual user testing of the ADDI-DATA drivers would be nice.

They did provide me with two boards, an APCIE-1532 and a APCIE-3121-16-8. I just haven't
had a chance to try them yet. Hopefully soon.

Regards,
Hartley



More information about the devel mailing list