| Message ID | 1427778057-9505-5-git-send-email-dja@axtens.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e02def5bce12b472e9eb6dcdd9f7af72239e6330 |
| Delegated to: | Michael Ellerman |
| Headers | show |
On Tuesday 31 March 2015 16:00:42 Daniel Axtens wrote: > Introduces the pci_controller_ops structure. > Add pci_controller_ops.dma_dev_setup, shadowing ppc_md.pci_dma_dev_setup. > Add a shim, and change the callsites to use the shim. > > Signed-off-by: Daniel Axtens <dja@axtens.net> > > --- > > v1 --> v2: > - Better commit message > - Use phb in favour of hose > - Make shim name match ppc_md name, not pci_controller_ops name. > --- > arch/powerpc/include/asm/pci-bridge.h | 21 +++++++++++++++++++++ > arch/powerpc/kernel/pci-common.c | 3 +-- > 2 files changed, 22 insertions(+), 2 deletions(-) > > diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h > index 546d036..347d49d 100644 > --- a/arch/powerpc/include/asm/pci-bridge.h > +++ b/arch/powerpc/include/asm/pci-bridge.h > @@ -15,6 +15,13 @@ > struct device_node; > > /* > + * PCI controller operations > + */ > +struct pci_controller_ops { > + void (*dma_dev_setup)(struct pci_dev *dev); > +}; > + > +/* Please see https://patchwork.ozlabs.org/patch/431333/ for related work. I think it would be better not to introduce another architecture-specific pci host bridge operations structure, but instead consolidate into the one that is already there. We are also adding a generic way to set up PCI DMA, so it would seems reasonable to hook into that place. Arnd
> Please see https://patchwork.ozlabs.org/patch/431333/ for related work. > I'm familiar with that patch series - I've been helping Yijing get it up to speed on PowerPC. > I think it would be better not to introduce another architecture-specific > pci host bridge operations structure, but instead consolidate into > the one that is already there. We are also adding a generic way to set up > PCI DMA, so it would seems reasonable to hook into that place. I see what you're getting at, and I agree that we want to move towards generic operations. However, I think this should go in as is at this point, for two main reasons: 1) This is a good midpoint that makes it easier to move to a generic structure. Our arch specific stuff is quirky and difficult. This patch series does a lot to reduce the complexity, and would make it very easy to move these ops into a generic structure at some future point. 2) Trying to go generic at this point risks making the change set so complex and wide ranging that it will really struggle to get in. For example, Yijing's patch set, despite not changing any of the quirky stuff in PowerPC, is already quite long, and will require agreement from a lot of people before it can go in. Much as I would like to have everything as generic as possible, if we were to try to do the whole job in one go, it'd become a big, difficult, messy patch set, and would be less likely to happen than if we were to do it in two steps. Regards, Daniel
On Tuesday 07 April 2015 10:31:36 Daniel Axtens wrote: > > Please see https://patchwork.ozlabs.org/patch/431333/ for related work. > > > I'm familiar with that patch series - I've been helping Yijing get it up > to speed on PowerPC. > > > > I think it would be better not to introduce another architecture-specific > > pci host bridge operations structure, but instead consolidate into > > the one that is already there. We are also adding a generic way to set up > > PCI DMA, so it would seems reasonable to hook into that place. > I see what you're getting at, and I agree that we want to move towards > generic operations. > > However, I think this should go in as is at this point, for two main > reasons: > > 1) This is a good midpoint that makes it easier to move to a generic > structure. Our arch specific stuff is quirky and difficult. This patch > series does a lot to reduce the complexity, and would make it very easy > to move these ops into a generic structure at some future point. > > 2) Trying to go generic at this point risks making the change set so > complex and wide ranging that it will really struggle to get in. For > example, Yijing's patch set, despite not changing any of the quirky > stuff in PowerPC, is already quite long, and will require agreement from > a lot of people before it can go in. > > Much as I would like to have everything as generic as possible, if we > were to try to do the whole job in one go, it'd become a big, difficult, > messy patch set, and would be less likely to happen than if we were to > do it in two steps. Ok, fair enough. Let's do this one first then. Arnd
On Tue, 2015-04-07 at 09:44 +0200, Arnd Bergmann wrote: > On Tuesday 07 April 2015 10:31:36 Daniel Axtens wrote: > > > Please see https://patchwork.ozlabs.org/patch/431333/ for related work. > > > > > I'm familiar with that patch series - I've been helping Yijing get it up > > to speed on PowerPC. > > > > > > > I think it would be better not to introduce another architecture-specific > > > pci host bridge operations structure, but instead consolidate into > > > the one that is already there. We are also adding a generic way to set up > > > PCI DMA, so it would seems reasonable to hook into that place. > > I see what you're getting at, and I agree that we want to move towards > > generic operations. > > > > However, I think this should go in as is at this point, for two main > > reasons: > > > > 1) This is a good midpoint that makes it easier to move to a generic > > structure. Our arch specific stuff is quirky and difficult. This patch > > series does a lot to reduce the complexity, and would make it very easy > > to move these ops into a generic structure at some future point. > > > > 2) Trying to go generic at this point risks making the change set so > > complex and wide ranging that it will really struggle to get in. For > > example, Yijing's patch set, despite not changing any of the quirky > > stuff in PowerPC, is already quite long, and will require agreement from > > a lot of people before it can go in. > > > > Much as I would like to have everything as generic as possible, if we > > were to try to do the whole job in one go, it'd become a big, difficult, > > messy patch set, and would be less likely to happen than if we were to > > do it in two steps. > > Ok, fair enough. Let's do this one first then. Thanks Arnd. cheers
diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h index 546d036..347d49d 100644 --- a/arch/powerpc/include/asm/pci-bridge.h +++ b/arch/powerpc/include/asm/pci-bridge.h @@ -15,6 +15,13 @@ struct device_node; /* + * PCI controller operations + */ +struct pci_controller_ops { + void (*dma_dev_setup)(struct pci_dev *dev); +}; + +/* * Structure of a PCI controller (host bridge) */ struct pci_controller { @@ -46,6 +53,7 @@ struct pci_controller { resource_size_t isa_mem_phys; resource_size_t isa_mem_size; + struct pci_controller_ops controller_ops; struct pci_ops *ops; unsigned int __iomem *cfg_addr; void __iomem *cfg_data; @@ -258,5 +266,18 @@ static inline int pcibios_vaddr_is_ioport(void __iomem *address) } #endif /* CONFIG_PCI */ +/* + * Shims to prefer pci_controller version over ppc_md where available. + */ +static inline void pci_dma_dev_setup(struct pci_dev *dev) +{ + struct pci_controller *phb = pci_bus_to_host(dev->bus); + + if (phb->controller_ops.dma_dev_setup) + phb->controller_ops.dma_dev_setup(dev); + else if (ppc_md.pci_dma_dev_setup) + ppc_md.pci_dma_dev_setup(dev); +} + #endif /* __KERNEL__ */ #endif /* _ASM_POWERPC_PCI_BRIDGE_H */ diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c index 3d07d81..bce6356 100644 --- a/arch/powerpc/kernel/pci-common.c +++ b/arch/powerpc/kernel/pci-common.c @@ -969,8 +969,7 @@ static void pcibios_setup_device(struct pci_dev *dev) set_dma_offset(&dev->dev, PCI_DRAM_OFFSET); /* Additional platform DMA/iommu setup */ - if (ppc_md.pci_dma_dev_setup) - ppc_md.pci_dma_dev_setup(dev); + pci_dma_dev_setup(dev); /* Read default IRQs and fixup if necessary */ pci_read_irq_line(dev);
Introduces the pci_controller_ops structure. Add pci_controller_ops.dma_dev_setup, shadowing ppc_md.pci_dma_dev_setup. Add a shim, and change the callsites to use the shim. Signed-off-by: Daniel Axtens <dja@axtens.net> --- v1 --> v2: - Better commit message - Use phb in favour of hose - Make shim name match ppc_md name, not pci_controller_ops name. --- arch/powerpc/include/asm/pci-bridge.h | 21 +++++++++++++++++++++ arch/powerpc/kernel/pci-common.c | 3 +-- 2 files changed, 22 insertions(+), 2 deletions(-)