Hi Russell,
These are two patches I've been carrying in a larger series that
converts the Tegra PCIe controller driver to a proper platform driver.
Since the complete series didn't get much feedback, I've begun to post
smaller subsets in an effort to get them merged more easily.
The first patch in this series converts the __init annotations for
pci_common_init() (and pcibios_init_hw() because it is called from the
former) to __devinit to make sure that they stay around after the init
stage. This is required because the Tegra driver depends on regulators
that become available only very late during boot and uses deferred
probing to handle this situation. It turned out that this postpones the
PCI bus initialization until after init, thus this patch.
The second patch is used to pass per-controller or per-host-bridge data
to the driver, such that it can be associated with the corresponding
bus. This is also required by the Tegra driver in order to pass a
driver-private structure to the PCI bus (or more precisely the
pci_sys_data structure associated with a bus). It is subsequently used
to obtain the root port private data given the corresponding PCI bus.
Thierry
Thierry Reding (2):
ARM: pci: Keep pci_common_init() around after init
ARM: pci: Allow passing per-controller private data
arch/arm/include/asm/mach/pci.h | 1 +
arch/arm/kernel/bios32.c | 7 +++++--
2 files changed, 6 insertions(+), 2 deletions(-)
--
1.7.12
When using deferred driver probing, PCI host controller drivers may
actually require this function after the init stage.
Signed-off-by: Thierry Reding <redacted>
---
arch/arm/kernel/bios32.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
In order to allow drivers to specify private data for each controller,
this commit adds a private_data field to the struct hw_pci. This field
is an array of nr_controllers pointers that will be used to initialize
the private_data field of the corresponding controller's pci_sys_data
structure.
Signed-off-by: Thierry Reding <redacted>
---
arch/arm/include/asm/mach/pci.h | 1 +
arch/arm/kernel/bios32.c | 3 +++
2 files changed, 4 insertions(+)
On Fri, Sep 14, 2012 at 3:11 PM, Thierry Reding
[off-list ref] wrote:
quoted hunk
When using deferred driver probing, PCI host controller drivers may
actually require this function after the init stage.
Signed-off-by: Thierry Reding <redacted>
---
arch/arm/kernel/bios32.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
On Fri, Sep 14, 2012 at 3:11 PM, Thierry Reding
[off-list ref] wrote:
In order to allow drivers to specify private data for each controller,
this commit adds a private_data field to the struct hw_pci. This field
is an array of nr_controllers pointers that will be used to initialize
the private_data field of the corresponding controller's pci_sys_data
structure.
I guess you aren't changing the design here because struct hw_pci
already includes "nr_controllers," but having nr_controllers and a
private_data[] array sounds like something that might make it hard to
hot-add a host bridge after boot.
On Tue, Sep 18, 2012 at 11:13:23AM -0600, Bjorn Helgaas wrote:
On Fri, Sep 14, 2012 at 3:11 PM, Thierry Reding
[off-list ref] wrote:
quoted
When using deferred driver probing, PCI host controller drivers may
actually require this function after the init stage.
Signed-off-by: Thierry Reding <redacted>
---
arch/arm/kernel/bios32.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Shouldn't we just remove __init completely, since Greg KH is in the
process of removing __devinit?
Yes, we probably should. In fact I have a patch on top of this marked
work-in-progress that I forgot to squash into this. That patch includes
the removal of __devinit for pcibios_update_irq() that should be covered
by a different patch I sent a few days ago as well as pcibios_swizzle()
and pcibios_init_resources(). I'll squash it into this patch and will
resend.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120918/1d937aaa/attachment.sig>
On Tue, Sep 18, 2012 at 11:21:21AM -0600, Bjorn Helgaas wrote:
On Fri, Sep 14, 2012 at 3:11 PM, Thierry Reding
[off-list ref] wrote:
quoted
In order to allow drivers to specify private data for each controller,
this commit adds a private_data field to the struct hw_pci. This field
is an array of nr_controllers pointers that will be used to initialize
the private_data field of the corresponding controller's pci_sys_data
structure.
I guess you aren't changing the design here because struct hw_pci
already includes "nr_controllers," but having nr_controllers and a
private_data[] array sounds like something that might make it hard to
hot-add a host bridge after boot.
What I do in the Tegra PCIe driver is to pass each of the root ports to
pci_common_init() individually because they can be enabled or disabled
by device tree data. I suppose to some degree you can consider that hot-
adding. Not that Tegra is likely to ever need to support that. I don't
know how likely it is for any ARM platform to ever need support for hot-
adding a host bridge.
Eventually I think it would be advantageous for this to be generalized
further such that PCI initialization can be shared across architectures.
That's probably not an easy task so I was going to start by making
incremental changes that enable the Tegra code to work and, if time
allows, help further with subsequent improvements.
It also seems that parts of the PCI core aren't ready yet for hot-adding
host bridges. One thing I came across while working on the Tegra code is
that MSI setup and teardown needs to be done by the arch_setup_msi_irq()
and arch_teardown_msi_irq() respectively, which are expected to be
builtin. That was also the last issue that keeps the Tegra PCIe driver
from being built as a module. I think that will also make it impossible
to hot-add host bridges. On x86 this seems to be handled by platform
code, but on Tegra for example MSI setup and teardown is tightly coupled
to the PCIe controller. That was one of the things I thought I could
take a look at eventually, but getting Tegra support cleaned up is
higher priority right now.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120918/e8ee9f3a/attachment-0001.sig>
On Tue, Sep 18, 2012 at 12:34 PM, Thierry Reding
[off-list ref] wrote:
On Tue, Sep 18, 2012 at 11:21:21AM -0600, Bjorn Helgaas wrote:
quoted
On Fri, Sep 14, 2012 at 3:11 PM, Thierry Reding
[off-list ref] wrote:
quoted
In order to allow drivers to specify private data for each controller,
this commit adds a private_data field to the struct hw_pci. This field
is an array of nr_controllers pointers that will be used to initialize
the private_data field of the corresponding controller's pci_sys_data
structure.
I guess you aren't changing the design here because struct hw_pci
already includes "nr_controllers," but having nr_controllers and a
private_data[] array sounds like something that might make it hard to
hot-add a host bridge after boot.
What I do in the Tegra PCIe driver is to pass each of the root ports to
pci_common_init() individually because they can be enabled or disabled
by device tree data. I suppose to some degree you can consider that hot-
adding. Not that Tegra is likely to ever need to support that. I don't
know how likely it is for any ARM platform to ever need support for hot-
adding a host bridge.
Eventually I think it would be advantageous for this to be generalized
further such that PCI initialization can be shared across architectures.
That's probably not an easy task so I was going to start by making
incremental changes that enable the Tegra code to work and, if time
allows, help further with subsequent improvements.
It also seems that parts of the PCI core aren't ready yet for hot-adding
host bridges. One thing I came across while working on the Tegra code is
that MSI setup and teardown needs to be done by the arch_setup_msi_irq()
and arch_teardown_msi_irq() respectively, which are expected to be
builtin. That was also the last issue that keeps the Tegra PCIe driver
from being built as a module. I think that will also make it impossible
to hot-add host bridges. On x86 this seems to be handled by platform
code, but on Tegra for example MSI setup and teardown is tightly coupled
to the PCIe controller. That was one of the things I thought I could
take a look at eventually, but getting Tegra support cleaned up is
higher priority right now.
The PCI core doesn't support hot-add of host bridges yet, but there's
a lot of work in that area right now, which is why it's on my mind :)
I'm not suggesting you change anything here; just keep it in the back
of your mind for the future. Thanks for the arch_setup_msi_irq() tip;
I'll look into that.
Bjorn
On Tue, Sep 18, 2012 at 12:38:53PM -0600, Bjorn Helgaas wrote:
On Tue, Sep 18, 2012 at 12:34 PM, Thierry Reding
[off-list ref] wrote:
quoted
On Tue, Sep 18, 2012 at 11:21:21AM -0600, Bjorn Helgaas wrote:
quoted
On Fri, Sep 14, 2012 at 3:11 PM, Thierry Reding
[off-list ref] wrote:
quoted
In order to allow drivers to specify private data for each controller,
this commit adds a private_data field to the struct hw_pci. This field
is an array of nr_controllers pointers that will be used to initialize
the private_data field of the corresponding controller's pci_sys_data
structure.
I guess you aren't changing the design here because struct hw_pci
already includes "nr_controllers," but having nr_controllers and a
private_data[] array sounds like something that might make it hard to
hot-add a host bridge after boot.
What I do in the Tegra PCIe driver is to pass each of the root ports to
pci_common_init() individually because they can be enabled or disabled
by device tree data. I suppose to some degree you can consider that hot-
adding. Not that Tegra is likely to ever need to support that. I don't
know how likely it is for any ARM platform to ever need support for hot-
adding a host bridge.
Eventually I think it would be advantageous for this to be generalized
further such that PCI initialization can be shared across architectures.
That's probably not an easy task so I was going to start by making
incremental changes that enable the Tegra code to work and, if time
allows, help further with subsequent improvements.
It also seems that parts of the PCI core aren't ready yet for hot-adding
host bridges. One thing I came across while working on the Tegra code is
that MSI setup and teardown needs to be done by the arch_setup_msi_irq()
and arch_teardown_msi_irq() respectively, which are expected to be
builtin. That was also the last issue that keeps the Tegra PCIe driver
from being built as a module. I think that will also make it impossible
to hot-add host bridges. On x86 this seems to be handled by platform
code, but on Tegra for example MSI setup and teardown is tightly coupled
to the PCIe controller. That was one of the things I thought I could
take a look at eventually, but getting Tegra support cleaned up is
higher priority right now.
The PCI core doesn't support hot-add of host bridges yet, but there's
a lot of work in that area right now, which is why it's on my mind :)
I'm not suggesting you change anything here; just keep it in the back
of your mind for the future. Thanks for the arch_setup_msi_irq() tip;
I'll look into that.
I think it wouldn't even be that hard to implement. When I last looked
at this it seemed like a simple registration mechanism in the core
should work. That is the core would keep a list of registered host
bridges, each of which would implement setup_msi() and teardown_msi()
operations. Then, whenever an MSI is requested, the core would look up
the corresponding host bridge and call the associated setup_msi()
callback. Perhaps these could go into struct pci_ops. It already has
pointers for configuration space access, which is also controller-
specific.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120918/185827fa/attachment.sig>