Re: [PATCH 6/9] PCI: consolidate PCI config entry in drivers/pci
On Fri, Oct 19, 2018 at 02:09:49PM +0200, Christoph Hellwig wrote: > There is no good reason to duplicate the PCI menu in every architecture. > Instead provide a selectable HAVE_PCI symbol that indicates availability > of PCI support and the handle the rest in drivers/pci. > > Note that for powerpc we now select HAVE_PCI globally instead of the > convoluted mess of conditional or or non-conditional support per board, > similar to what we do e.g. on x86. For alpha PCI is selected for the > non-jensen configs as it was the default before, and a lot of code does > not compile without PCI enabled. On other architectures with limited > PCI support that wasn't as complicated I've left the selection as-is. > > Signed-off-by: Christoph Hellwig > Acked-by: Max Filippov > Acked-by: Thomas Gleixner > Acked-by: Bjorn Helgaas Sounds like Masahiro plans to take this series and I'm fine with that. Minor typo below, since it sounds like there's another revision coming. > --- a/drivers/pci/Kconfig > +++ b/drivers/pci/Kconfig > @@ -3,6 +3,18 @@ > # PCI configuration > # > > +config HAVE_PCI > + bool > + > +menuconfig PCI > + bool "PCI support" > + depends on HAVE_PCI > + > + help > + This option enables support for the PCI local bus, including > + support for PCI-X and the fundations for PCI Express support. s/fundations/foundations/ > + Say 'Y' here unless you know what you are doing. > + > source "drivers/pci/pcie/Kconfig"
Re: [PATCH 6/9] PCI: consolidate PCI config entry in drivers/pci
On Fri, Oct 19, 2018 at 09:58:46PM +0900, Masahiro Yamada wrote: > On Fri, Oct 19, 2018 at 9:23 PM Russell King - ARM Linux > wrote: > > > > index a68b34183107..b185794549be 100644 > > > --- a/arch/arm/mach-pxa/Kconfig > > > +++ b/arch/arm/mach-pxa/Kconfig > > > @@ -125,7 +125,7 @@ config MACH_ARMCORE > > > bool "CompuLab CM-X255/CM-X270 modules" > > > select ARCH_HAS_DMA_SET_COHERENT_MASK if PCI > > > select IWMMXT > > > - select MIGHT_HAVE_PCI > > > + select HAVE_PCI > > > select NEED_MACH_IO_H if PCI > > > select PXA25x > > > select PXA27x > > > > This is wrong. "MIGHT_HAVE_PCI" is _not_ the same as "HAVE_PCI" - we > > have a bunch of platforms that mandatorily have PCI and these select > > PCI directly. "MIGHT_HAVE_PCI" controls the _visibility_ of the PCI > > menu option, but does not prevent it being selected. Your patch will > > cause Kconfig to complain for those which mandatorily have PCI but > > do not set HAVE_PCI. > > > Good catch! > But, adding a bunch of 'select HAVE_PCI' along with 'select PCI' is ugly. > > Do you have any suggestion? > > How about letting CONFIG_ARM to select HAVE_PCI ? Well, the situation we have on ARM is rather optimal - when the rest of the configuration supports PCI, PCI is made visible. When there's no hardware support for PCI, PCI is hidden. When PCI is mandatory, PCI is selected and mostly always hidden. It seems that with this consolidation, we lose that - we end up with PCI being visible for every ARM config, not only those where PCI is "impossible" but also for those where PCI is forcefully selected. That said, offering PCI for platforms that do not have any possibility of PCI hardware shouldn't cause any compile issues, and would increase build coverage - but I'd say it wouldn't _usefully_ increase build coverage. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH 6/9] PCI: consolidate PCI config entry in drivers/pci
On Thu, Nov 01, 2018 at 01:05:26AM +0900, Masahiro Yamada wrote: > > How about letting CONFIG_ARM to select HAVE_PCI ? > > > > > I applied 1/9, 3/9, 4/9, 5/9. > (I think 2/9 should be squashed to 9/9) > > As Russell pointed out, we need to avoid > the unmet dependency. Yes, I think the HAVE_PCI is probably the nicest way, but we'll need to wait what Russell as the maintainer wants. > Are you planning to send > the updated version for 6/9 through - 9/9 ? > > If so, could you please rebase 6/9 > so that it is cleanly applicable ? Will do once I find some time after rc1.
Re: [PATCH 6/9] PCI: consolidate PCI config entry in drivers/pci
Hi Christoph, On Fri, Oct 19, 2018 at 9:58 PM Masahiro Yamada wrote: > > On Fri, Oct 19, 2018 at 9:23 PM Russell King - ARM Linux > wrote: > > > > index a68b34183107..b185794549be 100644 > > > --- a/arch/arm/mach-pxa/Kconfig > > > +++ b/arch/arm/mach-pxa/Kconfig > > > @@ -125,7 +125,7 @@ config MACH_ARMCORE > > > bool "CompuLab CM-X255/CM-X270 modules" > > > select ARCH_HAS_DMA_SET_COHERENT_MASK if PCI > > > select IWMMXT > > > - select MIGHT_HAVE_PCI > > > + select HAVE_PCI > > > select NEED_MACH_IO_H if PCI > > > select PXA25x > > > select PXA27x > > > > This is wrong. "MIGHT_HAVE_PCI" is _not_ the same as "HAVE_PCI" - we > > have a bunch of platforms that mandatorily have PCI and these select > > PCI directly. "MIGHT_HAVE_PCI" controls the _visibility_ of the PCI > > menu option, but does not prevent it being selected. Your patch will > > cause Kconfig to complain for those which mandatorily have PCI but > > do not set HAVE_PCI. > > > Good catch! > But, adding a bunch of 'select HAVE_PCI' along with 'select PCI' is ugly. > > Do you have any suggestion? > > How about letting CONFIG_ARM to select HAVE_PCI ? > I applied 1/9, 3/9, 4/9, 5/9. (I think 2/9 should be squashed to 9/9) As Russell pointed out, we need to avoid the unmet dependency. Are you planning to send the updated version for 6/9 through - 9/9 ? If so, could you please rebase 6/9 so that it is cleanly applicable ? Thanks. -- Best Regards Masahiro Yamada
Re: [PATCH 6/9] PCI: consolidate PCI config entry in drivers/pci
On Fri, 19 Oct 2018 05:09:49 PDT (-0700), Christoph Hellwig wrote: There is no good reason to duplicate the PCI menu in every architecture. Instead provide a selectable HAVE_PCI symbol that indicates availability of PCI support and the handle the rest in drivers/pci. Note that for powerpc we now select HAVE_PCI globally instead of the convoluted mess of conditional or or non-conditional support per board, similar to what we do e.g. on x86. For alpha PCI is selected for the non-jensen configs as it was the default before, and a lot of code does not compile without PCI enabled. On other architectures with limited PCI support that wasn't as complicated I've left the selection as-is. Signed-off-by: Christoph Hellwig Acked-by: Max Filippov Acked-by: Thomas Gleixner Acked-by: Bjorn Helgaas ... diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index a344980287a5..071952cd4cae 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -39,8 +39,10 @@ config RISCV select SPARSE_IRQ select SYSCTL_EXCEPTION_TRACE select HAVE_ARCH_TRACEHOOK + select HAVE_PCI select MODULES_USE_ELF_RELA if MODULES select THREAD_INFO_IN_TASK + select PCI_MSI if PCI select RISCV_TIMER select GENERIC_IRQ_MULTI_HANDLER select ARCH_HAS_PTE_SPECIAL @@ -216,28 +218,12 @@ source "kernel/Kconfig.hz" endmenu -menu "Bus support" - -config PCI - bool "PCI support" - select PCI_MSI - help - This feature enables support for PCI bus system. If you say Y - here, the kernel will include drivers and infrastructure code - to support PCI bus devices. - - If you don't know what to do here, say Y. - config PCI_DOMAINS def_bool PCI config PCI_DOMAINS_GENERIC def_bool PCI -source "drivers/pci/Kconfig" - -endmenu - menu "Power management options" source kernel/power/Kconfig Reviewed-by: Palmer Dabbelt I'm assuming this will go in via PCI tree of some sort, so I'm not going to touch it any further. Thanks for cleaning this up!
Re: [PATCH 6/9] PCI: consolidate PCI config entry in drivers/pci
On Fri, Oct 19, 2018 at 9:23 PM Russell King - ARM Linux wrote: > > index a68b34183107..b185794549be 100644 > > --- a/arch/arm/mach-pxa/Kconfig > > +++ b/arch/arm/mach-pxa/Kconfig > > @@ -125,7 +125,7 @@ config MACH_ARMCORE > > bool "CompuLab CM-X255/CM-X270 modules" > > select ARCH_HAS_DMA_SET_COHERENT_MASK if PCI > > select IWMMXT > > - select MIGHT_HAVE_PCI > > + select HAVE_PCI > > select NEED_MACH_IO_H if PCI > > select PXA25x > > select PXA27x > > This is wrong. "MIGHT_HAVE_PCI" is _not_ the same as "HAVE_PCI" - we > have a bunch of platforms that mandatorily have PCI and these select > PCI directly. "MIGHT_HAVE_PCI" controls the _visibility_ of the PCI > menu option, but does not prevent it being selected. Your patch will > cause Kconfig to complain for those which mandatorily have PCI but > do not set HAVE_PCI. Good catch! But, adding a bunch of 'select HAVE_PCI' along with 'select PCI' is ugly. Do you have any suggestion? How about letting CONFIG_ARM to select HAVE_PCI ? -- Best Regards Masahiro Yamada
Re: [PATCH 6/9] PCI: consolidate PCI config entry in drivers/pci
On Fri, Oct 19, 2018 at 02:09:49PM +0200, Christoph Hellwig wrote: > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index e33735ce1c14..7495d0a0aa31 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -149,9 +149,6 @@ config ARM_DMA_IOMMU_ALIGNMENT > > endif > > -config MIGHT_HAVE_PCI > - bool > - > config SYS_SUPPORTS_APM_EMULATION > bool > > @@ -320,7 +317,7 @@ config ARCH_MULTIPLATFORM > select COMMON_CLK > select GENERIC_CLOCKEVENTS > select GENERIC_IRQ_MULTI_HANDLER > - select MIGHT_HAVE_PCI > + select HAVE_PCI > select PCI_DOMAINS if PCI > select SPARSE_IRQ > select USE_OF > @@ -436,7 +433,7 @@ config ARCH_IXP4XX > select DMABOUNCE if PCI > select GENERIC_CLOCKEVENTS > select GPIOLIB > - select MIGHT_HAVE_PCI > + select HAVE_PCI > select NEED_MACH_IO_H > select USB_EHCI_BIG_ENDIAN_DESC > select USB_EHCI_BIG_ENDIAN_MMIO > @@ -449,7 +446,7 @@ config ARCH_DOVE > select GENERIC_CLOCKEVENTS > select GENERIC_IRQ_MULTI_HANDLER > select GPIOLIB > - select MIGHT_HAVE_PCI > + select HAVE_PCI > select MVEBU_MBUS > select PINCTRL > select PINCTRL_DOVE > @@ -1216,14 +1213,6 @@ config ISA_DMA > config ISA_DMA_API > bool > > -config PCI > - bool "PCI support" if MIGHT_HAVE_PCI > - help > - Find out whether you have a PCI motherboard. PCI is the name of a > - bus system, i.e. the way the CPU talks to the other stuff inside > - your box. Other bus systems are ISA, EISA, MicroChannel (MCA) or > - VESA. If you have PCI, say Y, otherwise N. > - > config PCI_DOMAINS > bool "Support for multiple PCI domains" > depends on PCI > @@ -1252,8 +1241,6 @@ config PCI_HOST_ITE8152 > default y > select DMABOUNCE > > -source "drivers/pci/Kconfig" > - > source "drivers/pcmcia/Kconfig" > > endmenu > diff --git a/arch/arm/mach-ks8695/Kconfig b/arch/arm/mach-ks8695/Kconfig > index a545976bdbd6..b3185c05fffa 100644 > --- a/arch/arm/mach-ks8695/Kconfig > +++ b/arch/arm/mach-ks8695/Kconfig > @@ -4,7 +4,7 @@ menu "Kendin/Micrel KS8695 Implementations" > > config MACH_KS8695 > bool "KS8695 development board" > - select MIGHT_HAVE_PCI > + select HAVE_PCI > help > Say 'Y' here if you want your kernel to run on the original > Kendin-Micrel KS8695 development board. > @@ -52,7 +52,7 @@ config MACH_CM4002 > > config MACH_CM4008 > bool "OpenGear CM4008" > - select MIGHT_HAVE_PCI > + select HAVE_PCI > help > Say 'Y' here if you want your kernel to support the OpenGear > CM4008 Console Server. See http://www.opengear.com for more > @@ -60,7 +60,7 @@ config MACH_CM4008 > > config MACH_CM41xx > bool "OpenGear CM41xx" > - select MIGHT_HAVE_PCI > + select HAVE_PCI > help > Say 'Y' here if you want your kernel to support the OpenGear > CM4016 or CM4048 Console Servers. See http://www.opengear.com for > @@ -68,7 +68,7 @@ config MACH_CM41xx > > config MACH_IM4004 > bool "OpenGear IM4004" > - select MIGHT_HAVE_PCI > + select HAVE_PCI > help > Say 'Y' here if you want your kernel to support the OpenGear > IM4004 Secure Access Server. See http://www.opengear.com for > @@ -76,7 +76,7 @@ config MACH_IM4004 > > config MACH_IM42xx > bool "OpenGear IM42xx" > - select MIGHT_HAVE_PCI > + select HAVE_PCI > help > Say 'Y' here if you want your kernel to support the OpenGear > IM4216 or IM4248 Console Servers. See http://www.opengear.com for > diff --git a/arch/arm/mach-pxa/Kconfig b/arch/arm/mach-pxa/Kconfig > index a68b34183107..b185794549be 100644 > --- a/arch/arm/mach-pxa/Kconfig > +++ b/arch/arm/mach-pxa/Kconfig > @@ -125,7 +125,7 @@ config MACH_ARMCORE > bool "CompuLab CM-X255/CM-X270 modules" > select ARCH_HAS_DMA_SET_COHERENT_MASK if PCI > select IWMMXT > - select MIGHT_HAVE_PCI > + select HAVE_PCI > select NEED_MACH_IO_H if PCI > select PXA25x > select PXA27x This is wrong. "MIGHT_HAVE_PCI" is _not_ the same as "HAVE_PCI" - we have a bunch of platforms that mandatorily have PCI and these select PCI directly. "MIGHT_HAVE_PCI" controls the _visibility_ of the PCI menu option, but does not prevent it being selected. Your patch will cause Kconfig to complain for those which mandatorily have PCI but do not set HAVE_PCI. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
[PATCH 6/9] PCI: consolidate PCI config entry in drivers/pci
There is no good reason to duplicate the PCI menu in every architecture. Instead provide a selectable HAVE_PCI symbol that indicates availability of PCI support and the handle the rest in drivers/pci. Note that for powerpc we now select HAVE_PCI globally instead of the convoluted mess of conditional or or non-conditional support per board, similar to what we do e.g. on x86. For alpha PCI is selected for the non-jensen configs as it was the default before, and a lot of code does not compile without PCI enabled. On other architectures with limited PCI support that wasn't as complicated I've left the selection as-is. Signed-off-by: Christoph Hellwig Acked-by: Max Filippov Acked-by: Thomas Gleixner Acked-by: Bjorn Helgaas --- arch/alpha/Kconfig | 15 ++--- arch/arc/Kconfig | 20 arch/arc/plat-axs10x/Kconfig | 2 +- arch/arc/plat-hsdk/Kconfig | 2 +- arch/arm/Kconfig | 19 ++-- arch/arm/mach-ks8695/Kconfig | 10 +++--- arch/arm/mach-pxa/Kconfig | 2 +- arch/arm64/Kconfig | 10 +- arch/hexagon/Kconfig | 3 -- arch/ia64/Kconfig | 9 +- arch/m68k/Kconfig.bus | 11 --- arch/m68k/Kconfig.cpu | 1 + arch/microblaze/Kconfig| 6 +--- arch/mips/Kconfig | 43 +- arch/mips/alchemy/Kconfig | 6 ++-- arch/mips/ath25/Kconfig| 2 +- arch/mips/ath79/Kconfig| 8 ++--- arch/mips/bcm63xx/Kconfig | 14 - arch/mips/lantiq/Kconfig | 2 +- arch/mips/loongson64/Kconfig | 6 ++-- arch/mips/pmcs-msp71xx/Kconfig | 10 +++--- arch/mips/ralink/Kconfig | 8 ++--- arch/mips/sibyte/Kconfig | 10 +++--- arch/mips/txx9/Kconfig | 8 ++--- arch/mips/vr41xx/Kconfig | 8 ++--- arch/parisc/Kconfig| 1 + arch/powerpc/Kconfig | 25 --- arch/powerpc/platforms/44x/Kconfig | 1 - arch/powerpc/platforms/512x/Kconfig| 1 - arch/powerpc/platforms/52xx/Kconfig| 1 - arch/powerpc/platforms/83xx/Kconfig| 1 - arch/powerpc/platforms/85xx/Kconfig| 1 - arch/powerpc/platforms/86xx/Kconfig| 2 -- arch/powerpc/platforms/Kconfig | 1 - arch/powerpc/platforms/Kconfig.cputype | 2 -- arch/powerpc/platforms/ps3/Kconfig | 1 - arch/riscv/Kconfig | 18 ++- arch/s390/Kconfig | 23 +- arch/sh/Kconfig| 19 ++-- arch/sh/boards/Kconfig | 30 +- arch/sparc/Kconfig | 15 + arch/um/Kconfig| 3 -- arch/unicore32/Kconfig | 11 +-- arch/x86/Kconfig | 12 +-- arch/x86/configs/i386_defconfig| 1 + arch/x86/configs/x86_64_defconfig | 1 + arch/xtensa/Kconfig| 16 +- arch/xtensa/configs/common_defconfig | 1 + arch/xtensa/configs/iss_defconfig | 1 - drivers/Kconfig| 4 +++ drivers/parisc/Kconfig | 11 --- drivers/pci/Kconfig| 12 +++ drivers/pci/endpoint/Kconfig | 2 +- 53 files changed, 133 insertions(+), 319 deletions(-) diff --git a/arch/alpha/Kconfig b/arch/alpha/Kconfig index 5b4f88363453..bb89924c0361 100644 --- a/arch/alpha/Kconfig +++ b/arch/alpha/Kconfig @@ -6,6 +6,8 @@ config ALPHA select ARCH_MIGHT_HAVE_PC_SERIO select ARCH_NO_PREEMPT select ARCH_USE_CMPXCHG_LOCKREF + select HAVE_PCI if !ALPHA_JENSEN + select PCI if !ALPHA_JENSEN select HAVE_AOUT select HAVE_IDE select HAVE_OPROFILE @@ -15,6 +17,7 @@ config ALPHA select NEED_SG_DMA_LENGTH select VIRT_TO_BUS select GENERIC_IRQ_PROBE + select GENERIC_PCI_IOMAP if PCI select AUTO_IRQ_AFFINITY if SMP select GENERIC_IRQ_SHOW select ARCH_WANT_IPC_PARSE_VERSION @@ -319,17 +322,6 @@ config ISA_DMA_API bool default y -config PCI - bool - depends on !ALPHA_JENSEN - select GENERIC_PCI_IOMAP - default y - help - Find out whether you have a PCI motherboard. PCI is the name of a - bus system, i.e. the way the CPU talks to the other stuff inside - your box. Other bus systems are ISA, EISA, MicroChannel (MCA) or - VESA. If you have PCI, say Y, otherwise N. - config PCI_DOMAINS bool default y @@ -681,7 +673,6 @@ config HZ default 1200 if HZ_1200 default 1024 -source "drivers/pci/Kconfig" source "drivers/eisa/Kconfig" source "drivers/pcmcia/Kconfig" diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig
