| Message ID | 20241008221657.1130181-4-terry.bowman@amd.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable CXL PCIe port protocol error handling and logging | expand |
I would describe this more as "renaming" than "refactoring". 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. > 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 >
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 >>
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;
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. 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(-)