cxl/region: Refactor cxl_port_setup_targets()
Alison Schofield
maintainerv4 cover cxl/region: Refactor cxl_port_setup_targets()
Changes in v4:
- Pass cxlsd to cxl_switch_port_setup_targets() (DaveJ)
- New Patch 4 from Richard, only calc peer distance on target reuse (RichardC)
- Add function comment to cxl_port_parent_interleave() (RichardC)
- New Patch 8 simplifies control flow (MingL, JonathanC)
- Remove claim in cover letter of a strictly mechanical refactor
- Drop LLM tags per latest guidance
v3: https://lore.kernel.org/linux-cxl/[email protected]/
Changes in v3:
- Drop patch "cxl/region: Name the interleave locals in cxl_port_setup_targets()".
- Move the distance comment above cxl_port_peer_distance() (Jonathan)
- Name the out-params out_iw/out_ig.
- Minor reverse fir and alignment cleanups
- Reorder the cxl_port_calc_interleave() declarations
v2: https://lore.kernel.org/linux-cxl/[email protected]/
Changes in v2:
- New patch 3 adds 'switch' to cxl_port_setup_targets() name (DaveJ)
- Add 'switch' to cxl_port_add_target() name (DaveJ)
- Pass the switch decoder to cxl_switch_port_add_target (DaveJ)
- Minor commit message updates based on function renamings
v1: https://lore.kernel.org/linux-cxl/[email protected]/
Begin Cover Letter:cxl_port_setup_targets() has grown to nearly 200 lines. It open codes each step of adding an endpoint to a port's decoder: finding the distance between peer positions, reading the parent interleave, calculating and validating the port's interleave, setting up the decoder, and adding the target.
This set is inspired by review feedback from Robert Richter on the series that adds mixed-granularity region support [1]. That series needs to change how this function calculates interleaves and Robert asked for that new code to be moved into a helper. Rather than tidy only the new code, this set splits up the whole function first.
Patch 1 prepares the code that moves. It is a suggestion Jonathan offered on the mixed gran series.
Patch 2 renames the function to cxl_switch_port_setup_targets(), since it configures switch decoder targets, and changes its parameter to accept a switch decoder rather than a port, both DaveJ suggestions.
Patches 3-9 extract the individual steps into helpers and add two new patches:
RichardC's patch 4 avoids calculating peer distance when an endpoint adds a new target, and patch 8, suggested by Ming and Jonathan, removes the goto paths before the final helper is extracted.
The result reduces cxl_switch_port_setup_targets() to 53 lines.
That base commit is cxl/next as that seems closest to 7.4-rc1 where I expect to rebase and resend.
[1] https://lore.kernel.org/linux-cxl/[email protected]/
Alison Schofield (8):
cxl: Add interleave encoding validity helpers cxl/region: Rename cxl_port_setup_targets() to
cxl_switch_port_setup_targets()
cxl/region: Factor out cxl_port_peer_distance() cxl/region: Factor out cxl_port_parent_interleave() cxl/region: Factor out cxl_port_calc_interleave() cxl/region: Factor out cxl_port_setup_decoder() cxl/region: Simplify switch port target setup control flow cxl/region: Factor out cxl_switch_port_add_target()
Richard Cheng (1):
cxl/region: Calculate peer distance only for reused targets
drivers/cxl/core/region.c | 243 +++++++++++++++++++++++--------------- drivers/cxl/cxl.h | 21 ++++ 2 files changed, 172 insertions(+), 92 deletions(-)
base-commit: cf15f9f2361dd4002203716b7b878393c0eee4bf
Signature · 1 line
2.37.3
Alison Schofield
maintainerPATCH v4 1/9 cxl: Add interleave encoding validity helpers
Some callers of ways_to_eiw(), granularity_to_eig(), and eig_to_granularity() only need to determine whether an input is valid, but must provide storage for a converted value that is then discarded. This obscures the purpose of the call and requires unnecessary temporary variables.
Define and use new helpers that provide a validation-only interface for those callers.
cxl_port_setup_targets() also encodes and then decodes its calculated granularity solely to validate it. Calculate the granularity directly and validate it instead, making the calculation explicit and removing the encode/decode round trip.
No functional change.
--- drivers/cxl/core/region.c | 49 +++++++++++++-------------------------- drivers/cxl/cxl.h | 21 +++++++++++++++++ 2 files changed, 37 insertions(+), 33 deletions(-)
drivers/cxl/core/region.c +16 -33
@@ -493,11 +493,9 @@ static int set_interleave_ways(struct cxl_region *cxlr, int val)struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;struct cxl_region_params *p = &cxlr->params;int save, rc; -u8 iw; - rc =ways_to_eiw (val, &iw); -if (rc) -return rc; +if (!ways_valid (val)) +return -EINVAL ;/* * Even for x3, x6, and x12 interleaves the region interleave must be a
@@ -566,12 +564,9 @@ static int set_interleave_granularity(struct cxl_region *cxlr, int val)struct cxl_root_decoder *cxlrd = cxlr->cxlrd;struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;struct cxl_region_params *p = &cxlr->params; -int rc; -u16 ig; - rc =granularity_to_eig (val, &ig); -if (rc) -return rc; +if (!granularity_valid (val)) +return -EINVAL ;/* * When the host-bridge is interleaved, disallow region granularity !=
@@ -1451,8 +1446,6 @@ static int cxl_port_setup_targets(struct cxl_port *port,struct cxl_decoder *cxld = cxl_rr->decoder;struct cxl_switch_decoder *cxlsd;struct cxl_port *iter = port; -u16 eig, peig; -u8 eiw, peiw;/* * While root level decoders support x3, x6, x12, switch level
@@ -1524,48 +1517,38 @@ static int cxl_port_setup_targets(struct cxl_port *port, parent_iw = parent_cxld->interleave_ways; } - rc =granularity_to_eig (parent_ig, &peig); -if (rc) { +if (!granularity_valid (parent_ig)) {dev_dbg (&cxlr->dev,"%s:%s: invalid parent granularity: %d\n" ,dev_name (parent_port->uport_dev),dev_name (&parent_port->dev), parent_ig); -return rc; +return -EINVAL ; } - rc =ways_to_eiw (parent_iw, &peiw); -if (rc) { +if (!ways_valid (parent_iw)) {dev_dbg (&cxlr->dev,"%s:%s: invalid parent interleave: %d\n" ,dev_name (parent_port->uport_dev),dev_name (&parent_port->dev), parent_iw); -return rc; +return -EINVAL ; } iw = cxl_rr->nr_targets; - rc =ways_to_eiw (iw, &eiw); -if (rc) { +if (!ways_valid (iw)) {dev_dbg (&cxlr->dev,"%s:%s: invalid port interleave: %d\n" ,dev_name (port->uport_dev),dev_name (&port->dev), iw); -return rc; +return -EINVAL ; }/* * Interleave granularity is a multiple of @parent_port granularity. * Multiplier is the parent port interleave ways. */ - rc =granularity_to_eig (parent_ig * parent_iw, &eig); -if (rc) { + ig = parent_ig * parent_iw; + +if (!granularity_valid (ig)) {dev_dbg (&cxlr->dev,"%s: invalid granularity calculation (%d * %d)\n" ,dev_name (&parent_port->dev), parent_ig, parent_iw); -return rc; - } - - rc =eig_to_granularity (eig, &ig); -if (rc) { -dev_dbg (&cxlr->dev,"%s:%s: invalid interleave: %d\n" , -dev_name (port->uport_dev),dev_name (&port->dev), -256 << eig); -return rc; +return -EINVAL ; }if (iw >8 || iw > cxlsd->nr_targets) {
@@ -3131,13 +3114,13 @@ static bool cxl_is_hpa_in_chunk(u64 hpa, struct cxl_region *cxlr, int pos)*/ int cxl_validate_translation_params (u8 eiw,u16 eig,int pos) { -int ways, gran; +int ways;if (eiw_to_ways (eiw, &ways)) {pr_debug ("%s: invalid eiw=%u\n" , __func__, eiw);return -EINVAL ; } -if (eig_to_granularity (eig, &gran)) { +if (!eig_valid (eig)) {pr_debug ("%s: invalid eig=%u\n" , __func__, eig);return -EINVAL ; }
drivers/cxl/cxl.h +21 -0
@@ -146,6 +146,27 @@ static inline int ways_to_eiw(unsigned int ways, u8 *eiw)return 0 ; } +static inline bool eig_valid (u16 eig) +{ +int granularity; + +return eig_to_granularity (eig, &granularity) ==0 ; +} + +static inline bool granularity_valid (int granularity) +{ +u16 eig; + +return granularity_to_eig (granularity, &eig) ==0 ; +} + +static inline bool ways_valid (unsigned int ways) +{ +u8 eiw; + +return ways_to_eiw (ways, &eiw) ==0 ; +} +/* RAS Registers CXL 2.0 8.2.5.9 CXL RAS Capability Structure */ #define CXL_RAS_UNCORRECTABLE_STATUS_OFFSET 0x0#define CXL_RAS_UNCORRECTABLE_STATUS_MASK (GENMASK(16, 14) | GENMASK(11, 0))
Signature · 1 line
2.37.3
Alison Schofield
maintainerPATCH v4 2/9 cxl/region: Rename cxl_port_setup_targets() to cxl_switch_port_setup_targets()
In reply to Alison Schofieldcxl_port_setup_targets() configures the switch decoder targets for a region. It runs for each switch port, including host bridges, between the root and an endpoint.
Rename the function to cxl_switch_port_setup_targets() to make its scope explicit and pass it the switch decoder so the parameter type enforces that scope.
No functional change.
--- drivers/cxl/core/region.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
drivers/cxl/core/region.c +10 -7
@@ -1432,10 +1432,11 @@ static int check_interleave_cap(struct cxl_decoder *cxld, int iw, int ig)return 0 ; } -static int cxl_port_setup_targets (struct cxl_port *port, -struct cxl_region *cxlr, -struct cxl_endpoint_decoder *cxled) +static int cxl_switch_port_setup_targets (struct cxl_switch_decoder *cxlsd, +struct cxl_region *cxlr, +struct cxl_endpoint_decoder *cxled) { +struct cxl_port *port =to_cxl_port (cxlsd->cxld.dev.parent);struct cxl_root_decoder *cxlrd = cxlr->cxlrd;int parent_iw, parent_ig, ig, iw, rc, pos = cxled->pos;struct cxl_port *parent_port =to_cxl_port (port->dev.parent);
@@ -1443,8 +1444,7 @@ static int cxl_port_setup_targets(struct cxl_port *port,struct cxl_memdev *cxlmd =cxled_to_memdev (cxled);struct cxl_ep *ep =cxl_ep_load (port, cxlmd);struct cxl_region_params *p = &cxlr->params; -struct cxl_decoder *cxld = cxl_rr->decoder; -struct cxl_switch_decoder *cxlsd; +struct cxl_decoder *cxld = &cxlsd->cxld;struct cxl_port *iter = port;/*
@@ -1458,7 +1458,6 @@ static int cxl_port_setup_targets(struct cxl_port *port,return -EINVAL ; } - cxlsd =to_cxl_switch_decoder (&cxld->dev);if (cxl_rr->nr_targets_set) {int i, distance =1 ;struct cxl_region_ref *cxl_rr_iter;
@@ -1716,7 +1715,11 @@ static int cxl_region_setup_targets(struct cxl_region *cxlr)*/ for (ep =cxl_ep_load (iter, cxlmd); iter; iter = ep->next, ep =cxl_ep_load (iter, cxlmd)) { - rc =cxl_port_setup_targets (iter, cxlr, cxled); +struct cxl_region_ref *cxl_rr =cxl_rr_load (iter, cxlr); +struct cxl_switch_decoder *cxlsd; + + cxlsd =to_cxl_switch_decoder (&cxl_rr->decoder->dev); + rc =cxl_switch_port_setup_targets (cxlsd, cxlr, cxled);if (rc) {cxl_region_teardown_targets (cxlr);return rc;
Signature · 1 line
2.37.3
Alison Schofield
maintainerPATCH v4 3/9 cxl/region: Factor out cxl_port_peer_distance()
In reply to Alison Schofieldcxl_switch_port_setup_targets() is nearly 200 lines long and open codes each step of adding an endpoint to a port's decoder. One of those steps calculates the distance between region positions that share a downstream port.
Move that calculation to a new helper, cxl_port_peer_distance().
No functional change.
--- drivers/cxl/core/region.c | 50 +++++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 21 deletions(-)
drivers/cxl/core/region.c +29 -21
@@ -1432,6 +1432,33 @@ static int check_interleave_cap(struct cxl_decoder *cxld, int iw, int ig)return 0 ; } +/* +* The "distance" between peer downstream ports represents which +* endpoint positions in the region interleave a given port can +* host. +* +* For example, at the root of a hierarchy the distance is +* always 1 as every index targets a different host-bridge. At +* each subsequent switch level those ports map every Nth region +* position where N is the width of the switch == distance. +*/ +static int cxl_port_peer_distance (struct cxl_port *port,struct cxl_region *cxlr) +{ +struct cxl_root_decoder *cxlrd = cxlr->cxlrd; +struct cxl_region_ref *cxl_rr_iter; +struct cxl_port *iter = port; +int distance =1 ; + +do { + cxl_rr_iter =cxl_rr_load (iter, cxlr); + distance *= cxl_rr_iter->nr_targets; + iter =to_cxl_port (iter->dev.parent); + }while (!is_cxl_root (iter)); + distance *= cxlrd->cxlsd.cxld.interleave_ways; + +return distance; +} +static int cxl_switch_port_setup_targets (struct cxl_switch_decoder *cxlsd,struct cxl_region *cxlr,struct cxl_endpoint_decoder *cxled)
@@ -1445,7 +1472,6 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd,struct cxl_ep *ep =cxl_ep_load (port, cxlmd);struct cxl_region_params *p = &cxlr->params;struct cxl_decoder *cxld = &cxlsd->cxld; -struct cxl_port *iter = port;/* * While root level decoders support x3, x6, x12, switch level
@@ -1459,27 +1485,9 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, }if (cxl_rr->nr_targets_set) { -int i, distance =1 ; -struct cxl_region_ref *cxl_rr_iter; +int distance =cxl_port_peer_distance (port, cxlr); -/* -* The "distance" between peer downstream ports represents which -* endpoint positions in the region interleave a given port can -* host. -* -* For example, at the root of a hierarchy the distance is -* always 1 as every index targets a different host-bridge. At -* each subsequent switch level those ports map every Nth region -* position where N is the width of the switch == distance. -*/ -do { - cxl_rr_iter =cxl_rr_load (iter, cxlr); - distance *= cxl_rr_iter->nr_targets; - iter =to_cxl_port (iter->dev.parent); - }while (!is_cxl_root (iter)); - distance *= cxlrd->cxlsd.cxld.interleave_ways; - -for (i =0 ; i < cxl_rr->nr_targets_set; i++) +for (int i =0 ; i < cxl_rr->nr_targets_set; i++)if (ep->dport == cxlsd->target[i]) { rc =check_last_peer (cxled, ep, cxl_rr, distance);
Signature · 1 line
2.37.3
Alison Schofield
maintainerPATCH v4 4/9 cxl/region: Calculate peer distance only for reused targets
In reply to Alison SchofieldFrom: Richard Cheng <[email protected]>
cxl_switch_port_setup_targets() calculates peer distance before searching the existing targets. When the endpoint needs a new target, the calculated distance is never used.
Move cxl_port_peer_distance() into the matching-target branch, immediately before check_last_peer(). This avoids unnecessary ancestor walks and per-port region lookups when adding distinct targets.
--- drivers/cxl/core/region.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/cxl/core/region.c +2 -2
@@ -1485,10 +1485,10 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, }if (cxl_rr->nr_targets_set) { -int distance =cxl_port_peer_distance (port, cxlr); -for (int i =0 ; i < cxl_rr->nr_targets_set; i++)if (ep->dport == cxlsd->target[i]) { +int distance =cxl_port_peer_distance (port, cxlr); + rc =check_last_peer (cxled, ep, cxl_rr, distance);if (rc)
Signature · 1 line
2.37.3
Re: [PATCH v4 4/9] cxl/region: Calculate peer distance only for reused targets
Quoted 6 lines · Alison Schofield
From: Richard Cheng <[email protected]>
cxl_switch_port_setup_targets()calculates peer distance before searching the existing targets. When the endpoint needs a new target, the calculated distance is never used.
Quoted 7 lines
Move
cxl_port_peer_distance()into the matching-target branch, immediately beforecheck_last_peer(). This avoids unnecessary ancestor walks and per-port region lookups when adding distinct targets.Signed-off-by Richard Cheng <[email protected]>Tested-by Alison Schofield <[email protected]>Signed-off-by Alison Schofield <[email protected]>
Hi Alison,
Really appreciate for the testing and if we can, can we add the testing result in the commit message demonstrate benefit it can bring us ?
Best regards,
Richard Cheng.
Quoted 24 lines
--- drivers/cxl/core/region.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/cxl/core/region.c +2
-2@@ -1485,10 +1485,10 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, }if (cxl_rr->nr_targets_set) { -int distance =cxl_port_peer_distance (port, cxlr); -for (int i =0 ; i < cxl_rr->nr_targets_set; i++)if (ep->dport == cxlsd->target[i]) { +int distance =cxl_port_peer_distance (port, cxlr); + rc =check_last_peer (cxled, ep, cxl_rr, distance);if (rc)Signature · 1 line
2.37.3
Alison Schofield
maintainerRe: [PATCH v4 4/9] cxl/region: Calculate peer distance only for reused targets
Quoted 10 lines · Richard Cheng
Quoted 9 lines · Alison Schofield
From: Richard Cheng <[email protected]>
cxl_switch_port_setup_targets()calculates peer distance before searching the existing targets. When the endpoint needs a new target, the calculated distance is never used.Move
cxl_port_peer_distance()into the matching-target branch, immediately beforecheck_last_peer(). This avoids unnecessary ancestor walks and per-port region lookups when adding distinct targets.
With cxl_test, the x8 region in cxl-region-sysfs.sh reduces calls to cxl_port_peer_distance() from 10 to 4, and the x2 auto region from 2 to 1. A misordered-endpoint test confirms that peer validation still rejects mismatched peers.
Quoted 9 lines
Quoted 4 lines
Signed-off-by Richard Cheng <[email protected]>Tested-by Alison Schofield <[email protected]>Signed-off-by Alison Schofield <[email protected]>Hi Alison,
Really appreciate for the testing and if we can, can we add the testing result in the commit message demonstrate benefit it can bring us ?
Added the above for v5, taken from:
https://lore.kernel.org/linux-cxl/[email protected]/)
Let me know if you want something else.
-- Alison
Quoted 29 lines
Best regards,
Richard Cheng.Quoted 24 lines
--- drivers/cxl/core/region.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/cxl/core/region.c +2
-2@@ -1485,10 +1485,10 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, }if (cxl_rr->nr_targets_set) { -int distance =cxl_port_peer_distance (port, cxlr); -for (int i =0 ; i < cxl_rr->nr_targets_set; i++)if (ep->dport == cxlsd->target[i]) { +int distance =cxl_port_peer_distance (port, cxlr); + rc =check_last_peer (cxled, ep, cxl_rr, distance);if (rc)Signature · 1 line
2.37.3
Re: [PATCH v4 4/9] cxl/region: Calculate peer distance only for reused targets
In reply to Alison SchofieldQuoted 6 lines · On Sat, 17 Oct 2026 12:19:49 -0700 Alison Schofield…
From: Richard Cheng <[email protected]>
cxl_switch_port_setup_targets()calculates peer distance before searching the existing targets. When the endpoint needs a new target, the calculated distance is never used.
Quoted 7 lines
Move
cxl_port_peer_distance()into the matching-target branch, immediately beforecheck_last_peer(). This avoids unnecessary ancestor walks and per-port region lookups when adding distinct targets.Signed-off-by Richard Cheng <[email protected]>Tested-by Alison Schofield <[email protected]>Signed-off-by Alison Schofield <[email protected]>
I'm assuming the commit message change Alison just posted or something along those lines.
Quoted 21 lines
--- drivers/cxl/core/region.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/cxl/core/region.c +2
-2@@ -1485,10 +1485,10 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, }if (cxl_rr->nr_targets_set) { -int distance =cxl_port_peer_distance (port, cxlr); -for (int i =0 ; i < cxl_rr->nr_targets_set; i++)if (ep->dport == cxlsd->target[i]) { +int distance =cxl_port_peer_distance (port, cxlr); + rc =check_last_peer (cxled, ep, cxl_rr, distance);if (rc)
Re: [PATCH v4 4/9] cxl/region: Calculate peer distance only for reused targets
In reply to Alison SchofieldQuoted 6 lines · Alison Schofield
From: Richard Cheng <[email protected]>
cxl_switch_port_setup_targets()calculates peer distance before searching the existing targets. When the endpoint needs a new target, the calculated distance is never used.
Quoted 7 lines
Move
cxl_port_peer_distance()into the matching-target branch, immediately beforecheck_last_peer(). This avoids unnecessary ancestor walks and per-port region lookups when adding distinct targets.Signed-off-by Richard Cheng <[email protected]>Tested-by Alison Schofield <[email protected]>Signed-off-by Alison Schofield <[email protected]>
Quoted 21 lines
--- drivers/cxl/core/region.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)drivers/cxl/core/region.c +2
-2@@ -1485,10 +1485,10 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, }if (cxl_rr->nr_targets_set) { -int distance =cxl_port_peer_distance (port, cxlr); -for (int i =0 ; i < cxl_rr->nr_targets_set; i++)if (ep->dport == cxlsd->target[i]) { +int distance =cxl_port_peer_distance (port, cxlr); + rc =check_last_peer (cxled, ep, cxl_rr, distance);if (rc)
Alison Schofield
maintainerPATCH v4 5/9 cxl/region: Factor out cxl_port_parent_interleave()
In reply to Alison Schofieldcxl_switch_port_setup_targets() derives a decoder's interleave from the interleave of the decoder above it. That parent interleave comes from either the root decoder or a switch decoder, and each case is open coded along with the parent validity checks.
Move the parent interleave lookup and validation to a new helper, cxl_port_parent_interleave().
No functional change.
--- drivers/cxl/core/region.c | 99 ++++++++++++++++++++++++--------------- 1 file changed, 60 insertions(+), 39 deletions(-)
drivers/cxl/core/region.c +60 -39
@@ -1459,44 +1459,21 @@ static int cxl_port_peer_distance(struct cxl_port *port, struct cxl_region *cxlrreturn distance; } -static int cxl_switch_port_setup_targets (struct cxl_switch_decoder *cxlsd, -struct cxl_region *cxlr, -struct cxl_endpoint_decoder *cxled) +/* +* Get the parent interleave ways and granularity. +* +* For address bit routing, use power-of-2 math for switch ports. +* Root interleave ways of 3, 6, or 12 are normalized to 1, 2, or 4 +* in @out_iw. +*/ +static int cxl_port_parent_interleave (struct cxl_port *port, +struct cxl_region *cxlr, +int *out_iw,int *out_ig) { -struct cxl_port *port =to_cxl_port (cxlsd->cxld.dev.parent); -struct cxl_root_decoder *cxlrd = cxlr->cxlrd; -int parent_iw, parent_ig, ig, iw, rc, pos = cxled->pos;struct cxl_port *parent_port =to_cxl_port (port->dev.parent); -struct cxl_region_ref *cxl_rr =cxl_rr_load (port, cxlr); -struct cxl_memdev *cxlmd =cxled_to_memdev (cxled); -struct cxl_ep *ep =cxl_ep_load (port, cxlmd); +struct cxl_root_decoder *cxlrd = cxlr->cxlrd;struct cxl_region_params *p = &cxlr->params; -struct cxl_decoder *cxld = &cxlsd->cxld; - -/* -* While root level decoders support x3, x6, x12, switch level -* decoders only support powers of 2 up to x16. -*/ -if (!is_power_of_2 (cxl_rr->nr_targets)) { -dev_dbg (&cxlr->dev,"%s:%s: invalid target count %d\n" , -dev_name (port->uport_dev),dev_name (&port->dev), - cxl_rr->nr_targets); -return -EINVAL ; - } - -if (cxl_rr->nr_targets_set) { -for (int i =0 ; i < cxl_rr->nr_targets_set; i++) -if (ep->dport == cxlsd->target[i]) { -int distance =cxl_port_peer_distance (port, cxlr); - - rc =check_last_peer (cxled, ep, cxl_rr, - distance); -if (rc) -return rc; - goto out_target_set; - } - goto add_target; - } +int parent_iw, parent_ig;if (is_cxl_root (parent_port)) {/*
@@ -1508,10 +1485,6 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd,*/ parent_ig = p->interleave_granularity; parent_iw = cxlrd->cxlsd.cxld.interleave_ways; -/* -* For purposes of address bit routing, use power-of-2 math for -* switch ports. -*/ if (!is_power_of_2 (parent_iw)) parent_iw /=3 ; }else {
@@ -1538,6 +1511,54 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd,return -EINVAL ; } + *out_iw = parent_iw; + *out_ig = parent_ig; + +return 0 ; +} + +static int cxl_switch_port_setup_targets (struct cxl_switch_decoder *cxlsd, +struct cxl_region *cxlr, +struct cxl_endpoint_decoder *cxled) +{ +struct cxl_port *port =to_cxl_port (cxlsd->cxld.dev.parent); +int parent_iw, parent_ig, ig, iw, rc, pos = cxled->pos; +struct cxl_port *parent_port =to_cxl_port (port->dev.parent); +struct cxl_region_ref *cxl_rr =cxl_rr_load (port, cxlr); +struct cxl_memdev *cxlmd =cxled_to_memdev (cxled); +struct cxl_ep *ep =cxl_ep_load (port, cxlmd); +struct cxl_region_params *p = &cxlr->params; +struct cxl_decoder *cxld = &cxlsd->cxld; + +/* +* While root level decoders support x3, x6, x12, switch level +* decoders only support powers of 2 up to x16. +*/ +if (!is_power_of_2 (cxl_rr->nr_targets)) { +dev_dbg (&cxlr->dev,"%s:%s: invalid target count %d\n" , +dev_name (port->uport_dev),dev_name (&port->dev), + cxl_rr->nr_targets); +return -EINVAL ; + } + +if (cxl_rr->nr_targets_set) { +for (int i =0 ; i < cxl_rr->nr_targets_set; i++) +if (ep->dport == cxlsd->target[i]) { +int distance =cxl_port_peer_distance (port, cxlr); + + rc =check_last_peer (cxled, ep, cxl_rr, + distance); +if (rc) +return rc; + goto out_target_set; + } + goto add_target; + } + + rc =cxl_port_parent_interleave (port, cxlr, &parent_iw, &parent_ig); +if (rc) +return rc; + iw = cxl_rr->nr_targets;if (!ways_valid (iw)) { dev_dbg(&cxlr->dev, "%s:%s: invalid port interleave: %d\n",
Signature · 1 line
2.37.3
Alison Schofield
maintainerPATCH v4 6/9 cxl/region: Factor out cxl_port_calc_interleave()
In reply to Alison Schofieldcxl_switch_port_setup_targets() calculates the interleave ways and granularity of a port's decoder from the parent interleave and the number of targets below the port, then validates the result. That calculation and its checks are open coded between the target search and the decoder setup.
Move the calculation and its checks to a new helper, cxl_port_calc_interleave(), so that the interleave for the port's decoder is calculated and validated in one place.
No functional change.
--- drivers/cxl/core/region.c | 81 ++++++++++++++++++++++++--------------- 1 file changed, 50 insertions(+), 31 deletions(-)
drivers/cxl/core/region.c +50 -31
@@ -1517,13 +1517,60 @@ static int cxl_port_parent_interleave(struct cxl_port *port,return 0 ; } +static int cxl_port_calc_interleave (struct cxl_port *port,struct cxl_region *cxlr, +struct cxl_region_ref *cxl_rr,int *out_iw, +int *out_ig) +{ +struct cxl_port *parent_port =to_cxl_port (port->dev.parent); +struct cxl_decoder *cxld = cxl_rr->decoder; +int parent_iw, parent_ig, iw, ig, rc; +struct cxl_switch_decoder *cxlsd; + + rc =cxl_port_parent_interleave (port, cxlr, &parent_iw, &parent_ig); +if (rc) +return rc; + + iw = cxl_rr->nr_targets; +if (!ways_valid (iw)) { +dev_dbg (&cxlr->dev,"%s:%s: invalid port interleave: %d\n" , +dev_name (port->uport_dev),dev_name (&port->dev), iw); +return -EINVAL ; + } + +/* +* Interleave granularity is a multiple of @parent_port granularity. +* Multiplier is the parent port interleave ways. +*/ + ig = parent_ig * parent_iw; + +if (!granularity_valid (ig)) { +dev_dbg (&cxlr->dev, +"%s: invalid granularity calculation (%d * %d)\n" , +dev_name (&parent_port->dev), parent_ig, parent_iw); +return -EINVAL ; + } + + cxlsd =to_cxl_switch_decoder (&cxld->dev); +if (iw >8 || iw > cxlsd->nr_targets) { +dev_dbg (&cxlr->dev, +"%s:%s:%s: ways: %d overflows targets: %d\n" , +dev_name (port->uport_dev),dev_name (&port->dev), +dev_name (&cxld->dev), iw, cxlsd->nr_targets); +return -ENXIO ; + } + + *out_iw = iw; + *out_ig = ig; + +return 0 ; +} +static int cxl_switch_port_setup_targets (struct cxl_switch_decoder *cxlsd,struct cxl_region *cxlr,struct cxl_endpoint_decoder *cxled) {struct cxl_port *port =to_cxl_port (cxlsd->cxld.dev.parent); -int parent_iw, parent_ig, ig, iw, rc, pos = cxled->pos; -struct cxl_port *parent_port =to_cxl_port (port->dev.parent); +int iw, ig, rc, pos = cxled->pos;struct cxl_region_ref *cxl_rr =cxl_rr_load (port, cxlr);struct cxl_memdev *cxlmd =cxled_to_memdev (cxled);struct cxl_ep *ep =cxl_ep_load (port, cxlmd);
@@ -1555,38 +1602,10 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, goto add_target; } - rc =cxl_port_parent_interleave (port, cxlr, &parent_iw, &parent_ig); + rc =cxl_port_calc_interleave (port, cxlr, cxl_rr, &iw, &ig);if (rc)return rc; - iw = cxl_rr->nr_targets; -if (!ways_valid (iw)) { -dev_dbg (&cxlr->dev,"%s:%s: invalid port interleave: %d\n" , -dev_name (port->uport_dev),dev_name (&port->dev), iw); -return -EINVAL ; - } - -/* -* Interleave granularity is a multiple of @parent_port granularity. -* Multiplier is the parent port interleave ways. -*/ - ig = parent_ig * parent_iw; - -if (!granularity_valid (ig)) { -dev_dbg (&cxlr->dev, -"%s: invalid granularity calculation (%d * %d)\n" , -dev_name (&parent_port->dev), parent_ig, parent_iw); -return -EINVAL ; - } - -if (iw >8 || iw > cxlsd->nr_targets) { -dev_dbg (&cxlr->dev, -"%s:%s:%s: ways: %d overflows targets: %d\n" , -dev_name (port->uport_dev),dev_name (&port->dev), -dev_name (&cxld->dev), iw, cxlsd->nr_targets); -return -ENXIO ; - } -if (test_bit (CXL_REGION_F_AUTO , &cxlr->flags)) { if (cxld->interleave_ways != iw || (iw > 1 && cxld->interleave_granularity != ig) ||
Signature · 1 line
2.37.3
Alison Schofield
maintainerPATCH v4 7/9 cxl/region: Factor out cxl_port_setup_decoder()
In reply to Alison SchofieldOnce cxl_switch_port_setup_targets() has calculated the interleave for a port's decoder, it either checks that interleave against the decoder settings found for an auto region, or checks the decoder's interleave capabilities and programs the decoder for a user created region. Both paths are open coded in cxl_switch_port_setup_targets().
Move both paths to a new helper, cxl_port_setup_decoder().
No functional change.
--- drivers/cxl/core/region.c | 88 ++++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 38 deletions(-)
drivers/cxl/core/region.c +50 -38
@@ -1565,46 +1565,11 @@ static int cxl_port_calc_interleave(struct cxl_port *port, struct cxl_region *cxreturn 0 ; } -static int cxl_switch_port_setup_targets (struct cxl_switch_decoder *cxlsd, -struct cxl_region *cxlr, -struct cxl_endpoint_decoder *cxled) +static int cxl_port_setup_decoder (struct cxl_port *port,struct cxl_region *cxlr, +struct cxl_decoder *cxld,int iw,int ig) { -struct cxl_port *port =to_cxl_port (cxlsd->cxld.dev.parent); -int iw, ig, rc, pos = cxled->pos; -struct cxl_region_ref *cxl_rr =cxl_rr_load (port, cxlr); -struct cxl_memdev *cxlmd =cxled_to_memdev (cxled); -struct cxl_ep *ep =cxl_ep_load (port, cxlmd);struct cxl_region_params *p = &cxlr->params; -struct cxl_decoder *cxld = &cxlsd->cxld; - -/* -* While root level decoders support x3, x6, x12, switch level -* decoders only support powers of 2 up to x16. -*/ -if (!is_power_of_2 (cxl_rr->nr_targets)) { -dev_dbg (&cxlr->dev,"%s:%s: invalid target count %d\n" , -dev_name (port->uport_dev),dev_name (&port->dev), - cxl_rr->nr_targets); -return -EINVAL ; - } - -if (cxl_rr->nr_targets_set) { -for (int i =0 ; i < cxl_rr->nr_targets_set; i++) -if (ep->dport == cxlsd->target[i]) { -int distance =cxl_port_peer_distance (port, cxlr); - - rc =check_last_peer (cxled, ep, cxl_rr, - distance); -if (rc) -return rc; - goto out_target_set; - } - goto add_target; - } - - rc =cxl_port_calc_interleave (port, cxlr, cxl_rr, &iw, &ig); -if (rc) -return rc; +int rc;if (test_bit (CXL_REGION_F_AUTO , &cxlr->flags)) { if (cxld->interleave_ways != iw ||
@@ -1643,6 +1608,53 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, }dev_dbg (&cxlr->dev,"%s:%s iw: %d ig: %d\n" ,dev_name (port->uport_dev),dev_name (&port->dev), iw, ig); + +return 0 ; +} + +static int cxl_switch_port_setup_targets (struct cxl_switch_decoder *cxlsd, +struct cxl_region *cxlr, +struct cxl_endpoint_decoder *cxled) +{ +struct cxl_port *port =to_cxl_port (cxlsd->cxld.dev.parent); +int iw, ig, rc, pos = cxled->pos; +struct cxl_region_ref *cxl_rr =cxl_rr_load (port, cxlr); +struct cxl_memdev *cxlmd =cxled_to_memdev (cxled); +struct cxl_ep *ep =cxl_ep_load (port, cxlmd); +struct cxl_decoder *cxld = &cxlsd->cxld; + +/* +* While root level decoders support x3, x6, x12, switch level +* decoders only support powers of 2 up to x16. +*/ +if (!is_power_of_2 (cxl_rr->nr_targets)) { +dev_dbg (&cxlr->dev,"%s:%s: invalid target count %d\n" , +dev_name (port->uport_dev),dev_name (&port->dev), + cxl_rr->nr_targets); +return -EINVAL ; + } + +if (cxl_rr->nr_targets_set) { +for (int i =0 ; i < cxl_rr->nr_targets_set; i++) +if (ep->dport == cxlsd->target[i]) { +int distance =cxl_port_peer_distance (port, cxlr); + + rc =check_last_peer (cxled, ep, cxl_rr, + distance); +if (rc) +return rc; + goto out_target_set; + } + goto add_target; + } + + rc =cxl_port_calc_interleave (port, cxlr, cxl_rr, &iw, &ig); +if (rc) +return rc; + + rc =cxl_port_setup_decoder (port, cxlr, cxld, iw, ig); +if (rc) +return rc; add_target:if (cxl_rr->nr_targets_set == cxl_rr->nr_targets) { dev_dbg(&cxlr->dev,
Signature · 1 line
2.37.3
Re: [PATCH v4 7/9] cxl/region: Factor out cxl_port_setup_decoder()
Quoted 6 lines · On Sat, 17 Oct 2026 12:28:53 -0700 Alison Schofield…
Once
cxl_switch_port_setup_targets()has calculated the interleave for a port's decoder, it either checks that interleave against the decoder settings found for an auto region, or checks the decoder's interleave capabilities and programs the decoder for a user created region. Both paths are open coded incxl_switch_port_setup_targets().
Quoted 7 lines
Move both paths to a new helper,
cxl_port_setup_decoder().No functional change.
Signed-off-by Alison Schofield <[email protected]>Reviewed-by Dave Jiang <[email protected]>Reviewed-by Li Ming <[email protected]>
given my feedback on v3 is now in the next patch.
Quoted 112 lines
--- drivers/cxl/core/region.c | 88 ++++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 38 deletions(-)drivers/cxl/core/region.c +50
-38@@ -1565,46 +1565,11 @@ static int cxl_port_calc_interleave(struct cxl_port *port, struct cxl_region *cxreturn 0 ; } -static int cxl_switch_port_setup_targets (struct cxl_switch_decoder *cxlsd, -struct cxl_region *cxlr, -struct cxl_endpoint_decoder *cxled) +static int cxl_port_setup_decoder (struct cxl_port *port,struct cxl_region *cxlr, +struct cxl_decoder *cxld,int iw,int ig) { -struct cxl_port *port =to_cxl_port (cxlsd->cxld.dev.parent); -int iw, ig, rc, pos = cxled->pos; -struct cxl_region_ref *cxl_rr =cxl_rr_load (port, cxlr); -struct cxl_memdev *cxlmd =cxled_to_memdev (cxled); -struct cxl_ep *ep =cxl_ep_load (port, cxlmd);struct cxl_region_params *p = &cxlr->params; -struct cxl_decoder *cxld = &cxlsd->cxld; - -/* -* While root level decoders support x3, x6, x12, switch level -* decoders only support powers of 2 up to x16. -*/ -if (!is_power_of_2 (cxl_rr->nr_targets)) { -dev_dbg (&cxlr->dev,"%s:%s: invalid target count %d\n" , -dev_name (port->uport_dev),dev_name (&port->dev), - cxl_rr->nr_targets); -return -EINVAL ; - } - -if (cxl_rr->nr_targets_set) { -for (int i =0 ; i < cxl_rr->nr_targets_set; i++) -if (ep->dport == cxlsd->target[i]) { -int distance =cxl_port_peer_distance (port, cxlr); - - rc =check_last_peer (cxled, ep, cxl_rr, - distance); -if (rc) -return rc; - goto out_target_set; - } - goto add_target; - } - - rc =cxl_port_calc_interleave (port, cxlr, cxl_rr, &iw, &ig); -if (rc) -return rc; +int rc;if (test_bit (CXL_REGION_F_AUTO , &cxlr->flags)) { if (cxld->interleave_ways != iw ||@@ -1643,6 +1608,53 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, }dev_dbg (&cxlr->dev,"%s:%s iw: %d ig: %d\n" ,dev_name (port->uport_dev),dev_name (&port->dev), iw, ig); + +return 0 ; +} + +static int cxl_switch_port_setup_targets (struct cxl_switch_decoder *cxlsd, +struct cxl_region *cxlr, +struct cxl_endpoint_decoder *cxled) +{ +struct cxl_port *port =to_cxl_port (cxlsd->cxld.dev.parent); +int iw, ig, rc, pos = cxled->pos; +struct cxl_region_ref *cxl_rr =cxl_rr_load (port, cxlr); +struct cxl_memdev *cxlmd =cxled_to_memdev (cxled); +struct cxl_ep *ep =cxl_ep_load (port, cxlmd); +struct cxl_decoder *cxld = &cxlsd->cxld; + +/* +* While root level decoders support x3, x6, x12, switch level +* decoders only support powers of 2 up to x16. +*/ +if (!is_power_of_2 (cxl_rr->nr_targets)) { +dev_dbg (&cxlr->dev,"%s:%s: invalid target count %d\n" , +dev_name (port->uport_dev),dev_name (&port->dev), + cxl_rr->nr_targets); +return -EINVAL ; + } + +if (cxl_rr->nr_targets_set) { +for (int i =0 ; i < cxl_rr->nr_targets_set; i++) +if (ep->dport == cxlsd->target[i]) { +int distance =cxl_port_peer_distance (port, cxlr); + + rc =check_last_peer (cxled, ep, cxl_rr, + distance); +if (rc) +return rc; + goto out_target_set; + } + goto add_target; + } + + rc =cxl_port_calc_interleave (port, cxlr, cxl_rr, &iw, &ig); +if (rc) +return rc; + + rc =cxl_port_setup_decoder (port, cxlr, cxld, iw, ig); +if (rc) +return rc; add_target:if (cxl_rr->nr_targets_set == cxl_rr->nr_targets) { dev_dbg(&cxlr->dev,
Alison Schofield
maintainerPATCH v4 8/9 cxl/region: Simplify switch port target setup control flow
In reply to Alison Schofieldcxl_switch_port_setup_targets() searches the decoder's existing targets for the endpoint's downstream port. On a match it checks the peer position and jumps to an out_target_set label to log the target. Otherwise it jumps to an add_target label, skipping the interleave calculation and decoder setup that only the first endpoint to reach a port needs.
Return the peer check result directly when the downstream port is already a target, and make the first endpoint's decoder setup the else branch of the target search. Both labels go away.
Returning directly after peer validation also removes a misleading debug message that reported a reused target as a new assignment, with a bad index.
--- drivers/cxl/core/region.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-)
drivers/cxl/core/region.c +15 -19
@@ -1635,27 +1635,23 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, }if (cxl_rr->nr_targets_set) { -for (int i =0 ; i < cxl_rr->nr_targets_set; i++) -if (ep->dport == cxlsd->target[i]) { -int distance =cxl_port_peer_distance (port, cxlr); +for (int i =0 ; i < cxl_rr->nr_targets_set; i++) { +if (ep->dport != cxlsd->target[i]) +continue ; - rc =check_last_peer (cxled, ep, cxl_rr, - distance); -if (rc) -return rc; - goto out_target_set; - } - goto add_target; +return check_last_peer (cxled, ep, cxl_rr, +cxl_port_peer_distance (port, cxlr)); + } + }else { + rc =cxl_port_calc_interleave (port, cxlr, cxl_rr, &iw, &ig); +if (rc) +return rc; + + rc =cxl_port_setup_decoder (port, cxlr, cxld, iw, ig); +if (rc) +return rc; } - rc =cxl_port_calc_interleave (port, cxlr, cxl_rr, &iw, &ig); -if (rc) -return rc; - - rc =cxl_port_setup_decoder (port, cxlr, cxld, iw, ig); -if (rc) -return rc; -add_target:if (cxl_rr->nr_targets_set == cxl_rr->nr_targets) { dev_dbg(&cxlr->dev, "%s:%s: targets full trying to add %s:%s at %d\n",
@@ -1677,7 +1673,7 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, cxlsd->cxld.target_map[cxl_rr->nr_targets_set] = ep->dport->port_id; } cxl_rr->nr_targets_set++; -out_target_set: + dev_dbg(&cxlr->dev, "%s:%s target[%d] = %s for %s:%s @ %d\n", dev_name(port->uport_dev), dev_name(&port->dev), cxl_rr->nr_targets_set - 1, dev_name(ep->dport->dport_dev),
Signature · 1 line
2.37.3
Re: [PATCH v4 8/9] cxl/region: Simplify switch port target setup control flow
Quoted 10 lines · On Sat, 17 Oct 2026 12:28:54 -0700 Alison Schofield…
cxl_switch_port_setup_targets()searches the decoder's existing targets for the endpoint's downstream port. On a match it checks the peer position and jumps to anout_target_setlabel to log the target. Otherwise it jumps to anadd_targetlabel, skipping the interleave calculation and decoder setup that only the first endpoint to reach a port needs.Return the peer check result directly when the downstream port is already a target, and make the first endpoint's decoder setup the else branch of the target search. Both labels go away.
Quoted 7 lines
Returning directly after peer validation also removes a misleading debug message that reported a reused target as a new assignment, with a bad index.
Suggested-by Li Ming <[email protected]>Suggested-by Jonathan Cameron <[email protected]>Signed-off-by Alison Schofield <[email protected]>
Re: [PATCH v4 8/9] cxl/region: Simplify switch port target setup control flow
In reply to Alison SchofieldQuoted 10 lines · Alison Schofield
cxl_switch_port_setup_targets()searches the decoder's existing targets for the endpoint's downstream port. On a match it checks the peer position and jumps to anout_target_setlabel to log the target. Otherwise it jumps to anadd_targetlabel, skipping the interleave calculation and decoder setup that only the first endpoint to reach a port needs.Return the peer check result directly when the downstream port is already a target, and make the first endpoint's decoder setup the else branch of the target search. Both labels go away.
Quoted 7 lines
Returning directly after peer validation also removes a misleading debug message that reported a reused target as a new assignment, with a bad index.
Suggested-by Li Ming <[email protected]>Suggested-by Jonathan Cameron <[email protected]>Signed-off-by Alison Schofield <[email protected]>
Quoted 59 lines
--- drivers/cxl/core/region.c | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-)drivers/cxl/core/region.c +15
-19@@ -1635,27 +1635,23 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, }if (cxl_rr->nr_targets_set) { -for (int i =0 ; i < cxl_rr->nr_targets_set; i++) -if (ep->dport == cxlsd->target[i]) { -int distance =cxl_port_peer_distance (port, cxlr); +for (int i =0 ; i < cxl_rr->nr_targets_set; i++) { +if (ep->dport != cxlsd->target[i]) +continue ; - rc =check_last_peer (cxled, ep, cxl_rr, - distance); -if (rc) -return rc; - goto out_target_set; - } - goto add_target; +return check_last_peer (cxled, ep, cxl_rr, +cxl_port_peer_distance (port, cxlr)); + } + }else { + rc =cxl_port_calc_interleave (port, cxlr, cxl_rr, &iw, &ig); +if (rc) +return rc; + + rc =cxl_port_setup_decoder (port, cxlr, cxld, iw, ig); +if (rc) +return rc; } - rc =cxl_port_calc_interleave (port, cxlr, cxl_rr, &iw, &ig); -if (rc) -return rc; - - rc =cxl_port_setup_decoder (port, cxlr, cxld, iw, ig); -if (rc) -return rc; -add_target:if (cxl_rr->nr_targets_set == cxl_rr->nr_targets) { dev_dbg(&cxlr->dev, "%s:%s: targets full trying to add %s:%s at %d\n",@@ -1677,7 +1673,7 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd, cxlsd->cxld.target_map[cxl_rr->nr_targets_set] = ep->dport->port_id; } cxl_rr->nr_targets_set++; -out_target_set: + dev_dbg(&cxlr->dev, "%s:%s target[%d] = %s for %s:%s @ %d\n", dev_name(port->uport_dev), dev_name(&port->dev), cxl_rr->nr_targets_set - 1, dev_name(ep->dport->dport_dev),
Alison Schofield
maintainerPATCH v4 9/9 cxl/region: Factor out cxl_switch_port_add_target()
In reply to Alison Schofieldcxl_switch_port_setup_targets() adds an endpoint's downstream port to a port's decoder target list. For an auto region it checks that the decoder already targets that downstream port at the next position. For a user created region it programs the downstream port at that position. That step is open coded.
Move that step to a new helper, cxl_switch_port_add_target(). The helper takes the switch decoder whose target list it updates, which makes the scope explicit.
No functional change.
--- drivers/cxl/core/region.c | 59 +++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 21 deletions(-)
drivers/cxl/core/region.c +38 -21
@@ -1612,6 +1612,41 @@ static int cxl_port_setup_decoder(struct cxl_port *port, struct cxl_region *cxlrreturn 0 ; } +static int cxl_switch_port_add_target (struct cxl_switch_decoder *cxlsd, +struct cxl_region *cxlr, +struct cxl_region_ref *cxl_rr, +struct cxl_endpoint_decoder *cxled, +struct cxl_ep *ep) +{ +struct cxl_memdev *cxlmd =cxled_to_memdev (cxled); +struct cxl_port *port = cxl_rr->port; +int pos = cxled->pos; + +if (cxl_rr->nr_targets_set == cxl_rr->nr_targets) { +dev_dbg (&cxlr->dev, +"%s:%s: targets full trying to add %s:%s at %d\n" , +dev_name (port->uport_dev),dev_name (&port->dev), +dev_name (&cxlmd->dev),dev_name (&cxled->cxld.dev), pos); +return -ENXIO ; + } +if (test_bit (CXL_REGION_F_AUTO , &cxlr->flags)) { +if (cxlsd->target[cxl_rr->nr_targets_set] != ep->dport) { +dev_dbg (&cxlr->dev,"%s:%s: %s expected %s at %d\n" , +dev_name (port->uport_dev),dev_name (&port->dev), +dev_name (&cxlsd->cxld.dev), +dev_name (ep->dport->dport_dev), + cxl_rr->nr_targets_set); +return -ENXIO ; + } + }else { + cxlsd->target[cxl_rr->nr_targets_set] = ep->dport; + cxlsd->cxld.target_map[cxl_rr->nr_targets_set] = ep->dport->port_id; + } + cxl_rr->nr_targets_set++; + +return 0 ; +} +static int cxl_switch_port_setup_targets (struct cxl_switch_decoder *cxlsd,struct cxl_region *cxlr,struct cxl_endpoint_decoder *cxled)
@@ -1652,27 +1687,9 @@ static int cxl_switch_port_setup_targets(struct cxl_switch_decoder *cxlsd,return rc; } -if (cxl_rr->nr_targets_set == cxl_rr->nr_targets) { -dev_dbg (&cxlr->dev, -"%s:%s: targets full trying to add %s:%s at %d\n" , -dev_name (port->uport_dev),dev_name (&port->dev), -dev_name (&cxlmd->dev),dev_name (&cxled->cxld.dev), pos); -return -ENXIO ; - } -if (test_bit (CXL_REGION_F_AUTO , &cxlr->flags)) { -if (cxlsd->target[cxl_rr->nr_targets_set] != ep->dport) { -dev_dbg (&cxlr->dev,"%s:%s: %s expected %s at %d\n" , -dev_name (port->uport_dev),dev_name (&port->dev), -dev_name (&cxlsd->cxld.dev), -dev_name (ep->dport->dport_dev), - cxl_rr->nr_targets_set); -return -ENXIO ; - } - }else { - cxlsd->target[cxl_rr->nr_targets_set] = ep->dport; - cxlsd->cxld.target_map[cxl_rr->nr_targets_set] = ep->dport->port_id; - } - cxl_rr->nr_targets_set++; + rc =cxl_switch_port_add_target (cxlsd, cxlr, cxl_rr, cxled, ep); +if (rc) +return rc; dev_dbg(&cxlr->dev, "%s:%s target[%d] = %s for %s:%s @ %d\n", dev_name(port->uport_dev), dev_name(&port->dev),
Signature · 1 line
2.37.3
Re: [PATCH v4 9/9] cxl/region: Factor out cxl_switch_port_add_target()
Quoted 10 lines · On Sat, 17 Oct 2026 12:28:55 -0700 Alison Schofield…
cxl_switch_port_setup_targets()adds an endpoint's downstream port to a port's decoder target list. For an auto region it checks that the decoder already targets that downstream port at the next position. For a user created region it programs the downstream port at that position. That step is open coded.Move that step to a new helper,
cxl_switch_port_add_target(). The helper takes the switch decoder whose target list it updates, which makes the scope explicit.
Quoted 5 lines
No functional change.
Signed-off-by Alison Schofield <[email protected]>Reviewed-by Richard Cheng <[email protected]>Reviewed-by Li Ming <[email protected]>
Nice series.
J