>On Tuesday, April 15, 2014 5:03 AM, Ian Abbott <abbotti@xxxxxxxxx> wrote: >>On 2014-04-15 06:54, Chase Southwood wrote: >>We can remove this function from the boardinfo and move the code from >>hwdrv_apci1564.c into addi_apci_1564.c since it is the only reset function >>used by the driver. The function was also messy and failed to reset a few >>registers, these issues were fixed on the move. >> >>Signed-off-by: Chase Southwood <chase.southwood@xxxxxxxxx> >>Cc: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx> >>--- >>Hartley, >>Could you take a look at the fixes I have done to this reset function? >>The old version was awfully disorganized and seemingly didn't clear all >>of the required registers, and I believe that it is better now. But, as I >>can only compile test, I'd appreciate it if you could make sure the function >>works as intended now. Thanks, Chase. >> >> .../comedi/drivers/addi-data/hwdrv_apci1564.c | 28 --------------- >> drivers/staging/comedi/drivers/addi_apci_1564.c | 41 +++++++++++++++++++++- >> 2 files changed, 40 insertions(+), 29 deletions(-) >> > > >I'm not sure how much the ordering of those register accesses matters, >but I'd be inclined to use the original ordering if there's no reason to >change it: > >write APCI1564_DI_IRQ_REG >read APCI1564_DI_INT_STATUS_REG >write APCI1564_DI_INT_MODE1_REG >write APCI1564_DI_INT_MODE2_REG > >A register-level programming manual would be handy, but I haven't >managed to find one. Ah yeah, this is a good point. I had reordered them to make sure that everything made it over properly, but I neglected to think about the possible ordering requirements...I'll switch them back around. Also, I agree, a register level manual would be great, but it appears as though none have been made public. >> + >> + /* Reset the output channels and disable interrupts */ >> + outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_REG); >> + outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_INT_CTRL_REG); >> + inl(devpriv->i_IobaseAmcc + APCI1564_DO_INT_STATUS_REG); >> + outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_IRQ_REG); > >I'm really not sure about the "correct" order of accesses to >APCI1564_DO_INT_CTRL_REG, APCI1564_DO_INT_STATUS_REG and >APCI1564_DO_IRQ_REG or even which ones are needed. (I'm guessing that >reading APCI1564_DO_INT_STATUS_REG wouldn't change the internal state of >the card, so would be inclined to omit that, but it should be harmless.) I'll delete the read of the status, no problem. As for the ordering of the others, I'll take a look at addi_apci_2032 driver, as it has digital output registers...it may have some insight regarding that. >> + >> + /* Reset the watchdog registers */ >> + addi_watchdog_reset(devpriv->i_IobaseAmcc + APCI1564_WDOG_REG); >> + >> + /* Reset the timer registers */ >> + outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_RELOAD_REG); >> + outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG); >> + >> + /* Reset the counter registers */ >> + outl(0x0, dev->iobase + APCI1564_TCW_RELOAD_REG(APCI1564_COUNTER1)); >> + outl(0x0, dev->iobase + APCI1564_TCW_RELOAD_REG(APCI1564_COUNTER1)); >> + outl(0x0, dev->iobase + APCI1564_TCW_RELOAD_REG(APCI1564_COUNTER1)); >> + outl(0x0, dev->iobase + APCI1564_TCW_RELOAD_REG(APCI1564_COUNTER1)); > >I think you forgot to edit those four lines to access different counters. Ah geez...That's what I get for copy/paste. Anyway, I'll get v2 of this out very soon (probably tomorrow), though it will probably come from my other mail address (as lkml is bouncing back mails from this account). Thanks, Chase _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
