Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1404586

RE: [PATCH 04/20] staging: comedi: drivers: re-do macros for PLX PCI 9080 LASxRR values

From Hartley Sweeten <HartleyS@visionengravers.com>
Newsgroups linux.kernel
Subject RE: [PATCH 04/20] staging: comedi: drivers: re-do macros for PLX PCI 9080 LASxRR values
Date 2016-05-20 18:40 +0200
Message-ID <rAVCV-7On-5@gated-at.bofh.it> (permalink)
References <rASYp-68P-19@gated-at.bofh.it> <rASYq-68P-39@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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 <abbotti@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

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 04/20] staging: comedi: drivers: re-do macros for PLX PCI 9080 LASxRR values Ian Abbott <abbotti@mev.co.uk> - 2016-05-20 15:50 +0200
  RE: [PATCH 04/20] staging: comedi: drivers: re-do macros for PLX PCI  9080 LASxRR values Hartley Sweeten <HartleyS@visionengravers.com> - 2016-05-20 18:40 +0200
    Re: [PATCH 04/20] staging: comedi: drivers: re-do macros for PLX PCI  9080 LASxRR values Ian Abbott <abbotti@mev.co.uk> - 2016-05-20 19:20 +0200
      RE: [PATCH 04/20] staging: comedi: drivers: re-do macros for PLX PCI  9080 LASxRR values Hartley Sweeten <HartleyS@visionengravers.com> - 2016-05-20 20:10 +0200
        Re: [PATCH 04/20] staging: comedi: drivers: re-do macros for PLX PCI  9080 LASxRR values Ian Abbott <abbotti@mev.co.uk> - 2016-05-23 13:30 +0200

csiph-web