[PATCH v2] vdpa/octeon_ep: Control PCI dev enabling manually

[PATCH v2] vdpa/octeon_ep: Control PCI dev enabling manually

[Philipp Stanner]

PCI region request functions such as pci_request_region() currently have
the problem of becoming sometimes managed functions, if
pcim_enable_device() instead of pci_enable_device() was called. The PCI
subsystem wants to remove this deprecated behavior from its interfaces.

octeopn_ep enables its device with pcim_enable_device(), but does so
only to get automatic disablement. The driver wants to manage its PCI
resources manually, without devres.

The easiest way not to use automatic resource management at all is by
also handling device enable- and disablement manually.

Replace pcim_enable_device() with pci_enable_device(). Add the necessary
calls to pci_disable_device().

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
Was formerly called "Use non-hybrid PCI devres API"

Changes in v2:
  - Don't replace the request functions. Enable / disable manually
    instead. (Vamsi)
---
 drivers/vdpa/octeon_ep/octep_vdpa_main.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/vdpa/octeon_ep/octep_vdpa_main.c b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
index f3d4dda4e04c..0a282f9186f4 100644
--- a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
+++ b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
@@ -492,6 +492,8 @@ static void octep_vdpa_remove(struct pci_dev *pdev)
 		octep_vdpa_remove_vf(pdev);
 	else
 		octep_vdpa_remove_pf(pdev);
+
+	pci_disable_device(pdev);
 }
 
 static int octep_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
@@ -825,7 +827,7 @@ static int octep_vdpa_probe_pf(struct pci_dev *pdev)
 	struct octep_pf *octpf;
 	int ret;
 
-	ret = pcim_enable_device(pdev);
+	ret = pci_enable_device(pdev);
 	if (ret) {
 		dev_err(dev, "Failed to enable device\n");
 		return ret;
@@ -834,15 +836,17 @@ static int octep_vdpa_probe_pf(struct pci_dev *pdev)
 	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
 	if (ret) {
 		dev_err(dev, "No usable DMA configuration\n");
-		return ret;
+		goto disable_pci;
 	}
 	octpf = devm_kzalloc(dev, sizeof(*octpf), GFP_KERNEL);
-	if (!octpf)
-		return -ENOMEM;
+	if (!octpf) {
+		ret = -ENOMEM;
+		goto disable_pci;
+	}
 
 	ret = octep_iomap_region(pdev, octpf->base, OCTEP_HW_MBOX_BAR);
 	if (ret)
-		return ret;
+		goto disable_pci;
 
 	pci_set_master(pdev);
 	pci_set_drvdata(pdev, octpf);
@@ -856,6 +860,8 @@ static int octep_vdpa_probe_pf(struct pci_dev *pdev)
 
 unmap_region:
 	octep_iounmap_region(pdev, octpf->base, OCTEP_HW_MBOX_BAR);
+disable_pci:
+	pci_disable_device(pdev);
 	return ret;
 }
 
-- 
2.48.1

Re: [PATCH v2] vdpa/octeon_ep: Control PCI dev enabling manually

[Philipp Stanner]

On Wed, 2025-04-23 at 12:39 +0200, Philipp Stanner wrote:
> PCI region request functions such as pci_request_region() currently
> have
> the problem of becoming sometimes managed functions, if
> pcim_enable_device() instead of pci_enable_device() was called. The
> PCI
> subsystem wants to remove this deprecated behavior from its
> interfaces.
> 
> octeopn_ep enables its device with pcim_enable_device(), but does so
> only to get automatic disablement. The driver wants to manage its PCI
> resources manually, without devres.
> 
> The easiest way not to use automatic resource management at all is by
> also handling device enable- and disablement manually.
> 
> Replace pcim_enable_device() with pci_enable_device(). Add the
> necessary
> calls to pci_disable_device().	struct
> list_head		pending_list;
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
> Was formerly called "Use non-hybrid PCI devres API"
> 
> Changes in v2:
>   - Don't replace the request functions. Enable / disable manually
>     instead. (Vamsi)
> ---
>  drivers/vdpa/octeon_ep/octep_vdpa_main.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> index f3d4dda4e04c..0a282f9186f4 100644
> --- a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> +++ b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> @@ -492,6 +492,8 @@ static void octep_vdpa_remove(struct pci_dev
> *pdev)
>  		octep_vdpa_remove_vf(pdev);
>  	else
>  		octep_vdpa_remove_pf(pdev);
> +
> +	pci_disable_device(pdev);

Taking a second glance, we probably want to do this only for PF, don't
we?

So VF would use pcim_enable_device(), but none of the request
functions, and PF would replace its pcim_enable_device(), and to
compensate for that only the PF remove / deactivate path would actively
call pci_disable_device. Right?

P.

>  }
>  
>  static int octep_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char
> *name,
> @@ -825,7 +827,7 @@ static int octep_vdpa_probe_pf(struct pci_dev
> *pdev)
>  	struct octep_pf *octpf;
>  	int ret;
>  
> -	ret = pcim_enable_device(pdev);
> +	ret = pci_enable_device(pdev);
>  	if (ret) {
>  		dev_err(dev, "Failed to enable device\n");
>  		return ret;
> @@ -834,15 +836,17 @@ static int octep_vdpa_probe_pf(struct pci_dev
> *pdev)
>  	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
>  	if (ret) {
>  		dev_err(dev, "No usable DMA configuration\n");
> -		return ret;
> +		goto disable_pci;
>  	}
>  	octpf = devm_kzalloc(dev, sizeof(*octpf), GFP_KERNEL);
> -	if (!octpf)
> -		return -ENOMEM;
> +	if (!octpf) {
> +		ret = -ENOMEM;
> +		goto disable_pci;
> +	}
>  
>  	ret = octep_iomap_region(pdev, octpf->base,
> OCTEP_HW_MBOX_BAR);
>  	if (ret)
> -		return ret;
> +		goto disable_pci;
>  
>  	pci_set_master(pdev);
>  	pci_set_drvdata(pdev, octpf);
> @@ -856,6 +860,8 @@ static int octep_vdpa_probe_pf(struct pci_dev
> *pdev)
>  
>  unmap_region:
>  	octep_iounmap_region(pdev, octpf->base, OCTEP_HW_MBOX_BAR);
> +disable_pci:
> +	pci_disable_device(pdev);
>  	return ret;
>  }
>