| Message ID | 20250306164448.3354845-6-rrichter@amd.com |
|---|---|
| State | New |
| Headers | show |
| Series | cxl: Address translation support, part 1: Cleanups and refactoring | expand |
Robert Richter wrote: > Current function cxl_region_find_decoder() is used to find a port's > decoder during region setup. The decoder is later used to attach the > port to a region. > > Rename function to cxl_find_decoder_early() to emphasize its use only > during region setup in the early pre-init setup stage when an endpoint > is not yet attached to a region (cxl_region_attach()). Once a port is > attached to a region, the region reference can be used to lookup a > region's port and decoder configuration (see struct cxl_region_ref). > > Signed-off-by: Robert Richter <rrichter@amd.com> > Reviewed-by: Gregory Price <gourry@gourry.net> > Tested-by: Gregory Price <gourry@gourry.net> "_early" to me has always meant "before some other subsystem dependency is available", like the "page allocator not initialized". So I do not think this name helps readability. > --- > drivers/cxl/core/region.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c > index 4f79cc17c9c8..e40ae0fefddc 100644 > --- a/drivers/cxl/core/region.c > +++ b/drivers/cxl/core/region.c > @@ -865,10 +865,18 @@ static int match_auto_decoder(struct device *dev, const void *data) > return 0; > } > > +/* > + * Only use cxl_find_decoder_early() during region setup in the early > + * pre-init setup stage when an endpoint is not yet attached to a > + * region (cxl_region_attach()). Once a port is attached to a region, > + * the region reference can be used to lookup a region's port and > + * decoder configuration (see struct cxl_region_ref). > +*/ I like the sentiment that this function is only for setup paths before the given port has a cxl_region_ref established for the region, but "pre-init" is not defined. Let's just call this function what it is and document when to use without making the reader of this comment wonder what "pre-init" means. Something like: /** * cxl_port_pick_region_decoder() - assign or lookup a decoder for a region * @port: a port in the ancestry of the endpoint implied by @cxled * @cxled: endpoint decoder to be, or currently, mapped by @port * @cxlr: region to establish, or validate, decode @port * * In the region creation path cxl_port_pick_region_decoder() is an * allocator to find a free port. In the region assembly path, it is * recalling the decoder that platform firmware picked for validation * purposes. * * The result is recorded in a 'struct cxl_region_ref' in @port for * later recall when other endpoints might also be targets of the picked * decoder. */
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c index 4f79cc17c9c8..e40ae0fefddc 100644 --- a/drivers/cxl/core/region.c +++ b/drivers/cxl/core/region.c @@ -865,10 +865,18 @@ static int match_auto_decoder(struct device *dev, const void *data) return 0; } +/* + * Only use cxl_find_decoder_early() during region setup in the early + * pre-init setup stage when an endpoint is not yet attached to a + * region (cxl_region_attach()). Once a port is attached to a region, + * the region reference can be used to lookup a region's port and + * decoder configuration (see struct cxl_region_ref). +*/ + static struct cxl_decoder * -cxl_region_find_decoder(struct cxl_port *port, - struct cxl_endpoint_decoder *cxled, - struct cxl_region *cxlr) +cxl_find_decoder_early(struct cxl_port *port, + struct cxl_endpoint_decoder *cxled, + struct cxl_region *cxlr) { struct device *dev; @@ -932,7 +940,7 @@ alloc_region_ref(struct cxl_port *port, struct cxl_region *cxlr, if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) { struct cxl_decoder *cxld; - cxld = cxl_region_find_decoder(port, cxled, cxlr); + cxld = cxl_find_decoder_early(port, cxled, cxlr); if (auto_order_ok(port, iter->region, cxld)) continue; } @@ -1020,7 +1028,7 @@ static int cxl_rr_alloc_decoder(struct cxl_port *port, struct cxl_region *cxlr, { struct cxl_decoder *cxld; - cxld = cxl_region_find_decoder(port, cxled, cxlr); + cxld = cxl_find_decoder_early(port, cxled, cxlr); if (!cxld) { dev_dbg(&cxlr->dev, "%s: no decoder available\n", dev_name(&port->dev));