RE: [PATCH v3 5/8] cxl: create emulated cxl_hdm for devices that do not have HDM decoders

[Dan Williams <dan.j.williams@intel.com>]

Dave Jiang wrote:
> CXL rev3 spec 8.1.3
> 
> RCDs may not have HDM register blocks. Create a fake HDM with information
> from the CXL PCIe DVSEC registers. The decoder count will be set to the
> HDM count retrieved from the DVSEC cap register.
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> ---
> v3:
> - Move relevant changes from next patch in series. (Jonathan)
> - Add kernel doc update. (Jonathan)
> v2:
> - Set target_count to same as number of ranges. (Jonathan)
> ---
>  drivers/cxl/core/hdm.c |   58 ++++++++++++++++++++++++++++++++++++++++--------
>  drivers/cxl/core/pci.c |    9 +++++--
>  drivers/cxl/cxl.h      |    3 ++
>  drivers/cxl/port.c     |    2 +-
>  4 files changed, 57 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index af1f5f906f52..86fe1be2e961 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -101,11 +101,34 @@ static int map_hdm_decoder_regs(struct cxl_port *port, void __iomem *crb,
>  				      BIT(CXL_CM_CAP_CAP_ID_HDM));
>  }
>  
> +static struct cxl_hdm *devm_cxl_setup_emulated_hdm(struct cxl_port *port,
> +						   struct cxl_endpoint_dvsec_info *info)
> +{
> +	struct device *dev = &port->dev;
> +	struct cxl_hdm *cxlhdm;
> +
> +	if (!info->mem_enabled)
> +		return ERR_PTR(-ENODEV);
> +
> +	cxlhdm = devm_kzalloc(dev, sizeof(*cxlhdm), GFP_KERNEL);
> +	if (!cxlhdm)
> +		return ERR_PTR(-ENOMEM);
> +
> +	cxlhdm->port = port;
> +	cxlhdm->decoder_count = info->ranges;
> +	cxlhdm->target_count = info->ranges;
> +	dev_set_drvdata(&port->dev, cxlhdm);
> +
> +	return cxlhdm;
> +}
> +
>  /**
>   * devm_cxl_setup_hdm - map HDM decoder component registers
>   * @port: cxl_port to map
> + * @info: cached DVSEC range register info
>   */
> -struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port)
> +struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port,
> +				   struct cxl_endpoint_dvsec_info *info)
>  {
>  	struct device *dev = &port->dev;
>  	struct cxl_hdm *cxlhdm;
> @@ -119,6 +142,9 @@ struct cxl_hdm *devm_cxl_setup_hdm(struct cxl_port *port)
>  	cxlhdm->port = port;
>  	crb = ioremap(port->component_reg_phys, CXL_COMPONENT_REG_BLOCK_SIZE);
>  	if (!crb) {
> +		if (info->mem_enabled)
> +			return devm_cxl_setup_emulated_hdm(port, info);
> +
>  		dev_err(dev, "No component registers mapped\n");
>  		return ERR_PTR(-ENXIO);
>  	}
> @@ -815,19 +841,15 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  	return 0;
>  }
>  
> -/**
> - * devm_cxl_enumerate_decoders - add decoder objects per HDM register set
> - * @cxlhdm: Structure to populate with HDM capabilities
> - */
> -int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
> -				struct cxl_endpoint_dvsec_info *info)
> +static void cxl_settle_decoders(struct cxl_hdm *cxlhdm)
>  {
>  	void __iomem *hdm = cxlhdm->regs.hdm_decoder;
> -	struct cxl_port *port = cxlhdm->port;
> -	int i, committed;
> -	u64 dpa_base = 0;
> +	int committed, i;
>  	u32 ctrl;
>  
> +	if (!hdm)
> +		return;

Nice, I like the semantic of a 'struct cxl_hdm' with NULL
regs.hdm_decoder == emulated.

This patch looks good to me.