Re: [PATCH 04/10] pci: devres: make devres region requests consistent

2024-01-16 Thread andy . shevchenko
Mon, Jan 15, 2024 at 03:46:15PM +0100, Philipp Stanner kirjoitti:
> Now that pure managed region request functions are available, the
> implementation of the hybrid-functions which are only sometimes managed
> can be made more consistent and readable by wrapping those
> always-managed functions.
> 
> Implement a new pcim_ function for exclusively requested regions.
> Have the pci_request / release functions call their pcim_ counterparts.
> Remove the now surplus region_mask from the devres struct.

...

> + if (pci_is_managed(pdev)) {
> + if (exclusive == IORESOURCE_EXCLUSIVE)
> + return pcim_request_region_exclusive(pdev, bar, 
> res_name);
> + else

Redundant 'else'

> + return pcim_request_region(pdev, bar, res_name);
> + }

-- 
With Best Regards,
Andy Shevchenko




[PATCH 04/10] pci: devres: make devres region requests consistent

2024-01-15 Thread Philipp Stanner
Now that pure managed region request functions are available, the
implementation of the hybrid-functions which are only sometimes managed
can be made more consistent and readable by wrapping those
always-managed functions.

Implement a new pcim_ function for exclusively requested regions.
Have the pci_request / release functions call their pcim_ counterparts.
Remove the now surplus region_mask from the devres struct.

Signed-off-by: Philipp Stanner 
---
 drivers/pci/devres.c | 49 ++-
 drivers/pci/pci.c| 50 +++-
 drivers/pci/pci.h|  6 --
 include/linux/pci.h  |  1 +
 4 files changed, 43 insertions(+), 63 deletions(-)

diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c
index e221919ebbe2..1e5cf950775d 100644
--- a/drivers/pci/devres.c
+++ b/drivers/pci/devres.c
@@ -22,18 +22,15 @@
  *_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(), while requests performed by
- *pcim_enable_device() + pci_*region*() are automatically cleaned up
- *through the for-loop in pcim_release().
+ *pcim_addr_resource_release().
+ *Users utilizing 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.
  *
- * TODO 1:
+ * TODO:
  * Remove the legacy table entirely once all calls to pcim_iomap_table() in
  * the kernel have been removed.
- *
- * TODO 2:
- * Port everyone calling pcim_enable_device() + pci_*region*() to using the
- * pcim_ functions. Then, remove all devres functionality from pci_*region*()
- * functions and remove the associated cleanups described above in point #2.
  */
 
 /*
@@ -407,21 +404,6 @@ static void pcim_release(struct device *gendev, void *res)
 {
struct pci_dev *dev = to_pci_dev(gendev);
struct pci_devres *this = res;
-   int i;
-
-   /*
-* This is legacy code.
-* All regions requested by a pcim_ function do get released through
-* pcim_addr_resource_release(). Thanks to the hybrid nature of the pci_
-* region-request functions, this for-loop has to release the regions
-* if they have been requested by such a function.
-*
-* TODO: Remove this once all users of pcim_enable_device() PLUS
-* pci-region-request-functions have been ported to pcim_ functions.
-*/
-   for (i = 0; i < DEVICE_COUNT_RESOURCE; i++)
-   if (this->region_mask & (1 << i))
-   pci_release_region(dev, i);
 
if (this->mwi)
pci_clear_mwi(dev);
@@ -971,6 +953,25 @@ 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 requestion region for
+ * @bar: Index of BAR to request
+ * @name: Name associated with the request
+ *
+ * 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);
+}
+EXPORT_SYMBOL(pcim_request_region_exclusive);
+
 /**
  * pcim_release_region - Release a PCI BAR
  * @pdev: PCI device to operate on
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7ca860acf351..66639dd754c7 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3873,7 +3873,15 @@ EXPORT_SYMBOL_GPL(pci_common_swizzle);
  */
 void pci_release_region(struct pci_dev *pdev, int bar)
 {
-   struct pci_devres *dr;
+   /*
+* 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;
@@ -3883,20 +3891,6 @@ void pci_release_region(struct pci_dev *pdev, int bar)
else if (pci_resource_flags(pdev, bar) & IORESOURCE_MEM)
release_mem_region(pci_resource_start(pdev, bar),
pci_resource_len(pdev, bar));
-
-   /*
-* This devres utility makes this function sometimes managed
-* (when pcim_enable_device() has been called before).
-* This is bad because it conf