diff mbox series

[v12,1/5] PCI: make pci_dev_is_disconnected() helper public for other drivers

Message ID 20240129034924.817005-2-haifeng.zhao@linux.intel.com (mailing list archive)
State Superseded
Headers show
Series fix vt-d hard lockup when hotplug ATS capable device | expand

Commit Message

Ethan Zhao Jan. 29, 2024, 3:49 a.m. UTC 
Make pci_dev_is_disconnected() public so that it can be called from
Intel VT-d driver to quickly fix/workaround the surprise removal
unplug hang issue for those ATS capable devices on PCIe switch downstream
hotplug capable ports.

Beside pci_device_is_present() function, this one has no config space
space access, so is light enough to optimize the normal pure surprise
removal and safe removal flow.

Tested-by: Haorong Ye <yehaorong@bytedance.com>
Signed-off-by: Ethan Zhao <haifeng.zhao@linux.intel.com>
---
 drivers/pci/pci.h   | 5 -----
 include/linux/pci.h | 5 +++++
 2 files changed, 5 insertions(+), 5 deletions(-)

Comments

Tian, Kevin Jan. 29, 2024, 8:50 a.m. UTC | #1 
> From: Ethan Zhao <haifeng.zhao@linux.intel.com>
> Sent: Monday, January 29, 2024 11:49 AM
> 
> Make pci_dev_is_disconnected() public so that it can be called from
> Intel VT-d driver to quickly fix/workaround the surprise removal
> unplug hang issue for those ATS capable devices on PCIe switch downstream
> hotplug capable ports.
> 
> Beside pci_device_is_present() function, this one has no config space

s/Beside/Unlike/

> space access, so is light enough to optimize the normal pure surprise
> removal and safe removal flow.

somehow this paragraph belongs to the patch using it. The 1st paragraph
is sufficient for the reason of exposure.
Ethan Zhao Jan. 30, 2024, 5:23 a.m. UTC | #2 
On 1/29/2024 4:50 PM, Tian, Kevin wrote:
>> From: Ethan Zhao <haifeng.zhao@linux.intel.com>
>> Sent: Monday, January 29, 2024 11:49 AM
>>
>> Make pci_dev_is_disconnected() public so that it can be called from
>> Intel VT-d driver to quickly fix/workaround the surprise removal
>> unplug hang issue for those ATS capable devices on PCIe switch downstream
>> hotplug capable ports.
>>
>> Beside pci_device_is_present() function, this one has no config space
> s/Beside/Unlike/

Beside means 'comparing with' here, perhaps it is not clear enough

or there is syntax error if used here ?



Thanks,

Ethan

>
>> space access, so is light enough to optimize the normal pure surprise
>> removal and safe removal flow.
> somehow this paragraph belongs to the patch using it. The 1st paragraph
> is sufficient for the reason of exposure.
Ethan Zhao Jan. 30, 2024, 5:25 a.m. UTC | #3 
On 1/29/2024 4:50 PM, Tian, Kevin wrote:
>> From: Ethan Zhao <haifeng.zhao@linux.intel.com>
>> Sent: Monday, January 29, 2024 11:49 AM
>>
>> Make pci_dev_is_disconnected() public so that it can be called from
>> Intel VT-d driver to quickly fix/workaround the surprise removal
>> unplug hang issue for those ATS capable devices on PCIe switch downstream
>> hotplug capable ports.
>>
>> Beside pci_device_is_present() function, this one has no config space
> s/Beside/Unlike/
>
>> space access, so is light enough to optimize the normal pure surprise
>> removal and safe removal flow.
> somehow this paragraph belongs to the patch using it. The 1st paragraph
> is sufficient for the reason of exposure.

pci_device_is_present() is aleardy exported symbol, why not just use it ?

Also try to make it clear, the difference and justification.


Thanks,
Ethan
Tian, Kevin Jan. 30, 2024, 6:23 a.m. UTC | #4 
> From: Ethan Zhao <haifeng.zhao@linux.intel.com>
> Sent: Tuesday, January 30, 2024 1:26 PM
> 
> On 1/29/2024 4:50 PM, Tian, Kevin wrote:
> >> From: Ethan Zhao <haifeng.zhao@linux.intel.com>
> >> Sent: Monday, January 29, 2024 11:49 AM
> >>
> >> Make pci_dev_is_disconnected() public so that it can be called from
> >> Intel VT-d driver to quickly fix/workaround the surprise removal
> >> unplug hang issue for those ATS capable devices on PCIe switch
> downstream
> >> hotplug capable ports.
> >>
> >> Beside pci_device_is_present() function, this one has no config space
> > s/Beside/Unlike/
> >
> >> space access, so is light enough to optimize the normal pure surprise
> >> removal and safe removal flow.
> > somehow this paragraph belongs to the patch using it. The 1st paragraph
> > is sufficient for the reason of exposure.
> 
> pci_device_is_present() is aleardy exported symbol, why not just use it ?
> 
> Also try to make it clear, the difference and justification.
> 

I misread it.
diff mbox series

Patch

diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 2336a8d1edab..94118c4cff54 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -368,11 +368,6 @@  static inline int pci_dev_set_disconnected(struct pci_dev *dev, void *unused)
 	return 0;
 }
 
-static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
-{
-	return dev->error_state == pci_channel_io_perm_failure;
-}
-
 /* pci_dev priv_flags */
 #define PCI_DEV_ADDED 0
 #define PCI_DPC_RECOVERED 1
diff --git a/include/linux/pci.h b/include/linux/pci.h
index add9368e6314..19dfe5f16fc9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -2512,6 +2512,11 @@  static inline struct pci_dev *pcie_find_root_port(struct pci_dev *dev)
 	return NULL;
 }
 
+static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
+{
+	return dev->error_state == pci_channel_io_perm_failure;
+}
+
 void pci_request_acs(void);
 bool pci_acs_enabled(struct pci_dev *pdev, u16 acs_flags);
 bool pci_acs_path_enabled(struct pci_dev *start,