RE: [PATCH 1/5] Staging: comedi: addi-data: tidy up digital input register map defines in hwdrv_apci1564.c

[Hartley Sweeten]

On Thursday, March 06, 2014 12:25 AM, Chase Southwood wrote:
> This patch for hwdrv_apci1564.c fixes the register map defines for the
> digital input registers such that they are all the real offsets to each
> register, rather than a mix of real offsets and adders to those offsets.
> Additionally, the defines have been renamed to assist with shortening
> some lines in excess of 80 characters.
>
> Further, some of the old defines were being used incorrectly in the
> i_APCI1564_Reset() function.  Upon swapping the old defines out for the
> new ones in this function, their use has been corrected.
>
> Signed-off-by: Chase Southwood <chase.southwood@xxxxxxxxx>
> ---

Chase,

Looks good. I have a couple minor nitpicks...

The commit message above is a bit wordy. We already know it's a patch for
hwdrv_apci1564.c from the $SUBJECT.

> .../comedi/drivers/addi-data/hwdrv_apci1564.c      | 78 +++++++++-------------
>  1 file changed, 31 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> index 2b47fa1..d178a3b 100644
> --- a/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> +++ b/drivers/staging/comedi/drivers/addi-data/hwdrv_apci1564.c
> @@ -47,11 +47,6 @@ This program is distributed in the hope that it will be useful, but WITHOUT ANY
>  #define APCI1564_ADDRESS_RANGE				128
>  
>  /* DIGITAL INPUT-OUTPUT DEFINE */
> -/* Input defines */
> -#define APCI1564_DIGITAL_IP				0x04
> -#define APCI1564_DIGITAL_IP_INTERRUPT_MODE1		4
> -#define APCI1564_DIGITAL_IP_INTERRUPT_MODE2		8
> -#define APCI1564_DIGITAL_IP_IRQ				16
>  
>  /* Output defines */
>  #define APCI1564_DIGITAL_OP				0x18
> @@ -63,9 +58,6 @@ This program is distributed in the hope that it will be useful, but WITHOUT ANY
>  #define ADDIDATA_OR					0
>  #define ADDIDATA_AND					1
>  
> -/* Digital Input Interrupt Status */
> -#define APCI1564_DIGITAL_IP_INTERRUPT_STATUS		12
> -
>  /* Digital Output Interrupt Status */
>  #define APCI1564_DIGITAL_OP_INTERRUPT_STATUS		8
>  
> @@ -99,6 +91,15 @@ This program is distributed in the hope that it will be useful, but WITHOUT ANY
>  #define APCI1564_TCW_WARN_TIMEVAL			24
>  #define APCI1564_TCW_WARN_TIMEBASE			28
>  
> +/*
> + * devpriv->i_IobaseAmcc Register Map
> + */
> +#define APCI1564_DI_REG				0x04
> +#define APCI1564_DI_INT_MODE1_REG			0x08
> +#define APCI1564_DI_INT_MODE2_REG			0x0c
> +#define APCI1564_DI_INT_STATUS_REG			0x10
> +#define APCI1564_DI_IRQ_REG				0x14

Please align the values.

Thanks,
Hartley

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/