Message-ID: <822ad0da702f0953b7aae1febd2c4dfcc4707864.camel@linux.ibm.com>
Date:   Mon, 10 Jan 2022 10:34:34 +0100
From:   Niklas Schnelle <schnelle@...ux.ibm.com>
To:     John Garry <john.garry@...wei.com>,
        Bjorn Helgaas <helgaas@...nel.org>
Cc:     Mauro Carvalho Chehab <mchehab@...nel.org>,
        Arnd Bergmann <arnd@...db.de>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Ettore Chimenti <ek5.chimenti@...il.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Arnd Bergmann <arnd@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Nick Hu <nickhu@...estech.com>,
        Greentime Hu <green.hu@...il.com>,
        Vincent Chen <deanbo422@...il.com>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>, Guo Ren <guoren@...nel.org>,
        Damien Le Moal <damien.lemoal@...nsource.wdc.com>,
        Ian Abbott <abbotti@....co.uk>,
        H Hartley Sweeten <hsweeten@...ionengravers.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Bartosz Golaszewski <brgl@...ev.pl>,
        Jean Delvare <jdelvare@...e.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Karsten Keil <isdn@...ux-pingi.de>,
        Sathya Prakash <sathya.prakash@...adcom.com>,
        Sreekanth Reddy <sreekanth.reddy@...adcom.com>,
        Suganath Prabu Subramani 
        <suganath-prabu.subramani@...adcom.com>,
        Michael Grzeschik <m.grzeschik@...gutronix.de>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        Tony Nguyen <anthony.l.nguyen@...el.com>,
        Kalle Valo <kvalo@...nel.org>, Jouni Malinen <j@...fi>,
        "James E.J. Bottomley" <jejb@...ux.ibm.com>,
        "Martin K. Petersen" <martin.petersen@...cle.com>,
        Hannes Reinecke <hare@...e.com>,
        Kashyap Desai <kashyap.desai@...adcom.com>,
        Sumit Saxena <sumit.saxena@...adcom.com>,
        Shivasharan S <shivasharan.srikanteshwara@...adcom.com>,
        Nilesh Javali <njavali@...vell.com>,
        GR-QLogic-Storage-Upstream@...vell.com,
        Mark Brown <broonie@...nel.org>,
        Sudip Mukherjee <sudipm.mukherjee@...il.com>,
        Teddy Wang <teddy.wang@...iconmotion.com>,
        Forest Bond <forest@...ttletooquiet.net>,
        Jiri Slaby <jirislaby@...nel.org>,
        Wim Van Sebroeck <wim@...ux-watchdog.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>, linux-kernel@...r.kernel.org,
        linux-arch@...r.kernel.org, linux-pci@...r.kernel.org,
        linux-riscv@...ts.infradead.org, linux-csky@...r.kernel.org,
        linux-ide@...r.kernel.org, linux-gpio@...r.kernel.org,
        linux-hwmon@...r.kernel.org, linux-i2c@...r.kernel.org,
        linux-input@...r.kernel.org, netdev@...r.kernel.org,
        linux-media@...r.kernel.org, MPT-FusionLinux.pdl@...adcom.com,
        linux-scsi@...r.kernel.org, intel-wired-lan@...ts.osuosl.org,
        linux-wireless@...r.kernel.org, megaraidlinux.pdl@...adcom.com,
        linux-spi@...r.kernel.org, linux-fbdev@...r.kernel.org,
        linux-serial@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        linux-watchdog@...r.kernel.org
Subject: Re: [RFC 01/32] Kconfig: introduce and depend on LEGACY_PCI

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.

Powered by blists - more mailing lists