Re: [PATCH v2 05/15] cxl/region: Calculate and store the SPA range of an endpoint

[Gregory Price]

On Thu, Feb 20, 2025 at 03:31:45PM -0700, Dave Jiang wrote:
> >  	for (iter = cxled_to_port(cxled); pos >= 0 && iter;
> >  	     iter = parent_port_of(iter))
> >  		pos = cxl_port_calc_interleave(iter, &ctx);
> > @@ -3262,7 +3305,8 @@ static int cxl_endpoint_decoder_initialize(struct cxl_endpoint_decoder *cxled)
> >  {
> >  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> >  	struct cxl_port *iter = cxled_to_port(cxled);
> > -	struct cxl_decoder *root, *cxld = &cxled->cxld;
> > +	struct cxl_port *parent = parent_port_of(iter);
> > +	struct cxl_decoder *cxld = &cxled->cxld;
> >  	struct range hpa = cxld->hpa_range;
> >  	struct cxl_interleave_context ctx;
> >  	int rc;
> > @@ -3271,25 +3315,33 @@ static int cxl_endpoint_decoder_initialize(struct cxl_endpoint_decoder *cxled)
> >  		.hpa_range = &hpa,
> >  	};
> >  
> > -	while (iter && !is_cxl_root(iter)) {
> > +	if (!iter || !parent)
> 
> While parent_port_of() will check NULL and return NULL, it just seems icky checking the pointer after use.
> 

I briefly had the same thought and dug into parent_port_of, erred on the
side of "How many places is cxl_endpoint_decoder_initialize going to be
called?" - but you're probably right, we probably should do the null
check if only for style. 

~Gregory