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 <[email protected]>
Signed-off-by: Niklas Schnelle <[email protected]>
---
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
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 <[email protected]>
> Signed-off-by: Niklas Schnelle <[email protected]>
> ---
> 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 <[email protected]>
> > Signed-off-by: Niklas Schnelle <[email protected]>
> > ---
> > 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, 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
On Fri, Apr 29, 2022 at 5:37 PM William Breathitt Gray
<[email protected]> 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
> <[email protected]> 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, 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 <[email protected]>
> > > Signed-off-by: Niklas Schnelle <[email protected]>
> > > ---
> > > 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
From: Linus Walleij
> Sent: 01 May 2022 22:56
>
> On Fri, Apr 29, 2022 at 5:37 PM William Breathitt Gray
> <[email protected]> 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
> > <[email protected]> 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
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 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
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