From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755150AbcEDXgI (ORCPT ); Wed, 4 May 2016 19:36:08 -0400 Received: from mout.kundenserver.de ([212.227.17.24]:58454 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753952AbcEDXgG (ORCPT ); Wed, 4 May 2016 19:36:06 -0400 From: Arnd Bergmann To: Bjorn Helgaas Cc: catalin.marinas@arm.com, linux-pci@vger.kernel.org, will.deacon@arm.com, Lorenzo Pieralisi , Tomasz Nowicki , ddaney@caviumnetworks.com, robert.richter@caviumnetworks.com, msalter@redhat.com, Liviu.Dudau@arm.com, jchandra@broadcom.com, linux-kernel@vger.kernel.org, hanjun.guo@linaro.org, Suravee.Suthikulpanit@amd.com, Thierry Reding Subject: Re: [PATCH 2/3] [RFC] pci: host-common: use new pci_register_host interface Date: Thu, 05 May 2016 01:35:32 +0200 Message-ID: <6392987.nMH0fFXVaW@wuerfel> User-Agent: KMail/4.11.5 (Linux/3.16.0-10-generic; KDE/4.11.5; x86_64; ; ) In-Reply-To: <20160504231418.GE3117@localhost> References: <1461970899-4150603-1-git-send-email-arnd@arndb.de> <1461970899-4150603-3-git-send-email-arnd@arndb.de> <20160504231418.GE3117@localhost> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:vjeRPTaW849tr12eGU1H86Q6hCxRaFmgnBK9jS64vf2EYRYTfUB Dm8wOZ7TCHaPfJ4IN+mvwzV3P4uMRMauf/TF8GnXwGuT8re6vNcUDX2GdbFejiPtppJk5hI QvTsUov/jw53rF4iOCEoLFnA9LUFPL162PGsyi02KBiQeo6J5r2tprAbkZeoL9B8EbJFi89 Fd/gc5RVCjAWizafaMAJg== X-UI-Out-Filterresults: notjunk:1;V01:K0:MCeT4LY9vF4=:weHfUPEBU1XTH3o0W+mNF0 0+P+uIakAUpM0WVEfw3Ji7pBkJXsmvEJ8IuDHmkw2cypegV0pQx0OfK5HVfGsSF5ozBDdkJv/ CTqo++mMMgEVR835Ak6DlxTMLVsMpCOmbYmTpHMiJ/zrtYZz4cAkHpPc2lMeXh95BG4zKFX+H ZffFqp5RmuCX0QbkTvgG28MDkVBBrcZn6uvWvez0a/7aJ27hTd+7NRbcfE2x1PjawEUNPbZZz fgggpMEaBUEXYul+btmOTy4H97blY7I/tdIQ3fyFAB6jOw6xVxp7TVlKLV05dADigb65TEkIa J8CeK8jZPxBmDQos6v2bdBUKpSVB/nyCld1x3BgvR17n2tAGrK5aA3TyJ5Lh0KPZlYVrhKzJt RdJjfcAM9mhqK61hSQ81Mt5vA1z8QLjopwXVgrBiTimug1E7kKUZOKaQ+kOcHPEqNLIXGI+lM C0P6RECNDONvR9t5qWwPBIac7jF02gnOk3uPo8+LLu4rVx83X93s6NuJunTdAPD+k4H2JGq8M Wdi8GasFEl66mzNKMlgwPCrHugLSgzOtU9gYbGtcF7OUhRUJayYN0c6+fYo4TEWmYKEXYSxxm Z3gp2O3iwyvAg98c4Vi4NtlDNOXh/fKj6UiDHNZt7Zk8wIJDl/lwccp9URjjxY+bkdHg3xTEi Yy9tvN6nqeIC/pUOLXCvbt4Qo9qcNtCkfYf0xR5vCKvsOCmmEAb9kMvjLdQF/dySBGl3TIOVU VyBHdNfTLD4YsbgE Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday 04 May 2016 18:14:18 Bjorn Helgaas wrote: > On Sat, Apr 30, 2016 at 01:01:38AM +0200, Arnd Bergmann wrote: > > > > +static void gen_pci_release(struct device *dev) > > +{ > > + struct gen_pci *pci = container_of(dev, struct gen_pci, host.dev); > > + > > + gen_pci_release_of_pci_ranges(pci); > > + kfree(pci); > > +} > > I don't really like the fact that the alloc of struct gen_pci is so > far away from the free. I haven't looked hard enough to figure out if > it's reasonable to put them closer. It should be easy enough to move the release function next to the one that does the allocation. If we go the other route of having a generic pci_host_alloc() function that every host driver has to call instead of kzalloc(), we can probably drop the need to specify a release function in each driver. > > + err = pci_register_host(&pci->host); > > + if (!err) { > > + dev_err(dev, "registering host failed"); > > + return err; > > } > > Where do we actually scan the bus here? I don't see it in > pci_register_host(). Ah, you are right, that was a mistake. As I said, I have not tried running the code. I left this out of pci_register_host() for compatibility with pci_create_root_bus(), which also doesn't scan the bus, but then I didn't notice that I effectively remove the scan during the conversion of this driver. > > pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci); > > > > if (!pci_has_flag(PCI_PROBE_ONLY)) { > > - pci_bus_size_bridges(bus); > > - pci_bus_assign_resources(bus); > > + pci_bus_size_bridges(pci->host.bus); > > + pci_bus_assign_resources(pci->host.bus); > > > > - list_for_each_entry(child, &bus->children, node) > > + list_for_each_entry(child, &pci->host.bus->children, node) > > pcie_bus_configure_settings(child); > > } > > > > - pci_bus_add_devices(bus); > > + pci_bus_add_devices(pci->host.bus); > > return 0; I was actually thinking we could move both the scan and all the code above into pci_register_host(), based on some flags or other struct members we assign in the pci_host_bridge structure, with the most common combination being the default. I'm still unsure why we need to do the pci_fixup_irqs() instead of having the normal irq setting do the right thing, but if necessary, the host driver can set a flag to ask the core to do it, or we could add an optional function pointer to the of_irq_parse_and_map_pci function (or a host specific one if needed) to struct pci_ops and the call pci_common_swizzle with that. For all the other stuff (size_bridges, assign_resources, configure_settings, add_devices), I'd say a host driver should not really have to worry about this unless it needs to do something special inbetween. Of course we can't do it for the existing pci_scan_root_bus() etc, because the callers expect it not to be done. Arnd