Re: [PATCH v2 1/3] cxl/region: Prepare the decoder match range helper for reuse

[Alison Schofield <alison.schofield@intel.com>]

On Tue, Oct 17, 2023 at 05:24:45PM +0000, Jim Harris wrote:
> 
> 
> > On Oct 17, 2023, at 9:20 AM, Jim Harris <jim.harris@samsung.com> wrote:
> > 
> > 
> > 
> >> On Oct 15, 2023, at 11:02 PM, alison.schofield@intel.com wrote:
> >> 
> >> From: Alison Schofield <alison.schofield@intel.com>
> >> 
> >> 
> >> -static int decoder_match_range(struct device *dev, void *data)
> >> +static int match_switch_decoder_by_range(struct device *dev, void *data)
> >> {
> >> - struct cxl_endpoint_decoder *cxled = data;
> >> + struct range *r1, *r2 = data;
> >> struct cxl_switch_decoder *cxlsd;
> >> 
> >> if (!is_switch_decoder(dev))
> >> return 0;
> >> 
> >> cxlsd = to_cxl_switch_decoder(dev);
> >> - return range_contains(&cxlsd->cxld.hpa_range, &cxled->cxld.hpa_range);
> >> + r1 = &cxlsd->cxld.hpa_range;
> >> + return range_contains(r1, r2);
> >> +}
> > 
> > Hi Alison,
> > 
> > This stray closing brace needs to be removed from this patch.
> > 
> > Rest of the patch looks good, I agree the naming is much better with these
> > changes.
> > 
> > Reviewed-by: Jim Harris <jim.harris@samsung.com>
> > 
> 
> Looking at this again, I think this also needs to check if the switch
> decoder is active. Non-active decoders will have range 0 to UINT64_MAX
> which would pass the range_contains() check.

Combining this and Dan's input, changing this to check for an exact
match on the switch decoder, so not worry about the uncommitteds.


>