[RFC,v2,21/39] net: add HAS_IOPORT dependencies

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. It also turns out that with HAS_IOPORT handled
explicitly HAMRADIO does not need the !S390 dependency and successfully
builds the bpqether driver.

Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 drivers/net/Kconfig                | 2 +-
 drivers/net/arcnet/Kconfig         | 2 +-
 drivers/net/can/cc770/Kconfig      | 1 +
 drivers/net/can/sja1000/Kconfig    | 1 +
 drivers/net/ethernet/8390/Kconfig  | 2 +-
 drivers/net/ethernet/amd/Kconfig   | 2 +-
 drivers/net/ethernet/intel/Kconfig | 2 +-
 drivers/net/ethernet/sis/Kconfig   | 4 ++--
 drivers/net/ethernet/ti/Kconfig    | 2 +-
 drivers/net/ethernet/via/Kconfig   | 1 +
 drivers/net/fddi/Kconfig           | 2 +-
 drivers/net/hamradio/Kconfig       | 6 +++---
 drivers/net/wan/Kconfig            | 2 +-
 net/ax25/Kconfig                   | 2 +-
 14 files changed, 17 insertions(+), 14 deletions(-)

On 29.04.2022 15:50:33, 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. It also turns out that with HAS_IOPORT handled
> explicitly HAMRADIO does not need the !S390 dependency and successfully
> builds the bpqether driver.
> 
> Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>
> Co-developed-by: Arnd Bergmann <arnd@kernel.org>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
>  drivers/net/can/cc770/Kconfig      | 1 +
>  drivers/net/can/sja1000/Kconfig    | 1 +

For drivers/net/can:

Acked-by: Marc Kleine-Budde <mkl@pengutronix.de>

regards,
Marc

On Fri, 29 Apr 2022, 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. It also turns out that with HAS_IOPORT handled
> explicitly HAMRADIO does not need the !S390 dependency and successfully
> builds the bpqether driver.
[...]
> diff --git a/drivers/net/fddi/Kconfig b/drivers/net/fddi/Kconfig
> index 846bf41c2717..fa3f1e0fe143 100644
> --- a/drivers/net/fddi/Kconfig
> +++ b/drivers/net/fddi/Kconfig
> @@ -29,7 +29,7 @@ config DEFZA
>  
>  config DEFXX
>  	tristate "Digital DEFTA/DEFEA/DEFPA adapter support"
> -	depends on FDDI && (PCI || EISA || TC)
> +	depends on FDDI && (PCI || EISA || TC) && HAS_IOPORT
>  	help
>  	  This is support for the DIGITAL series of TURBOchannel (DEFTA),
>  	  EISA (DEFEA) and PCI (DEFPA) controllers which can connect you

 NAK, this has to be sorted out differently (and I think we discussed it 
before).

 The driver works just fine with MMIO where available, so if `inb'/`outb' 
do get removed, then only parts that rely on port I/O need to be disabled.  
In fact there's already such provision there in drivers/net/fddi/defxx.c 
for TURBOchannel systems (CONFIG_TC), which have no port I/O space either:

#if defined(CONFIG_EISA) || defined(CONFIG_PCI)
#define dfx_use_mmio bp->mmio
#else
#define dfx_use_mmio true
#endif

so I guess it's just the conditional that will have to be changed to:

#ifdef CONFIG_HAS_IOPORT

replacing the current explicit bus dependency list.  The compiler will 
then optimise away all the port I/O stuff (though I suspect dummy function 
declarations may be required for `inb'/`outb', etc.).

 I can verify a suitable change with a TURBOchannel configuration once the 
MIPS part has been sorted.

  Maciej

On Sun, 2022-05-01 at 23:48 +0100, Maciej W. Rozycki wrote:
> On Fri, 29 Apr 2022, 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. It also turns out that with HAS_IOPORT handled
> > explicitly HAMRADIO does not need the !S390 dependency and successfully
> > builds the bpqether driver.
> [...]
> > diff --git a/drivers/net/fddi/Kconfig b/drivers/net/fddi/Kconfig
> > index 846bf41c2717..fa3f1e0fe143 100644
> > --- a/drivers/net/fddi/Kconfig
> > +++ b/drivers/net/fddi/Kconfig
> > @@ -29,7 +29,7 @@ config DEFZA
> >  
> >  config DEFXX
> >  	tristate "Digital DEFTA/DEFEA/DEFPA adapter support"
> > -	depends on FDDI && (PCI || EISA || TC)
> > +	depends on FDDI && (PCI || EISA || TC) && HAS_IOPORT
> >  	help
> >  	  This is support for the DIGITAL series of TURBOchannel (DEFTA),
> >  	  EISA (DEFEA) and PCI (DEFPA) controllers which can connect you
> 
>  NAK, this has to be sorted out differently (and I think we discussed it 
> before).
> 
>  The driver works just fine with MMIO where available, so if `inb'/`outb' 
> do get removed, then only parts that rely on port I/O need to be disabled.  
> In fact there's already such provision there in drivers/net/fddi/defxx.c 
> for TURBOchannel systems (CONFIG_TC), which have no port I/O space either:
> 
> #if defined(CONFIG_EISA) || defined(CONFIG_PCI)
> #define dfx_use_mmio bp->mmio
> #else
> #define dfx_use_mmio true
> #endif
> 
> so I guess it's just the conditional that will have to be changed to:
> 
> #ifdef CONFIG_HAS_IOPORT
> 
> replacing the current explicit bus dependency list.  The compiler will 
> then optimise away all the port I/O stuff (though I suspect dummy function 
> declarations may be required for `inb'/`outb', etc.).
> 
>  I can verify a suitable change with a TURBOchannel configuration once the 
> MIPS part has been sorted.
> 
>   Maciej

With dfx_use_mmio changed as you propose above things compile on s390
which previously ran into missing (now __compile_error()) inl() via
dfx_port_read_long() -> dfx_inl() ->  inl().

Looking at the other uses of dfx_use_mmio I notice however that in
dfx_get_bars(), inb() actually gets called when dfx_use_mmio is true.
This happens if dfx_bus_eisa is also true. Now that variable is just
the cached result of DFX_BUS_EISA(dev) which is defined to 0 if
CONFIG_EISA is unset. I'm not 100% sure if going through a local
variable is still considered trivial enough dead code elimination, at
least it works for me™. I did also check the GCC docs and they
explicitly say that __attribute__(error) is supposed to be used when
dead code elimination gets rid of the error paths.

I think we also need a "depends on HAS_IOPORT" for "config HAVE_EISA"
just as I'm adding for "config ISA".

On Tue, 3 May 2022, Niklas Schnelle wrote:

> >  The driver works just fine with MMIO where available, so if `inb'/`outb' 
> > do get removed, then only parts that rely on port I/O need to be disabled.  
> > In fact there's already such provision there in drivers/net/fddi/defxx.c 
> > for TURBOchannel systems (CONFIG_TC), which have no port I/O space either:
> > 
> > #if defined(CONFIG_EISA) || defined(CONFIG_PCI)
> > #define dfx_use_mmio bp->mmio
> > #else
> > #define dfx_use_mmio true
> > #endif
> > 
> > so I guess it's just the conditional that will have to be changed to:
> > 
> > #ifdef CONFIG_HAS_IOPORT
> > 
> > replacing the current explicit bus dependency list.  The compiler will 
> > then optimise away all the port I/O stuff (though I suspect dummy function 
> > declarations may be required for `inb'/`outb', etc.).
[...]
> With dfx_use_mmio changed as you propose above things compile on s390
> which previously ran into missing (now __compile_error()) inl() via
> dfx_port_read_long() -> dfx_inl() ->  inl().

 Great, thanks for checking!  And I note referring `__compile_error' is 
roughly equivalent to a dummy declaration, so you've got that part sorted.

> Looking at the other uses of dfx_use_mmio I notice however that in
> dfx_get_bars(), inb() actually gets called when dfx_use_mmio is true.
> This happens if dfx_bus_eisa is also true. Now that variable is just
> the cached result of DFX_BUS_EISA(dev) which is defined to 0 if
> CONFIG_EISA is unset. I'm not 100% sure if going through a local
> variable is still considered trivial enough dead code elimination, at
> least it works for me™. I did also check the GCC docs and they
> explicitly say that __attribute__(error) is supposed to be used when
> dead code elimination gets rid of the error paths.

 Yeah, dead code elimination is supposed to handle such cases.  The local
automatic variable is essentially a syntactic feature not to use the same 
expression inline over and over throughout a function (for clarity the 
variable should probably be declared `const', but that is not essential) 
and it is up to the compiler whether to reuse the value previously 
calculated or to re-evaluate the expression.

> I think we also need a "depends on HAS_IOPORT" for "config HAVE_EISA"
> just as I'm adding for "config ISA".

 Oh absolutely!  There's the slot-specific port I/O space that is used to 
identify EISA option cards in device discovery, so no EISA device will 
ever work without port I/O.  Have a look at `decode_eisa_sig' in 
drivers/eisa/eisa-bus.c for the very obvious code.  Note that some ISA 
cards can be configured to appear as EISA devices as well (I have a 3c509B 
Ethernet NIC set up that way).

  Maciej