diff mbox

[1/4] isa: Allow ISA-style drivers on modern systems

Message ID 484d20f0a177f48a6d9a93520933b830d4412cca.1464014576.git.vilhelm.gray@gmail.com
State New
Headers show

Commit Message

William Breathitt Gray May 23, 2016, 2:58 p.m. UTC
Several modern devices, such as PC/104 cards, are expected to run on
modern systems via an ISA bus interface. Since ISA is a legacy interface
for most modern architectures, ISA support should remain disabled in
general. Support for ISA-style drivers should be enabled on a per driver
basis.

To allow ISA-style drivers on modern systems, this patch introduces the
ISA_BUS_API and ISA_BUS Kconfig options. The X86 ISA bus driver will now
build conditionally on the ISA_BUS_API Kconfig option, which defaults to
the legacy ISA Kconfig option. The ISA_BUS Kconfig option allows the
ISA_BUS_API Kconfig option to be selected on architectures which do not
enable ISA (e.g. X86_64).

For now, the ISA_BUS Kconfig option is only be available on X86
architectures. Support for other architectures may be added as required.

Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
---
 arch/x86/Kconfig      | 13 +++++++++++++
 drivers/base/Makefile |  2 +-
 2 files changed, 14 insertions(+), 1 deletion(-)

Comments

Guenter Roeck May 23, 2016, 5:21 p.m. UTC | #1
On Mon, May 23, 2016 at 10:58:41AM -0400, William Breathitt Gray wrote:
> Several modern devices, such as PC/104 cards, are expected to run on
> modern systems via an ISA bus interface. Since ISA is a legacy interface
> for most modern architectures, ISA support should remain disabled in
> general. Support for ISA-style drivers should be enabled on a per driver
> basis.
> 
> To allow ISA-style drivers on modern systems, this patch introduces the
> ISA_BUS_API and ISA_BUS Kconfig options. The X86 ISA bus driver will now
> build conditionally on the ISA_BUS_API Kconfig option, which defaults to
> the legacy ISA Kconfig option. The ISA_BUS Kconfig option allows the
> ISA_BUS_API Kconfig option to be selected on architectures which do not
> enable ISA (e.g. X86_64).
> 
> For now, the ISA_BUS Kconfig option is only be available on X86
> architectures. Support for other architectures may be added as required.
> 
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> ---
>  arch/x86/Kconfig      | 13 +++++++++++++
>  drivers/base/Makefile |  2 +-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 0a7b885..a41c0b8 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2439,6 +2439,19 @@ config PCI_CNB20LE_QUIRK
>  
>  source "drivers/pci/Kconfig"
>  
> +config ISA_BUS_API
> +	def_bool ISA
> +
> +config ISA_BUS
> +	bool "ISA-style bus support on modern systems" if (X86 && EXPERT)
> +	default y

Sure you want it enabled by default ?

> +	select ISA_BUS_API
> +	help
> +	  Enables ISA-style drivers on modern systems. This is necessary to
> +	  support PC/104 devices on X86_64 platforms.
> +
> +	  If unsure, say Y.
> +
>  # x86_64 have no ISA slots, but can have ISA-style DMA.
>  config ISA_DMA_API
>  	bool "ISA-style DMA support" if (X86_64 && EXPERT)
> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
> index 6b2a84e..2609ba2 100644
> --- a/drivers/base/Makefile
> +++ b/drivers/base/Makefile
> @@ -10,7 +10,7 @@ obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
>  obj-y			+= power/
>  obj-$(CONFIG_HAS_DMA)	+= dma-mapping.o
>  obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
> -obj-$(CONFIG_ISA)	+= isa.o
> +obj-$(CONFIG_ISA_BUS_API)	+= isa.o

Unless I am missing something, this is insufficient, and I am a bit surprised
that it actually works. include/linux/isa.h declares isa_register_driver()
and isa_unregister_driver() as dummies if CONFIG_ISA is not enabled.
Doesn't this cause a compile error ? Confused.

Thanks,
Guenter

>  obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
>  obj-$(CONFIG_NUMA)	+= node.o
>  obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o
> -- 
> 2.7.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
William Breathitt Gray May 23, 2016, 5:43 p.m. UTC | #2
On Mon, May 23, 2016 at 10:21:25AM -0700, Guenter Roeck wrote:
>On Mon, May 23, 2016 at 10:58:41AM -0400, William Breathitt Gray wrote:
>> +config ISA_BUS_API
>> +	def_bool ISA
>> +
>> +config ISA_BUS
>> +	bool "ISA-style bus support on modern systems" if (X86 && EXPERT)
>> +	default y
>
>Sure you want it enabled by default ?

Since the X86 ISA bus driver is more of an abstraction interface and
doesn't perform any hardware operations, I believe it's safe enough to
enable by default, thus allowing drivers dependent on it to show up for
selection in menuconfig.

>> +	select ISA_BUS_API
>> +	help
>> +	  Enables ISA-style drivers on modern systems. This is necessary to
>> +	  support PC/104 devices on X86_64 platforms.
>> +
>> +	  If unsure, say Y.
>> +
>>  # x86_64 have no ISA slots, but can have ISA-style DMA.
>>  config ISA_DMA_API
>>  	bool "ISA-style DMA support" if (X86_64 && EXPERT)
>> diff --git a/drivers/base/Makefile b/drivers/base/Makefile
>> index 6b2a84e..2609ba2 100644
>> --- a/drivers/base/Makefile
>> +++ b/drivers/base/Makefile
>> @@ -10,7 +10,7 @@ obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
>>  obj-y			+= power/
>>  obj-$(CONFIG_HAS_DMA)	+= dma-mapping.o
>>  obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
>> -obj-$(CONFIG_ISA)	+= isa.o
>> +obj-$(CONFIG_ISA_BUS_API)	+= isa.o
>
>Unless I am missing something, this is insufficient, and I am a bit surprised
>that it actually works. include/linux/isa.h declares isa_register_driver()
>and isa_unregister_driver() as dummies if CONFIG_ISA is not enabled.
>Doesn't this cause a compile error ? Confused.

You are correct: CONFIG_ISA in include/linux/isa.h should be
CONFIG_ISA_BUS_API. I'll add the change and submit version 2 of this
patchset after retesting.

William Breathitt Gray
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Torvalds May 23, 2016, 6:03 p.m. UTC | #3
On Mon, May 23, 2016 at 7:58 AM, William Breathitt Gray
<vilhelm.gray@gmail.com> wrote:
>
> For now, the ISA_BUS Kconfig option is only be available on X86
> architectures. Support for other architectures may be added as required.

So I'd prefer to see that

> +config ISA_BUS_API
> +       def_bool ISA

part in arch/Kconfig.

Why?

Because other architectures _already_ define that ISA symbol, and we
want the "ISA_BUS_API" to be a complete superset of ISA.

So whenever ISA is enabled, ISA_BUS_API should be enabled.

And the way you did that, that's not true. Now, if you were to enable
ISA on ARM, you'd not get ISA_BUS_API. And that sounds insane to me.
It also sounds *wrong* because it effectively changes the meaning of
this:

  --- a/drivers/base/Makefile
  +++ b/drivers/base/Makefile
  @@ -10,7 +10,7 @@ obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
  -obj-$(CONFIG_ISA)      += isa.o
  +obj-$(CONFIG_ISA_BUS_API)      += isa.o

where now that "isa.c" file gets built only on x86, whereas it *used*
to get built whenever ISA was enabled.

So the reason I suggested a separate ISA_BUS_API config option (that
then a particular architecture can choose to enable, in this case the
x86 choice of selecting ISA_BUS) was _exactly_ this issue. The plain
"ISA" config variable is not limited to x86, and the new subset of it
(the ISA_BUS_API) thus also must not be limited to just x86.

               Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 0a7b885..a41c0b8 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2439,6 +2439,19 @@  config PCI_CNB20LE_QUIRK
 
 source "drivers/pci/Kconfig"
 
+config ISA_BUS_API
+	def_bool ISA
+
+config ISA_BUS
+	bool "ISA-style bus support on modern systems" if (X86 && EXPERT)
+	default y
+	select ISA_BUS_API
+	help
+	  Enables ISA-style drivers on modern systems. This is necessary to
+	  support PC/104 devices on X86_64 platforms.
+
+	  If unsure, say Y.
+
 # x86_64 have no ISA slots, but can have ISA-style DMA.
 config ISA_DMA_API
 	bool "ISA-style DMA support" if (X86_64 && EXPERT)
diff --git a/drivers/base/Makefile b/drivers/base/Makefile
index 6b2a84e..2609ba2 100644
--- a/drivers/base/Makefile
+++ b/drivers/base/Makefile
@@ -10,7 +10,7 @@  obj-$(CONFIG_DMA_CMA) += dma-contiguous.o
 obj-y			+= power/
 obj-$(CONFIG_HAS_DMA)	+= dma-mapping.o
 obj-$(CONFIG_HAVE_GENERIC_DMA_COHERENT) += dma-coherent.o
-obj-$(CONFIG_ISA)	+= isa.o
+obj-$(CONFIG_ISA_BUS_API)	+= isa.o
 obj-$(CONFIG_FW_LOADER)	+= firmware_class.o
 obj-$(CONFIG_NUMA)	+= node.o
 obj-$(CONFIG_MEMORY_HOTPLUG_SPARSE) += memory.o