Re: [PATCH v2 2/4] PCI: Refresh root ports in pci_bridge_d3_update()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/12/2023 13:25, Rafael J. Wysocki wrote:
On Mon, Dec 4, 2023 at 7:07 AM Mario Limonciello
<mario.limonciello@xxxxxxx> wrote:

If pci_d3cold_enable() or pci_d3cold_disable() is called on a root
port it is ignored because there is no upstream bridge.

The kerneldoc comment of pci_bridge_d3_update() explains what that
function is for which also covers why it does not take effect when
called on root ports.

I'm sorry but can you clarify the intent of your comment?

Are you suggesting we should introduce a different function/logic for root ports, kernel doc should be updated, or root ports should be special cased in that function?


If called on a root port, use `no_d3cold` variable to decide policy

It is unclear that this is about pci_bridge_d3_possible() which
applies to both D3hot and D3cold, not just D3cold AFAICS.  I don't
think that no_d3cold should affect the D3hot behavior.

IMO the semantics are confusing depending upon what device you called pci_d3cold_disable()/pci_d3cold_enable() with as an argument.

Both devices and root ports are used by existing driver in the kernel.

If you called pci_d3cold_disable() with a device, that actually prevents the /bridge above it/ from going to D3hot as well (bridge_d3 is set to the result)


and also immediately refresh whether D3 is possible.

Which isn't correct AFAICS.

Why?


Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
---
  drivers/pci/pci.c | 9 ++++++++-
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 72505794cc72..3d4aaecda457 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3023,6 +3023,9 @@ bool pci_bridge_d3_possible(struct pci_dev *bridge)
                 if (pci_bridge_d3_disable)
                         return false;

+               if (bridge->no_d3cold)
+                       return false;
+
                 /*
                  * Hotplug ports handled by firmware in System Management Mode
                  * may not be put into D3 by the OS (Thunderbolt on non-Macs).
@@ -3098,7 +3101,11 @@ void pci_bridge_d3_update(struct pci_dev *dev)
         bool d3cold_ok = true;

         bridge = pci_upstream_bridge(dev);
-       if (!bridge || !pci_bridge_d3_possible(bridge))
+       if (!bridge) {
+               dev->bridge_d3 = pci_bridge_d3_possible(dev);
+               return;
+       }
+       if (!pci_bridge_d3_possible(bridge))
                 return;

         /*
--
2.34.1







[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux