Re: [PATCH] cxl/pci: the ctrl register should be read when it is being used

[Alison Schofield <alison.schofield@intel.com>]

On Wed, May 29, 2024 at 05:33:54PM +0800, Foryun Ma wrote:
> When the cap register and wait_for_valid checks fail, the ctrl register
> read will be redundant.
> 

Thanks for the patch. I suggest updating the commit message to match the
style of cxl/pci.c, something like:

cxl/pci: Only read the ctrl register it checking contents

A bit more below -


> Signed-off-by: Foryun Ma <foryun.ma@jaguarmicro.com>
> ---
>  drivers/cxl/core/pci.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 8567dd11eaac..627be83881e9 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -338,10 +338,6 @@ int cxl_dvsec_rr_decode(struct device *dev, int d,
>  	if (rc)
>  		return rc;
>  
> -	rc = pci_read_config_word(pdev, d + CXL_DVSEC_CTRL_OFFSET, &ctrl);
> -	if (rc)
> -		return rc;
> -
>  	if (!(cap & CXL_DVSEC_MEM_CAPABLE)) {
>  		dev_dbg(dev, "Not MEM Capable\n");
>  		return -ENXIO;
> @@ -363,6 +359,10 @@ int cxl_dvsec_rr_decode(struct device *dev, int d,
>  		return rc;
>  	}
>  
> +	rc = pci_read_config_word(pdev, d + CXL_DVSEC_CTRL_OFFSET, &ctrl);
> +	if (rc)
> +		return rc;
> +

How about moving the read a few more lines down. Put it after the
comment so that it is directly before the check.


>  	/*
>  	 * The current DVSEC values are moot if the memory capability is
>  	 * disabled, and they will remain moot after the HDM Decoder
> -- 
> 2.34.1
> 
>