[PATCH] PCI/AER: Remove deprecated documentation for pcie_enable_pcie_error_reporting()

[PATCH] PCI/AER: Remove deprecated documentation for pcie_enable_pcie_error_reporting()

[Dave Jiang]

With commit [1] upstream that enables AER reporting by default for all PCIe
devices, the documentation for pcie_enable_pcie_error_reporting() is no
longer necessary. Remove references to the helper function.

[1]: commit f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native")

Suggested-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 Documentation/PCI/pcieaer-howto.rst |   18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
index 0b36b9ebfa4b..a82802795a06 100644
--- a/Documentation/PCI/pcieaer-howto.rst
+++ b/Documentation/PCI/pcieaer-howto.rst
@@ -135,15 +135,6 @@ hierarchy and links. These errors do not include any device specific
 errors because device specific errors will still get sent directly to
 the device driver.
 
-Configure the AER capability structure
---------------------------------------
-
-AER aware drivers of PCI Express component need change the device
-control registers to enable AER. They also could change AER registers,
-including mask and severity registers. Helper function
-pci_enable_pcie_error_reporting could be used to enable AER. See
-section 3.3.
-
 Provide callbacks
 -----------------
 
@@ -214,15 +205,6 @@ to mmio_enabled.
 
 helper functions
 ----------------
-::
-
-  int pci_enable_pcie_error_reporting(struct pci_dev *dev);
-
-pci_enable_pcie_error_reporting enables the device to send error
-messages to root port when an error is detected. Note that devices
-don't enable the error reporting by default, so device drivers need
-call this function to enable it.
-
 ::
 
   int pci_disable_pcie_error_reporting(struct pci_dev *dev);

Re: [PATCH] PCI/AER: Remove deprecated documentation for pcie_enable_pcie_error_reporting()

[Bjorn Helgaas]

[+cc Stefan, Sathy, Jonathan]

On Tue, Feb 14, 2023 at 09:48:55AM -0700, Dave Jiang wrote:
> With commit [1] upstream that enables AER reporting by default for all PCIe
> devices, the documentation for pcie_enable_pcie_error_reporting() is no
> longer necessary. Remove references to the helper function.
> 
> [1]: commit f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native")
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Thanks!  I'll attach my work-in-progress patch from yesterday for your
comments.  I think we can go even a little further because I don't
think we need to encourage drivers to configure AER registers (if they
do, they almost certainly don't pay attention to ownership via _OSC),
and if they don't use pci_enable_pcie_error_reporting(), they
shouldn't use pci_disable_pcie_error_reporting() either.

> ---
>  Documentation/PCI/pcieaer-howto.rst |   18 ------------------
>  1 file changed, 18 deletions(-)
> 
> diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
> index 0b36b9ebfa4b..a82802795a06 100644
> --- a/Documentation/PCI/pcieaer-howto.rst
> +++ b/Documentation/PCI/pcieaer-howto.rst
> @@ -135,15 +135,6 @@ hierarchy and links. These errors do not include any device specific
>  errors because device specific errors will still get sent directly to
>  the device driver.
>  
> -Configure the AER capability structure
> ---------------------------------------
> -
> -AER aware drivers of PCI Express component need change the device
> -control registers to enable AER. They also could change AER registers,
> -including mask and severity registers. Helper function
> -pci_enable_pcie_error_reporting could be used to enable AER. See
> -section 3.3.
> -
>  Provide callbacks
>  -----------------
>  
> @@ -214,15 +205,6 @@ to mmio_enabled.
>  
>  helper functions
>  ----------------
> -::
> -
> -  int pci_enable_pcie_error_reporting(struct pci_dev *dev);
> -
> -pci_enable_pcie_error_reporting enables the device to send error
> -messages to root port when an error is detected. Note that devices
> -don't enable the error reporting by default, so device drivers need
> -call this function to enable it.
> -
>  ::
>  
>    int pci_disable_pcie_error_reporting(struct pci_dev *dev);


commit d7b36abe72db ("Remove AER Capability configuration")
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Mon Feb 13 11:53:42 2023 -0600

    Remove AER Capability configuration

diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
index 0b36b9ebfa4b..c98a229ea9f5 100644
--- a/Documentation/PCI/pcieaer-howto.rst
+++ b/Documentation/PCI/pcieaer-howto.rst
@@ -96,8 +96,8 @@ Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
 Developer Guide
 ===============
 
-To enable AER aware support requires a software driver to configure
-the AER capability structure within its device and to provide callbacks.
+To enable AER aware support requires a software driver to provide
+callbacks.
 
 To support AER better, developers need understand how AER does work
 firstly.
@@ -135,15 +135,6 @@ hierarchy and links. These errors do not include any device specific
 errors because device specific errors will still get sent directly to
 the device driver.
 
-Configure the AER capability structure
---------------------------------------
-
-AER aware drivers of PCI Express component need change the device
-control registers to enable AER. They also could change AER registers,
-including mask and severity registers. Helper function
-pci_enable_pcie_error_reporting could be used to enable AER. See
-section 3.3.
-
 Provide callbacks
 -----------------
 
@@ -212,31 +203,6 @@ to reset the link. If error_detected returns PCI_ERS_RESULT_CAN_RECOVER
 and reset_link returns PCI_ERS_RESULT_RECOVERED, the error handling goes
 to mmio_enabled.
 
-helper functions
-----------------
-::
-
-  int pci_enable_pcie_error_reporting(struct pci_dev *dev);
-
-pci_enable_pcie_error_reporting enables the device to send error
-messages to root port when an error is detected. Note that devices
-don't enable the error reporting by default, so device drivers need
-call this function to enable it.
-
-::
-
-  int pci_disable_pcie_error_reporting(struct pci_dev *dev);
-
-pci_disable_pcie_error_reporting disables the device to send error
-messages to root port when an error is detected.
-
-::
-
-  int pci_aer_clear_nonfatal_status(struct pci_dev *dev);`
-
-pci_aer_clear_nonfatal_status clears non-fatal errors in the uncorrectable
-error status register.
-
 Frequent Asked Questions
 ------------------------
 
@@ -257,24 +223,6 @@ A:
   Fatal error recovery will fail if the errors are reported by the
   upstream ports who are attached by the service driver.
 
-Q:
-  How does this infrastructure deal with driver that is not PCI
-  Express aware?
-
-A:
-  This infrastructure calls the error callback functions of the
-  driver when an error happens. But if the driver is not aware of
-  PCI Express, the device might not report its own errors to root
-  port.
-
-Q:
-  What modifications will that driver need to make it compatible
-  with the PCI Express AER Root driver?
-
-A:
-  It could call the helper functions to enable AER in devices and
-  cleanup uncorrectable status register. Pls. refer to section 3.3.
-
 
 Software error injection
 ========================

Re: [PATCH] PCI/AER: Remove deprecated documentation for pcie_enable_pcie_error_reporting()

[Dave Jiang]

On 2/14/23 10:28 AM, Bjorn Helgaas wrote:
> [+cc Stefan, Sathy, Jonathan]
> 
> On Tue, Feb 14, 2023 at 09:48:55AM -0700, Dave Jiang wrote:
>> With commit [1] upstream that enables AER reporting by default for all PCIe
>> devices, the documentation for pcie_enable_pcie_error_reporting() is no
>> longer necessary. Remove references to the helper function.
>>
>> [1]: commit f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native")
>>
>> Suggested-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> Thanks!  I'll attach my work-in-progress patch from yesterday for your
> comments.  I think we can go even a little further because I don't
> think we need to encourage drivers to configure AER registers (if they
> do, they almost certainly don't pay attention to ownership via _OSC),
> and if they don't use pci_enable_pcie_error_reporting(), they
> shouldn't use pci_disable_pcie_error_reporting() either.
> 
>> ---
>>   Documentation/PCI/pcieaer-howto.rst |   18 ------------------
>>   1 file changed, 18 deletions(-)
>>
>> diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
>> index 0b36b9ebfa4b..a82802795a06 100644
>> --- a/Documentation/PCI/pcieaer-howto.rst
>> +++ b/Documentation/PCI/pcieaer-howto.rst
>> @@ -135,15 +135,6 @@ hierarchy and links. These errors do not include any device specific
>>   errors because device specific errors will still get sent directly to
>>   the device driver.
>>   
>> -Configure the AER capability structure
>> ---------------------------------------
>> -
>> -AER aware drivers of PCI Express component need change the device
>> -control registers to enable AER. They also could change AER registers,
>> -including mask and severity registers. Helper function
>> -pci_enable_pcie_error_reporting could be used to enable AER. See
>> -section 3.3.
>> -
>>   Provide callbacks
>>   -----------------
>>   
>> @@ -214,15 +205,6 @@ to mmio_enabled.
>>   
>>   helper functions
>>   ----------------
>> -::
>> -
>> -  int pci_enable_pcie_error_reporting(struct pci_dev *dev);
>> -
>> -pci_enable_pcie_error_reporting enables the device to send error
>> -messages to root port when an error is detected. Note that devices
>> -don't enable the error reporting by default, so device drivers need
>> -call this function to enable it.
>> -
>>   ::
>>   
>>     int pci_disable_pcie_error_reporting(struct pci_dev *dev);
> 
> 
> commit d7b36abe72db ("Remove AER Capability configuration")
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Mon Feb 13 11:53:42 2023 -0600
> 
>      Remove AER Capability configuration

The changes LGTM.
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> 
> diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
> index 0b36b9ebfa4b..c98a229ea9f5 100644
> --- a/Documentation/PCI/pcieaer-howto.rst
> +++ b/Documentation/PCI/pcieaer-howto.rst
> @@ -96,8 +96,8 @@ Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
>   Developer Guide
>   ===============
>   
> -To enable AER aware support requires a software driver to configure
> -the AER capability structure within its device and to provide callbacks.
> +To enable AER aware support requires a software driver to provide
> +callbacks.
>   
>   To support AER better, developers need understand how AER does work
>   firstly.
> @@ -135,15 +135,6 @@ hierarchy and links. These errors do not include any device specific
>   errors because device specific errors will still get sent directly to
>   the device driver.
>   
> -Configure the AER capability structure
> ---------------------------------------
> -
> -AER aware drivers of PCI Express component need change the device
> -control registers to enable AER. They also could change AER registers,
> -including mask and severity registers. Helper function
> -pci_enable_pcie_error_reporting could be used to enable AER. See
> -section 3.3.
> -
>   Provide callbacks
>   -----------------
>   
> @@ -212,31 +203,6 @@ to reset the link. If error_detected returns PCI_ERS_RESULT_CAN_RECOVER
>   and reset_link returns PCI_ERS_RESULT_RECOVERED, the error handling goes
>   to mmio_enabled.
>   
> -helper functions
> -----------------
> -::
> -
> -  int pci_enable_pcie_error_reporting(struct pci_dev *dev);
> -
> -pci_enable_pcie_error_reporting enables the device to send error
> -messages to root port when an error is detected. Note that devices
> -don't enable the error reporting by default, so device drivers need
> -call this function to enable it.
> -
> -::
> -
> -  int pci_disable_pcie_error_reporting(struct pci_dev *dev);
> -
> -pci_disable_pcie_error_reporting disables the device to send error
> -messages to root port when an error is detected.
> -
> -::
> -
> -  int pci_aer_clear_nonfatal_status(struct pci_dev *dev);`
> -
> -pci_aer_clear_nonfatal_status clears non-fatal errors in the uncorrectable
> -error status register.
> -
>   Frequent Asked Questions
>   ------------------------
>   
> @@ -257,24 +223,6 @@ A:
>     Fatal error recovery will fail if the errors are reported by the
>     upstream ports who are attached by the service driver.
>   
> -Q:
> -  How does this infrastructure deal with driver that is not PCI
> -  Express aware?
> -
> -A:
> -  This infrastructure calls the error callback functions of the
> -  driver when an error happens. But if the driver is not aware of
> -  PCI Express, the device might not report its own errors to root
> -  port.
> -
> -Q:
> -  What modifications will that driver need to make it compatible
> -  with the PCI Express AER Root driver?
> -
> -A:
> -  It could call the helper functions to enable AER in devices and
> -  cleanup uncorrectable status register. Pls. refer to section 3.3.
> -
>   
>   Software error injection
>   ========================

Re: [PATCH] PCI/AER: Remove deprecated documentation for pcie_enable_pcie_error_reporting()

[Sathyanarayanan Kuppuswamy]

On 2/14/23 8:48 AM, Dave Jiang wrote:
> With commit [1] upstream that enables AER reporting by default for all PCIe
> devices, the documentation for pcie_enable_pcie_error_reporting() is no

/s/pcie_enable_pcie_error_reporting/pci_enable_pcie_error_reporting

> longer necessary. Remove references to the helper function.

Before removing the documentation, are the references removed from the
code? I think Bjorn only cleaned up the net drivers.

> 
> [1]: commit f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native")
> 
> Suggested-by: Dan Williams <dan.j.williams@intel.com>
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  Documentation/PCI/pcieaer-howto.rst |   18 ------------------
>  1 file changed, 18 deletions(-)
> 
> diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
> index 0b36b9ebfa4b..a82802795a06 100644
> --- a/Documentation/PCI/pcieaer-howto.rst
> +++ b/Documentation/PCI/pcieaer-howto.rst
> @@ -135,15 +135,6 @@ hierarchy and links. These errors do not include any device specific
>  errors because device specific errors will still get sent directly to
>  the device driver.
>  
> -Configure the AER capability structure
> ---------------------------------------
> -
> -AER aware drivers of PCI Express component need change the device
> -control registers to enable AER. They also could change AER registers,
> -including mask and severity registers. Helper function
> -pci_enable_pcie_error_reporting could be used to enable AER. See
> -section 3.3.
> -
>  Provide callbacks
>  -----------------
>  
> @@ -214,15 +205,6 @@ to mmio_enabled.
>  
>  helper functions
>  ----------------
> -::
> -
> -  int pci_enable_pcie_error_reporting(struct pci_dev *dev);
> -
> -pci_enable_pcie_error_reporting enables the device to send error
> -messages to root port when an error is detected. Note that devices
> -don't enable the error reporting by default, so device drivers need
> -call this function to enable it.
> -
>  ::
>  
>    int pci_disable_pcie_error_reporting(struct pci_dev *dev);
> 
> 

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH] PCI/AER: Remove deprecated documentation for pcie_enable_pcie_error_reporting()

[Dave Jiang]

On 2/14/23 10:37 AM, Sathyanarayanan Kuppuswamy wrote:
> 
> 
> On 2/14/23 8:48 AM, Dave Jiang wrote:
>> With commit [1] upstream that enables AER reporting by default for all PCIe
>> devices, the documentation for pcie_enable_pcie_error_reporting() is no
> 
> /s/pcie_enable_pcie_error_reporting/pci_enable_pcie_error_reporting
> 
>> longer necessary. Remove references to the helper function.
> 
> Before removing the documentation, are the references removed from the
> code? I think Bjorn only cleaned up the net drivers.

I have not touched any of the calling code. This popped up because of 
CXL RAS enabling. The removal of documentation is the hope to stop new 
implementations from calling the function.

> 
>>
>> [1]: commit f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native")
>>
>> Suggested-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>> ---
>>   Documentation/PCI/pcieaer-howto.rst |   18 ------------------
>>   1 file changed, 18 deletions(-)
>>
>> diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
>> index 0b36b9ebfa4b..a82802795a06 100644
>> --- a/Documentation/PCI/pcieaer-howto.rst
>> +++ b/Documentation/PCI/pcieaer-howto.rst
>> @@ -135,15 +135,6 @@ hierarchy and links. These errors do not include any device specific
>>   errors because device specific errors will still get sent directly to
>>   the device driver.
>>   
>> -Configure the AER capability structure
>> ---------------------------------------
>> -
>> -AER aware drivers of PCI Express component need change the device
>> -control registers to enable AER. They also could change AER registers,
>> -including mask and severity registers. Helper function
>> -pci_enable_pcie_error_reporting could be used to enable AER. See
>> -section 3.3.
>> -
>>   Provide callbacks
>>   -----------------
>>   
>> @@ -214,15 +205,6 @@ to mmio_enabled.
>>   
>>   helper functions
>>   ----------------
>> -::
>> -
>> -  int pci_enable_pcie_error_reporting(struct pci_dev *dev);
>> -
>> -pci_enable_pcie_error_reporting enables the device to send error
>> -messages to root port when an error is detected. Note that devices
>> -don't enable the error reporting by default, so device drivers need
>> -call this function to enable it.
>> -
>>   ::
>>   
>>     int pci_disable_pcie_error_reporting(struct pci_dev *dev);
>>
>>
> 

Re: [PATCH] PCI/AER: Remove deprecated documentation for pcie_enable_pcie_error_reporting()

[Sathyanarayanan Kuppuswamy]

On 2/14/23 9:28 AM, Bjorn Helgaas wrote:
> [+cc Stefan, Sathy, Jonathan]
> 
> On Tue, Feb 14, 2023 at 09:48:55AM -0700, Dave Jiang wrote:
>> With commit [1] upstream that enables AER reporting by default for all PCIe
>> devices, the documentation for pcie_enable_pcie_error_reporting() is no
>> longer necessary. Remove references to the helper function.
>>
>> [1]: commit f26e58bf6f54 ("PCI/AER: Enable error reporting when AER is native")
>>
>> Suggested-by: Dan Williams <dan.j.williams@intel.com>
>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> Thanks!  I'll attach my work-in-progress patch from yesterday for your
> comments.  I think we can go even a little further because I don't
> think we need to encourage drivers to configure AER registers (if they
> do, they almost certainly don't pay attention to ownership via _OSC),
> and if they don't use pci_enable_pcie_error_reporting(), they
> shouldn't use pci_disable_pcie_error_reporting() either.
> 
>> ---
>>  Documentation/PCI/pcieaer-howto.rst |   18 ------------------
>>  1 file changed, 18 deletions(-)
>>
>> diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
>> index 0b36b9ebfa4b..a82802795a06 100644
>> --- a/Documentation/PCI/pcieaer-howto.rst
>> +++ b/Documentation/PCI/pcieaer-howto.rst
>> @@ -135,15 +135,6 @@ hierarchy and links. These errors do not include any device specific
>>  errors because device specific errors will still get sent directly to
>>  the device driver.
>>  
>> -Configure the AER capability structure
>> ---------------------------------------
>> -
>> -AER aware drivers of PCI Express component need change the device
>> -control registers to enable AER. They also could change AER registers,
>> -including mask and severity registers. Helper function
>> -pci_enable_pcie_error_reporting could be used to enable AER. See
>> -section 3.3.
>> -
>>  Provide callbacks
>>  -----------------
>>  
>> @@ -214,15 +205,6 @@ to mmio_enabled.
>>  
>>  helper functions
>>  ----------------
>> -::
>> -
>> -  int pci_enable_pcie_error_reporting(struct pci_dev *dev);
>> -
>> -pci_enable_pcie_error_reporting enables the device to send error
>> -messages to root port when an error is detected. Note that devices
>> -don't enable the error reporting by default, so device drivers need
>> -call this function to enable it.
>> -
>>  ::
>>  
>>    int pci_disable_pcie_error_reporting(struct pci_dev *dev);
> 
> 
> commit d7b36abe72db ("Remove AER Capability configuration")
> Author: Bjorn Helgaas <bhelgaas@google.com>
> Date:   Mon Feb 13 11:53:42 2023 -0600
> 
>     Remove AER Capability configuration
> 

Looks good to me.

> diff --git a/Documentation/PCI/pcieaer-howto.rst b/Documentation/PCI/pcieaer-howto.rst
> index 0b36b9ebfa4b..c98a229ea9f5 100644
> --- a/Documentation/PCI/pcieaer-howto.rst
> +++ b/Documentation/PCI/pcieaer-howto.rst
> @@ -96,8 +96,8 @@ Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
>  Developer Guide
>  ===============
>  
> -To enable AER aware support requires a software driver to configure
> -the AER capability structure within its device and to provide callbacks.
> +To enable AER aware support requires a software driver to provide
> +callbacks.
>  
>  To support AER better, developers need understand how AER does work
>  firstly.
> @@ -135,15 +135,6 @@ hierarchy and links. These errors do not include any device specific
>  errors because device specific errors will still get sent directly to
>  the device driver.
>  
> -Configure the AER capability structure
> ---------------------------------------
> -
> -AER aware drivers of PCI Express component need change the device
> -control registers to enable AER. They also could change AER registers,
> -including mask and severity registers. Helper function
> -pci_enable_pcie_error_reporting could be used to enable AER. See
> -section 3.3.
> -
>  Provide callbacks
>  -----------------
>  
> @@ -212,31 +203,6 @@ to reset the link. If error_detected returns PCI_ERS_RESULT_CAN_RECOVER
>  and reset_link returns PCI_ERS_RESULT_RECOVERED, the error handling goes
>  to mmio_enabled.
>  
> -helper functions
> -----------------
> -::
> -
> -  int pci_enable_pcie_error_reporting(struct pci_dev *dev);
> -
> -pci_enable_pcie_error_reporting enables the device to send error
> -messages to root port when an error is detected. Note that devices
> -don't enable the error reporting by default, so device drivers need
> -call this function to enable it.
> -
> -::
> -
> -  int pci_disable_pcie_error_reporting(struct pci_dev *dev);
> -
> -pci_disable_pcie_error_reporting disables the device to send error
> -messages to root port when an error is detected.
> -
> -::
> -
> -  int pci_aer_clear_nonfatal_status(struct pci_dev *dev);`
> -
> -pci_aer_clear_nonfatal_status clears non-fatal errors in the uncorrectable
> -error status register.
> -
>  Frequent Asked Questions
>  ------------------------
>  
> @@ -257,24 +223,6 @@ A:
>    Fatal error recovery will fail if the errors are reported by the
>    upstream ports who are attached by the service driver.
>  
> -Q:
> -  How does this infrastructure deal with driver that is not PCI
> -  Express aware?
> -
> -A:
> -  This infrastructure calls the error callback functions of the
> -  driver when an error happens. But if the driver is not aware of
> -  PCI Express, the device might not report its own errors to root
> -  port.
> -
> -Q:
> -  What modifications will that driver need to make it compatible
> -  with the PCI Express AER Root driver?
> -
> -A:
> -  It could call the helper functions to enable AER in devices and
> -  cleanup uncorrectable status register. Pls. refer to section 3.3.
> -
>  
>  Software error injection
>  ========================

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH] PCI/AER: Remove deprecated documentation for pcie_enable_pcie_error_reporting()

[Stefan Roese]

On 2/14/23 18:37, Dave Jiang wrote:
> 
> 
> On 2/14/23 10:28 AM, Bjorn Helgaas wrote:
>> [+cc Stefan, Sathy, Jonathan]
>>
>> On Tue, Feb 14, 2023 at 09:48:55AM -0700, Dave Jiang wrote:
>>> With commit [1] upstream that enables AER reporting by default for 
>>> all PCIe
>>> devices, the documentation for pcie_enable_pcie_error_reporting() is no
>>> longer necessary. Remove references to the helper function.
>>>
>>> [1]: commit f26e58bf6f54 ("PCI/AER: Enable error reporting when AER 
>>> is native")
>>>
>>> Suggested-by: Dan Williams <dan.j.williams@intel.com>
>>> Signed-off-by: Dave Jiang <dave.jiang@intel.com>
>>
>> Thanks!  I'll attach my work-in-progress patch from yesterday for your
>> comments.  I think we can go even a little further because I don't
>> think we need to encourage drivers to configure AER registers (if they
>> do, they almost certainly don't pay attention to ownership via _OSC),
>> and if they don't use pci_enable_pcie_error_reporting(), they
>> shouldn't use pci_disable_pcie_error_reporting() either.
>>
>>> ---
>>>   Documentation/PCI/pcieaer-howto.rst |   18 ------------------
>>>   1 file changed, 18 deletions(-)
>>>
>>> diff --git a/Documentation/PCI/pcieaer-howto.rst 
>>> b/Documentation/PCI/pcieaer-howto.rst
>>> index 0b36b9ebfa4b..a82802795a06 100644
>>> --- a/Documentation/PCI/pcieaer-howto.rst
>>> +++ b/Documentation/PCI/pcieaer-howto.rst
>>> @@ -135,15 +135,6 @@ hierarchy and links. These errors do not include 
>>> any device specific
>>>   errors because device specific errors will still get sent directly to
>>>   the device driver.
>>> -Configure the AER capability structure
>>> ---------------------------------------
>>> -
>>> -AER aware drivers of PCI Express component need change the device
>>> -control registers to enable AER. They also could change AER registers,
>>> -including mask and severity registers. Helper function
>>> -pci_enable_pcie_error_reporting could be used to enable AER. See
>>> -section 3.3.
>>> -
>>>   Provide callbacks
>>>   -----------------
>>> @@ -214,15 +205,6 @@ to mmio_enabled.
>>>   helper functions
>>>   ----------------
>>> -::
>>> -
>>> -  int pci_enable_pcie_error_reporting(struct pci_dev *dev);
>>> -
>>> -pci_enable_pcie_error_reporting enables the device to send error
>>> -messages to root port when an error is detected. Note that devices
>>> -don't enable the error reporting by default, so device drivers need
>>> -call this function to enable it.
>>> -
>>>   ::
>>>     int pci_disable_pcie_error_reporting(struct pci_dev *dev);
>>
>>
>> commit d7b36abe72db ("Remove AER Capability configuration")
>> Author: Bjorn Helgaas <bhelgaas@google.com>
>> Date:   Mon Feb 13 11:53:42 2023 -0600
>>
>>      Remove AER Capability configuration
> 
> The changes LGTM.
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> 
>>
>> diff --git a/Documentation/PCI/pcieaer-howto.rst 
>> b/Documentation/PCI/pcieaer-howto.rst
>> index 0b36b9ebfa4b..c98a229ea9f5 100644
>> --- a/Documentation/PCI/pcieaer-howto.rst
>> +++ b/Documentation/PCI/pcieaer-howto.rst
>> @@ -96,8 +96,8 @@ 
>> Documentation/ABI/testing/sysfs-bus-pci-devices-aer_stats
>>   Developer Guide
>>   ===============
>> -To enable AER aware support requires a software driver to configure
>> -the AER capability structure within its device and to provide callbacks.
>> +To enable AER aware support requires a software driver to provide
>> +callbacks.
>>   To support AER better, developers need understand how AER does work
>>   firstly.
>> @@ -135,15 +135,6 @@ hierarchy and links. These errors do not include 
>> any device specific
>>   errors because device specific errors will still get sent directly to
>>   the device driver.
>> -Configure the AER capability structure
>> ---------------------------------------
>> -
>> -AER aware drivers of PCI Express component need change the device
>> -control registers to enable AER. They also could change AER registers,
>> -including mask and severity registers. Helper function
>> -pci_enable_pcie_error_reporting could be used to enable AER. See
>> -section 3.3.
>> -
>>   Provide callbacks
>>   -----------------
>> @@ -212,31 +203,6 @@ to reset the link. If error_detected returns 
>> PCI_ERS_RESULT_CAN_RECOVER
>>   and reset_link returns PCI_ERS_RESULT_RECOVERED, the error handling 
>> goes
>>   to mmio_enabled.
>> -helper functions
>> -----------------
>> -::
>> -
>> -  int pci_enable_pcie_error_reporting(struct pci_dev *dev);
>> -
>> -pci_enable_pcie_error_reporting enables the device to send error
>> -messages to root port when an error is detected. Note that devices
>> -don't enable the error reporting by default, so device drivers need
>> -call this function to enable it.
>> -
>> -::
>> -
>> -  int pci_disable_pcie_error_reporting(struct pci_dev *dev);
>> -
>> -pci_disable_pcie_error_reporting disables the device to send error
>> -messages to root port when an error is detected.
>> -
>> -::
>> -
>> -  int pci_aer_clear_nonfatal_status(struct pci_dev *dev);`
>> -
>> -pci_aer_clear_nonfatal_status clears non-fatal errors in the 
>> uncorrectable
>> -error status register.
>> -
>>   Frequent Asked Questions
>>   ------------------------
>> @@ -257,24 +223,6 @@ A:
>>     Fatal error recovery will fail if the errors are reported by the
>>     upstream ports who are attached by the service driver.
>> -Q:
>> -  How does this infrastructure deal with driver that is not PCI
>> -  Express aware?
>> -
>> -A:
>> -  This infrastructure calls the error callback functions of the
>> -  driver when an error happens. But if the driver is not aware of
>> -  PCI Express, the device might not report its own errors to root
>> -  port.
>> -
>> -Q:
>> -  What modifications will that driver need to make it compatible
>> -  with the PCI Express AER Root driver?
>> -
>> -A:
>> -  It could call the helper functions to enable AER in devices and
>> -  cleanup uncorrectable status register. Pls. refer to section 3.3.
>> -
>>   Software error injection
>>   ========================

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de