Re: [PATCH v3 5/6] cxl: Kill enum cxl_decoder_mode

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

Jonathan Cameron wrote:
> On Mon, 03 Feb 2025 20:24:29 -0800
> Dan Williams <dan.j.williams@intel.com> wrote:
> 
> > Now that the operational mode of DPA capacity (ram vs pmem... etc) is
> > tracked in the partition, and no code paths have dependencies on the
> > mode implying the partition index, the ambiguous 'enum cxl_decoder_mode'
> > can be cleaned up, specifically this ambiguity on whether the operation
> > mode implied anything about the partition order.
> > 
> > Endpoint decoders simply reference their assigned partition where the
> > operational mode can be retrieved as partition mode.
> > 
> > With this in place PMEM can now be partition0 which happens today when
> > the RAM capacity size is zero. Dynamic RAM can appear above PMEM when
> > DCD arrives, etc. Code sequences that hard coded the "PMEM after RAM"
> > assumption can now just iterate partitions and consult the partition
> > mode after the fact.
> > 
> > Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> > Reviewed-by: Alejandro Lucero <alucerop@amd.com>
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> 
> One trivial equality check inline to tidy up otherwise nice.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
[..]
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index 78ecb88bad7e..d705dec1471e 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -359,7 +359,6 @@ static int __cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
> 
> > -int cxl_dpa_set_mode(struct cxl_endpoint_decoder *cxled,
> > -		     enum cxl_decoder_mode mode)
> > +int cxl_dpa_set_part(struct cxl_endpoint_decoder *cxled,
> > +		     enum cxl_partition_mode mode)
> >  {
> >  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> >  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
> >  	struct device *dev = &cxled->cxld.dev;
> > -
> > -	switch (mode) {
> > -	case CXL_DECODER_RAM:
> > -	case CXL_DECODER_PMEM:
> > -		break;
> > -	default:
> > -		dev_dbg(dev, "unsupported mode: %d\n", mode);
> > -		return -EINVAL;
> > -	}
> > +	int part;
> >  
> >  	guard(rwsem_write)(&cxl_dpa_rwsem);
> >  	if (cxled->cxld.flags & CXL_DECODER_F_ENABLE)
> >  		return -EBUSY;
> >  
> > -	/*
> > -	 * Only allow modes that are supported by the current partition
> > -	 * configuration
> > -	 */
> > -	if (mode == CXL_DECODER_PMEM && !cxl_pmem_size(cxlds)) {
> > -		dev_dbg(dev, "no available pmem capacity\n");
> > -		return -ENXIO;
> > +	for (part = 0; part < cxlds->nr_partitions; part++)
> > +		if (cxlds->part[part].mode == mode)
> > +			break;
> > +
> > +	if (part >= cxlds->nr_partitions) {
> 
> How would it be greater?

It would never be greater in this context, this is just a personal
coding style preference for ">=" identifying the opposite set of numbers
from "<" rather than "==".