Re: [PATCH v6 18/27] cxl/hdm: Use stored Component Register mappings to map HDM decoder capability

[Dave Jiang <dave.jiang@intel.com>]

On 6/21/23 20:51, Terry Bowman wrote:
> From: Robert Richter <rrichter@amd.com>
> 
> Now, that the Component Register mappings are stored, use them to
> enable and map the HDM decoder capabilities. The Component Registers
> do not need to be probed again for this, remove probing code.
> 
> The HDM capability applies to Endpoints, USPs and VH Host Bridges. The
> Endpoint's component register mappings are located in the cxlds and
> else in the port's structure. Provide a helper function
> cxl_port_get_comp_map() to locate the mappings depending on the
> component's type.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>   drivers/cxl/core/hdm.c | 59 +++++++++++++++++++++---------------------
>   1 file changed, 30 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 5abfa9276dac..8dcd9f0b22d8 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -81,26 +81,6 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
>   		cxlhdm->interleave_mask |= GENMASK(14, 12);
>   }
>   
> -static int map_hdm_decoder_regs(struct cxl_port *port, void __iomem *crb,
> -				struct cxl_component_regs *regs)
> -{
> -	struct cxl_register_map map = {
> -		.dev = &port->dev,
> -		.resource = port->component_reg_phys,
> -		.base = crb,
> -		.max_size = CXL_COMPONENT_REG_BLOCK_SIZE,
> -	};
> -
> -	cxl_probe_component_regs(&port->dev, crb, &map.component_map);
> -	if (!map.component_map.hdm_decoder.valid) {
> -		dev_dbg(&port->dev, "HDM decoder registers not implemented\n");
> -		/* unique error code to indicate no HDM decoder capability */
> -		return -ENODEV;
> -	}
> -
> -	return cxl_map_component_regs(&map, regs, BIT(CXL_CM_CAP_CAP_ID_HDM));
> -}
> -
>   static bool should_emulate_decoders(struct cxl_endpoint_dvsec_info *info)
>   {
>   	struct cxl_hdm *cxlhdm;
> @@ -145,6 +125,22 @@ static bool should_emulate_decoders(struct cxl_endpoint_dvsec_info *info)
>   	return true;
>   }
>   
> +static struct cxl_register_map *cxl_port_get_comp_map(struct cxl_port *port)
> +{
> +	/*
> +	 * HDM capability applies to Endpoints, USPs and VH Host
> +	 * Bridges. The Endpoint's component register mappings are
> +	 * located in the cxlds.
> +	 */
> +	if (is_cxl_endpoint(port)) {
> +		struct cxl_memdev *memdev = to_cxl_memdev(port->uport_dev);
> +
> +		return &memdev->cxlds->comp_map;
> +	}
> +
> +	return &port->comp_map;
> +}
> +
>   /**
>    * devm_cxl_setup_hdm - map HDM decoder component registers
>    * @port: cxl_port to map
> @@ -155,7 +151,7 @@ struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port,
>   {
>   	struct device *dev = &port->dev;
>   	struct cxl_hdm *cxlhdm;
> -	void __iomem *crb;
> +	struct cxl_register_map *comp_map;
>   	int rc;
>   
>   	cxlhdm = devm_kzalloc(dev, sizeof(*cxlhdm), GFP_KERNEL);
> @@ -164,19 +160,24 @@ struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port,
>   	cxlhdm->port = port;
>   	dev_set_drvdata(dev, cxlhdm);
>   
> -	crb = ioremap(port->component_reg_phys, CXL_COMPONENT_REG_BLOCK_SIZE);
> -	if (!crb && info && info->mem_enabled) {
> -		cxlhdm->decoder_count = info->ranges;
> -		return cxlhdm;
> -	} else if (!crb) {
> +	comp_map = cxl_port_get_comp_map(port);
> +
> +	if (!comp_map->component_map.hdm_decoder.valid) {
> +		dev_dbg(&port->dev, "HDM decoder registers not found\n");
> +		if (info && info->mem_enabled) {
> +			cxlhdm->decoder_count = info->ranges;
> +			return cxlhdm;
> +		}
>   		dev_err(dev, "No component registers mapped\n");
>   		return ERR_PTR(-ENXIO);
>   	}
>   
> -	rc = map_hdm_decoder_regs(port, crb, &cxlhdm->regs);
> -	iounmap(crb);
> -	if (rc)
> +	rc = cxl_map_component_regs(comp_map, &cxlhdm->regs,
> +				    BIT(CXL_CM_CAP_CAP_ID_HDM));
> +	if (rc) {
> +		dev_dbg(dev, "Failed to map HDM capability.\n");
>   		return ERR_PTR(rc);
> +	}
>   
>   	parse_hdm_decoder_caps(cxlhdm);
>   	if (cxlhdm->decoder_count == 0) {