On Wednesday, November 12, 2014 9:20 AM, Ian Abbott wrote: > On 12/11/14 16:07, Hartley Sweeten wrote: >> The write to the Miscellaneous Control register (DMM32AT_CTRL_REG) in >> the ISR routine is actually safe. According to the user manual: >> >> INTRST Writing a 1 to this location resets the interrupt request circuit on the >> board. The programmer must write a 1 to this bit during the interrupt >> service routine, or further interrupts will not occur. Writing a 1 to this >> bit does not disturb the values of the PAGE bits. > > Oh right, so the other bits in the written value get ignored if that bit > is set. >> >> The only other writes to the DMM32AT_CTRL_REG are in: >> >> dmm32at_reset() - sets the RESETA bit to cause a full reset of the board >> >> dmm32at_ai_cmd() - sets INTRST to reset the interrupt (this is probably not >> necessary, maybe it should be moved to the (*cancel)). >> >> dmm32at_setaitimer() - selects the 8254 page >> >> dmm32at_8255_io() - selects the 8255 page >> >> The only place the protection is needed is in the dmm32at_setaitimer() and >> dmm32at_8255_io() functions. >> >> The PAGE bits are readable in the FIFO Status register (DMM32AT_FIFO_STATUS_REG) >> but a spin-lock is probably safer. Could we use the comedi_device 'spinlock' for this? >> The core does not seem to use it. Is this just ageneral purpose spin-lock for the drivers? >> It really needs a comment of some sort. > > Yes, it's general purpose for the driver and the comedi core doesn't use > it itself. I don't think the spin-lock is strictly necessary in the > dmm32at_setaitimer() and dmm32at_8255_io() functions as they currently > only get called while dev->mutex is locked. OK, so no change needed. Side-note... Currently the board reset also probes the board to try and "verify" that the board is actually a dmm32at. Do you think this is actually necessary? I feel the board reset just needs to set the RESETA bit to reset the board. This causes a full reset of all features of the board, including the DACs, the FIFO, the digital I/O, and all internal registers. The other writes to the board registers are not needed since they are only setup the registers for the verify check. The udelay() calls can also probably be removed since there is no mention of a delay needed after the reset in the user manual. There is still the issue of the SD1/0 bits in the AI status register. These provide feedback of how the jumpers are set to configure the AI channels for single-ended or differential input. This could be worked out so the differential mode actually works. Your thoughts... Hartley _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
