| Message ID | 20240807083018.8734-2-pstanner@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d140f80f603584d8282817994c0c6241e736bef4 |
| Headers | show |
| Series | [1/2] PCI: Deprecate pcim_iomap_regions() in favor of pcim_iomap_region() | expand |
On Wed, Aug 07, 2024 at 10:30:18AM +0200, Philipp Stanner wrote: > pcim_iomap_regions() is a complicated function that uses a bit mask to > determine the BARs the user wishes to request and ioremap. Almost all > users only ever set a single bit in that mask, making that mechanism > questionable. > > pcim_iomap_region() is now available as a more simple replacement. > > Make pcim_iomap_region() a public function. > > Mark pcim_iomap_regions() as deprecated. > > Signed-off-by: Philipp Stanner <pstanner@redhat.com> The interesting part of this little series is in ast_drv.c, but there may be similar conversions for other drivers coming as well. To avoid races during the merge window, I propose merging this via the PCI tree so I can ensure that any other conversions happen after pcim_iomap_region() becomes public. That would require an ack from Dave. But if you'd rather take this yourself, Dave, here's my ack for the PCI piece: Acked-by: Bjorn Helgaas <bhelgaas@google.com> > --- > drivers/pci/devres.c | 8 ++++++-- > include/linux/pci.h | 2 ++ > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c > index 3780a9f9ec00..89ec26ea1501 100644 > --- a/drivers/pci/devres.c > +++ b/drivers/pci/devres.c > @@ -728,7 +728,7 @@ EXPORT_SYMBOL(pcim_iounmap); > * 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, > +void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, > const char *name) > { > int ret; > @@ -761,6 +761,7 @@ static void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, > > return IOMEM_ERR_PTR(ret); > } > +EXPORT_SYMBOL(pcim_iomap_region); > > /** > * pcim_iounmap_region - Unmap and release a PCI BAR > @@ -783,7 +784,7 @@ static void pcim_iounmap_region(struct pci_dev *pdev, int bar) > } > > /** > - * pcim_iomap_regions - Request and iomap PCI BARs > + * pcim_iomap_regions - Request and iomap PCI BARs (DEPRECATED) > * @pdev: PCI device to map IO resources for > * @mask: Mask of BARs to request and iomap > * @name: Name associated with the requests > @@ -791,6 +792,9 @@ static void pcim_iounmap_region(struct pci_dev *pdev, int bar) > * Returns: 0 on success, negative error code on failure. > * > * Request and iomap regions specified by @mask. > + * > + * This function is DEPRECATED. Do not use it in new code. > + * Use pcim_iomap_region() instead. > */ > int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name) > { > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 4cf89a4b4cbc..fc30176d28ca 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -2292,6 +2292,8 @@ static inline void pci_fixup_device(enum pci_fixup_pass pass, > void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen); > void pcim_iounmap(struct pci_dev *pdev, void __iomem *addr); > void __iomem * const *pcim_iomap_table(struct pci_dev *pdev); > +void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, > + const char *name); > int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name); > int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask, > const char *name); > -- > 2.45.2 >
On Thu, Aug 8, 2024 at 6:33 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Wed, Aug 07, 2024 at 10:30:18AM +0200, Philipp Stanner wrote: > > pcim_iomap_regions() is a complicated function that uses a bit mask to > > determine the BARs the user wishes to request and ioremap. Almost all > > users only ever set a single bit in that mask, making that mechanism > > questionable. > > > > pcim_iomap_region() is now available as a more simple replacement. > > > > Make pcim_iomap_region() a public function. > > > > Mark pcim_iomap_regions() as deprecated. > > > > Signed-off-by: Philipp Stanner <pstanner@redhat.com> > > The interesting part of this little series is in ast_drv.c, but there > may be similar conversions for other drivers coming as well. > > To avoid races during the merge window, I propose merging this via the > PCI tree so I can ensure that any other conversions happen after > pcim_iomap_region() becomes public. > > That would require an ack from Dave. But if you'd rather take this > yourself, Dave, here's my ack for the PCI piece: > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> I'm fine with it going in via pci. Acked-by: Dave Airlie <airlied@redhat.com> Dave. > > > --- > > drivers/pci/devres.c | 8 ++++++-- > > include/linux/pci.h | 2 ++ > > 2 files changed, 8 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c > > index 3780a9f9ec00..89ec26ea1501 100644 > > --- a/drivers/pci/devres.c > > +++ b/drivers/pci/devres.c > > @@ -728,7 +728,7 @@ EXPORT_SYMBOL(pcim_iounmap); > > * 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, > > +void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, > > const char *name) > > { > > int ret; > > @@ -761,6 +761,7 @@ static void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, > > > > return IOMEM_ERR_PTR(ret); > > } > > +EXPORT_SYMBOL(pcim_iomap_region); > > > > /** > > * pcim_iounmap_region - Unmap and release a PCI BAR > > @@ -783,7 +784,7 @@ static void pcim_iounmap_region(struct pci_dev *pdev, int bar) > > } > > > > /** > > - * pcim_iomap_regions - Request and iomap PCI BARs > > + * pcim_iomap_regions - Request and iomap PCI BARs (DEPRECATED) > > * @pdev: PCI device to map IO resources for > > * @mask: Mask of BARs to request and iomap > > * @name: Name associated with the requests > > @@ -791,6 +792,9 @@ static void pcim_iounmap_region(struct pci_dev *pdev, int bar) > > * Returns: 0 on success, negative error code on failure. > > * > > * Request and iomap regions specified by @mask. > > + * > > + * This function is DEPRECATED. Do not use it in new code. > > + * Use pcim_iomap_region() instead. > > */ > > int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name) > > { > > diff --git a/include/linux/pci.h b/include/linux/pci.h > > index 4cf89a4b4cbc..fc30176d28ca 100644 > > --- a/include/linux/pci.h > > +++ b/include/linux/pci.h > > @@ -2292,6 +2292,8 @@ static inline void pci_fixup_device(enum pci_fixup_pass pass, > > void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen); > > void pcim_iounmap(struct pci_dev *pdev, void __iomem *addr); > > void __iomem * const *pcim_iomap_table(struct pci_dev *pdev); > > +void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, > > + const char *name); > > int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name); > > int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask, > > const char *name); > > -- > > 2.45.2 > > >
On Wed, Aug 07, 2024 at 10:30:18AM +0200, Philipp Stanner wrote: > pcim_iomap_regions() is a complicated function that uses a bit mask to > determine the BARs the user wishes to request and ioremap. Almost all > users only ever set a single bit in that mask, making that mechanism > questionable. > > pcim_iomap_region() is now available as a more simple replacement. > > Make pcim_iomap_region() a public function. > > Mark pcim_iomap_regions() as deprecated. > > Signed-off-by: Philipp Stanner <pstanner@redhat.com> Both applied (second with Thomas' reviewed-by and Dave's ack) to pci/devres for v6.12, thanks! > --- > drivers/pci/devres.c | 8 ++++++-- > include/linux/pci.h | 2 ++ > 2 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c > index 3780a9f9ec00..89ec26ea1501 100644 > --- a/drivers/pci/devres.c > +++ b/drivers/pci/devres.c > @@ -728,7 +728,7 @@ EXPORT_SYMBOL(pcim_iounmap); > * 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, > +void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, > const char *name) > { > int ret; > @@ -761,6 +761,7 @@ static void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, > > return IOMEM_ERR_PTR(ret); > } > +EXPORT_SYMBOL(pcim_iomap_region); > > /** > * pcim_iounmap_region - Unmap and release a PCI BAR > @@ -783,7 +784,7 @@ static void pcim_iounmap_region(struct pci_dev *pdev, int bar) > } > > /** > - * pcim_iomap_regions - Request and iomap PCI BARs > + * pcim_iomap_regions - Request and iomap PCI BARs (DEPRECATED) > * @pdev: PCI device to map IO resources for > * @mask: Mask of BARs to request and iomap > * @name: Name associated with the requests > @@ -791,6 +792,9 @@ static void pcim_iounmap_region(struct pci_dev *pdev, int bar) > * Returns: 0 on success, negative error code on failure. > * > * Request and iomap regions specified by @mask. > + * > + * This function is DEPRECATED. Do not use it in new code. > + * Use pcim_iomap_region() instead. > */ > int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name) > { > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 4cf89a4b4cbc..fc30176d28ca 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -2292,6 +2292,8 @@ static inline void pci_fixup_device(enum pci_fixup_pass pass, > void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen); > void pcim_iounmap(struct pci_dev *pdev, void __iomem *addr); > void __iomem * const *pcim_iomap_table(struct pci_dev *pdev); > +void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, > + const char *name); > int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name); > int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask, > const char *name); > -- > 2.45.2 >
diff --git a/drivers/pci/devres.c b/drivers/pci/devres.c index 3780a9f9ec00..89ec26ea1501 100644 --- a/drivers/pci/devres.c +++ b/drivers/pci/devres.c @@ -728,7 +728,7 @@ EXPORT_SYMBOL(pcim_iounmap); * 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, +void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, const char *name) { int ret; @@ -761,6 +761,7 @@ static void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, return IOMEM_ERR_PTR(ret); } +EXPORT_SYMBOL(pcim_iomap_region); /** * pcim_iounmap_region - Unmap and release a PCI BAR @@ -783,7 +784,7 @@ static void pcim_iounmap_region(struct pci_dev *pdev, int bar) } /** - * pcim_iomap_regions - Request and iomap PCI BARs + * pcim_iomap_regions - Request and iomap PCI BARs (DEPRECATED) * @pdev: PCI device to map IO resources for * @mask: Mask of BARs to request and iomap * @name: Name associated with the requests @@ -791,6 +792,9 @@ static void pcim_iounmap_region(struct pci_dev *pdev, int bar) * Returns: 0 on success, negative error code on failure. * * Request and iomap regions specified by @mask. + * + * This function is DEPRECATED. Do not use it in new code. + * Use pcim_iomap_region() instead. */ int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name) { diff --git a/include/linux/pci.h b/include/linux/pci.h index 4cf89a4b4cbc..fc30176d28ca 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -2292,6 +2292,8 @@ static inline void pci_fixup_device(enum pci_fixup_pass pass, void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen); void pcim_iounmap(struct pci_dev *pdev, void __iomem *addr); void __iomem * const *pcim_iomap_table(struct pci_dev *pdev); +void __iomem *pcim_iomap_region(struct pci_dev *pdev, int bar, + const char *name); int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name); int pcim_iomap_regions_request_all(struct pci_dev *pdev, int mask, const char *name);
pcim_iomap_regions() is a complicated function that uses a bit mask to determine the BARs the user wishes to request and ioremap. Almost all users only ever set a single bit in that mask, making that mechanism questionable. pcim_iomap_region() is now available as a more simple replacement. Make pcim_iomap_region() a public function. Mark pcim_iomap_regions() as deprecated. Signed-off-by: Philipp Stanner <pstanner@redhat.com> --- drivers/pci/devres.c | 8 ++++++-- include/linux/pci.h | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-)