Re: [PATCH v6] cxl/region: check interleave capability

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

> >  drivers/cxl/core/hdm.c    | 10 +++++
> >  drivers/cxl/core/region.c | 89 +++++++++++++++++++++++++++++++++++++++
> >  drivers/cxl/cxl.h         |  2 +
> >  drivers/cxl/cxlmem.h      |  1 +
> >  4 files changed, 102 insertions(+)
> > 
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index 7d97790b893d..5b7dff19bbfa 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -52,6 +52,11 @@ int devm_cxl_add_passthrough_decoder(struct cxl_port *port)
> >  	struct cxl_dport *dport = NULL;
> >  	int single_port_map[1];
> >  	unsigned long index;
> > +	struct cxl_hdm *cxlhdm = dev_get_drvdata(&port->dev);
> > +
> > +	/* allow all the interleave capabilities for passthrough decoder */
> > +	cxlhdm->interleave_mask = GENMASK(14, 8);
> > +	cxlhdm->iw_cap_mask = BIT(1) | BIT(2) | BIT(4) | BIT(8);
> >  
> >  	cxlsd = cxl_switch_decoder_alloc(port, 1);
> >  	if (IS_ERR(cxlsd))
> > @@ -79,6 +84,11 @@ 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->iw_cap_mask = BIT(1) | BIT(2) | BIT(4) | BIT(8);  
> 
> Should define a mask for this. Probably should have defined masks for all of the open coded BIT() sets.

I don't agreed.  These bits are logically independent specifying each
supported value of interleave ways.
Put them under a define and I think you end up with less readable code.

> 
> > +	if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_3_6_12_WAY, hdm_cap))
> > +		cxlhdm->iw_cap_mask |= BIT(3) | BIT(6) | BIT(12);
> > +	if (FIELD_GET(CXL_HDM_DECODER_INTERLEAVE_16_WAY, hdm_cap))
> > +		cxlhdm->iw_cap_mask |= BIT(16);
> >  }