Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1373455 > unrolled thread

[PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-04-07 16:50 +0200
    [PATCH 01/10] isa: Implement the module_isa_driver macro William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-04-07 16:50 +0200
    [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-04-07 16:50 +0200
      Re: [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver Guenter Roeck <linux@roeck-us.net> - 2016-04-08 02:40 +0200
        Re: [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-04-08 14:10 +0200
    [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-04-07 17:00 +0200
      Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS Guenter Roeck <linux@roeck-us.net> - 2016-04-08 02:50 +0200
        Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-04-08 14:40 +0200
          Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS Guenter Roeck <linux@roeck-us.net> - 2016-04-08 15:20 +0200
            Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-04-08 17:20 +0200
              Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS Guenter Roeck <linux@roeck-us.net> - 2016-04-08 20:30 +0200
                Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-04-08 21:30 +0200
                  Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-04-09 15:00 +0200
                    Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-04-09 16:00 +0200
                      Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-04-09 18:00 +0200
    [PATCH 02/10] isa: Implement the max_num_isa_dev macro William Breathitt Gray <vilhelm.gray@gmail.com> - 2016-04-07 17:00 +0200
    Re: [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices Linus Walleij <linus.walleij@linaro.org> - 2016-04-11 09:00 +0200

#1373455 — [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices

This patchset is based on top of commit 3a3a5fece6f2 ("fs: kernfs: Replace
CURRENT_TIME by current_fs_time()") of the driver-core-next branch of the
driver-core repository.

Two new ISA bus driver macros are introduced in this patchset:
module_isa_driver and max_num_isa_dev.

The module_isa_driver macro is a helper macro for ISA drivers which do not
do anything special in module init/exit. This macro is modelled after the
module_pci_driver macro and eliminates a lot of module init/exit
boilerplate code.

The max_num_isa_dev macro is used to determine the maximum possible number
of ISA devices which may be registered in the I/O port address space given
the address extent of the ISA devices. This macro is useful for computing
the maximum number of elements necessary to hold the base addresses and
interrupt line numbers of ISA devices for the respective ISA driver.

Lacking other documentation, I often found myself repeatedly returning to
the commit message of the initial commit for drivers/base/isa.c authored by
Rene Herman. A verbatim copy of this commit message has been added to
Documentation/isa.txt, along with descriptions for the module_isa driver
and max_num_isa_dev macros, for posterity.

The Apex Embedded Systems STX104 may be used on 64-bit X86 systems. This
patchset allows the Apex Embedded Systems STX104 DAC driver to be compiled
for both 32-bit and 64-bit X86 systems by depending on the ISA_BUS
configuration option rather than the ISA configuration option.

Similarly, many PC/104 and ISA devices may also be used on 64-bit X86
systems. The platform driver had been used to enable support for these
devices on 64-bit X86 systems. With the introduction of the ISA_BUS
configuration option, the respective drivers for these devices may now
utilize the ISA bus driver without restricting support to only 32-bit X86
systems; the following drivers now utilize the ISA bus driver over the
platform driver:

 * WinSystems EBC-C384 watchdog timer
 * ACCES 104-DIO-48E GPIO driver
 * ACCES 104-IDI-48 GPIO driver
 * ACCES 104-IDIO-16 GPIO driver
 * WinSystems WS16C48 GPIO driver

With the utilization of the ISA bus driver, the GPIO drivers in this
patchset may now support multiple devices for each of their respective ISA
drivers. A naming convention for module array parameters has been set based
on the Apex Embedded Systems STX104 DAC driver.

The "base" array module parameter sets the I/O port base address of each
device. The "irq" array module parameter sets the interrupt line number of
each device. Each element of the "base" array corresponds to a discrete
device; each element of the "irq" array corresponds to the respective
device addressed in the respective "base" array element.

William Breathitt Gray (10):
  isa: Implement the module_isa_driver macro
  isa: Implement the max_num_isa_dev macro
  Documentation: Add ISA bus driver documentation
  iio: stx104: Change STX104 dependency to ISA_BUS
  iio: stx104: Utilize the module_isa_driver and max_num_isa_dev macros
  watchdog: ebc-c384_wdt: Utilize the ISA bus driver
  gpio: 104-dio-48e: Utilize the ISA bus driver
  gpio: 104-idi-48: Utilize the ISA bus driver
  gpio: 104-idio-16: Utilize the ISA bus driver
  gpio: ws16c48: Utilize the ISA bus driver

 Documentation/isa.txt           | 121 ++++++++++++++++++++++++++++++++++++++++
 MAINTAINERS                     |   5 ++
 drivers/gpio/Kconfig            |  38 +++++++------
 drivers/gpio/gpio-104-dio-48e.c | 106 +++++++++++++----------------------
 drivers/gpio/gpio-104-idi-48.c  |  86 ++++++++++------------------
 drivers/gpio/gpio-104-idio-16.c |  85 ++++++++++------------------
 drivers/gpio/gpio-ws16c48.c     |  88 ++++++++++-------------------
 drivers/iio/dac/Kconfig         |   2 +-
 drivers/iio/dac/stx104.c        |  24 +-------
 drivers/watchdog/Kconfig        |   2 +-
 drivers/watchdog/ebc-c384_wdt.c |  43 ++++----------
 include/linux/isa.h             |  32 +++++++++++
 12 files changed, 317 insertions(+), 315 deletions(-)
 create mode 100644 Documentation/isa.txt

-- 
2.7.3

[toc] | [next] | [standalone]

#1373456 — [PATCH 01/10] isa: Implement the module_isa_driver macro

The module_isa_driver macro is a helper macro for ISA drivers which do
not do anything special in module init/exit. This eliminates a lot of
boilerplate code. Each module may only use this macro once, and calling
it replaces module_init and module_exit.

Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
---
 include/linux/isa.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/isa.h b/include/linux/isa.h
index 2a02862..d410259 100644
--- a/include/linux/isa.h
+++ b/include/linux/isa.h
@@ -36,4 +36,25 @@ static inline void isa_unregister_driver(struct isa_driver *d)
 }
 #endif
 
+/**
+ * module_isa_driver() - Helper macro for registering a ISA driver
+ * @__isa_driver: isa_driver struct
+ * @__num_isa_dev: number of devices to register
+ *
+ * Helper macro for ISA drivers which do not do anything special in module
+ * init/exit. This eliminates a lot of boilerplate code. Each module may only
+ * use this macro once, and calling it replaces module_init and module_exit.
+ */
+#define module_isa_driver(__isa_driver, __num_isa_dev) \
+static int __init __isa_driver##_init(void) \
+{ \
+	return isa_register_driver(&(__isa_driver), __num_isa_dev); \
+} \
+module_init(__isa_driver##_init); \
+static void __exit __isa_driver##_exit(void) \
+{ \
+	isa_unregister_driver(&(__isa_driver)); \
+} \
+module_exit(__isa_driver##_exit);
+
 #endif /* __LINUX_ISA_H */
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]

#1373458 — [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver

The WinSystems EBC-C384 watchdog timer is controlled via ISA bus
communication. As such, the ISA bus driver is more appropriate than the
platform driver for the WinSystems EBC-C384 watchdog timer driver.

Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
---
 drivers/watchdog/Kconfig        |  2 +-
 drivers/watchdog/ebc-c384_wdt.c | 43 ++++++++++-------------------------------
 2 files changed, 11 insertions(+), 34 deletions(-)

diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index fb94765..b10761d 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -738,7 +738,7 @@ config ALIM7101_WDT
 
 config EBC_C384_WDT
 	tristate "WinSystems EBC-C384 Watchdog Timer"
-	depends on X86
+	depends on X86 && ISA_BUS
 	select WATCHDOG_CORE
 	help
 	  Enables watchdog timer support for the watchdog timer on the
diff --git a/drivers/watchdog/ebc-c384_wdt.c b/drivers/watchdog/ebc-c384_wdt.c
index 77fda0b..4b849b8 100644
--- a/drivers/watchdog/ebc-c384_wdt.c
+++ b/drivers/watchdog/ebc-c384_wdt.c
@@ -16,10 +16,10 @@
 #include <linux/errno.h>
 #include <linux/io.h>
 #include <linux/ioport.h>
+#include <linux/isa.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
-#include <linux/platform_device.h>
 #include <linux/types.h>
 #include <linux/watchdog.h>
 
@@ -95,9 +95,8 @@ static const struct watchdog_info ebc_c384_wdt_info = {
 	.identity = MODULE_NAME
 };
 
-static int __init ebc_c384_wdt_probe(struct platform_device *pdev)
+static int ebc_c384_wdt_probe(struct device *dev, unsigned int id)
 {
-	struct device *dev = &pdev->dev;
 	struct watchdog_device *wdd;
 
 	if (!devm_request_region(dev, BASE_ADDR, ADDR_EXTENT, dev_name(dev))) {
@@ -122,61 +121,39 @@ static int __init ebc_c384_wdt_probe(struct platform_device *pdev)
 		dev_warn(dev, "Invalid timeout (%u seconds), using default (%u seconds)\n",
 			timeout, WATCHDOG_TIMEOUT);
 
-	platform_set_drvdata(pdev, wdd);
+	dev_set_drvdata(dev, wdd);
 
 	return watchdog_register_device(wdd);
 }
 
-static int ebc_c384_wdt_remove(struct platform_device *pdev)
+static int ebc_c384_wdt_remove(struct device *dev, unsigned int id)
 {
-	struct watchdog_device *wdd = platform_get_drvdata(pdev);
+	struct watchdog_device *wdd = dev_get_drvdata(dev);
 
 	watchdog_unregister_device(wdd);
 
 	return 0;
 }
 
-static struct platform_driver ebc_c384_wdt_driver = {
+static struct isa_driver ebc_c384_wdt_driver = {
+	.probe = ebc_c384_wdt_probe,
 	.driver = {
 		.name = MODULE_NAME
 	},
 	.remove = ebc_c384_wdt_remove
 };
 
-static struct platform_device *ebc_c384_wdt_device;
-
 static int __init ebc_c384_wdt_init(void)
 {
-	int err;
-
 	if (!dmi_match(DMI_BOARD_NAME, "EBC-C384 SBC"))
 		return -ENODEV;
 
-	ebc_c384_wdt_device = platform_device_alloc(MODULE_NAME, -1);
-	if (!ebc_c384_wdt_device)
-		return -ENOMEM;
-
-	err = platform_device_add(ebc_c384_wdt_device);
-	if (err)
-		goto err_platform_device;
-
-	err = platform_driver_probe(&ebc_c384_wdt_driver, ebc_c384_wdt_probe);
-	if (err)
-		goto err_platform_driver;
-
-	return 0;
-
-err_platform_driver:
-	platform_device_del(ebc_c384_wdt_device);
-err_platform_device:
-	platform_device_put(ebc_c384_wdt_device);
-	return err;
+	return isa_register_driver(&ebc_c384_wdt_driver, 1);
 }
 
 static void __exit ebc_c384_wdt_exit(void)
 {
-	platform_device_unregister(ebc_c384_wdt_device);
-	platform_driver_unregister(&ebc_c384_wdt_driver);
+	isa_unregister_driver(&ebc_c384_wdt_driver);
 }
 
 module_init(ebc_c384_wdt_init);
@@ -185,4 +162,4 @@ module_exit(ebc_c384_wdt_exit);
 MODULE_AUTHOR("William Breathitt Gray <vilhelm.gray@gmail.com>");
 MODULE_DESCRIPTION("WinSystems EBC-C384 watchdog timer driver");
 MODULE_LICENSE("GPL v2");
-MODULE_ALIAS("platform:" MODULE_NAME);
+MODULE_ALIAS("isa:" MODULE_NAME);
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]

#1373898 — Re: [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver

On Thu, Apr 07, 2016 at 10:47:27AM -0400, William Breathitt Gray wrote:
> The WinSystems EBC-C384 watchdog timer is controlled via ISA bus
> communication. As such, the ISA bus driver is more appropriate than the
> platform driver for the WinSystems EBC-C384 watchdog timer driver.
> 
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> ---
>  drivers/watchdog/Kconfig        |  2 +-
>  drivers/watchdog/ebc-c384_wdt.c | 43 ++++++++++-------------------------------
>  2 files changed, 11 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index fb94765..b10761d 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -738,7 +738,7 @@ config ALIM7101_WDT
>  
>  config EBC_C384_WDT
>  	tristate "WinSystems EBC-C384 Watchdog Timer"
> -	depends on X86
> +	depends on X86 && ISA_BUS

I am a bit concerend that the newly introduced ISA_BUS is not automatically
enabled. Effectively this means that all drivers depending on it will
be disabled until someone enables ISA_BUS in the distribution.

Is this a concern for anyone but me ?

Anyway, since you are the driver maintainer, I assume that you are ok
with it, so

Acked-by: Guenter Roeck <linux@roeck-us.net>

Side note for Wim: ISA_BUS was introduced with commit b3c1be1b789c
("base: isa: Remove X86_32 dependency") in -next.

Guenter

>  	select WATCHDOG_CORE
>  	help
>  	  Enables watchdog timer support for the watchdog timer on the
> diff --git a/drivers/watchdog/ebc-c384_wdt.c b/drivers/watchdog/ebc-c384_wdt.c
> index 77fda0b..4b849b8 100644
> --- a/drivers/watchdog/ebc-c384_wdt.c
> +++ b/drivers/watchdog/ebc-c384_wdt.c
> @@ -16,10 +16,10 @@
>  #include <linux/errno.h>
>  #include <linux/io.h>
>  #include <linux/ioport.h>
> +#include <linux/isa.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
>  #include <linux/moduleparam.h>
> -#include <linux/platform_device.h>
>  #include <linux/types.h>
>  #include <linux/watchdog.h>
>  
> @@ -95,9 +95,8 @@ static const struct watchdog_info ebc_c384_wdt_info = {
>  	.identity = MODULE_NAME
>  };
>  
> -static int __init ebc_c384_wdt_probe(struct platform_device *pdev)
> +static int ebc_c384_wdt_probe(struct device *dev, unsigned int id)
>  {
> -	struct device *dev = &pdev->dev;
>  	struct watchdog_device *wdd;
>  
>  	if (!devm_request_region(dev, BASE_ADDR, ADDR_EXTENT, dev_name(dev))) {
> @@ -122,61 +121,39 @@ static int __init ebc_c384_wdt_probe(struct platform_device *pdev)
>  		dev_warn(dev, "Invalid timeout (%u seconds), using default (%u seconds)\n",
>  			timeout, WATCHDOG_TIMEOUT);
>  
> -	platform_set_drvdata(pdev, wdd);
> +	dev_set_drvdata(dev, wdd);
>  
>  	return watchdog_register_device(wdd);
>  }
>  
> -static int ebc_c384_wdt_remove(struct platform_device *pdev)
> +static int ebc_c384_wdt_remove(struct device *dev, unsigned int id)
>  {
> -	struct watchdog_device *wdd = platform_get_drvdata(pdev);
> +	struct watchdog_device *wdd = dev_get_drvdata(dev);
>  
>  	watchdog_unregister_device(wdd);
>  
>  	return 0;
>  }
>  
> -static struct platform_driver ebc_c384_wdt_driver = {
> +static struct isa_driver ebc_c384_wdt_driver = {
> +	.probe = ebc_c384_wdt_probe,
>  	.driver = {
>  		.name = MODULE_NAME
>  	},
>  	.remove = ebc_c384_wdt_remove
>  };
>  
> -static struct platform_device *ebc_c384_wdt_device;
> -
>  static int __init ebc_c384_wdt_init(void)
>  {
> -	int err;
> -
>  	if (!dmi_match(DMI_BOARD_NAME, "EBC-C384 SBC"))
>  		return -ENODEV;
>  
> -	ebc_c384_wdt_device = platform_device_alloc(MODULE_NAME, -1);
> -	if (!ebc_c384_wdt_device)
> -		return -ENOMEM;
> -
> -	err = platform_device_add(ebc_c384_wdt_device);
> -	if (err)
> -		goto err_platform_device;
> -
> -	err = platform_driver_probe(&ebc_c384_wdt_driver, ebc_c384_wdt_probe);
> -	if (err)
> -		goto err_platform_driver;
> -
> -	return 0;
> -
> -err_platform_driver:
> -	platform_device_del(ebc_c384_wdt_device);
> -err_platform_device:
> -	platform_device_put(ebc_c384_wdt_device);
> -	return err;
> +	return isa_register_driver(&ebc_c384_wdt_driver, 1);
>  }
>  
>  static void __exit ebc_c384_wdt_exit(void)
>  {
> -	platform_device_unregister(ebc_c384_wdt_device);
> -	platform_driver_unregister(&ebc_c384_wdt_driver);
> +	isa_unregister_driver(&ebc_c384_wdt_driver);
>  }
>  
>  module_init(ebc_c384_wdt_init);
> @@ -185,4 +162,4 @@ module_exit(ebc_c384_wdt_exit);
>  MODULE_AUTHOR("William Breathitt Gray <vilhelm.gray@gmail.com>");
>  MODULE_DESCRIPTION("WinSystems EBC-C384 watchdog timer driver");
>  MODULE_LICENSE("GPL v2");
> -MODULE_ALIAS("platform:" MODULE_NAME);
> +MODULE_ALIAS("isa:" MODULE_NAME);
> -- 
> 2.7.3
> 

[toc] | [prev] | [next] | [standalone]

#1374177 — Re: [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver

On Thu, Apr 07, 2016 at 05:35:35PM -0700, Guenter Roeck wrote:
>I am a bit concerend that the newly introduced ISA_BUS is not automatically
>enabled. Effectively this means that all drivers depending on it will
>be disabled until someone enables ISA_BUS in the distribution.
>
>Is this a concern for anyone but me ?
>
>Anyway, since you are the driver maintainer, I assume that you are ok
>with it, so
>
>Acked-by: Guenter Roeck <linux@roeck-us.net>
>
>Side note for Wim: ISA_BUS was introduced with commit b3c1be1b789c
>("base: isa: Remove X86_32 dependency") in -next.
>
>Guenter

Since the ISA bus lacks standardized probing functionality, and the
majority of ISA devices I've encountered expect the user to start
writing to the device's I/O port addresses from the get-go, I think
ISA_BUS should remain an explicit dependency rather than become selected
when a user chooses a driver. That is to say, it is more appropriate for
a user to explicitly enable ISA_BUS if their system has an ISA bus;
otherwise a user may enable a driver with the expectation of a device
probe, whereas the driver will simply start writing to I/O port
addresses unexpectedly.

William Breathitt Gray

[toc] | [prev] | [next] | [standalone]

#1373462 — [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS

The Apex Embedded Systems STX104 may be used on 64-bit X86 systems. This
patch allows the Apex Embedded Systems STX104 DAC driver to be compiled
for both 32-bit and 64-bit X86 systems.

Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
---
 drivers/iio/dac/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index a995139..df4b55d 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -210,7 +210,7 @@ config MCP4922
 
 config STX104
 	tristate "Apex Embedded Systems STX104 DAC driver"
-	depends on ISA
+	depends on X86 && ISA_BUS
 	help
 	  Say yes here to build support for the 2-channel DAC on the Apex
 	  Embedded Systems STX104 integrated analog PC/104 card. The base port
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]

#1373912 — Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS

On Thu, Apr 07, 2016 at 10:47:25AM -0400, William Breathitt Gray wrote:
> The Apex Embedded Systems STX104 may be used on 64-bit X86 systems. This
> patch allows the Apex Embedded Systems STX104 DAC driver to be compiled
> for both 32-bit and 64-bit X86 systems.
> 
> Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
> ---
>  drivers/iio/dac/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
> index a995139..df4b55d 100644
> --- a/drivers/iio/dac/Kconfig
> +++ b/drivers/iio/dac/Kconfig
> @@ -210,7 +210,7 @@ config MCP4922
>  
>  config STX104
>  	tristate "Apex Embedded Systems STX104 DAC driver"
> -	depends on ISA
> +	depends on X86 && ISA_BUS

This means for this and other similar drivers that the driver is no longer
supported on architectures which support ISA but not the newly introduced
ISA_BUS. Affected architectures are alpha, arm, m32r, m68k, mips, powerpc,
and parisc.

A typical example is SCSI_AHA1542, which is no longer supported on those
architectures. It builds, but isa_register_driver() will be a dummy and fail.
Actually, this is true for _all_ drivers calling isa_register_driver().

I hope this is understood and doesn't cause any problems.

Thanks,
Guenter

>  	help
>  	  Say yes here to build support for the 2-channel DAC on the Apex
>  	  Embedded Systems STX104 integrated analog PC/104 card. The base port
> -- 
> 2.7.3
> 

[toc] | [prev] | [next] | [standalone]

#1374188 — Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS

On Thu, Apr 07, 2016 at 05:45:03PM -0700, Guenter Roeck wrote:
>This means for this and other similar drivers that the driver is no longer
>supported on architectures which support ISA but not the newly introduced
>ISA_BUS. Affected architectures are alpha, arm, m32r, m68k, mips, powerpc,
>and parisc.
>
>A typical example is SCSI_AHA1542, which is no longer supported on those
>architectures. It builds, but isa_register_driver() will be a dummy and fail.
>Actually, this is true for _all_ drivers calling isa_register_driver().
>
>I hope this is understood and doesn't cause any problems.
>
>Thanks,
>Guenter

That's a good catch. I overlooked this when I submitted the ISA_BUS
patch; I had improperly assumed the ISA option to have a dependency on
X86_32 based on arch/x86/Kconfig. The intention of the ISA_BUS is to
allow the proper definition of the isa_register_driver and
isa_unregister_driver functions without the dependency on X86_32 (e.g.
on X86_64 systems). How can this be resolved without ending support for
ISA on these other architectures? Would it be appropriate to add the
ISA_BUS dependency to every "config ISA" block for the other
architectures?

My avoidance of making ISA a selection of ISA_BUS is the possibility of
an invalid configuration: a user may initially enable ISA_BUS, then
later disable ISA, resulting in ISA_BUS remaining enabled without ISA
selected.

As a side note, should the dummy isa_register_driver return 0? Would it
be more appropriate for it to return an error code to indicate lack of
support for ISA, rather than silently fail?

William Breathitt Gray

[toc] | [prev] | [next] | [standalone]

#1374234 — Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS

On 04/08/2016 05:31 AM, William Breathitt Gray wrote:
> On Thu, Apr 07, 2016 at 05:45:03PM -0700, Guenter Roeck wrote:
>> This means for this and other similar drivers that the driver is no longer
>> supported on architectures which support ISA but not the newly introduced
>> ISA_BUS. Affected architectures are alpha, arm, m32r, m68k, mips, powerpc,
>> and parisc.
>>
>> A typical example is SCSI_AHA1542, which is no longer supported on those
>> architectures. It builds, but isa_register_driver() will be a dummy and fail.
>> Actually, this is true for _all_ drivers calling isa_register_driver().
>>
>> I hope this is understood and doesn't cause any problems.
>>
>> Thanks,
>> Guenter
>
> That's a good catch. I overlooked this when I submitted the ISA_BUS
> patch; I had improperly assumed the ISA option to have a dependency on
> X86_32 based on arch/x86/Kconfig. The intention of the ISA_BUS is to
> allow the proper definition of the isa_register_driver and
> isa_unregister_driver functions without the dependency on X86_32 (e.g.
> on X86_64 systems). How can this be resolved without ending support for
> ISA on these other architectures? Would it be appropriate to add the
> ISA_BUS dependency to every "config ISA" block for the other
> architectures?
>
 From the context, arm and mips use "select ISA". For those, adding and
auto-selecting ISA_BUS would make sense. For the remaining architectures
you could simply add "config ISA_BUS". I would suggest to update default
configurations, though.

There is also "um", for which you effectively disabled ISA support
as far as I can see. You might want to look into that as well.

> My avoidance of making ISA a selection of ISA_BUS is the possibility of
> an invalid configuration: a user may initially enable ISA_BUS, then
> later disable ISA, resulting in ISA_BUS remaining enabled without ISA
> selected.
>
Does that even make sense ? Not sure I understand why you don't just
select ISA_BUS if ISA is selected. That would also be backward compatible
and avoid the problem I was concerned about.

> As a side note, should the dummy isa_register_driver return 0? Would it
> be more appropriate for it to return an error code to indicate lack of
> support for ISA, rather than silently fail?
>
One should think so.

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

#1374298 — Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS

On Fri, Apr 08, 2016 at 06:18:09AM -0700, Guenter Roeck wrote:
> From the context, arm and mips use "select ISA". For those, adding and
>auto-selecting ISA_BUS would make sense. For the remaining architectures
>you could simply add "config ISA_BUS". I would suggest to update default
>configurations, though.
>
>There is also "um", for which you effectively disabled ISA support
>as far as I can see. You might want to look into that as well.
>
>> My avoidance of making ISA a selection of ISA_BUS is the possibility of
>> an invalid configuration: a user may initially enable ISA_BUS, then
>> later disable ISA, resulting in ISA_BUS remaining enabled without ISA
>> selected.
>>
>Does that even make sense ? Not sure I understand why you don't just
>select ISA_BUS if ISA is selected. That would also be backward compatible
>and avoid the problem I was concerned about.

I feel now that the introduction of the ISA_BUS option may the wrong
approach to resolve lack of ISA support for the X86_64 architecture;
adding ISA_BUS depends or selects through various Kconfigs would simply
obfuscate the ISA option. The true issue is that various driver
configs are assuming X86_32 architecture when they depend on the ISA
option, but the ISA bus does not require an X86_32 architecture.

The proper resolution then is to remove the misguided ISA_BUS option and
move the X86_32 dependency to the relevant drivers configs explicitly.
A grep for isa_register_driver calls within the kernel reveals that only
a few drivers explicitly use it. It should be trivial to create a patch
to add the explicit X86_32 dependency to the relevant drivers, so I will
submit one soon when I get the time to decouple X86_32 from the ISA
config option.

Once ISA is freed from the X86_32 dependency, I will simply use it
instead of ISA_BUS, and rebase this patchset for version 2.

>> As a side note, should the dummy isa_register_driver return 0? Would it
>> be more appropriate for it to return an error code to indicate lack of
>> support for ISA, rather than silently fail?
>>
>One should think so.
>
>Thanks,
>Guenter
>

I'll submit a separate patch for this as well then.

William Breathitt Gray

[toc] | [prev] | [next] | [standalone]

#1374410 — Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS

On Fri, Apr 08, 2016 at 11:09:22AM -0400, William Breathitt Gray wrote:
> On Fri, Apr 08, 2016 at 06:18:09AM -0700, Guenter Roeck wrote:
> > From the context, arm and mips use "select ISA". For those, adding and
> >auto-selecting ISA_BUS would make sense. For the remaining architectures
> >you could simply add "config ISA_BUS". I would suggest to update default
> >configurations, though.
> >
> >There is also "um", for which you effectively disabled ISA support
> >as far as I can see. You might want to look into that as well.
> >
> >> My avoidance of making ISA a selection of ISA_BUS is the possibility of
> >> an invalid configuration: a user may initially enable ISA_BUS, then
> >> later disable ISA, resulting in ISA_BUS remaining enabled without ISA
> >> selected.
> >>
> >Does that even make sense ? Not sure I understand why you don't just
> >select ISA_BUS if ISA is selected. That would also be backward compatible
> >and avoid the problem I was concerned about.
> 
> I feel now that the introduction of the ISA_BUS option may the wrong
> approach to resolve lack of ISA support for the X86_64 architecture;
> adding ISA_BUS depends or selects through various Kconfigs would simply
> obfuscate the ISA option. The true issue is that various driver
> configs are assuming X86_32 architecture when they depend on the ISA
> option, but the ISA bus does not require an X86_32 architecture.
> 
> The proper resolution then is to remove the misguided ISA_BUS option and
> move the X86_32 dependency to the relevant drivers configs explicitly.
> A grep for isa_register_driver calls within the kernel reveals that only
> a few drivers explicitly use it. It should be trivial to create a patch
> to add the explicit X86_32 dependency to the relevant drivers, so I will
> submit one soon when I get the time to decouple X86_32 from the ISA
> config option.
> 

That might be tricky: At least some if not many of those drivers are expected
to run on non-X86 architectures, and thus don't really depend on X86_32
(possibly some depend on 32 bit - I didn't check).

I count 44 calls to isa_register_driver() in the current mainline.
Not sure if this counts as "only a few drivers".

Thanks,
Guenter

> Once ISA is freed from the X86_32 dependency, I will simply use it
> instead of ISA_BUS, and rebase this patchset for version 2.
> 
> >> As a side note, should the dummy isa_register_driver return 0? Would it
> >> be more appropriate for it to return an error code to indicate lack of
> >> support for ISA, rather than silently fail?
> >>
> >One should think so.
> >
> >Thanks,
> >Guenter
> >
> 
> I'll submit a separate patch for this as well then.
> 
> William Breathitt Gray
> --
> 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

[toc] | [prev] | [next] | [standalone]

#1374439 — Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS

On Fri, Apr 08, 2016 at 11:28:01AM -0700, Guenter Roeck wrote:
>On Fri, Apr 08, 2016 at 11:09:22AM -0400, William Breathitt Gray wrote:
>> I feel now that the introduction of the ISA_BUS option may the wrong
>> approach to resolve lack of ISA support for the X86_64 architecture;
>> adding ISA_BUS depends or selects through various Kconfigs would simply
>> obfuscate the ISA option. The true issue is that various driver
>> configs are assuming X86_32 architecture when they depend on the ISA
>> option, but the ISA bus does not require an X86_32 architecture.
>> 
>> The proper resolution then is to remove the misguided ISA_BUS option and
>> move the X86_32 dependency to the relevant drivers configs explicitly.
>> A grep for isa_register_driver calls within the kernel reveals that only
>> a few drivers explicitly use it. It should be trivial to create a patch
>> to add the explicit X86_32 dependency to the relevant drivers, so I will
>> submit one soon when I get the time to decouple X86_32 from the ISA
>> config option.
>> 
>
>That might be tricky: At least some if not many of those drivers are expected
>to run on non-X86 architectures, and thus don't really depend on X86_32
>(possibly some depend on 32 bit - I didn't check).
>
>I count 44 calls to isa_register_driver() in the current mainline.
>Not sure if this counts as "only a few drivers".
>
>Thanks,
>Guenter

You're right, I there are more drivers that call isa_register_driver
than I had estimated. I think a different approach may work however.

When I initially proposed a patch to decouple the X86_32 dependency from
the ISA config option, I received a reply from the 0-DAY kernel test
auto-build indicating the errors from the drivers assuming a X86_32
architecture
(http://www.gossamer-threads.com/lists/linux/kernel/2350122#2350122).
After reviewing the debug messages, I realized there are only four main
issues:

 1. sound/isa/sscape.c:594:14:
     snd_printk format uses '%d' but a size_t is passed in
 2. arch/x86/mm/extable.c:23:15:
     'SEGMENT_IS_PNP_CODE' is undeclared
 3. drivers/pnp/pnpbios/bioscall.c:
     a slew of undeclared symbols and casting type mismatches
 4. drivers/scsi/ultrastor.c:674:29:
     sprintf format uses '%05X' but a kernel pointer is passed in

Both issue 1 and issue 4 are easily fixed by patching the respective
lines to match the format string with the variable type and vice-versa.

Issue 2 and 3 are the actual X86_32 assumption I had suspected: these
files depend on symbols declared in the include/asm/segment.h file; the
declaration of these symbols is conditional: #ifdef CONFIG_X86_32.

I believe this is the source of the issues I encountered on my initial
attempt to decouple the X86_32 dependency from the ISA option. I suspect
if I add an explicit X86_32 dependency to the PNPBIOS driver, I will be
able to remove the X86_32 dependency from the ISA option without
incident from the other drivers.

William Breathitt Gray

[toc] | [prev] | [next] | [standalone]

#1374626 — Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS

> I believe this is the source of the issues I encountered on my initial
> attempt to decouple the X86_32 dependency from the ISA option. I suspect
> if I add an explicit X86_32 dependency to the PNPBIOS driver, I will be
> able to remove the X86_32 dependency from the ISA option without
> incident from the other drivers.

That would be correct. PnPBIOS is obsoleted by ACPI so a 64bit x86
platform shouldn't be using PnPBIOS nor anything non x86. Strictly
speaking PnpBIOS is not ISA, it's onboard devices.

ISA devices that can be enumerated are usually enumerated via ISAPnP
which is platform independent.

Quite a few of the ISA drivers if you review them more carefully have
other endian and size assumptions, IRQ assumptions and probably fun bugs
because they've simply never been run on anything else even when it is
possible.

Alan

[toc] | [prev] | [next] | [standalone]

#1374632 — Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS

On Sat, Apr 09, 2016 at 01:58:14PM +0100, One Thousand Gnomes wrote:
>> I believe this is the source of the issues I encountered on my initial
>> attempt to decouple the X86_32 dependency from the ISA option. I suspect
>> if I add an explicit X86_32 dependency to the PNPBIOS driver, I will be
>> able to remove the X86_32 dependency from the ISA option without
>> incident from the other drivers.
>
>That would be correct. PnPBIOS is obsoleted by ACPI so a 64bit x86
>platform shouldn't be using PnPBIOS nor anything non x86. Strictly
>speaking PnpBIOS is not ISA, it's onboard devices.
>
>ISA devices that can be enumerated are usually enumerated via ISAPnP
>which is platform independent.
>
>Quite a few of the ISA drivers if you review them more carefully have
>other endian and size assumptions, IRQ assumptions and probably fun bugs
>because they've simply never been run on anything else even when it is
>possible.
>
>Alan

It looks like I'm in quite a pickle. Even if the patch for the PnPBIOS
driver removes the errors and warnings, there may be runtime bugs in
other drivers expecting X86_32. The only way I can see to prevent that
is to audit all the drivers which depend on the ISA option -- a behemoth
undertaking which would be far too impractical and error-prone for me to
do.

The alternative then is to do as Guenter Roeck suggests and
introduce/select ISA_BUS in the various other architectures which lack
it. In this scenario, I would expect the ISA option to be avoided for
new drivers, wherefore the ISA_BUS option can be used regardless of
architecture configuration.

I would prefer for a single ISA configuration option, but not at the
expense on breaking existing drivers; therefore, I will work instead on
adding the necessary ISA_BUS code to the various areas which require
them. If there are problems with this plan too, let me know.

William Breathitt Gray

[toc] | [prev] | [next] | [standalone]

#1374644 — Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS

> It looks like I'm in quite a pickle. Even if the patch for the PnPBIOS
> driver removes the errors and warnings, there may be runtime bugs in
> other drivers expecting X86_32. The only way I can see to prevent that
> is to audit all the drivers which depend on the ISA option -- a behemoth
> undertaking which would be far too impractical and error-prone for me to
> do.

I actually wouldn't worry. We've got lots of other drivers that don't
work on all the platforms they should because nobody has ever tried them
out.

At least if they compile on the different platforms people will be able
to try them. It's not making anything any worse - it used to not work and
it still doesn't work is the failure case 8)

Alan

[toc] | [prev] | [next] | [standalone]

#1373463 — [PATCH 02/10] isa: Implement the max_num_isa_dev macro

max_num_isa_dev is a macro to determine the maximum possible number of
ISA devices which may be registered in the I/O port address space given
the address extent of the ISA devices.

The highest base address possible for an ISA device is 0x3FF; this
results in 1024 possible base addresses. Dividing the number of possible
base addresses by the address extent taken by each device results in the
maximum number of devices on a system.

Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com>
---
 include/linux/isa.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/isa.h b/include/linux/isa.h
index d410259..bc0a7c0 100644
--- a/include/linux/isa.h
+++ b/include/linux/isa.h
@@ -57,4 +57,15 @@ static void __exit __isa_driver##_exit(void) \
 } \
 module_exit(__isa_driver##_exit);
 
+/**
+ * max_num_isa_dev() - Maximum possible number registered of an ISA device
+ * @__ida_dev_ext: ISA device address extent
+ *
+ * The highest base address possible for an ISA device is 0x3FF; this results in
+ * 1024 possible base addresses. Dividing the number of possible base addresses
+ * by the address extent taken by each device results in the maximum number of
+ * devices on a system.
+ */
+#define max_num_isa_dev(__isa_dev_ext) (1024 / __isa_dev_ext)
+
 #endif /* __LINUX_ISA_H */
-- 
2.7.3

[toc] | [prev] | [next] | [standalone]

#1375525

On Thu, Apr 7, 2016 at 4:47 PM, William Breathitt Gray
<vilhelm.gray@gmail.com> wrote:

> Two new ISA bus driver macros are introduced in this patchset:
> module_isa_driver and max_num_isa_dev.
(...)
>   gpio: 104-dio-48e: Utilize the ISA bus driver
>   gpio: 104-idi-48: Utilize the ISA bus driver
>   gpio: 104-idio-16: Utilize the ISA bus driver
>   gpio: ws16c48: Utilize the ISA bus driver

For these:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

So if the driver core maintainers want to merge this as part
of the series, go ahead.

Yours,
Linus Walleij

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web