On Thu, 2022-01-06 at 17:41 +0000, John Garry wrote: > On 05/01/2022 19:47, Bjorn Helgaas wrote: > > > > > > ok if the PCI maintainers decide otherwise. > > > > > I don't really like the "LEGACY_PCI" Kconfig option. "Legacy" just > > > > > means something old and out of favor; it doesn't say*what* that > > > > > something is. > > > > > > > > > > I think you're specifically interested in I/O port space usage, and it > > > > > seems that you want all PCI drivers that*only* use I/O port space to > > > > > depend on LEGACY_PCI? Drivers that can use either I/O or memory > > > > > space or both would not depend on LEGACY_PCI? This seems a little > > > > > murky and error-prone. > > > > I'd like to hear Arnd's opinion on this but you're the PCI maintainer > > > > so of course your buy-in would be quite important for such an option. > > I'd like to hear Arnd's opinion, too. If we do add LEGACY_PCI, I > > think we need a clear guide for when to use it, e.g., "a PCI driver > > that uses inb() must depend on LEGACY_PCI" or whatever it is. > > > > I must be missing something because I don't see what we gain from > > this. We have PCI drivers, e.g., megaraid [1], for devices that have > > either MEM or I/O BARs. I think we want to build drivers like that on > > any arch that supports PCI. > > > > If the arch doesn't support I/O port space, devices that only have I/O > > BARs won't work, of course, and hopefully the PCI core and driver can > > figure that out and gracefully fail the probe. > > > > But that same driver should still work with devices that have MEM > > BARs. If inb() isn't always present, I guess we could litter these > > drivers with #ifdefs, but that would be pretty ugly. I think this is the big question here. If we do go with a compile-time solution as requested by Linus we will either get a lot of #ifdeffery, coarse driver dependencies or as proposed by Alan Stern for the USB #ifdefs might end up turning inb() into a compile-time nop. The originally proposed change that returned ~0 from inb() and printed a warning clearly is the simpler change and sure we could also drop the warning. I'm honestly torn, I do agree with Linus that we shouldn't have run-time things that we know at compile-time will not work but I also dislike all the #ifdeffery a compile-time solution requires. Sadly C really doesn't give us any better tools here. Also I 100% agree with you Bjorn how likely it is to see a device on a platform really shouldn't matter. Without going into details, on s390 we have already beneffited from PCI drivers working with 0 changes to support devices we previously didn't have on the platform or anticipated we would get in the future. Consequently drivers that could work in principle should be built. > > > > There were some ifdefs added to the 8250 drivers in Arnd's original > patch [0], but it does not seem included here. > > Niklas, what happened to the 8250 and the other driver changes? I missed it during the rebase, likely because the changed files compile depend on !S390 via config SERIAL_8250 and thus didn't cause any errors for my allyesconfig. That !S390 dependency is of course not really what we want if the driver can use MEM BARs.
