| Message ID | alpine.DEB.2.21.2202122313460.34636@angie.orcam.me.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | parport_pc: Also enable driver for PCI systems | expand |
Hi Maciej, CC Niklas On Sun, Feb 13, 2022 at 1:45 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote: > Nowadays PC-style parallel ports come in the form of PCI and PCIe option > cards and there are some combined parallel/serial option cards as well > that we handle in the parport subsystem. There is nothing in particular > that would prevent them from being used in any system equipped with PCI > or PCIe connectivity, except that we do not permit the PARPORT_PC config > option to be selected for platforms for which ARCH_MIGHT_HAVE_PC_PARPORT > has not been set for. > > The only PCI platforms that actually can't make use of PC-style parallel > port hardware are those newer PCIe systems that have no support for I/O > cycles in the host bridge, required by such parallel ports. An example > of such a host bridge is the POWER9 PHB4 device, but it is an exception > rather than the norm. Also it is not clear whether the serial port side Note that this hardware dependency is being addressed in "[RFC 00/32] Kconfig: Introduce HAS_IOPORT and LEGACY_PCI options" https://lore.kernel.org/all/20211227164317.4146918-1-schnelle@linux.ibm.com/ > --- linux-macro.orig/drivers/parport/Kconfig > +++ linux-macro/drivers/parport/Kconfig > @@ -42,7 +42,7 @@ if PARPORT > > config PARPORT_PC > tristate "PC-style hardware" > - depends on ARCH_MIGHT_HAVE_PC_PARPORT > + depends on ARCH_MIGHT_HAVE_PC_PARPORT || PCI > help > You should say Y here if you have a PC-style parallel port. All > IBM PC compatible computers and some Alphas have PC-style > @@ -77,7 +77,7 @@ config PARPORT_PC_FIFO > > config PARPORT_PC_SUPERIO > bool "SuperIO chipset support" > - depends on PARPORT_PC && !PARISC > + depends on ARCH_MIGHT_HAVE_PC_PARPORT && PARPORT_PC && !PARISC > help > Saying Y here enables some probes for Super-IO chipsets in order to > find out things like base addresses, IRQ lines and DMA channels. It > Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Mon, 2022-02-14 at 09:20 +0100, Geert Uytterhoeven wrote: > Hi Maciej, > > CC Niklas > > On Sun, Feb 13, 2022 at 1:45 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote: > > Nowadays PC-style parallel ports come in the form of PCI and PCIe option > > cards and there are some combined parallel/serial option cards as well > > that we handle in the parport subsystem. There is nothing in particular > > that would prevent them from being used in any system equipped with PCI > > or PCIe connectivity, except that we do not permit the PARPORT_PC config > > option to be selected for platforms for which ARCH_MIGHT_HAVE_PC_PARPORT > > has not been set for. > > > > The only PCI platforms that actually can't make use of PC-style parallel > > port hardware are those newer PCIe systems that have no support for I/O > > cycles in the host bridge, required by such parallel ports. An example > > of such a host bridge is the POWER9 PHB4 device, but it is an exception > > rather than the norm. Also it is not clear whether the serial port side > > Note that this hardware dependency is being addressed in > "[RFC 00/32] Kconfig: Introduce HAS_IOPORT and LEGACY_PCI options" > https://lore.kernel.org/all/20211227164317.4146918-1-schnelle@linux.ibm.com/ Thanks, for Cc'ing me. Note that this series is currently in kind of a hold as we haven't yet found a clear direction yet. There was some clear opposition for the LEGACY_PCI option introduced in that series and only little support for HAS_IOPORT. > > > --- linux-macro.orig/drivers/parport/Kconfig > > +++ linux-macro/drivers/parport/Kconfig > > @@ -42,7 +42,7 @@ if PARPORT > > > > config PARPORT_PC > > tristate "PC-style hardware" > > - depends on ARCH_MIGHT_HAVE_PC_PARPORT > > + depends on ARCH_MIGHT_HAVE_PC_PARPORT || PCI This would allow selecting PARPORT_PC on s390 e.g. for allyesconfig and randconfig and like POWER9 we definitely do not support I/O port access. We will also get warnings when compiling with clang. A problem my series was originally started to address. So I'd really like to see a better solution here for such a change. With the HAS_IOPORT option from my series this would be simple but we don't currently have that though maybe this is also an argument for introducing HAS_IOPORT even if we don't add the LEGACY_PCI option. > > help > > You should say Y here if you have a PC-style parallel port. All > > IBM PC compatible computers and some Alphas have PC-style > > @@ -77,7 +77,7 @@ config PARPORT_PC_FIFO > > > > config PARPORT_PC_SUPERIO > > bool "SuperIO chipset support" > > - depends on PARPORT_PC && !PARISC > > + depends on ARCH_MIGHT_HAVE_PC_PARPORT && PARPORT_PC && !PARISC > > help > > Saying Y here enables some probes for Super-IO chipsets in order to > > find out things like base addresses, IRQ lines and DMA channels. It > > > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
On Mon, 14 Feb 2022, Geert Uytterhoeven wrote: > > The only PCI platforms that actually can't make use of PC-style parallel > > port hardware are those newer PCIe systems that have no support for I/O > > cycles in the host bridge, required by such parallel ports. An example > > of such a host bridge is the POWER9 PHB4 device, but it is an exception > > rather than the norm. Also it is not clear whether the serial port side > > Note that this hardware dependency is being addressed in > "[RFC 00/32] Kconfig: Introduce HAS_IOPORT and LEGACY_PCI options" > https://lore.kernel.org/all/20211227164317.4146918-1-schnelle@linux.ibm.com/ Thanks for the pointer, I have missed the series in the LKML flood! The idea sounds good, although it's not clear to me if a config option is enough to get it properly covered as I don't know offhand if a single say ppc64le kernel can't run on systems that do and do not have support for port I/O over PCIe. I got hit with that the hard way with a distribution kernel where a driver tried to do port I/O and poked at a random location in the address space causing weird errors to be reported by the system firmware. Also I have skimmed over the series and there appears to be confusion between legacy PCI and conventional PCI, which are obviously not the same. For example this piece: diff --git a/drivers/net/fddi/Kconfig b/drivers/net/fddi/Kconfig index 846bf41c2717..1753c08d6423 100644 --- a/drivers/net/fddi/Kconfig +++ b/drivers/net/fddi/Kconfig @@ -5,7 +5,7 @@ config FDDI tristate "FDDI driver support" - depends on PCI || EISA || TC + depends on LEGACY_PCI || EISA || TC help Fiber Distributed Data Interface is a high speed local area network design; essentially a replacement for high speed Ethernet. FDDI can @@ -29,7 +29,7 @@ config DEFZA config DEFXX tristate "Digital DEFTA/DEFEA/DEFPA adapter support" - depends on FDDI && (PCI || EISA || TC) + depends on FDDI && (LEGACY_PCI || EISA || TC) help This is support for the DIGITAL series of TURBOchannel (DEFTA), EISA (DEFEA) and PCI (DEFPA) controllers which can connect you is clearly wrong. While the DEFPA card is a conventional PCI option it does support MMIO and does run with my legacy-free POWER9 system just fine (with a suitable PCIe-to-PCI bridge installed at 0031:01:00.0): # lspci 0000:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) 0001:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) 0001:01:00.0 Serial controller: Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART 0002:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) 0002:01:00.0 Serial Attached SCSI controller: Adaptec Series 8 12G SAS/PCIe 3 (rev 01) 0003:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) 0003:01:00.0 USB controller: Texas Instruments TUSB73x0 SuperSpeed USB 3.0 xHCI Host Controller (rev 02) 0004:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) 0004:01:00.0 Ethernet controller: Broadcom Inc. and subsidiaries NetXtreme BCM5719 Gigabit Ethernet PCIe (rev 01) 0004:01:00.1 Ethernet controller: Broadcom Inc. and subsidiaries NetXtreme BCM5719 Gigabit Ethernet PCIe (rev 01) 0005:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) 0005:01:00.0 PCI bridge: ASPEED Technology, Inc. AST1150 PCI-to-PCI Bridge (rev 04) 0005:02:00.0 VGA compatible controller: ASPEED Technology, Inc. ASPEED Graphics Family (rev 41) 0030:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) 0030:01:00.0 PCI bridge: PMC-Sierra Inc. PM8562 Switchtec PFX-L 32xG3 Fanout-Lite PCIe Gen3 Switch 0030:02:00.0 PCI bridge: PMC-Sierra Inc. PM8562 Switchtec PFX-L 32xG3 Fanout-Lite PCIe Gen3 Switch 0030:02:01.0 PCI bridge: PMC-Sierra Inc. PM8562 Switchtec PFX-L 32xG3 Fanout-Lite PCIe Gen3 Switch 0030:02:02.0 PCI bridge: PMC-Sierra Inc. PM8562 Switchtec PFX-L 32xG3 Fanout-Lite PCIe Gen3 Switch 0030:02:03.0 PCI bridge: PMC-Sierra Inc. PM8562 Switchtec PFX-L 32xG3 Fanout-Lite PCIe Gen3 Switch 0030:03:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd NVMe SSD Controller SM981/PM981/PM983 0030:04:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd NVMe SSD Controller SM981/PM981/PM983 0030:05:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd NVMe SSD Controller SM981/PM981/PM983 0030:06:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd NVMe SSD Controller SM981/PM981/PM983 0031:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) 0031:01:00.0 PCI bridge: Texas Instruments XIO2000(A)/XIO2200A PCI Express-to-PCI Bridge (rev 03) 0031:02:04.0 FDDI network controller: Digital Equipment Corporation PCI-to-PDQ Interface Chip [PFI] FDDI (DEFPA) (rev 02) 0031:02:05.0 ATM network controller: Microsemi / PMC / IDT IDT77201/77211 155Mbps ATM SAR Controller [NICStAR] (rev 03) 0032:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) 0033:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) # dmesg | grep 0031:02:04.0 | cut -c16- pci 0031:02:04.0: [1011:000f] type 00 class 0x020200 pci 0031:02:04.0: reg 0x10: [mem 0x620c080020000-0x620c08002007f] pci 0031:02:04.0: reg 0x14: [io 0x0000-0x007f] pci 0031:02:04.0: reg 0x18: [mem 0x620c080030000-0x620c08003ffff] pci 0031:02:04.0: BAR0 [mem size 0x00000080]: requesting alignment to 0x10000 pci 0031:02:04.0: BAR 0: assigned [mem 0x620c080020000-0x620c08002007f] pci 0031:02:04.0: BAR 2: assigned [mem 0x620c080030000-0x620c08003ffff] pci 0031:02:04.0: BAR 1: no space for [io size 0x0080] pci 0031:02:04.0: BAR 1: failed to assign [io size 0x0080] pci 0031:02:04.0: Configured PE#fc pci 0031:02:04.0: Adding to iommu group 9 defxx 0031:02:04.0: enabling device (0140 -> 0142) 0031:02:04.0: DEFPA at MMIO addr = 0x620c080020000, IRQ = 57, Hardware addr = 00-60-6d-93-91-98 0031:02:04.0: registered as fddi0 $ ip link show dev fddi0 4: fddi0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 4470 qdisc pfifo_fast state UNKNOWN mode DEFAULT group default qlen 100 link/fddi 00:60:6d:93:91:98 brd ff:ff:ff:ff:ff:ff $ While older versions of the driver did have to be explicitly configured for MMIO rather than port I/O, a feature added with commit e89a2cfb7d7b ("[TC] defxx: TURBOchannel support"), the driver has been improved with commit 795e272e5474 ("FDDI: defxx: Implement dynamic CSR I/O address space selection") and the selection of the I/O space to use now fully automatic. Then what about the other FDDI driver there, SKFP? It's not marked as LEGACY_PCI, although it's not selectable anyway due to the dependency of FDDI on LEGACY_PCI. Niklas, what was the criterion for placing the LEGACY_PCI dependency? Also do you plan to post an updated series anytime soon? I'm asking because like with the m68k port also the MIPS one needs a more finegrained approach and I suspect there may be other corner cases and I'd rather look at the most recent version of your series. Otherwise I'll have a look through your original submission, but it may have to wait until the next weekend due to my other commitments. Maciej
On Mon, 14 Feb 2022, Niklas Schnelle wrote: > > > --- linux-macro.orig/drivers/parport/Kconfig > > > +++ linux-macro/drivers/parport/Kconfig > > > @@ -42,7 +42,7 @@ if PARPORT > > > > > > config PARPORT_PC > > > tristate "PC-style hardware" > > > - depends on ARCH_MIGHT_HAVE_PC_PARPORT > > > + depends on ARCH_MIGHT_HAVE_PC_PARPORT || PCI > > This would allow selecting PARPORT_PC on s390 e.g. for allyesconfig and > randconfig and like POWER9 we definitely do not support I/O port > access. I guess we'll have to stop it with !S390 then short-term. I don't think an issue with s390 should be blocking all the other platforms that can use these drivers just fine. I'll post v2 with that installed if you agree. Maciej
On Mon, 2022-02-14 at 09:21 +0000, Maciej W. Rozycki wrote: > On Mon, 14 Feb 2022, Niklas Schnelle wrote: > > > > > --- linux-macro.orig/drivers/parport/Kconfig > > > > +++ linux-macro/drivers/parport/Kconfig > > > > @@ -42,7 +42,7 @@ if PARPORT > > > > > > > > config PARPORT_PC > > > > tristate "PC-style hardware" > > > > - depends on ARCH_MIGHT_HAVE_PC_PARPORT > > > > + depends on ARCH_MIGHT_HAVE_PC_PARPORT || PCI > > > > This would allow selecting PARPORT_PC on s390 e.g. for allyesconfig and > > randconfig and like POWER9 we definitely do not support I/O port > > access. > > I guess we'll have to stop it with !S390 then short-term. I don't think > an issue with s390 should be blocking all the other platforms that can use > these drivers just fine. I'll post v2 with that installed if you agree. > > Maciej Though a bit ugly that sounds fine by me. Thanks.
On Mon, 2022-02-14 at 09:16 +0000, Maciej W. Rozycki wrote: > On Mon, 14 Feb 2022, Geert Uytterhoeven wrote: > > > > The only PCI platforms that actually can't make use of PC-style parallel > > > port hardware are those newer PCIe systems that have no support for I/O > > > cycles in the host bridge, required by such parallel ports. An example > > > of such a host bridge is the POWER9 PHB4 device, but it is an exception > > > rather than the norm. Also it is not clear whether the serial port side > > > > Note that this hardware dependency is being addressed in > > "[RFC 00/32] Kconfig: Introduce HAS_IOPORT and LEGACY_PCI options" > > https://lore.kernel.org/all/20211227164317.4146918-1-schnelle@linux.ibm.com/ > > Thanks for the pointer, I have missed the series in the LKML flood! > > The idea sounds good, although it's not clear to me if a config option is > enough to get it properly covered as I don't know offhand if a single say > ppc64le kernel can't run on systems that do and do not have support for > port I/O over PCIe. I got hit with that the hard way with a distribution > kernel where a driver tried to do port I/O and poked at a random location > in the address space causing weird errors to be reported by the system > firmware. > > Also I have skimmed over the series and there appears to be confusion > between legacy PCI and conventional PCI, which are obviously not the same. > For example this piece: > > diff --git a/drivers/net/fddi/Kconfig b/drivers/net/fddi/Kconfig > index 846bf41c2717..1753c08d6423 100644 > --- a/drivers/net/fddi/Kconfig > +++ b/drivers/net/fddi/Kconfig > @@ -5,7 +5,7 @@ > > config FDDI > tristate "FDDI driver support" > - depends on PCI || EISA || TC > + depends on LEGACY_PCI || EISA || TC > help > Fiber Distributed Data Interface is a high speed local area network > design; essentially a replacement for high speed Ethernet. FDDI can > @@ -29,7 +29,7 @@ config DEFZA > > config DEFXX > tristate "Digital DEFTA/DEFEA/DEFPA adapter support" > - depends on FDDI && (PCI || EISA || TC) > + depends on FDDI && (LEGACY_PCI || EISA || TC) > help > This is support for the DIGITAL series of TURBOchannel (DEFTA), > EISA (DEFEA) and PCI (DEFPA) controllers which can connect you > > is clearly wrong. While the DEFPA card is a conventional PCI option it > does support MMIO and does run with my legacy-free POWER9 system just > fine (with a suitable PCIe-to-PCI bridge installed at 0031:01:00.0): > > # lspci > 0000:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) > 0001:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) > 0001:01:00.0 Serial controller: Oxford Semiconductor Ltd OXPCIe952 Dual Native 950 UART > 0002:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) > 0002:01:00.0 Serial Attached SCSI controller: Adaptec Series 8 12G SAS/PCIe 3 (rev 01) > 0003:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) > 0003:01:00.0 USB controller: Texas Instruments TUSB73x0 SuperSpeed USB 3.0 xHCI Host Controller (rev 02) > 0004:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) > 0004:01:00.0 Ethernet controller: Broadcom Inc. and subsidiaries NetXtreme BCM5719 Gigabit Ethernet PCIe (rev 01) > 0004:01:00.1 Ethernet controller: Broadcom Inc. and subsidiaries NetXtreme BCM5719 Gigabit Ethernet PCIe (rev 01) > 0005:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) > 0005:01:00.0 PCI bridge: ASPEED Technology, Inc. AST1150 PCI-to-PCI Bridge (rev 04) > 0005:02:00.0 VGA compatible controller: ASPEED Technology, Inc. ASPEED Graphics Family (rev 41) > 0030:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) > 0030:01:00.0 PCI bridge: PMC-Sierra Inc. PM8562 Switchtec PFX-L 32xG3 Fanout-Lite PCIe Gen3 Switch > 0030:02:00.0 PCI bridge: PMC-Sierra Inc. PM8562 Switchtec PFX-L 32xG3 Fanout-Lite PCIe Gen3 Switch > 0030:02:01.0 PCI bridge: PMC-Sierra Inc. PM8562 Switchtec PFX-L 32xG3 Fanout-Lite PCIe Gen3 Switch > 0030:02:02.0 PCI bridge: PMC-Sierra Inc. PM8562 Switchtec PFX-L 32xG3 Fanout-Lite PCIe Gen3 Switch > 0030:02:03.0 PCI bridge: PMC-Sierra Inc. PM8562 Switchtec PFX-L 32xG3 Fanout-Lite PCIe Gen3 Switch > 0030:03:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd NVMe SSD Controller SM981/PM981/PM983 > 0030:04:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd NVMe SSD Controller SM981/PM981/PM983 > 0030:05:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd NVMe SSD Controller SM981/PM981/PM983 > 0030:06:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd NVMe SSD Controller SM981/PM981/PM983 > 0031:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) > 0031:01:00.0 PCI bridge: Texas Instruments XIO2000(A)/XIO2200A PCI Express-to-PCI Bridge (rev 03) > 0031:02:04.0 FDDI network controller: Digital Equipment Corporation PCI-to-PDQ Interface Chip [PFI] FDDI (DEFPA) (rev 02) > 0031:02:05.0 ATM network controller: Microsemi / PMC / IDT IDT77201/77211 155Mbps ATM SAR Controller [NICStAR] (rev 03) > 0032:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) > 0033:00:00.0 PCI bridge: IBM POWER9 Host Bridge (PHB4) > # dmesg | grep 0031:02:04.0 | cut -c16- > pci 0031:02:04.0: [1011:000f] type 00 class 0x020200 > pci 0031:02:04.0: reg 0x10: [mem 0x620c080020000-0x620c08002007f] > pci 0031:02:04.0: reg 0x14: [io 0x0000-0x007f] > pci 0031:02:04.0: reg 0x18: [mem 0x620c080030000-0x620c08003ffff] > pci 0031:02:04.0: BAR0 [mem size 0x00000080]: requesting alignment to 0x10000 > pci 0031:02:04.0: BAR 0: assigned [mem 0x620c080020000-0x620c08002007f] > pci 0031:02:04.0: BAR 2: assigned [mem 0x620c080030000-0x620c08003ffff] > pci 0031:02:04.0: BAR 1: no space for [io size 0x0080] > pci 0031:02:04.0: BAR 1: failed to assign [io size 0x0080] > pci 0031:02:04.0: Configured PE#fc > pci 0031:02:04.0: Adding to iommu group 9 > defxx 0031:02:04.0: enabling device (0140 -> 0142) > 0031:02:04.0: DEFPA at MMIO addr = 0x620c080020000, IRQ = 57, Hardware addr = 00-60-6d-93-91-98 > 0031:02:04.0: registered as fddi0 > $ ip link show dev fddi0 > 4: fddi0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 4470 qdisc pfifo_fast > state UNKNOWN mode DEFAULT group default qlen 100 > link/fddi 00:60:6d:93:91:98 brd ff:ff:ff:ff:ff:ff > $ > > While older versions of the driver did have to be explicitly configured > for MMIO rather than port I/O, a feature added with commit e89a2cfb7d7b > ("[TC] defxx: TURBOchannel support"), the driver has been improved with > commit 795e272e5474 ("FDDI: defxx: Implement dynamic CSR I/O address space > selection") and the selection of the I/O space to use now fully automatic. Very interesting and thanks for the input! On s390 we really only have very few different PCI devices and I can only test another hand full with my private x86 and ARM systems. > > Then what about the other FDDI driver there, SKFP? It's not marked as > LEGACY_PCI, although it's not selectable anyway due to the dependency of > FDDI on LEGACY_PCI. > > Niklas, what was the criterion for placing the LEGACY_PCI dependency? Hmm, honestly I haven't really worked on this recently. There were some open questions from Bjorn towards Arnd and I was waiting for his reply but I guess he missed those. I think what you noticed was the main problem, there wasn't really a clear set of criteria for LEGACY_PCI and even for HAS_IOPORT we missed some uses if they were not compiled on s390's allyesconfig due to other dependencies. > > Also do you plan to post an updated series anytime soon? I'm asking > because like with the m68k port also the MIPS one needs a more finegrained > approach and I suspect there may be other corner cases and I'd rather look > at the most recent version of your series. Otherwise I'll have a look > through your original submission, but it may have to wait until the next > weekend due to my other commitments. > > Maciej That sounds like you do see a need for something like HAS_IOPORT too, correct? Maybe with some input what you need and possibly stripping the LEGACY_PCI option it might make sense to do a new version. Rather than possibly getting in your way could directly work in your input.
On Mon, 14 Feb 2022, Niklas Schnelle wrote: > > While older versions of the driver did have to be explicitly configured > > for MMIO rather than port I/O, a feature added with commit e89a2cfb7d7b > > ("[TC] defxx: TURBOchannel support"), the driver has been improved with > > commit 795e272e5474 ("FDDI: defxx: Implement dynamic CSR I/O address space > > selection") and the selection of the I/O space to use now fully automatic. > > Very interesting and thanks for the input! On s390 we really only have > very few different PCI devices and I can only test another hand full > with my private x86 and ARM systems. Note that for TURBOchannel support, which is likewise MMIO only, the driver has this: #if defined(CONFIG_EISA) || defined(CONFIG_PCI) #define dfx_use_mmio bp->mmio #else #define dfx_use_mmio true #endif so if your proposal to add HAS_IOPORT goes forward it'll be enough if we update the condition to: #if defined(CONFIG_HAS_IOPORT) or maybe even rewrite the entire piece as: #define dfx_use_mmio (!IS_ENABLED(CONFIG_HAS_IOPORT) || bp->mmio) and all the port I/O stuff will be optimised away by the compiler. The only part of the driver that actually cannot do without port I/O is EISA support, which uses the EISA slot port I/O space for BAR accesses even if the actual CSR block has been set up to be decoded in the MMIO space. > > Then what about the other FDDI driver there, SKFP? It's not marked as > > LEGACY_PCI, although it's not selectable anyway due to the dependency of > > FDDI on LEGACY_PCI. > > > > Niklas, what was the criterion for placing the LEGACY_PCI dependency? > > Hmm, honestly I haven't really worked on this recently. There were some > open questions from Bjorn towards Arnd and I was waiting for his reply > but I guess he missed those. I think what you noticed was the main > problem, there wasn't really a clear set of criteria for LEGACY_PCI and > even for HAS_IOPORT we missed some uses if they were not compiled on > s390's allyesconfig due to other dependencies. A dynamic boolean variable might be good having for platforms which may or may not have PCI port I/O available depending on the specific system model in addition to a compile-time constant of HAS_IOPORT. I looked into it briefly in the context of the POWER9 system when I got it back in 2020, but figured out it wasn't straightforward enough and decided I could not afford the time for a proper investigation. > > Also do you plan to post an updated series anytime soon? I'm asking > > because like with the m68k port also the MIPS one needs a more finegrained > > approach and I suspect there may be other corner cases and I'd rather look > > at the most recent version of your series. Otherwise I'll have a look > > through your original submission, but it may have to wait until the next > > weekend due to my other commitments. > > That sounds like you do see a need for something like HAS_IOPORT too, > correct? Maybe with some input what you need and possibly stripping the > LEGACY_PCI option it might make sense to do a new version. Rather than > possibly getting in your way could directly work in your input. Yes, it does seem to me like a good direction, but will surely require some coordination from platform and driver maintainers, as it's not always easy for someone not familiar with a specific piece what the context is (such as with the defxx driver as I noted above). Maciej
Index: linux-macro/arch/arm64/include/asm/Kbuild =================================================================== --- linux-macro.orig/arch/arm64/include/asm/Kbuild +++ linux-macro/arch/arm64/include/asm/Kbuild @@ -3,6 +3,7 @@ generic-y += early_ioremap.h generic-y += mcs_spinlock.h generic-y += qrwlock.h generic-y += qspinlock.h +generic-y += parport.h generic-y += user.h generated-y += cpucaps.h Index: linux-macro/arch/csky/include/asm/Kbuild =================================================================== --- linux-macro.orig/arch/csky/include/asm/Kbuild +++ linux-macro/arch/csky/include/asm/Kbuild @@ -4,5 +4,6 @@ generic-y += extable.h generic-y += gpio.h generic-y += kvm_para.h generic-y += qrwlock.h +generic-y += parport.h generic-y += user.h generic-y += vmlinux.lds.h Index: linux-macro/arch/riscv/include/asm/Kbuild =================================================================== --- linux-macro.orig/arch/riscv/include/asm/Kbuild +++ linux-macro/arch/riscv/include/asm/Kbuild @@ -2,5 +2,6 @@ generic-y += early_ioremap.h generic-y += flat.h generic-y += kvm_para.h +generic-y += parport.h generic-y += user.h generic-y += vmlinux.lds.h Index: linux-macro/arch/s390/include/asm/Kbuild =================================================================== --- linux-macro.orig/arch/s390/include/asm/Kbuild +++ linux-macro/arch/s390/include/asm/Kbuild @@ -8,3 +8,4 @@ generic-y += asm-offsets.h generic-y += export.h generic-y += kvm_types.h generic-y += mcs_spinlock.h +generic-y += parport.h Index: linux-macro/arch/um/include/asm/Kbuild =================================================================== --- linux-macro.orig/arch/um/include/asm/Kbuild +++ linux-macro/arch/um/include/asm/Kbuild @@ -17,6 +17,7 @@ generic-y += mcs_spinlock.h generic-y += mmiowb.h generic-y += module.lds.h generic-y += param.h +generic-y += parport.h generic-y += percpu.h generic-y += preempt.h generic-y += softirq_stack.h Index: linux-macro/arch/xtensa/include/asm/Kbuild =================================================================== --- linux-macro.orig/arch/xtensa/include/asm/Kbuild +++ linux-macro/arch/xtensa/include/asm/Kbuild @@ -4,6 +4,7 @@ generic-y += extable.h generic-y += kvm_para.h generic-y += mcs_spinlock.h generic-y += param.h +generic-y += parport.h generic-y += qrwlock.h generic-y += qspinlock.h generic-y += user.h Index: linux-macro/drivers/parport/Kconfig =================================================================== --- linux-macro.orig/drivers/parport/Kconfig +++ linux-macro/drivers/parport/Kconfig @@ -42,7 +42,7 @@ if PARPORT config PARPORT_PC tristate "PC-style hardware" - depends on ARCH_MIGHT_HAVE_PC_PARPORT + depends on ARCH_MIGHT_HAVE_PC_PARPORT || PCI help You should say Y here if you have a PC-style parallel port. All IBM PC compatible computers and some Alphas have PC-style @@ -77,7 +77,7 @@ config PARPORT_PC_FIFO config PARPORT_PC_SUPERIO bool "SuperIO chipset support" - depends on PARPORT_PC && !PARISC + depends on ARCH_MIGHT_HAVE_PC_PARPORT && PARPORT_PC && !PARISC help Saying Y here enables some probes for Super-IO chipsets in order to find out things like base addresses, IRQ lines and DMA channels. It
Nowadays PC-style parallel ports come in the form of PCI and PCIe option cards and there are some combined parallel/serial option cards as well that we handle in the parport subsystem. There is nothing in particular that would prevent them from being used in any system equipped with PCI or PCIe connectivity, except that we do not permit the PARPORT_PC config option to be selected for platforms for which ARCH_MIGHT_HAVE_PC_PARPORT has not been set for. The only PCI platforms that actually can't make use of PC-style parallel port hardware are those newer PCIe systems that have no support for I/O cycles in the host bridge, required by such parallel ports. An example of such a host bridge is the POWER9 PHB4 device, but it is an exception rather than the norm. Also it is not clear whether the serial port side of devices enabled by the PARPORT_SERIAL option uses port I/O or MMIO. Therefore for PCI platforms PARPORT_PC should generally be available, except for Super I/O solutions, which are either ISA or platform devices. Make the PARPORT_PC option selectable also for PCI systems then, however limit the availability of PARPORT_PC_SUPERIO to platforms that have ARCH_MIGHT_HAVE_PC_PARPORT enabled. Update platforms accordingly for the required <asm/parport.h> header. Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk> --- Hi, I have verified this lightly by booting a kernel with PARPORT_PC and PARPORT_SERIAL enabled on a RISC-V HiFive Unmatched system. While I do have a PCIe parallel port option available that I could use with my RISC-V machine (based on the OxSemi OXPCIe952 chip) it is currently plugged in the wrong system, and both machines are in my remote lab I have currently no visit scheduled to in the near future. For the record the device reports as: PCI parallel port detected: 1415:c118, I/O at 0x1000(0x1008), IRQ 18 parport1: PC-style at 0x1000 (0x1008), irq 18, using FIFO [PCSPP,TRISTATE,COMPAT,EPP,ECP] in the other system. I'll see if I can verify it with the Unmatched at the next opportunity, though it seems like an overkill to me given that a PC-style parallel port is a generic PCIe device. The OXPCIe952 implements a multifunction device, so it doesn't rely on PARPORT_SERIAL. NB platforms to be updated for <asm/parport.h> generation were chosen by the presence of the HAVE_PCI or FORCE_PCI option from ones that do not already have or generate that header. Let me know if I got anything wrong here. Maciej --- arch/arm64/include/asm/Kbuild | 1 + arch/csky/include/asm/Kbuild | 1 + arch/riscv/include/asm/Kbuild | 1 + arch/s390/include/asm/Kbuild | 1 + arch/um/include/asm/Kbuild | 1 + arch/xtensa/include/asm/Kbuild | 1 + drivers/parport/Kconfig | 4 ++-- 7 files changed, 8 insertions(+), 2 deletions(-) linux-parport-pc-pci.diff