[RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[Niklas Schnelle]

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

Re: [RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 1097 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[Niklas Schnelle]

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.

Re: [RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 1976 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[Linus Walleij]

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

Re: [RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[Niklas Schnelle]

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.

RE: [RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[David Laight]

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)

Re: [RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[William Breathitt Gray]

[-- Attachment #1: Type: text/plain, Size: 2826 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

RE: [RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[Maciej W. Rozycki]

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

RE: [RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[David Laight]

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)

RE: [RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[Maciej W. Rozycki]

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

Re: [RFC v2 10/39] gpio: add HAS_IOPORT dependencies

[Maciej W. Rozycki]

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