On Friday, May 20, 2016 6:49 AM, Ian Abbott wrote:
> Rename the macros for the PLX PCI 9080 LAS0RR and LAS1RR registers in
> "plx9080.h", using the prefix `PLX_LASRR_`. Make use of the `BIT(x)`
> and `GENMASK(h,l)` macros to define the values.
>
> Define a macro `PLX_LASRR_PREFETCH` for the "prefetchable memory" bit in
> this register, and define a macro `PLX_LASRR_MLOC_MASK` to mask the PCI
> memory location control bits.
>
> Signed-off-by: Ian Abbott <abbo...@mev.co.uk>
> ---
[snip]
> diff --git a/drivers/staging/comedi/drivers/plx9080.h
> b/drivers/staging/comedi/drivers/plx9080.h
> index 92d2480..8788117 100644
> --- a/drivers/staging/comedi/drivers/plx9080.h
> +++ b/drivers/staging/comedi/drivers/plx9080.h
> @@ -54,14 +54,16 @@ struct plx_dma_desc {
> /* Local Address Space 1 Range Register */
> #define PLX_REG_LAS1RR 0x00f0
>
> -#define LRNG_IO 0x00000001 /* Map to: 1=I/O, 0=Mem */
> -#define LRNG_ANY32 0x00000000 /* Locate anywhere in 32 bit */
> -#define LRNG_LT1MB 0x00000002 /* Locate in 1st meg */
> -#define LRNG_ANY64 0x00000004 /* Locate anywhere in 64 bit */
> -/* bits that specify range for memory io */
> -#define LRNG_MEM_MASK 0xfffffff0
> -/* bits that specify range for normal io */
> -#define LRNG_IO_MASK 0xfffffffc
> +#define PLX_LASRR_IO BIT(0) /* Map to: 1=I/O, 0=Mem */
> +#define PLX_LASRR_ANY32 (BIT(1) * 0) /* Locate anywhere in
> 32 bit */
> +#define PLX_LASRR_LT1MB (BIT(1) * 1) /* Locate in 1st meg */
> +#define PLX_LASRR_ANY64 (BIT(1) * 2) /* Locate anywhere in
> 64 bit */
The (BIT(n) * x) looks ugly.
These bit define the memory space encoding. I would prefer something
like this:
#define PLX_LASSR_MLOC(x) (((x) & 0x3) << 1)
#define PLX_LASSR_MLOC_ANY32 PLX_LASSR_MLOC(0)
#define PLX_LASSR_MLOC_LT1MB PLX_LASSR_MLOC(1)
#define PLX_LASSR_MLOC_ANY64 PLX_LASSR_MLOC(2)
> +#define PLX_LASRR_MLOC_MASK GENMASK(2, 1) /* Memory location bits */
I guess the GENMASK() macro is common but it's currently
not used by any of the comedi code.
Using the macro above, the 'mask' would be:
#define PLX_LASSR_MLOC_MASK PLX_LASSR_MLOC(3)
> +#define PLX_LASRR_PREFETCH BIT(3) /* Memory is prefetchable */
> +/* bits that specify range for memory space decode bits */
> +#define PLX_LASRR_MEM_MASK GENMASK(31, 4)
> +/* bits that specify range for i/o space decode bits */
> +#define PLX_LASRR_IO_MASK GENMASK(31, 2)
I suppose the GENMASK() use makes sense for these.
Regards,
Hartley
