| Message ID | 20250121-enable_ecam-v3-3-cd84d3b2a7ba@oss.qualcomm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | PCI: dwc: Add ECAM support with iATU configuration | expand |
On Tue, Jan 21, 2025 at 02:32:21PM +0530, Krishna Chaitanya Chundru wrote: > Allow DWC glue drivers to allocate the host bridge, avoiding redundant > device tree reads primarily in dw_pcie_ecam_supported(). > I don't understand what you mean by 'redundant device tree reads'. Please explain. - Mani > Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com> > --- > drivers/pci/controller/dwc/pcie-designware-host.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c > index 3888f9fe5af1..0acf9db44f2c 100644 > --- a/drivers/pci/controller/dwc/pcie-designware-host.c > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c > @@ -484,8 +484,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > struct device *dev = pci->dev; > struct device_node *np = dev->of_node; > struct platform_device *pdev = to_platform_device(dev); > + struct pci_host_bridge *bridge = pp->bridge; > struct resource_entry *win; > - struct pci_host_bridge *bridge; > struct resource *res; > int ret; > > @@ -497,11 +497,12 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) > return -ENODEV; > } > > - bridge = devm_pci_alloc_host_bridge(dev, 0); > - if (!bridge) > - return -ENOMEM; > - > - pp->bridge = bridge; > + if (!pp->bridge) { > + bridge = devm_pci_alloc_host_bridge(dev, 0); > + if (!bridge) > + return -ENOMEM; > + pp->bridge = bridge; > + } > > pp->cfg0_size = resource_size(res); > pp->cfg0_base = res->start; > > -- > 2.34.1 >
On 1/24/2025 11:48 AM, Manivannan Sadhasivam wrote: > On Tue, Jan 21, 2025 at 02:32:21PM +0530, Krishna Chaitanya Chundru wrote: >> Allow DWC glue drivers to allocate the host bridge, avoiding redundant >> device tree reads primarily in dw_pcie_ecam_supported(). >> > > I don't understand what you mean by 'redundant device tree reads'. Please > explain. > In dw_pcie_ecam_supported () we are trying to read bus-range to find maximum bus range value. devm_pci_alloc_host_bridge() is already reading bus range it. If we move devm_pci_alloc_host_bridge() to start of the controller probe we can avoid reading the dt and use values stored in the host bridge. This was recommended by bjorn in the v2. I will update the commit text in the next series. > - Mani > >> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com> >> --- >> drivers/pci/controller/dwc/pcie-designware-host.c | 13 +++++++------ >> 1 file changed, 7 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c >> index 3888f9fe5af1..0acf9db44f2c 100644 >> --- a/drivers/pci/controller/dwc/pcie-designware-host.c >> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c >> @@ -484,8 +484,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) >> struct device *dev = pci->dev; >> struct device_node *np = dev->of_node; >> struct platform_device *pdev = to_platform_device(dev); >> + struct pci_host_bridge *bridge = pp->bridge; >> struct resource_entry *win; >> - struct pci_host_bridge *bridge; >> struct resource *res; >> int ret; >> >> @@ -497,11 +497,12 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) >> return -ENODEV; >> } >> >> - bridge = devm_pci_alloc_host_bridge(dev, 0); >> - if (!bridge) >> - return -ENOMEM; >> - >> - pp->bridge = bridge; >> + if (!pp->bridge) { >> + bridge = devm_pci_alloc_host_bridge(dev, 0); >> + if (!bridge) >> + return -ENOMEM; >> + pp->bridge = bridge; >> + } >> >> pp->cfg0_size = resource_size(res); >> pp->cfg0_base = res->start; >> >> -- >> 2.34.1 >> >
On Fri, Jan 24, 2025 at 01:56:25PM +0530, Krishna Chaitanya Chundru wrote: > > > On 1/24/2025 11:48 AM, Manivannan Sadhasivam wrote: > > On Tue, Jan 21, 2025 at 02:32:21PM +0530, Krishna Chaitanya Chundru wrote: > > > Allow DWC glue drivers to allocate the host bridge, avoiding redundant > > > device tree reads primarily in dw_pcie_ecam_supported(). > > > > > > > I don't understand what you mean by 'redundant device tree reads'. Please > > explain. > > > In dw_pcie_ecam_supported () we are trying to read bus-range to find > maximum bus range value. devm_pci_alloc_host_bridge() is already reading > bus range it. If we move devm_pci_alloc_host_bridge() to start of the > controller probe we can avoid reading the dt and use values stored in the > host bridge. This is the exact information you should put into the description. Do not assume that the readers will have the background. > This was recommended by bjorn in the v2. > Then you should add 'Suggested-by' tag. - Mani
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c index 3888f9fe5af1..0acf9db44f2c 100644 --- a/drivers/pci/controller/dwc/pcie-designware-host.c +++ b/drivers/pci/controller/dwc/pcie-designware-host.c @@ -484,8 +484,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) struct device *dev = pci->dev; struct device_node *np = dev->of_node; struct platform_device *pdev = to_platform_device(dev); + struct pci_host_bridge *bridge = pp->bridge; struct resource_entry *win; - struct pci_host_bridge *bridge; struct resource *res; int ret; @@ -497,11 +497,12 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp) return -ENODEV; } - bridge = devm_pci_alloc_host_bridge(dev, 0); - if (!bridge) - return -ENOMEM; - - pp->bridge = bridge; + if (!pp->bridge) { + bridge = devm_pci_alloc_host_bridge(dev, 0); + if (!bridge) + return -ENOMEM; + pp->bridge = bridge; + } pp->cfg0_size = resource_size(res); pp->cfg0_base = res->start;
Allow DWC glue drivers to allocate the host bridge, avoiding redundant device tree reads primarily in dw_pcie_ecam_supported(). Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com> --- drivers/pci/controller/dwc/pcie-designware-host.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)