Hi Sathya/Bjorn, Right, from Dell we did perform limited testing on one selected platform with this EDR patch and it did work. But if need to be tested with multiple platforms then we need to test few other configs. Please advise Thanks, Satish Internal Use - Confidential -----Original Message----- From: Kuppuswamy Sathyanarayanan +ADw-sathyanarayanan.kuppuswamy+AEA-linux.intel.com+AD4- Sent: Tuesday, April 30, 2024 9:26 PM To: Bjorn Helgaas +ADw-bhelgaas+AEA-google.com+AD4- Cc: linux-pci+AEA-vger.kernel.org+ADs- linux-kernel+AEA-vger.kernel.org+ADs- Thatchanamurthy, Satish +ADw-Satish.Thatchanamurt+AEA-Dell.com+AD4- Subject: +AFs-PATCH v1+AF0- PCI/EDR: Align EDR implementation with PCI firmware r3.3 spec +AFs-EXTERNAL EMAIL+AF0- During the Error Disconnect Recover (EDR) spec transition from r3.2 ECN to PCI firmware spec r3.3, improvements were made to definitions of EDR+AF8-PORT+AF8-DPC+AF8-ENABLE+AF8-DSM (0x0C) and EDR+AF8-PORT+AF8-LOCATE+AF8-DSM(0x0D) +AF8-DSMs. Specifically, +ACo- EDR+AF8-PORT+AF8-DPC+AF8-ENABLE+AF8-DSM +AF8-DSM version changed from 5 to 6, and arg4 is now a package type instead of an integer in version 5. +ACo- EDR+AF8-PORT+AF8-LOCATE+AF8-DSM +AF8-DSM uses BIT(31) to return the status of the operation. Ensure +AF8-DSM implementation aligns with PCI firmware r3.3 spec recommendation. More details about the EDR+AF8-PORT+AF8-DPC+AF8-ENABLE+AF8-DSM and EDR+AF8-PORT+AF8-LOCATE+AF8-DSM +AF8-DSMs can be found in PCI firmware specification, r3.3, sec 4.6.12 and sec 4.6.13. While at it, fix a typo in EDR+AF8-PORT+AF8-LOCATE+AF8-DSM comments. Fixes: ac1c8e35a326 (+ACI-PCI/DPC: Add Error Disconnect Recover (EDR) support+ACI-) Signed-off-by: Kuppuswamy Sathyanarayanan +ADw-sathyanarayanan.kuppuswamy+AEA-linux.intel.com+AD4- --- drivers/pci/pcie/edr.c +AHw- 23 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+------- 1 file changed, 17 insertions(+-), 6 deletions(-) diff --git a/drivers/pci/pcie/edr.c b/drivers/pci/pcie/edr.c index 5f4914d313a1..fea098e22e3e 100644 --- a/drivers/pci/pcie/edr.c +-+-+- b/drivers/pci/pcie/edr.c +AEAAQA- -35,7 +-35,7 +AEAAQA- static int acpi+AF8-enable+AF8-dpc(struct pci+AF8-dev +ACo-pdev) +ACo- Behavior when calling unsupported +AF8-DSM functions is undefined, +ACo- so check whether EDR+AF8-PORT+AF8-DPC+AF8-ENABLE+AF8-DSM is supported. +ACo-/ - if (+ACE-acpi+AF8-check+AF8-dsm(adev-+AD4-handle, +ACY-pci+AF8-acpi+AF8-dsm+AF8-guid, 5, +- if (+ACE-acpi+AF8-check+AF8-dsm(adev-+AD4-handle, +ACY-pci+AF8-acpi+AF8-dsm+AF8-guid, 6, 1ULL +ADwAPA- EDR+AF8-PORT+AF8-DPC+AF8-ENABLE+AF8-DSM)) return 0+ADs- +AEAAQA- -47,11 +-47,11 +AEAAQA- static int acpi+AF8-enable+AF8-dpc(struct pci+AF8-dev +ACo-pdev) argv4.package.elements +AD0- +ACY-req+ADs- /+ACo- - +ACo- Per Downstream Port Containment Related Enhancements ECN to PCI - +ACo- Firmware Specification r3.2, sec 4.6.12, EDR+AF8-PORT+AF8-DPC+AF8-ENABLE+AF8-DSM is - +ACo- optional. Return success if it's not implemented. +- +ACo- Per PCI Firmware Specification r3.3, sec 4.6.12, +- +ACo- EDR+AF8-PORT+AF8-DPC+AF8-ENABLE+AF8-DSM is optional. Return success if it's not +- +ACo- implemented. +ACo-/ - obj +AD0- acpi+AF8-evaluate+AF8-dsm(adev-+AD4-handle, +ACY-pci+AF8-acpi+AF8-dsm+AF8-guid, 5, +- obj +AD0- acpi+AF8-evaluate+AF8-dsm(adev-+AD4-handle, +ACY-pci+AF8-acpi+AF8-dsm+AF8-guid, 6, EDR+AF8-PORT+AF8-DPC+AF8-ENABLE+AF8-DSM, +ACY-argv4)+ADs- if (+ACE-obj) return 0+ADs- +AEAAQA- -86,7 +-86,7 +AEAAQA- static struct pci+AF8-dev +ACo-acpi+AF8-dpc+AF8-port+AF8-get(struct pci+AF8-dev +ACo-pdev) /+ACo- +ACo- Behavior when calling unsupported +AF8-DSM functions is undefined, - +ACo- so check whether EDR+AF8-PORT+AF8-DPC+AF8-ENABLE+AF8-DSM is supported. +- +ACo- so check whether EDR+AF8-PORT+AF8-LOCATE+AF8-DSM is supported. +ACo-/ if (+ACE-acpi+AF8-check+AF8-dsm(adev-+AD4-handle, +ACY-pci+AF8-acpi+AF8-dsm+AF8-guid, 5, 1ULL +ADwAPA- EDR+AF8-PORT+AF8-LOCATE+AF8-DSM)) +AEAAQA- -103,6 +-103,17 +AEAAQA- static struct pci+AF8-dev +ACo-acpi+AF8-dpc+AF8-port+AF8-get(struct pci+AF8-dev +ACo-pdev) return NULL+ADs- +AH0- +- /+ACo- +- +ACo- Per PCI Firmware Specification r3.3, sec 4.6.13, bit 31 represents +- +ACo- the success/failure of the operation. If bit 31 is set, the operation +- +ACo- is failed. +- +ACo-/ +- if (obj-+AD4-integer.value +ACY- BIT(31)) +AHs- +- ACPI+AF8-FREE(obj)+ADs- +- pci+AF8-err(pdev, +ACI-Locate Port +AF8-DSM failed+AFw-n+ACI-)+ADs- +- return NULL+ADs- +- +AH0- +- /+ACo- +ACo- Firmware returns DPC port BDF details in following format: +ACo- 15:8 +AD0- bus -- 2.25.1