Message ID | 20190319154755.31049-1-tbogendoerfer@suse.de (mailing list archive) |
---|---|
Headers | show |
Series | MIPS: SGI-IP27 rework part2 | expand |
SGI IP27 (Origin/Onyx2) and SGI IP30 (Octane) have a similair architecture and share some hardware (ioc3/bridge). To share the software parts this patchset reworks SGI IP27 interrupt and pci bridge code. By using features Linux gained during the many years since SGI IP27 code was integrated this even results in code reduction and IMHO cleaner code. Tests have been done on a two module O200 (4 CPUs) and an Origin 2000 (8 CPUs). My next step in integrating SGI IP30 support is splitting ioc3eth into a MFD and subdevice drivers, which will be submitted soon. Changes in v3: - dropped patches accepted by Paul - moved IP27 specific __phys_to_dma/__dma_to_phys into its own file - moved pcibios_to_node into IP27 specific file - moved PCI bus address resources setup out of pci-xtalk code into IP27 specific code - dropped bit from hub_irq_data and use hwirq from irq_data - introduced intr_addr for setting up bridge interrupts (IP30 preperation) Changes in v2: - replaced HUB_L/HUB_S by __raw_readq/__raw_writeq - removed union bridge_ate - replaced remaing fields in slice_data by per_cpu data - use generic_handle_irq instead of do_IRQ - use hierarchy irq domain for stacking bridge and hub interrupt - moved __dma_to_phys/__phy_to_dma to mach-ip27/dma-direct.h - use dev_to_node() for pcibus_to_node() implementation Thomas Bogendoerfer (10): MIPS: SGI-IP27: get rid of volatile and hubreg_t MIPS: SGI-IP27: clean up bridge access and header files MIPS: SGI-IP27: use pr_info/pr_emerg and pr_cont to fix output MIPS: SGI-IP27: do xtalk scanning later MIPS: SGI-IP27: do boot CPU init later MIPS: SGI-IP27: rework HUB interrupts PCI: call add_bus method also for root bus MIPS: SGI-IP27: use generic PCI driver genirq/irqdomain: fall back to default domain when creating hierarchy domain MIPS: SGI-IP27: abstract chipset irq from bridge arch/mips/Kconfig | 4 + arch/mips/include/asm/dma-direct.h | 2 + arch/mips/include/asm/mach-generic/dma-direct.h | 7 + arch/mips/include/asm/mach-ip27/dma-direct.h | 20 + arch/mips/include/asm/mach-ip27/irq.h | 12 +- arch/mips/include/asm/mach-ip27/mmzone.h | 9 - arch/mips/include/asm/mach-ip27/topology.h | 13 +- arch/mips/include/asm/pci.h | 8 + arch/mips/include/asm/pci/bridge.h | 211 ++++---- arch/mips/include/asm/smp-ops.h | 1 + arch/mips/include/asm/sn/addrs.h | 72 +-- arch/mips/include/asm/sn/arch.h | 2 - arch/mips/include/asm/sn/intr.h | 6 + arch/mips/include/asm/sn/io.h | 2 +- arch/mips/include/asm/sn/sn0/addrs.h | 5 - arch/mips/include/asm/xtalk/xtalk.h | 9 - arch/mips/kernel/smp.c | 2 + arch/mips/pci/Makefile | 1 - arch/mips/pci/ops-bridge.c | 322 ------------ arch/mips/pci/pci-ip27.c | 233 --------- arch/mips/sgi-ip27/Makefile | 3 +- arch/mips/sgi-ip27/ip27-hubio.c | 4 +- arch/mips/sgi-ip27/ip27-init.c | 41 +- arch/mips/sgi-ip27/ip27-irq-pci.c | 266 ---------- arch/mips/sgi-ip27/ip27-irq.c | 331 ++++++++----- arch/mips/sgi-ip27/ip27-irqno.c | 48 -- arch/mips/sgi-ip27/ip27-memory.c | 34 +- arch/mips/sgi-ip27/ip27-nmi.c | 64 +-- arch/mips/sgi-ip27/ip27-smp.c | 5 +- arch/mips/sgi-ip27/ip27-timer.c | 42 +- arch/mips/sgi-ip27/ip27-xtalk.c | 44 +- drivers/pci/controller/Kconfig | 3 + drivers/pci/controller/Makefile | 1 + drivers/pci/controller/pci-xtalk-bridge.c | 623 ++++++++++++++++++++++++ drivers/pci/probe.c | 6 + include/linux/platform_data/xtalk-bridge.h | 17 + kernel/irq/irqdomain.c | 5 +- 37 files changed, 1126 insertions(+), 1352 deletions(-) create mode 100644 arch/mips/include/asm/mach-generic/dma-direct.h create mode 100644 arch/mips/include/asm/mach-ip27/dma-direct.h delete mode 100644 arch/mips/pci/ops-bridge.c delete mode 100644 arch/mips/pci/pci-ip27.c delete mode 100644 arch/mips/sgi-ip27/ip27-irq-pci.c delete mode 100644 arch/mips/sgi-ip27/ip27-irqno.c create mode 100644 drivers/pci/controller/pci-xtalk-bridge.c create mode 100644 include/linux/platform_data/xtalk-bridge.h
Hi Thomas, On Tue, Mar 19, 2019 at 04:47:49PM +0100, Thomas Bogendoerfer wrote: > SGI IP27 (Origin/Onyx2) and SGI IP30 (Octane) have a similair > architecture and share some hardware (ioc3/bridge). To share > the software parts this patchset reworks SGI IP27 interrupt > and pci bridge code. By using features Linux gained during the > many years since SGI IP27 code was integrated this even results > in code reduction and IMHO cleaner code. > > Tests have been done on a two module O200 (4 CPUs) and an > Origin 2000 (8 CPUs). Thanks for doing all this work! It seems like it basically converts some of the SGI PCI code to the structure typical of current host controller drivers and moves it to drivers/pci/controller, which all seems great to me. The patches were kind of in limbo as far as Patchwork. Lorenzo handles the native host controller drivers, so I just delegated them to him, so now they should be on his radar. Bjorn > My next step in integrating SGI IP30 support is splitting ioc3eth > into a MFD and subdevice drivers, which will be submitted soon. > > Changes in v3: > > - dropped patches accepted by Paul > - moved IP27 specific __phys_to_dma/__dma_to_phys into its own file > - moved pcibios_to_node into IP27 specific file > - moved PCI bus address resources setup out of pci-xtalk code into > IP27 specific code > - dropped bit from hub_irq_data and use hwirq from irq_data > - introduced intr_addr for setting up bridge interrupts (IP30 preperation) > > Changes in v2: > > - replaced HUB_L/HUB_S by __raw_readq/__raw_writeq > - removed union bridge_ate > - replaced remaing fields in slice_data by per_cpu data > - use generic_handle_irq instead of do_IRQ > - use hierarchy irq domain for stacking bridge and hub interrupt > - moved __dma_to_phys/__phy_to_dma to mach-ip27/dma-direct.h > - use dev_to_node() for pcibus_to_node() implementation > > Thomas Bogendoerfer (3): > MIPS: SGI-IP27: move IP27 specific code out of pci-ip27.c into new > file > MIPS: SGI-IP27: use generic PCI driver > MIPS: SGI-IP27: abstract chipset irq from bridge > > arch/mips/Kconfig | 3 + > arch/mips/include/asm/mach-ip27/topology.h | 11 +- > arch/mips/include/asm/pci/bridge.h | 14 +- > arch/mips/include/asm/sn/irq_alloc.h | 11 + > arch/mips/include/asm/xtalk/xtalk.h | 9 - > arch/mips/pci/Makefile | 1 - > arch/mips/pci/ops-bridge.c | 302 -------------- > arch/mips/pci/pci-ip27.c | 214 ---------- > arch/mips/sgi-ip27/Makefile | 4 +- > arch/mips/sgi-ip27/ip27-init.c | 2 + > arch/mips/sgi-ip27/ip27-irq.c | 191 ++++----- > arch/mips/sgi-ip27/ip27-pci.c | 30 ++ > arch/mips/sgi-ip27/ip27-xtalk.c | 61 ++- > drivers/pci/controller/Kconfig | 3 + > drivers/pci/controller/Makefile | 1 + > drivers/pci/controller/pci-xtalk-bridge.c | 610 +++++++++++++++++++++++++++++ > include/linux/platform_data/xtalk-bridge.h | 22 ++ > 17 files changed, 822 insertions(+), 667 deletions(-) > create mode 100644 arch/mips/include/asm/sn/irq_alloc.h > delete mode 100644 arch/mips/pci/ops-bridge.c > delete mode 100644 arch/mips/pci/pci-ip27.c > create mode 100644 arch/mips/sgi-ip27/ip27-pci.c > create mode 100644 drivers/pci/controller/pci-xtalk-bridge.c > create mode 100644 include/linux/platform_data/xtalk-bridge.h > > -- > 2.13.7 >
On Thu, 18 Apr 2019 15:57:26 -0500 Bjorn Helgaas <helgaas@kernel.org> wrote: > Hi Thomas, > > On Tue, Mar 19, 2019 at 04:47:49PM +0100, Thomas Bogendoerfer wrote: > > SGI IP27 (Origin/Onyx2) and SGI IP30 (Octane) have a similair > > architecture and share some hardware (ioc3/bridge). To share > > the software parts this patchset reworks SGI IP27 interrupt > > and pci bridge code. By using features Linux gained during the > > many years since SGI IP27 code was integrated this even results > > in code reduction and IMHO cleaner code. > > > > Tests have been done on a two module O200 (4 CPUs) and an > > Origin 2000 (8 CPUs). > > Thanks for doing all this work! It seems like it basically converts > some of the SGI PCI code to the structure typical of current host > controller drivers and moves it to drivers/pci/controller, which all > seems great to me. > > The patches were kind of in limbo as far as Patchwork. Lorenzo > handles the native host controller drivers, so I just delegated them > to him, so now they should be on his radar. Is there a chance to still get this into v5.2 ? Thomas.
On Mon, May 06, 2019 at 12:37:24PM +0200, Thomas Bogendoerfer wrote: > On Thu, 18 Apr 2019 15:57:26 -0500 > Bjorn Helgaas <helgaas@kernel.org> wrote: > > > Hi Thomas, > > > > On Tue, Mar 19, 2019 at 04:47:49PM +0100, Thomas Bogendoerfer wrote: > > > SGI IP27 (Origin/Onyx2) and SGI IP30 (Octane) have a similair > > > architecture and share some hardware (ioc3/bridge). To share > > > the software parts this patchset reworks SGI IP27 interrupt > > > and pci bridge code. By using features Linux gained during the > > > many years since SGI IP27 code was integrated this even results > > > in code reduction and IMHO cleaner code. > > > > > > Tests have been done on a two module O200 (4 CPUs) and an > > > Origin 2000 (8 CPUs). > > > > Thanks for doing all this work! It seems like it basically converts > > some of the SGI PCI code to the structure typical of current host > > controller drivers and moves it to drivers/pci/controller, which all > > seems great to me. > > > > The patches were kind of in limbo as far as Patchwork. Lorenzo > > handles the native host controller drivers, so I just delegated them > > to him, so now they should be on his radar. > > Is there a chance to still get this into v5.2 ? I do not think so, I will have a look as soon as possible, sorry for the delay. Lorenzo
On Thu, Apr 18, 2019 at 03:57:26PM -0500, Bjorn Helgaas wrote: > Hi Thomas, > > On Tue, Mar 19, 2019 at 04:47:49PM +0100, Thomas Bogendoerfer wrote: > > SGI IP27 (Origin/Onyx2) and SGI IP30 (Octane) have a similair > > architecture and share some hardware (ioc3/bridge). To share > > the software parts this patchset reworks SGI IP27 interrupt > > and pci bridge code. By using features Linux gained during the > > many years since SGI IP27 code was integrated this even results > > in code reduction and IMHO cleaner code. > > > > Tests have been done on a two module O200 (4 CPUs) and an > > Origin 2000 (8 CPUs). > > Thanks for doing all this work! It seems like it basically converts > some of the SGI PCI code to the structure typical of current host > controller drivers and moves it to drivers/pci/controller, which all > seems great to me. I had a look and the code is really, really MIPS specific, actually I would be interested in understanding how many platforms it supports, it is not even FW configurable. To give another piece of evidence why some PCI driver code is still in arch code, look at arch/arm, we did not move it to drivers/pci/controller for the same reasons, eg lack of DT support. With hard-coded resources, <asm/...> includes in driver code and MIPS specific kludges even if it does reuse some APIs shared with controller drivers I am not 100% certain that moving it to drivers/pci/controller buys us anything, this is really arch specific code, however we slice it. The line between what stays in arch and what goes to drivers/pci/controller is thin but this code is definitely more on the arch side IMHO. I do not question Thomas' effort, which I appreciate, I question the end result and its usefulness, this series is even increasing lines of kernel code, I would like to see the benefits. Lorenzo > The patches were kind of in limbo as far as Patchwork. Lorenzo > handles the native host controller drivers, so I just delegated them > to him, so now they should be on his radar. > > Bjorn > > > My next step in integrating SGI IP30 support is splitting ioc3eth > > into a MFD and subdevice drivers, which will be submitted soon. > > > > Changes in v3: > > > > - dropped patches accepted by Paul > > - moved IP27 specific __phys_to_dma/__dma_to_phys into its own file > > - moved pcibios_to_node into IP27 specific file > > - moved PCI bus address resources setup out of pci-xtalk code into > > IP27 specific code > > - dropped bit from hub_irq_data and use hwirq from irq_data > > - introduced intr_addr for setting up bridge interrupts (IP30 preperation) > > > > Changes in v2: > > > > - replaced HUB_L/HUB_S by __raw_readq/__raw_writeq > > - removed union bridge_ate > > - replaced remaing fields in slice_data by per_cpu data > > - use generic_handle_irq instead of do_IRQ > > - use hierarchy irq domain for stacking bridge and hub interrupt > > - moved __dma_to_phys/__phy_to_dma to mach-ip27/dma-direct.h > > - use dev_to_node() for pcibus_to_node() implementation > > > > Thomas Bogendoerfer (3): > > MIPS: SGI-IP27: move IP27 specific code out of pci-ip27.c into new > > file > > MIPS: SGI-IP27: use generic PCI driver > > MIPS: SGI-IP27: abstract chipset irq from bridge > > > > arch/mips/Kconfig | 3 + > > arch/mips/include/asm/mach-ip27/topology.h | 11 +- > > arch/mips/include/asm/pci/bridge.h | 14 +- > > arch/mips/include/asm/sn/irq_alloc.h | 11 + > > arch/mips/include/asm/xtalk/xtalk.h | 9 - > > arch/mips/pci/Makefile | 1 - > > arch/mips/pci/ops-bridge.c | 302 -------------- > > arch/mips/pci/pci-ip27.c | 214 ---------- > > arch/mips/sgi-ip27/Makefile | 4 +- > > arch/mips/sgi-ip27/ip27-init.c | 2 + > > arch/mips/sgi-ip27/ip27-irq.c | 191 ++++----- > > arch/mips/sgi-ip27/ip27-pci.c | 30 ++ > > arch/mips/sgi-ip27/ip27-xtalk.c | 61 ++- > > drivers/pci/controller/Kconfig | 3 + > > drivers/pci/controller/Makefile | 1 + > > drivers/pci/controller/pci-xtalk-bridge.c | 610 +++++++++++++++++++++++++++++ > > include/linux/platform_data/xtalk-bridge.h | 22 ++ > > 17 files changed, 822 insertions(+), 667 deletions(-) > > create mode 100644 arch/mips/include/asm/sn/irq_alloc.h > > delete mode 100644 arch/mips/pci/ops-bridge.c > > delete mode 100644 arch/mips/pci/pci-ip27.c > > create mode 100644 arch/mips/sgi-ip27/ip27-pci.c > > create mode 100644 drivers/pci/controller/pci-xtalk-bridge.c > > create mode 100644 include/linux/platform_data/xtalk-bridge.h > > > > -- > > 2.13.7 > >
On Tue, 7 May 2019 16:31:17 +0100 Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Thu, Apr 18, 2019 at 03:57:26PM -0500, Bjorn Helgaas wrote: > > Hi Thomas, > > > > On Tue, Mar 19, 2019 at 04:47:49PM +0100, Thomas Bogendoerfer wrote: > > > SGI IP27 (Origin/Onyx2) and SGI IP30 (Octane) have a similair > > > architecture and share some hardware (ioc3/bridge). To share > > > the software parts this patchset reworks SGI IP27 interrupt > > > and pci bridge code. By using features Linux gained during the > > > many years since SGI IP27 code was integrated this even results > > > in code reduction and IMHO cleaner code. > > > > > > Tests have been done on a two module O200 (4 CPUs) and an > > > Origin 2000 (8 CPUs). > > > > Thanks for doing all this work! It seems like it basically converts > > some of the SGI PCI code to the structure typical of current host > > controller drivers and moves it to drivers/pci/controller, which all > > seems great to me. > > I had a look and the code is really, really MIPS specific, actually > I would be interested in understanding how many platforms it supports, > it is not even FW configurable. it's MIPS only and used in basically 3 different SGI platforms. > With hard-coded resources, <asm/...> includes in driver code and MIPS > specific kludges even if it does reuse some APIs shared with controller > drivers I am not 100% certain that moving it to drivers/pci/controller > buys us anything, this is really arch specific code, however we slice > it. hmm, I thought the idea of having one drivers/pci/controller directory is to have all of them in one place. > The line between what stays in arch and what goes to > drivers/pci/controller is thin but this code is definitely more on the > arch side IMHO. what makes the xgene driver different from the xtalk-bridge driver ? Ok it used DT, but it's still just for a specific type of SOCs from one vendor, isn't it ? > I do not question Thomas' effort, which I appreciate, I question > the end result and its usefulness, this series is even increasing > lines of kernel code, I would like to see the benefits. the move from arch/mips/pci to drivers/pci/controller increases lines of code by two lines. The whole patchset adds 155 lines, but also adds functionality to be able to use the driver with different platforms. Anyway I can live with not moving to drivers/pci/controller if you don't like it there. Thomas.
On Tue, May 07, 2019 at 05:48:20PM +0200, Thomas Bogendoerfer wrote: > On Tue, 7 May 2019 16:31:17 +0100 > Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > > > On Thu, Apr 18, 2019 at 03:57:26PM -0500, Bjorn Helgaas wrote: > > > Hi Thomas, > > > > > > On Tue, Mar 19, 2019 at 04:47:49PM +0100, Thomas Bogendoerfer wrote: > > > > SGI IP27 (Origin/Onyx2) and SGI IP30 (Octane) have a similair > > > > architecture and share some hardware (ioc3/bridge). To share > > > > the software parts this patchset reworks SGI IP27 interrupt > > > > and pci bridge code. By using features Linux gained during the > > > > many years since SGI IP27 code was integrated this even results > > > > in code reduction and IMHO cleaner code. > > > > > > > > Tests have been done on a two module O200 (4 CPUs) and an > > > > Origin 2000 (8 CPUs). > > > > > > Thanks for doing all this work! It seems like it basically converts > > > some of the SGI PCI code to the structure typical of current host > > > controller drivers and moves it to drivers/pci/controller, which all > > > seems great to me. > > > > I had a look and the code is really, really MIPS specific, actually > > I would be interested in understanding how many platforms it supports, > > it is not even FW configurable. > > it's MIPS only and used in basically 3 different SGI platforms. > > > With hard-coded resources, <asm/...> includes in driver code and MIPS > > specific kludges even if it does reuse some APIs shared with controller > > drivers I am not 100% certain that moving it to drivers/pci/controller > > buys us anything, this is really arch specific code, however we slice > > it. > > hmm, I thought the idea of having one drivers/pci/controller directory > is to have all of them in one place. Well, drivers/pci/controller code should at least try to be arch agnostic to belong there, there are exceptions, as I said but they are the exception not the rule. > > The line between what stays in arch and what goes to > > drivers/pci/controller is thin but this code is definitely more on the > > arch side IMHO. > > what makes the xgene driver different from the xtalk-bridge driver ? Ok > it used DT, but it's still just for a specific type of SOCs from one > vendor, isn't it ? Eg no <asm/...> includes required; its code can in principle be reused on other arches, this series can't, it is arch code IMO. > > I do not question Thomas' effort, which I appreciate, I question > > the end result and its usefulness, this series is even increasing > > lines of kernel code, I would like to see the benefits. > > the move from arch/mips/pci to drivers/pci/controller increases lines of > code by two lines. The whole patchset adds 155 lines, but also adds > functionality to be able to use the driver with different platforms. I am not convinced it will buy you much to be honest, I might be wrong. If we move the code we have to have a compelling reason for it to happen and at the moment I do not necessarily see one, happy to be proven wrong if you extend it to support other platforms cleanly (eg with firmware bindings) on top of this series. > Anyway I can live with not moving to drivers/pci/controller if you don't > like it there. I can't certainly maintain it, it is too MIPS specific and I do not like this code either but that's certainly not your fault. - Hardcoded resources - No firmware bindings - Lots of MIPS specific workarounds - arch code dependency I do not think that as it stands it belongs in driver/pci/controller, no. Thanks, Lorenzo
On Tue, May 07, 2019 at 05:48:20PM +0200, Thomas Bogendoerfer wrote: > > I had a look and the code is really, really MIPS specific, actually > > I would be interested in understanding how many platforms it supports, > > it is not even FW configurable. > > it's MIPS only and used in basically 3 different SGI platforms. Actually this isn't quite true, this whole infraѕtructure was also shared by the SGI IA64 systems. The early SN1 prototypes were basically just IP35 systems with IA64 CPUs glued in, and even the later SN2 that were released as Altix shared the same xtalk + bridge infrastructure, just with some hardware updates. It just turned out SGI at some point decided to hide all that in firmware. Linux 2.4 was still accessing this natively, but the code was a mess. I tried to consolidate it with the mips version at that point, but SGI decided that hiding it under firmware calls was easier as they could just move the messy code there. So, not it isn't really MIPS only in theory, although in practice it very much is..