Re: [PATCH 0/3] staging: comedi: addi_apci_1564: miscellaneous fixes and cleanups
Hi all, On Mon, Jun 30, 2014 at 5:25 AM, Ian Abbott wrote: > On 2014-06-28 05:47, Chase Southwood wrote: >> >> This patchset moves a misplaced include to the proper file, swaps out an >> overly >> aggressive placement of apci1564_reset(), and cleans up >> apci1564_interrupt(). >> >> Chase Southwood (3): >>staging: comedi: addi_apci_1564: move addi_watchdog.h include to >> addi_apci_1564.c >>staging: comedi: addi_apci_1564: fix use of apci1564_reset() to >> disable DI interrupts >>staging: comedi: addi_apci_1564: clean up apci1564_interrupt() >> > > It's okay, but I think you can simplify the interrupt handling a bit more by > not bothering to check for interrupts from unknown sources. If a source > hasn't been enabled, it shouldn't generate interrupts, right? Besides, since > the does nothing to stop further interrupts from unknown sources, it would > just keep getting further interrupts repeatedly in that case. > > Then you can get rid of the ui_DO, ui_DI, ui_Timer, and counters[] variables > and just check the registers directly (e.g. replace 'if (ui_Timer == 1)' > with 'if (inl(devpriv->amcc_iobase + APCI1564_TIMER_IRQ_REG) & 0x01)'). I received some more good comments about the interrupt function, and looks like Greg is on vacation this week anyway, so I think I will just redo patch 3 and send out a v2 patchset. Thanks, Chase -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] staging: comedi: addi_apci_1564: miscellaneous fixes and cleanups
Hi all, On Mon, Jun 30, 2014 at 5:25 AM, Ian Abbott abbo...@mev.co.uk wrote: On 2014-06-28 05:47, Chase Southwood wrote: This patchset moves a misplaced include to the proper file, swaps out an overly aggressive placement of apci1564_reset(), and cleans up apci1564_interrupt(). Chase Southwood (3): staging: comedi: addi_apci_1564: move addi_watchdog.h include to addi_apci_1564.c staging: comedi: addi_apci_1564: fix use of apci1564_reset() to disable DI interrupts staging: comedi: addi_apci_1564: clean up apci1564_interrupt() It's okay, but I think you can simplify the interrupt handling a bit more by not bothering to check for interrupts from unknown sources. If a source hasn't been enabled, it shouldn't generate interrupts, right? Besides, since the does nothing to stop further interrupts from unknown sources, it would just keep getting further interrupts repeatedly in that case. Then you can get rid of the ui_DO, ui_DI, ui_Timer, and counters[] variables and just check the registers directly (e.g. replace 'if (ui_Timer == 1)' with 'if (inl(devpriv-amcc_iobase + APCI1564_TIMER_IRQ_REG) 0x01)'). I received some more good comments about the interrupt function, and looks like Greg is on vacation this week anyway, so I think I will just redo patch 3 and send out a v2 patchset. Thanks, Chase -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] staging: comedi: addi_apci_1564: miscellaneous fixes and cleanups
On 2014-06-28 05:47, Chase Southwood wrote: This patchset moves a misplaced include to the proper file, swaps out an overly aggressive placement of apci1564_reset(), and cleans up apci1564_interrupt(). Chase Southwood (3): staging: comedi: addi_apci_1564: move addi_watchdog.h include to addi_apci_1564.c staging: comedi: addi_apci_1564: fix use of apci1564_reset() to disable DI interrupts staging: comedi: addi_apci_1564: clean up apci1564_interrupt() It's okay, but I think you can simplify the interrupt handling a bit more by not bothering to check for interrupts from unknown sources. If a source hasn't been enabled, it shouldn't generate interrupts, right? Besides, since the does nothing to stop further interrupts from unknown sources, it would just keep getting further interrupts repeatedly in that case. Then you can get rid of the ui_DO, ui_DI, ui_Timer, and counters[] variables and just check the registers directly (e.g. replace 'if (ui_Timer == 1)' with 'if (inl(devpriv->amcc_iobase + APCI1564_TIMER_IRQ_REG) & 0x01)'). Reviewed-by: Ian Abbott -- -=( Ian Abbott @ MEV Ltd.E-mail: )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/3] staging: comedi: addi_apci_1564: miscellaneous fixes and cleanups
On 2014-06-28 05:47, Chase Southwood wrote: This patchset moves a misplaced include to the proper file, swaps out an overly aggressive placement of apci1564_reset(), and cleans up apci1564_interrupt(). Chase Southwood (3): staging: comedi: addi_apci_1564: move addi_watchdog.h include to addi_apci_1564.c staging: comedi: addi_apci_1564: fix use of apci1564_reset() to disable DI interrupts staging: comedi: addi_apci_1564: clean up apci1564_interrupt() It's okay, but I think you can simplify the interrupt handling a bit more by not bothering to check for interrupts from unknown sources. If a source hasn't been enabled, it shouldn't generate interrupts, right? Besides, since the does nothing to stop further interrupts from unknown sources, it would just keep getting further interrupts repeatedly in that case. Then you can get rid of the ui_DO, ui_DI, ui_Timer, and counters[] variables and just check the registers directly (e.g. replace 'if (ui_Timer == 1)' with 'if (inl(devpriv-amcc_iobase + APCI1564_TIMER_IRQ_REG) 0x01)'). Reviewed-by: Ian Abbott abbo...@mev.co.uk -- -=( Ian Abbott @ MEV Ltd.E-mail: abbo...@mev.co.uk)=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=- -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] staging: comedi: addi_apci_1564: miscellaneous fixes and cleanups
This patchset moves a misplaced include to the proper file, swaps out an overly aggressive placement of apci1564_reset(), and cleans up apci1564_interrupt(). Chase Southwood (3): staging: comedi: addi_apci_1564: move addi_watchdog.h include to addi_apci_1564.c staging: comedi: addi_apci_1564: fix use of apci1564_reset() to disable DI interrupts staging: comedi: addi_apci_1564: clean up apci1564_interrupt() .../comedi/drivers/addi-data/hwdrv_apci1564.c | 2 - drivers/staging/comedi/drivers/addi_apci_1564.c| 114 + 2 files changed, 28 insertions(+), 88 deletions(-) -- 2.0.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH 0/3] staging: comedi: addi_apci_1564: miscellaneous fixes and cleanups
This patchset moves a misplaced include to the proper file, swaps out an overly aggressive placement of apci1564_reset(), and cleans up apci1564_interrupt(). Chase Southwood (3): staging: comedi: addi_apci_1564: move addi_watchdog.h include to addi_apci_1564.c staging: comedi: addi_apci_1564: fix use of apci1564_reset() to disable DI interrupts staging: comedi: addi_apci_1564: clean up apci1564_interrupt() .../comedi/drivers/addi-data/hwdrv_apci1564.c | 2 - drivers/staging/comedi/drivers/addi_apci_1564.c| 114 + 2 files changed, 28 insertions(+), 88 deletions(-) -- 2.0.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/