| Message ID | 20250211095349.981096-14-rrichter@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: Address translation support, part 1: Cleanups and refactoring | expand |
On Tue, 11 Feb 2025 10:53:43 +0100 Robert Richter <rrichter@amd.com> wrote: > Factor out code to find the switch decoder of a port for a specific > address range. Reuse the code to search a root decoder, create the > function cxl_port_find_switch_decoder() and rework > match_root_decoder_by_range() to be usable for switch decoders too. > > Signed-off-by: Robert Richter <rrichter@amd.com> > Reviewed-by: Gregory Price <gourry@gourry.net> > Tested-by: Gregory Price <gourry@gourry.net> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
On 2/11/25 2:53 AM, Robert Richter wrote: > Factor out code to find the switch decoder of a port for a specific > address range. Reuse the code to search a root decoder, create the > function cxl_port_find_switch_decoder() and rework > match_root_decoder_by_range() to be usable for switch decoders too. > > Signed-off-by: Robert Richter <rrichter@amd.com> > Reviewed-by: Gregory Price <gourry@gourry.net> > Tested-by: Gregory Price <gourry@gourry.net> Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/cxl/core/region.c | 48 +++++++++++++++++++++++---------------- > 1 file changed, 28 insertions(+), 20 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index cfcd235f311e..72e991e7d9ab 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -3189,28 +3189,44 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr) > return rc; > } > > -static int match_root_decoder_by_range(struct device *dev, > - const void *data) > +static int match_decoder_by_range(struct device *dev, const void *data) > { > const struct range *r1, *r2 = data; > - struct cxl_root_decoder *cxlrd; > + struct cxl_decoder *cxld; > > - if (!is_root_decoder(dev)) > + if (!is_switch_decoder(dev)) > return 0; > > - cxlrd = to_cxl_root_decoder(dev); > - r1 = &cxlrd->cxlsd.cxld.hpa_range; > + cxld = to_cxl_decoder(dev); > + r1 = &cxld->hpa_range; > return range_contains(r1, r2); > } > > +static struct cxl_decoder * > +cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa) > +{ > + /* > + * device_find_child() increments the reference count of the > + * the switch decoder's parent port to protect the reference > + * to its child. The port is already a parent of the endpoint > + * decoder's port, at least indirectly in the port hierarchy. > + * Thus, the endpoint already holds a reference for the parent > + * port of the switch decoder. Free the unnecessary reference > + * here. > + */ > + struct device *cxld_dev __free(put_device) = > + device_find_child(&port->dev, hpa, match_decoder_by_range); > + > + return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL; > +} > + > static struct cxl_root_decoder * > cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) > { > struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); > struct cxl_port *iter = cxled_to_port(cxled); > - struct cxl_decoder *cxld = &cxled->cxld; > + struct cxl_decoder *root, *cxld = &cxled->cxld; > struct range *hpa = &cxld->hpa_range; > - struct device *cxlrd_dev; > > while (iter && !is_cxl_root(iter)) > iter = to_cxl_port(iter->dev.parent); > @@ -3218,9 +3234,8 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) > if (!iter) > return NULL; > > - cxlrd_dev = device_find_child(&iter->dev, hpa, > - match_root_decoder_by_range); > - if (!cxlrd_dev) { > + root = cxl_port_find_switch_decoder(iter, hpa); > + if (!root) { > dev_err(cxlmd->dev.parent, > "%s:%s no CXL window for range %#llx:%#llx\n", > dev_name(&cxlmd->dev), dev_name(&cxld->dev), > @@ -3228,16 +3243,9 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) > return NULL; > } > > - /* > - * device_find_child() created a reference to the root > - * decoder. Since the root decoder exists as long as the root > - * port exists and the endpoint already holds a reference to > - * the root port, this additional reference is not needed. > - * Free it here. > - */ > - put_device(cxlrd_dev); > > - return to_cxl_root_decoder(cxlrd_dev); > + > + return to_cxl_root_decoder(&root->dev); > } > > static int match_region_by_range(struct device *dev, const void *data)
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index cfcd235f311e..72e991e7d9ab 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -3189,28 +3189,44 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr) return rc; } -static int match_root_decoder_by_range(struct device *dev, - const void *data) +static int match_decoder_by_range(struct device *dev, const void *data) { const struct range *r1, *r2 = data; - struct cxl_root_decoder *cxlrd; + struct cxl_decoder *cxld; - if (!is_root_decoder(dev)) + if (!is_switch_decoder(dev)) return 0; - cxlrd = to_cxl_root_decoder(dev); - r1 = &cxlrd->cxlsd.cxld.hpa_range; + cxld = to_cxl_decoder(dev); + r1 = &cxld->hpa_range; return range_contains(r1, r2); } +static struct cxl_decoder * +cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa) +{ + /* + * device_find_child() increments the reference count of the + * the switch decoder's parent port to protect the reference + * to its child. The port is already a parent of the endpoint + * decoder's port, at least indirectly in the port hierarchy. + * Thus, the endpoint already holds a reference for the parent + * port of the switch decoder. Free the unnecessary reference + * here. + */ + struct device *cxld_dev __free(put_device) = + device_find_child(&port->dev, hpa, match_decoder_by_range); + + return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL; +} + static struct cxl_root_decoder * cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) { struct cxl_memdev *cxlmd = cxled_to_memdev(cxled); struct cxl_port *iter = cxled_to_port(cxled); - struct cxl_decoder *cxld = &cxled->cxld; + struct cxl_decoder *root, *cxld = &cxled->cxld; struct range *hpa = &cxld->hpa_range; - struct device *cxlrd_dev; while (iter && !is_cxl_root(iter)) iter = to_cxl_port(iter->dev.parent); @@ -3218,9 +3234,8 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) if (!iter) return NULL; - cxlrd_dev = device_find_child(&iter->dev, hpa, - match_root_decoder_by_range); - if (!cxlrd_dev) { + root = cxl_port_find_switch_decoder(iter, hpa); + if (!root) { dev_err(cxlmd->dev.parent, "%s:%s no CXL window for range %#llx:%#llx\n", dev_name(&cxlmd->dev), dev_name(&cxld->dev), @@ -3228,16 +3243,9 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled) return NULL; } - /* - * device_find_child() created a reference to the root - * decoder. Since the root decoder exists as long as the root - * port exists and the endpoint already holds a reference to - * the root port, this additional reference is not needed. - * Free it here. - */ - put_device(cxlrd_dev); - return to_cxl_root_decoder(cxlrd_dev); + + return to_cxl_root_decoder(&root->dev); } static int match_region_by_range(struct device *dev, const void *data)