Re: [PATCH v10 04/17] CXL/AER: Introduce CXL specific AER driver file

[<dan.j.williams@intel.com>]

Terry Bowman wrote:
> The CXL AER error handling logic currently resides in the AER driver file,
> drivers/pci/pcie/aer.c. CXL specific changes are conditionally compiled
> using #ifdefs.
> 
> Improve the AER driver maintainability by separating the CXL specific logic
> from the AER driver's core functionality and removing the #ifdefs.
> Introduce drivers/pci/pcie/cxl_aer.c and move the CXL AER logic into the
> new file.
> 
> Update the makefile to conditionally compile the CXL file using the
> existing CONFIG_PCIEAER_CXL Kconfig.
> 
> Signed-off-by: Terry Bowman <terry.bowman@amd.com>
> ---

After reading patch5 I want to qualify my Reviewed-by:...

>  drivers/pci/pci.h          |   8 +++
>  drivers/pci/pcie/Makefile  |   1 +
>  drivers/pci/pcie/aer.c     | 138 -------------------------------------
>  drivers/pci/pcie/cxl_aer.c | 138 +++++++++++++++++++++++++++++++++++++

This is a poor name for this file because the functionality only relates to
code that supports a dead-end generation of RCH / RCD hardware platforms. 

I do agree that it should be removed from aer.c so typical PCIe AER
maintenance does not need to trip over that cruft.

Please call it something like rch_aer.c so it is tucked out of the way,
sticks out as odd in any future diffstat, and does not confuse from the
CXL VH error handling that supports current and future generation
hardware.

Perhaps even move it to its own silent Kconfig symbol with a deprecation
warning, something like below, so someone remembers to delete it.

diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
index 17919b99fa66..da88358bbb4f 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -58,6 +58,13 @@ config PCIEAER_CXL
 
 	  If unsure, say Y.
 
+# Restricted CXL Host (RCH) error handling supports first generation CXL
+# hardware and can be deprecated in 7-10 years when only CXL Virtual Host
+# (CXL specification version 2+) hardware remains in service
+config RCH_AER
+	def_bool y
+	depends on PCIEAER_CXL
+
 #
 # PCI Express ECRC
 #