[PATCH 1/3] PCI: tegra: add missing __iomem annotation

[PATCH 1/3] PCI: tegra: add missing __iomem annotation

[Jingoo Han]

Added missing __iomem annotation in order to fix the following
sparse warnings:

drivers/pci/host/pci-tegra.c:411:41: warning: incorrect type in return expression (different address spaces)
drivers/pci/host/pci-tegra.c:411:41:    expected void [noderef] <asn:2>*
drivers/pci/host/pci-tegra.c:411:41:    got void *addr
drivers/pci/host/pci-tegra.c:419:25: warning: incorrect type in return expression (different address spaces)
drivers/pci/host/pci-tegra.c:419:25:    expected void [noderef] <asn:2>*
drivers/pci/host/pci-tegra.c:419:25:    got void *addr

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/pci/host/pci-tegra.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/host/pci-tegra.c b/drivers/pci/host/pci-tegra.c
index 2e9888a..7c4f38d 100644
--- a/drivers/pci/host/pci-tegra.c
+++ b/drivers/pci/host/pci-tegra.c
@@ -408,7 +408,7 @@ static void __iomem *tegra_pcie_bus_map(struct tegra_pcie *pcie,
 
 	list_for_each_entry(bus, &pcie->busses, list)
 		if (bus->nr == busnr)
-			return bus->area->addr;
+			return (void __iomem *)bus->area->addr;
 
 	bus = tegra_pcie_bus_alloc(pcie, busnr);
 	if (IS_ERR(bus))
@@ -416,7 +416,7 @@ static void __iomem *tegra_pcie_bus_map(struct tegra_pcie *pcie,
 
 	list_add_tail(&bus->list, &pcie->busses);
 
-	return bus->area->addr;
+	return (void __iomem *)bus->area->addr;
 }
 
 static void __iomem *tegra_pcie_conf_address(struct pci_bus *bus,
-- 
1.7.10.4

[PATCH 2/3] PCI: mvebu: make local functions static

[Jingoo Han]

mvebu_pcie_add_bus(), mvebu_pcie_align_resource()  are used only
in this file. Thus, these local functions should be staticized
in order to fix the following sparse warnings:

drivers/pci/host/pci-mvebu.c:684:6: warning: symbol 'mvebu_pcie_add_bus' was not declared. Should it be static?
drivers/pci/host/pci-mvebu.c:690:17: warning: symbol 'mvebu_pcie_align_resource' was not declared. Should it be static?

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/pci/host/pci-mvebu.c |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index d66530e..b54ceb1 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -681,17 +681,17 @@ static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	return bus;
 }
 
-void mvebu_pcie_add_bus(struct pci_bus *bus)
+static void mvebu_pcie_add_bus(struct pci_bus *bus)
 {
 	struct mvebu_pcie *pcie = sys_to_pcie(bus->sysdata);
 	bus->msi = pcie->msi;
 }
 
-resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev,
-					  const struct resource *res,
-					  resource_size_t start,
-					  resource_size_t size,
-					  resource_size_t align)
+static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev,
+						const struct resource *res,
+						resource_size_t start,
+						resource_size_t size,
+						resource_size_t align)
 {
 	if (dev->bus->number != 0)
 		return start;
-- 
1.7.10.4

Re: [PATCH 2/3] PCI: mvebu: make local functions static

[Thomas Petazzoni]

Dear Jingoo Han,

On Thu, 12 Sep 2013 18:36:33 +0900, Jingoo Han wrote:
> mvebu_pcie_add_bus(), mvebu_pcie_align_resource()  are used only
> in this file. Thus, these local functions should be staticized
> in order to fix the following sparse warnings:
> 
> drivers/pci/host/pci-mvebu.c:684:6: warning: symbol
> 'mvebu_pcie_add_bus' was not declared. Should it be static?
> drivers/pci/host/pci-mvebu.c:690:17: warning: symbol
> 'mvebu_pcie_align_resource' was not declared. Should it be static?
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>

Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[PATCH 3/3] PCI: mvebu: add missing __iomem annotation

[Jingoo Han]

Added missing __iomem annotation in order to fix the following
sparse warning:

drivers/pci/host/pci-mvebu.c:744:31: warning: incorrect type in return expression (different address spaces)
drivers/pci/host/pci-mvebu.c:744:31:    expected void [noderef] <asn:2>*
drivers/pci/host/pci-mvebu.c:744:31:    got void *

Signed-off-by: Jingoo Han <jg1.han@samsung.com>
---
 drivers/pci/host/pci-mvebu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index b54ceb1..2d8e478 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -741,7 +741,7 @@ static void __iomem *mvebu_pcie_map_registers(struct platform_device *pdev,
 
 	ret = of_address_to_resource(np, 0, &regs);
 	if (ret)
-		return ERR_PTR(ret);
+		return (void __iomem *)ERR_PTR(ret);
 
 	return devm_ioremap_resource(&pdev->dev, &regs);
 }
-- 
1.7.10.4

Re: [PATCH 3/3] PCI: mvebu: add missing __iomem annotation

[Thomas Petazzoni]

Dear Jingoo Han,

On Thu, 12 Sep 2013 18:37:18 +0900, Jingoo Han wrote:

>  	ret = of_address_to_resource(np, 0, &regs);
>  	if (ret)
> -		return ERR_PTR(ret);
> +		return (void __iomem *)ERR_PTR(ret);

This doesn't look very pretty to tell the truth, but I don't quite see
any other option. Just return NULL when of_address_to_resource() fails
instead of trying to propagate the error? Make this entire function
return an 'int' and have the iomem address returned through a pointer
passed by address as argument to the function? Any other suggestion?

Thanks,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 3/3] PCI: mvebu: add missing __iomem annotation

[Jingoo Han]

On Tuesday, September 17, 2013 12:57 AM, Thomas Petazzoni wrote:
> On Thu, 12 Sep 2013 18:37:18 +0900, Jingoo Han wrote:
> 
> >  	ret = of_address_to_resource(np, 0, &regs);
> >  	if (ret)
> > -		return ERR_PTR(ret);
> > +		return (void __iomem *)ERR_PTR(ret);
> 
> This doesn't look very pretty to tell the truth, but I don't quite see
> any other option. Just return NULL when of_address_to_resource() fails
> instead of trying to propagate the error? Make this entire function
> return an 'int' and have the iomem address returned through a pointer
> passed by address as argument to the function? Any other suggestion?
> 

Hi Thomas Petazzoni,
I appreciated your feedback. :-)

'Just returning NULL when of_address_to_resource() fails
instead of trying to propagate the error' looks better.

Then, how about the following?

--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -728,7 +728,7 @@ mvebu_pcie_map_registers(struct platform_device *pdev,

	ret = of_address_to_resource(np, 0, &regs);
	if (ret)
-		return ERR_PTR(ret);
+		return NULL;

	return devm_ioremap_resource(&pdev->dev, &regs);
 }
@@ -874,7 +874,7 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
		}

		port->base = mvebu_pcie_map_registers(pdev, child, port);
-		if (IS_ERR(port->base)) {
+		if (!port->base) {
			dev_err(&pdev->dev, "PCIe%d.%d: cannot map registers\n",
  				port->port, port->lane);
			port->base = NULL;

Best regards,
Jingoo Han

Re: [PATCH 3/3] PCI: mvebu: add missing __iomem annotation

[Thomas Petazzoni]

Dear Jingoo Han,

On Tue, 17 Sep 2013 09:52:06 +0900, Jingoo Han wrote:

> 'Just returning NULL when of_address_to_resource() fails
> instead of trying to propagate the error' looks better.
> 
> Then, how about the following?
> 
> --- a/drivers/pci/host/pci-mvebu.c
> +++ b/drivers/pci/host/pci-mvebu.c
> @@ -728,7 +728,7 @@ mvebu_pcie_map_registers(struct platform_device *pdev,
> 
> 	ret = of_address_to_resource(np, 0, &regs);
> 	if (ret)
> -		return ERR_PTR(ret);
> +		return NULL;
> 
> 	return devm_ioremap_resource(&pdev->dev, &regs);
>  }
> @@ -874,7 +874,7 @@ static int __init mvebu_pcie_probe(struct platform_device *pdev)
> 		}
> 
> 		port->base = mvebu_pcie_map_registers(pdev, child, port);
> -		if (IS_ERR(port->base)) {
> +		if (!port->base) {
> 			dev_err(&pdev->dev, "PCIe%d.%d: cannot map registers\n",
>   				port->port, port->lane);
> 			port->base = NULL;

Yes, I believe it makes more sense since we were anyway to using the
error code returned by of_address_to_ressource().

However, you can get rid of the port->base = NULL assignment in the if
() { ... } code now.

With that fixed, you can add my:

Acked-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Re: [PATCH 1/3] PCI: tegra: add missing __iomem annotation

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 925 bytes --]

On Thu, Sep 12, 2013 at 06:32:45PM +0900, Jingoo Han wrote:
> Added missing __iomem annotation in order to fix the following
> sparse warnings:
> 
> drivers/pci/host/pci-tegra.c:411:41: warning: incorrect type in return expression (different address spaces)
> drivers/pci/host/pci-tegra.c:411:41:    expected void [noderef] <asn:2>*
> drivers/pci/host/pci-tegra.c:411:41:    got void *addr
> drivers/pci/host/pci-tegra.c:419:25: warning: incorrect type in return expression (different address spaces)
> drivers/pci/host/pci-tegra.c:419:25:    expected void [noderef] <asn:2>*
> drivers/pci/host/pci-tegra.c:419:25:    got void *addr
> 
> Signed-off-by: Jingoo Han <jg1.han@samsung.com>
> ---
>  drivers/pci/host/pci-tegra.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

I can carry this in a local tree or if somebody else wants to take it:

Acked-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]