On Thu, 2024-06-13 at 16:28 -0500, Bjorn Helgaas wrote: > On Thu, Jun 13, 2024 at 01:50:16PM +0200, Philipp Stanner wrote: > > With the current PCI devres API implementing a managed version of > > pci_iomap_range() is impossible. > > > > Furthermore, the PCI devres API currently is inconsistent and > > complicated. This is in large part due to the fact that there are > > hybrid > > functions which are only sometimes managed via devres, and > > functions > > IO-mapping and requesting several BARs at once and returning > > mappings > > through a separately administrated table. > > > > This table's indexing mechanism does not support partial-BAR > > mappings. > > > > Another notable problem is that there are no separate managed > > counterparts for region-request functions such as > > pci_request_region(), > > as they exist for other PCI functions (e.g., pci_iomap() <-> > > pcim_iomap()). Instead, functions based on __pci_request_region() > > change > > their internal behavior and suddenly become managed functions when > > pcim_enable_device() instead of pci_enable_device() is used. > > The hybrid thing is certainly a problem, but does this patch address > it? I don't see that it does (other than adding comments in > __pci_request_region() and pci_release_region()), but maybe I missed > it. This is just justification for why __pcim_request_region() etc. are implemented. They bypass the hybrid nature of __pci_request_region(). If the latter wouldn't have that behavior > > Correct me if I'm wrong, but I don't think this patch makes any > user-visible changes. Except for deprecating that two functions and adding a new public one, the entire series shouldn't make user-visible changes. That's the point. P. > > I'm proposing this: > > PCI: Add managed partial-BAR request and map infrastructure > > The pcim_iomap_devres table tracks entire-BAR mappings, so we can't > use it > to build a managed version of pci_iomap_range(), which maps partial > BARs. > > Add struct pcim_addr_devres, which can track request and mapping of > both > entire BARs and partial BARs. > > Add the following internal devres functions based on struct > pcim_addr_devres: > > pcim_iomap_region() # request & map entire BAR > pcim_iounmap_region() # unmap & release entire BAR > pcim_request_region() # request entire BAR > pcim_release_region() # release entire BAR > pcim_request_all_regions() # request all entire BARs > pcim_release_all_regions() # release all entire BARs > > Rework the following public interfaces using the new infrastructure > listed above: > > pcim_iomap() # map partial BAR > pcim_iounmap() # unmap partial BAR > pcim_iomap_regions() # request & map specified BARs > pcim_iomap_regions_request_all() # request all BARs, map > specified BARs > pcim_iounmap_regions() # unmap & release specified > BARs > > > > This API is hard to understand and potentially bug-provoking. > > Hence, it > > should be made more consistent. > > > > This patch adds the necessary infrastructure for partial-BAR > > mappings > > managed with devres. That infrastructure also serves as a ground > > layer > > for significantly simplifying the PCI devres API in subsequent > > patches > > which can then cleanly separate managed and unmanaged API. > > > > When having the long term goal of providing always managed > > functions > > prefixed with "pcim_" and never managed functions prefixed with > > "pci_" > > and, thus, separating managed and unmanaged APIs cleanly, new PCI > > devres > > infrastructure cannot use __pci_request_region() and its wrappers > > since > > those would then again interact with PCI devres and, consequently, > > prevent the managed nature from being removed from the pci_* > > functions > > in the first place. Thus, it's necessary to provide an alternative > > to > > __pci_request_region(). > > > > This patch addresses the following problems of the PCI devres API: > > > > a) There is no PCI devres infrastructure on which a managed > > counter > > part to pci_iomap_range() could be based on. > > > > b) The vast majority of the users of plural functions such as > > pcim_iomap_regions() only ever sets a single bit in the bit > > mask, > > consequently making them singular functions anyways. > > > > c) region-request functions being sometimes managed and sometimes > > not > > is bug-provoking. pcim_* functions should always be managed, > > pci_* > > functions never. > > > > Add a new PCI device resource, pcim_addr_devres, that serves to > > encapsulate all device resource types related to region requests > > and > > IO-mappings since those are very frequently created together. > > > > Add a set of alternatives cleanly separated from the hybrid > > mechanism in > > __pci_request_region() and its respective wrappers: > > - __pcim_request_region_range() > > - __pcim_release_region_range() > > - __pcim_request_region() > > - __pcim_release_region() > > > > Add the following PCI-internal devres functions based on the above: > > - pcim_iomap_region() > > - pcim_iounmap_region() > > - _pcim_request_region() > > - pcim_request_region() > > - pcim_release_region() > > - pcim_request_all_regions() > > - pcim_release_all_regions() > > > > Add new needed helper pcim_remove_bar_from_legacy_table(). > > > > Rework the following public interfaces using the new infrastructure > > listed above: > > - pcim_iomap_release() > > - pcim_iomap() > > - pcim_iounmap() > > - pcim_iomap_regions() > > - pcim_iomap_regions_request_all() > > - pcim_iounmap_regions() > > > > Update API documentation. > > > > Link: > > https://lore.kernel.org/r/20240605081605.18769-5-pstanner@xxxxxxxxxx > > Signed-off-by: Philipp Stanner <pstanner@xxxxxxxxxx> > > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > --- > > drivers/pci/devres.c | 608 ++++++++++++++++++++++++++++++++++++++- > > ---- > > drivers/pci/pci.c | 22 ++ > > drivers/pci/pci.h | 5 + > > 3 files changed, 568 insertions(+), 67 deletions(-) > > > > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c > > index 845d6fab0ce7..cf2c11b54ca6 100644 > > --- a/drivers/pci/devres.c > > +++ b/drivers/pci/devres.c > > @@ -4,14 +4,243 @@ > > #include "pci.h" > > > > /* > > - * PCI iomap devres > > + * On the state of PCI's devres implementation: > > + * > > + * The older devres API for PCI has two significant problems: > > + * > > + * 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, wheras 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(), while requests performed by > > + * pcim_enable_device() + pci_*region*() are automatically > > cleaned up > > + * through the for-loop in pcim_release(). > > + * > > + * TODO 1: > > + * 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. > > */ > > -#define PCIM_IOMAP_MAX PCI_STD_NUM_BARS > > > > +/* > > + * Legacy struct storing addresses to whole mapped BARs. > > + */ > > struct pcim_iomap_devres { > > - void __iomem *table[PCIM_IOMAP_MAX]; > > + void __iomem *table[PCI_STD_NUM_BARS]; > > +}; > > + > > +enum pcim_addr_devres_type { > > + /* Default initializer. */ > > + PCIM_ADDR_DEVRES_TYPE_INVALID, > > + > > + /* A requested region spanning an entire BAR. */ > > + PCIM_ADDR_DEVRES_TYPE_REGION, > > + > > + /* > > + * A requested region spanning an entire BAR, and a mapping > > for > > + * the entire BAR. > > + */ > > + PCIM_ADDR_DEVRES_TYPE_REGION_MAPPING, > > + > > + /* > > + * A mapping within a BAR, either spanning the whole BAR or > > just a > > + * range. Without a requested region. > > + */ > > + PCIM_ADDR_DEVRES_TYPE_MAPPING, > > }; > > > > +/* > > + * This struct envelops IO or MEM addresses, i.e., mappings and > > region > > + * requests, because those are very frequently requested and > > released > > + * together. > > + */ > > +struct pcim_addr_devres { > > + enum pcim_addr_devres_type type; > > + void __iomem *baseaddr; > > + unsigned long offset; > > + unsigned long len; > > + short bar; > > +}; > > + > > +static inline void pcim_addr_devres_clear(struct pcim_addr_devres > > *res) > > +{ > > + memset(res, 0, sizeof(*res)); > > + res->bar = -1; > > +} > > + > > +/* > > + * 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. > > + * > > + * To separate the APIs cleanly, we define our own, simplified > > versions here. > > + */ > > + > > +/** > > + * __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 associated with the request > > + * @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); > > + struct pcim_addr_devres *res = resource_raw; > > + > > + switch (res->type) { > > + case PCIM_ADDR_DEVRES_TYPE_REGION: > > + __pcim_release_region(pdev, res->bar); > > + break; > > + case PCIM_ADDR_DEVRES_TYPE_REGION_MAPPING: > > + pci_iounmap(pdev, res->baseaddr); > > + __pcim_release_region(pdev, res->bar); > > + break; > > + case PCIM_ADDR_DEVRES_TYPE_MAPPING: > > + pci_iounmap(pdev, res->baseaddr); > > + break; > > + default: > > + break; > > + } > > +} > > + > > +static struct pcim_addr_devres *pcim_addr_devres_alloc(struct > > pci_dev *pdev) > > +{ > > + struct pcim_addr_devres *res; > > + > > + res = devres_alloc_node(pcim_addr_resource_release, > > sizeof(*res), > > + GFP_KERNEL, dev_to_node(&pdev->dev)); > > + if (res) > > + pcim_addr_devres_clear(res); > > + return res; > > +} > > + > > +/* Just for consistency and readability. */ > > +static inline void pcim_addr_devres_free(struct pcim_addr_devres > > *res) > > +{ > > + devres_free(res); > > +} > > + > > +/* > > + * Used by devres to identify a pcim_addr_devres. > > + */ > > +static int pcim_addr_resources_match(struct device *dev, void > > *a_raw, void *b_raw) > > +{ > > + struct pcim_addr_devres *a, *b; > > + > > + a = a_raw; > > + b = b_raw; > > + > > + if (a->type != b->type) > > + return 0; > > + > > + switch (a->type) { > > + case PCIM_ADDR_DEVRES_TYPE_REGION: > > + case PCIM_ADDR_DEVRES_TYPE_REGION_MAPPING: > > + return a->bar == b->bar; > > + case PCIM_ADDR_DEVRES_TYPE_MAPPING: > > + return a->baseaddr == b->baseaddr; > > + default: > > + return 0; > > + } > > +} > > > > static void devm_pci_unmap_iospace(struct device *dev, void *ptr) > > { > > @@ -92,8 +321,8 @@ EXPORT_SYMBOL(devm_pci_remap_cfgspace); > > * > > * All operations are managed and will be undone on driver detach. > > * > > - * Returns a pointer to the remapped memory or an ERR_PTR() > > encoded error code > > - * on failure. Usage example:: > > + * Returns a pointer to the remapped memory or an IOMEM_ERR_PTR() > > encoded error > > + * code on failure. Usage example:: > > * > > * res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > > * base = devm_pci_remap_cfg_resource(&pdev->dev, res); > > @@ -172,6 +401,17 @@ static void pcim_release(struct device > > *gendev, void *res) > > 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 (mask_contains_bar(this->region_mask, i)) > > pci_release_region(dev, i); > > @@ -258,19 +498,21 @@ EXPORT_SYMBOL(pcim_pin_device); > > > > static void pcim_iomap_release(struct device *gendev, void *res) > > { > > - struct pci_dev *dev = to_pci_dev(gendev); > > - struct pcim_iomap_devres *this = res; > > - int i; > > - > > - for (i = 0; i < PCIM_IOMAP_MAX; i++) > > - if (this->table[i]) > > - pci_iounmap(dev, this->table[i]); > > + /* > > + * Do nothing. This is legacy code. > > + * > > + * Cleanup of the mappings is now done directly through the > > callbacks > > + * registered when creating them. > > + */ > > } > > > > /** > > * pcim_iomap_table - access iomap allocation table > > * @pdev: PCI device to access iomap table for > > * > > + * Returns: > > + * Const pointer to array of __iomem pointers on success, NULL on > > failure. > > + * > > * Access iomap allocation table for @dev. If iomap table doesn't > > * exist and @pdev is managed, it will be allocated. All iomaps > > * recorded in the iomap table are automatically unmapped on > > driver > > @@ -343,30 +585,67 @@ static void > > pcim_remove_mapping_from_legacy_table(struct pci_dev *pdev, > > } > > } > > > > +/* > > + * The same as pcim_remove_mapping_from_legacy_table(), but > > identifies the > > + * mapping by its BAR index. > > + */ > > +static void pcim_remove_bar_from_legacy_table(struct pci_dev > > *pdev, short bar) > > +{ > > + void __iomem **legacy_iomap_table; > > + > > + if (bar >= PCI_STD_NUM_BARS) > > + return; > > + > > + legacy_iomap_table = (void __iomem > > **)pcim_iomap_table(pdev); > > + if (!legacy_iomap_table) > > + return; > > + > > + legacy_iomap_table[bar] = NULL; > > +} > > + > > /** > > * pcim_iomap - Managed pcim_iomap() > > * @pdev: PCI device to iomap for > > * @bar: BAR to iomap > > * @maxlen: Maximum length of iomap > > * > > - * Managed pci_iomap(). Map is automatically unmapped on driver > > - * detach. > > + * Returns: __iomem pointer on success, NULL on failure. > > + * > > + * Managed pci_iomap(). Map is automatically unmapped on driver > > detach. If > > + * desired, unmap manually only with pcim_iounmap(). > > + * > > + * This SHOULD only be used once per BAR. > > + * > > + * NOTE: > > + * Contrary to the other pcim_* functions, this function does not > > return an > > + * IOMEM_ERR_PTR() on failure, but a simple NULL. This is done for > > backwards > > + * compatibility. > > */ > > void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned > > long maxlen) > > { > > void __iomem *mapping; > > + struct pcim_addr_devres *res; > > + > > + res = pcim_addr_devres_alloc(pdev); > > + if (!res) > > + return NULL; > > + res->type = PCIM_ADDR_DEVRES_TYPE_MAPPING; > > > > mapping = pci_iomap(pdev, bar, maxlen); > > if (!mapping) > > - return NULL; > > + goto err_iomap; > > + res->baseaddr = mapping; > > > > if (pcim_add_mapping_to_legacy_table(pdev, mapping, bar) != > > 0) > > goto err_table; > > > > + devres_add(&pdev->dev, res); > > return mapping; > > > > err_table: > > pci_iounmap(pdev, mapping); > > +err_iomap: > > + pcim_addr_devres_free(res); > > return NULL; > > } > > EXPORT_SYMBOL(pcim_iomap); > > @@ -376,91 +655,291 @@ EXPORT_SYMBOL(pcim_iomap); > > * @pdev: PCI device to iounmap for > > * @addr: Address to unmap > > * > > - * Managed pci_iounmap(). @addr must have been mapped using > > pcim_iomap(). > > + * Managed pci_iounmap(). @addr must have been mapped using a > > pcim_* mapping > > + * function. > > */ > > void pcim_iounmap(struct pci_dev *pdev, void __iomem *addr) > > { > > - pci_iounmap(pdev, addr); > > + struct pcim_addr_devres res_searched; > > + > > + pcim_addr_devres_clear(&res_searched); > > + res_searched.type = PCIM_ADDR_DEVRES_TYPE_MAPPING; > > + res_searched.baseaddr = addr; > > + > > + if (devres_release(&pdev->dev, pcim_addr_resource_release, > > + pcim_addr_resources_match, &res_searched) > > != 0) { > > + /* Doesn't exist. User passed nonsense. */ > > + return; > > + } > > > > pcim_remove_mapping_from_legacy_table(pdev, addr); > > } > > EXPORT_SYMBOL(pcim_iounmap); > > > > +/** > > + * pcim_iomap_region - Request and iomap a PCI BAR > > + * @pdev: PCI device to map IO resources for > > + * @bar: Index of a BAR to map > > + * @name: Name associated with the request > > + * > > + * Returns: __iomem pointer on success, an IOMEM_ERR_PTR on > > failure. > > + * > > + * Mapping and region will get automatically released on driver > > detach. If > > + * desired, release manually only with pcim_iounmap_region(). > > + */ > > +static void __iomem *pcim_iomap_region(struct pci_dev *pdev, int > > bar, > > + const char *name) > > +{ > > + int ret; > > + struct pcim_addr_devres *res; > > + > > + res = pcim_addr_devres_alloc(pdev); > > + if (!res) > > + return IOMEM_ERR_PTR(-ENOMEM); > > + > > + res->type = PCIM_ADDR_DEVRES_TYPE_REGION_MAPPING; > > + res->bar = bar; > > + > > + ret = __pcim_request_region(pdev, bar, name, 0); > > + if (ret != 0) > > + goto err_region; > > + > > + res->baseaddr = pci_iomap(pdev, bar, 0); > > + if (!res->baseaddr) { > > + ret = -EINVAL; > > + goto err_iomap; > > + } > > + > > + devres_add(&pdev->dev, res); > > + return res->baseaddr; > > + > > +err_iomap: > > + __pcim_release_region(pdev, bar); > > +err_region: > > + pcim_addr_devres_free(res); > > + > > + return IOMEM_ERR_PTR(ret); > > +} > > + > > +/** > > + * pcim_iounmap_region - Unmap and release a PCI BAR > > + * @pdev: PCI device to operate on > > + * @bar: Index of BAR to unmap and release > > + * > > + * Unmap a BAR and release its region manually. Only pass BARs > > that were > > + * previously mapped by pcim_iomap_region(). > > + */ > > +static void pcim_iounmap_region(struct pci_dev *pdev, int bar) > > +{ > > + struct pcim_addr_devres res_searched; > > + > > + pcim_addr_devres_clear(&res_searched); > > + res_searched.type = PCIM_ADDR_DEVRES_TYPE_REGION_MAPPING; > > + res_searched.bar = bar; > > + > > + devres_release(&pdev->dev, pcim_addr_resource_release, > > + pcim_addr_resources_match, &res_searched); > > +} > > + > > /** > > * pcim_iomap_regions - Request and iomap PCI BARs > > * @pdev: PCI device to map IO resources for > > * @mask: Mask of BARs to request and iomap > > - * @name: Name used when requesting regions > > + * @name: Name associated with the requests > > + * > > + * Returns: 0 on success, negative error code on failure. > > * > > * Request and iomap regions specified by @mask. > > */ > > int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char > > *name) > > { > > - void __iomem * const *iomap; > > - int i, rc; > > + int ret; > > + short bar; > > + void __iomem *mapping; > > > > - iomap = pcim_iomap_table(pdev); > > - if (!iomap) > > - return -ENOMEM; > > + for (bar = 0; bar < DEVICE_COUNT_RESOURCE; bar++) { > > + if (!mask_contains_bar(mask, bar)) > > + continue; > > > > - for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) { > > - unsigned long len; > > + mapping = pcim_iomap_region(pdev, bar, name); > > + if (IS_ERR(mapping)) { > > + ret = PTR_ERR(mapping); > > + goto err; > > + } > > + ret = pcim_add_mapping_to_legacy_table(pdev, > > mapping, bar); > > + if (ret != 0) > > + goto err; > > + } > > > > - if (!mask_contains_bar(mask, i)) > > - continue; > > + return 0; > > > > - rc = -EINVAL; > > - len = pci_resource_len(pdev, i); > > - if (!len) > > - goto err_inval; > > +err: > > + while (--bar >= 0) { > > + pcim_iounmap_region(pdev, bar); > > + pcim_remove_bar_from_legacy_table(pdev, bar); > > + } > > > > - rc = pci_request_region(pdev, i, name); > > - if (rc) > > - goto err_inval; > > + return ret; > > +} > > +EXPORT_SYMBOL(pcim_iomap_regions); > > > > - rc = -ENOMEM; > > - if (!pcim_iomap(pdev, i, 0)) > > - goto err_region; > > +static int _pcim_request_region(struct pci_dev *pdev, int bar, > > const char *name, > > + int request_flags) > > +{ > > + int ret; > > + struct pcim_addr_devres *res; > > + > > + 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; > > +} > > > > - err_region: > > - pci_release_region(pdev, i); > > - err_inval: > > - while (--i >= 0) { > > - if (!mask_contains_bar(mask, i)) > > - continue; > > - pcim_iounmap(pdev, iomap[i]); > > - pci_release_region(pdev, i); > > +/** > > + * pcim_request_region - Request a PCI BAR > > + * @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. > > + * > > + * The region will automatically be released on driver detach. If > > desired, > > + * release manually only with pcim_release_region(). > > + */ > > +static int pcim_request_region(struct pci_dev *pdev, int bar, > > const char *name) > > +{ > > + return _pcim_request_region(pdev, bar, name, 0); > > +} > > + > > +/** > > + * pcim_release_region - Release a PCI BAR > > + * @pdev: PCI device to operate on > > + * @bar: Index of BAR to release > > + * > > + * Release a region manually that was previously requested by > > + * pcim_request_region(). > > + */ > > +static void pcim_release_region(struct pci_dev *pdev, int bar) > > +{ > > + struct pcim_addr_devres res_searched; > > + > > + pcim_addr_devres_clear(&res_searched); > > + res_searched.type = PCIM_ADDR_DEVRES_TYPE_REGION; > > + res_searched.bar = bar; > > + > > + devres_release(&pdev->dev, pcim_addr_resource_release, > > + pcim_addr_resources_match, &res_searched); > > +} > > + > > + > > +/** > > + * pcim_release_all_regions - Release all regions of a PCI-device > > + * @pdev: the PCI device > > + * > > + * Release all regions previously requested through > > pcim_request_region() > > + * or pcim_request_all_regions(). > > + * > > + * Can be called from any context, i.e., not necessarily as a > > counterpart to > > + * pcim_request_all_regions(). > > + */ > > +static void pcim_release_all_regions(struct pci_dev *pdev) > > +{ > > + short bar; > > + > > + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) > > + pcim_release_region(pdev, bar); > > +} > > + > > +/** > > + * pcim_request_all_regions - Request all regions > > + * @pdev: PCI device to map IO resources for > > + * @name: name associated with the request > > + * > > + * Returns: 0 on success, negative error code on failure. > > + * > > + * Requested regions will automatically be released at driver > > detach. If > > + * desired, release individual regions with pcim_release_region() > > or all of > > + * them at once with pcim_release_all_regions(). > > + */ > > +static int pcim_request_all_regions(struct pci_dev *pdev, const > > char *name) > > +{ > > + int ret; > > + short bar; > > + > > + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) { > > + ret = pcim_request_region(pdev, bar, name); > > + if (ret != 0) > > + goto err; > > } > > > > - return rc; > > + return 0; > > + > > +err: > > + pcim_release_all_regions(pdev); > > + > > + return ret; > > } > > -EXPORT_SYMBOL(pcim_iomap_regions); > > > > /** > > * pcim_iomap_regions_request_all - Request all BARs and iomap > > specified ones > > * @pdev: PCI device to map IO resources for > > * @mask: Mask of BARs to iomap > > - * @name: Name used when requesting regions > > + * @name: Name associated with the requests > > + * > > + * Returns: 0 on success, negative error code on failure. > > * > > * Request all PCI BARs and iomap regions specified by @mask. > > + * > > + * To release these resources manually, call pcim_release_region() > > for the > > + * regions and pcim_iounmap() for the mappings. > > */ > > int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask, > > const char *name) > > { > > - int request_mask = ((1 << 6) - 1) & ~mask; > > - int rc; > > + short bar; > > + int ret; > > + void __iomem **legacy_iomap_table; > > > > - rc = pci_request_selected_regions(pdev, request_mask, > > name); > > - if (rc) > > - return rc; > > + ret = pcim_request_all_regions(pdev, name); > > + if (ret != 0) > > + return ret; > > > > - rc = pcim_iomap_regions(pdev, mask, name); > > - if (rc) > > - pci_release_selected_regions(pdev, request_mask); > > - return rc; > > + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) { > > + if (!mask_contains_bar(mask, bar)) > > + continue; > > + if (!pcim_iomap(pdev, bar, 0)) > > + goto err; > > + } > > + > > + return 0; > > + > > +err: > > + /* > > + * If bar is larger than 0, then pcim_iomap() above has > > most likely > > + * failed because of -EINVAL. If it is equal 0, most likely > > the table > > + * couldn't be created, indicating -ENOMEM. > > + */ > > + ret = bar > 0 ? -EINVAL : -ENOMEM; > > + legacy_iomap_table = (void __iomem > > **)pcim_iomap_table(pdev); > > + > > + while (--bar >= 0) > > + pcim_iounmap(pdev, legacy_iomap_table[bar]); > > + > > + pcim_release_all_regions(pdev); > > + > > + return ret; > > } > > EXPORT_SYMBOL(pcim_iomap_regions_request_all); > > > > @@ -473,19 +952,14 @@ > > EXPORT_SYMBOL(pcim_iomap_regions_request_all); > > */ > > void pcim_iounmap_regions(struct pci_dev *pdev, int mask) > > { > > - void __iomem * const *iomap; > > - int i; > > + short bar; > > > > - iomap = pcim_iomap_table(pdev); > > - if (!iomap) > > - return; > > - > > - for (i = 0; i < PCIM_IOMAP_MAX; i++) { > > - if (!mask_contains_bar(mask, i)) > > + for (bar = 0; bar < PCI_STD_NUM_BARS; bar++) { > > + if (!mask_contains_bar(mask, bar)) > > continue; > > > > - pcim_iounmap(pdev, iomap[i]); > > - pci_release_region(pdev, i); > > + pcim_iounmap_region(pdev, bar); > > + pcim_remove_bar_from_legacy_table(pdev, bar); > > } > > } > > EXPORT_SYMBOL(pcim_iounmap_regions); > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index 59e0949fb079..d94445f5f882 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -3883,6 +3883,17 @@ void pci_release_region(struct pci_dev > > *pdev, int bar) > > 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 conflicts with the pcim_ > > functions being > > + * exclusively responsible for managed PCI. Its "sometimes > > yes, > > + * sometimes no" nature can cause bugs. > > + * > > + * TODO: Remove this once all users that use > > pcim_enable_device() PLUS > > + * a region request function have been ported to using > > pcim_ functions. > > + */ > > dr = find_pci_dr(pdev); > > if (dr) > > dr->region_mask &= ~(1 << bar); > > @@ -3927,6 +3938,17 @@ static int __pci_request_region(struct > > pci_dev *pdev, int bar, > > goto err_out; > > } > > > > + /* > > + * This devres utility makes this function sometimes > > managed > > + * (when pcim_enable_device() has been called before). > > + * > > + * This is bad because it conflicts with the pcim_ > > functions being > > + * exclusively responsible for managed pci. Its "sometimes > > yes, > > + * sometimes no" nature can cause bugs. > > + * > > + * TODO: Remove this once all users that use > > pcim_enable_device() PLUS > > + * a region request function have been ported to using > > pcim_ functions. > > + */ > > dr = find_pci_dr(pdev); > > if (dr) > > dr->region_mask |= 1 << bar; > > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > > index fd44565c4756..c09487f5550c 100644 > > --- a/drivers/pci/pci.h > > +++ b/drivers/pci/pci.h > > @@ -826,6 +826,11 @@ struct pci_devres { > > unsigned int orig_intx:1; > > unsigned int restore_intx:1; > > unsigned int mwi:1; > > + > > + /* > > + * TODO: remove the region_mask once everyone calling > > + * pcim_enable_device() + pci_*region*() is ported to pcim_ > > functions. > > + */ > > u32 region_mask; > > }; > > > > -- > > 2.45.0 > > >
