RE: [PATCH v2 2/3] cxl: Add CXL spec v3.0 interleave support

[Dan Williams <dan.j.williams@intel.com>]

Dave Jiang wrote:
> CXL spec v3.0 added 2 CAP bits to the CXL HDM Decoder Capability Register.
> CXL spec v3.0 8.2.4.19.1. Bit 11 indicates that 3, 6, and 12 way interleave
> is capable. Bit 12 indicates that 16 way interleave is capable.
> 
> Add code to parse_hdm_decoder_caps() to cache those new bits. Add check in
> cxl_interleave_verify() call to make sure those CAP bits matches the passed
> in interleave value.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/hdm.c |    6 ++++++
>  drivers/cxl/cxl.h      |    2 ++
>  drivers/cxl/cxlmem.h   |   13 +++++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 8143e2615957..0baf3c4820a5 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -80,6 +80,12 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
>  		cxlhdm->interleave_mask |= GENMASK(11, 8);
>  	if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_14_12, hdm_cap))
>  		cxlhdm->interleave_mask |= GENMASK(14, 12);
> +
> +	cxlhdm->interleave_cap = CXL_HDM_INTERLEAVE_CAP_DEFAULT;
> +	if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY, hdm_cap))
> +		cxlhdm->interleave_cap |= CXL_HDM_INTERLEAVE_CAP_3_6_12;
> +	if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_16_WAY, hdm_cap))
> +		cxlhdm->interleave_cap |= CXL_HDM_INTERLEAVE_CAP_16;
>  }
>  
>  static void __iomem *map_hdm_decoder_regs(struct cxl_port *port,
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index bc604b7e44fb..105d814941e7 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -42,6 +42,8 @@
>  #define   CXL_HDM_DECODER_TARGET_COUNT_MASK GENMASK(7, 4)
>  #define   CXL_HDM_DECODER_INTERLEAVE_11_8 BIT(8)
>  #define   CXL_HDM_DECODER_INTERLEAVE_14_12 BIT(9)
> +#define   CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY BIT(11)
> +#define   CXL_HDM_DECODER_INTERLEAVE_16_WAY BIT(12)
>  #define CXL_HDM_DECODER_CTRL_OFFSET 0x4
>  #define   CXL_HDM_DECODER_ENABLE BIT(1)
>  #define CXL_HDM_DECODER0_BASE_LOW_OFFSET(i) (0x20 * (i) + 0x10)
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 625fce0b6c2c..ebd645d8bbb4 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -393,14 +393,24 @@ static inline void cxl_mem_active_dec(void)
>  }
>  #endif
>  
> +#define CXL_HDM_INTERLEAVE_CAP_DEFAULT BIT(1) | BIT(2) | BIT(4) | BIT(8)
> +#define CXL_HDM_INTERLEAVE_CAP_3_6_12 BIT(3) | BIT(6) | BIT(12)
> +#define CXL_HDM_INTERLEAVE_CAP_16 BIT(16)
> +
>  struct cxl_hdm {
>  	struct cxl_component_regs regs;
>  	unsigned int decoder_count;
>  	unsigned int target_count;
>  	unsigned int interleave_mask;
> +	unsigned long interleave_cap;
>  	struct cxl_port *port;
>  };
>  
> +static inline bool valid_interleave_ways(struct cxl_hdm *cxlhdm, u8 iw)
> +{
> +	return test_bit(iw, &cxlhdm->interleave_cap);
> +}

I think "test_bit(iw, &cxlhdm->interleave_cap)" is self explanatory, no
need for the valid_interleave_ways() wrapper.

Other than that:

Reviewed-by: Dan Williams <dan.j.williams@intel.com>