Date:	Fri, 26 Sep 2014 10:54:32 +0200
From:	Thierry Reding <thierry.reding@...il.com>
To:	Yijing Wang <wangyijing@...wei.com>
Cc:	Liviu Dudau <liviu@...au.co.uk>,
	Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, Xinwei Hu <huxinwei@...wei.com>,
	Wuyun <wuyun.wu@...wei.com>,
	linux-arm-kernel@...ts.infradead.org,
	Russell King <linux@....linux.org.uk>,
	linux-arch@...r.kernel.org, arnab.basu@...escale.com,
	Bharat.Bhushan@...escale.com, x86@...nel.org,
	Arnd Bergmann <arnd@...db.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	xen-devel@...ts.xenproject.org, Joerg Roedel <joro@...tes.org>,
	iommu@...ts.linux-foundation.org, linux-mips@...ux-mips.org,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	linuxppc-dev@...ts.ozlabs.org, linux-s390@...r.kernel.org,
	Sebastian Ott <sebott@...ux.vnet.ibm.com>,
	Tony Luck <tony.luck@...el.com>, linux-ia64@...r.kernel.org,
	"David S. Miller" <davem@...emloft.net>,
	sparclinux@...r.kernel.org, Chris Metcalf <cmetcalf@...era.com>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Lucas Stach <l.stach@...gutronix.de>,
	David Vrabel <david.vrabel@...rix.com>,
	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>,
	Michael Ellerman <mpe@...erman.id.au>,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
Subject: Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X
 in all platforms

On Fri, Sep 26, 2014 at 02:20:35PM +0800, Yijing Wang wrote:
> >> The PCI core can already deal with that. An MSI chip can be set per bus
> >> and the weak pcibios_add_bus() can be used to set that. Often it might
> >> not even be necessary to do it via pcibios_add_bus() if you create the
> >> root bus directly, since you can attach the MSI chip at that time.
> > 
> > But I'm thinking that we need to move away from pcibios_add_bus() interface to do
> > something that should be generic. You don't need to be called for every bus when all
> > you want is just the root bus in order to add the MSI chip. Also, from looking at
> > the current patchset, a lot of architectures would set the MSI chip to a global
> > variable, which means you don't have an option to choose the MSI chip based on the
> > bus.
> 
> I also agree to remove the pcibios_add_bus() in arm which call .add_bus() to associate msi_chip
> and PCI bus.
> 
> In my opinions, all PCI devices under the same PCI hostbridge must share same msi chip, right ?
> So if we can associate msi chip and PCI hostbridge, every PCI device can find correct msi chip.
> PCI hostbridge private attributes can be saved in PCI sysdata, and this data will be propagate to
> PCI root bus and its child buses.

struct pci_sys_data is architecture specific, so the code won't become
any more generic than it is now.

> >>> What I would like to see is a way of creating the pci_host_bridge structure outside
> >>> the pci_create_root_bus(). That would then allow us to pass this sort of platform
> >>> details like associated msi_chip into the host bridge and the child busses will
> >>> have an easy way of finding the information needed by finding the root bus and then
> >>> the host bridge structure. Then the generic pci_scan_root_bus() can be used by (mostly)
> >>> everyone and the drivers can remove their kludges that try to work around the
> >>> current limitations.
> >>
> >> I think both issues are orthogonal. Last time I checked a lot of work
> >> was still necessary to unify host bridges enough so that it could be
> >> shared across architectures. But perhaps some of that work has
> >> happened in the meantime.
> > 
> > Breaking out the host bridge creation from root bus creation is not difficult, just not
> > agree upon. That would be the first step in making the generic host brige structure
> > useful for sharing, specially if used as a sort of "parent" structure that you can
> > wrap with your actual host bridge structure.
> 
> Breaking out the host bridge creation is a good idea, but there need a lot of changes, we can
> do it in another series.

I agree, this can be done step by step.

> And if we save msi chip in pci sysdata now, it will be easy to
> move it to generic pci host bridge. We can also move the pci domain number and other common info to it.

But like I said above, we wouldn't gain anything by moving the MSI chip
into struct pci_sys_data, since the core code couldn't access it from
there. The code wouldn't become more generic than the current approach
of using pcibios_add_bus(). At least for Tegra it's trivial to just hook
it up in tegra_pcie_scan_bus() directly (patch attached). So I think a
generic solution would be to allow it to be easily associated with a
bus.

Perhaps it would be as simple as adding a pci_scan_root_bus_with_msi()
or something with a less awkward name, or extending the existing
pci_scan_root_bus() with an MSI chip parameter. The function already has
too many arguments for my taste, though, so I'd like to avoid the
latter.

Thierry

Content of type "application/pgp-signature" skipped

Powered by blists - more mailing lists