Re: [PATCH 03/15] cxl/aer/pci: Refactor AER driver's existing interfaces to support CXL PCIe ports

[Terry Bowman <Terry.Bowman@amd.com>]

Hi Bjorn,


On 10/10/24 14:11, Bjorn Helgaas wrote:
> I would describe this more as "renaming" than "refactoring".
> 

Good point. Renaming is more correct. Thanks. 

> On Tue, Oct 08, 2024 at 05:16:45PM -0500, Terry Bowman wrote:
>> The AER service driver already includes support for CXL restricted host
>> (RCH) downstream port error handling. The current implementation is based
>> CXl1.1 using a root complex event collector.
>>
>> Update the function interfaces and parameters where necessary to add
>> virtual hierarchy (VH) mode CXL PCIe port error handling alongside the RCH
>> handling. The CXL PCIe port error handling will be added in a future patch.
> 
> "Virtual Hierarchy mode" sounds like something defined by the spec.
> If so, add a citation and capitalize it the same way it's used in the
> spec.
> 
> Same for "restricted host", at least in terms of styling.  That
> support was added previously, so a citation probably isn't necessary
> here, but since this is part of *adding* VH support, hints about VH
> will be more helpful.
> 

Ok.

Regards,
Terry

>> Limit changes to refactoring variable and function names. No
>> functional changes are added.
>>
>> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
>> ---
>>  drivers/pci/pcie/aer.c | 28 ++++++++++++++--------------
>>  1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/pci/pcie/aer.c b/drivers/pci/pcie/aer.c
>> index 1e72829a249f..dc8b17999001 100644
>> --- a/drivers/pci/pcie/aer.c
>> +++ b/drivers/pci/pcie/aer.c
>> @@ -1030,7 +1030,7 @@ static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
>>  	return 0;
>>  }
>>  
>> -static void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info)
>> +static void cxl_handle_error(struct pci_dev *dev, struct aer_err_info *info)
>>  {
>>  	/*
>>  	 * Internal errors of an RCEC indicate an AER error in an
>> @@ -1053,30 +1053,30 @@ static int handles_cxl_error_iter(struct pci_dev *dev, void *data)
>>  	return *handles_cxl;
>>  }
>>  
>> -static bool handles_cxl_errors(struct pci_dev *rcec)
>> +static bool handles_cxl_errors(struct pci_dev *dev)
>>  {
>>  	bool handles_cxl = false;
>>  
>> -	if (pci_pcie_type(rcec) == PCI_EXP_TYPE_RC_EC &&
>> -	    pcie_aer_is_native(rcec))
>> -		pcie_walk_rcec(rcec, handles_cxl_error_iter, &handles_cxl);
>> +	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC &&
>> +	    pcie_aer_is_native(dev))
>> +		pcie_walk_rcec(dev, handles_cxl_error_iter, &handles_cxl);
>>  
>>  	return handles_cxl;
>>  }
>>  
>> -static void cxl_rch_enable_rcec(struct pci_dev *rcec)
>> +static void cxl_enable_internal_errors(struct pci_dev *dev)
>>  {
>> -	if (!handles_cxl_errors(rcec))
>> +	if (!handles_cxl_errors(dev))
>>  		return;
>>  
>> -	pci_aer_unmask_internal_errors(rcec);
>> -	pci_info(rcec, "CXL: Internal errors unmasked");
>> +	pci_aer_unmask_internal_errors(dev);
>> +	pci_info(dev, "CXL: Internal errors unmasked");
>>  }
>>  
>>  #else
>> -static inline void cxl_rch_enable_rcec(struct pci_dev *dev) { }
>> -static inline void cxl_rch_handle_error(struct pci_dev *dev,
>> -					struct aer_err_info *info) { }
>> +static inline void cxl_enable_internal_errors(struct pci_dev *dev) { }
>> +static inline void cxl_handle_error(struct pci_dev *dev,
>> +				    struct aer_err_info *info) { }
>>  #endif
>>  
>>  void register_cxl_port_hndlrs(struct cxl_port_err_hndlrs *_cxl_port_hndlrs)
>> @@ -1134,7 +1134,7 @@ 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_rch_handle_error(dev, info);
>> +	cxl_handle_error(dev, info);
>>  	pci_aer_handle_error(dev, info);
>>  	pci_dev_put(dev);
>>  }
>> @@ -1512,7 +1512,7 @@ static int aer_probe(struct pcie_device *dev)
>>  		return status;
>>  	}
>>  
>> -	cxl_rch_enable_rcec(port);
>> +	cxl_enable_internal_errors(port);
>>  	aer_enable_rootport(rpc);
>>  	pci_info(port, "enabled with IRQ %d\n", dev->irq);
>>  	return 0;
>> -- 
>> 2.34.1
>>