Re: [PATCH v3 10/14] cxl/region: Collect host bridge decoders

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 27 Jan 2022 16:27:03 -0800
Ben Widawsky <ben.widawsky@intel.com> wrote:

> Part of host bridge verification in the CXL Type 3 Memory Device
> Software Guide calculates the host bridge interleave target list (6th
> step in the flow chart), ie. verification and state update are done in
> the same step. Host bridge verification is already in place, so go ahead
> and store the decoders with their target lists.
> 
> Switches are implemented in a separate patch.
> 
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>
Looks like a little bit of code got in here that I think
belongs in a different patch.

> ---
>  drivers/cxl/region.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/region.c b/drivers/cxl/region.c
> index 145d7bb02714..b8982be13bfe 100644
> --- a/drivers/cxl/region.c
> +++ b/drivers/cxl/region.c
> @@ -428,6 +428,7 @@ static bool region_hb_rp_config_valid(struct cxl_region *cxlr,
>  		return simple_config(cxlr, hbs[0]);
>  
>  	for (i = 0; i < hb_count; i++) {
> +		struct cxl_decoder *cxld;
>  		int idx, position_mask;
>  		struct cxl_dport *rp;
>  		struct cxl_port *hb;
> @@ -486,6 +487,18 @@ static bool region_hb_rp_config_valid(struct cxl_region *cxlr,
>  						"One or more devices are not connected to the correct Host Bridge Root Port\n");
>  					goto err;
>  				}
> +
> +				if (!state_update)
> +					continue;
> +
> +				if (dev_WARN_ONCE(&cxld->dev,
> +						  port_grouping >= cxld->nr_targets,
> +						  "Invalid port grouping %d/%d\n",
> +						  port_grouping, cxld->nr_targets))
> +					goto err;
> +
> +				cxld->interleave_ways++;
> +				cxld->target[port_grouping] = get_rp(ep);
>  			}
>  		}
>  	}
> @@ -538,7 +551,7 @@ static bool rootd_valid(const struct cxl_region *cxlr,
>  
>  struct rootd_context {
>  	const struct cxl_region *cxlr;
> -	struct cxl_port *hbs[CXL_DECODER_MAX_INTERLEAVE];
> +	const struct cxl_port *hbs[CXL_DECODER_MAX_INTERLEAVE];
>  	int count;
>  };
>  
> @@ -564,7 +577,7 @@ static struct cxl_decoder *find_rootd(const struct cxl_region *cxlr,
>  	struct rootd_context ctx;
>  	struct device *ret;
>  
> -	ctx.cxlr = cxlr;
> +	ctx.cxlr = (struct cxl_region *)cxlr;

Why is this here?  If it's needed, that need doesn't seem to have come in
as part of this patch.

>  
>  	ret = device_find_child((struct device *)&root->dev, &ctx, rootd_match);
>  	if (ret)