| Message ID | 20250313-pci_fixup_addr-v11-4-01d2313502ab@nxp.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | PCI: Use device bus range info to cleanup RC Host/EP pci_fixup_addr() | expand |
On Thu, Mar 13, 2025 at 11:38:40AM -0400, Frank Li wrote: > Move devm_pci_alloc_host_bridge() to the beginning of dw_pcie_host_init(). > Since devm_pci_alloc_host_bridge() is common code that doesn't depend on > any DWC resource, moving it earlier improves code logic and readability. > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index c57831902686e..52a441662cabe 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -452,6 +452,12 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > raw_spin_lock_init(&pp->lock); > > + bridge = devm_pci_alloc_host_bridge(dev, 0); > + if (!bridge) > + return bridge; This returns NULL (0) where it previously returned -ENOMEM. Callers interpret zero as "success", so I think it should stil return -ENOMEM. I tentatively changed it back to -ENOMEM locally, let me know if that's wrong. > + pp->bridge = bridge; > + > ret = dw_pcie_get_resources(pci); > if (ret) > return ret; > @@ -460,12 +466,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > if (ret) > return ret; > > - bridge = devm_pci_alloc_host_bridge(dev, 0); > - if (!bridge) > - return -ENOMEM; > - > - pp->bridge = bridge; > - > /* Get the I/O range from DT */ > win = resource_list_first_type(&bridge->windows, IORESOURCE_IO); > if (win) { > > -- > 2.34.1 >
On Thu, Mar 13, 2025 at 02:22:54PM -0500, Bjorn Helgaas wrote: > On Thu, Mar 13, 2025 at 11:38:40AM -0400, Frank Li wrote: > > Move devm_pci_alloc_host_bridge() to the beginning of dw_pcie_host_init(). > > Since devm_pci_alloc_host_bridge() is common code that doesn't depend on > > any DWC resource, moving it earlier improves code logic and readability. > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > drivers/pci/controller/dwc/pcie-designware-host.c | 12 ++++++------ > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > > index c57831902686e..52a441662cabe 100644 > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > @@ -452,6 +452,12 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > > > raw_spin_lock_init(&pp->lock); > > > > + bridge = devm_pci_alloc_host_bridge(dev, 0); > > + if (!bridge) > > + return bridge; > > This returns NULL (0) where it previously returned -ENOMEM. Callers > interpret zero as "success", so I think it should stil return -ENOMEM. It should be -ENOMEM. Sorry for that. Strange, not sure what happen when I copy/past code. Do you need respin it or you can fix it? Frank > > I tentatively changed it back to -ENOMEM locally, let me know if > that's wrong. > > > + pp->bridge = bridge; > > + > > ret = dw_pcie_get_resources(pci); > > if (ret) > > return ret; > > @@ -460,12 +466,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > if (ret) > > return ret; > > > > - bridge = devm_pci_alloc_host_bridge(dev, 0); > > - if (!bridge) > > - return -ENOMEM; > > - > > - pp->bridge = bridge; > > - > > /* Get the I/O range from DT */ > > win = resource_list_first_type(&bridge->windows, IORESOURCE_IO); > > if (win) { > > > > -- > > 2.34.1 > >
On Thu, Mar 13, 2025 at 04:45:26PM -0400, Frank Li wrote: > On Thu, Mar 13, 2025 at 02:22:54PM -0500, Bjorn Helgaas wrote: > > On Thu, Mar 13, 2025 at 11:38:40AM -0400, Frank Li wrote: > > > Move devm_pci_alloc_host_bridge() to the beginning of dw_pcie_host_init(). > > > Since devm_pci_alloc_host_bridge() is common code that doesn't depend on > > > any DWC resource, moving it earlier improves code logic and readability. > > > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > > --- > > > drivers/pci/controller/dwc/pcie-designware-host.c | 12 ++++++------ > > > 1 file changed, 6 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > > > index c57831902686e..52a441662cabe 100644 > > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > > > @@ -452,6 +452,12 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > > > > > > raw_spin_lock_init(&pp->lock); > > > > > > + bridge = devm_pci_alloc_host_bridge(dev, 0); > > > + if (!bridge) > > > + return bridge; > > > > This returns NULL (0) where it previously returned -ENOMEM. Callers > > interpret zero as "success", so I think it should stil return -ENOMEM. > > It should be -ENOMEM. Sorry for that. Strange, not sure what happen when > I copy/past code. > > Do you need respin it or you can fix it? I fixed it locally. But you should fix it, too, in case we do another spin for other reasons. > > I tentatively changed it back to -ENOMEM locally, let me know if > > that's wrong. > > > > > + pp->bridge = bridge; > > > +
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index c57831902686e..52a441662cabe 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -452,6 +452,12 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) raw_spin_lock_init(&pp->lock); + bridge = devm_pci_alloc_host_bridge(dev, 0); + if (!bridge) + return bridge; + + pp->bridge = bridge; + ret = dw_pcie_get_resources(pci); if (ret) return ret; @@ -460,12 +466,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) if (ret) return ret; - bridge = devm_pci_alloc_host_bridge(dev, 0); - if (!bridge) - return -ENOMEM; - - pp->bridge = bridge; - /* Get the I/O range from DT */ win = resource_list_first_type(&bridge->windows, IORESOURCE_IO); if (win) {
Move devm_pci_alloc_host_bridge() to the beginning of dw_pcie_host_init(). Since devm_pci_alloc_host_bridge() is common code that doesn't depend on any DWC resource, moving it earlier improves code logic and readability. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- drivers/pci/controller/dwc/pcie-designware-host.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)