| Message ID | 20220429135108.2781579-19-schnelle@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Fri, Apr 29, 2022 at 03:50:16PM +0200, Niklas Schnelle wrote: > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends > not being declared. We thus need to add HAS_IOPORT as dependency for > those drivers using them. > > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > --- > drivers/gpio/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > index 45764ec3b2eb..14e5998ee95c 100644 > --- a/drivers/gpio/Kconfig > +++ b/drivers/gpio/Kconfig > @@ -697,7 +697,7 @@ config GPIO_VR41XX > > config GPIO_VX855 > tristate "VIA VX855/VX875 GPIO" > - depends on (X86 || COMPILE_TEST) && PCI > + depends on (X86 || COMPILE_TEST) && PCI && HAS_IOPORT > select MFD_CORE > select MFD_VX855 > help > -- > 2.32.0 I noticed a number of other GPIO drivers make use of inb()/outb() -- for example the PC104 drivers -- should the respective Kconfigs for those drivers also be adjusted here? William Breathitt Gray
On Fri, 2022-04-29 at 10:32 -0400, William Breathitt Gray wrote: > On Fri, Apr 29, 2022 at 03:50:16PM +0200, Niklas Schnelle wrote: > > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends > > not being declared. We thus need to add HAS_IOPORT as dependency for > > those drivers using them. > > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > > --- > > drivers/gpio/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > > index 45764ec3b2eb..14e5998ee95c 100644 > > --- a/drivers/gpio/Kconfig > > +++ b/drivers/gpio/Kconfig > > @@ -697,7 +697,7 @@ config GPIO_VR41XX > > > > config GPIO_VX855 > > tristate "VIA VX855/VX875 GPIO" > > - depends on (X86 || COMPILE_TEST) && PCI > > + depends on (X86 || COMPILE_TEST) && PCI && HAS_IOPORT > > select MFD_CORE > > select MFD_VX855 > > help > > -- > > 2.32.0 > > I noticed a number of other GPIO drivers make use of inb()/outb() -- for > example the PC104 drivers -- should the respective Kconfigs for those > drivers also be adjusted here? > > William Breathitt Gray Good question. As far as I can see most (all?) of these have "select ISA_BUS_API" which is "def_bool ISA". Now "config ISA" seems to currently be repeated in architectures and doesn't have an explicit HAS_IOPORT dependency (it maybe should have one). But it does only make sense on architectures with HAS_IOPORT set.
On Fri, Apr 29, 2022 at 04:46:00PM +0200, Niklas Schnelle wrote: > On Fri, 2022-04-29 at 10:32 -0400, William Breathitt Gray wrote: > > On Fri, Apr 29, 2022 at 03:50:16PM +0200, Niklas Schnelle wrote: > > > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends > > > not being declared. We thus need to add HAS_IOPORT as dependency for > > > those drivers using them. > > > > > > Co-developed-by: Arnd Bergmann <arnd@kernel.org> > > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> > > > --- > > > drivers/gpio/Kconfig | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig > > > index 45764ec3b2eb..14e5998ee95c 100644 > > > --- a/drivers/gpio/Kconfig > > > +++ b/drivers/gpio/Kconfig > > > @@ -697,7 +697,7 @@ config GPIO_VR41XX > > > > > > config GPIO_VX855 > > > tristate "VIA VX855/VX875 GPIO" > > > - depends on (X86 || COMPILE_TEST) && PCI > > > + depends on (X86 || COMPILE_TEST) && PCI && HAS_IOPORT > > > select MFD_CORE > > > select MFD_VX855 > > > help > > > -- > > > 2.32.0 > > > > I noticed a number of other GPIO drivers make use of inb()/outb() -- for > > example the PC104 drivers -- should the respective Kconfigs for those > > drivers also be adjusted here? > > > > William Breathitt Gray > > Good question. As far as I can see most (all?) of these have "select > ISA_BUS_API" which is "def_bool ISA". Now "config ISA" seems to > currently be repeated in architectures and doesn't have an explicit > HAS_IOPORT dependency (it maybe should have one). But it does only make > sense on architectures with HAS_IOPORT set. There is such a thing as ISA DMA, but you'll still need to initialize the device via the IO Port bus first, so perhaps setting HAS_IOPORT for "config ISA" is the right thing to do: all ISA devices are expected to communicate in some way via ioport. William Breathitt Gray
On Fri, Apr 29, 2022 at 5:37 PM William Breathitt Gray <william.gray@linaro.org> wrote: > On Fri, Apr 29, 2022 at 04:46:00PM +0200, Niklas Schnelle wrote: > > Good question. As far as I can see most (all?) of these have "select > > ISA_BUS_API" which is "def_bool ISA". Now "config ISA" seems to > > currently be repeated in architectures and doesn't have an explicit > > HAS_IOPORT dependency (it maybe should have one). But it does only make > > sense on architectures with HAS_IOPORT set. > > There is such a thing as ISA DMA, but you'll still need to initialize > the device via the IO Port bus first, so perhaps setting HAS_IOPORT for > "config ISA" is the right thing to do: all ISA devices are expected to > communicate in some way via ioport. Adding that dependency seems like the right solution to me. Yours, Linus Walleij
On Sun, 2022-05-01 at 23:55 +0200, Linus Walleij wrote: > On Fri, Apr 29, 2022 at 5:37 PM William Breathitt Gray > <william.gray@linaro.org> wrote: > > On Fri, Apr 29, 2022 at 04:46:00PM +0200, Niklas Schnelle wrote: > > > Good question. As far as I can see most (all?) of these have "select > > > ISA_BUS_API" which is "def_bool ISA". Now "config ISA" seems to > > > currently be repeated in architectures and doesn't have an explicit > > > HAS_IOPORT dependency (it maybe should have one). But it does only make > > > sense on architectures with HAS_IOPORT set. > > > > There is such a thing as ISA DMA, but you'll still need to initialize > > the device via the IO Port bus first, so perhaps setting HAS_IOPORT for > > "config ISA" is the right thing to do: all ISA devices are expected to > > communicate in some way via ioport. > > Adding that dependency seems like the right solution to me. > > Yours, > Linus Walleij One thing I forgot to mention, config HAS_IOPORT does have a "def_bool ISA" but yes I agree an explicit "depends on HAS_IOPORT" for ISA seems more logical. I also haven't found issues trying this out locally so far.
On Fri, 29 Apr 2022, William Breathitt Gray wrote: > > Good question. As far as I can see most (all?) of these have "select > > ISA_BUS_API" which is "def_bool ISA". Now "config ISA" seems to > > currently be repeated in architectures and doesn't have an explicit > > HAS_IOPORT dependency (it maybe should have one). But it does only make > > sense on architectures with HAS_IOPORT set. > > There is such a thing as ISA DMA, but you'll still need to initialize > the device via the IO Port bus first, so perhaps setting HAS_IOPORT for > "config ISA" is the right thing to do: all ISA devices are expected to > communicate in some way via ioport. Strictly speaking you can make an ISA device that only does MMIO (and I believe in the early PC days there used to be ISA memory expansion cards along with the EMS standard) which is also why the host memory area in the 15-16MiB range, the top 1MiB addressable on 16-bit ISA, can be excluded from decoding to DRAM and accesses made there forwarded to ISA in I believe all chipsets that provide actual ISA bus circuitry (rather than just a degenerate form like LPC). That's an exception rather than the rule though, nearly all ISA devices do decode in the port I/O space. After all I/O is what the port I/O address space has been invented for. FWIW, Maciej
From: Linus Walleij > Sent: 01 May 2022 22:56 > > On Fri, Apr 29, 2022 at 5:37 PM William Breathitt Gray > <william.gray@linaro.org> wrote: > > On Fri, Apr 29, 2022 at 04:46:00PM +0200, Niklas Schnelle wrote: > > > > Good question. As far as I can see most (all?) of these have "select > > > ISA_BUS_API" which is "def_bool ISA". Now "config ISA" seems to > > > currently be repeated in architectures and doesn't have an explicit > > > HAS_IOPORT dependency (it maybe should have one). But it does only make > > > sense on architectures with HAS_IOPORT set. > > > > There is such a thing as ISA DMA, but you'll still need to initialize > > the device via the IO Port bus first, so perhaps setting HAS_IOPORT for > > "config ISA" is the right thing to do: all ISA devices are expected to > > communicate in some way via ioport. > > Adding that dependency seems like the right solution to me. I think it all depends on what HAS_IOPORT is meant to mean and how portable kernel binaries need to be. x86 is (probably) the only architecture that actually has 'in' and 'out' instructions - but that doesn't mean that some other cpu (and I mean cpu+pcb not architecture) have the ability to generate 'IO' bus cycles on a specific physical bus. While the obvious case is a physical address window that generates PCI(e) IO cycles from normal memory cycles it isn't the only one. I've used sparc cpu systems that have pcmcia card slots. These are pretty much ISA and the drivers might expect to access port 0x300 (etc) - certainly that would be right on x86. In this case is isn't so much that the ISA_BUS depends on support for in/out but that presence of the ISA bus provides the required in/out support. Now, maybe, the drivers should be using some ioremap variant and then calling ioread8() rather than directly calling inb(). But that seems orthogonal to this changeset. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Tue, May 03, 2022 at 01:08:04PM +0000, David Laight wrote: > From: Linus Walleij > > Sent: 01 May 2022 22:56 > > > > On Fri, Apr 29, 2022 at 5:37 PM William Breathitt Gray > > <william.gray@linaro.org> wrote: > > > On Fri, Apr 29, 2022 at 04:46:00PM +0200, Niklas Schnelle wrote: > > > > > > Good question. As far as I can see most (all?) of these have "select > > > > ISA_BUS_API" which is "def_bool ISA". Now "config ISA" seems to > > > > currently be repeated in architectures and doesn't have an explicit > > > > HAS_IOPORT dependency (it maybe should have one). But it does only make > > > > sense on architectures with HAS_IOPORT set. > > > > > > There is such a thing as ISA DMA, but you'll still need to initialize > > > the device via the IO Port bus first, so perhaps setting HAS_IOPORT for > > > "config ISA" is the right thing to do: all ISA devices are expected to > > > communicate in some way via ioport. > > > > Adding that dependency seems like the right solution to me. > > I think it all depends on what HAS_IOPORT is meant to mean and > how portable kernel binaries need to be. > > x86 is (probably) the only architecture that actually has 'in' > and 'out' instructions - but that doesn't mean that some other > cpu (and I mean cpu+pcb not architecture) have the ability to > generate 'IO' bus cycles on a specific physical bus. > > While the obvious case is a physical address window that generates > PCI(e) IO cycles from normal memory cycles it isn't the only one. > > I've used sparc cpu systems that have pcmcia card slots. > These are pretty much ISA and the drivers might expect to > access port 0x300 (etc) - certainly that would be right on x86. > > In this case is isn't so much that the ISA_BUS depends on support > for in/out but that presence of the ISA bus provides the required > in/out support. That's true, it does seem somewhat backwards to have a depends on line when the bus is really just providing the support for devices that want to use it rather than requiring it. Do you think a HAVE_IOPORT line should be added independently for each driver instead of adding it to ISA_BUS? > Now, maybe, the drivers should be using some ioremap variant and > then calling ioread8() rather than directly calling inb(). > But that seems orthogonal to this changeset. > > David > > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) Using ioremap() does have the benefit of making it easier to reuse the code for some of these PC104 drivers with their PCI device variants; the ioread8() calls and such can stay the same and we just initialize to the proper address during probe. I plan to look into this in the future then. William Breathitt Gray
On Tue, 3 May 2022, David Laight wrote: > > > There is such a thing as ISA DMA, but you'll still need to initialize > > > the device via the IO Port bus first, so perhaps setting HAS_IOPORT for > > > "config ISA" is the right thing to do: all ISA devices are expected to > > > communicate in some way via ioport. > > > > Adding that dependency seems like the right solution to me. > > I think it all depends on what HAS_IOPORT is meant to mean and > how portable kernel binaries need to be. > > x86 is (probably) the only architecture that actually has 'in' > and 'out' instructions - but that doesn't mean that some other > cpu (and I mean cpu+pcb not architecture) have the ability to > generate 'IO' bus cycles on a specific physical bus. I am fairly sure IA-64 has some form of IN/OUT machine instructions too. > While the obvious case is a physical address window that generates > PCI(e) IO cycles from normal memory cycles it isn't the only one. > > I've used sparc cpu systems that have pcmcia card slots. > These are pretty much ISA and the drivers might expect to > access port 0x300 (etc) - certainly that would be right on x86. > > In this case is isn't so much that the ISA_BUS depends on support > for in/out but that presence of the ISA bus provides the required > in/out support. Well, one can implement a pluggable PCI/e expansion card with a PCI-ISA bridge on it and a backplane to plug ISA cards into. Without support for issuing I/O cycles to PCI from the host however you won't be able to make use of the ISA backplane except maybe for some ancient ISA memory cards. So logically I think CONFIG_ISA should depend on CONFIG_HAS_IOPORT and CONFIG_HAS_IOPORT ought to be selected by platform configurations. ISTR there was a company that manufactured a USB-ISA option (providing an external ISA backplane). We never supported it, but in principle if we wanted to, then it would be the USB-ISA device's driver config option that CONFIG_ISA would additionally depend on as an alternative. That wouldn't enable CONFIG_HAS_IOPORT though because the presence of this particular USB-ISA device would not itself permit the use of I/O cycles with any PCI/e buses a machine might independently have, so devices for PCI/e options that require port I/O shouldn't be made available at the same time. I think that company might have actually manufactured a similar PCI-ISA option as well, but that I suppose did rely on support for I/O cycles on PCI. Early 2000s BTW. Maciej
From: Maciej W. Rozycki > Sent: 04 May 2022 12:47 > > On Tue, 3 May 2022, David Laight wrote: > > > > > There is such a thing as ISA DMA, but you'll still need to initialize > > > > the device via the IO Port bus first, so perhaps setting HAS_IOPORT for > > > > "config ISA" is the right thing to do: all ISA devices are expected to > > > > communicate in some way via ioport. > > > > > > Adding that dependency seems like the right solution to me. > > > > I think it all depends on what HAS_IOPORT is meant to mean and > > how portable kernel binaries need to be. > > > > x86 is (probably) the only architecture that actually has 'in' > > and 'out' instructions - but that doesn't mean that some other > > cpu (and I mean cpu+pcb not architecture) have the ability to > > generate 'IO' bus cycles on a specific physical bus. > > I am fairly sure IA-64 has some form of IN/OUT machine instructions too. > > > While the obvious case is a physical address window that generates > > PCI(e) IO cycles from normal memory cycles it isn't the only one. > > > > I've used sparc cpu systems that have pcmcia card slots. > > These are pretty much ISA and the drivers might expect to > > access port 0x300 (etc) - certainly that would be right on x86. > > > > In this case is isn't so much that the ISA_BUS depends on support > > for in/out but that presence of the ISA bus provides the required > > in/out support. > > Well, one can implement a pluggable PCI/e expansion card with a PCI-ISA > bridge on it and a backplane to plug ISA cards into. Without support for > issuing I/O cycles to PCI from the host however you won't be able to make > use of the ISA backplane except maybe for some ancient ISA memory cards. > So logically I think CONFIG_ISA should depend on CONFIG_HAS_IOPORT and > CONFIG_HAS_IOPORT ought to be selected by platform configurations. But generating a PCI(e) I/O cycle doesn't need the cpu to be able to generate an I/O cycle on its local bus interface. All that required is for the PCI(e) host bridge to determine that it needs to relevant kind of cycle on the target bus. This can easily be based on the physical address. Many years ago I used a system with the strongarm SA1100/1101 pair. (Not running Linux, but I think that it could have - slowly.) Two (adjacent?) areas of the physical address map generated memory and I/O cycles to a pair of pcmcia slots. What you end up with is definitely ISA, but ARM definitely doesn't have in/out instructions. Now, while this is rather hypothetical, a 'generic' arm kernel running on that hardware would be able to support drivers that expect an ISA bus. But on different hardware the same generic kernel would have nowhere to 'attach' the same drivers - but they could still be part of the kernel (maybe as modules). What you should probably be doing is (outside of 'platform' code) change the drivers to use ioread8() instead of inb(). Then adding in the required calls to get the correct 'token' to pass to ioread8() to perform an I/O cycle on the correct target bus. It is really the attachment of the driver that can't succeed, not the compilation. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
On Wed, 4 May 2022, David Laight wrote: > > Well, one can implement a pluggable PCI/e expansion card with a PCI-ISA > > bridge on it and a backplane to plug ISA cards into. Without support for > > issuing I/O cycles to PCI from the host however you won't be able to make > > use of the ISA backplane except maybe for some ancient ISA memory cards. > > So logically I think CONFIG_ISA should depend on CONFIG_HAS_IOPORT and > > CONFIG_HAS_IOPORT ought to be selected by platform configurations. > > But generating a PCI(e) I/O cycle doesn't need the cpu to be able to > generate an I/O cycle on its local bus interface. > All that required is for the PCI(e) host bridge to determine that it > needs to relevant kind of cycle on the target bus. > This can easily be based on the physical address. Sure, you can encode address spaces however you like (there are no special machine instructions either for PCI/e configuration space access that I would know of in any CPU architecture), but the host bridge must be willing to issue those PCI/e I/O cycles in the first place (see my other message on POWER9 in this thread). > What you should probably be doing is (outside of 'platform' code) > change the drivers to use ioread8() instead of inb(). > Then adding in the required calls to get the correct 'token' to > pass to ioread8() to perform an I/O cycle on the correct target bus. Yes, probably. > It is really the attachment of the driver that can't succeed, not the > compilation. Except it makes no sense to offer those drivers for platforms known not to provide for port I/O on PCI/e. Maciej
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 45764ec3b2eb..14e5998ee95c 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -697,7 +697,7 @@ config GPIO_VR41XX config GPIO_VX855 tristate "VIA VX855/VX875 GPIO" - depends on (X86 || COMPILE_TEST) && PCI + depends on (X86 || COMPILE_TEST) && PCI && HAS_IOPORT select MFD_CORE select MFD_VX855 help
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends not being declared. We thus need to add HAS_IOPORT as dependency for those drivers using them. Co-developed-by: Arnd Bergmann <arnd@kernel.org> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> --- drivers/gpio/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)