Skip to content

cxl/region: Refactor cxl_port_setup_targets()

Alison Schofield linux-cxl 5 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v5 Latest9 patches 1 replyIn review
  2. v49 patches 8 repliesSuperseded
  3. v37 patches 25 repliesSuperseded
Earlier versions · 2
VersionPostedPatchesRepliesStatus
v28 patches7 repliesSuperseded
v17 patches9 repliesSuperseded

Changes in v5 from the cover letter

- Update patch 4 commit message w test result & benefit info (RichardC)
- Rebase on 7.4-rc1
v4: https://lore.kernel.org/all/[email protected]/

Patches in v5 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/9cxl: Add interleave encoding validity helpersdrivers/cxl/core/region.c drivers/cxl/cxl.h+37 -330 replies
2/9cxl/region: Rename cxl_port_setup_targets() to cxl_switch_port_setup_targets()drivers/cxl/core/region.c+10 -70 replies
3/9cxl/region: Factor out cxl_port_peer_distance()drivers/cxl/core/region.c+29 -210 replies
4/9cxl/region: Calculate peer distance only for reused targetsdrivers/cxl/core/region.c+2 -20 replies
5/9cxl/region: Factor out cxl_port_parent_interleave()drivers/cxl/core/region.c+60 -390 replies
6/9cxl/region: Factor out cxl_port_calc_interleave()drivers/cxl/core/region.c+50 -310 replies
7/9cxl/region: Factor out cxl_port_setup_decoder()drivers/cxl/core/region.c+50 -380 replies
8/9cxl/region: Simplify switch port target setup control flowdrivers/cxl/core/region.c+15 -190 replies
9/9cxl/region: Factor out cxl_switch_port_add_target()drivers/cxl/core/region.c+38 -210 replies

Reviews trailers per patch

7 of 9 patches have a Reviewed-by or Acked-by.

PatchDave JiangJonathan CameronRichard ChengLi MingAlison Schofield
1/9R-b R-b R-b R-b
2/9R-b R-b R-b
3/9R-b R-b R-b R-b
4/9T-b
5/9R-b R-b R-b
6/9R-b R-b R-b R-b
7/9R-b R-b
8/9
9/9R-b R-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes. Scroll the table sideways for all 5 reviewers.

Read the v5 thread

Cover letter [PATCH v5 0/9] cxl/region: Refactor cxl_port_setup_targets()

Changes in v5:
- Update patch 4 commit message w test result & benefit info (RichardC)
- Rebase on 7.4-rc1
v4: https://lore.kernel.org/all/[email protected]/

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.

[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: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

Signature · 1 line
2.37.3