From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: MIME-Version: 1.0 In-Reply-To: <20170602114956.GA20004@red-moon> References: <20170426111809.19922-1-lorenzo.pieralisi@arm.com> <20170426111809.19922-9-lorenzo.pieralisi@arm.com> <20170602114956.GA20004@red-moon> From: Arnd Bergmann Date: Fri, 2 Jun 2017 15:12:48 +0200 Message-ID: Subject: Re: [RFC/RFT PATCH 08/18] PCI: designware: Convert PCI scan API to pci_scan_root_bus_bridge() To: Lorenzo Pieralisi List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Wenrui Li , Gabriele Paoloni , linux-pci , Shawn Lin , Will Deacon , Michal Simek , Thierry Reding , Tanmay Inamdar , Matthew Minter , Rob Herring , Joao Pinto , Pratyush Anand , Russell King , Murali Karicheri , Bharat Kumar Gogada , Simon Horman , Bjorn Helgaas , Mingkai Hu , Linux ARM , Thomas Petazzoni , Jingoo Han , Stanimir Varbanov , Minghuan Lian , Zhou Wang , Roy Zang Content-Type: text/plain; charset="us-ascii" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+bjorn=helgaas.com@lists.infradead.org List-ID: On Fri, Jun 2, 2017 at 1:49 PM, Lorenzo Pieralisi wrote: > On Fri, Apr 28, 2017 at 03:13:04PM +0200, Arnd Bergmann wrote: >> On Wed, Apr 26, 2017 at 1:17 PM, Lorenzo Pieralisi >> wrote: >> > The introduction of pci_scan_root_bus_bridge() provides a PCI core >> > API to scan a PCI root bus backed by an already initialized >> > struct pci_host_bridge object, which simplifies the bus scan >> > interface and makes the PCI scan root bus interface easier to >> > generalize as members are added to the struct pci_host_bridge(). >> > >> > Convert PCI designware host code to pci_scan_root_bus_bridge() to >> > improve the PCI root bus scanning interface. >> > >> > Signed-off-by: Lorenzo Pieralisi >> > Cc: Jingoo Han >> > Cc: Bjorn Helgaas >> > Cc: Joao Pinto >> > --- >> > drivers/pci/dwc/pcie-designware-host.c | 36 +++++++++++++++++++++------------- >> > 1 file changed, 22 insertions(+), 14 deletions(-) >> > >> > diff --git a/drivers/pci/dwc/pcie-designware-host.c b/drivers/pci/dwc/pcie-designware-host.c >> > index 5ba3349..e43c21a 100644 >> > --- a/drivers/pci/dwc/pcie-designware-host.c >> > +++ b/drivers/pci/dwc/pcie-designware-host.c >> > @@ -294,16 +294,21 @@ int dw_pcie_host_init(struct pcie_port *pp) >> > dev_err(dev, "missing *config* reg space\n"); >> > } >> > >> > - ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &res, &pp->io_base); >> > + bridge = pci_alloc_host_bridge(0); >> > + if (!bridge) >> > + return -ENOMEM; >> > + >> >> I think here we warn to have the pci_alloc_host_bridge() called in the >> individual >> drivers, to have them allocate the dw_pcie structure as part of the host >> bridge allocation, before calling hisi_add_pcie_port(). > > I am almost done refactoring this series and this is the last review to > address. What you suggest above makes sense but: > > 1) it means patching ALL dw_pcie_host_init() callers and allocate the > host bridge there, not sure it simplifies, certainly it complicates > the error handling path (given that the host bridge memory is > probably the only memory that is allocated with kzalloc instead of > devm interface so it needs unwinding) > > 2) I noticed that allocating the host bridge in the caller complicates > error handling, in particular code in the mainline (ie tegra and > ftpci100) using pci_alloc_host_bridge() needs already patching since it > may leak memory if PCI host bridge drivers probing fails. > > I can implement (1) and (2) but I wanted to ask first to understand if > that's the direction we want to take. Good question. Maybe we should add a devm_pci_alloc_host_bridge() instead? Would that solve these problems without creating bigger ones? Arnd _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel