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

Terry Bowman
2024-10-14T17:27:08Z

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
>>