[v2,10/25] PCI: rockchip: Use pci_parse_request_of_pci_ranges()
diff mbox series

Message ID 20191016200647.32050-11-robh@kernel.org
State Superseded
Delegated to: Lorenzo Pieralisi
Headers show
Series
  • PCI host resource consolidation
Related show

Commit Message

Rob Herring Oct. 16, 2019, 8:06 p.m. UTC
Convert the Rockchip host bridge to use the common
pci_parse_request_of_pci_ranges().

There's no need to assign the resources to a temporary list first. Just
use bridge->windows directly and remove all the temporary list handling.

Cc: Shawn Lin <shawn.lin@rock-chips.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Andrew Murray <andrew.murray@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: linux-rockchip@lists.infradead.org
Signed-off-by: Rob Herring <robh@kernel.org>
---
v2:
- New patch

 drivers/pci/controller/pcie-rockchip-host.c | 36 ++++-----------------
 1 file changed, 7 insertions(+), 29 deletions(-)

--
2.20.1

Comments

Andrew Murray Oct. 18, 2019, 3:51 p.m. UTC | #1
On Wed, Oct 16, 2019 at 03:06:32PM -0500, Rob Herring wrote:
> Convert the Rockchip host bridge to use the common
> pci_parse_request_of_pci_ranges().
> 
> There's no need to assign the resources to a temporary list first. Just
> use bridge->windows directly and remove all the temporary list handling.
> 
> Cc: Shawn Lin <shawn.lin@rock-chips.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Andrew Murray <andrew.murray@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: linux-rockchip@lists.infradead.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> v2:
> - New patch
> 
>  drivers/pci/controller/pcie-rockchip-host.c | 36 ++++-----------------
>  1 file changed, 7 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> index ef8e677ce9d1..8d2e6f2e141e 100644
> --- a/drivers/pci/controller/pcie-rockchip-host.c
> +++ b/drivers/pci/controller/pcie-rockchip-host.c
> @@ -950,14 +950,10 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct pci_bus *bus, *child;
>  	struct pci_host_bridge *bridge;
> +	struct resource *bus_res;
>  	struct resource_entry *win;
> -	resource_size_t io_base;
> -	struct resource	*mem;
> -	struct resource	*io;
>  	int err;
> 
> -	LIST_HEAD(res);
> -
>  	if (!dev->of_node)
>  		return -ENODEV;
> 
> @@ -995,29 +991,20 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	if (err < 0)
>  		goto err_deinit_port;
> 
> -	err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
> -						    &res, &io_base);
> +	err = pci_parse_request_of_pci_ranges(dev, &bridge->windows, &bus_res);
>  	if (err)
>  		goto err_remove_irq_domain;
> 
> -	err = devm_request_pci_bus_resources(dev, &res);
> -	if (err)
> -		goto err_free_res;
> +	rockchip->root_bus_nr = bus_res->start;
> 
>  	/* Get the I/O and memory ranges from DT */
> -	resource_list_for_each_entry(win, &res) {
> +	resource_list_for_each_entry(win, &bridge->windows) {
>  		switch (resource_type(win->res)) {
>  		case IORESOURCE_IO:
>  			io = win->res;
>  			io->name = "I/O";

In some patches of this series we drop the custom naming of memory resources,
yet in others, like this one, we preserve the custom naming.

Should we be consistent in preserving the existing naming?

Thanks,

Andrew Murray

>  			rockchip->io_size = resource_size(io);
>  			rockchip->io_bus_addr = io->start - win->offset;
> -			err = pci_remap_iospace(io, io_base);
> -			if (err) {
> -				dev_warn(dev, "error %d: failed to map resource %pR\n",
> -					 err, io);
> -				continue;
> -			}
>  			rockchip->io = io;
>  			break;
>  		case IORESOURCE_MEM:
> @@ -1026,9 +1013,6 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  			rockchip->mem_size = resource_size(mem);
>  			rockchip->mem_bus_addr = mem->start - win->offset;
>  			break;
> -		case IORESOURCE_BUS:
> -			rockchip->root_bus_nr = win->res->start;
> -			break;
>  		default:
>  			continue;
>  		}
> @@ -1036,15 +1020,14 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
> 
>  	err = rockchip_pcie_cfg_atu(rockchip);
>  	if (err)
> -		goto err_unmap_iospace;
> +		goto err_remove_irq_domain;
> 
>  	rockchip->msg_region = devm_ioremap(dev, rockchip->msg_bus_addr, SZ_1M);
>  	if (!rockchip->msg_region) {
>  		err = -ENOMEM;
> -		goto err_unmap_iospace;
> +		goto err_remove_irq_domain;
>  	}
> 
> -	list_splice_init(&res, &bridge->windows);
>  	bridge->dev.parent = dev;
>  	bridge->sysdata = rockchip;
>  	bridge->busnr = 0;
> @@ -1054,7 +1037,7 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
> 
>  	err = pci_scan_root_bus_bridge(bridge);
>  	if (err < 0)
> -		goto err_unmap_iospace;
> +		goto err_remove_irq_domain;
> 
>  	bus = bridge->bus;
> 
> @@ -1068,10 +1051,6 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
>  	pci_bus_add_devices(bus);
>  	return 0;
> 
> -err_unmap_iospace:
> -	pci_unmap_iospace(rockchip->io);
> -err_free_res:
> -	pci_free_resource_list(&res);
>  err_remove_irq_domain:
>  	irq_domain_remove(rockchip->irq_domain);
>  err_deinit_port:
> @@ -1097,7 +1076,6 @@ static int rockchip_pcie_remove(struct platform_device *pdev)
> 
>  	pci_stop_root_bus(rockchip->root_bus);
>  	pci_remove_root_bus(rockchip->root_bus);
> -	pci_unmap_iospace(rockchip->io);
>  	irq_domain_remove(rockchip->irq_domain);
> 
>  	rockchip_pcie_deinit_phys(rockchip);
> --
> 2.20.1
Rob Herring Oct. 20, 2019, 9:36 p.m. UTC | #2
On Fri, Oct 18, 2019 at 10:52 AM Andrew Murray <andrew.murray@arm.com> wrote:
>
> On Wed, Oct 16, 2019 at 03:06:32PM -0500, Rob Herring wrote:
> > Convert the Rockchip host bridge to use the common
> > pci_parse_request_of_pci_ranges().
> >
> > There's no need to assign the resources to a temporary list first. Just
> > use bridge->windows directly and remove all the temporary list handling.
> >
> > Cc: Shawn Lin <shawn.lin@rock-chips.com>
> > Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Andrew Murray <andrew.murray@arm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Heiko Stuebner <heiko@sntech.de>
> > Cc: linux-rockchip@lists.infradead.org
> > Signed-off-by: Rob Herring <robh@kernel.org>
> > ---
> > v2:
> > - New patch
> >
> >  drivers/pci/controller/pcie-rockchip-host.c | 36 ++++-----------------
> >  1 file changed, 7 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> > index ef8e677ce9d1..8d2e6f2e141e 100644
> > --- a/drivers/pci/controller/pcie-rockchip-host.c
> > +++ b/drivers/pci/controller/pcie-rockchip-host.c
> > @@ -950,14 +950,10 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
> >       struct device *dev = &pdev->dev;
> >       struct pci_bus *bus, *child;
> >       struct pci_host_bridge *bridge;
> > +     struct resource *bus_res;
> >       struct resource_entry *win;
> > -     resource_size_t io_base;
> > -     struct resource *mem;
> > -     struct resource *io;
> >       int err;
> >
> > -     LIST_HEAD(res);
> > -
> >       if (!dev->of_node)
> >               return -ENODEV;
> >
> > @@ -995,29 +991,20 @@ static int rockchip_pcie_probe(struct platform_device *pdev)
> >       if (err < 0)
> >               goto err_deinit_port;
> >
> > -     err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
> > -                                                 &res, &io_base);
> > +     err = pci_parse_request_of_pci_ranges(dev, &bridge->windows, &bus_res);
> >       if (err)
> >               goto err_remove_irq_domain;
> >
> > -     err = devm_request_pci_bus_resources(dev, &res);
> > -     if (err)
> > -             goto err_free_res;
> > +     rockchip->root_bus_nr = bus_res->start;
> >
> >       /* Get the I/O and memory ranges from DT */
> > -     resource_list_for_each_entry(win, &res) {
> > +     resource_list_for_each_entry(win, &bridge->windows) {
> >               switch (resource_type(win->res)) {
> >               case IORESOURCE_IO:
> >                       io = win->res;
> >                       io->name = "I/O";
>
> In some patches of this series we drop the custom naming of memory resources,
> yet in others, like this one, we preserve the custom naming.

Actually, patch #11 drops it for rockchip.

> Should we be consistent in preserving the existing naming?

The custom naming seems a bit pointless IMO and something mindlessly
copied and pasted around, so I'd rather see removing the remaining
cases. The only ones left AFAICT are designware and V3.

Rob

Patch
diff mbox series

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index ef8e677ce9d1..8d2e6f2e141e 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -950,14 +950,10 @@  static int rockchip_pcie_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct pci_bus *bus, *child;
 	struct pci_host_bridge *bridge;
+	struct resource *bus_res;
 	struct resource_entry *win;
-	resource_size_t io_base;
-	struct resource	*mem;
-	struct resource	*io;
 	int err;

-	LIST_HEAD(res);
-
 	if (!dev->of_node)
 		return -ENODEV;

@@ -995,29 +991,20 @@  static int rockchip_pcie_probe(struct platform_device *pdev)
 	if (err < 0)
 		goto err_deinit_port;

-	err = devm_of_pci_get_host_bridge_resources(dev, 0, 0xff,
-						    &res, &io_base);
+	err = pci_parse_request_of_pci_ranges(dev, &bridge->windows, &bus_res);
 	if (err)
 		goto err_remove_irq_domain;

-	err = devm_request_pci_bus_resources(dev, &res);
-	if (err)
-		goto err_free_res;
+	rockchip->root_bus_nr = bus_res->start;

 	/* Get the I/O and memory ranges from DT */
-	resource_list_for_each_entry(win, &res) {
+	resource_list_for_each_entry(win, &bridge->windows) {
 		switch (resource_type(win->res)) {
 		case IORESOURCE_IO:
 			io = win->res;
 			io->name = "I/O";
 			rockchip->io_size = resource_size(io);
 			rockchip->io_bus_addr = io->start - win->offset;
-			err = pci_remap_iospace(io, io_base);
-			if (err) {
-				dev_warn(dev, "error %d: failed to map resource %pR\n",
-					 err, io);
-				continue;
-			}
 			rockchip->io = io;
 			break;
 		case IORESOURCE_MEM:
@@ -1026,9 +1013,6 @@  static int rockchip_pcie_probe(struct platform_device *pdev)
 			rockchip->mem_size = resource_size(mem);
 			rockchip->mem_bus_addr = mem->start - win->offset;
 			break;
-		case IORESOURCE_BUS:
-			rockchip->root_bus_nr = win->res->start;
-			break;
 		default:
 			continue;
 		}
@@ -1036,15 +1020,14 @@  static int rockchip_pcie_probe(struct platform_device *pdev)

 	err = rockchip_pcie_cfg_atu(rockchip);
 	if (err)
-		goto err_unmap_iospace;
+		goto err_remove_irq_domain;

 	rockchip->msg_region = devm_ioremap(dev, rockchip->msg_bus_addr, SZ_1M);
 	if (!rockchip->msg_region) {
 		err = -ENOMEM;
-		goto err_unmap_iospace;
+		goto err_remove_irq_domain;
 	}

-	list_splice_init(&res, &bridge->windows);
 	bridge->dev.parent = dev;
 	bridge->sysdata = rockchip;
 	bridge->busnr = 0;
@@ -1054,7 +1037,7 @@  static int rockchip_pcie_probe(struct platform_device *pdev)

 	err = pci_scan_root_bus_bridge(bridge);
 	if (err < 0)
-		goto err_unmap_iospace;
+		goto err_remove_irq_domain;

 	bus = bridge->bus;

@@ -1068,10 +1051,6 @@  static int rockchip_pcie_probe(struct platform_device *pdev)
 	pci_bus_add_devices(bus);
 	return 0;

-err_unmap_iospace:
-	pci_unmap_iospace(rockchip->io);
-err_free_res:
-	pci_free_resource_list(&res);
 err_remove_irq_domain:
 	irq_domain_remove(rockchip->irq_domain);
 err_deinit_port:
@@ -1097,7 +1076,6 @@  static int rockchip_pcie_remove(struct platform_device *pdev)

 	pci_stop_root_bus(rockchip->root_bus);
 	pci_remove_root_bus(rockchip->root_bus);
-	pci_unmap_iospace(rockchip->io);
 	irq_domain_remove(rockchip->irq_domain);

 	rockchip_pcie_deinit_phys(rockchip);