On Thu, 2025-04-17 at 09:02 +0000, Vamsi Krishna Attunuru wrote:
>
>
> > -----Original Message-----
> > From: Philipp Stanner <pha...@kernel.org>
> > Sent: Thursday, April 17, 2025 1:32 PM
> > To: Srujana Challa <scha...@marvell.com>; Vamsi Krishna Attunuru
> > <vattun...@marvell.com>; Michael S. Tsirkin <m...@redhat.com>; Jason
> > Wang <jasow...@redhat.com>; Xuan Zhuo <xuanz...@linux.alibaba.com>;
> > Eugenio Pérez <epere...@redhat.com>; Shijith Thotton
> > <sthot...@marvell.com>; Dan Carpenter <dan.carpen...@linaro.org>;
> > Philipp Stanner <pha...@kernel.org>; Satha Koteswara Rao Kottidi
> > <skotesh...@marvell.com>
> > Cc: virtualizat...@lists.linux.dev; linux-kernel@vger.kernel.org
> > Subject: [EXTERNAL] [PATCH] vdpa/octeon_ep: Use non-hybrid PCI
> > devres
> > API
> >
> > octeon enables its PCI device with pcim_enable_device(). This,
> > implicitly,
> > switches the function pci_request_region() into managed mode, where
> > it
> > becomes a devres function. The PCI subsystem wants to remove this
> > hybrid
> > nature from its interfaces.
> > octeon enables its PCI device with pcim_enable_device(). This,
> > implicitly,
> > switches the function pci_request_region() into managed mode, where
> > it
> > becomes a devres function.
> >
> > The PCI subsystem wants to remove this hybrid nature from its
> > interfaces. To
> > do so, users of the aforementioned combination of functions must be
> > ported
> > to non-hybrid functions.
> >
> > Moreover, since both functions are already managed in this driver,
> > the calls to
> > pci_release_region() are unnecessary.
> >
> > Remove the calls to pci_release_region().
> >
> > Replace the call to sometimes-managed pci_request_region() with one
> > to the
> > always-managed pcim_request_region().
>
Hi,
> Thanks you, Philipps, for the patch. The Octeon EP driver does not
> use managed calls for handling resource regions.
> This is because the PCIe PF function reduces its resources to share
> them with its VFs and later restores them to original size
> once the VFs are released. Due to this resource sharing requirement,
> the driver cannot use the always-managed request
> regions calls.
so this would mean that the driver is already broken.
pci_request_region() in your driver is always-managed since you call
pcim_enable_device(). Or am I missing something?
The only way for you to, currently, have non-managed versions is by
using pci_enable_device() instead and then doing pci_disable_device()
manually in remove() and the other appropriate places.
This patch should not change behavior in any way.
If you're sure that having no management is not a problem, then we
could simply drop this patch and I later remove the hybrid feature from
PCI. Then your call to pci_request_region() will become unmanaged, even
if you keep the pcim_enable_device().
Tell me if you have a preference.
P.
>
> >
> > Signed-off-by: Philipp Stanner <pha...@kernel.org>
> > ---
> > drivers/vdpa/octeon_ep/octep_vdpa_main.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> > b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> > index f3d4dda4e04c..e0da6367661e 100644
> > --- a/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> > +++ b/drivers/vdpa/octeon_ep/octep_vdpa_main.c
> > @@ -391,7 +391,7 @@ static int octep_iomap_region(struct pci_dev
> > *pdev,
> > u8 __iomem **tbl, u8 bar) {
> > int ret;
> >
> > - ret = pci_request_region(pdev, bar,
> > OCTEP_VDPA_DRIVER_NAME);
> > + ret = pcim_request_region(pdev, bar,
> > OCTEP_VDPA_DRIVER_NAME);
> > if (ret) {
> > dev_err(&pdev->dev, "Failed to request BAR:%u
> > region\n",
> > bar);
> > return ret;
> > @@ -400,7 +400,6 @@ static int octep_iomap_region(struct pci_dev
> > *pdev,
> > u8 __iomem **tbl, u8 bar)
> > tbl[bar] = pci_iomap(pdev, bar, pci_resource_len(pdev,
> > bar));
> > if (!tbl[bar]) {
> > dev_err(&pdev->dev, "Failed to iomap BAR:%u\n",
> > bar);
> > - pci_release_region(pdev, bar);
> > ret = -ENOMEM;
> > }
> >
> > @@ -410,7 +409,6 @@ static int octep_iomap_region(struct pci_dev
> > *pdev,
> > u8 __iomem **tbl, u8 bar) static void octep_iounmap_region(struct
> > pci_dev
> > *pdev, u8 __iomem **tbl, u8 bar) {
> > pci_iounmap(pdev, tbl[bar]);
> > - pci_release_region(pdev, bar);
> > }
> >
> > static void octep_vdpa_pf_bar_shrink(struct octep_pf *octpf)
> > --
> > 2.48.1
>
