| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <534D03F2.5070502@mev.co.uk>
Date: Tue, 15 Apr 2014 11:03:30 +0100
From: Ian Abbott <abbotti@....co.uk>
To: Chase Southwood <chase.southwood@...oo.com>,
<gregkh@...uxfoundation.org>
CC: <hsweeten@...ionengravers.com>, <devel@...verdev.osuosl.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] staging: comedi: addi_apci_1564: fixup and absorb
apci1564_reset()
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@...oo.com>
> Cc: H Hartley Sweeten <hsweeten@...ionengravers.com>
> ---
> 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(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> index 4db3bea..45617cf 100644
> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> @@ -576,31 +576,3 @@ static void apci1564_interrupt(int irq, void *d)
> }
> return;
> }
> -
> -static int apci1564_reset(struct comedi_device *dev)
> -{
> - struct addi_private *devpriv = dev->private;
> -
> - /* disable the interrupts */
> - outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
> - /* Reset the interrupt status register */
> - inl(devpriv->i_IobaseAmcc + APCI1564_DI_INT_STATUS_REG);
> - /* Disable the and/or interrupt */
> - outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE1_REG);
> - outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE2_REG);
> - devpriv->b_DigitalOutputRegister = 0;
> - ui_Type = 0;
> - /* Resets the output channels */
> - outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_REG);
> - /* Disables the interrupt. */
> - outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DO_INT_CTRL_REG);
> - outl(0x0, devpriv->i_IobaseAmcc + APCI1564_WDOG_RELOAD_REG);
> - outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_REG);
> - outl(0x0, devpriv->i_IobaseAmcc + APCI1564_TIMER_CTRL_REG);
> -
> - outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
> - outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
> - outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
> - outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
> - return 0;
> -}
> diff --git a/drivers/staging/comedi/drivers/addi_apci_1564.c b/drivers/staging/comedi/drivers/addi_apci_1564.c
> index 11aa0bd..c1b2260 100644
> --- a/drivers/staging/comedi/drivers/addi_apci_1564.c
> +++ b/drivers/staging/comedi/drivers/addi_apci_1564.c
> @@ -4,6 +4,7 @@
> #include "../comedidev.h"
> #include "comedi_fc.h"
> #include "amcc_s5933.h"
> +#include "addi_watchdog.h"
>
> #include "addi-data/addi_common.h"
>
> @@ -22,7 +23,6 @@ static const struct addi_board apci1564_boardtypes[] = {
> .i_DoMaxdata = 0xffffffff,
> .i_Timer = 1,
> .interrupt = apci1564_interrupt,
> - .reset = apci1564_reset,
> .di_config = apci1564_di_config,
> .di_bits = apci1564_di_insn_bits,
> .do_config = apci1564_do_config,
> @@ -34,6 +34,45 @@ static const struct addi_board apci1564_boardtypes[] = {
> },
> };
>
> +static int apci1564_reset(struct comedi_device *dev)
> +{
> + struct addi_private *devpriv = dev->private;
> +
> + ui_Type = 0;
> +
> + /* Disable the input interrupts and reset status register */
> + outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE1_REG);
> + outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_INT_MODE2_REG);
> + inl(devpriv->i_IobaseAmcc + APCI1564_DI_INT_STATUS_REG);
> + outl(0x0, devpriv->i_IobaseAmcc + APCI1564_DI_IRQ_REG);
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.
> +
> + /* 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.)
> +
> + /* 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.
> +
> + outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER1));
> + outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER2));
> + outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER3));
> + outl(0x0, dev->iobase + APCI1564_TCW_CTRL_REG(APCI1564_COUNTER4));
> +
> + return 0;
> +}
> +
> static int apci1564_auto_attach(struct comedi_device *dev,
> unsigned long context)
> {
>
--
-=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@....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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists