Re: [PATCH v2 6/6] cxl/mem: Replace redundant debug message with a comment

From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
To: Dan Williams <dan.j.williams@intel.com>
Cc: <linux-cxl@vger.kernel.org>, <ben.widawsky@intel.com>,
	<ira.weiny@intel.com>, <vishal.l.verma@intel.com>,
	<alison.schofield@intel.com>
Subject: Re: [PATCH v2 6/6] cxl/mem: Replace redundant debug message with a comment
Date: Fri, 25 Mar 2022 11:54:45 +0000	[thread overview]
Message-ID: <20220325115445.00000e4f@huawei.com> (raw)
In-Reply-To: <164730736948.3806189.17828261054974867700.stgit@dwillia2-desk3.amr.corp.intel.com>

On Mon, 14 Mar 2022 18:22:49 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> cxl_mem_probe() already emits a log message when HDM operation can not
> be established. Delete the similar one in cxl_hdm_decode_init().
> 
> What is less obvious is why global_ctrl being enabled makes positive
> values of info->ranges irrelevant.
> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Assuming comment on ignore recommendation added for v3.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/mem.c |   13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index 3baae1332760..6def7d7b6bfd 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -107,11 +107,16 @@ __mock bool cxl_hdm_decode_init(struct cxl_dev_state *cxlds)
>  	global_ctrl = readl(crb + cmap->hdm_decoder.offset +
>  			    CXL_HDM_DECODER_CTRL_OFFSET);
>  	global_enable = global_ctrl & CXL_HDM_DECODER_ENABLE;
> -	if (!global_enable && info->ranges) {
> -		dev_dbg(cxlds->dev,
> -			"DVSEC ranges already programmed and HDM decoders not enabled.\n");
> +
> +	/*
> +	 * Per CXL 2.0 Section 8.1.3.8.3 and 8.1.3.8.4 DVSEC CXL Range 1 Base
> +	 * [High,Low] when HDM operation is enabled the range register values
> +	 * are ignored by the device, but the spec also recommends matching the
> +	 * DVSEC Range 1,2 to HDM Decoder Range 0,1 so, non-zero info->ranges
> +	 * are expected.
> +	 */
> +	if (!global_enable && info->ranges)
>  		goto out;
> -	}
>  
>  	retval = true;
>  
>