[PATCH v2 0/6] PCI: Remove hybrid-devres region requests

[PATCH v2 0/6] PCI: Remove hybrid-devres region requests

[Philipp Stanner]

Changes in v2:
  - Drop patch for removing forgotten header. Patch is unrelated. Will
    resend seperately. (Andy)
  - Make docu patch headline "Documentation/driver-api:". There seems to
    be no canonical way, but this style is quite frequent. (Andy)
  - Apply Andy's RBs where applicable.

Howdy,

the great day has finally arrived, I managed to get rid of one of the
big three remaining problems in the PCI devres API (the other two being
MSI having hybrid-devres, too, and the good old pcim_iomap_tablle)!

It turned out that there aren't even that many users of the hybrid API,
where pcim_enable_device() switches certain functions in pci.c into
managed devres mode, which we want to remove.

The affected drivers can be found with:

grep -rlZ "pcim_enable_device" | xargs -0 grep -l "pci_request"

These were:

	ASoC [1]
	alsa [2] 
	cardreader [3]
	cirrus [4]
	i2c [5]
	mmc [6]
	mtd [7]
	mxser [8]
	net [9]
	spi [10]
	vdpa [11]
	vmwgfx [12]

All of those have been merged and are queued up for the merge window.
The only possible exception is vdpa, but it seems to be ramped up right
now; vdpa, however, doesn't even use the hybrid behavior, so that patch
is just for generic cleanup anyways.

With the users of the hybrid feature gone, the feature itself can
finally be burned.

So I'm sending out this series now to probe whether it's judged to be
good enough for the upcoming merge window. If we could take it, we would
make it impossible that anyone adds new users of the hybrid thing.

If it's too late for the merge window, then that's what it is, of
course.

In any case I'm glad we can get rid of most of that legacy stuff now.

Regards,
Philipp

[1] https://lore.kernel.org/all/174657893832.4155013.12131767110464880040.b4-ty@kernel.org/
[2] https://lore.kernel.org/all/8734dy3tvz.wl-tiwai@suse.de/
[3] https://lore.kernel.org/all/20250417091532.26520-2-phasta@kernel.org/ (private confirmation mail from Greg KH)
[4] https://lore.kernel.org/dri-devel/e7c45c099f8981257866396e01a91df1afcfbf97.camel@mailbox.org/
[5] https://lore.kernel.org/all/l26azmnpceka2obq4gtwozziq6lbilb2owx57aajtp3t6jhd3w@llmeikgjvqyh/
[6] https://lore.kernel.org/all/CAPDyKFqqV2VEqi17UHmFE0b9Y+h5q2YaNfHTux8U=7DgF+svEw@mail.gmail.com/
[7] https://lore.kernel.org/all/174591865790.993381.15992314896975862083.b4-ty@bootlin.com/
[8] https://lore.kernel.org/all/20250417081333.20917-2-phasta@kernel.org/ (private confirmation mail from Greg KH)
[9] https://lore.kernel.org/all/174588423950.1081621.6688170836136857875.git-patchwork-notify@kernel.org/
[10] https://lore.kernel.org/all/174492457740.248895.3318833401427095151.b4-ty@kernel.org/
[11] https://lore.kernel.org/all/20250515072724-mutt-send-email-mst@kernel.org/
[12] https://lore.kernel.org/dri-devel/CABQX2QNQbO4dMq-Hi6tvpi7OTwcVfjM62eCr1OGkzF8Phy-Shw@mail.gmail.com/

Philipp Stanner (6):
  PCI: Remove hybrid devres nature from request functions
  Documentation/driver-api: Update pcim_enable_device()
  PCI: Remove pcim_request_region_exclusive()
  PCI: Remove request_flags relict from devres
  PCI: Remove redundant set of request funcs
  PCI: Remove hybrid-devres hazzard warnings from doc

 .../driver-api/driver-model/devres.rst        |   2 +-
 drivers/pci/devres.c                          | 201 +++---------------
 drivers/pci/iomap.c                           |  16 --
 drivers/pci/pci.c                             |  42 ----
 drivers/pci/pci.h                             |   3 -
 5 files changed, 32 insertions(+), 232 deletions(-)

-- 
2.49.0

[PATCH v2 1/6] PCI: Remove hybrid devres nature from request functions

[Philipp Stanner]

All functions based on __pci_request_region() and its release counter
part support "hybrid mode", where the functions become managed if the
PCI device was enabled with pcim_enable_device().

Removing this undesirable feature requires to remove all users who
activated their device with that function and use one of the affected
request functions.

These users were:
	ASoC
	alsa
	cardreader
	cirrus
	i2c
	mmc
	mtd
	mtd
	mxser
	net
	spi
	vdpa
	vmwgfx

all of which have been ported to always-managed pcim_ functions by now.

The hybrid nature can, thus, be removed from the aforementioned PCI
functions.

Remove all function guards and documentation in pci.c related to the
hybrid redirection. Adjust the visibility of pcim_release_region().

Signed-off-by: Philipp Stanner <phasta@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pci/devres.c | 39 ++++++++++++---------------------------
 drivers/pci/pci.c    | 42 ------------------------------------------
 drivers/pci/pci.h    |  1 -
 3 files changed, 12 insertions(+), 70 deletions(-)

diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 73047316889e..5480d537f400 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -6,30 +6,13 @@
 /*
  * On the state of PCI's devres implementation:
  *
- * The older devres API for PCI has two significant problems:
+ * The older PCI devres API has one significant problem:
  *
- * 1. It is very strongly tied to the statically allocated mapping table in
- *    struct pcim_iomap_devres below. This is mostly solved in the sense of the
- *    pcim_ functions in this file providing things like ranged mapping by
- *    bypassing this table, whereas the functions that were present in the old
- *    API still enter the mapping addresses into the table for users of the old
- *    API.
- *
- * 2. The region-request-functions in pci.c do become managed IF the device has
- *    been enabled with pcim_enable_device() instead of pci_enable_device().
- *    This resulted in the API becoming inconsistent: Some functions have an
- *    obviously managed counter-part (e.g., pci_iomap() <-> pcim_iomap()),
- *    whereas some don't and are never managed, while others don't and are
- *    _sometimes_ managed (e.g. pci_request_region()).
- *
- *    Consequently, in the new API, region requests performed by the pcim_
- *    functions are automatically cleaned up through the devres callback
- *    pcim_addr_resource_release().
- *
- *    Users of pcim_enable_device() + pci_*region*() are redirected in
- *    pci.c to the managed functions here in this file. This isn't exactly
- *    perfect, but the only alternative way would be to port ALL drivers
- *    using said combination to pcim_ functions.
+ * It is very strongly tied to the statically allocated mapping table in struct
+ * pcim_iomap_devres below. This is mostly solved in the sense of the pcim_
+ * functions in this file providing things like ranged mapping by bypassing
+ * this table, whereas the functions that were present in the old API still
+ * enter the mapping addresses into the table for users of the old API.
  *
  * TODO:
  * Remove the legacy table entirely once all calls to pcim_iomap_table() in
@@ -89,10 +72,12 @@ static inline void pcim_addr_devres_clear(struct pcim_addr_devres *res)
 
 /*
  * The following functions, __pcim_*_region*, exist as counterparts to the
- * versions from pci.c - which, unfortunately, can be in "hybrid mode", i.e.,
- * sometimes managed, sometimes not.
+ * versions from pci.c - which, unfortunately, were in the past in "hybrid
+ * mode", i.e., sometimes managed, sometimes not.
  *
- * To separate the APIs cleanly, we define our own, simplified versions here.
+ * To separate the APIs cleanly, we defined our own, simplified versions here.
+ *
+ * TODO: unify those functions with the counterparts in pci.c
  */
 
 /**
@@ -893,7 +878,7 @@ int pcim_request_region_exclusive(struct pci_dev *pdev, int bar, const char *nam
  * Release a region manually that was previously requested by
  * pcim_request_region().
  */
-void pcim_release_region(struct pci_dev *pdev, int bar)
+static void pcim_release_region(struct pci_dev *pdev, int bar)
 {
 	struct pcim_addr_devres res_searched;
 
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e77d5b53c0ce..4acc23823637 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3937,16 +3937,6 @@ void pci_release_region(struct pci_dev *pdev, int bar)
 	if (!pci_bar_index_is_valid(bar))
 		return;
 
-	/*
-	 * This is done for backwards compatibility, because the old PCI devres
-	 * API had a mode in which the function became managed if it had been
-	 * enabled with pcim_enable_device() instead of pci_enable_device().
-	 */
-	if (pci_is_managed(pdev)) {
-		pcim_release_region(pdev, bar);
-		return;
-	}
-
 	if (pci_resource_len(pdev, bar) == 0)
 		return;
 	if (pci_resource_flags(pdev, bar) & IORESOURCE_IO)
@@ -3984,13 +3974,6 @@ static int __pci_request_region(struct pci_dev *pdev, int bar,
 	if (!pci_bar_index_is_valid(bar))
 		return -EINVAL;
 
-	if (pci_is_managed(pdev)) {
-		if (exclusive == IORESOURCE_EXCLUSIVE)
-			return pcim_request_region_exclusive(pdev, bar, name);
-
-		return pcim_request_region(pdev, bar, name);
-	}
-
 	if (pci_resource_len(pdev, bar) == 0)
 		return 0;
 
@@ -4027,11 +4010,6 @@ static int __pci_request_region(struct pci_dev *pdev, int bar,
  *
  * Returns 0 on success, or %EBUSY on error.  A warning
  * message is also printed on failure.
- *
- * NOTE:
- * This is a "hybrid" function: It's normally unmanaged, but becomes managed
- * when pcim_enable_device() has been called in advance. This hybrid feature is
- * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
  */
 int pci_request_region(struct pci_dev *pdev, int bar, const char *name)
 {
@@ -4084,11 +4062,6 @@ static int __pci_request_selected_regions(struct pci_dev *pdev, int bars,
  * @name: Name of the driver requesting the resources
  *
  * Returns: 0 on success, negative error code on failure.
- *
- * NOTE:
- * This is a "hybrid" function: It's normally unmanaged, but becomes managed
- * when pcim_enable_device() has been called in advance. This hybrid feature is
- * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
  */
 int pci_request_selected_regions(struct pci_dev *pdev, int bars,
 				 const char *name)
@@ -4104,11 +4077,6 @@ EXPORT_SYMBOL(pci_request_selected_regions);
  * @name: name of the driver requesting the resources
  *
  * Returns: 0 on success, negative error code on failure.
- *
- * NOTE:
- * This is a "hybrid" function: It's normally unmanaged, but becomes managed
- * when pcim_enable_device() has been called in advance. This hybrid feature is
- * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
  */
 int pci_request_selected_regions_exclusive(struct pci_dev *pdev, int bars,
 					   const char *name)
@@ -4144,11 +4112,6 @@ EXPORT_SYMBOL(pci_release_regions);
  *
  * Returns 0 on success, or %EBUSY on error.  A warning
  * message is also printed on failure.
- *
- * NOTE:
- * This is a "hybrid" function: It's normally unmanaged, but becomes managed
- * when pcim_enable_device() has been called in advance. This hybrid feature is
- * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
  */
 int pci_request_regions(struct pci_dev *pdev, const char *name)
 {
@@ -4173,11 +4136,6 @@ EXPORT_SYMBOL(pci_request_regions);
  *
  * Returns 0 on success, or %EBUSY on error.  A warning message is also
  * printed on failure.
- *
- * NOTE:
- * This is a "hybrid" function: It's normally unmanaged, but becomes managed
- * when pcim_enable_device() has been called in advance. This hybrid feature is
- * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
  */
 int pci_request_regions_exclusive(struct pci_dev *pdev, const char *name)
 {
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index b81e99cd4b62..8c3e5fb2443a 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1062,7 +1062,6 @@ static inline pci_power_t mid_pci_get_power_state(struct pci_dev *pdev)
 int pcim_intx(struct pci_dev *dev, int enable);
 int pcim_request_region_exclusive(struct pci_dev *pdev, int bar,
 				  const char *name);
-void pcim_release_region(struct pci_dev *pdev, int bar);
 
 /*
  * Config Address for PCI Configuration Mechanism #1
-- 
2.49.0

[PATCH v2 2/6] Documentation/driver-api: Update pcim_enable_device()

[Philipp Stanner]

pcim_enable_device() is not related anymore to switching the mode of
operation of any functions. It merely sets up a devres callback for
automatically disabling the PCI device on driver detach.

Adjust the function's documentation.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 Documentation/driver-api/driver-model/devres.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
index d75728eb05f8..9443911c4742 100644
--- a/Documentation/driver-api/driver-model/devres.rst
+++ b/Documentation/driver-api/driver-model/devres.rst
@@ -391,7 +391,7 @@ PCI
   devm_pci_remap_cfgspace()	: ioremap PCI configuration space
   devm_pci_remap_cfg_resource()	: ioremap PCI configuration space resource
 
-  pcim_enable_device()		: after success, some PCI ops become managed
+  pcim_enable_device()		: after success, PCI dev gets deactivated automatically
   pcim_iomap()			: do iomap() on a single BAR
   pcim_iomap_regions()		: do request_region() and iomap() on multiple BARs
   pcim_iomap_table()		: array of mapped addresses indexed by BAR
-- 
2.49.0

[PATCH v2 3/6] PCI: Remove pcim_request_region_exclusive()

[Philipp Stanner]

pcim_request_region_exclusive() was only needed for redirecting the
relatively exotic exclusive request functions in pci.c in case of them
operating in managed mode.

The managed nature has been removed from those functions and no one else
uses pcim_request_region_exclusive().

Remove pcim_request_region_exclusive().

Signed-off-by: Philipp Stanner <phasta@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pci/devres.c | 18 ------------------
 drivers/pci/pci.h    |  2 --
 2 files changed, 20 deletions(-)

diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 5480d537f400..769b92f4f66a 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -852,24 +852,6 @@ int pcim_request_region(struct pci_dev *pdev, int bar, const char *name)
 }
 EXPORT_SYMBOL(pcim_request_region);
 
-/**
- * pcim_request_region_exclusive - Request a PCI BAR exclusively
- * @pdev: PCI device to request region for
- * @bar: Index of BAR to request
- * @name: Name of the driver requesting the resource
- *
- * Returns: 0 on success, a negative error code on failure.
- *
- * Request region specified by @bar exclusively.
- *
- * The region will automatically be released on driver detach. If desired,
- * release manually only with pcim_release_region().
- */
-int pcim_request_region_exclusive(struct pci_dev *pdev, int bar, const char *name)
-{
-	return _pcim_request_region(pdev, bar, name, IORESOURCE_EXCLUSIVE);
-}
-
 /**
  * pcim_release_region - Release a PCI BAR
  * @pdev: PCI device to operate on
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 8c3e5fb2443a..cfc9e71a4d84 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -1060,8 +1060,6 @@ static inline pci_power_t mid_pci_get_power_state(struct pci_dev *pdev)
 #endif
 
 int pcim_intx(struct pci_dev *dev, int enable);
-int pcim_request_region_exclusive(struct pci_dev *pdev, int bar,
-				  const char *name);
 
 /*
  * Config Address for PCI Configuration Mechanism #1
-- 
2.49.0

[PATCH v2 4/6] PCI: Remove request_flags relict from devres

[Philipp Stanner]

pcim_request_region_exclusive(), the only user in PCI devres that needed
exclusive region requests, has been removed.

All features related to exclusive requests can, therefore, be removed,
too. Remove them.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
---
 drivers/pci/devres.c | 46 +++++++++++++++++++-------------------------
 1 file changed, 20 insertions(+), 26 deletions(-)

diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index 769b92f4f66a..ae79e5f95c8a 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -808,31 +808,6 @@ int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name)
 }
 EXPORT_SYMBOL(pcim_iomap_regions);
 
-static int _pcim_request_region(struct pci_dev *pdev, int bar, const char *name,
-				int request_flags)
-{
-	int ret;
-	struct pcim_addr_devres *res;
-
-	if (!pci_bar_index_is_valid(bar))
-		return -EINVAL;
-
-	res = pcim_addr_devres_alloc(pdev);
-	if (!res)
-		return -ENOMEM;
-	res->type = PCIM_ADDR_DEVRES_TYPE_REGION;
-	res->bar = bar;
-
-	ret = __pcim_request_region(pdev, bar, name, request_flags);
-	if (ret != 0) {
-		pcim_addr_devres_free(res);
-		return ret;
-	}
-
-	devres_add(&pdev->dev, res);
-	return 0;
-}
-
 /**
  * pcim_request_region - Request a PCI BAR
  * @pdev: PCI device to request region for
@@ -848,7 +823,26 @@ static int _pcim_request_region(struct pci_dev *pdev, int bar, const char *name,
  */
 int pcim_request_region(struct pci_dev *pdev, int bar, const char *name)
 {
-	return _pcim_request_region(pdev, bar, name, 0);
+	int ret;
+	struct pcim_addr_devres *res;
+
+	if (!pci_bar_index_is_valid(bar))
+		return -EINVAL;
+
+	res = pcim_addr_devres_alloc(pdev);
+	if (!res)
+		return -ENOMEM;
+	res->type = PCIM_ADDR_DEVRES_TYPE_REGION;
+	res->bar = bar;
+
+	ret = __pcim_request_region(pdev, bar, name, 0);
+	if (ret != 0) {
+		pcim_addr_devres_free(res);
+		return ret;
+	}
+
+	devres_add(&pdev->dev, res);
+	return 0;
 }
 EXPORT_SYMBOL(pcim_request_region);
 
-- 
2.49.0

[PATCH v2 5/6] PCI: Remove redundant set of request funcs

[Philipp Stanner]

When the demangling of the hybrid devres functions within PCI was
implemented, it was necessary to implement several PCI functions a
second time to avoid cyclic calls, since the hybrid functions in pci.c
call the managed functions in devres.c, which in turn can be directly
used outside of PCI and needed request infrastructure, too.

Therefore, __pcim_request_region_range(), __pci_release_region_range()
and wrappers around them were implemented.

The hybrid nature has recently been removed from all functions in pci.c.
Therefore, the functions in devres.c can now directly use their
counterparts in pci.c without causing a call-cycle.

Remove __pcim_request_region_range(), __pcim_request_region_range() and
the wrappers. Use the corresponding request functions from pci.c in
devres.c

Signed-off-by: Philipp Stanner <phasta@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pci/devres.c | 110 ++-----------------------------------------
 1 file changed, 5 insertions(+), 105 deletions(-)

diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index ae79e5f95c8a..4a4604b78b90 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -70,106 +70,6 @@ static inline void pcim_addr_devres_clear(struct pcim_addr_devres *res)
 	res->bar = -1;
 }
 
-/*
- * The following functions, __pcim_*_region*, exist as counterparts to the
- * versions from pci.c - which, unfortunately, were in the past in "hybrid
- * mode", i.e., sometimes managed, sometimes not.
- *
- * To separate the APIs cleanly, we defined our own, simplified versions here.
- *
- * TODO: unify those functions with the counterparts in pci.c
- */
-
-/**
- * __pcim_request_region_range - Request a ranged region
- * @pdev: PCI device the region belongs to
- * @bar: BAR the range is within
- * @offset: offset from the BAR's start address
- * @maxlen: length in bytes, beginning at @offset
- * @name: name of the driver requesting the resource
- * @req_flags: flags for the request, e.g., for kernel-exclusive requests
- *
- * Returns: 0 on success, a negative error code on failure.
- *
- * Request a range within a device's PCI BAR.  Sanity check the input.
- */
-static int __pcim_request_region_range(struct pci_dev *pdev, int bar,
-				       unsigned long offset,
-				       unsigned long maxlen,
-				       const char *name, int req_flags)
-{
-	resource_size_t start = pci_resource_start(pdev, bar);
-	resource_size_t len = pci_resource_len(pdev, bar);
-	unsigned long dev_flags = pci_resource_flags(pdev, bar);
-
-	if (start == 0 || len == 0) /* Unused BAR. */
-		return 0;
-	if (len <= offset)
-		return -EINVAL;
-
-	start += offset;
-	len -= offset;
-
-	if (len > maxlen && maxlen != 0)
-		len = maxlen;
-
-	if (dev_flags & IORESOURCE_IO) {
-		if (!request_region(start, len, name))
-			return -EBUSY;
-	} else if (dev_flags & IORESOURCE_MEM) {
-		if (!__request_mem_region(start, len, name, req_flags))
-			return -EBUSY;
-	} else {
-		/* That's not a device we can request anything on. */
-		return -ENODEV;
-	}
-
-	return 0;
-}
-
-static void __pcim_release_region_range(struct pci_dev *pdev, int bar,
-					unsigned long offset,
-					unsigned long maxlen)
-{
-	resource_size_t start = pci_resource_start(pdev, bar);
-	resource_size_t len = pci_resource_len(pdev, bar);
-	unsigned long flags = pci_resource_flags(pdev, bar);
-
-	if (len <= offset || start == 0)
-		return;
-
-	if (len == 0 || maxlen == 0) /* This an unused BAR. Do nothing. */
-		return;
-
-	start += offset;
-	len -= offset;
-
-	if (len > maxlen)
-		len = maxlen;
-
-	if (flags & IORESOURCE_IO)
-		release_region(start, len);
-	else if (flags & IORESOURCE_MEM)
-		release_mem_region(start, len);
-}
-
-static int __pcim_request_region(struct pci_dev *pdev, int bar,
-				 const char *name, int flags)
-{
-	unsigned long offset = 0;
-	unsigned long len = pci_resource_len(pdev, bar);
-
-	return __pcim_request_region_range(pdev, bar, offset, len, name, flags);
-}
-
-static void __pcim_release_region(struct pci_dev *pdev, int bar)
-{
-	unsigned long offset = 0;
-	unsigned long len = pci_resource_len(pdev, bar);
-
-	__pcim_release_region_range(pdev, bar, offset, len);
-}
-
 static void pcim_addr_resource_release(struct device *dev, void *resource_raw)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
@@ -177,11 +77,11 @@ static void pcim_addr_resource_release(struct device *dev, void *resource_raw)
 
 	switch (res->type) {
 	case PCIM_ADDR_DEVRES_TYPE_REGION:
-		__pcim_release_region(pdev, res->bar);
+		pci_release_region(pdev, res->bar);
 		break;
 	case PCIM_ADDR_DEVRES_TYPE_REGION_MAPPING:
 		pci_iounmap(pdev, res->baseaddr);
-		__pcim_release_region(pdev, res->bar);
+		pci_release_region(pdev, res->bar);
 		break;
 	case PCIM_ADDR_DEVRES_TYPE_MAPPING:
 		pci_iounmap(pdev, res->baseaddr);
@@ -720,7 +620,7 @@ void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar,
 	res->type = PCIM_ADDR_DEVRES_TYPE_REGION_MAPPING;
 	res->bar = bar;
 
-	ret = __pcim_request_region(pdev, bar, name, 0);
+	ret = pci_request_region(pdev, bar, name);
 	if (ret != 0)
 		goto err_region;
 
@@ -734,7 +634,7 @@ void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar,
 	return res->baseaddr;
 
 err_iomap:
-	__pcim_release_region(pdev, bar);
+	pci_release_region(pdev, bar);
 err_region:
 	pcim_addr_devres_free(res);
 
@@ -835,7 +735,7 @@ int pcim_request_region(struct pci_dev *pdev, int bar, const char *name)
 	res->type = PCIM_ADDR_DEVRES_TYPE_REGION;
 	res->bar = bar;
 
-	ret = __pcim_request_region(pdev, bar, name, 0);
+	ret = pci_request_region(pdev, bar, name);
 	if (ret != 0) {
 		pcim_addr_devres_free(res);
 		return ret;
-- 
2.49.0

[PATCH v2 6/6] PCI: Remove hybrid-devres hazzard warnings from doc

[Philipp Stanner]

pci/iomap.c still contains warnings about those functions not behaving
in a managed manner if pcim_enable_device() was called. Since all hybrid
behavior that users could know about has been removed by now, those
explicit warnings are no longer necessary.

Remove the hybrid-devres warnings from the docstrings.

Signed-off-by: Philipp Stanner <phasta@kernel.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/pci/iomap.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/pci/iomap.c b/drivers/pci/iomap.c
index fe706ed946df..ea86c282a386 100644
--- a/drivers/pci/iomap.c
+++ b/drivers/pci/iomap.c
@@ -25,10 +25,6 @@
  *
  * @maxlen specifies the maximum length to map. If you want to get access to
  * the complete BAR from offset to the end, pass %0 here.
- *
- * NOTE:
- * This function is never managed, even if you initialized with
- * pcim_enable_device().
  * */
 void __iomem *pci_iomap_range(struct pci_dev *dev,
 			      int bar,
@@ -76,10 +72,6 @@ EXPORT_SYMBOL(pci_iomap_range);
  *
  * @maxlen specifies the maximum length to map. If you want to get access to
  * the complete BAR from offset to the end, pass %0 here.
- *
- * NOTE:
- * This function is never managed, even if you initialized with
- * pcim_enable_device().
  * */
 void __iomem *pci_iomap_wc_range(struct pci_dev *dev,
 				 int bar,
@@ -127,10 +119,6 @@ EXPORT_SYMBOL_GPL(pci_iomap_wc_range);
  *
  * @maxlen specifies the maximum length to map. If you want to get access to
  * the complete BAR without checking for its length first, pass %0 here.
- *
- * NOTE:
- * This function is never managed, even if you initialized with
- * pcim_enable_device(). If you need automatic cleanup, use pcim_iomap().
  * */
 void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
 {
@@ -152,10 +140,6 @@ EXPORT_SYMBOL(pci_iomap);
  *
  * @maxlen specifies the maximum length to map. If you want to get access to
  * the complete BAR without checking for its length first, pass %0 here.
- *
- * NOTE:
- * This function is never managed, even if you initialized with
- * pcim_enable_device().
  * */
 void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long maxlen)
 {
-- 
2.49.0

Re: [PATCH v2 0/6] PCI: Remove hybrid-devres region requests

[Krzysztof Wilczyński]

Hello,

[...]
> the great day has finally arrived, I managed to get rid of one of the
> big three remaining problems in the PCI devres API (the other two being
> MSI having hybrid-devres, too, and the good old pcim_iomap_tablle)!
> 
> It turned out that there aren't even that many users of the hybrid API,
> where pcim_enable_device() switches certain functions in pci.c into
> managed devres mode, which we want to remove.
> 
> The affected drivers can be found with:
> 
> grep -rlZ "pcim_enable_device" | xargs -0 grep -l "pci_request"
> 
> These were:
> 
> 	ASoC [1]
> 	alsa [2] 
> 	cardreader [3]
> 	cirrus [4]
> 	i2c [5]
> 	mmc [6]
> 	mtd [7]
> 	mxser [8]
> 	net [9]
> 	spi [10]
> 	vdpa [11]
> 	vmwgfx [12]
> 
> All of those have been merged and are queued up for the merge window.
> The only possible exception is vdpa, but it seems to be ramped up right
> now; vdpa, however, doesn't even use the hybrid behavior, so that patch
> is just for generic cleanup anyways.
> 
> With the users of the hybrid feature gone, the feature itself can
> finally be burned.
> 
> So I'm sending out this series now to probe whether it's judged to be
> good enough for the upcoming merge window. If we could take it, we would
> make it impossible that anyone adds new users of the hybrid thing.
> 
> If it's too late for the merge window, then that's what it is, of
> course.
> 
> In any case I'm glad we can get rid of most of that legacy stuff now.

Thank you for sending a v2!  Much appreciated.

I pulled tentatively to the for-ci/devres branch, to get some soak time
with 0-day bot and KernelCI, while we wait for reviews and such.

Thank you!

	Krzysztof

Re: [PATCH v2 2/6] Documentation/driver-api: Update pcim_enable_device()

[Randy Dunlap]

Hi,

On 5/16/25 10:41 AM, Philipp Stanner wrote:
> pcim_enable_device() is not related anymore to switching the mode of
> operation of any functions. It merely sets up a devres callback for
> automatically disabling the PCI device on driver detach.
> 
> Adjust the function's documentation.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>  Documentation/driver-api/driver-model/devres.rst | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
> index d75728eb05f8..9443911c4742 100644
> --- a/Documentation/driver-api/driver-model/devres.rst
> +++ b/Documentation/driver-api/driver-model/devres.rst
> @@ -391,7 +391,7 @@ PCI
>    devm_pci_remap_cfgspace()	: ioremap PCI configuration space
>    devm_pci_remap_cfg_resource()	: ioremap PCI configuration space resource
>  
> -  pcim_enable_device()		: after success, some PCI ops become managed
> +  pcim_enable_device()		: after success, PCI dev gets deactivated automatically

I think that the patch description has a better comment that could be put here. ^^^^^

>    pcim_iomap()			: do iomap() on a single BAR
>    pcim_iomap_regions()		: do request_region() and iomap() on multiple BARs
>    pcim_iomap_table()		: array of mapped addresses indexed by BAR

-- 
~Randy

Re: [PATCH v2 2/6] Documentation/driver-api: Update pcim_enable_device()

[ALOK TIWARI]

On 16-05-2025 23:11, Philipp Stanner wrote:
> pcim_enable_device() is not related anymore to switching the mode of
> operation of any functions. It merely sets up a devres callback for
> automatically disabling the PCI device on driver detach.
> 
> Adjust the function's documentation.
> 
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> ---
>   Documentation/driver-api/driver-model/devres.rst | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst
> index d75728eb05f8..9443911c4742 100644
> --- a/Documentation/driver-api/driver-model/devres.rst
> +++ b/Documentation/driver-api/driver-model/devres.rst
> @@ -391,7 +391,7 @@ PCI
>     devm_pci_remap_cfgspace()	: ioremap PCI configuration space
>     devm_pci_remap_cfg_resource()	: ioremap PCI configuration space resource
>   
> -  pcim_enable_device()		: after success, some PCI ops become managed
> +  pcim_enable_device()		: after success, PCI dev gets deactivated automatically

PCI dev -> PCI device
something like this if work ?"after success, PCI device disables 
automatically once driver detaches"

>     pcim_iomap()			: do iomap() on a single BAR
>     pcim_iomap_regions()		: do request_region() and iomap() on multiple BARs
>     pcim_iomap_table()		: array of mapped addresses indexed by BAR


Thanks,
Alok

Re: [PATCH v2 1/6] PCI: Remove hybrid devres nature from request functions

[Sathyanarayanan Kuppuswamy]

Hi,

On 5/16/25 10:41 AM, Philipp Stanner wrote:
> All functions based on __pci_request_region() and its release counter
> part support "hybrid mode", where the functions become managed if the
> PCI device was enabled with pcim_enable_device().
>
> Removing this undesirable feature requires to remove all users who
> activated their device with that function and use one of the affected
> request functions.
>
> These users were:
> 	ASoC
> 	alsa
> 	cardreader
> 	cirrus
> 	i2c
> 	mmc
> 	mtd
> 	mtd
> 	mxser
> 	net
> 	spi
> 	vdpa
> 	vmwgfx
>
> all of which have been ported to always-managed pcim_ functions by now.
>
> The hybrid nature can, thus, be removed from the aforementioned PCI
> functions.
>
> Remove all function guards and documentation in pci.c related to the
> hybrid redirection. Adjust the visibility of pcim_release_region().
>
> Signed-off-by: Philipp Stanner <phasta@kernel.org>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---

Reviewed-by: Kuppuswamy Sathyanarayanan 
<sathyanarayanan.kuppuswamy@linux.intel.com>

>   drivers/pci/devres.c | 39 ++++++++++++---------------------------
>   drivers/pci/pci.c    | 42 ------------------------------------------
>   drivers/pci/pci.h    |  1 -
>   3 files changed, 12 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
> index 73047316889e..5480d537f400 100644
> --- a/drivers/pci/devres.c
> +++ b/drivers/pci/devres.c
> @@ -6,30 +6,13 @@
>   /*
>    * On the state of PCI's devres implementation:
>    *
> - * The older devres API for PCI has two significant problems:
> + * The older PCI devres API has one significant problem:
>    *
> - * 1. It is very strongly tied to the statically allocated mapping table in
> - *    struct pcim_iomap_devres below. This is mostly solved in the sense of the
> - *    pcim_ functions in this file providing things like ranged mapping by
> - *    bypassing this table, whereas the functions that were present in the old
> - *    API still enter the mapping addresses into the table for users of the old
> - *    API.
> - *
> - * 2. The region-request-functions in pci.c do become managed IF the device has
> - *    been enabled with pcim_enable_device() instead of pci_enable_device().
> - *    This resulted in the API becoming inconsistent: Some functions have an
> - *    obviously managed counter-part (e.g., pci_iomap() <-> pcim_iomap()),
> - *    whereas some don't and are never managed, while others don't and are
> - *    _sometimes_ managed (e.g. pci_request_region()).
> - *
> - *    Consequently, in the new API, region requests performed by the pcim_
> - *    functions are automatically cleaned up through the devres callback
> - *    pcim_addr_resource_release().
> - *
> - *    Users of pcim_enable_device() + pci_*region*() are redirected in
> - *    pci.c to the managed functions here in this file. This isn't exactly
> - *    perfect, but the only alternative way would be to port ALL drivers
> - *    using said combination to pcim_ functions.
> + * It is very strongly tied to the statically allocated mapping table in struct
> + * pcim_iomap_devres below. This is mostly solved in the sense of the pcim_
> + * functions in this file providing things like ranged mapping by bypassing
> + * this table, whereas the functions that were present in the old API still
> + * enter the mapping addresses into the table for users of the old API.
>    *
>    * TODO:
>    * Remove the legacy table entirely once all calls to pcim_iomap_table() in
> @@ -89,10 +72,12 @@ static inline void pcim_addr_devres_clear(struct pcim_addr_devres *res)
>   
>   /*
>    * The following functions, __pcim_*_region*, exist as counterparts to the
> - * versions from pci.c - which, unfortunately, can be in "hybrid mode", i.e.,
> - * sometimes managed, sometimes not.
> + * versions from pci.c - which, unfortunately, were in the past in "hybrid
> + * mode", i.e., sometimes managed, sometimes not.

Why not remove "hybrid mode"  reference like other places?

>    *
> - * To separate the APIs cleanly, we define our own, simplified versions here.
> + * To separate the APIs cleanly, we defined our own, simplified versions here.
> + *
> + * TODO: unify those functions with the counterparts in pci.c
>    */
>   
>   /**
> @@ -893,7 +878,7 @@ int pcim_request_region_exclusive(struct pci_dev *pdev, int bar, const char *nam
>    * Release a region manually that was previously requested by
>    * pcim_request_region().
>    */
> -void pcim_release_region(struct pci_dev *pdev, int bar)
> +static void pcim_release_region(struct pci_dev *pdev, int bar)
>   {
>   	struct pcim_addr_devres res_searched;
>   
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e77d5b53c0ce..4acc23823637 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -3937,16 +3937,6 @@ void pci_release_region(struct pci_dev *pdev, int bar)
>   	if (!pci_bar_index_is_valid(bar))
>   		return;
>   
> -	/*
> -	 * This is done for backwards compatibility, because the old PCI devres
> -	 * API had a mode in which the function became managed if it had been
> -	 * enabled with pcim_enable_device() instead of pci_enable_device().
> -	 */
> -	if (pci_is_managed(pdev)) {
> -		pcim_release_region(pdev, bar);
> -		return;
> -	}
> -
>   	if (pci_resource_len(pdev, bar) == 0)
>   		return;
>   	if (pci_resource_flags(pdev, bar) & IORESOURCE_IO)
> @@ -3984,13 +3974,6 @@ static int __pci_request_region(struct pci_dev *pdev, int bar,
>   	if (!pci_bar_index_is_valid(bar))
>   		return -EINVAL;
>   
> -	if (pci_is_managed(pdev)) {
> -		if (exclusive == IORESOURCE_EXCLUSIVE)
> -			return pcim_request_region_exclusive(pdev, bar, name);
> -
> -		return pcim_request_region(pdev, bar, name);
> -	}
> -
>   	if (pci_resource_len(pdev, bar) == 0)
>   		return 0;
>   
> @@ -4027,11 +4010,6 @@ static int __pci_request_region(struct pci_dev *pdev, int bar,
>    *
>    * Returns 0 on success, or %EBUSY on error.  A warning
>    * message is also printed on failure.
> - *
> - * NOTE:
> - * This is a "hybrid" function: It's normally unmanaged, but becomes managed
> - * when pcim_enable_device() has been called in advance. This hybrid feature is
> - * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
>    */
>   int pci_request_region(struct pci_dev *pdev, int bar, const char *name)
>   {
> @@ -4084,11 +4062,6 @@ static int __pci_request_selected_regions(struct pci_dev *pdev, int bars,
>    * @name: Name of the driver requesting the resources
>    *
>    * Returns: 0 on success, negative error code on failure.
> - *
> - * NOTE:
> - * This is a "hybrid" function: It's normally unmanaged, but becomes managed
> - * when pcim_enable_device() has been called in advance. This hybrid feature is
> - * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
>    */
>   int pci_request_selected_regions(struct pci_dev *pdev, int bars,
>   				 const char *name)
> @@ -4104,11 +4077,6 @@ EXPORT_SYMBOL(pci_request_selected_regions);
>    * @name: name of the driver requesting the resources
>    *
>    * Returns: 0 on success, negative error code on failure.
> - *
> - * NOTE:
> - * This is a "hybrid" function: It's normally unmanaged, but becomes managed
> - * when pcim_enable_device() has been called in advance. This hybrid feature is
> - * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
>    */
>   int pci_request_selected_regions_exclusive(struct pci_dev *pdev, int bars,
>   					   const char *name)
> @@ -4144,11 +4112,6 @@ EXPORT_SYMBOL(pci_release_regions);
>    *
>    * Returns 0 on success, or %EBUSY on error.  A warning
>    * message is also printed on failure.
> - *
> - * NOTE:
> - * This is a "hybrid" function: It's normally unmanaged, but becomes managed
> - * when pcim_enable_device() has been called in advance. This hybrid feature is
> - * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
>    */
>   int pci_request_regions(struct pci_dev *pdev, const char *name)
>   {
> @@ -4173,11 +4136,6 @@ EXPORT_SYMBOL(pci_request_regions);
>    *
>    * Returns 0 on success, or %EBUSY on error.  A warning message is also
>    * printed on failure.
> - *
> - * NOTE:
> - * This is a "hybrid" function: It's normally unmanaged, but becomes managed
> - * when pcim_enable_device() has been called in advance. This hybrid feature is
> - * DEPRECATED! If you want managed cleanup, use the pcim_* functions instead.
>    */
>   int pci_request_regions_exclusive(struct pci_dev *pdev, const char *name)
>   {
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index b81e99cd4b62..8c3e5fb2443a 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -1062,7 +1062,6 @@ static inline pci_power_t mid_pci_get_power_state(struct pci_dev *pdev)
>   int pcim_intx(struct pci_dev *dev, int enable);
>   int pcim_request_region_exclusive(struct pci_dev *pdev, int bar,
>   				  const char *name);
> -void pcim_release_region(struct pci_dev *pdev, int bar);
>   

Since you removed the only use of pcim_request_region_exclusive(), why 
not remove the definition in the same patch?
>   /*
>    * Config Address for PCI Configuration Mechanism #1

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH v2 1/6] PCI: Remove hybrid devres nature from request functions

[Sathyanarayanan Kuppuswamy]

On 5/16/25 3:58 PM, Sathyanarayanan Kuppuswamy wrote:
> Hi,
>
> On 5/16/25 10:41 AM, Philipp Stanner wrote:
>> All functions based on __pci_request_region() and its release counter
>> part support "hybrid mode", where the functions become managed if the
>> PCI device was enabled with pcim_enable_device().
>>
>> Removing this undesirable feature requires to remove all users who
>> activated their device with that function and use one of the affected
>> request functions.
>>
>> These users were:
>>     ASoC
>>     alsa
>>     cardreader
>>     cirrus
>>     i2c
>>     mmc
>>     mtd
>>     mtd
>>     mxser
>>     net
>>     spi
>>     vdpa
>>     vmwgfx
>>
>> all of which have been ported to always-managed pcim_ functions by now.
>>
>> The hybrid nature can, thus, be removed from the aforementioned PCI
>> functions.
>>
>> Remove all function guards and documentation in pci.c related to the
>> hybrid redirection. Adjust the visibility of pcim_release_region().
>>
>> Signed-off-by: Philipp Stanner <phasta@kernel.org>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>
> Reviewed-by: Kuppuswamy Sathyanarayanan 
> <sathyanarayanan.kuppuswamy@linux.intel.com>
>
>>   drivers/pci/devres.c | 39 ++++++++++++---------------------------
>>   drivers/pci/pci.c    | 42 ------------------------------------------
>>   drivers/pci/pci.h    |  1 -
>>   3 files changed, 12 insertions(+), 70 deletions(-)
>>
>> diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
>> index 73047316889e..5480d537f400 100644
>> --- a/drivers/pci/devres.c
>> +++ b/drivers/pci/devres.c
>> @@ -6,30 +6,13 @@
>>   /*
>>    * On the state of PCI's devres implementation:
>>    *
>> - * The older devres API for PCI has two significant problems:
>> + * The older PCI devres API has one significant problem:
>>    *
>> - * 1. It is very strongly tied to the statically allocated mapping 
>> table in
>> - *    struct pcim_iomap_devres below. This is mostly solved in the 
>> sense of the
>> - *    pcim_ functions in this file providing things like ranged 
>> mapping by
>> - *    bypassing this table, whereas the functions that were present 
>> in the old
>> - *    API still enter the mapping addresses into the table for users 
>> of the old
>> - *    API.
>> - *
>> - * 2. The region-request-functions in pci.c do become managed IF the 
>> device has
>> - *    been enabled with pcim_enable_device() instead of 
>> pci_enable_device().
>> - *    This resulted in the API becoming inconsistent: Some functions 
>> have an
>> - *    obviously managed counter-part (e.g., pci_iomap() <-> 
>> pcim_iomap()),
>> - *    whereas some don't and are never managed, while others don't 
>> and are
>> - *    _sometimes_ managed (e.g. pci_request_region()).
>> - *
>> - *    Consequently, in the new API, region requests performed by the 
>> pcim_
>> - *    functions are automatically cleaned up through the devres 
>> callback
>> - *    pcim_addr_resource_release().
>> - *
>> - *    Users of pcim_enable_device() + pci_*region*() are redirected in
>> - *    pci.c to the managed functions here in this file. This isn't 
>> exactly
>> - *    perfect, but the only alternative way would be to port ALL 
>> drivers
>> - *    using said combination to pcim_ functions.
>> + * It is very strongly tied to the statically allocated mapping 
>> table in struct
>> + * pcim_iomap_devres below. This is mostly solved in the sense of 
>> the pcim_
>> + * functions in this file providing things like ranged mapping by 
>> bypassing
>> + * this table, whereas the functions that were present in the old 
>> API still
>> + * enter the mapping addresses into the table for users of the old API.
>>    *
>>    * TODO:
>>    * Remove the legacy table entirely once all calls to 
>> pcim_iomap_table() in
>> @@ -89,10 +72,12 @@ static inline void pcim_addr_devres_clear(struct 
>> pcim_addr_devres *res)
>>     /*
>>    * The following functions, __pcim_*_region*, exist as counterparts 
>> to the
>> - * versions from pci.c - which, unfortunately, can be in "hybrid 
>> mode", i.e.,
>> - * sometimes managed, sometimes not.
>> + * versions from pci.c - which, unfortunately, were in the past in 
>> "hybrid
>> + * mode", i.e., sometimes managed, sometimes not.
>
> Why not remove "hybrid mode"  reference like other places?

Please ignore my comment. It looks like you have cleaned up everything
int the end.

>
>>    *
>> - * To separate the APIs cleanly, we define our own, simplified 
>> versions here.
>> + * To separate the APIs cleanly, we defined our own, simplified 
>> versions here.
>> + *
>> + * TODO: unify those functions with the counterparts in pci.c
>>    */
>>     /**
>> @@ -893,7 +878,7 @@ int pcim_request_region_exclusive(struct pci_dev 
>> *pdev, int bar, const char *nam
>>    * Release a region manually that was previously requested by
>>    * pcim_request_region().
>>    */
>> -void pcim_release_region(struct pci_dev *pdev, int bar)
>> +static void pcim_release_region(struct pci_dev *pdev, int bar)
>>   {
>>       struct pcim_addr_devres res_searched;
>>   diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>> index e77d5b53c0ce..4acc23823637 100644
>> --- a/drivers/pci/pci.c
>> +++ b/drivers/pci/pci.c
>> @@ -3937,16 +3937,6 @@ void pci_release_region(struct pci_dev *pdev, 
>> int bar)
>>       if (!pci_bar_index_is_valid(bar))
>>           return;
>>   -    /*
>> -     * This is done for backwards compatibility, because the old PCI 
>> devres
>> -     * API had a mode in which the function became managed if it had 
>> been
>> -     * enabled with pcim_enable_device() instead of 
>> pci_enable_device().
>> -     */
>> -    if (pci_is_managed(pdev)) {
>> -        pcim_release_region(pdev, bar);
>> -        return;
>> -    }
>> -
>>       if (pci_resource_len(pdev, bar) == 0)
>>           return;
>>       if (pci_resource_flags(pdev, bar) & IORESOURCE_IO)
>> @@ -3984,13 +3974,6 @@ static int __pci_request_region(struct pci_dev 
>> *pdev, int bar,
>>       if (!pci_bar_index_is_valid(bar))
>>           return -EINVAL;
>>   -    if (pci_is_managed(pdev)) {
>> -        if (exclusive == IORESOURCE_EXCLUSIVE)
>> -            return pcim_request_region_exclusive(pdev, bar, name);
>> -
>> -        return pcim_request_region(pdev, bar, name);
>> -    }
>> -
>>       if (pci_resource_len(pdev, bar) == 0)
>>           return 0;
>>   @@ -4027,11 +4010,6 @@ static int __pci_request_region(struct 
>> pci_dev *pdev, int bar,
>>    *
>>    * Returns 0 on success, or %EBUSY on error.  A warning
>>    * message is also printed on failure.
>> - *
>> - * NOTE:
>> - * This is a "hybrid" function: It's normally unmanaged, but becomes 
>> managed
>> - * when pcim_enable_device() has been called in advance. This hybrid 
>> feature is
>> - * DEPRECATED! If you want managed cleanup, use the pcim_* functions 
>> instead.
>>    */
>>   int pci_request_region(struct pci_dev *pdev, int bar, const char 
>> *name)
>>   {
>> @@ -4084,11 +4062,6 @@ static int 
>> __pci_request_selected_regions(struct pci_dev *pdev, int bars,
>>    * @name: Name of the driver requesting the resources
>>    *
>>    * Returns: 0 on success, negative error code on failure.
>> - *
>> - * NOTE:
>> - * This is a "hybrid" function: It's normally unmanaged, but becomes 
>> managed
>> - * when pcim_enable_device() has been called in advance. This hybrid 
>> feature is
>> - * DEPRECATED! If you want managed cleanup, use the pcim_* functions 
>> instead.
>>    */
>>   int pci_request_selected_regions(struct pci_dev *pdev, int bars,
>>                    const char *name)
>> @@ -4104,11 +4077,6 @@ EXPORT_SYMBOL(pci_request_selected_regions);
>>    * @name: name of the driver requesting the resources
>>    *
>>    * Returns: 0 on success, negative error code on failure.
>> - *
>> - * NOTE:
>> - * This is a "hybrid" function: It's normally unmanaged, but becomes 
>> managed
>> - * when pcim_enable_device() has been called in advance. This hybrid 
>> feature is
>> - * DEPRECATED! If you want managed cleanup, use the pcim_* functions 
>> instead.
>>    */
>>   int pci_request_selected_regions_exclusive(struct pci_dev *pdev, 
>> int bars,
>>                          const char *name)
>> @@ -4144,11 +4112,6 @@ EXPORT_SYMBOL(pci_release_regions);
>>    *
>>    * Returns 0 on success, or %EBUSY on error.  A warning
>>    * message is also printed on failure.
>> - *
>> - * NOTE:
>> - * This is a "hybrid" function: It's normally unmanaged, but becomes 
>> managed
>> - * when pcim_enable_device() has been called in advance. This hybrid 
>> feature is
>> - * DEPRECATED! If you want managed cleanup, use the pcim_* functions 
>> instead.
>>    */
>>   int pci_request_regions(struct pci_dev *pdev, const char *name)
>>   {
>> @@ -4173,11 +4136,6 @@ EXPORT_SYMBOL(pci_request_regions);
>>    *
>>    * Returns 0 on success, or %EBUSY on error.  A warning message is 
>> also
>>    * printed on failure.
>> - *
>> - * NOTE:
>> - * This is a "hybrid" function: It's normally unmanaged, but becomes 
>> managed
>> - * when pcim_enable_device() has been called in advance. This hybrid 
>> feature is
>> - * DEPRECATED! If you want managed cleanup, use the pcim_* functions 
>> instead.
>>    */
>>   int pci_request_regions_exclusive(struct pci_dev *pdev, const char 
>> *name)
>>   {
>> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
>> index b81e99cd4b62..8c3e5fb2443a 100644
>> --- a/drivers/pci/pci.h
>> +++ b/drivers/pci/pci.h
>> @@ -1062,7 +1062,6 @@ static inline pci_power_t 
>> mid_pci_get_power_state(struct pci_dev *pdev)
>>   int pcim_intx(struct pci_dev *dev, int enable);
>>   int pcim_request_region_exclusive(struct pci_dev *pdev, int bar,
>>                     const char *name);
>> -void pcim_release_region(struct pci_dev *pdev, int bar);
>
> Since you removed the only use of pcim_request_region_exclusive(), why 
> not remove the definition in the same patch?
>>   /*
>>    * Config Address for PCI Configuration Mechanism #1
>
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH v2 0/6] PCI: Remove hybrid-devres region requests

[Sathyanarayanan Kuppuswamy]

On 5/16/25 10:41 AM, Philipp Stanner wrote:
> Changes in v2:
>    - Drop patch for removing forgotten header. Patch is unrelated. Will
>      resend seperately. (Andy)
>    - Make docu patch headline "Documentation/driver-api:". There seems to
>      be no canonical way, but this style is quite frequent. (Andy)
>    - Apply Andy's RBs where applicable.
>
> Howdy,
>
> the great day has finally arrived, I managed to get rid of one of the
> big three remaining problems in the PCI devres API (the other two being
> MSI having hybrid-devres, too, and the good old pcim_iomap_tablle)!
>
> It turned out that there aren't even that many users of the hybrid API,
> where pcim_enable_device() switches certain functions in pci.c into
> managed devres mode, which we want to remove.
>
> The affected drivers can be found with:
>
> grep -rlZ "pcim_enable_device" | xargs -0 grep -l "pci_request"
>
> These were:
>
> 	ASoC [1]
> 	alsa [2]
> 	cardreader [3]
> 	cirrus [4]
> 	i2c [5]
> 	mmc [6]
> 	mtd [7]
> 	mxser [8]
> 	net [9]
> 	spi [10]
> 	vdpa [11]
> 	vmwgfx [12]
>
> All of those have been merged and are queued up for the merge window.
> The only possible exception is vdpa, but it seems to be ramped up right
> now; vdpa, however, doesn't even use the hybrid behavior, so that patch
> is just for generic cleanup anyways.
>
> With the users of the hybrid feature gone, the feature itself can
> finally be burned.
>
> So I'm sending out this series now to probe whether it's judged to be
> good enough for the upcoming merge window. If we could take it, we would
> make it impossible that anyone adds new users of the hybrid thing.
>
> If it's too late for the merge window, then that's what it is, of
> course.
>
> In any case I'm glad we can get rid of most of that legacy stuff now.

Looks good to me.

Reviewed-by: Kuppuswamy Sathyanarayanan 
<sathyanarayanan.kuppuswamy@linux.intel.com>


>
> Regards,
> Philipp
>
> [1] https://lore.kernel.org/all/174657893832.4155013.12131767110464880040.b4-ty@kernel.org/
> [2] https://lore.kernel.org/all/8734dy3tvz.wl-tiwai@suse.de/
> [3] https://lore.kernel.org/all/20250417091532.26520-2-phasta@kernel.org/ (private confirmation mail from Greg KH)
> [4] https://lore.kernel.org/dri-devel/e7c45c099f8981257866396e01a91df1afcfbf97.camel@mailbox.org/
> [5] https://lore.kernel.org/all/l26azmnpceka2obq4gtwozziq6lbilb2owx57aajtp3t6jhd3w@llmeikgjvqyh/
> [6] https://lore.kernel.org/all/CAPDyKFqqV2VEqi17UHmFE0b9Y+h5q2YaNfHTux8U=7DgF+svEw@mail.gmail.com/
> [7] https://lore.kernel.org/all/174591865790.993381.15992314896975862083.b4-ty@bootlin.com/
> [8] https://lore.kernel.org/all/20250417081333.20917-2-phasta@kernel.org/ (private confirmation mail from Greg KH)
> [9] https://lore.kernel.org/all/174588423950.1081621.6688170836136857875.git-patchwork-notify@kernel.org/
> [10] https://lore.kernel.org/all/174492457740.248895.3318833401427095151.b4-ty@kernel.org/
> [11] https://lore.kernel.org/all/20250515072724-mutt-send-email-mst@kernel.org/
> [12] https://lore.kernel.org/dri-devel/CABQX2QNQbO4dMq-Hi6tvpi7OTwcVfjM62eCr1OGkzF8Phy-Shw@mail.gmail.com/
>
> Philipp Stanner (6):
>    PCI: Remove hybrid devres nature from request functions
>    Documentation/driver-api: Update pcim_enable_device()
>    PCI: Remove pcim_request_region_exclusive()
>    PCI: Remove request_flags relict from devres
>    PCI: Remove redundant set of request funcs
>    PCI: Remove hybrid-devres hazzard warnings from doc
>
>   .../driver-api/driver-model/devres.rst        |   2 +-
>   drivers/pci/devres.c                          | 201 +++---------------
>   drivers/pci/iomap.c                           |  16 --
>   drivers/pci/pci.c                             |  42 ----
>   drivers/pci/pci.h                             |   3 -
>   5 files changed, 32 insertions(+), 232 deletions(-)
>
-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH v2 0/6] PCI: Remove hybrid-devres region requests

[Andy Shevchenko]

On Fri, May 16, 2025 at 04:14:47PM -0700, Sathyanarayanan Kuppuswamy wrote:
> On 5/16/25 10:41 AM, Philipp Stanner wrote:

> Looks good to me.
> 
> Reviewed-by: Kuppuswamy Sathyanarayanan
> <sathyanarayanan.kuppuswamy@linux.intel.com>

Please, fix your tools, it's always goes two lines while it should be only a
single one.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 0/6] PCI: Remove hybrid-devres region requests

[Sathyanarayanan Kuppuswamy]

On 5/17/25 9:33 AM, Andy Shevchenko wrote:
> On Fri, May 16, 2025 at 04:14:47PM -0700, Sathyanarayanan Kuppuswamy wrote:
>> On 5/16/25 10:41 AM, Philipp Stanner wrote:
>> Looks good to me.
>>
>> Reviewed-by: Kuppuswamy Sathyanarayanan
>> <sathyanarayanan.kuppuswamy@linux.intel.com>
> Please, fix your tools, it's always goes two lines while it should be only a
> single one.
>

Thanks for pointing that out. I use Thunderbird, which auto-wraps lines at 72
characters by default. I've adjusted the settings so it shouldn't be a problem
anymore.

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH v2 1/6] PCI: Remove hybrid devres nature from request functions

[Philipp Stanner]

On Fri, 2025-05-16 at 15:58 -0700, Sathyanarayanan Kuppuswamy wrote:
> Hi,
> 
> On 5/16/25 10:41 AM, Philipp Stanner wrote:
> > All functions based on __pci_request_region() and its release
> > counter
> > part support "hybrid mode", where the functions become managed if
> > the
> > PCI device was enabled with pcim_enable_device().
> > 
> > Removing this undesirable feature requires to remove all users who
> > activated their device with that function and use one of the
> > affected
> > request functions.
> > 
> > These users were:
> > 	ASoC
> > 	alsa
> > 	cardreader
> > 	cirrus
> > 	i2c
> > 	mmc
> > 	mtd
> > 	mtd
> > 	mxser
> > 	net
> > 	spi
> > 	vdpa
> > 	vmwgfx
> > 
> > all of which have been ported to always-managed pcim_ functions by
> > now.
> > 
> > The hybrid nature can, thus, be removed from the aforementioned PCI
> > functions.
> > 
> > Remove all function guards and documentation in pci.c related to
> > the
> > hybrid redirection. Adjust the visibility of pcim_release_region().
> > 
> > Signed-off-by: Philipp Stanner <phasta@kernel.org>
> > Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> 
> Reviewed-by: Kuppuswamy Sathyanarayanan 
> <sathyanarayanan.kuppuswamy@linux.intel.com>
> 
> >   drivers/pci/devres.c | 39 ++++++++++++---------------------------
> >   drivers/pci/pci.c    | 42 ---------------------------------------
> > ---
> >   drivers/pci/pci.h    |  1 -
> >   3 files changed, 12 insertions(+), 70 deletions(-)
> > 
> > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
> > index 73047316889e..5480d537f400 100644
> > --- a/drivers/pci/devres.c
> > +++ b/drivers/pci/devres.c
> > @@ -6,30 +6,13 @@
> >   /*
> >    * On the state of PCI's devres implementation:
> >    *
> > - * The older devres API for PCI has two significant problems:
> > + * The older PCI devres API has one significant problem:
> >    *
> > - * 1. It is very strongly tied to the statically allocated mapping
> > table in
> > - *    struct pcim_iomap_devres below. This is mostly solved in the
> > sense of the
> > - *    pcim_ functions in this file providing things like ranged
> > mapping by
> > - *    bypassing this table, whereas the functions that were
> > present in the old
> > - *    API still enter the mapping addresses into the table for
> > users of the old
> > - *    API.
> > - *
> > - * 2. The region-request-functions in pci.c do become managed IF
> > the device has
> > - *    been enabled with pcim_enable_device() instead of
> > pci_enable_device().
> > - *    This resulted in the API becoming inconsistent: Some
> > functions have an
> > - *    obviously managed counter-part (e.g., pci_iomap() <->
> > pcim_iomap()),
> > - *    whereas some don't and are never managed, while others don't
> > and are
> > - *    _sometimes_ managed (e.g. pci_request_region()).
> > - *
> > - *    Consequently, in the new API, region requests performed by
> > the pcim_
> > - *    functions are automatically cleaned up through the devres
> > callback
> > - *    pcim_addr_resource_release().
> > - *
> > - *    Users of pcim_enable_device() + pci_*region*() are
> > redirected in
> > - *    pci.c to the managed functions here in this file. This isn't
> > exactly
> > - *    perfect, but the only alternative way would be to port ALL
> > drivers
> > - *    using said combination to pcim_ functions.
> > + * It is very strongly tied to the statically allocated mapping
> > table in struct
> > + * pcim_iomap_devres below. This is mostly solved in the sense of
> > the pcim_
> > + * functions in this file providing things like ranged mapping by
> > bypassing
> > + * this table, whereas the functions that were present in the old
> > API still
> > + * enter the mapping addresses into the table for users of the old
> > API.
> >    *
> >    * TODO:
> >    * Remove the legacy table entirely once all calls to
> > pcim_iomap_table() in
> > @@ -89,10 +72,12 @@ static inline void
> > pcim_addr_devres_clear(struct pcim_addr_devres *res)
> >   
> >   /*
> >    * The following functions, __pcim_*_region*, exist as
> > counterparts to the
> > - * versions from pci.c - which, unfortunately, can be in "hybrid
> > mode", i.e.,
> > - * sometimes managed, sometimes not.
> > + * versions from pci.c - which, unfortunately, were in the past in
> > "hybrid
> > + * mode", i.e., sometimes managed, sometimes not.
> 
> Why not remove "hybrid mode"  reference like other places?
> 
> >    *
> > - * To separate the APIs cleanly, we define our own, simplified
> > versions here.
> > + * To separate the APIs cleanly, we defined our own, simplified
> > versions here.
> > + *
> > + * TODO: unify those functions with the counterparts in pci.c
> >    */
> >   
> >   /**
> > @@ -893,7 +878,7 @@ int pcim_request_region_exclusive(struct
> > pci_dev *pdev, int bar, const char *nam
> >    * Release a region manually that was previously requested by
> >    * pcim_request_region().
> >    */
> > -void pcim_release_region(struct pci_dev *pdev, int bar)
> > +static void pcim_release_region(struct pci_dev *pdev, int bar)
> >   {
> >   	struct pcim_addr_devres res_searched;
> >   
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index e77d5b53c0ce..4acc23823637 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3937,16 +3937,6 @@ void pci_release_region(struct pci_dev
> > *pdev, int bar)
> >   	if (!pci_bar_index_is_valid(bar))
> >   		return;
> >   
> > -	/*
> > -	 * This is done for backwards compatibility, because the
> > old PCI devres
> > -	 * API had a mode in which the function became managed if
> > it had been
> > -	 * enabled with pcim_enable_device() instead of
> > pci_enable_device().
> > -	 */
> > -	if (pci_is_managed(pdev)) {
> > -		pcim_release_region(pdev, bar);
> > -		return;
> > -	}
> > -
> >   	if (pci_resource_len(pdev, bar) == 0)
> >   		return;
> >   	if (pci_resource_flags(pdev, bar) & IORESOURCE_IO)
> > @@ -3984,13 +3974,6 @@ static int __pci_request_region(struct
> > pci_dev *pdev, int bar,
> >   	if (!pci_bar_index_is_valid(bar))
> >   		return -EINVAL;
> >   
> > -	if (pci_is_managed(pdev)) {
> > -		if (exclusive == IORESOURCE_EXCLUSIVE)
> > -			return pcim_request_region_exclusive(pdev,
> > bar, name);
> > -
> > -		return pcim_request_region(pdev, bar, name);
> > -	}
> > -
> >   	if (pci_resource_len(pdev, bar) == 0)
> >   		return 0;
> >   
> > @@ -4027,11 +4010,6 @@ static int __pci_request_region(struct
> > pci_dev *pdev, int bar,
> >    *
> >    * Returns 0 on success, or %EBUSY on error.  A warning
> >    * message is also printed on failure.
> > - *
> > - * NOTE:
> > - * This is a "hybrid" function: It's normally unmanaged, but
> > becomes managed
> > - * when pcim_enable_device() has been called in advance. This
> > hybrid feature is
> > - * DEPRECATED! If you want managed cleanup, use the pcim_*
> > functions instead.
> >    */
> >   int pci_request_region(struct pci_dev *pdev, int bar, const char
> > *name)
> >   {
> > @@ -4084,11 +4062,6 @@ static int
> > __pci_request_selected_regions(struct pci_dev *pdev, int bars,
> >    * @name: Name of the driver requesting the resources
> >    *
> >    * Returns: 0 on success, negative error code on failure.
> > - *
> > - * NOTE:
> > - * This is a "hybrid" function: It's normally unmanaged, but
> > becomes managed
> > - * when pcim_enable_device() has been called in advance. This
> > hybrid feature is
> > - * DEPRECATED! If you want managed cleanup, use the pcim_*
> > functions instead.
> >    */
> >   int pci_request_selected_regions(struct pci_dev *pdev, int bars,
> >   				 const char *name)
> > @@ -4104,11 +4077,6 @@ EXPORT_SYMBOL(pci_request_selected_regions);
> >    * @name: name of the driver requesting the resources
> >    *
> >    * Returns: 0 on success, negative error code on failure.
> > - *
> > - * NOTE:
> > - * This is a "hybrid" function: It's normally unmanaged, but
> > becomes managed
> > - * when pcim_enable_device() has been called in advance. This
> > hybrid feature is
> > - * DEPRECATED! If you want managed cleanup, use the pcim_*
> > functions instead.
> >    */
> >   int pci_request_selected_regions_exclusive(struct pci_dev *pdev,
> > int bars,
> >   					   const char *name)
> > @@ -4144,11 +4112,6 @@ EXPORT_SYMBOL(pci_release_regions);
> >    *
> >    * Returns 0 on success, or %EBUSY on error.  A warning
> >    * message is also printed on failure.
> > - *
> > - * NOTE:
> > - * This is a "hybrid" function: It's normally unmanaged, but
> > becomes managed
> > - * when pcim_enable_device() has been called in advance. This
> > hybrid feature is
> > - * DEPRECATED! If you want managed cleanup, use the pcim_*
> > functions instead.
> >    */
> >   int pci_request_regions(struct pci_dev *pdev, const char *name)
> >   {
> > @@ -4173,11 +4136,6 @@ EXPORT_SYMBOL(pci_request_regions);
> >    *
> >    * Returns 0 on success, or %EBUSY on error.  A warning message
> > is also
> >    * printed on failure.
> > - *
> > - * NOTE:
> > - * This is a "hybrid" function: It's normally unmanaged, but
> > becomes managed
> > - * when pcim_enable_device() has been called in advance. This
> > hybrid feature is
> > - * DEPRECATED! If you want managed cleanup, use the pcim_*
> > functions instead.
> >    */
> >   int pci_request_regions_exclusive(struct pci_dev *pdev, const
> > char *name)
> >   {
> > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> > index b81e99cd4b62..8c3e5fb2443a 100644
> > --- a/drivers/pci/pci.h
> > +++ b/drivers/pci/pci.h
> > @@ -1062,7 +1062,6 @@ static inline pci_power_t
> > mid_pci_get_power_state(struct pci_dev *pdev)
> >   int pcim_intx(struct pci_dev *dev, int enable);
> >   int pcim_request_region_exclusive(struct pci_dev *pdev, int bar,
> >   				  const char *name);
> > -void pcim_release_region(struct pci_dev *pdev, int bar);
> >   
> 
> Since you removed the only use of pcim_request_region_exclusive(),
> why 
> not remove the definition in the same patch?

Each maintainer has his own philosophy there – in previous patch
series's the feedback from Bjorn was to provide series's which rather
contain more small atomic patches than larger summarizing ones.

That said, the PCI maintainers of course can squash the patches as they
see fit. But this way around it's easier for me, because providing
something small that will be squashed is more simple than splitting
something large into smaller chunks after being told so.


Regards
P.

> >   /*
> >    * Config Address for PCI Configuration Mechanism #1
>