Re: [PATCH v3 05/15] PCI/AER: Add CXL PCIe port correctable error support in AER service driver

[Lukas Wunner <lukas@wunner.de>]

On Wed, Nov 13, 2024 at 03:54:19PM -0600, Terry Bowman wrote:
> @@ -1115,8 +1131,11 @@ static void pci_aer_handle_error(struct pci_dev *dev, struct aer_err_info *info)
>  
>  static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)
>  {
> -	cxl_handle_error(dev, info);
> -	pci_aer_handle_error(dev, info);
> +	if (is_internal_error(info) && handles_cxl_errors(dev))
> +		cxl_handle_error(dev, info);
> +	else
> +		pci_aer_handle_error(dev, info);
> +
>  	pci_dev_put(dev);
>  }

If you just do this at the top of cxl_handle_error()...

	if (!is_internal_error(info))
		return;

...you avoid the need to move is_internal_error() around and the
patch becomes simpler and easier to review.


> The AER service driver supports handling downstream port protocol errors in
> restricted CXL host (RCH) mode also known as CXL1.1. It needs the same
> functionality for CXL PCIe ports operating in virtual hierarchy (VH)
> mode.[1]

This is somewhat minor but by convention, patches in the PCI subsystem
adhere to spec language and capitalization, e.g. "Downstream Port"
instead of "downstream port".  Makes it easier to connect the commit
message or code comments to the spec.  So maybe you want to consider
that if/when respinning.

Thanks,

Lukas