diff mbox series

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

Message ID 8dd4210f464e971a7989c7c923747cf2e10409ef.1697433770.git.alison.schofield@intel.com
State Superseded
Headers show
Series cxl/region: Autodiscovery position repair | expand

Commit Message

Alison Schofield Oct. 16, 2023, 6:02 a.m. UTC
From: Alison Schofield <alison.schofield@intel.com>

match_decoder_by_range() and decoder_match_range() both determine
if an HPA range matches a decoder. The first does it for root
decoders and the second one operates on switch decoders.

Tidy these up with clear naming and make the switch helper more
like the root decoder helper in style and functionality. Make it
take the actual range, rather than an endpoint decoder from which
it extracts the range.

Aside from aesthetics and maintainability, this is in preparation
for reuse.

Fixes: a32320b71f08 ("cxl/region: Add region autodiscovery")
Reported-by: Dmytro Adamenko <dmytro.adamenko@intel.com>
Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 drivers/cxl/core/region.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

Comments

Jim Harris Oct. 17, 2023, 4:21 p.m. UTC | #1
> On Oct 15, 2023, at 11:02 PM, alison.schofield@intel.com wrote:
> 
> From: Alison Schofield <alison.schofield@intel.com>
> 
> match_decoder_by_range() and decoder_match_range() both determine
> if an HPA range matches a decoder. The first does it for root
> decoders and the second one operates on switch decoders.
> 
> Tidy these up with clear naming and make the switch helper more
> like the root decoder helper in style and functionality. Make it
> take the actual range, rather than an endpoint decoder from which
> it extracts the range.
> 
> Aside from aesthetics and maintainability, this is in preparation
> for reuse.
> 
> Fixes: a32320b71f08 ("cxl/region: Add region autodiscovery")
> Reported-by: Dmytro Adamenko <dmytro.adamenko@intel.com>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
> drivers/cxl/core/region.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 6d63b8798c29..64206fc4d99b 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1487,16 +1487,19 @@ static struct cxl_port *next_port(struct cxl_port *port)
> return port->parent_dport->port;
> }
> 
> -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>

> +
> }
> 
> static void find_positions(const struct cxl_switch_decoder *cxlsd,
> @@ -1565,7 +1568,8 @@ static int cmp_decode_pos(const void *a, const void *b)
> goto err;
> }
> 
> - dev = device_find_child(&port->dev, cxled_a, decoder_match_range);
> + dev = device_find_child(&port->dev, &cxled_a->cxld.hpa_range,
> + match_switch_decoder_by_range);
> if (!dev) {
> struct range *range = &cxled_a->cxld.hpa_range;
> 
> @@ -2696,7 +2700,7 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
> return rc;
> }
> 
> -static int match_decoder_by_range(struct device *dev, void *data)
> +static int match_root_decoder_by_range(struct device *dev, void *data)
> {
> struct range *r1, *r2 = data;
> struct cxl_root_decoder *cxlrd;
> @@ -2827,7 +2831,7 @@ int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled)
> int rc;
> 
> cxlrd_dev = device_find_child(&root->dev, &cxld->hpa_range,
> -      match_decoder_by_range);
> +      match_root_decoder_by_range);
> if (!cxlrd_dev) {
> dev_err(cxlmd->dev.parent,
> "%s:%s no CXL window for range %#llx:%#llx\n",
> -- 
> 2.37.3
> 
>
Jim Harris Oct. 17, 2023, 5:24 p.m. UTC | #2
> 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.
Alison Schofield Oct. 17, 2023, 8:43 p.m. UTC | #3
On Tue, Oct 17, 2023 at 04:21:06PM +0000, Jim Harris wrote:
> 
> 
> > On Oct 15, 2023, at 11:02 PM, alison.schofield@intel.com wrote:
> > 
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > match_decoder_by_range() and decoder_match_range() both determine
> > if an HPA range matches a decoder. The first does it for root
> > decoders and the second one operates on switch decoders.
> > 
> > Tidy these up with clear naming and make the switch helper more
> > like the root decoder helper in style and functionality. Make it
> > take the actual range, rather than an endpoint decoder from which
> > it extracts the range.
> > 
> > Aside from aesthetics and maintainability, this is in preparation
> > for reuse.
> > 
> > Fixes: a32320b71f08 ("cxl/region: Add region autodiscovery")
> > Reported-by: Dmytro Adamenko <dmytro.adamenko@intel.com>
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> > drivers/cxl/core/region.c | 16 ++++++++++------
> > 1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 6d63b8798c29..64206fc4d99b 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -1487,16 +1487,19 @@ static struct cxl_port *next_port(struct cxl_port *port)
> > return port->parent_dport->port;
> > }
> > 
> > -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>
>

Thanks for the review.

Your replies are removing the leading indentation from the original
patch. I see it viewing in my mailer and w browser:
https://lore.kernel.org/linux-cxl/cover.1697433770.git.alison.schofield@intel.com/T/#m1d4fba325a175520b8a43964ae44a28424177448

So, I think it's something on your end.

???

Alison

> > +
> > }
> > 
> > static void find_positions(const struct cxl_switch_decoder *cxlsd,
> > @@ -1565,7 +1568,8 @@ static int cmp_decode_pos(const void *a, const void *b)
> > goto err;
> > }
> > 
> > - dev = device_find_child(&port->dev, cxled_a, decoder_match_range);
> > + dev = device_find_child(&port->dev, &cxled_a->cxld.hpa_range,
> > + match_switch_decoder_by_range);
> > if (!dev) {
> > struct range *range = &cxled_a->cxld.hpa_range;
> > 
> > @@ -2696,7 +2700,7 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
> > return rc;
> > }
> > 
> > -static int match_decoder_by_range(struct device *dev, void *data)
> > +static int match_root_decoder_by_range(struct device *dev, void *data)
> > {
> > struct range *r1, *r2 = data;
> > struct cxl_root_decoder *cxlrd;
> > @@ -2827,7 +2831,7 @@ int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled)
> > int rc;
> > 
> > cxlrd_dev = device_find_child(&root->dev, &cxld->hpa_range,
> > -      match_decoder_by_range);
> > +      match_root_decoder_by_range);
> > if (!cxlrd_dev) {
> > dev_err(cxlmd->dev.parent,
> > "%s:%s no CXL window for range %#llx:%#llx\n",
> > -- 
> > 2.37.3
> > 
> > 
>
Jim Harris Oct. 17, 2023, 10:59 p.m. UTC | #4
On Tue, Oct 17, 2023 at 01:43:15PM -0700, Alison Schofield wrote:
> On Tue, Oct 17, 2023 at 04:21:06PM +0000, Jim Harris wrote:
> > 
> > 
> > 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>
> >
> 
> Thanks for the review.
> 
> Your replies are removing the leading indentation from the original
> patch. I see it viewing in my mailer and w browser:
> 
> So, I think it's something on your end.
> 
> ???
> 
> Alison
> 

My apologies, I'll stop trying to get macOS Mail to work with the
mailing list.

(Sent with mutt)
Alison Schofield Oct. 23, 2023, 5:51 p.m. UTC | #5
On Tue, Oct 17, 2023 at 04:21:06PM +0000, Jim Harris wrote:
> 
> 
> > On Oct 15, 2023, at 11:02 PM, alison.schofield@intel.com wrote:
> > 
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > match_decoder_by_range() and decoder_match_range() both determine
> > if an HPA range matches a decoder. The first does it for root
> > decoders and the second one operates on switch decoders.
> > 
> > Tidy these up with clear naming and make the switch helper more
> > like the root decoder helper in style and functionality. Make it
> > take the actual range, rather than an endpoint decoder from which
> > it extracts the range.
> > 
> > Aside from aesthetics and maintainability, this is in preparation
> > for reuse.
> > 
> > Fixes: a32320b71f08 ("cxl/region: Add region autodiscovery")
> > Reported-by: Dmytro Adamenko <dmytro.adamenko@intel.com>
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> > drivers/cxl/core/region.c | 16 ++++++++++------
> > 1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 6d63b8798c29..64206fc4d99b 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -1487,16 +1487,19 @@ static struct cxl_port *next_port(struct cxl_port *port)
> > return port->parent_dport->port;
> > }
> > 
> > -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.

Got it, thanks!


> 
> Rest of the patch looks good, I agree the naming is much better with these
> changes.
> 
> Reviewed-by: Jim Harris <jim.harris@samsung.com>
> 
> > +
> > }
> > 
> > static void find_positions(const struct cxl_switch_decoder *cxlsd,
> > @@ -1565,7 +1568,8 @@ static int cmp_decode_pos(const void *a, const void *b)
> > goto err;
> > }
> > 
> > - dev = device_find_child(&port->dev, cxled_a, decoder_match_range);
> > + dev = device_find_child(&port->dev, &cxled_a->cxld.hpa_range,
> > + match_switch_decoder_by_range);
> > if (!dev) {
> > struct range *range = &cxled_a->cxld.hpa_range;
> > 
> > @@ -2696,7 +2700,7 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
> > return rc;
> > }
> > 
> > -static int match_decoder_by_range(struct device *dev, void *data)
> > +static int match_root_decoder_by_range(struct device *dev, void *data)
> > {
> > struct range *r1, *r2 = data;
> > struct cxl_root_decoder *cxlrd;
> > @@ -2827,7 +2831,7 @@ int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled)
> > int rc;
> > 
> > cxlrd_dev = device_find_child(&root->dev, &cxld->hpa_range,
> > -      match_decoder_by_range);
> > +      match_root_decoder_by_range);
> > if (!cxlrd_dev) {
> > dev_err(cxlmd->dev.parent,
> > "%s:%s no CXL window for range %#llx:%#llx\n",
> > -- 
> > 2.37.3
> > 
> > 
>
Dan Williams Oct. 23, 2023, 8:54 p.m. UTC | #6
alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> match_decoder_by_range() and decoder_match_range() both determine
> if an HPA range matches a decoder. The first does it for root
> decoders and the second one operates on switch decoders.
> 
> Tidy these up with clear naming and make the switch helper more
> like the root decoder helper in style and functionality. Make it
> take the actual range, rather than an endpoint decoder from which
> it extracts the range.
> 
> Aside from aesthetics and maintainability, this is in preparation
> for reuse.

Some minor changelog comments:

> 
> Fixes: a32320b71f08 ("cxl/region: Add region autodiscovery")

Save "Fixes:" for commits that need to be backported to address an
issue. This will be a dependency for backporting patch2, but on its own
it does not fix anything.

> Reported-by: Dmytro Adamenko <dmytro.adamenko@intel.com>

Along the same lines this probably moves to the patch that fixes the
report.

> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  drivers/cxl/core/region.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 6d63b8798c29..64206fc4d99b 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1487,16 +1487,19 @@ static struct cxl_port *next_port(struct cxl_port *port)
>  	return port->parent_dport->port;
>  }
>  
> -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);

I know this is just maintaining the status quo, but thinking about it a
bit deeper I think range_contains() is only correct for root decoders.
For a given region the root decoder will contain the region, but all the
switch decoders must be a 1:1 mapping. Linux only supports scenarios
where all endpoint decoders map the same HPA range and identify only 1
decoder per-switch port to map that region.

So "range_contains(r1, r2)" becomes "return *r1 == *r2".
Alison Schofield Oct. 23, 2023, 11:22 p.m. UTC | #7
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.


>
Alison Schofield Oct. 23, 2023, 11:30 p.m. UTC | #8
On Mon, Oct 23, 2023 at 01:54:26PM -0700, Dan Williams wrote:
> alison.schofield@ wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > match_decoder_by_range() and decoder_match_range() both determine
> > if an HPA range matches a decoder. The first does it for root
> > decoders and the second one operates on switch decoders.
> > 
> > Tidy these up with clear naming and make the switch helper more
> > like the root decoder helper in style and functionality. Make it
> > take the actual range, rather than an endpoint decoder from which
> > it extracts the range.
> > 
> > Aside from aesthetics and maintainability, this is in preparation
> > for reuse.
> 
> Some minor changelog comments:
> 
> > 
> > Fixes: a32320b71f08 ("cxl/region: Add region autodiscovery")
> 
> Save "Fixes:" for commits that need to be backported to address an
> issue. This will be a dependency for backporting patch2, but on its own
> it does not fix anything.
> 
> > Reported-by: Dmytro Adamenko <dmytro.adamenko@intel.com>
> 
> Along the same lines this probably moves to the patch that fixes the
> report.
> 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  drivers/cxl/core/region.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)

snip

> > 
> > +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);
> 
> I know this is just maintaining the status quo, but thinking about it a
> bit deeper I think range_contains() is only correct for root decoders.
> For a given region the root decoder will contain the region, but all the
> switch decoders must be a 1:1 mapping. Linux only supports scenarios
> where all endpoint decoders map the same HPA range and identify only 1
> decoder per-switch port to map that region.
> 
> So "range_contains(r1, r2)" becomes "return *r1 == *r2".

Thanks Dan - picked up your changelog suggestions and replace this
range_contains w an 1:1 comparison.

Alison
diff mbox series

Patch

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 6d63b8798c29..64206fc4d99b 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1487,16 +1487,19 @@  static struct cxl_port *next_port(struct cxl_port *port)
 	return port->parent_dport->port;
 }
 
-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);
+}
+
 }
 
 static void find_positions(const struct cxl_switch_decoder *cxlsd,
@@ -1565,7 +1568,8 @@  static int cmp_decode_pos(const void *a, const void *b)
 		goto err;
 	}
 
-	dev = device_find_child(&port->dev, cxled_a, decoder_match_range);
+	dev = device_find_child(&port->dev, &cxled_a->cxld.hpa_range,
+				match_switch_decoder_by_range);
 	if (!dev) {
 		struct range *range = &cxled_a->cxld.hpa_range;
 
@@ -2696,7 +2700,7 @@  static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
 	return rc;
 }
 
-static int match_decoder_by_range(struct device *dev, void *data)
+static int match_root_decoder_by_range(struct device *dev, void *data)
 {
 	struct range *r1, *r2 = data;
 	struct cxl_root_decoder *cxlrd;
@@ -2827,7 +2831,7 @@  int cxl_add_to_region(struct cxl_port *root, struct cxl_endpoint_decoder *cxled)
 	int rc;
 
 	cxlrd_dev = device_find_child(&root->dev, &cxld->hpa_range,
-				      match_decoder_by_range);
+				      match_root_decoder_by_range);
 	if (!cxlrd_dev) {
 		dev_err(cxlmd->dev.parent,
 			"%s:%s no CXL window for range %#llx:%#llx\n",