Re: [PATCH v2 7/8] cxl: Add emulation when HDM decoders are not committed

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

On Mon, 09 Jan 2023 14:44:03 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> For the case where DVSEC range register(s) are active and HDM decoders are
> not committed, use RR to provide emulation. A first pass is done to note
> whether any decoders are committed. If there are no committed endpoint
> decoders, then DVSEC ranges will be used for emulation.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Hi Dave

One trivial suggestion inline. With that tidied up
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/core/hdm.c |   39 ++++++++++++++++++++++++++++++++-------
>  drivers/cxl/cxl.h      |    1 +
>  2 files changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index ed5e9ef3aa9b..40844ff2fe52 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -729,6 +729,33 @@ static int cxl_setup_hdm_decoder_from_dvsec(struct cxl_port *port,
>  	return 0;
>  }
>  
> +static bool should_emulate_decoders(struct cxl_hdm *cxlhdm)
> +{
> +	void __iomem *hdm = cxlhdm->regs.hdm_decoder;
> +	bool committed;
> +	u32 ctrl;
> +	int i;
> +
> +	if (!is_cxl_endpoint(cxlhdm->port))
> +		return false;
> +
> +	if (!hdm)
> +		return true;
> +
> +	/*
> +	 * If any decoders are committed already, there should not be any
> +	 * emulated DVSEC decoders.
> +	 */
> +	for (i = 0; i < cxlhdm->decoder_count; i++) {
> +		ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(i));
> +		committed = !!(ctrl & CXL_HDM_DECODER0_CTRL_COMMITTED);

FIELD_GET() is nicer than !! I think.

Also, local variable isn't useful so I'd get rid of it.

		if (FIELD_GET(ctrl, CXL_HDM_DECODER0_CTRL_COMMITTED))
			return false;


> +		if (committed)
> +			return false;
> +	}
> +
> +	return true;
> +}
> +

>