Re: [PATCH v4 03/14] cxl/region: Rename misleading variable name @hpa to @hpa_range

[Dave Jiang]

On 11/3/25 11:47 AM, Robert Richter wrote:
> @hpa is actually a @hpa_range, rename variables accordingly.
> 
> Reviewed-by: Gregory Price <gourry@xxxxxxxxxx>
> Signed-off-by: Robert Richter <rrichter@xxxxxxx>
Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>

> ---
>  drivers/cxl/core/region.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index a780e65532a7..bb889c891cf7 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3363,9 +3363,9 @@ static int match_decoder_by_range(struct device *dev, const void *data)
>  }
>  
>  static struct cxl_decoder *
> -cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa)
> +cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa_range)
>  {
> -	struct device *cxld_dev = device_find_child(&port->dev, hpa,
> +	struct device *cxld_dev = device_find_child(&port->dev, hpa_range,
>  						    match_decoder_by_range);
>  
>  	return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL;
> @@ -3378,14 +3378,14 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled)
>  	struct cxl_port *port = cxled_to_port(cxled);
>  	struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
>  	struct cxl_decoder *root, *cxld = &cxled->cxld;
> -	struct range *hpa = &cxld->hpa_range;
> +	struct range *hpa_range = &cxld->hpa_range;
>  
> -	root = cxl_port_find_switch_decoder(&cxl_root->port, hpa);
> +	root = cxl_port_find_switch_decoder(&cxl_root->port, hpa_range);
>  	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),
> -			cxld->hpa_range.start, cxld->hpa_range.end);
> +			hpa_range->start, hpa_range->end);
>  		return NULL;
>  	}
>  
> @@ -3451,7 +3451,7 @@ static int __construct_region(struct cxl_region *cxlr,
>  {
>  	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> -	struct range *hpa = &cxled->cxld.hpa_range;
> +	struct range *hpa_range = &cxled->cxld.hpa_range;
>  	struct cxl_region_params *p;
>  	struct resource *res;
>  	int rc;
> @@ -3467,13 +3467,13 @@ static int __construct_region(struct cxl_region *cxlr,
>  	}
>  
>  	set_bit(CXL_REGION_F_AUTO, &cxlr->flags);
> -	cxlr->hpa_range = *hpa;
> +	cxlr->hpa_range = *hpa_range;
>  
>  	res = kmalloc(sizeof(*res), GFP_KERNEL);
>  	if (!res)
>  		return -ENOMEM;
>  
> -	*res = DEFINE_RES_MEM_NAMED(hpa->start, range_len(hpa),
> +	*res = DEFINE_RES_MEM_NAMED(hpa_range->start, range_len(hpa_range),
>  				    dev_name(&cxlr->dev));
>  
>  	rc = cxl_extended_linear_cache_resize(cxlr, res);
> @@ -3552,11 +3552,12 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>  }
>  
>  static struct cxl_region *
> -cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
> +cxl_find_region_by_range(struct cxl_root_decoder *cxlrd,
> +			 struct range *hpa_range)
>  {
>  	struct device *region_dev;
>  
> -	region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, hpa,
> +	region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, hpa_range,
>  				       match_region_by_range);
>  	if (!region_dev)
>  		return NULL;
> @@ -3566,7 +3567,7 @@ cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
>  
>  int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  {
> -	struct range *hpa = &cxled->cxld.hpa_range;
> +	struct range *hpa_range = &cxled->cxld.hpa_range;
>  	struct cxl_region_params *p;
>  	bool attach = false;
>  	int rc;
> @@ -3577,12 +3578,13 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  		return -ENXIO;
>  
>  	/*
> -	 * Ensure that if multiple threads race to construct_region() for @hpa
> -	 * one does the construction and the others add to that.
> +	 * Ensure that if multiple threads race to construct_region()
> +	 * for the HPA range one does the construction and the others
> +	 * add to that.
>  	 */
>  	mutex_lock(&cxlrd->range_lock);
>  	struct cxl_region *cxlr __free(put_cxl_region) =
> -		cxl_find_region_by_range(cxlrd, hpa);
> +		cxl_find_region_by_range(cxlrd, hpa_range);
>  	if (!cxlr)
>  		cxlr = construct_region(cxlrd, cxled);
>  	mutex_unlock(&cxlrd->range_lock);