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

Gregory Price
2025-02-20T22:37:25Z

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