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

[Philipp Stanner <phasta@mailbox.org>]

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;
>  }
>