If a CXL switch is under a CXL root port, The Port GPF Phase timeout
will be updated on the CXL root port when each cxl memory device under
the CXL switch is attaching. It is possible to be updated more than
once. Actually, it is enough to initialize once, other extra
initializations are redundant.

When the first EP attaching, it always triggers its ancestor dports to
locate their own Port GPF DVSEC. The change is that updating Port GPF
Phase timeout on these ancestor dports after ancestor dport locating a
Port GPF DVSEC. It guaranttess that Port GPF Phase timeout updating on a
dport only happens during the first EP attaching.

Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 drivers/cxl/core/pci.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index aab0a505d527..edbdaf1681e8 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -1130,12 +1130,11 @@ static int update_gpf_port_dvsec(struct pci_dev *pdev, int dvsec, int phase)
 
 int cxl_gpf_port_setup(struct cxl_dport *dport)
 {
-	struct pci_dev *pdev;
-
 	if (!dport)
 		return -EINVAL;
 
 	if (!dport->gpf_dvsec) {
+		struct pci_dev *pdev;
 		int dvsec;
 
 		dvsec = cxl_gpf_get_dvsec(dport->dport_dev, true);
@@ -1143,11 +1142,10 @@ int cxl_gpf_port_setup(struct cxl_dport *dport)
 			return -EINVAL;
 
 		dport->gpf_dvsec = dvsec;
+		pdev = to_pci_dev(dport->dport_dev);
+		update_gpf_port_dvsec(pdev, dport->gpf_dvsec, 1);
+		update_gpf_port_dvsec(pdev, dport->gpf_dvsec, 2);
 	}
 
-	pdev = to_pci_dev(dport->dport_dev);
-	update_gpf_port_dvsec(pdev, dport->gpf_dvsec, 1);
-	update_gpf_port_dvsec(pdev, dport->gpf_dvsec, 2);
-
 	return 0;
 }
-- 
2.34.1

On Wed, 19 Mar 2025, Li Ming wrote:

>If a CXL switch is under a CXL root port, The Port GPF Phase timeout
>will be updated on the CXL root port when each cxl memory device under
>the CXL switch is attaching. It is possible to be updated more than
>once. Actually, it is enough to initialize once, other extra
>initializations are redundant.

It's actually not updated more than necessary because update_gpf_port_dvsec()
checks first:

	if (FIELD_GET(base, ctrl) == GPF_TIMEOUT_BASE_MAX &&
	    FIELD_GET(scale, ctrl) == GPF_TIMEOUT_SCALE_MAX)
		return 0;

>When the first EP attaching, it always triggers its ancestor dports to
>locate their own Port GPF DVSEC. The change is that updating Port GPF
>Phase timeout on these ancestor dports after ancestor dport locating a
>Port GPF DVSEC. It guaranttess that Port GPF Phase timeout updating on a

s/guaranttess/guarantees

>dport only happens during the first EP attaching.

... but yeah, I think this is still better, logically.

Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>

(with the caveat that if patch 1 is not necessary then this would need to
be redone).

On Thu, 20 Mar 2025 22:40:07 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:

> On Wed, 19 Mar 2025, Li Ming wrote:
> 
> >If a CXL switch is under a CXL root port, The Port GPF Phase timeout
> >will be updated on the CXL root port when each cxl memory device under
> >the CXL switch is attaching. It is possible to be updated more than
> >once. Actually, it is enough to initialize once, other extra
> >initializations are redundant.  
> 
> It's actually not updated more than necessary because update_gpf_port_dvsec()
> checks first:
> 
> 	if (FIELD_GET(base, ctrl) == GPF_TIMEOUT_BASE_MAX &&
> 	    FIELD_GET(scale, ctrl) == GPF_TIMEOUT_SCALE_MAX)
> 		return 0;
> 
> >When the first EP attaching, it always triggers its ancestor dports to
> >locate their own Port GPF DVSEC. The change is that updating Port GPF
> >Phase timeout on these ancestor dports after ancestor dport locating a
> >Port GPF DVSEC. It guaranttess that Port GPF Phase timeout updating on a  
> 
> s/guaranttess/guarantees
> 
> >dport only happens during the first EP attaching.  
> 
> ... but yeah, I think this is still better, logically.
> 
> Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Agree that this seems sensible.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> (with the caveat that if patch 1 is not necessary then this would need to
> be redone).