diff mbox series

[RFC,v2,25/39] pcmcia: add HAS_IOPORT dependencies

Message ID 20220429135108.2781579-44-schnelle@linux.ibm.com
State New
Headers show
Series None | expand

Commit Message

Niklas Schnelle April 29, 2022, 1:50 p.m. UTC 
In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. PCMCIA devices are either LEGACY_PCI devices
which implies HAS_IOPORT or require HAS_IOPORT.

Acked-by: Dominik Brodowski <linux@dominikbrodowski.net>
Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 drivers/pcmcia/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bjorn Helgaas May 3, 2022, 11:38 p.m. UTC | #1 
On Fri, Apr 29, 2022 at 03:50:41PM +0200, Niklas Schnelle wrote:
> In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> not being declared. PCMCIA devices are either LEGACY_PCI devices
> which implies HAS_IOPORT or require HAS_IOPORT.
> 
> Acked-by: Dominik Brodowski <linux@dominikbrodowski.net>
> Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
>  drivers/pcmcia/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pcmcia/Kconfig b/drivers/pcmcia/Kconfig
> index 2ce261cfff8e..32b5cd324c58 100644
> --- a/drivers/pcmcia/Kconfig
> +++ b/drivers/pcmcia/Kconfig
> @@ -5,7 +5,7 @@
>  
>  menuconfig PCCARD
>  	tristate "PCCard (PCMCIA/CardBus) support"
> -	depends on !UML
> +	depends on HAS_IOPORT

I don't know much about PC Card.  Is there a requirement that these
devices must use I/O port space?  If so, can you include a spec
reference in the commit log?

I do see the PC Card spec, r8.1, sec 5.5.4.2.2 says:

  All CardBus PC Card adapters must support either memory-mapped I/O
  or both memory-mapped I/O and I/O space. The selection will depend
  largely on the system architecture the adapter is intended to be
  used in. The requirement to also support memory-mapped I/O, if I/O
  space is supported, is driven by the potential emergence of
  memory-mapped I/O only cards. Supporting both modes may also
  position the adapter to be sold into multiple system architectures.

which sounds like I/O space is optional.

>  	help
>  	  Say Y here if you want to attach PCMCIA- or PC-cards to your Linux
>  	  computer.  These are credit-card size devices such as network cards,
> -- 
> 2.32.0
>
Arnd Bergmann May 4, 2022, 10:33 a.m. UTC | #2 
On Wed, May 4, 2022 at 1:38 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Fri, Apr 29, 2022 at 03:50:41PM +0200, Niklas Schnelle wrote:
> > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > not being declared. PCMCIA devices are either LEGACY_PCI devices
> > which implies HAS_IOPORT or require HAS_IOPORT.
> >
> > Acked-by: Dominik Brodowski <linux@dominikbrodowski.net>
> > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > ---
> >  drivers/pcmcia/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pcmcia/Kconfig b/drivers/pcmcia/Kconfig
> > index 2ce261cfff8e..32b5cd324c58 100644
> > --- a/drivers/pcmcia/Kconfig
> > +++ b/drivers/pcmcia/Kconfig
> > @@ -5,7 +5,7 @@
> >
> >  menuconfig PCCARD
> >       tristate "PCCard (PCMCIA/CardBus) support"
> > -     depends on !UML
> > +     depends on HAS_IOPORT
>
> I don't know much about PC Card.  Is there a requirement that these
> devices must use I/O port space?  If so, can you include a spec
> reference in the commit log?

I think for PCMCIA devices, the dependency makes sense because
all device drivers for PCMCIA devices need I/O ports.

For cardbus, we can go either way, I don't see any reference to
I/O ports in yenta_socket.c or the pccard core, so it would build
fine with or without I/O ports.

> I do see the PC Card spec, r8.1, sec 5.5.4.2.2 says:
>
>   All CardBus PC Card adapters must support either memory-mapped I/O
>   or both memory-mapped I/O and I/O space. The selection will depend
>   largely on the system architecture the adapter is intended to be
>   used in. The requirement to also support memory-mapped I/O, if I/O
>   space is supported, is driven by the potential emergence of
>   memory-mapped I/O only cards. Supporting both modes may also
>   position the adapter to be sold into multiple system architectures.
>
> which sounds like I/O space is optional.

An earlier version of the patch series had a separate
CONFIG_LEGACY_PCI that required CONFIG_HAS_IOPORT
here, which I think made this clearer:

Almost all architectures that support CONFIG_PCI also provide
HAS_IOPORT today (at least at compile time, if not at runtime),
with s390 as a notable exception. Any machines that have legacy
PCI device support will also have I/O ports because a lot of
legacy PCI cards used it, and any machine with a pc-card slot
should also support legacy PCI devices.

If we get new architectures without I/O space in the future, they
would certainly not care about supporting old cardbus devices.

        Arnd
Maciej W. Rozycki May 4, 2022, 12:38 p.m. UTC | #3 
On Wed, 4 May 2022, Arnd Bergmann wrote:

> Almost all architectures that support CONFIG_PCI also provide
> HAS_IOPORT today (at least at compile time, if not at runtime),
> with s390 as a notable exception. Any machines that have legacy
> PCI device support will also have I/O ports because a lot of
> legacy PCI cards used it, and any machine with a pc-card slot
> should also support legacy PCI devices.
> 
> If we get new architectures without I/O space in the future, they
> would certainly not care about supporting old cardbus devices.

 POWER9 is another architecture with no port I/O space[1]:

Table 3-2. PCIe TLP command summary
-----------+-----------------------------+-------------------------------
  Class    |           Type Name         |           Notes
===========+=============================+===============================
Completion | Completion without Data     | For PCI CFG Writes (nonposted)
           |                             | or for error responses.
Completion | Completion with Data        | CI load responses.
Nonposted  | Configuration Read Request  | Outbound only.
Nonposted  | Configuration Write Request | Outbound only.
Posted     | Message Request             | Inbound only.
Nonposted  | Memory Read Request         |
Posted     | Memory Write Request        |
===========+=============================+===============================
 1. All other valid PCIe command types are ignored and dropped.
 2. Invalid PCIe request command types will result in a completion
    response of Unsupported Request.
------------------------------------+------------------------------------

that we do support -- I have such a system.  And I guess POWER10 is the 
same, as will be all future architecture updates.

References:

[1] "Power Systems Host Bridge 4 (PHB4) Specification", Version 1.0, 
    International Business Machines Corporation, 27 July 2018, Section 3.1 
    "PHB4 Command Details", p.29

  Maciej
Arnd Bergmann May 4, 2022, 2:07 p.m. UTC | #4 
On Wed, May 4, 2022 at 2:38 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Wed, 4 May 2022, Arnd Bergmann wrote:
>
> > Almost all architectures that support CONFIG_PCI also provide
> > HAS_IOPORT today (at least at compile time, if not at runtime),
> > with s390 as a notable exception. Any machines that have legacy
> > PCI device support will also have I/O ports because a lot of
> > legacy PCI cards used it, and any machine with a pc-card slot
> > should also support legacy PCI devices.
> >
> > If we get new architectures without I/O space in the future, they
> > would certainly not care about supporting old cardbus devices.
>
>  POWER9 is another architecture with no port I/O space[1]:

POWER9 is just an implementation of the power architecture
that has a particular PCI host bridge. I would assume that
arch/powerpc/ would continue to set HAS_IOPORT because
it knows how to access I/O ports at compile-time.

If a particular host bridge does not declare an I/O port range
in its DT, then of course it won't be accessible, but that is
different from architectures that have no concept of I/O ports.

         Arnd
Maciej W. Rozycki May 4, 2022, 2:24 p.m. UTC | #5 
On Wed, 4 May 2022, Arnd Bergmann wrote:

> >  POWER9 is another architecture with no port I/O space[1]:
> 
> POWER9 is just an implementation of the power architecture
> that has a particular PCI host bridge. I would assume that
> arch/powerpc/ would continue to set HAS_IOPORT because
> it knows how to access I/O ports at compile-time.

 Well, yes, except I would expect POWER9_CPU (and any higher versions we 
eventually get) to clear HAS_IOPORT.  Generic configurations (GENERIC_CPU) 
would set HAS_IOPORT of course, as would any lower architecture variants 
that do or may support port I/O (it's not clear to me if there are any 
that do not).  Ideally a generic configuration would not issue accesses to 
random MMIO locations for port I/O accesses via `inb'/`outb', etc. for 
systems that do not support port I/O (which it now does, or at least used 
to until recently).

  Maciej
David Laight May 4, 2022, 2:44 p.m. UTC | #6 
From: Arnd Bergmann
> Sent: 04 May 2022 11:33
> 
> On Wed, May 4, 2022 at 1:38 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Fri, Apr 29, 2022 at 03:50:41PM +0200, Niklas Schnelle wrote:
> > > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > > not being declared. PCMCIA devices are either LEGACY_PCI devices
> > > which implies HAS_IOPORT or require HAS_IOPORT.
> > >
> > > Acked-by: Dominik Brodowski <linux@dominikbrodowski.net>
> > > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > > ---
> > >  drivers/pcmcia/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pcmcia/Kconfig b/drivers/pcmcia/Kconfig
> > > index 2ce261cfff8e..32b5cd324c58 100644
> > > --- a/drivers/pcmcia/Kconfig
> > > +++ b/drivers/pcmcia/Kconfig
> > > @@ -5,7 +5,7 @@
> > >
> > >  menuconfig PCCARD
> > >       tristate "PCCard (PCMCIA/CardBus) support"
> > > -     depends on !UML
> > > +     depends on HAS_IOPORT
> >
> > I don't know much about PC Card.  Is there a requirement that these
> > devices must use I/O port space?  If so, can you include a spec
> > reference in the commit log?
> 
> I think for PCMCIA devices, the dependency makes sense because
> all device drivers for PCMCIA devices need I/O ports.

ISTR some PCMCIA linear non-volatile memory cards that only
supported memory accesses.
I'm pretty sure some didn't even decode config space properly.
(I bet none of them still work after 25 years though.)

I've used I/O addresses on pcmcia cards from sparc and ARM cpu.

> For cardbus, we can go either way, I don't see any reference to
> I/O ports in yenta_socket.c or the pccard core, so it would build
> fine with or without I/O ports.

cardbus is basically PCI.
I think you can find cardbus cards that have a pci bridge and a cable
link to an expansion chassis into which you can insert standard PCI cards.
If you are really lucky the initial enumeration allocates the
'high field' bus numbers, io addresses and plenty of memory
space to the bridge - otherwise you lose.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Niklas Schnelle May 4, 2022, 2:59 p.m. UTC | #7 
On Wed, 2022-05-04 at 12:33 +0200, Arnd Bergmann wrote:
> On Wed, May 4, 2022 at 1:38 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Fri, Apr 29, 2022 at 03:50:41PM +0200, Niklas Schnelle wrote:
> > > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
> > > not being declared. PCMCIA devices are either LEGACY_PCI devices
> > > which implies HAS_IOPORT or require HAS_IOPORT.
> > > 
> > > Acked-by: Dominik Brodowski <linux@dominikbrodowski.net>
> > > Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> > > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > > ---
> > >  drivers/pcmcia/Kconfig | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/pcmcia/Kconfig b/drivers/pcmcia/Kconfig
> > > index 2ce261cfff8e..32b5cd324c58 100644
> > > --- a/drivers/pcmcia/Kconfig
> > > +++ b/drivers/pcmcia/Kconfig
> > > @@ -5,7 +5,7 @@
> > > 
> > >  menuconfig PCCARD
> > >       tristate "PCCard (PCMCIA/CardBus) support"
> > > -     depends on !UML
> > > +     depends on HAS_IOPORT
> > 
> > I don't know much about PC Card.  Is there a requirement that these
> > devices must use I/O port space?  If so, can you include a spec
> > reference in the commit log?
> 
> I think for PCMCIA devices, the dependency makes sense because
> all device drivers for PCMCIA devices need I/O ports.
> 
> For cardbus, we can go either way, I don't see any reference to
> I/O ports in yenta_socket.c or the pccard core, so it would build
> fine with or without I/O ports.
> 
> > I do see the PC Card spec, r8.1, sec 5.5.4.2.2 says:
> > 
> >   All CardBus PC Card adapters must support either memory-mapped I/O
> >   or both memory-mapped I/O and I/O space. The selection will depend
> >   largely on the system architecture the adapter is intended to be
> >   used in. The requirement to also support memory-mapped I/O, if I/O
> >   space is supported, is driven by the potential emergence of
> >   memory-mapped I/O only cards. Supporting both modes may also
> >   position the adapter to be sold into multiple system architectures.
> > 
> > which sounds like I/O space is optional.
> 
> An earlier version of the patch series had a separate
> CONFIG_LEGACY_PCI that required CONFIG_HAS_IOPORT
> here, which I think made this clearer:
> 
> Almost all architectures that support CONFIG_PCI also provide
> HAS_IOPORT today (at least at compile time, if not at runtime),
> with s390 as a notable exception. Any machines that have legacy
> PCI device support will also have I/O ports because a lot of
> legacy PCI cards used it, and any machine with a pc-card slot
> should also support legacy PCI devices.
> 
> If we get new architectures without I/O space in the future, they
> would certainly not care about supporting old cardbus devices.
> 
>         Arnd

I tested removing the HAS_IOPORT dependency on PCCARD thus ungating
PCMCIA and CARDBUS. This then requires about 30 additional HAS_IOPORT
dependencies to build my s390 allyesconfig.

The only exceptions I found that depends on PCMCIA and isn't otherwise
dependend on ISA or a specific platform are USB_SL811_CS and
PCMCIA_RAYCS (Aviator/Raytheon 2.4GHz wireless).

As for CARDBUS the only "depends on CARDBUS" is in
drivers/net/ethernet/dec/tulip/Kconfig. Now I tested with allyesconfig
which also sets CONFIG_TULIP_MMIO and with that the drivers there did
compile. I guess one should then have "select TULIP_MMIO if
!HAS_IOPORT" in the config NET_TULIP.

So not sure, it seems unlikely we're going to see any new
PCMCIA/CardBus device drivers added and that's a lot of churn for
compile testing so few drivers but in theory it looks like it is
possible to have non-I/O port PCMCIA/CardBus.

Thanks,
Niklas
Bjorn Helgaas May 4, 2022, 5:22 p.m. UTC | #8 
On Wed, May 04, 2022 at 03:24:39PM +0100, Maciej W. Rozycki wrote:
> On Wed, 4 May 2022, Arnd Bergmann wrote:
> 
> > >  POWER9 is another architecture with no port I/O space[1]:
> > 
> > POWER9 is just an implementation of the power architecture
> > that has a particular PCI host bridge. I would assume that
> > arch/powerpc/ would continue to set HAS_IOPORT because
> > it knows how to access I/O ports at compile-time.
> 
>  Well, yes, except I would expect POWER9_CPU (and any higher versions we 
> eventually get) to clear HAS_IOPORT.  Generic configurations (GENERIC_CPU) 
> would set HAS_IOPORT of course, as would any lower architecture variants 
> that do or may support port I/O (it's not clear to me if there are any 
> that do not).  Ideally a generic configuration would not issue accesses to 
> random MMIO locations for port I/O accesses via `inb'/`outb', etc. for 
> systems that do not support port I/O (which it now does, or at least used 
> to until recently).

It would seem weird to me that a module would build and run on a
generic kernel running on POWER9 (with some safe way of handling
inb/outb that don't actually work), but not on a kernel built
specifically for POWER9_CPU.

It sounds like inb/outb in a generic kernel on POWER9 may not
currently do something sensible, but that's fixable, e.g., make inb()
return 0xff and outb() a no-op.  I would naively expect the same
behavior in a POWER9_CPU kernel.

Bjorn
Maciej W. Rozycki May 5, 2022, 8:45 a.m. UTC | #9 
On Wed, 4 May 2022, Bjorn Helgaas wrote:

> >  Well, yes, except I would expect POWER9_CPU (and any higher versions we 
> > eventually get) to clear HAS_IOPORT.  Generic configurations (GENERIC_CPU) 
> > would set HAS_IOPORT of course, as would any lower architecture variants 
> > that do or may support port I/O (it's not clear to me if there are any 
> > that do not).  Ideally a generic configuration would not issue accesses to 
> > random MMIO locations for port I/O accesses via `inb'/`outb', etc. for 
> > systems that do not support port I/O (which it now does, or at least used 
> > to until recently).
> 
> It would seem weird to me that a module would build and run on a
> generic kernel running on POWER9 (with some safe way of handling
> inb/outb that don't actually work), but not on a kernel built
> specifically for POWER9_CPU.

 Why?  If you say configure your Alpha kernel for ALPHA_JENSEN, a pure 
EISA system, then you won't get PCI support nor any PCI drivers offered 
even though a generic Alpha kernel will get them all and still run on a 
Jensen system.  I find that no different from our case here.

 And if we do ever get TURBOchannel Alpha support, then a generic kernel 
configuration will offer EISA, PCI and TURBOchannel drivers, while you 
won't be offered TURBOchannel drivers for a PCI system and vice versa.  
It would make no sense to me.

 Please mind that the main objective for system-specific configurations is 
optimisation, including both size and speed, and a part of the solution is 
discarding stuff that's irrelevant for the respective system.  So in our 
case we do want any port I/O code not to be there at all in compiled code 
and consequently any driver that absolutely requires port I/O code to work 
will have to become a useless stub in its compiled form.  What would be 
the point then of having it there in the first place except to spread 
confusion?

  Maciej
Maciej W. Rozycki May 5, 2022, 12:03 p.m. UTC | #10 
On Wed, 4 May 2022, David Laight wrote:

> I think you can find cardbus cards that have a pci bridge and a cable
> link to an expansion chassis into which you can insert standard PCI cards.
> If you are really lucky the initial enumeration allocates the
> 'high field' bus numbers, io addresses and plenty of memory
> space to the bridge - otherwise you lose.

 No need to rely on luck as (given that no single size fits all) we have 
the `hpbussize', `hpiosize', `hpmemsize', `hpmmioprefsize', `hpmmiosize', 
options to the `pci=...' kernel parameter for people to tune the settings 
according to their needs.  I don't have such a CardBus option, but I do 
have a couple of such ExpressCard devices, and mixed PCIe/PCI expansion 
backplanes for them.

  Maciej
Bjorn Helgaas May 5, 2022, 7:38 p.m. UTC | #11 
On Thu, May 05, 2022 at 09:45:14AM +0100, Maciej W. Rozycki wrote:
> On Wed, 4 May 2022, Bjorn Helgaas wrote:
> 
> > >  Well, yes, except I would expect POWER9_CPU (and any higher versions we 
> > > eventually get) to clear HAS_IOPORT.  Generic configurations (GENERIC_CPU) 
> > > would set HAS_IOPORT of course, as would any lower architecture variants 
> > > that do or may support port I/O (it's not clear to me if there are any 
> > > that do not).  Ideally a generic configuration would not issue accesses to 
> > > random MMIO locations for port I/O accesses via `inb'/`outb', etc. for 
> > > systems that do not support port I/O (which it now does, or at least used 
> > > to until recently).
> > 
> > It would seem weird to me that a module would build and run on a
> > generic kernel running on POWER9 (with some safe way of handling
> > inb/outb that don't actually work), but not on a kernel built
> > specifically for POWER9_CPU.
> 
>  Why?  If you say configure your Alpha kernel for ALPHA_JENSEN, a pure 
> EISA system, then you won't get PCI support nor any PCI drivers offered 
> even though a generic Alpha kernel will get them all and still run on a 
> Jensen system.  I find that no different from our case here.
> 
>  And if we do ever get TURBOchannel Alpha support, then a generic kernel 
> configuration will offer EISA, PCI and TURBOchannel drivers, while you 
> won't be offered TURBOchannel drivers for a PCI system and vice versa.  
> It would make no sense to me.
> 
>  Please mind that the main objective for system-specific configurations is 
> optimisation, including both size and speed, and a part of the solution is 
> discarding stuff that's irrelevant for the respective system.  So in our 
> case we do want any port I/O code not to be there at all in compiled code 
> and consequently any driver that absolutely requires port I/O code to work 
> will have to become a useless stub in its compiled form.  What would be 
> the point then of having it there in the first place except to spread 
> confusion?

Good points.
diff mbox series

Patch

diff --git a/drivers/pcmcia/Kconfig b/drivers/pcmcia/Kconfig
index 2ce261cfff8e..32b5cd324c58 100644
--- a/drivers/pcmcia/Kconfig
+++ b/drivers/pcmcia/Kconfig
@@ -5,7 +5,7 @@ 
 
 menuconfig PCCARD
 	tristate "PCCard (PCMCIA/CardBus) support"
-	depends on !UML
+	depends on HAS_IOPORT
 	help
 	  Say Y here if you want to attach PCMCIA- or PC-cards to your Linux
 	  computer.  These are credit-card size devices such as network cards,