On Tue, Mar 14, 2023, at 13:11, Niklas Schnelle wrote:
> We introduce a new HAS_IOPORT Kconfig option to indicate support for I/O
> Port access. In a future patch HAS_IOPORT=n will disable compilation of
> the I/O accessor functions inb()/outb() and friends on architectures
> which can not meaningfully support legacy I/O spaces such as s390. Also
> add dependencies on HAS_IOPORT for the ISA and HAVE_EISA config options
> as these busses always go along with HAS_IOPORT.
>
> The "depends on" relations on HAS_IOPORT in drivers as well as ifdefs
> for HAS_IOPORT specific sections will be added in subsequent patches on
> a per subsystem basis.
I think it would be helpful to enumerate which architectures
do not get HAS_IOPORT added, as they will be affected more.
> Co-developed-by: Arnd Bergmann <[email protected]>
> Signed-off-by: Niklas Schnelle <[email protected]>
If there are no objections, I could send this first patch for the
asm-generic tree as a preparation for 6.3, so we are able to merge
the other patches through subsystem maintainer tree for 6.4.
arch/loongarch/ will now also need to select HAS_IOPORT
uncontitionally, this architecture was added after you
sent v2.
> diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
> index a98940e64243..5eeacc72e4da 100644
> --- a/arch/parisc/Kconfig
> +++ b/arch/parisc/Kconfig
> @@ -47,6 +47,7 @@ config PARISC
> select MODULES_USE_ELF_RELA
> select CLONE_BACKWARDS
> select TTY # Needed for pdc_cons.c
> + select HAS_IOPORT if PCI
It's also needed for EISA and I think you should select it
from CONFIG_GSC in drivers/parisc/Kconfig for this purpose.
This could also be 'select HAS_IOPORT if PCI || EISA', but
that would require removing the 'depends on HAS_IOPORT'
under drivers/eisa/.
> select HAVE_DEBUG_STACKOVERFLOW
> select HAVE_ARCH_AUDITSYSCALL
> select HAVE_ARCH_HASH
> @@ -131,6 +132,7 @@ config STACKTRACE_SUPPORT
>
> config ISA_DMA_API
> bool
> + depends on HAS_IOPORT
>
This line is not really needed since there is no way to
enable ISA_DMA_API.
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index a6c4407d3ec8..f7de646c074a 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -188,6 +188,7 @@ config PPC
> select GENERIC_SMP_IDLE_THREAD
> select GENERIC_TIME_VSYSCALL
> select GENERIC_VDSO_TIME_NS
> + select HAS_IOPORT if PCI
> select HAVE_ARCH_AUDITSYSCALL
> select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP
> select HAVE_ARCH_HUGE_VMAP if PPC_RADIX_MMU || PPC_8xx
> @@ -1070,7 +1071,6 @@ menu "Bus options"
>
> config ISA
> bool "Support for ISA-bus hardware"
> - depends on PPC_CHRP
> select PPC_I8259
> help
> Find out whether you have ISA slots on your motherboard. ISA is the
This line looks wrong, I think we should keep that dependency.
Did you get a circular dependency if you leave it in?
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index a825bf031f49..634dd42532f3 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -162,6 +162,7 @@ config X86
> select GUP_GET_PXX_LOW_HIGH if X86_PAE
> select HARDIRQS_SW_RESEND
> select HARDLOCKUP_CHECK_TIMESTAMP if X86_64
> + select HAS_IOPORT
> select HAVE_ACPI_APEI if ACPI
> select HAVE_ACPI_APEI_NMI if ACPI
> select HAVE_ALIGNED_STRUCT_PAGE if SLUB
> @@ -2893,6 +2894,7 @@ if X86_32
>
> config ISA
> bool "ISA support"
> + depends on HAS_IOPORT
> help
HAS_IOPORT is selected unconditionally already, so this doesn't
really do anything.
> diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb
> index 3b9a44008433..c68e4d9dcecb 100644
> --- a/lib/Kconfig.kgdb
> +++ b/lib/Kconfig.kgdb
> @@ -121,7 +121,8 @@ config KDB_DEFAULT_ENABLE
>
> config KDB_KEYBOARD
> bool "KGDB_KDB: keyboard as input device"
> - depends on VT && KGDB_KDB && !PARISC
> + depends on HAS_IOPORT
> + depends on VT && KGDB_KDB
> default n
This loses the !PARISC dependency, which I don't think is
intentional. The added HAS_IOPORT dependency makes sense
here, but I think this should be in a different patch
and not in the preparation.
Arnd
On Tue, 2023-03-14 at 14:29 +0100, Arnd Bergmann wrote:
> On Tue, Mar 14, 2023, at 13:11, Niklas Schnelle wrote:
> > We introduce a new HAS_IOPORT Kconfig option to indicate support for I/O
> > Port access. In a future patch HAS_IOPORT=n will disable compilation of
> > the I/O accessor functions inb()/outb() and friends on architectures
> > which can not meaningfully support legacy I/O spaces such as s390. Also
> > add dependencies on HAS_IOPORT for the ISA and HAVE_EISA config options
> > as these busses always go along with HAS_IOPORT.
> >
> > The "depends on" relations on HAS_IOPORT in drivers as well as ifdefs
> > for HAS_IOPORT specific sections will be added in subsequent patches on
> > a per subsystem basis.
>
> I think it would be helpful to enumerate which architectures
> do not get HAS_IOPORT added, as they will be affected more.
>
> > Co-developed-by: Arnd Bergmann <[email protected]>
> > Signed-off-by: Niklas Schnelle <[email protected]>
>
> If there are no objections, I could send this first patch for the
> asm-generic tree as a preparation for 6.3, so we are able to merge
> the other patches through subsystem maintainer tree for 6.4.
>
> arch/loongarch/ will now also need to select HAS_IOPORT
> uncontitionally, this architecture was added after you
> sent v2.
Ah right. Added "select HAS_IOPORT" for LoongArch.
>
> > diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
> > index a98940e64243..5eeacc72e4da 100644
> > --- a/arch/parisc/Kconfig
> > +++ b/arch/parisc/Kconfig
> > @@ -47,6 +47,7 @@ config PARISC
> > select MODULES_USE_ELF_RELA
> > select CLONE_BACKWARDS
> > select TTY # Needed for pdc_cons.c
> > + select HAS_IOPORT if PCI
>
> It's also needed for EISA and I think you should select it
> from CONFIG_GSC in drivers/parisc/Kconfig for this purpose.
>
> This could also be 'select HAS_IOPORT if PCI || EISA', but
> that would require removing the 'depends on HAS_IOPORT'
> under drivers/eisa/.
I did use "select HAS_IOPORT if PCI || ISA || ATARI_ROM_ISA" in m68k so
I think ideally we would handle both in the same way. I don't have a
strong preference but I think the "select HAS_IOPORT if ..." puts it
all in a single place which is nice. Also I think this would make it
more similar architectures with unconditional HAS_IOPORT that thus
don't need "depends on HAS_IOPORT" in their "config ISA" either. As
also pointed by your comment below for x86. So will try to go this
route.
>
> > select HAVE_DEBUG_STACKOVERFLOW
> > select HAVE_ARCH_AUDITSYSCALL
> > select HAVE_ARCH_HASH
> > @@ -131,6 +132,7 @@ config STACKTRACE_SUPPORT
> >
> > config ISA_DMA_API
> > bool
> > + depends on HAS_IOPORT
> >
>
> This line is not really needed since there is no way to
> enable ISA_DMA_API.
Removed
>
> > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> > index a6c4407d3ec8..f7de646c074a 100644
> > --- a/arch/powerpc/Kconfig
> > +++ b/arch/powerpc/Kconfig
> > @@ -188,6 +188,7 @@ config PPC
> > select GENERIC_SMP_IDLE_THREAD
> > select GENERIC_TIME_VSYSCALL
> > select GENERIC_VDSO_TIME_NS
> > + select HAS_IOPORT if PCI
> > select HAVE_ARCH_AUDITSYSCALL
> > select HAVE_ARCH_HUGE_VMALLOC if HAVE_ARCH_HUGE_VMAP
> > select HAVE_ARCH_HUGE_VMAP if PPC_RADIX_MMU || PPC_8xx
> > @@ -1070,7 +1071,6 @@ menu "Bus options"
> >
> > config ISA
> > bool "Support for ISA-bus hardware"
> > - depends on PPC_CHRP
> > select PPC_I8259
> > help
> > Find out whether you have ISA slots on your motherboard. ISA is the
>
> This line looks wrong, I think we should keep that dependency.
> Did you get a circular dependency if you leave it in?
I don't recall why this was removed. I guess it happened when I was
experimenting with adding "depends on HAS_IOPORT" for the ISA config
options but that ultimately lead to circular dependencies, must have
messed up when removing this here.
>
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index a825bf031f49..634dd42532f3 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -162,6 +162,7 @@ config X86
> > select GUP_GET_PXX_LOW_HIGH if X86_PAE
> > select HARDIRQS_SW_RESEND
> > select HARDLOCKUP_CHECK_TIMESTAMP if X86_64
> > + select HAS_IOPORT
> > select HAVE_ACPI_APEI if ACPI
> > select HAVE_ACPI_APEI_NMI if ACPI
> > select HAVE_ALIGNED_STRUCT_PAGE if SLUB
> > @@ -2893,6 +2894,7 @@ if X86_32
> >
> > config ISA
> > bool "ISA support"
> > + depends on HAS_IOPORT
> > help
>
> HAS_IOPORT is selected unconditionally already, so this doesn't
> really do anything.
Removed.
>
> > diff --git a/lib/Kconfig.kgdb b/lib/Kconfig.kgdb
> > index 3b9a44008433..c68e4d9dcecb 100644
> > --- a/lib/Kconfig.kgdb
> > +++ b/lib/Kconfig.kgdb
> > @@ -121,7 +121,8 @@ config KDB_DEFAULT_ENABLE
> >
> > config KDB_KEYBOARD
> > bool "KGDB_KDB: keyboard as input device"
> > - depends on VT && KGDB_KDB && !PARISC
> > + depends on HAS_IOPORT
> > + depends on VT && KGDB_KDB
> > default n
>
> This loses the !PARISC dependency, which I don't think is
> intentional. The added HAS_IOPORT dependency makes sense
> here, but I think this should be in a different patch
> and not in the preparation.
>
> Arnd
Agree will put into its own patch and re-add the !PARISC