PCI: Remove hybrid-devres region requests
Subsystems (3)
cover PCI: Remove hybrid-devres region requests
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/[email protected]/
[2] https://lore.kernel.org/all/[email protected]/
[3] https://lore.kernel.org/all/[email protected]/ (private confirmation mail from Greg KH)
[4] https://lore.kernel.org/dri-devel/[email protected]/
[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/[email protected]/
[8] https://lore.kernel.org/all/[email protected]/ (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/[email protected]/
[11] https://lore.kernel.org/all/[email protected]/
[12] https://lore.kernel.org/dri-devel/CABQX2QNQbO4dMq-Hi6tvpi7OTwcVfjM62eCr1OGkzF8Phy-Shw@mail.gmail.com/
Philipp Stanner (7):
PCI: Remove hybrid devres nature from request functions
Docu: PCI: 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 unnecessary prototype from pci.h
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 | 5 - 5 files changed, 32 insertions(+), 234 deletions(-)
Signature · 1 line
2.49.0
PATCH 1/7 PCI: Remove hybrid devres nature from request functions
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().
--- drivers/pci/devres.c | 39 ++++++++++++--------------------------- drivers/pci/pci.c | 42 ------------------------------------------ drivers/pci/pci.h | 1 - 3 files changed, 12 insertions(+), 70 deletions(-)
drivers/pci/devres.c +12 -27
@@ -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;
drivers/pci/pci.c +0 -42
84 lines not on this page · raw message
drivers/pci/pci.h +0 -1
@@ -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
Signature · 1 line
2.49.0
PATCH 2/7 Docu: PCI: Update pcim_enable_device()
In reply to Philipp Stannerpcim_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.
--- Documentation/driver-api/driver-model/devres.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Documentation/driver-api/driver-model/devres.rst +1 -1
@@ -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
Signature · 1 line
2.49.0
Re: [PATCH 2/7] Docu: PCI: Update pcim_enable_device()
Quoted 5 lines · 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.
Is the "Docu" prefix in thew Subject aligned with the git history of this file?
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 2/7] Docu: PCI: Update pcim_enable_device()
Quoted 11 lines · Andy Shevchenko
Quoted 6 lines · 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.
Is the "Docu" prefix in thew Subject aligned with the git history of this file?
Seems its "Documentation: ". Can fix.
P.
Re: [PATCH 2/7] Docu: PCI: Update pcim_enable_device()
Hello,
[...]
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
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.
Quoted 7 lines
Quoted 5 lines
Quoted 1 line
Adjust the function's documentation.
Is the "Docu" prefix in thew Subject aligned with the git history of this file?
Seems its "Documentation: ". Can fix.
Has Andy been sending his review off-list? Or something is broken on my side...
Philipp, if you have a v2 ready, then I would love to pull it, while we await for more reviews, just to get enough soak time and allow for the 0-day and KernelCI to run their usual tests, etc.
Thank you!
Krzysztof
Re: [PATCH 2/7] Docu: PCI: Update pcim_enable_device()
Quoted 17 lines · Krzysztof Wilczyński
Hello,
[...]
Quoted 14 lines
Quoted 13 lines
Quoted 8 lines
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.
Is the "Docu" prefix in thew Subject aligned with the git history of
this file?
Quoted 5 lines
Quoted 1 line
Seems its "Documentation: ". Can fix.
Has Andy been sending his review off-list? Or something is broken on my
side...
Nope, it's on-list. Andy's a veteran ;-)
https://lore.kernel.org/linux-pci/[email protected]/
Quoted 8 lines
Philipp, if you have a v2 ready, then I would love to pull it, while we
await for more reviews, just to get enough soak time and allow for the
0-day and KernelCI to run their usual tests, etc.Thank you!
I don't have it ready, but should be trivial to do, since it's not fundamental critcism. I'll do my best.
P.
Quoted 2 lines
Krzysztof
Re: [PATCH 2/7] Docu: PCI: Update pcim_enable_device()
Hello,
[...]
Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
Quoted 3 lines
Is the "Docu" prefix in thew Subject aligned with the git history of this file?
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Seems its "Documentation: ". Can fix.
Has Andy been sending his review off-list? Or something is broken on my side...
Nope, it's on-list. Andy's a veteran ;-)
Thank you!
I should have checked on lore, too. Time to move to lei, I suppose...
Quoted 8 lines
Quoted 5 lines
Philipp, if you have a v2 ready, then I would love to pull it, while we await for more reviews, just to get enough soak time and allow for the 0-day and KernelCI to run their usual tests, etc.
Thank you!
I don't have it ready, but should be trivial to do, since it's not fundamental critcism. I'll do my best.
I would be much obliged.
Thank you!
Krzysztof
Re: [PATCH 2/7] Docu: PCI: Update pcim_enable_device()
On Fri, May 16, 2025 at 10:48:39PM +0900, Krzysztof Wilczyński wrote:
[...]
Quoted 10 lines
Quoted 6 lines
Quoted 2 lines
Has Andy been sending his review off-list? Or something is broken on my side...
Nope, it's on-list. Andy's a veteran ;-)
Thank you!
I should have checked on lore, too. Time to move to lei, I suppose...
I need to setup DKIM, that's might be the reason you got it in the SPAM folder.
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH 3/7 PCI: Remove pcim_request_region_exclusive()
In reply to Philipp Stannerpcim_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().
--- drivers/pci/devres.c | 18 ------------------ drivers/pci/pci.h | 2 -- 2 files changed, 20 deletions(-)
drivers/pci/devres.c +0 -18
@@ -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
drivers/pci/pci.h +0 -2
@@ -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
Signature · 1 line
2.49.0
PATCH 4/7 PCI: Remove request_flags relict from devres
In reply to Philipp Stannerpcim_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.
--- drivers/pci/devres.c | 46 +++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 26 deletions(-)
drivers/pci/devres.c +20 -26
@@ -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);
Signature · 1 line
2.49.0
Re: [PATCH 4/7] PCI: Remove request_flags relict from devres
Quoted 5 lines · 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.
...
Quoted 8 lines
int
pcim_request_region(struct pci_dev *pdev, int bar, const char *name){drivers/pci/devres.c +5
-1-return _pcim_request_region (pdev, bar, name,0 ); +int ret; +struct pcim_addr_devres *res; + +if (!pci_bar_index_is_valid (bar)) +return -EINVAL ;
drivers/pci/devres.c static int _pcim_request_region(struct pci_dev *pdev, int bar, const char *name,
Quoted 10 lines
drivers/pci/devres.c +9
-0+ + 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 ) {
While at it, drop the ' != 0' part?
Quoted 7 lines
drivers/pci/devres.c +6
-0+pcim_addr_devres_free (res); +return ret; + } + +devres_add (&pdev->dev, res); +return 0 ; }
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 4/7] PCI: Remove request_flags relict from devres
Quoted 29 lines · Andy Shevchenko
Quoted 7 lines · 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....
Quoted 17 lines
int
pcim_request_region(struct pci_dev *pdev, int bar, const char *name)
{drivers/pci/devres.c +13
-1-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 );
Quoted 3 lines
drivers/pci/devres.c static int _pcim_request_region(struct pci_dev *pdev, int bar, const char *name, · line 839
Quoted 1 line
drivers/pci/devres.c +1
-0+if (ret !=0 ) {While at it, drop the ' != 0' part?
I want it to be clear to be just a pure code move. It's a shame that the git-diff can't just handle that through the function head, resulting in a +/-2 diff
Style adjustments, which might also be necessary in pci.c in many places, could be done through dedicated commits.
P.
Quoted 9 lines
Quoted 8 lines
drivers/pci/devres.c +6
-0+pcim_addr_devres_free (res); +return ret; + } + +devres_add (&pdev->dev, res); +return 0 ;}
PATCH 5/7 PCI: Remove redundant set of request funcs
In reply to Philipp StannerWhen 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
--- drivers/pci/devres.c | 110 ++----------------------------------------- 1 file changed, 5 insertions(+), 105 deletions(-)
drivers/pci/devres.c +5 -105
@@ -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;
Signature · 1 line
2.49.0
PATCH 6/7 PCI: Remove unnecessary prototype from pci.h
In reply to Philipp Stannerpcim_intx() once was an internal PCI function, but since then has been published and is used by drivers, and, therefore, available in include/linux/pci.h. The function is not used within PCI anymore.
Remove pcim_intx()'s prototype from drivers/pci/pci.h
--- drivers/pci/pci.h | 2 -- 1 file changed, 2 deletions(-)
drivers/pci/pci.h +0 -2
@@ -1059,8 +1059,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); -/* * Config Address for PCI Configuration Mechanism #1 *
Signature · 1 line
2.49.0
Re: [PATCH 6/7] PCI: Remove unnecessary prototype from pci.h
Quoted 5 lines · Philipp Stanner
pcim_intx()once was an internal PCI function, but since then has been published and is used by drivers, and, therefore, available ininclude/linux/pci.h. The function is not used within PCI anymore.Remove pcim_intx()'s prototype from
drivers/pci/pci.h
Can this be moved up in the series? Or is there other dependencies? I.o.w. this looks like a leftover from something of the previous work.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 6/7] PCI: Remove unnecessary prototype from pci.h
Quoted 6 lines · Andy Shevchenko
Quoted 5 lines · Philipp Stanner
pcim_intx()once was an internal PCI function, but since then has been
published and is used by drivers, and, therefore, available ininclude/linux/pci.h. The function is not used within PCI anymore.
Quoted 6 lines
Quoted 1 line
Remove pcim_intx()'s prototype from
drivers/pci/pci.hCan this be moved up in the series? Or is there other dependencies? I.o.w. this
looks like a leftover from something of the previous work.
That can be moved to anywhere, including a separate patch. It's an independent patch, a leftover from last year. But it's related to devres, because it was also added because of the problem with pcim_enable_device().
P.
Re: [PATCH 6/7] PCI: Remove unnecessary prototype from pci.h
Quoted 9 lines · Philipp Stanner
Quoted 8 lines · Andy Shevchenko
Quoted 6 lines · Philipp Stanner
pcim_intx()once was an internal PCI function, but since then has been
published and is used by drivers, and, therefore, available ininclude/linux/pci.h. The function is not used within PCI anymore.Remove pcim_intx()'s prototype from
drivers/pci/pci.h
Quoted 8 lines
Quoted 3 lines
Can this be moved up in the series? Or is there other dependencies? I.o.w. this
looks like a leftover from something of the previous work.That can be moved to anywhere, including a separate patch. It's an independent patch, a leftover from last year. But it's related to devres, because it was also added because of the problem with
pcim_enable_device().
When put at the end of the series it makes an illusion that there are dependencies. Separate patch is ideal, being first is good enough to me.
Signature · 2 lines
With Best Regards, Andy Shevchenko
PATCH 7/7 PCI: Remove hybrid-devres hazzard warnings from doc
In reply to Philipp Stannerpci/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.
--- drivers/pci/iomap.c | 16 ---------------- 1 file changed, 16 deletions(-)
drivers/pci/iomap.c +0 -16
@@ -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) {
Signature · 1 line
2.49.0
Quoted 10 lines · Philipp Stanner
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 inpci.cinto managed devres mode, which we want to remove.
Quoted 8 lines
The affected drivers can be found with:
grep -rlZ "
pcim_enable_device" | xargs -0 grep -l "pci_request"These were:
ASoC [1] alsa [2]
FWIW, tailing space here.
Quoted 19 lines
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.
Quoted 8 lines
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.
For all non-commented patches,
Signature · 2 lines
With Best Regards, Andy Shevchenko