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 <hswee...@visionengravers.com>
>> Cc: Ian Abbott <abbo...@mev.co.uk>
>> Cc: Greg Kroah-Hartman <gre...@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
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
