PCI: Refactor pci_ioremap_bar() and pci_ioremap_wc_bar()

Currently, functions pci_ioremap_bar() and pci_ioremap_wc_bar() share
similar implementation details as both functions were almost identical
in the past, especially when the latter was initially introduced in the
commit c43996f4001d ("PCI: Add pci_ioremap_wc_bar()") as somewhat exact
copy of the function pci_ioremap_bar().

However, function pci_ioremap_bar() received several updates that were
never introduced to the function pci_ioremap_wc_bar().

Thus, to align implementation of both functions and reduce the need to
duplicate code between them, introduce a new internal function called
__pci_ioremap_resource() as a helper with a shared codebase intended to
be called from functions pci_ioremap_bar() and pci_ioremap_wc_bar().

The  __pci_ioremap_resource() function will therefore include a check
for the IORESOURCE_UNSET flag that has previously been added to the
function pci_ioremap_bar() in the commit 646c0282df04 ("PCI: Fail
pci_ioremap_bar() on unassigned resources") and otherwise has been
missing from function pci_ioremap_wc_bar().

Additionally, function __pci_ioremap_resource() will retire the usage of
the WARN_ON() macro and replace it with pci_err() to show information
such as the driver name, the BAR number and resource details in case of
a failure, instead of printing a complete backtrace. The WARN_ON() has
already been replaced with pci_warn() in the commit 1f7bf3bfb5d6 ("PCI:
Show driver, BAR#, and resource on pci_ioremap_bar() failure") which
sadly didn't include an update to the function pci_ioremap_wc_bar() at
that time.

Finally, a direct use of functions ioremap() and ioremap_wc() in the
function __pci_ioremap_resource() will be replaced with calls to the
pci_iomap_range() and pci_iomap_wc_range() functions respectively.

Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
---
 drivers/pci/pci.c | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

On Tue, Jul 13, 2021 at 10:24:36AM +0000, Krzysztof Wilczyński wrote:
> Currently, functions pci_ioremap_bar() and pci_ioremap_wc_bar() share
> similar implementation details as both functions were almost identical
> in the past, especially when the latter was initially introduced in the
> commit c43996f4001d ("PCI: Add pci_ioremap_wc_bar()") as somewhat exact
> copy of the function pci_ioremap_bar().
> 
> However, function pci_ioremap_bar() received several updates that were
> never introduced to the function pci_ioremap_wc_bar().
> 
> Thus, to align implementation of both functions and reduce the need to
> duplicate code between them, introduce a new internal function called
> __pci_ioremap_resource() as a helper with a shared codebase intended to
> be called from functions pci_ioremap_bar() and pci_ioremap_wc_bar().
> 
> The  __pci_ioremap_resource() function will therefore include a check
> for the IORESOURCE_UNSET flag that has previously been added to the
> function pci_ioremap_bar() in the commit 646c0282df04 ("PCI: Fail
> pci_ioremap_bar() on unassigned resources") and otherwise has been
> missing from function pci_ioremap_wc_bar().
> 
> Additionally, function __pci_ioremap_resource() will retire the usage of
> the WARN_ON() macro and replace it with pci_err() to show information
> such as the driver name, the BAR number and resource details in case of
> a failure, instead of printing a complete backtrace. The WARN_ON() has
> already been replaced with pci_warn() in the commit 1f7bf3bfb5d6 ("PCI:
> Show driver, BAR#, and resource on pci_ioremap_bar() failure") which
> sadly didn't include an update to the function pci_ioremap_wc_bar() at
> that time.
> 
> Finally, a direct use of functions ioremap() and ioremap_wc() in the
> function __pci_ioremap_resource() will be replaced with calls to the
> pci_iomap_range() and pci_iomap_wc_range() functions respectively.
> 
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>

Nice cleanup, thanks!

Applied to pci/resource for v5.15.

I reverted to using plain ioremap() since pci_iomap_range() doesn't
seem to add anything except overhead.

> ---
>  drivers/pci/pci.c | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index e3bb0d073352..4bae55f0700b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -206,7 +206,8 @@ int pci_status_get_and_clear_errors(struct pci_dev *pdev)
>  EXPORT_SYMBOL_GPL(pci_status_get_and_clear_errors);
>  
>  #ifdef CONFIG_HAS_IOMEM
> -void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
> +static void __iomem *__pci_ioremap_resource(struct pci_dev *pdev, int bar,
> +					    bool write_combine)
>  {
>  	struct resource *res = &pdev->resource[bar];
>  
> @@ -214,24 +215,25 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
>  	 * Make sure the BAR is actually a memory resource, not an IO resource
>  	 */
>  	if (res->flags & IORESOURCE_UNSET || !(res->flags & IORESOURCE_MEM)) {
> -		pci_warn(pdev, "can't ioremap BAR %d: %pR\n", bar, res);
> +		pci_err(pdev, "can't ioremap BAR %d: %pR\n", bar, res);
>  		return NULL;
>  	}
> -	return ioremap(res->start, resource_size(res));
> +
> +	if (write_combine)
> +		return pci_iomap_wc_range(pdev, bar, 0, 0);
> +
> +	return pci_iomap_range(pdev, bar, 0, 0);
> +}
> +
> +void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
> +{
> +	return __pci_ioremap_resource(pdev, bar, false);
>  }
>  EXPORT_SYMBOL_GPL(pci_ioremap_bar);
>  
>  void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar)
>  {
> -	/*
> -	 * Make sure the BAR is actually a memory resource, not an IO resource
> -	 */
> -	if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) {
> -		WARN_ON(1);
> -		return NULL;
> -	}
> -	return ioremap_wc(pci_resource_start(pdev, bar),
> -			  pci_resource_len(pdev, bar));
> +	return __pci_ioremap_resource(pdev, bar, true);
>  }
>  EXPORT_SYMBOL_GPL(pci_ioremap_wc_bar);
>  #endif
> -- 
> 2.32.0
>