You are right, thanks for catching this. I missed that pci_disable_ptm() now decrements dev->ptm_enable_cnt unconditionally and recurses upstream, so calling it after a failed enable would corrupt the counts of parents shared with other endpoints.
v3 will guard both call sites with pcie_ptm_enabled(), which idpf already uses in idpf_ptp_set_caps(), so no new adapter field is needed. v2 also needs a rebase since pci_enable_ptm() lost its granularity argument. Thanks, Myeonghun 2026년 7월 18일 (토) 오전 5:21, Tony Nguyen <[email protected]>님이 작성: > > > > On 7/15/2026 12:43 AM, Myeonghun Pak wrote: > > idpf_probe() enables PCIe Precision Time Measurement with > > pci_enable_ptm(pdev, NULL), which programs the PTM control bits and sets > > pdev->ptm_enabled when the bus/controller supports it. > > > > If a later probe step fails, the error path releases the allocated > > workqueues and adapter memory without disabling PTM. The remove path has > > the same imbalance when a successfully probed device is detached. In > > both cases, the PCI core's software PTM state and the device's PTM control > > bits remain set with no bound driver. > > > > Add pci_disable_ptm() to the common probe unwind after the PTM enable and > > to idpf_remove(). pci_disable_ptm() is a no-op when PTM was not enabled, > > so the non-fatal pci_enable_ptm() failure remains safe. > > pcim_enable_device() only arranges for pci_disable_device() and does not > > undo the PTM enable. > > > > Fixes: 8d5e12c5921c ("idpf: add initial PTP support") > > Co-developed-by: Ijae Kim <[email protected]> > > Signed-off-by: Ijae Kim <[email protected]> > > Signed-off-by: Myeonghun Pak <[email protected]> > > --- > > Changes in v2: > > - Disable PTM in the probe error path, as requested by Emil Tantilov. > > > > drivers/net/ethernet/intel/idpf/idpf_main.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/idpf/idpf_main.c > > b/drivers/net/ethernet/intel/idpf/idpf_main.c > > index 0dd741dcfc..f08ce13c20 100644 > > --- a/drivers/net/ethernet/intel/idpf/idpf_main.c > > +++ b/drivers/net/ethernet/intel/idpf/idpf_main.c > > @@ -159,6 +159,7 @@ static void idpf_remove(struct pci_dev *pdev) > > mutex_destroy(&adapter->queue_lock); > > mutex_destroy(&adapter->vc_buf_lock); > > > > + pci_disable_ptm(pdev); > > Sashiko says: > > If pci_enable_ptm() failed earlier during idpf_probe() because it was > unsupported by the bus or controller, does this unconditional call to > pci_disable_ptm() corrupt the upstream PTM enable counts? > Looking at pci_disable_ptm(), it does not check if the local device was > successfully enabled. It climbs the PCIe tree and erroneously decrements > atomic counters like ptm_enable_cnt on parent switches. > Could this eventually break PTM for other downstream devices sharing the > same > switch if a parent's count is incorrectly decremented to -1? > > > pci_set_drvdata(pdev, NULL); > > kfree(adapter); > > } > > @@ -266,7 +267,7 @@ static int idpf_probe(struct pci_dev *pdev, const > > struct pci_device_id *ent) > > if (err) { > > pci_err(pdev, "DMA configuration failed: %pe\n", > > ERR_PTR(err)); > > > > - goto err_free; > > + goto err_disable_ptm; > > } > > > > pci_set_master(pdev); > > @@ -279,7 +280,7 @@ static int idpf_probe(struct pci_dev *pdev, const > > struct pci_device_id *ent) > > if (!adapter->init_wq) { > > dev_err(dev, "Failed to allocate init workqueue\n"); > > err = -ENOMEM; > > - goto err_free; > > + goto err_disable_ptm; > > } > > > > adapter->serv_wq = alloc_workqueue("%s-%s-service", > > @@ -366,6 +367,8 @@ err_mbx_wq_alloc: > > destroy_workqueue(adapter->serv_wq); > > err_serv_wq_alloc: > > destroy_workqueue(adapter->init_wq); > > +err_disable_ptm: > > + pci_disable_ptm(pdev); > > Like the issue in idpf_remove(), does this error path unconditionally > disable > PTM even if the earlier enable attempt failed? > If a subsequent probe step fails (such as dma_set_mask_and_coherent or > workqueue allocation), this path is taken regardless of whether > pci_enable_ptm() succeeded. > Would it make sense to track the PTM state in the adapter struct, and only > call pci_disable_ptm() when it was successfully enabled? > > > err_free: > > kfree(adapter); > > return err; >
