[PATCH 0/4] PCI: designware: init order/resource alloc fixes

[PATCH 0/4] PCI: designware: init order/resource alloc fixes

[Lucas Stach]

This series fixes a couple of issues with the initialisation
ordering and ressource allocation handling in the designware
driver and moves its behavior much closer to other PCI host
controller drivers like MVEBU and Tegra.
For more detailed explanation see the commit messages.

This effectively enables the i.MX6 pcie driver to be
probed from module initcall level, instead of relying on
being probed early.

Lucas Stach (4):
  PCI: designware: start parsing bus-range
  PCI: designware: get rid of pci_scan_root_bus
  PCI: designware: remove pci_assign_unassigned_resources call
  PCI: imx6: move to module_init

 .../devicetree/bindings/pci/designware-pcie.txt    |  3 +++
 drivers/pci/host/pci-imx6.c                        |  2 +-
 drivers/pci/host/pcie-designware.c                 | 29 ++++++++++++++--------
 drivers/pci/host/pcie-designware.h                 |  1 +
 4 files changed, 24 insertions(+), 11 deletions(-)

-- 
2.0.1

[PATCH 1/4] PCI: designware: start parsing bus-range

[Lucas Stach]

This allows to explicitly specify the covered bus
numbers in the devicetree, which will come in handy
once we see a SoC with more than one PCIe host
controller instance.

Previously the driver relied on the behavior of
pci_scan_root_bus() to fill in a range of 0x00-0xff
if no valid range was found. We fall back to the
same range if no valid DT entry was found to keep
backwards compatibility, but now do it explicitly.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 Documentation/devicetree/bindings/pci/designware-pcie.txt |  3 +++
 drivers/pci/host/pcie-designware.c                        | 13 ++++++++++++-
 drivers/pci/host/pcie-designware.h                        |  1 +
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
index ed0d9b9fff2b..9f4faa8e8d00 100644
--- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
+++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
@@ -23,3 +23,6 @@ Required properties:
 
 Optional properties:
 - reset-gpio: gpio pin number of power good signal
+- bus-range: PCI bus numbers covered (it is recommended for new devicetrees to
+  specify this property, to keep backwards compatibility a range of 0x00-0xff
+  is assumed if not present)
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 52bd3a143563..b13a830c8b0f 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -425,7 +425,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
 	struct resource *cfg_res;
 	u32 val, na, ns;
 	const __be32 *addrp;
-	int i, index;
+	int i, index, ret;
 
 	/* Find the address cell size and the number of cells in order to get
 	 * the untranslated address.
@@ -500,6 +500,16 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
 		}
 	}
 
+	ret = of_pci_parse_bus_range(np, &pp->busn);
+	if (ret < 0) {
+		dev_dbg(pp->dev, "failed to parse bus-range property: %d, using default [0x00-0xff]\n",
+			ret);
+		pp->busn.name = np->name;
+		pp->busn.start = 0;
+		pp->busn.end = 0xff;
+		pp->busn.flags = IORESOURCE_BUS;
+	}
+
 	if (!pp->dbi_base) {
 		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
 					resource_size(&pp->cfg));
@@ -781,6 +791,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
 
 	sys->mem_offset = pp->mem.start - pp->config.mem_bus_addr;
 	pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
+	pci_add_resource(&sys->resources, &pp->busn);
 
 	return 1;
 }
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index daf81f922cda..9a09633ecdf8 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -48,6 +48,7 @@ struct pcie_port {
 	struct resource		cfg;
 	struct resource		io;
 	struct resource		mem;
+	struct resource		busn;
 	struct pcie_port_info	config;
 	int			irq;
 	u32			lanes;
-- 
2.0.1

[PATCH 2/4] PCI: designware: get rid of pci_scan_root_bus

[Lucas Stach]

Use pci_create_root_bus() similar to other pci
host controller drivers.

The main problem with pci_scan_root_bus() is that
not only it will create the root bus, but also try
to activate all devices on the bus. This triggers
PCI device drivers probe, which try to use the device
but fail as resource allocation isn't done at that
point in time.

To work around this we made sure that the host
controller driver is probed early and finishes resource
allocation before any other device drivers are registered.
Switching to pci_create_root_bus() allows us to get rid
of this special handling.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/pci/host/pcie-designware.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index b13a830c8b0f..b869e202367c 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -801,14 +801,13 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
 	struct pci_bus *bus;
 	struct pcie_port *pp = sys_to_pcie(sys);
 
-	if (pp) {
-		pp->root_bus_nr = sys->busnr;
-		bus = pci_scan_root_bus(pp->dev, sys->busnr, &dw_pcie_ops,
-					sys, &sys->resources);
-	} else {
-		bus = NULL;
-		BUG();
-	}
+	pp->root_bus_nr = sys->busnr;
+	bus = pci_create_root_bus(pp->dev, sys->busnr,
+				  &dw_pcie_ops, sys, &sys->resources);
+	if (!bus)
+		return NULL;
+
+	pci_scan_child_bus(bus);
 
 	return bus;
 }
-- 
2.0.1

[PATCH 3/4] PCI: designware: remove pci_assign_unassigned_resources call

[Lucas Stach]

The pci_common_init_dev() call right before will already
handle the device resource allocation, so this call
was a no-op.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/pci/host/pcie-designware.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index b869e202367c..dde5e6d4afa2 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -568,7 +568,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
 	dw_pci.private_data = (void **)&pp;
 
 	pci_common_init_dev(pp->dev, &dw_pci);
-	pci_assign_unassigned_resources();
 #ifdef CONFIG_PCI_DOMAINS
 	dw_pci.domain++;
 #endif
-- 
2.0.1

[PATCH 4/4] PCI: imx6: move to module_init

[Lucas Stach]

This effectively reverts f216f57ffe6eede3c8a763add65d331e688f8c56
"PCI: imx6: Probe the PCIe in fs_initcall()" as the ressource
allocation issue that prevented the driver from working properly
at module_initcall level is now fixed in pcie-designware.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/pci/host/pci-imx6.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index a568efaa331c..0fc2ba8ea006 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -609,7 +609,7 @@ static int __init imx6_pcie_init(void)
 {
 	return platform_driver_probe(&imx6_pcie_driver, imx6_pcie_probe);
 }
-fs_initcall(imx6_pcie_init);
+module_init(imx6_pcie_init);
 
 MODULE_AUTHOR("Sean Cross <xobs@kosagi.com>");
 MODULE_DESCRIPTION("Freescale i.MX6 PCIe host controller driver");
-- 
2.0.1

Re: [PATCH 0/4] PCI: designware: init order/resource alloc fixes

[Marek Vasut]

On Wednesday, July 23, 2014 at 07:52:37 PM, Lucas Stach wrote:
> This series fixes a couple of issues with the initialisation
> ordering and ressource allocation handling in the designware
> driver and moves its behavior much closer to other PCI host
> controller drivers like MVEBU and Tegra.
> For more detailed explanation see the commit messages.
> 
> This effectively enables the i.MX6 pcie driver to be
> probed from module initcall level, instead of relying on
> being probed early.
> 
> Lucas Stach (4):
>   PCI: designware: start parsing bus-range
>   PCI: designware: get rid of pci_scan_root_bus
>   PCI: designware: remove pci_assign_unassigned_resources call
>   PCI: imx6: move to module_init
> 
>  .../devicetree/bindings/pci/designware-pcie.txt    |  3 +++
>  drivers/pci/host/pci-imx6.c                        |  2 +-
>  drivers/pci/host/pcie-designware.c                 | 29
> ++++++++++++++-------- drivers/pci/host/pcie-designware.h                
> |  1 +
>  4 files changed, 24 insertions(+), 11 deletions(-)

Looks reasonable, thanks

Acked-by: Marek Vasut <marex@denx.de>

Best regards,
Marek Vasut

Re: [PATCH 2/4] PCI: designware: get rid of pci_scan_root_bus

[Jingoo Han]

On Thursday, July 24, 2014 2:53 AM, Lucas Stach wrote:
> 
> Use pci_create_root_bus() similar to other pci
> host controller drivers.
> 
> The main problem with pci_scan_root_bus() is that
> not only it will create the root bus, but also try
> to activate all devices on the bus. This triggers
> PCI device drivers probe, which try to use the device
> but fail as resource allocation isn't done at that
> point in time.
> 
> To work around this we made sure that the host
> controller driver is probed early and finishes resource
> allocation before any other device drivers are registered.
> Switching to pci_create_root_bus() allows us to get rid
> of this special handling.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/pci/host/pcie-designware.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index b13a830c8b0f..b869e202367c 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -801,14 +801,13 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
>  	struct pci_bus *bus;
>  	struct pcie_port *pp = sys_to_pcie(sys);
> 
> -	if (pp) {
> -		pp->root_bus_nr = sys->busnr;
> -		bus = pci_scan_root_bus(pp->dev, sys->busnr, &dw_pcie_ops,
> -					sys, &sys->resources);
> -	} else {
> -		bus = NULL;
> -		BUG();
> -	}
> +	pp->root_bus_nr = sys->busnr;
> +	bus = pci_create_root_bus(pp->dev, sys->busnr,
> +				  &dw_pcie_ops, sys, &sys->resources);
> +	if (!bus)
> +		return NULL;
> +
> +	pci_scan_child_bus(bus);


I think that we still need to check if pp is NULL.
Then, how about adding NULL check as below?


+	if (!pp) {
+		BUG();
+		return NULL;
+	}
+
+	pp->root_bus_nr = sys->busnr;
+	bus = pci_create_root_bus(pp->dev, sys->busnr,
+				  &dw_pcie_ops, sys, &sys->resources);
+	if (!bus)
+		return NULL;
+
+	pci_scan_child_bus(bus);


Best regards,
Jingoo Han

> 
>  	return bus;
>  }
> --
> 2.0.1

RE: [PATCH 4/4] PCI: imx6: move to module_init

[Hong-Xing.Zhu]

> -----Original Message-----
> From: Lucas Stach [mailto:l.stach@pengutronix.de]
> Sent: Thursday, July 24, 2014 1:53 AM
> To: Bjorn Helgaas
> Cc: linux-pci@vger.kernel.org; Zhu Richard-R65037; Mohit Kumar; Jingoo Han;
> Marek Vasut; Kishon Vijay Abraham I; kernel@pengutronix.de
> Subject: [PATCH 4/4] PCI: imx6: move to module_init
> 
> This effectively reverts f216f57ffe6eede3c8a763add65d331e688f8c56
> "PCI: imx6: Probe the PCIe in fs_initcall()" as the ressource allocation issue
> that prevented the driver from working properly at module_initcall level is
> now fixed in pcie-designware.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/pci/host/pci-imx6.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c index
> a568efaa331c..0fc2ba8ea006 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -609,7 +609,7 @@ static int __init imx6_pcie_init(void)  {
>  	return platform_driver_probe(&imx6_pcie_driver, imx6_pcie_probe);  } -
> fs_initcall(imx6_pcie_init);
> +module_init(imx6_pcie_init);
> 
Acked-by: Richard Zhu <r65037@freescale.com>

Best Regards
Richard Zhu
>  MODULE_AUTHOR("Sean Cross <xobs@kosagi.com>");  MODULE_DESCRIPTION("Freescale
> i.MX6 PCIe host controller driver");
> --
> 2.0.1

Re: [PATCH 2/4] PCI: designware: get rid of pci_scan_root_bus

[Lucas Stach]

Hi Jingoo,

Am Donnerstag, den 24.07.2014, 13:11 +0900 schrieb Jingoo Han:
> On Thursday, July 24, 2014 2:53 AM, Lucas Stach wrote:
> > 
> > Use pci_create_root_bus() similar to other pci
> > host controller drivers.
> > 
> > The main problem with pci_scan_root_bus() is that
> > not only it will create the root bus, but also try
> > to activate all devices on the bus. This triggers
> > PCI device drivers probe, which try to use the device
> > but fail as resource allocation isn't done at that
> > point in time.
> > 
> > To work around this we made sure that the host
> > controller driver is probed early and finishes resource
> > allocation before any other device drivers are registered.
> > Switching to pci_create_root_bus() allows us to get rid
> > of this special handling.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  drivers/pci/host/pcie-designware.c | 15 +++++++--------
> >  1 file changed, 7 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > index b13a830c8b0f..b869e202367c 100644
> > --- a/drivers/pci/host/pcie-designware.c
> > +++ b/drivers/pci/host/pcie-designware.c
> > @@ -801,14 +801,13 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
> >  	struct pci_bus *bus;
> >  	struct pcie_port *pp = sys_to_pcie(sys);
> > 
> > -	if (pp) {
> > -		pp->root_bus_nr = sys->busnr;
> > -		bus = pci_scan_root_bus(pp->dev, sys->busnr, &dw_pcie_ops,
> > -					sys, &sys->resources);
> > -	} else {
> > -		bus = NULL;
> > -		BUG();
> > -	}
> > +	pp->root_bus_nr = sys->busnr;
> > +	bus = pci_create_root_bus(pp->dev, sys->busnr,
> > +				  &dw_pcie_ops, sys, &sys->resources);
> > +	if (!bus)
> > +		return NULL;
> > +
> > +	pci_scan_child_bus(bus);
> 
> 
> I think that we still need to check if pp is NULL.
> Then, how about adding NULL check as below?
> 
No I don't think it's needed as it doesn't provide any additional
security.

The only code path that is calling this dw_pci_scan_bus() is in
pcibios_init_hw() which always fills sys->private_data with
hw_pci->private_data. As we are always filling hw_pci->private_data with
a pointer to our pcie_port structure in dw_pcie_host_init() the only way
for pp to be NULL at this point would be for pp to be NULL in
dw_pcie_host_init(). This would obviously crash a lot earlier.

So this check is just adding to LOC without adding any value.

Regards,
Lucas

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [PATCH 2/4] PCI: designware: get rid of pci_scan_root_bus

[Jingoo Han]

On Thursday, July 24, 2014 5:32 PM, Lucas Stach wrote:
> Am Donnerstag, den 24.07.2014, 13:11 +0900 schrieb Jingoo Han:
> > On Thursday, July 24, 2014 2:53 AM, Lucas Stach wrote:
> > >
> > > Use pci_create_root_bus() similar to other pci
> > > host controller drivers.
> > >
> > > The main problem with pci_scan_root_bus() is that
> > > not only it will create the root bus, but also try
> > > to activate all devices on the bus. This triggers
> > > PCI device drivers probe, which try to use the device
> > > but fail as resource allocation isn't done at that
> > > point in time.
> > >
> > > To work around this we made sure that the host
> > > controller driver is probed early and finishes resource
> > > allocation before any other device drivers are registered.
> > > Switching to pci_create_root_bus() allows us to get rid
> > > of this special handling.
> > >
> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > ---
> > >  drivers/pci/host/pcie-designware.c | 15 +++++++--------
> > >  1 file changed, 7 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > > index b13a830c8b0f..b869e202367c 100644
> > > --- a/drivers/pci/host/pcie-designware.c
> > > +++ b/drivers/pci/host/pcie-designware.c
> > > @@ -801,14 +801,13 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
> > >  	struct pci_bus *bus;
> > >  	struct pcie_port *pp = sys_to_pcie(sys);
> > >
> > > -	if (pp) {
> > > -		pp->root_bus_nr = sys->busnr;
> > > -		bus = pci_scan_root_bus(pp->dev, sys->busnr, &dw_pcie_ops,
> > > -					sys, &sys->resources);
> > > -	} else {
> > > -		bus = NULL;
> > > -		BUG();
> > > -	}
> > > +	pp->root_bus_nr = sys->busnr;
> > > +	bus = pci_create_root_bus(pp->dev, sys->busnr,
> > > +				  &dw_pcie_ops, sys, &sys->resources);
> > > +	if (!bus)
> > > +		return NULL;
> > > +
> > > +	pci_scan_child_bus(bus);
> >
> >
> > I think that we still need to check if pp is NULL.
> > Then, how about adding NULL check as below?
> >
> No I don't think it's needed as it doesn't provide any additional
> security.
> 
> The only code path that is calling this dw_pci_scan_bus() is in
> pcibios_init_hw() which always fills sys->private_data with
> hw_pci->private_data. As we are always filling hw_pci->private_data with
> a pointer to our pcie_port structure in dw_pcie_host_init() the only way
> for pp to be NULL at this point would be for pp to be NULL in
> dw_pcie_host_init(). This would obviously crash a lot earlier.
> 
> So this check is just adding to LOC without adding any value.

Hi Lucas,

I see what you mean. However, pcie-designware.c has been handled by
various engineers. So, it is not sure that the current sequence will
not change in the future. Anyway, thank you for your comment. :-)

Best regards,
Jingoo Han

> 
> Regards,
> Lucas
> 
> --
> Pengutronix e.K.             | Lucas Stach                 |
> Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [PATCH 2/4] PCI: designware: get rid of pci_scan_root_bus

[Lucas Stach]

Am Donnerstag, den 24.07.2014, 17:55 +0900 schrieb Jingoo Han:
> On Thursday, July 24, 2014 5:32 PM, Lucas Stach wrote:
> > Am Donnerstag, den 24.07.2014, 13:11 +0900 schrieb Jingoo Han:
> > > On Thursday, July 24, 2014 2:53 AM, Lucas Stach wrote:
> > > >
> > > > Use pci_create_root_bus() similar to other pci
> > > > host controller drivers.
> > > >
> > > > The main problem with pci_scan_root_bus() is that
> > > > not only it will create the root bus, but also try
> > > > to activate all devices on the bus. This triggers
> > > > PCI device drivers probe, which try to use the device
> > > > but fail as resource allocation isn't done at that
> > > > point in time.
> > > >
> > > > To work around this we made sure that the host
> > > > controller driver is probed early and finishes resource
> > > > allocation before any other device drivers are registered.
> > > > Switching to pci_create_root_bus() allows us to get rid
> > > > of this special handling.
> > > >
> > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > > ---
> > > >  drivers/pci/host/pcie-designware.c | 15 +++++++--------
> > > >  1 file changed, 7 insertions(+), 8 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > > > index b13a830c8b0f..b869e202367c 100644
> > > > --- a/drivers/pci/host/pcie-designware.c
> > > > +++ b/drivers/pci/host/pcie-designware.c
> > > > @@ -801,14 +801,13 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
> > > >  	struct pci_bus *bus;
> > > >  	struct pcie_port *pp = sys_to_pcie(sys);
> > > >
> > > > -	if (pp) {
> > > > -		pp->root_bus_nr = sys->busnr;
> > > > -		bus = pci_scan_root_bus(pp->dev, sys->busnr, &dw_pcie_ops,
> > > > -					sys, &sys->resources);
> > > > -	} else {
> > > > -		bus = NULL;
> > > > -		BUG();
> > > > -	}
> > > > +	pp->root_bus_nr = sys->busnr;
> > > > +	bus = pci_create_root_bus(pp->dev, sys->busnr,
> > > > +				  &dw_pcie_ops, sys, &sys->resources);
> > > > +	if (!bus)
> > > > +		return NULL;
> > > > +
> > > > +	pci_scan_child_bus(bus);
> > >
> > >
> > > I think that we still need to check if pp is NULL.
> > > Then, how about adding NULL check as below?
> > >
> > No I don't think it's needed as it doesn't provide any additional
> > security.
> > 
> > The only code path that is calling this dw_pci_scan_bus() is in
> > pcibios_init_hw() which always fills sys->private_data with
> > hw_pci->private_data. As we are always filling hw_pci->private_data with
> > a pointer to our pcie_port structure in dw_pcie_host_init() the only way
> > for pp to be NULL at this point would be for pp to be NULL in
> > dw_pcie_host_init(). This would obviously crash a lot earlier.
> > 
> > So this check is just adding to LOC without adding any value.
> 
> Hi Lucas,
> 
> I see what you mean. However, pcie-designware.c has been handled by
> various engineers. So, it is not sure that the current sequence will
> not change in the future. Anyway, thank you for your comment. :-)
> 
I just looked this up and we we have a lot (and in some cases
inconsistent checks) for this sprinkled through the driver. I would
prefer to keep this patch as is and instead add this check to
sys_to_pcie(). This way we could remove it from the various call sites
while keeping the safety it provides. If you agree I can send a patch to
do this.

Regards,
Lucas

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

Re: [PATCH 2/4] PCI: designware: get rid of pci_scan_root_bus

[Jingoo Han]

On Thursday, July 24, 2014 6:02 PM, Jingoo Han wrote:
> Am Donnerstag, den 24.07.2014, 17:55 +0900 schrieb Jingoo Han:
> > On Thursday, July 24, 2014 5:32 PM, Lucas Stach wrote:
> > > Am Donnerstag, den 24.07.2014, 13:11 +0900 schrieb Jingoo Han:
> > > > On Thursday, July 24, 2014 2:53 AM, Lucas Stach wrote:
> > > > >
> > > > > Use pci_create_root_bus() similar to other pci
> > > > > host controller drivers.
> > > > >
> > > > > The main problem with pci_scan_root_bus() is that
> > > > > not only it will create the root bus, but also try
> > > > > to activate all devices on the bus. This triggers
> > > > > PCI device drivers probe, which try to use the device
> > > > > but fail as resource allocation isn't done at that
> > > > > point in time.
> > > > >
> > > > > To work around this we made sure that the host
> > > > > controller driver is probed early and finishes resource
> > > > > allocation before any other device drivers are registered.
> > > > > Switching to pci_create_root_bus() allows us to get rid
> > > > > of this special handling.
> > > > >
> > > > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > > > ---
> > > > >  drivers/pci/host/pcie-designware.c | 15 +++++++--------
> > > > >  1 file changed, 7 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> > > > > index b13a830c8b0f..b869e202367c 100644
> > > > > --- a/drivers/pci/host/pcie-designware.c
> > > > > +++ b/drivers/pci/host/pcie-designware.c
> > > > > @@ -801,14 +801,13 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
> > > > >  	struct pci_bus *bus;
> > > > >  	struct pcie_port *pp = sys_to_pcie(sys);
> > > > >
> > > > > -	if (pp) {
> > > > > -		pp->root_bus_nr = sys->busnr;
> > > > > -		bus = pci_scan_root_bus(pp->dev, sys->busnr, &dw_pcie_ops,
> > > > > -					sys, &sys->resources);
> > > > > -	} else {
> > > > > -		bus = NULL;
> > > > > -		BUG();
> > > > > -	}
> > > > > +	pp->root_bus_nr = sys->busnr;
> > > > > +	bus = pci_create_root_bus(pp->dev, sys->busnr,
> > > > > +				  &dw_pcie_ops, sys, &sys->resources);
> > > > > +	if (!bus)
> > > > > +		return NULL;
> > > > > +
> > > > > +	pci_scan_child_bus(bus);
> > > >
> > > >
> > > > I think that we still need to check if pp is NULL.
> > > > Then, how about adding NULL check as below?
> > > >
> > > No I don't think it's needed as it doesn't provide any additional
> > > security.
> > >
> > > The only code path that is calling this dw_pci_scan_bus() is in
> > > pcibios_init_hw() which always fills sys->private_data with
> > > hw_pci->private_data. As we are always filling hw_pci->private_data with
> > > a pointer to our pcie_port structure in dw_pcie_host_init() the only way
> > > for pp to be NULL at this point would be for pp to be NULL in
> > > dw_pcie_host_init(). This would obviously crash a lot earlier.
> > >
> > > So this check is just adding to LOC without adding any value.
> >
> > Hi Lucas,
> >
> > I see what you mean. However, pcie-designware.c has been handled by
> > various engineers. So, it is not sure that the current sequence will
> > not change in the future. Anyway, thank you for your comment. :-)
> >
> I just looked this up and we we have a lot (and in some cases
> inconsistent checks) for this sprinkled through the driver. I would
> prefer to keep this patch as is and instead add this check to
> sys_to_pcie(). This way we could remove it from the various call sites
> while keeping the safety it provides. If you agree I can send a patch to
> do this.

Hi Lucas Stach,

OK, I agree with your suggestion. However, I want to get other opinions
from other CC'ed people. Thank you.

Best regards,
Jingoo Han

Re: [PATCH 1/4] PCI: designware: start parsing bus-range

[Pratyush Anand]

On Wed, Jul 23, 2014 at 11:22 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> This allows to explicitly specify the covered bus
> numbers in the devicetree, which will come in handy
> once we see a SoC with more than one PCIe host
> controller instance.
>
> Previously the driver relied on the behavior of
> pci_scan_root_bus() to fill in a range of 0x00-0xff
> if no valid range was found. We fall back to the
> same range if no valid DT entry was found to keep
> backwards compatibility, but now do it explicitly.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt |  3 +++
>  drivers/pci/host/pcie-designware.c                        | 13 ++++++++++++-
>  drivers/pci/host/pcie-designware.h                        |  1 +
>  3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index ed0d9b9fff2b..9f4faa8e8d00 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -23,3 +23,6 @@ Required properties:
>
>  Optional properties:
>  - reset-gpio: gpio pin number of power good signal
> +- bus-range: PCI bus numbers covered (it is recommended for new devicetrees to
> +  specify this property, to keep backwards compatibility a range of 0x00-0xff
> +  is assumed if not present)
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 52bd3a143563..b13a830c8b0f 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -425,7 +425,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>         struct resource *cfg_res;
>         u32 val, na, ns;
>         const __be32 *addrp;
> -       int i, index;
> +       int i, index, ret;
>
>         /* Find the address cell size and the number of cells in order to get
>          * the untranslated address.
> @@ -500,6 +500,16 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>                 }
>         }
>
> +       ret = of_pci_parse_bus_range(np, &pp->busn);
> +       if (ret < 0) {
> +               dev_dbg(pp->dev, "failed to parse bus-range property: %d, using default [0x00-0xff]\n",
> +                       ret);
> +               pp->busn.name = np->name;
> +               pp->busn.start = 0;
> +               pp->busn.end = 0xff;
> +               pp->busn.flags = IORESOURCE_BUS;
> +       }
> +
>         if (!pp->dbi_base) {
>                 pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>                                         resource_size(&pp->cfg));
> @@ -781,6 +791,7 @@ static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
>
>         sys->mem_offset = pp->mem.start - pp->config.mem_bus_addr;
>         pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
> +       pci_add_resource(&sys->resources, &pp->busn);
>
>         return 1;
>  }
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index daf81f922cda..9a09633ecdf8 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -48,6 +48,7 @@ struct pcie_port {
>         struct resource         cfg;
>         struct resource         io;
>         struct resource         mem;
> +       struct resource         busn;
>         struct pcie_port_info   config;
>         int                     irq;
>         u32                     lanes;
> --
> 2.0.1
>

Looks fine to me.
Reviewed-by: Pratyush Anand <pratyush.anand@st.com>

> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/4] PCI: designware: get rid of pci_scan_root_bus

[Pratyush Anand]

On Wed, Jul 23, 2014 at 11:22 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> Use pci_create_root_bus() similar to other pci
> host controller drivers.
>
> The main problem with pci_scan_root_bus() is that
> not only it will create the root bus, but also try
> to activate all devices on the bus. This triggers
> PCI device drivers probe, which try to use the device
> but fail as resource allocation isn't done at that
> point in time.
>
> To work around this we made sure that the host
> controller driver is probed early and finishes resource
> allocation before any other device drivers are registered.
> Switching to pci_create_root_bus() allows us to get rid
> of this special handling.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/pci/host/pcie-designware.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index b13a830c8b0f..b869e202367c 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -801,14 +801,13 @@ static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
>         struct pci_bus *bus;
>         struct pcie_port *pp = sys_to_pcie(sys);
>
> -       if (pp) {
> -               pp->root_bus_nr = sys->busnr;
> -               bus = pci_scan_root_bus(pp->dev, sys->busnr, &dw_pcie_ops,
> -                                       sys, &sys->resources);
> -       } else {
> -               bus = NULL;
> -               BUG();
> -       }
> +       pp->root_bus_nr = sys->busnr;
> +       bus = pci_create_root_bus(pp->dev, sys->busnr,
> +                                 &dw_pcie_ops, sys, &sys->resources);
> +       if (!bus)
> +               return NULL;
> +
> +       pci_scan_child_bus(bus);
>
>         return bus;
>  }
> --

Looks fine to me.
Reviewed-by: Pratyush Anand <pratyush.anand@st.com>

> 2.0.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 3/4] PCI: designware: remove pci_assign_unassigned_resources call

[Pratyush Anand]

On Wed, Jul 23, 2014 at 11:22 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
> The pci_common_init_dev() call right before will already
> handle the device resource allocation, so this call
> was a no-op.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/pci/host/pcie-designware.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index b869e202367c..dde5e6d4afa2 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -568,7 +568,6 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>         dw_pci.private_data = (void **)&pp;
>
>         pci_common_init_dev(pp->dev, &dw_pci);
> -       pci_assign_unassigned_resources();

I am not sure here. IIRC, then calling  pci_assign_unassigned_resources after
pci_common_init_dev was needed to handle few PCIe cards with EP behind
bridge.

I do not have good understanding of pci resource allocation code.
@Yinghai: Can you please help(rather teach) with the description of
different resource allocator
available in setup-bus.c. Can try reading code, but if a documentation
exists, that would
be helpful.

 ~Pratyush

Re: [PATCH 3/4] PCI: designware: remove pci_assign_unassigned_resources call

[Yinghai Lu]

On Sat, Aug 16, 2014 at 6:13 AM, Pratyush Anand
<pratyush.anand@gmail.com> wrote:
> On Wed, Jul 23, 2014 at 11:22 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
>
> I am not sure here. IIRC, then calling  pci_assign_unassigned_resources after
> pci_common_init_dev was needed to handle few PCIe cards with EP behind
> bridge.
>
> I do not have good understanding of pci resource allocation code.
> @Yinghai: Can you please help(rather teach) with the description of
> different resource allocator
> available in setup-bus.c. Can try reading code, but if a documentation
> exists, that would
> be helpful.

pci_assign_unassigned_resources() should try several times to make sure
assign resource to PCI bars that are not assigned by BIOS or not valid value
from BIOS.
so it will honor the setting from BIOS.

in your arm case, pci_common_init_dev() is doing sizing and assign.
so you should not need that pci_assign_unassigned_resources() anymore.

Or your setup have PCI_PROBE_ONLY ?

Yinghai

Re: [PATCH 3/4] PCI: designware: remove pci_assign_unassigned_resources call

[Pratyush Anand]

On Sun, Aug 17, 2014 at 8:29 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Sat, Aug 16, 2014 at 6:13 AM, Pratyush Anand
> <pratyush.anand@gmail.com> wrote:
>> On Wed, Jul 23, 2014 at 11:22 PM, Lucas Stach <l.stach@pengutronix.de> wrote:
>>
>> I am not sure here. IIRC, then calling  pci_assign_unassigned_resources after
>> pci_common_init_dev was needed to handle few PCIe cards with EP behind
>> bridge.
>>
>> I do not have good understanding of pci resource allocation code.
>> @Yinghai: Can you please help(rather teach) with the description of
>> different resource allocator
>> available in setup-bus.c. Can try reading code, but if a documentation
>> exists, that would
>> be helpful.
>
> pci_assign_unassigned_resources() should try several times to make sure
> assign resource to PCI bars that are not assigned by BIOS or not valid value
> from BIOS.
> so it will honor the setting from BIOS.
>
> in your arm case, pci_common_init_dev() is doing sizing and assign.
> so you should not need that pci_assign_unassigned_resources() anymore.
>
> Or your setup have PCI_PROBE_ONLY ?

Thanks for the clarification.

I think none of the designware based platform boots with *firmware*, so none of
the setup should have PCI_PROBE_ONLY. Then patch seems fine.

~Pratyush

>
> Yinghai

Re: [PATCH 3/4] PCI: designware: remove pci_assign_unassigned_resources call

[Murali Karicheri]

On 08/17/2014 02:20 AM, Pratyush Anand wrote:
> On Sun, Aug 17, 2014 at 8:29 AM, Yinghai Lu<yinghai@kernel.org>  wrote:
>> On Sat, Aug 16, 2014 at 6:13 AM, Pratyush Anand
>> <pratyush.anand@gmail.com>  wrote:
>>> On Wed, Jul 23, 2014 at 11:22 PM, Lucas Stach<l.stach@pengutronix.de>  wrote:
>>>
>>> I am not sure here. IIRC, then calling  pci_assign_unassigned_resources after
>>> pci_common_init_dev was needed to handle few PCIe cards with EP behind
>>> bridge.
>>>
>>> I do not have good understanding of pci resource allocation code.
>>> @Yinghai: Can you please help(rather teach) with the description of
>>> different resource allocator
>>> available in setup-bus.c. Can try reading code, but if a documentation
>>> exists, that would
>>> be helpful.
>>
>> pci_assign_unassigned_resources() should try several times to make sure
>> assign resource to PCI bars that are not assigned by BIOS or not valid value
>> from BIOS.
>> so it will honor the setting from BIOS.
>>
>> in your arm case, pci_common_init_dev() is doing sizing and assign.
>> so you should not need that pci_assign_unassigned_resources() anymore.
>>
>> Or your setup have PCI_PROBE_ONLY ?
>
> Thanks for the clarification.
>
> I think none of the designware based platform boots with *firmware*, so none of
> the setup should have PCI_PROBE_ONLY. Then patch seems fine.
>
There are customers using Keystone platform on which u-boot sets up BAR 
and use firmware option to honor BAR assignment done by the boot loader. 
How does this change that use case? If this still works with this 
change, no issues. If not, I would suggest this shouldn't be removed. 
Any expert here who can comment on this? Keystone driver is just getting 
merged to upstream branch and is based on designware.

Murali
> ~Pratyush
>
>>
>> Yinghai
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/4] PCI: designware: init order/resource alloc fixes

[Bjorn Helgaas]

On Wed, Jul 23, 2014 at 07:52:37PM +0200, Lucas Stach wrote:
> This series fixes a couple of issues with the initialisation
> ordering and ressource allocation handling in the designware
> driver and moves its behavior much closer to other PCI host
> controller drivers like MVEBU and Tegra.
> For more detailed explanation see the commit messages.
> 
> This effectively enables the i.MX6 pcie driver to be
> probed from module initcall level, instead of relying on
> being probed early.
> 
> Lucas Stach (4):
>   PCI: designware: start parsing bus-range
>   PCI: designware: get rid of pci_scan_root_bus
>   PCI: designware: remove pci_assign_unassigned_resources call
>   PCI: imx6: move to module_init
> 
>  .../devicetree/bindings/pci/designware-pcie.txt    |  3 +++
>  drivers/pci/host/pci-imx6.c                        |  2 +-
>  drivers/pci/host/pcie-designware.c                 | 29 ++++++++++++++--------
>  drivers/pci/host/pcie-designware.h                 |  1 +
>  4 files changed, 24 insertions(+), 11 deletions(-)

I'm waiting for an ack from Mohit and/or Jingoo before applying these.
I think Richard already acked the one that touches IMX6.

Bjorn

RE: [PATCH 0/4] PCI: designware: init order/resource alloc fixes

[Mohit KUMAR DCG]

Hello Lucas,

> -----Original Message-----
> From: Bjorn Helgaas [mailto:bhelgaas@google.com]
> Sent: Wednesday, September 03, 2014 4:40 AM
> To: Lucas Stach
> Cc: linux-pci@vger.kernel.org; Richard Zhu; Mohit KUMAR DCG; Jingoo Han;
> Marek Vasut; Kishon Vijay Abraham I; kernel@pengutronix.de
> Subject: Re: [PATCH 0/4] PCI: designware: init order/resource alloc fixes
> 
> On Wed, Jul 23, 2014 at 07:52:37PM +0200, Lucas Stach wrote:
> > This series fixes a couple of issues with the initialisation ordering
> > and ressource allocation handling in the designware driver and moves
> > its behavior much closer to other PCI host controller drivers like
> > MVEBU and Tegra.
> > For more detailed explanation see the commit messages.
> >
> > This effectively enables the i.MX6 pcie driver to be probed from
> > module initcall level, instead of relying on being probed early.
> >
> > Lucas Stach (4):
> >   PCI: designware: start parsing bus-range
> >   PCI: designware: get rid of pci_scan_root_bus
> >   PCI: designware: remove pci_assign_unassigned_resources call
> >   PCI: imx6: move to module_init
> >
> >  .../devicetree/bindings/pci/designware-pcie.txt    |  3 +++
> >  drivers/pci/host/pci-imx6.c                        |  2 +-
> >  drivers/pci/host/pcie-designware.c                 | 29 ++++++++++++++--------
> >  drivers/pci/host/pcie-designware.h                 |  1 +
> >  4 files changed, 24 insertions(+), 11 deletions(-)
> 
> I'm waiting for an ack from Mohit and/or Jingoo before applying these.
> I think Richard already acked the one that touches IMX6.
> 
> Bjorn

- I have tested patch#1,2 and3 and looks fine to me.

Sorry for my delayed response due to some problem with my platform.

Acked-by: Mohit Kumar <mohit.kumar@st.com>

Regards
Mohit

Re: [PATCH 0/4] PCI: designware: init order/resource alloc fixes

[Bjorn Helgaas]

On Wed, Jul 23, 2014 at 07:52:37PM +0200, Lucas Stach wrote:
> This series fixes a couple of issues with the initialisation
> ordering and ressource allocation handling in the designware
> driver and moves its behavior much closer to other PCI host
> controller drivers like MVEBU and Tegra.
> For more detailed explanation see the commit messages.
> 
> This effectively enables the i.MX6 pcie driver to be
> probed from module initcall level, instead of relying on
> being probed early.
> 
> Lucas Stach (4):
>   PCI: designware: start parsing bus-range
>   PCI: designware: get rid of pci_scan_root_bus
>   PCI: designware: remove pci_assign_unassigned_resources call
>   PCI: imx6: move to module_init
> 
>  .../devicetree/bindings/pci/designware-pcie.txt    |  3 +++
>  drivers/pci/host/pci-imx6.c                        |  2 +-
>  drivers/pci/host/pcie-designware.c                 | 29 ++++++++++++++--------
>  drivers/pci/host/pcie-designware.h                 |  1 +
>  4 files changed, 24 insertions(+), 11 deletions(-)

Applied with Mohit's acks (and Richard's for [4/4]) to pci/host-designware
for v3.18, thanks!

Re: [PATCH 1/4] PCI: designware: start parsing bus-range

[Bjorn Helgaas]

On Wed, Jul 23, 2014 at 07:52:38PM +0200, Lucas Stach wrote:
> This allows to explicitly specify the covered bus
> numbers in the devicetree, which will come in handy
> once we see a SoC with more than one PCIe host
> controller instance.
> 
> Previously the driver relied on the behavior of
> pci_scan_root_bus() to fill in a range of 0x00-0xff
> if no valid range was found. We fall back to the
> same range if no valid DT entry was found to keep
> backwards compatibility, but now do it explicitly.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  Documentation/devicetree/bindings/pci/designware-pcie.txt |  3 +++
>  drivers/pci/host/pcie-designware.c                        | 13 ++++++++++++-
>  drivers/pci/host/pcie-designware.h                        |  1 +
>  3 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/pci/designware-pcie.txt b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> index ed0d9b9fff2b..9f4faa8e8d00 100644
> --- a/Documentation/devicetree/bindings/pci/designware-pcie.txt
> +++ b/Documentation/devicetree/bindings/pci/designware-pcie.txt
> @@ -23,3 +23,6 @@ Required properties:
>  
>  Optional properties:
>  - reset-gpio: gpio pin number of power good signal
> +- bus-range: PCI bus numbers covered (it is recommended for new devicetrees to
> +  specify this property, to keep backwards compatibility a range of 0x00-0xff
> +  is assumed if not present)
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 52bd3a143563..b13a830c8b0f 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -425,7 +425,7 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>  	struct resource *cfg_res;
>  	u32 val, na, ns;
>  	const __be32 *addrp;
> -	int i, index;
> +	int i, index, ret;
>  
>  	/* Find the address cell size and the number of cells in order to get
>  	 * the untranslated address.
> @@ -500,6 +500,16 @@ int __init dw_pcie_host_init(struct pcie_port *pp)
>  		}
>  	}
>  
> +	ret = of_pci_parse_bus_range(np, &pp->busn);
> +	if (ret < 0) {
> +		dev_dbg(pp->dev, "failed to parse bus-range property: %d, using default [0x00-0xff]\n",
> +			ret);
> +		pp->busn.name = np->name;
> +		pp->busn.start = 0;
> +		pp->busn.end = 0xff;
> +		pp->busn.flags = IORESOURCE_BUS;
> +	}
> +

I tweaked this to look like:

+       if (ret < 0) {
+               pp->busn.name = np->name;
+               pp->busn.start = 0;
+               pp->busn.end = 0xff;
+               pp->busn.flags = IORESOURCE_BUS;
+               dev_dbg(pp->dev, "failed to parse bus-range property: %d, using default %pR\n",
+                       ret, &pp->busn);

to avoid the repetition of 0x00 and 0xff.

Pratyush, I added your Reviewed-by to patches 1 and 2, which I missed before.

Bjorn