* [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices @ 2016-04-07 14:47 ` William Breathitt Gray 0 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linux-0h96xk9xTtrk1uMJSBkQmQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA, William Breathitt Gray 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 -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices @ 2016-04-07 14:47 ` William Breathitt Gray 0 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio, William Breathitt Gray 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 01/10] isa: Implement the module_isa_driver macro 2016-04-07 14:47 ` William Breathitt Gray (?) @ 2016-04-07 14:47 ` William Breathitt Gray -1 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio, William Breathitt Gray 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 02/10] isa: Implement the max_num_isa_dev macro 2016-04-07 14:47 ` William Breathitt Gray (?) (?) @ 2016-04-07 14:47 ` William Breathitt Gray -1 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio, William Breathitt Gray 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 03/10] Documentation: Add ISA bus driver documentation 2016-04-07 14:47 ` William Breathitt Gray ` (2 preceding siblings ...) (?) @ 2016-04-07 14:47 ` William Breathitt Gray 2016-05-01 21:26 ` Greg KH -1 siblings, 1 reply; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio, William Breathitt Gray This is a verbatim copy of the original commit message of the initial commit of the ISA bus driver authored by Rene Herman. Descriptions of the module_isa_driver macro and max_num_isa_dev macro are provided at the end. Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> --- Documentation/isa.txt | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++ MAINTAINERS | 5 +++ 2 files changed, 126 insertions(+) create mode 100644 Documentation/isa.txt diff --git a/Documentation/isa.txt b/Documentation/isa.txt new file mode 100644 index 0000000..f232c26 --- /dev/null +++ b/Documentation/isa.txt @@ -0,0 +1,121 @@ +ISA Drivers +----------- + +The following text is adapted from the commit message of the initial +commit of the ISA bus driver authored by Rene Herman. + +During the recent "isa drivers using platform devices" discussion it was +pointed out that (ALSA) ISA drivers ran into the problem of not having +the option to fail driver load (device registration rather) upon not +finding their hardware due to a probe() error not being passed up +through the driver model. In the course of that, I suggested a separate +ISA bus might be best; Russell King agreed and suggested this bus could +use the .match() method for the actual device discovery. + +The attached does this. For this old non (generically) discoverable ISA +hardware only the driver itself can do discovery so as a difference with +the platform_bus, this isa_bus also distributes match() up to the +driver. + +As another difference: these devices only exist in the driver model due +to the driver creating them because it might want to drive them, meaning +that all device creation has been made internal as well. + +The usage model this provides is nice, and has been acked from the ALSA +side by Takashi Iwai and Jaroslav Kysela. The ALSA driver module_init's +now (for oldisa-only drivers) become: + +static int __init alsa_card_foo_init(void) +{ + return isa_register_driver(&snd_foo_isa_driver, SNDRV_CARDS); +} + +static void __exit alsa_card_foo_exit(void) +{ + isa_unregister_driver(&snd_foo_isa_driver); +} + +Quite like the other bus models therefore. This removes a lot of +duplicated init code from the ALSA ISA drivers. + +The passed in isa_driver struct is the regular driver struct embedding a +struct device_driver, the normal probe/remove/shutdown/suspend/resume +callbacks, and as indicated that .match callback. + +The "SNDRV_CARDS" you see being passed in is a "unsigned int ndev" +parameter, indicating how many devices to create and call our methods +with. + +The platform_driver callbacks are called with a platform_device param; +the isa_driver callbacks are being called with a "struct device *dev, +unsigned int id" pair directly -- with the device creation completely +internal to the bus it's much cleaner to not leak isa_dev's by passing +them in at all. The id is the only thing we ever want other then the +struct device * anyways, and it makes for nicer code in the callbacks as +well. + +With this additional .match() callback ISA drivers have all options. If +ALSA would want to keep the old non-load behaviour, it could stick all +of the old .probe in .match, which would only keep them registered after +everything was found to be present and accounted for. If it wanted the +behaviour of always loading as it inadvertently did for a bit after the +changeover to platform devices, it could just not provide a .match() and +do everything in .probe() as before. + +If it, as Takashi Iwai already suggested earlier as a way of following +the model from saner buses more closely, wants to load when a later bind +could conceivably succeed, it could use .match() for the prerequisites +(such as checking the user wants the card enabled and that port/irq/dma +values have been passed in) and .probe() for everything else. This is +the nicest model. + +To the code... + +This exports only two functions; isa_{,un}register_driver(). + +isa_register_driver() register's the struct device_driver, and then +loops over the passed in ndev creating devices and registering them. +This causes the bus match method to be called for them, which is: + +int isa_bus_match(struct device *dev, struct device_driver *driver) +{ + struct isa_driver *isa_driver = to_isa_driver(driver); + + if (dev->platform_data == isa_driver) { + if (!isa_driver->match || + isa_driver->match(dev, to_isa_dev(dev)->id)) + return 1; + dev->platform_data = NULL; + } + return 0; +} + +The first thing this does is check if this device is in fact one of this +driver's devices by seeing if the device's platform_data pointer is set +to this driver. Platform devices compare strings, but we don't need to +do that with everything being internal, so isa_register_driver() abuses +dev->platform_data as a isa_driver pointer which we can then check here. +I believe platform_data is available for this, but if rather not, moving +the isa_driver pointer to the private struct isa_dev is ofcourse fine as +well. + +Then, if the the driver did not provide a .match, it matches. If it did, +the driver match() method is called to determine a match. + +If it did _not_ match, dev->platform_data is reset to indicate this to +isa_register_driver which can then unregister the device again. + +If during all this, there's any error, or no devices matched at all +everything is backed out again and the error, or -ENODEV, is returned. + +isa_unregister_driver() just unregisters the matched devices and the +driver itself. + +module_isa_driver 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. + +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. diff --git a/MAINTAINERS b/MAINTAINERS index 03e00c7..3713010 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -5994,6 +5994,11 @@ F: include/linux/irqdomain.h F: kernel/irq/irqdomain.c F: kernel/irq/msi.c +ISA +M: William Breathitt Gray <vilhelm.gray@gmail.com> +S: Maintained +F: Documentation/isa.txt + ISAPNP M: Jaroslav Kysela <perex@perex.cz> S: Maintained -- 2.7.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 03/10] Documentation: Add ISA bus driver documentation 2016-04-07 14:47 ` [PATCH 03/10] Documentation: Add ISA bus driver documentation William Breathitt Gray @ 2016-05-01 21:26 ` Greg KH 0 siblings, 0 replies; 39+ messages in thread From: Greg KH @ 2016-05-01 21:26 UTC (permalink / raw) To: William Breathitt Gray Cc: tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio On Thu, Apr 07, 2016 at 10:47:24AM -0400, William Breathitt Gray wrote: > This is a verbatim copy of the original commit message of the initial > commit of the ISA bus driver authored by Rene Herman. Descriptions of > the module_isa_driver macro and max_num_isa_dev macro are provided at > the end. > > Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> > --- > Documentation/isa.txt | 121 ++++++++++++++++++++++++++++++++++++++++++++++++++ > MAINTAINERS | 5 +++ > 2 files changed, 126 insertions(+) > create mode 100644 Documentation/isa.txt > > diff --git a/Documentation/isa.txt b/Documentation/isa.txt > new file mode 100644 > index 0000000..f232c26 > --- /dev/null > +++ b/Documentation/isa.txt > @@ -0,0 +1,121 @@ > +ISA Drivers > +----------- > + > +The following text is adapted from the commit message of the initial > +commit of the ISA bus driver authored by Rene Herman. > + > +During the recent "isa drivers using platform devices" discussion it was > +pointed out that (ALSA) ISA drivers ran into the problem of not having > +the option to fail driver load (device registration rather) upon not > +finding their hardware due to a probe() error not being passed up > +through the driver model. In the course of that, I suggested a separate > +ISA bus might be best; Russell King agreed and suggested this bus could > +use the .match() method for the actual device discovery. > + > +The attached does this. For this old non (generically) discoverable ISA > +hardware only the driver itself can do discovery so as a difference with > +the platform_bus, this isa_bus also distributes match() up to the > +driver. > + > +As another difference: these devices only exist in the driver model due > +to the driver creating them because it might want to drive them, meaning > +that all device creation has been made internal as well. > + > +The usage model this provides is nice, and has been acked from the ALSA > +side by Takashi Iwai and Jaroslav Kysela. The ALSA driver module_init's > +now (for oldisa-only drivers) become: > + > +static int __init alsa_card_foo_init(void) > +{ > + return isa_register_driver(&snd_foo_isa_driver, SNDRV_CARDS); > +} > + > +static void __exit alsa_card_foo_exit(void) > +{ > + isa_unregister_driver(&snd_foo_isa_driver); > +} > + > +Quite like the other bus models therefore. This removes a lot of > +duplicated init code from the ALSA ISA drivers. > + > +The passed in isa_driver struct is the regular driver struct embedding a > +struct device_driver, the normal probe/remove/shutdown/suspend/resume > +callbacks, and as indicated that .match callback. > + > +The "SNDRV_CARDS" you see being passed in is a "unsigned int ndev" > +parameter, indicating how many devices to create and call our methods > +with. > + > +The platform_driver callbacks are called with a platform_device param; > +the isa_driver callbacks are being called with a "struct device *dev, > +unsigned int id" pair directly -- with the device creation completely > +internal to the bus it's much cleaner to not leak isa_dev's by passing > +them in at all. The id is the only thing we ever want other then the > +struct device * anyways, and it makes for nicer code in the callbacks as > +well. > + > +With this additional .match() callback ISA drivers have all options. If > +ALSA would want to keep the old non-load behaviour, it could stick all > +of the old .probe in .match, which would only keep them registered after > +everything was found to be present and accounted for. If it wanted the > +behaviour of always loading as it inadvertently did for a bit after the > +changeover to platform devices, it could just not provide a .match() and > +do everything in .probe() as before. > + > +If it, as Takashi Iwai already suggested earlier as a way of following > +the model from saner buses more closely, wants to load when a later bind > +could conceivably succeed, it could use .match() for the prerequisites > +(such as checking the user wants the card enabled and that port/irq/dma > +values have been passed in) and .probe() for everything else. This is > +the nicest model. > + > +To the code... > + > +This exports only two functions; isa_{,un}register_driver(). > + > +isa_register_driver() register's the struct device_driver, and then > +loops over the passed in ndev creating devices and registering them. > +This causes the bus match method to be called for them, which is: > + > +int isa_bus_match(struct device *dev, struct device_driver *driver) > +{ > + struct isa_driver *isa_driver = to_isa_driver(driver); > + > + if (dev->platform_data == isa_driver) { > + if (!isa_driver->match || > + isa_driver->match(dev, to_isa_dev(dev)->id)) > + return 1; > + dev->platform_data = NULL; > + } > + return 0; > +} > + > +The first thing this does is check if this device is in fact one of this > +driver's devices by seeing if the device's platform_data pointer is set > +to this driver. Platform devices compare strings, but we don't need to > +do that with everything being internal, so isa_register_driver() abuses > +dev->platform_data as a isa_driver pointer which we can then check here. > +I believe platform_data is available for this, but if rather not, moving > +the isa_driver pointer to the private struct isa_dev is ofcourse fine as > +well. > + > +Then, if the the driver did not provide a .match, it matches. If it did, > +the driver match() method is called to determine a match. > + > +If it did _not_ match, dev->platform_data is reset to indicate this to > +isa_register_driver which can then unregister the device again. > + > +If during all this, there's any error, or no devices matched at all > +everything is backed out again and the error, or -ENODEV, is returned. > + > +isa_unregister_driver() just unregisters the matched devices and the > +driver itself. > + > +module_isa_driver 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. > + > +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. > diff --git a/MAINTAINERS b/MAINTAINERS > index 03e00c7..3713010 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5994,6 +5994,11 @@ F: include/linux/irqdomain.h > F: kernel/irq/irqdomain.c > F: kernel/irq/msi.c > > +ISA > +M: William Breathitt Gray <vilhelm.gray@gmail.com> > +S: Maintained > +F: Documentation/isa.txt You are just in charge of one documentation file? That seems odd for a whole subsystem :( Why not list all of the needed files? thanks, greg k-h ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS 2016-04-07 14:47 ` William Breathitt Gray ` (3 preceding siblings ...) (?) @ 2016-04-07 14:47 ` William Breathitt Gray [not found] ` <783be62acf68b35f3fe4785a2cedfe017624688b.1460040201.git.vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> -1 siblings, 1 reply; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio, William Breathitt Gray 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
[parent not found: <783be62acf68b35f3fe4785a2cedfe017624688b.1460040201.git.vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS 2016-04-07 14:47 ` [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS William Breathitt Gray @ 2016-04-08 0:45 ` Guenter Roeck 0 siblings, 0 replies; 39+ messages in thread From: Guenter Roeck @ 2016-04-08 0:45 UTC (permalink / raw) To: William Breathitt Gray Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > --- > 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 > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS @ 2016-04-08 0:45 ` Guenter Roeck 0 siblings, 0 replies; 39+ messages in thread From: Guenter Roeck @ 2016-04-08 0:45 UTC (permalink / raw) To: William Breathitt Gray Cc: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio 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 > ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20160408004503.GB10211-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>]
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS 2016-04-08 0:45 ` Guenter Roeck @ 2016-04-08 12:31 ` William Breathitt Gray -1 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-08 12:31 UTC (permalink / raw) To: Guenter Roeck Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS @ 2016-04-08 12:31 ` William Breathitt Gray 0 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-08 12:31 UTC (permalink / raw) To: Guenter Roeck Cc: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS 2016-04-08 12:31 ` William Breathitt Gray @ 2016-04-08 13:18 ` Guenter Roeck -1 siblings, 0 replies; 39+ messages in thread From: Guenter Roeck @ 2016-04-08 13:18 UTC (permalink / raw) To: William Breathitt Gray Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS @ 2016-04-08 13:18 ` Guenter Roeck 0 siblings, 0 replies; 39+ messages in thread From: Guenter Roeck @ 2016-04-08 13:18 UTC (permalink / raw) To: William Breathitt Gray Cc: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <5707AF91.5010704-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>]
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS 2016-04-08 13:18 ` Guenter Roeck @ 2016-04-08 15:09 ` William Breathitt Gray -1 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-08 15:09 UTC (permalink / raw) To: Guenter Roeck Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA 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 -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS @ 2016-04-08 15:09 ` William Breathitt Gray 0 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-08 15:09 UTC (permalink / raw) To: Guenter Roeck Cc: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS 2016-04-08 15:09 ` William Breathitt Gray (?) @ 2016-04-08 18:28 ` Guenter Roeck [not found] ` <20160408182801.GB7083-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> -1 siblings, 1 reply; 39+ messages in thread From: Guenter Roeck @ 2016-04-08 18:28 UTC (permalink / raw) To: William Breathitt Gray Cc: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20160408182801.GB7083-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org>]
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS 2016-04-08 18:28 ` Guenter Roeck @ 2016-04-08 19:27 ` William Breathitt Gray 0 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-08 19:27 UTC (permalink / raw) To: Guenter Roeck Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA 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 -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS @ 2016-04-08 19:27 ` William Breathitt Gray 0 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-08 19:27 UTC (permalink / raw) To: Guenter Roeck Cc: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS 2016-04-08 19:27 ` William Breathitt Gray (?) @ 2016-04-09 12:58 ` One Thousand Gnomes [not found] ` <20160409135814.359e24d6-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org> -1 siblings, 1 reply; 39+ messages in thread From: One Thousand Gnomes @ 2016-04-09 12:58 UTC (permalink / raw) To: William Breathitt Gray Cc: Guenter Roeck, gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio > 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <20160409135814.359e24d6-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org>]
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS 2016-04-09 12:58 ` One Thousand Gnomes @ 2016-04-09 13:50 ` William Breathitt Gray 0 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-09 13:50 UTC (permalink / raw) To: One Thousand Gnomes Cc: Guenter Roeck, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS @ 2016-04-09 13:50 ` William Breathitt Gray 0 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-09 13:50 UTC (permalink / raw) To: One Thousand Gnomes Cc: Guenter Roeck, gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS 2016-04-09 13:50 ` William Breathitt Gray @ 2016-04-09 15:51 ` One Thousand Gnomes -1 siblings, 0 replies; 39+ messages in thread From: One Thousand Gnomes @ 2016-04-09 15:51 UTC (permalink / raw) To: William Breathitt Gray Cc: Guenter Roeck, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA > 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS @ 2016-04-09 15:51 ` One Thousand Gnomes 0 siblings, 0 replies; 39+ messages in thread From: One Thousand Gnomes @ 2016-04-09 15:51 UTC (permalink / raw) To: William Breathitt Gray Cc: Guenter Roeck, gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio > 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 05/10] iio: stx104: Utilize the module_isa_driver and max_num_isa_dev macros 2016-04-07 14:47 ` William Breathitt Gray ` (4 preceding siblings ...) (?) @ 2016-04-07 14:47 ` William Breathitt Gray -1 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio, William Breathitt Gray The Apex Embedded Systems STX104 DAC drivers does not do anything special in module init/exit. This patch eliminates the module init/exit boilerplate code by utilizing the module_isa_driver macro. Additionally, the max_num_isa_dev macro is utilized to simplify the determination of maximum possible STX104 devices in the system. Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> --- drivers/iio/dac/stx104.c | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/drivers/iio/dac/stx104.c b/drivers/iio/dac/stx104.c index 174f4b7..2794122 100644 --- a/drivers/iio/dac/stx104.c +++ b/drivers/iio/dac/stx104.c @@ -33,16 +33,9 @@ } #define STX104_EXTENT 16 -/** - * 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_STX104 (1024 / STX104_EXTENT) -static unsigned base[MAX_NUM_STX104]; -static unsigned num_stx104; +static unsigned int base[max_num_isa_dev(STX104_EXTENT)]; +static unsigned int num_stx104; module_param_array(base, uint, &num_stx104, 0); MODULE_PARM_DESC(base, "Apex Embedded Systems STX104 base addresses"); @@ -134,18 +127,7 @@ static struct isa_driver stx104_driver = { } }; -static void __exit stx104_exit(void) -{ - isa_unregister_driver(&stx104_driver); -} - -static int __init stx104_init(void) -{ - return isa_register_driver(&stx104_driver, num_stx104); -} - -module_init(stx104_init); -module_exit(stx104_exit); +module_isa_driver(stx104_driver, num_stx104); MODULE_AUTHOR("William Breathitt Gray <vilhelm.gray@gmail.com>"); MODULE_DESCRIPTION("Apex Embedded Systems STX104 DAC driver"); -- 2.7.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver 2016-04-07 14:47 ` William Breathitt Gray ` (5 preceding siblings ...) (?) @ 2016-04-07 14:47 ` William Breathitt Gray [not found] ` <1f5bf2e21006f0fd4f10ab3948cf69a737c0b039.1460040201.git.vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> -1 siblings, 1 reply; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio, William Breathitt Gray 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 ^ permalink raw reply related [flat|nested] 39+ messages in thread
[parent not found: <1f5bf2e21006f0fd4f10ab3948cf69a737c0b039.1460040201.git.vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver 2016-04-07 14:47 ` [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver William Breathitt Gray @ 2016-04-08 0:35 ` Guenter Roeck 0 siblings, 0 replies; 39+ messages in thread From: Guenter Roeck @ 2016-04-08 0:35 UTC (permalink / raw) To: William Breathitt Gray Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > --- > 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-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>"); > 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver @ 2016-04-08 0:35 ` Guenter Roeck 0 siblings, 0 replies; 39+ messages in thread From: Guenter Roeck @ 2016-04-08 0:35 UTC (permalink / raw) To: William Breathitt Gray Cc: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio 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 > ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver 2016-04-08 0:35 ` Guenter Roeck (?) @ 2016-04-08 12:03 ` William Breathitt Gray -1 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-08 12:03 UTC (permalink / raw) To: Guenter Roeck Cc: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver 2016-04-07 14:47 ` [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver William Breathitt Gray @ 2016-05-11 17:04 ` Sasha Levin 0 siblings, 0 replies; 39+ messages in thread From: Sasha Levin @ 2016-05-11 17:04 UTC (permalink / raw) To: William Breathitt Gray, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linux-0h96xk9xTtrk1uMJSBkQmQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA On 04/07/2016 10:47 AM, 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> Hey William, I'm seeing this on boot: kernel BUG at drivers/base/driver.c:153! invalid opcode: 0000 [#1] PREEMPT SMP KASAN Modules linked in: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.6.0-rc7-next-20160511-sasha-00024-g13dfe33 #3081 task: ffff88005b2f8000 ti: ffff88005b300000 task.ti: ffff88005b300000 RIP: driver_register (drivers/base/driver.c:153 (discriminator 1)) RSP: 0000:ffff88005b307c68 EFLAGS: 00010282 RAX: 0000000000000000 RBX: ffffffffb3987c30 RCX: 1ffffffff68acf26 RDX: 0000000000000000 RSI: dffffc0000000000 RDI: ffffffffb4567930 RBP: ffff88005b307c88 R08: 1ffffffff606e3dc R09: dffffc0000000000 R10: 0000000080000000 R11: 1ffffffff79c42e2 R12: ffffffffb45678a0 R13: ffffffffb3987c38 R14: ffffffffb3987c30 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff880063e40000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000030023000 CR4: 00000000000406a0 Stack: 1ffff1000b660fa4 dffffc0000000000 0000000000000000 ffffffffb3987c00 ffff88005b307cf0 ffffffffa5bf826d ffff88005b307dc0 ffffffffb3987c30 ffffffffb3987ca8 ffffffffb39847e0 ffffffffb39847e0 00000000f673090a Call Trace: isa_register_driver (drivers/base/isa.c:123) dio48e_driver_init (drivers/gpio/gpio-104-dio-48e.c:396) do_one_initcall (init/main.c:770) kernel_init_freeable (init/main.c:834 init/main.c:843 init/main.c:861 init/main.c:1008) kernel_init (init/main.c:936) ret_from_fork (arch/x86/entry/entry_64.S:390) Code: be 00 00 00 00 00 fc ff df 48 89 f9 48 c1 e9 03 80 3c 31 00 74 05 e8 4a 18 be fd 49 83 bc 24 90 00 00 00 00 75 13 e8 2a 89 a0 fd <0f> 0b 48 c7 c7 40 21 51 b4 e8 6f 78 58 ff e8 17 89 a0 fd 49 8d All code ======== 0: be 00 00 00 00 mov $0x0,%esi 5: 00 fc add %bh,%ah 7: ff df lcallq *<internal disassembler error> 9: 48 89 f9 mov %rdi,%rcx c: 48 c1 e9 03 shr $0x3,%rcx 10: 80 3c 31 00 cmpb $0x0,(%rcx,%rsi,1) 14: 74 05 je 0x1b 16: e8 4a 18 be fd callq 0xfffffffffdbe1865 1b: 49 83 bc 24 90 00 00 cmpq $0x0,0x90(%r12) 22: 00 00 24: 75 13 jne 0x39 26: e8 2a 89 a0 fd callq 0xfffffffffda08955 2b:* 0f 0b ud2 <-- trapping instruction 2d: 48 c7 c7 40 21 51 b4 mov $0xffffffffb4512140,%rdi 34: e8 6f 78 58 ff callq 0xffffffffff5878a8 39: e8 17 89 a0 fd callq 0xfffffffffda08955 3e: 49 8d 00 lea (%r8),%rax Code starting with the faulting instruction =========================================== 0: 0f 0b ud2 2: 48 c7 c7 40 21 51 b4 mov $0xffffffffb4512140,%rdi 9: e8 6f 78 58 ff callq 0xffffffffff58787d e: e8 17 89 a0 fd callq 0xfffffffffda0892a 13: 49 8d 00 lea (%r8),%rax RIP driver_register (drivers/base/driver.c:153 (discriminator 1)) RSP <ffff88005b307c68> ---[ end trace 96103422392be10c ]--- ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver @ 2016-05-11 17:04 ` Sasha Levin 0 siblings, 0 replies; 39+ messages in thread From: Sasha Levin @ 2016-05-11 17:04 UTC (permalink / raw) To: William Breathitt Gray, gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio On 04/07/2016 10:47 AM, 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> Hey William, I'm seeing this on boot: kernel BUG at drivers/base/driver.c:153! invalid opcode: 0000 [#1] PREEMPT SMP KASAN Modules linked in: CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.6.0-rc7-next-20160511-sasha-00024-g13dfe33 #3081 task: ffff88005b2f8000 ti: ffff88005b300000 task.ti: ffff88005b300000 RIP: driver_register (drivers/base/driver.c:153 (discriminator 1)) RSP: 0000:ffff88005b307c68 EFLAGS: 00010282 RAX: 0000000000000000 RBX: ffffffffb3987c30 RCX: 1ffffffff68acf26 RDX: 0000000000000000 RSI: dffffc0000000000 RDI: ffffffffb4567930 RBP: ffff88005b307c88 R08: 1ffffffff606e3dc R09: dffffc0000000000 R10: 0000000080000000 R11: 1ffffffff79c42e2 R12: ffffffffb45678a0 R13: ffffffffb3987c38 R14: ffffffffb3987c30 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff880063e40000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000000 CR3: 0000000030023000 CR4: 00000000000406a0 Stack: 1ffff1000b660fa4 dffffc0000000000 0000000000000000 ffffffffb3987c00 ffff88005b307cf0 ffffffffa5bf826d ffff88005b307dc0 ffffffffb3987c30 ffffffffb3987ca8 ffffffffb39847e0 ffffffffb39847e0 00000000f673090a Call Trace: isa_register_driver (drivers/base/isa.c:123) dio48e_driver_init (drivers/gpio/gpio-104-dio-48e.c:396) do_one_initcall (init/main.c:770) kernel_init_freeable (init/main.c:834 init/main.c:843 init/main.c:861 init/main.c:1008) kernel_init (init/main.c:936) ret_from_fork (arch/x86/entry/entry_64.S:390) Code: be 00 00 00 00 00 fc ff df 48 89 f9 48 c1 e9 03 80 3c 31 00 74 05 e8 4a 18 be fd 49 83 bc 24 90 00 00 00 00 75 13 e8 2a 89 a0 fd <0f> 0b 48 c7 c7 40 21 51 b4 e8 6f 78 58 ff e8 17 89 a0 fd 49 8d All code ======== 0: be 00 00 00 00 mov $0x0,%esi 5: 00 fc add %bh,%ah 7: ff df lcallq *<internal disassembler error> 9: 48 89 f9 mov %rdi,%rcx c: 48 c1 e9 03 shr $0x3,%rcx 10: 80 3c 31 00 cmpb $0x0,(%rcx,%rsi,1) 14: 74 05 je 0x1b 16: e8 4a 18 be fd callq 0xfffffffffdbe1865 1b: 49 83 bc 24 90 00 00 cmpq $0x0,0x90(%r12) 22: 00 00 24: 75 13 jne 0x39 26: e8 2a 89 a0 fd callq 0xfffffffffda08955 2b:* 0f 0b ud2 <-- trapping instruction 2d: 48 c7 c7 40 21 51 b4 mov $0xffffffffb4512140,%rdi 34: e8 6f 78 58 ff callq 0xffffffffff5878a8 39: e8 17 89 a0 fd callq 0xfffffffffda08955 3e: 49 8d 00 lea (%r8),%rax Code starting with the faulting instruction =========================================== 0: 0f 0b ud2 2: 48 c7 c7 40 21 51 b4 mov $0xffffffffb4512140,%rdi 9: e8 6f 78 58 ff callq 0xffffffffff58787d e: e8 17 89 a0 fd callq 0xfffffffffda0892a 13: 49 8d 00 lea (%r8),%rax RIP driver_register (drivers/base/driver.c:153 (discriminator 1)) RSP <ffff88005b307c68> ---[ end trace 96103422392be10c ]--- ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <57336622.9070508-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver 2016-05-11 17:04 ` Sasha Levin @ 2016-05-11 19:34 ` William Breathitt Gray -1 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-05-11 19:34 UTC (permalink / raw) To: Sasha Levin Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r, tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linux-0h96xk9xTtrk1uMJSBkQmQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA On Wed, May 11, 2016 at 01:04:34PM -0400, Sasha Levin wrote: >On 04/07/2016 10:47 AM, 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> > >Hey William, > >I'm seeing this on boot: > >kernel BUG at drivers/base/driver.c:153! >invalid opcode: 0000 [#1] PREEMPT SMP KASAN >Modules linked in: >CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.6.0-rc7-next-20160511-sasha-00024-g13dfe33 #3081 >task: ffff88005b2f8000 ti: ffff88005b300000 task.ti: ffff88005b300000 >RIP: driver_register (drivers/base/driver.c:153 (discriminator 1)) >RSP: 0000:ffff88005b307c68 EFLAGS: 00010282 >RAX: 0000000000000000 RBX: ffffffffb3987c30 RCX: 1ffffffff68acf26 >RDX: 0000000000000000 RSI: dffffc0000000000 RDI: ffffffffb4567930 >RBP: ffff88005b307c88 R08: 1ffffffff606e3dc R09: dffffc0000000000 >R10: 0000000080000000 R11: 1ffffffff79c42e2 R12: ffffffffb45678a0 >R13: ffffffffb3987c38 R14: ffffffffb3987c30 R15: 0000000000000000 >FS: 0000000000000000(0000) GS:ffff880063e40000(0000) knlGS:0000000000000000 >CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >CR2: 0000000000000000 CR3: 0000000030023000 CR4: 00000000000406a0 >Stack: >1ffff1000b660fa4 dffffc0000000000 0000000000000000 ffffffffb3987c00 >ffff88005b307cf0 ffffffffa5bf826d ffff88005b307dc0 ffffffffb3987c30 >ffffffffb3987ca8 ffffffffb39847e0 ffffffffb39847e0 00000000f673090a >Call Trace: >isa_register_driver (drivers/base/isa.c:123) >dio48e_driver_init (drivers/gpio/gpio-104-dio-48e.c:396) >do_one_initcall (init/main.c:770) >kernel_init_freeable (init/main.c:834 init/main.c:843 init/main.c:861 init/main.c:1008) >kernel_init (init/main.c:936) >ret_from_fork (arch/x86/entry/entry_64.S:390) >Code: be 00 00 00 00 00 fc ff df 48 89 f9 48 c1 e9 03 80 3c 31 00 74 05 e8 4a 18 be fd 49 83 bc 24 90 00 00 00 00 75 13 e8 2a 89 a0 fd <0f> 0b 48 c7 c7 40 21 51 b4 e8 6f 78 58 ff e8 17 89 a0 fd 49 8d >All code >======== > 0: be 00 00 00 00 mov $0x0,%esi > 5: 00 fc add %bh,%ah > 7: ff df lcallq *<internal disassembler error> > 9: 48 89 f9 mov %rdi,%rcx > c: 48 c1 e9 03 shr $0x3,%rcx > 10: 80 3c 31 00 cmpb $0x0,(%rcx,%rsi,1) > 14: 74 05 je 0x1b > 16: e8 4a 18 be fd callq 0xfffffffffdbe1865 > 1b: 49 83 bc 24 90 00 00 cmpq $0x0,0x90(%r12) > 22: 00 00 > 24: 75 13 jne 0x39 > 26: e8 2a 89 a0 fd callq 0xfffffffffda08955 > 2b:* 0f 0b ud2 <-- trapping instruction > 2d: 48 c7 c7 40 21 51 b4 mov $0xffffffffb4512140,%rdi > 34: e8 6f 78 58 ff callq 0xffffffffff5878a8 > 39: e8 17 89 a0 fd callq 0xfffffffffda08955 > 3e: 49 8d 00 lea (%r8),%rax > >Code starting with the faulting instruction >=========================================== > 0: 0f 0b ud2 > 2: 48 c7 c7 40 21 51 b4 mov $0xffffffffb4512140,%rdi > 9: e8 6f 78 58 ff callq 0xffffffffff58787d > e: e8 17 89 a0 fd callq 0xfffffffffda0892a > 13: 49 8d 00 lea (%r8),%rax >RIP driver_register (drivers/base/driver.c:153 (discriminator 1)) >RSP <ffff88005b307c68> >---[ end trace 96103422392be10c ]--- Hi Sasha, I believe this bug arose from a BUG_ON inside driver_register (specifically line 153 of drivers/base/driver.c): BUG_ON(!drv->bus->p) The 'p' member of the struct bus_type has not been initialized by the time driver_register was called for the gpio-104-dio-48e driver. This member is typically initialized by bus_register, which is called inside isa_bus_init (drivers/base/isa.c). The isa_bus_init function address is set to be called by the device_initcall macro. This causes a race condition between the isa_bus_init function and isa drivers init functions (which use module_init). I believe this can be resolved by changing device_initcall to postcore_initcall, thus ensuring that isa_bus_init is called before any isa drivers are registered. I will implement a fix and test it out, then submit the patch if I don't encounter any errors. Thanks, William Breathitt Gray ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver @ 2016-05-11 19:34 ` William Breathitt Gray 0 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-05-11 19:34 UTC (permalink / raw) To: Sasha Levin Cc: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio On Wed, May 11, 2016 at 01:04:34PM -0400, Sasha Levin wrote: >On 04/07/2016 10:47 AM, 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> > >Hey William, > >I'm seeing this on boot: > >kernel BUG at drivers/base/driver.c:153! >invalid opcode: 0000 [#1] PREEMPT SMP KASAN >Modules linked in: >CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.6.0-rc7-next-20160511-sasha-00024-g13dfe33 #3081 >task: ffff88005b2f8000 ti: ffff88005b300000 task.ti: ffff88005b300000 >RIP: driver_register (drivers/base/driver.c:153 (discriminator 1)) >RSP: 0000:ffff88005b307c68 EFLAGS: 00010282 >RAX: 0000000000000000 RBX: ffffffffb3987c30 RCX: 1ffffffff68acf26 >RDX: 0000000000000000 RSI: dffffc0000000000 RDI: ffffffffb4567930 >RBP: ffff88005b307c88 R08: 1ffffffff606e3dc R09: dffffc0000000000 >R10: 0000000080000000 R11: 1ffffffff79c42e2 R12: ffffffffb45678a0 >R13: ffffffffb3987c38 R14: ffffffffb3987c30 R15: 0000000000000000 >FS: 0000000000000000(0000) GS:ffff880063e40000(0000) knlGS:0000000000000000 >CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >CR2: 0000000000000000 CR3: 0000000030023000 CR4: 00000000000406a0 >Stack: >1ffff1000b660fa4 dffffc0000000000 0000000000000000 ffffffffb3987c00 >ffff88005b307cf0 ffffffffa5bf826d ffff88005b307dc0 ffffffffb3987c30 >ffffffffb3987ca8 ffffffffb39847e0 ffffffffb39847e0 00000000f673090a >Call Trace: >isa_register_driver (drivers/base/isa.c:123) >dio48e_driver_init (drivers/gpio/gpio-104-dio-48e.c:396) >do_one_initcall (init/main.c:770) >kernel_init_freeable (init/main.c:834 init/main.c:843 init/main.c:861 init/main.c:1008) >kernel_init (init/main.c:936) >ret_from_fork (arch/x86/entry/entry_64.S:390) >Code: be 00 00 00 00 00 fc ff df 48 89 f9 48 c1 e9 03 80 3c 31 00 74 05 e8 4a 18 be fd 49 83 bc 24 90 00 00 00 00 75 13 e8 2a 89 a0 fd <0f> 0b 48 c7 c7 40 21 51 b4 e8 6f 78 58 ff e8 17 89 a0 fd 49 8d >All code >======== > 0: be 00 00 00 00 mov $0x0,%esi > 5: 00 fc add %bh,%ah > 7: ff df lcallq *<internal disassembler error> > 9: 48 89 f9 mov %rdi,%rcx > c: 48 c1 e9 03 shr $0x3,%rcx > 10: 80 3c 31 00 cmpb $0x0,(%rcx,%rsi,1) > 14: 74 05 je 0x1b > 16: e8 4a 18 be fd callq 0xfffffffffdbe1865 > 1b: 49 83 bc 24 90 00 00 cmpq $0x0,0x90(%r12) > 22: 00 00 > 24: 75 13 jne 0x39 > 26: e8 2a 89 a0 fd callq 0xfffffffffda08955 > 2b:* 0f 0b ud2 <-- trapping instruction > 2d: 48 c7 c7 40 21 51 b4 mov $0xffffffffb4512140,%rdi > 34: e8 6f 78 58 ff callq 0xffffffffff5878a8 > 39: e8 17 89 a0 fd callq 0xfffffffffda08955 > 3e: 49 8d 00 lea (%r8),%rax > >Code starting with the faulting instruction >=========================================== > 0: 0f 0b ud2 > 2: 48 c7 c7 40 21 51 b4 mov $0xffffffffb4512140,%rdi > 9: e8 6f 78 58 ff callq 0xffffffffff58787d > e: e8 17 89 a0 fd callq 0xfffffffffda0892a > 13: 49 8d 00 lea (%r8),%rax >RIP driver_register (drivers/base/driver.c:153 (discriminator 1)) >RSP <ffff88005b307c68> >---[ end trace 96103422392be10c ]--- Hi Sasha, I believe this bug arose from a BUG_ON inside driver_register (specifically line 153 of drivers/base/driver.c): BUG_ON(!drv->bus->p) The 'p' member of the struct bus_type has not been initialized by the time driver_register was called for the gpio-104-dio-48e driver. This member is typically initialized by bus_register, which is called inside isa_bus_init (drivers/base/isa.c). The isa_bus_init function address is set to be called by the device_initcall macro. This causes a race condition between the isa_bus_init function and isa drivers init functions (which use module_init). I believe this can be resolved by changing device_initcall to postcore_initcall, thus ensuring that isa_bus_init is called before any isa drivers are registered. I will implement a fix and test it out, then submit the patch if I don't encounter any errors. Thanks, William Breathitt Gray ^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH 07/10] gpio: 104-dio-48e: Utilize the ISA bus driver 2016-04-07 14:47 ` William Breathitt Gray ` (6 preceding siblings ...) (?) @ 2016-04-07 14:47 ` William Breathitt Gray -1 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio, William Breathitt Gray The ACCES 104-DIO-48E series communicates via the ISA bus. As such, it is more appropriate to use the ISA bus driver over the platform driver to control the ACCES 104-DIO-48E GPIO driver. This patch also adds support for multiple devices via the base and irq module array parameters. 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. Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> --- drivers/gpio/Kconfig | 9 ++-- drivers/gpio/gpio-104-dio-48e.c | 106 ++++++++++++++-------------------------- 2 files changed, 43 insertions(+), 72 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 5f3429f..4ce3f8d 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -517,12 +517,13 @@ menu "Port-mapped I/O GPIO drivers" config GPIO_104_DIO_48E tristate "ACCES 104-DIO-48E GPIO support" + depends on ISA_BUS select GPIOLIB_IRQCHIP help - Enables GPIO support for the ACCES 104-DIO-48E family. The base port - address for the device may be configured via the dio_48e_base module - parameter. The interrupt line number for the device may be configured - via the dio_48e_irq module parameter. + Enables GPIO support for the ACCES 104-DIO-48E series (104-DIO-48E, + 104-DIO-24E). The base port addresses for the devices may be + configured via the base module parameter. The interrupt line numbers + for the devices may be configured via the irq module parameter. config GPIO_104_IDIO_16 tristate "ACCES 104-IDIO-16 GPIO support" diff --git a/drivers/gpio/gpio-104-dio-48e.c b/drivers/gpio/gpio-104-dio-48e.c index 448a903..1a647c0 100644 --- a/drivers/gpio/gpio-104-dio-48e.c +++ b/drivers/gpio/gpio-104-dio-48e.c @@ -1,5 +1,5 @@ /* - * GPIO driver for the ACCES 104-DIO-48E + * GPIO driver for the ACCES 104-DIO-48E series * Copyright (C) 2016 William Breathitt Gray * * This program is free software; you can redistribute it and/or modify @@ -10,6 +10,9 @@ * WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * General Public License for more details. + * + * This driver supports the following ACCES devices: 104-DIO-48E and + * 104-DIO-24E. */ #include <linux/bitops.h> #include <linux/device.h> @@ -19,18 +22,23 @@ #include <linux/ioport.h> #include <linux/interrupt.h> #include <linux/irqdesc.h> +#include <linux/isa.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/moduleparam.h> -#include <linux/platform_device.h> #include <linux/spinlock.h> -static unsigned dio_48e_base; -module_param(dio_48e_base, uint, 0); -MODULE_PARM_DESC(dio_48e_base, "ACCES 104-DIO-48E base address"); -static unsigned dio_48e_irq; -module_param(dio_48e_irq, uint, 0); -MODULE_PARM_DESC(dio_48e_irq, "ACCES 104-DIO-48E interrupt line number"); +#define DIO48E_EXTENT 16 +#define MAX_NUM_DIO48E max_num_isa_dev(DIO48E_EXTENT) + +static unsigned int base[MAX_NUM_DIO48E]; +static unsigned int num_dio48e; +module_param_array(base, uint, &num_dio48e, 0); +MODULE_PARM_DESC(base, "ACCES 104-DIO-48E base addresses"); + +static unsigned int irq[MAX_NUM_DIO48E]; +module_param_array(irq, uint, NULL, 0); +MODULE_PARM_DESC(irq, "ACCES 104-DIO-48E interrupt line numbers"); /** * struct dio48e_gpio - GPIO device private data structure @@ -294,23 +302,19 @@ static irqreturn_t dio48e_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } -static int __init dio48e_probe(struct platform_device *pdev) +static int dio48e_probe(struct device *dev, unsigned int id) { - struct device *dev = &pdev->dev; struct dio48e_gpio *dio48egpio; - const unsigned base = dio_48e_base; - const unsigned extent = 16; const char *const name = dev_name(dev); int err; - const unsigned irq = dio_48e_irq; dio48egpio = devm_kzalloc(dev, sizeof(*dio48egpio), GFP_KERNEL); if (!dio48egpio) return -ENOMEM; - if (!devm_request_region(dev, base, extent, name)) { + if (!devm_request_region(dev, base[id], DIO48E_EXTENT, name)) { dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n", - base, base + extent); + base[id], base[id] + DIO48E_EXTENT); return -EBUSY; } @@ -324,8 +328,8 @@ static int __init dio48e_probe(struct platform_device *pdev) dio48egpio->chip.direction_output = dio48e_gpio_direction_output; dio48egpio->chip.get = dio48e_gpio_get; dio48egpio->chip.set = dio48e_gpio_set; - dio48egpio->base = base; - dio48egpio->irq = irq; + dio48egpio->base = base[id]; + dio48egpio->irq = irq[id]; spin_lock_init(&dio48egpio->lock); @@ -338,19 +342,19 @@ static int __init dio48e_probe(struct platform_device *pdev) } /* initialize all GPIO as output */ - outb(0x80, base + 3); - outb(0x00, base); - outb(0x00, base + 1); - outb(0x00, base + 2); - outb(0x00, base + 3); - outb(0x80, base + 7); - outb(0x00, base + 4); - outb(0x00, base + 5); - outb(0x00, base + 6); - outb(0x00, base + 7); + outb(0x80, base[id] + 3); + outb(0x00, base[id]); + outb(0x00, base[id] + 1); + outb(0x00, base[id] + 2); + outb(0x00, base[id] + 3); + outb(0x80, base[id] + 7); + outb(0x00, base[id] + 4); + outb(0x00, base[id] + 5); + outb(0x00, base[id] + 6); + outb(0x00, base[id] + 7); /* disable IRQ by default */ - inb(base + 0xB); + inb(base[id] + 0xB); err = gpiochip_irqchip_add(&dio48egpio->chip, &dio48e_irqchip, 0, handle_edge_irq, IRQ_TYPE_NONE); @@ -359,7 +363,7 @@ static int __init dio48e_probe(struct platform_device *pdev) goto err_gpiochip_remove; } - err = request_irq(irq, dio48e_irq_handler, 0, name, dio48egpio); + err = request_irq(irq[id], dio48e_irq_handler, 0, name, dio48egpio); if (err) { dev_err(dev, "IRQ handler registering failed (%d)\n", err); goto err_gpiochip_remove; @@ -372,9 +376,9 @@ err_gpiochip_remove: return err; } -static int dio48e_remove(struct platform_device *pdev) +static int dio48e_remove(struct device *dev, unsigned int id) { - struct dio48e_gpio *const dio48egpio = platform_get_drvdata(pdev); + struct dio48e_gpio *const dio48egpio = dev_get_drvdata(dev); free_irq(dio48egpio->irq, dio48egpio); gpiochip_remove(&dio48egpio->chip); @@ -382,48 +386,14 @@ static int dio48e_remove(struct platform_device *pdev) return 0; } -static struct platform_device *dio48e_device; - -static struct platform_driver dio48e_driver = { +static struct isa_driver dio48e_driver = { + .probe = dio48e_probe, .driver = { .name = "104-dio-48e" }, .remove = dio48e_remove }; - -static void __exit dio48e_exit(void) -{ - platform_device_unregister(dio48e_device); - platform_driver_unregister(&dio48e_driver); -} - -static int __init dio48e_init(void) -{ - int err; - - dio48e_device = platform_device_alloc(dio48e_driver.driver.name, -1); - if (!dio48e_device) - return -ENOMEM; - - err = platform_device_add(dio48e_device); - if (err) - goto err_platform_device; - - err = platform_driver_probe(&dio48e_driver, dio48e_probe); - if (err) - goto err_platform_driver; - - return 0; - -err_platform_driver: - platform_device_del(dio48e_device); -err_platform_device: - platform_device_put(dio48e_device); - return err; -} - -module_init(dio48e_init); -module_exit(dio48e_exit); +module_isa_driver(dio48e_driver, num_dio48e); MODULE_AUTHOR("William Breathitt Gray <vilhelm.gray@gmail.com>"); MODULE_DESCRIPTION("ACCES 104-DIO-48E GPIO driver"); -- 2.7.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 08/10] gpio: 104-idi-48: Utilize the ISA bus driver 2016-04-07 14:47 ` William Breathitt Gray ` (7 preceding siblings ...) (?) @ 2016-04-07 14:47 ` William Breathitt Gray -1 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio, William Breathitt Gray The ACCES 104-IDI-48 series communicates via the ISA bus. As such, it is more appropriate to use the ISA bus driver over the platform driver to control the ACCES 104-IDI-48 GPIO driver. This patch also adds support for multiple devices via the base and irq module array parameters. 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. Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> --- drivers/gpio/Kconfig | 10 +++-- drivers/gpio/gpio-104-idi-48.c | 86 ++++++++++++++---------------------------- 2 files changed, 34 insertions(+), 62 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 4ce3f8d..46db6fc 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -536,12 +536,14 @@ config GPIO_104_IDIO_16 config GPIO_104_IDI_48 tristate "ACCES 104-IDI-48 GPIO support" + depends on ISA_BUS select GPIOLIB_IRQCHIP help - Enables GPIO support for the ACCES 104-IDI-48 family. The base port - address for the device may be configured via the idi_48_base module - parameter. The interrupt line number for the device may be configured - via the idi_48_irq module parameter. + Enables GPIO support for the ACCES 104-IDI-48 family (104-IDI-48A, + 104-IDI-48AC, 104-IDI-48B, 104-IDI-48BC). The base port addresses for + the devices may be configured via the base module parameter. The + interrupt line numbers for the devices may be configured via the irq + module parameter. config GPIO_F7188X tristate "F71869, F71869A, F71882FG, F71889F and F81866 GPIO support" diff --git a/drivers/gpio/gpio-104-idi-48.c b/drivers/gpio/gpio-104-idi-48.c index e37cd4c..6c75c83 100644 --- a/drivers/gpio/gpio-104-idi-48.c +++ b/drivers/gpio/gpio-104-idi-48.c @@ -10,6 +10,9 @@ * WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * General Public License for more details. + * + * This driver supports the following ACCES devices: 104-IDI-48A, + * 104-IDI-48AC, 104-IDI-48B, and 104-IDI-48BC. */ #include <linux/bitops.h> #include <linux/device.h> @@ -19,18 +22,23 @@ #include <linux/ioport.h> #include <linux/interrupt.h> #include <linux/irqdesc.h> +#include <linux/isa.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/moduleparam.h> -#include <linux/platform_device.h> #include <linux/spinlock.h> -static unsigned idi_48_base; -module_param(idi_48_base, uint, 0); -MODULE_PARM_DESC(idi_48_base, "ACCES 104-IDI-48 base address"); -static unsigned idi_48_irq; -module_param(idi_48_irq, uint, 0); -MODULE_PARM_DESC(idi_48_irq, "ACCES 104-IDI-48 interrupt line number"); +#define IDI_48_EXTENT 8 +#define MAX_NUM_IDI_48 max_num_isa_dev(IDI_48_EXTENT) + +static unsigned int base[MAX_NUM_IDI_48]; +static unsigned int num_idi_48; +module_param_array(base, uint, &num_idi_48, 0); +MODULE_PARM_DESC(base, "ACCES 104-IDI-48 base addresses"); + +static unsigned int irq[MAX_NUM_IDI_48]; +module_param_array(irq, uint, NULL, 0); +MODULE_PARM_DESC(irq, "ACCES 104-IDI-48 interrupt line numbers"); /** * struct idi_48_gpio - GPIO device private data structure @@ -211,23 +219,19 @@ static irqreturn_t idi_48_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } -static int __init idi_48_probe(struct platform_device *pdev) +static int idi_48_probe(struct device *dev, unsigned int id) { - struct device *dev = &pdev->dev; struct idi_48_gpio *idi48gpio; - const unsigned base = idi_48_base; - const unsigned extent = 8; const char *const name = dev_name(dev); int err; - const unsigned irq = idi_48_irq; idi48gpio = devm_kzalloc(dev, sizeof(*idi48gpio), GFP_KERNEL); if (!idi48gpio) return -ENOMEM; - if (!devm_request_region(dev, base, extent, name)) { + if (!devm_request_region(dev, base[id], IDI_48_EXTENT, name)) { dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n", - base, base + extent); + base[id], base[id] + IDI_48_EXTENT); return -EBUSY; } @@ -239,8 +243,8 @@ static int __init idi_48_probe(struct platform_device *pdev) idi48gpio->chip.get_direction = idi_48_gpio_get_direction; idi48gpio->chip.direction_input = idi_48_gpio_direction_input; idi48gpio->chip.get = idi_48_gpio_get; - idi48gpio->base = base; - idi48gpio->irq = irq; + idi48gpio->base = base[id]; + idi48gpio->irq = irq[id]; spin_lock_init(&idi48gpio->lock); @@ -253,8 +257,8 @@ static int __init idi_48_probe(struct platform_device *pdev) } /* Disable IRQ by default */ - outb(0, base + 7); - inb(base + 7); + outb(0, base[id] + 7); + inb(base[id] + 7); err = gpiochip_irqchip_add(&idi48gpio->chip, &idi_48_irqchip, 0, handle_edge_irq, IRQ_TYPE_NONE); @@ -263,7 +267,7 @@ static int __init idi_48_probe(struct platform_device *pdev) goto err_gpiochip_remove; } - err = request_irq(irq, idi_48_irq_handler, IRQF_SHARED, name, + err = request_irq(irq[id], idi_48_irq_handler, IRQF_SHARED, name, idi48gpio); if (err) { dev_err(dev, "IRQ handler registering failed (%d)\n", err); @@ -277,9 +281,9 @@ err_gpiochip_remove: return err; } -static int idi_48_remove(struct platform_device *pdev) +static int idi_48_remove(struct device *dev, unsigned int id) { - struct idi_48_gpio *const idi48gpio = platform_get_drvdata(pdev); + struct idi_48_gpio *const idi48gpio = dev_get_drvdata(dev); free_irq(idi48gpio->irq, idi48gpio); gpiochip_remove(&idi48gpio->chip); @@ -287,48 +291,14 @@ static int idi_48_remove(struct platform_device *pdev) return 0; } -static struct platform_device *idi_48_device; - -static struct platform_driver idi_48_driver = { +static struct isa_driver idi_48_driver = { + .probe = idi_48_probe, .driver = { .name = "104-idi-48" }, .remove = idi_48_remove }; - -static void __exit idi_48_exit(void) -{ - platform_device_unregister(idi_48_device); - platform_driver_unregister(&idi_48_driver); -} - -static int __init idi_48_init(void) -{ - int err; - - idi_48_device = platform_device_alloc(idi_48_driver.driver.name, -1); - if (!idi_48_device) - return -ENOMEM; - - err = platform_device_add(idi_48_device); - if (err) - goto err_platform_device; - - err = platform_driver_probe(&idi_48_driver, idi_48_probe); - if (err) - goto err_platform_driver; - - return 0; - -err_platform_driver: - platform_device_del(idi_48_device); -err_platform_device: - platform_device_put(idi_48_device); - return err; -} - -module_init(idi_48_init); -module_exit(idi_48_exit); +module_isa_driver(idi_48_driver, num_idi_48); MODULE_AUTHOR("William Breathitt Gray <vilhelm.gray@gmail.com>"); MODULE_DESCRIPTION("ACCES 104-IDI-48 GPIO driver"); -- 2.7.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 09/10] gpio: 104-idio-16: Utilize the ISA bus driver 2016-04-07 14:47 ` William Breathitt Gray ` (8 preceding siblings ...) (?) @ 2016-04-07 14:47 ` William Breathitt Gray -1 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio, William Breathitt Gray The ACCES 104-IDIO-16 series communicates via the ISA bus. As such, it is more appropriate to use the ISA bus driver over the platform driver to control the ACCES 104-IDIO-16 GPIO driver. This patch also adds support for multiple devices via the base and irq module array parameters. 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. Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> --- drivers/gpio/Kconfig | 10 +++-- drivers/gpio/gpio-104-idio-16.c | 85 ++++++++++++++--------------------------- 2 files changed, 34 insertions(+), 61 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 46db6fc..5ee6706 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -527,12 +527,14 @@ config GPIO_104_DIO_48E config GPIO_104_IDIO_16 tristate "ACCES 104-IDIO-16 GPIO support" + depends on ISA_BUS select GPIOLIB_IRQCHIP help - Enables GPIO support for the ACCES 104-IDIO-16 family. The base port - address for the device may be set via the idio_16_base module - parameter. The interrupt line number for the device may be set via the - idio_16_irq module parameter. + Enables GPIO support for the ACCES 104-IDIO-16 family (104-IDIO-16, + 104-IDIO-16E, 104-IDO-16, 104-IDIO-8, 104-IDIO-8E, 104-IDO-8). The + base port addresses for the devices may be configured via the base + module parameter. The interrupt line numbers for the devices may be + configured via the irq module parameter. config GPIO_104_IDI_48 tristate "ACCES 104-IDI-48 GPIO support" diff --git a/drivers/gpio/gpio-104-idio-16.c b/drivers/gpio/gpio-104-idio-16.c index ecc85fe..6787b8f 100644 --- a/drivers/gpio/gpio-104-idio-16.c +++ b/drivers/gpio/gpio-104-idio-16.c @@ -10,6 +10,9 @@ * WITHOUT ANY WARRANTY; without even the implied warranty of * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU * General Public License for more details. + * + * This driver supports the following ACCES devices: 104-IDIO-16, + * 104-IDIO-16E, 104-IDO-16, 104-IDIO-8, 104-IDIO-8E, and 104-IDO-8. */ #include <linux/bitops.h> #include <linux/device.h> @@ -19,18 +22,23 @@ #include <linux/ioport.h> #include <linux/interrupt.h> #include <linux/irqdesc.h> +#include <linux/isa.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/moduleparam.h> -#include <linux/platform_device.h> #include <linux/spinlock.h> -static unsigned idio_16_base; -module_param(idio_16_base, uint, 0); -MODULE_PARM_DESC(idio_16_base, "ACCES 104-IDIO-16 base address"); -static unsigned idio_16_irq; -module_param(idio_16_irq, uint, 0); -MODULE_PARM_DESC(idio_16_irq, "ACCES 104-IDIO-16 interrupt line number"); +#define IDIO_16_EXTENT 8 +#define MAX_NUM_IDIO_16 max_num_isa_dev(IDIO_16_EXTENT) + +static unsigned int base[MAX_NUM_IDIO_16]; +static unsigned int num_idio_16; +module_param_array(base, uint, &num_idio_16, 0); +MODULE_PARM_DESC(base, "ACCES 104-IDIO-16 base addresses"); + +static unsigned int irq[MAX_NUM_IDIO_16]; +module_param_array(irq, uint, NULL, 0); +MODULE_PARM_DESC(irq, "ACCES 104-IDIO-16 interrupt line numbers"); /** * struct idio_16_gpio - GPIO device private data structure @@ -185,23 +193,19 @@ static irqreturn_t idio_16_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } -static int __init idio_16_probe(struct platform_device *pdev) +static int idio_16_probe(struct device *dev, unsigned int id) { - struct device *dev = &pdev->dev; struct idio_16_gpio *idio16gpio; - const unsigned base = idio_16_base; - const unsigned extent = 8; const char *const name = dev_name(dev); int err; - const unsigned irq = idio_16_irq; idio16gpio = devm_kzalloc(dev, sizeof(*idio16gpio), GFP_KERNEL); if (!idio16gpio) return -ENOMEM; - if (!devm_request_region(dev, base, extent, name)) { + if (!devm_request_region(dev, base[id], IDIO_16_EXTENT, name)) { dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n", - base, base + extent); + base[id], base[id] + IDIO_16_EXTENT); return -EBUSY; } @@ -215,8 +219,8 @@ static int __init idio_16_probe(struct platform_device *pdev) idio16gpio->chip.direction_output = idio_16_gpio_direction_output; idio16gpio->chip.get = idio_16_gpio_get; idio16gpio->chip.set = idio_16_gpio_set; - idio16gpio->base = base; - idio16gpio->irq = irq; + idio16gpio->base = base[id]; + idio16gpio->irq = irq[id]; idio16gpio->out_state = 0xFFFF; spin_lock_init(&idio16gpio->lock); @@ -230,8 +234,8 @@ static int __init idio_16_probe(struct platform_device *pdev) } /* Disable IRQ by default */ - outb(0, base + 2); - outb(0, base + 1); + outb(0, base[id] + 2); + outb(0, base[id] + 1); err = gpiochip_irqchip_add(&idio16gpio->chip, &idio_16_irqchip, 0, handle_edge_irq, IRQ_TYPE_NONE); @@ -240,7 +244,7 @@ static int __init idio_16_probe(struct platform_device *pdev) goto err_gpiochip_remove; } - err = request_irq(irq, idio_16_irq_handler, 0, name, idio16gpio); + err = request_irq(irq[id], idio_16_irq_handler, 0, name, idio16gpio); if (err) { dev_err(dev, "IRQ handler registering failed (%d)\n", err); goto err_gpiochip_remove; @@ -253,9 +257,9 @@ err_gpiochip_remove: return err; } -static int idio_16_remove(struct platform_device *pdev) +static int idio_16_remove(struct device *dev, unsigned int id) { - struct idio_16_gpio *const idio16gpio = platform_get_drvdata(pdev); + struct idio_16_gpio *const idio16gpio = dev_get_drvdata(dev); free_irq(idio16gpio->irq, idio16gpio); gpiochip_remove(&idio16gpio->chip); @@ -263,48 +267,15 @@ static int idio_16_remove(struct platform_device *pdev) return 0; } -static struct platform_device *idio_16_device; - -static struct platform_driver idio_16_driver = { +static struct isa_driver idio_16_driver = { + .probe = idio_16_probe, .driver = { .name = "104-idio-16" }, .remove = idio_16_remove }; -static void __exit idio_16_exit(void) -{ - platform_device_unregister(idio_16_device); - platform_driver_unregister(&idio_16_driver); -} - -static int __init idio_16_init(void) -{ - int err; - - idio_16_device = platform_device_alloc(idio_16_driver.driver.name, -1); - if (!idio_16_device) - return -ENOMEM; - - err = platform_device_add(idio_16_device); - if (err) - goto err_platform_device; - - err = platform_driver_probe(&idio_16_driver, idio_16_probe); - if (err) - goto err_platform_driver; - - return 0; - -err_platform_driver: - platform_device_del(idio_16_device); -err_platform_device: - platform_device_put(idio_16_device); - return err; -} - -module_init(idio_16_init); -module_exit(idio_16_exit); +module_isa_driver(idio_16_driver, num_idio_16); MODULE_AUTHOR("William Breathitt Gray <vilhelm.gray@gmail.com>"); MODULE_DESCRIPTION("ACCES 104-IDIO-16 GPIO driver"); -- 2.7.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH 10/10] gpio: ws16c48: Utilize the ISA bus driver 2016-04-07 14:47 ` William Breathitt Gray ` (9 preceding siblings ...) (?) @ 2016-04-07 14:47 ` William Breathitt Gray -1 siblings, 0 replies; 39+ messages in thread From: William Breathitt Gray @ 2016-04-07 14:47 UTC (permalink / raw) To: gregkh, tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou Cc: linux-kernel, linux-iio, linux-watchdog, linux-gpio, William Breathitt Gray The WinSystems WS16C48 communicates via the ISA bus. As such, it is more appropriate to use the ISA bus driver over the platform driver to control the WinSystems WS16C48 GPIO driver. This patch also adds support for multiple devices via the base and irq module array parameters. 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. Signed-off-by: William Breathitt Gray <vilhelm.gray@gmail.com> --- drivers/gpio/Kconfig | 9 ++--- drivers/gpio/gpio-ws16c48.c | 88 +++++++++++++++------------------------------ 2 files changed, 33 insertions(+), 64 deletions(-) diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index 5ee6706..25f2553 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -614,12 +614,13 @@ config GPIO_TS5500 config GPIO_WS16C48 tristate "WinSystems WS16C48 GPIO support" + depends on ISA_BUS select GPIOLIB_IRQCHIP help - Enables GPIO support for the WinSystems WS16C48. The base port address - for the device may be configured via the ws16c48_base module - parameter. The interrupt line number for the device may be configured - via the ws16c48_irq module parameter. + Enables GPIO support for the WinSystems WS16C48. The base port + addresses for the devices may be configured via the base module + parameter. The interrupt line numbers for the devices may be + configured via the irq module parameter. endmenu diff --git a/drivers/gpio/gpio-ws16c48.c b/drivers/gpio/gpio-ws16c48.c index 51f41e8..eaa71d4 100644 --- a/drivers/gpio/gpio-ws16c48.c +++ b/drivers/gpio/gpio-ws16c48.c @@ -19,18 +19,23 @@ #include <linux/ioport.h> #include <linux/interrupt.h> #include <linux/irqdesc.h> +#include <linux/isa.h> #include <linux/kernel.h> #include <linux/module.h> #include <linux/moduleparam.h> -#include <linux/platform_device.h> #include <linux/spinlock.h> -static unsigned ws16c48_base; -module_param(ws16c48_base, uint, 0); -MODULE_PARM_DESC(ws16c48_base, "WinSystems WS16C48 base address"); -static unsigned ws16c48_irq; -module_param(ws16c48_irq, uint, 0); -MODULE_PARM_DESC(ws16c48_irq, "WinSystems WS16C48 interrupt line number"); +#define WS16C48_EXTENT 16 +#define MAX_NUM_WS16C48 max_num_isa_dev(WS16C48_EXTENT) + +static unsigned int base[MAX_NUM_WS16C48]; +static unsigned int num_ws16c48; +module_param_array(base, uint, &num_ws16c48, 0); +MODULE_PARM_DESC(base, "WinSystems WS16C48 base addresses"); + +static unsigned int irq[MAX_NUM_WS16C48]; +module_param_array(irq, uint, NULL, 0); +MODULE_PARM_DESC(irq, "WinSystems WS16C48 interrupt line numbers"); /** * struct ws16c48_gpio - GPIO device private data structure @@ -298,23 +303,19 @@ static irqreturn_t ws16c48_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } -static int __init ws16c48_probe(struct platform_device *pdev) +static int ws16c48_probe(struct device *dev, unsigned int id) { - struct device *dev = &pdev->dev; struct ws16c48_gpio *ws16c48gpio; - const unsigned base = ws16c48_base; - const unsigned extent = 16; const char *const name = dev_name(dev); int err; - const unsigned irq = ws16c48_irq; ws16c48gpio = devm_kzalloc(dev, sizeof(*ws16c48gpio), GFP_KERNEL); if (!ws16c48gpio) return -ENOMEM; - if (!devm_request_region(dev, base, extent, name)) { + if (!devm_request_region(dev, base[id], WS16C48_EXTENT, name)) { dev_err(dev, "Unable to lock port addresses (0x%X-0x%X)\n", - base, base + extent); + base[id], base[id] + WS16C48_EXTENT); return -EBUSY; } @@ -328,8 +329,8 @@ static int __init ws16c48_probe(struct platform_device *pdev) ws16c48gpio->chip.direction_output = ws16c48_gpio_direction_output; ws16c48gpio->chip.get = ws16c48_gpio_get; ws16c48gpio->chip.set = ws16c48_gpio_set; - ws16c48gpio->base = base; - ws16c48gpio->irq = irq; + ws16c48gpio->base = base[id]; + ws16c48gpio->irq = irq[id]; spin_lock_init(&ws16c48gpio->lock); @@ -342,11 +343,11 @@ static int __init ws16c48_probe(struct platform_device *pdev) } /* Disable IRQ by default */ - outb(0x80, base + 7); - outb(0, base + 8); - outb(0, base + 9); - outb(0, base + 10); - outb(0xC0, base + 7); + outb(0x80, base[id] + 7); + outb(0, base[id] + 8); + outb(0, base[id] + 9); + outb(0, base[id] + 10); + outb(0xC0, base[id] + 7); err = gpiochip_irqchip_add(&ws16c48gpio->chip, &ws16c48_irqchip, 0, handle_edge_irq, IRQ_TYPE_NONE); @@ -355,7 +356,7 @@ static int __init ws16c48_probe(struct platform_device *pdev) goto err_gpiochip_remove; } - err = request_irq(irq, ws16c48_irq_handler, IRQF_SHARED, name, + err = request_irq(irq[id], ws16c48_irq_handler, IRQF_SHARED, name, ws16c48gpio); if (err) { dev_err(dev, "IRQ handler registering failed (%d)\n", err); @@ -369,9 +370,9 @@ err_gpiochip_remove: return err; } -static int ws16c48_remove(struct platform_device *pdev) +static int ws16c48_remove(struct device *dev, unsigned int id) { - struct ws16c48_gpio *const ws16c48gpio = platform_get_drvdata(pdev); + struct ws16c48_gpio *const ws16c48gpio = dev_get_drvdata(dev); free_irq(ws16c48gpio->irq, ws16c48gpio); gpiochip_remove(&ws16c48gpio->chip); @@ -379,48 +380,15 @@ static int ws16c48_remove(struct platform_device *pdev) return 0; } -static struct platform_device *ws16c48_device; - -static struct platform_driver ws16c48_driver = { +static struct isa_driver ws16c48_driver = { + .probe = ws16c48_probe, .driver = { .name = "ws16c48" }, .remove = ws16c48_remove }; -static void __exit ws16c48_exit(void) -{ - platform_device_unregister(ws16c48_device); - platform_driver_unregister(&ws16c48_driver); -} - -static int __init ws16c48_init(void) -{ - int err; - - ws16c48_device = platform_device_alloc(ws16c48_driver.driver.name, -1); - if (!ws16c48_device) - return -ENOMEM; - - err = platform_device_add(ws16c48_device); - if (err) - goto err_platform_device; - - err = platform_driver_probe(&ws16c48_driver, ws16c48_probe); - if (err) - goto err_platform_driver; - - return 0; - -err_platform_driver: - platform_device_del(ws16c48_device); -err_platform_device: - platform_device_put(ws16c48_device); - return err; -} - -module_init(ws16c48_init); -module_exit(ws16c48_exit); +module_isa_driver(ws16c48_driver, num_ws16c48); MODULE_AUTHOR("William Breathitt Gray <vilhelm.gray@gmail.com>"); MODULE_DESCRIPTION("WinSystems WS16C48 GPIO driver"); -- 2.7.3 ^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices 2016-04-07 14:47 ` William Breathitt Gray ` (10 preceding siblings ...) (?) @ 2016-04-11 6:59 ` Linus Walleij -1 siblings, 0 replies; 39+ messages in thread From: Linus Walleij @ 2016-04-11 6:59 UTC (permalink / raw) To: William Breathitt Gray Cc: Greg KH, Thomas Gleixner, Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen, Peter Meerwald-Stadler, Wim Van Sebroeck, Guenter Roeck, Alexandre Courbot, linux-kernel, linux-iio, linux-watchdog, linux-gpio 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 ^ permalink raw reply [flat|nested] 39+ messages in thread
[parent not found: <cover.1460040201.git.vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices 2016-04-07 14:47 ` William Breathitt Gray @ 2016-05-01 21:26 ` Greg KH -1 siblings, 0 replies; 39+ messages in thread From: Greg KH @ 2016-05-01 21:26 UTC (permalink / raw) To: William Breathitt Gray Cc: tglx-hfZtesqFncYOwBW4kG4KsQ, jic23-DgEjT+Ai2ygdnm+yROfE0A, knaack.h-Mmb7MZpHnFY, lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg, wim-IQzOog9fTRqzQB+pC5nmwQ, linux-0h96xk9xTtrk1uMJSBkQmQ, linus.walleij-QSEj5FYQhm4dnm+yROfE0A, gnurou-Re5JQEeQqe8AvxtiuMwx3w, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-watchdog-u79uwXL29TY76Z2rM5mHXA, linux-gpio-u79uwXL29TY76Z2rM5mHXA On Thu, Apr 07, 2016 at 10:47:21AM -0400, William Breathitt Gray wrote: > 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. Can you fix up the MAINTAINERS patch, add the ACKs you received, and resend this series so that I can apply it? thanks, greg k-h ^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices @ 2016-05-01 21:26 ` Greg KH 0 siblings, 0 replies; 39+ messages in thread From: Greg KH @ 2016-05-01 21:26 UTC (permalink / raw) To: William Breathitt Gray Cc: tglx, jic23, knaack.h, lars, pmeerw, wim, linux, linus.walleij, gnurou, linux-kernel, linux-iio, linux-watchdog, linux-gpio On Thu, Apr 07, 2016 at 10:47:21AM -0400, William Breathitt Gray wrote: > 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. Can you fix up the MAINTAINERS patch, add the ACKs you received, and resend this series so that I can apply it? thanks, greg k-h ^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2016-05-11 19:34 UTC | newest] Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-07 14:47 [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices William Breathitt Gray 2016-04-07 14:47 ` William Breathitt Gray 2016-04-07 14:47 ` [PATCH 01/10] isa: Implement the module_isa_driver macro William Breathitt Gray 2016-04-07 14:47 ` [PATCH 02/10] isa: Implement the max_num_isa_dev macro William Breathitt Gray 2016-04-07 14:47 ` [PATCH 03/10] Documentation: Add ISA bus driver documentation William Breathitt Gray 2016-05-01 21:26 ` Greg KH 2016-04-07 14:47 ` [PATCH 04/10] iio: stx104: Change STX104 dependency to ISA_BUS William Breathitt Gray [not found] ` <783be62acf68b35f3fe4785a2cedfe017624688b.1460040201.git.vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2016-04-08 0:45 ` Guenter Roeck 2016-04-08 0:45 ` Guenter Roeck [not found] ` <20160408004503.GB10211-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> 2016-04-08 12:31 ` William Breathitt Gray 2016-04-08 12:31 ` William Breathitt Gray 2016-04-08 13:18 ` Guenter Roeck 2016-04-08 13:18 ` Guenter Roeck [not found] ` <5707AF91.5010704-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> 2016-04-08 15:09 ` William Breathitt Gray 2016-04-08 15:09 ` William Breathitt Gray 2016-04-08 18:28 ` Guenter Roeck [not found] ` <20160408182801.GB7083-0h96xk9xTtrk1uMJSBkQmQ@public.gmane.org> 2016-04-08 19:27 ` William Breathitt Gray 2016-04-08 19:27 ` William Breathitt Gray 2016-04-09 12:58 ` One Thousand Gnomes [not found] ` <20160409135814.359e24d6-qBU/x9rampVanCEyBjwyrvXRex20P6io@public.gmane.org> 2016-04-09 13:50 ` William Breathitt Gray 2016-04-09 13:50 ` William Breathitt Gray 2016-04-09 15:51 ` One Thousand Gnomes 2016-04-09 15:51 ` One Thousand Gnomes 2016-04-07 14:47 ` [PATCH 05/10] iio: stx104: Utilize the module_isa_driver and max_num_isa_dev macros William Breathitt Gray 2016-04-07 14:47 ` [PATCH 06/10] watchdog: ebc-c384_wdt: Utilize the ISA bus driver William Breathitt Gray [not found] ` <1f5bf2e21006f0fd4f10ab3948cf69a737c0b039.1460040201.git.vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2016-04-08 0:35 ` Guenter Roeck 2016-04-08 0:35 ` Guenter Roeck 2016-04-08 12:03 ` William Breathitt Gray 2016-05-11 17:04 ` Sasha Levin 2016-05-11 17:04 ` Sasha Levin [not found] ` <57336622.9070508-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> 2016-05-11 19:34 ` William Breathitt Gray 2016-05-11 19:34 ` William Breathitt Gray 2016-04-07 14:47 ` [PATCH 07/10] gpio: 104-dio-48e: " William Breathitt Gray 2016-04-07 14:47 ` [PATCH 08/10] gpio: 104-idi-48: " William Breathitt Gray 2016-04-07 14:47 ` [PATCH 09/10] gpio: 104-idio-16: " William Breathitt Gray 2016-04-07 14:47 ` [PATCH 10/10] gpio: ws16c48: " William Breathitt Gray 2016-04-11 6:59 ` [PATCH 00/10] Use the ISA bus driver for PC/104 and ISA devices Linus Walleij [not found] ` <cover.1460040201.git.vilhelm.gray-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2016-05-01 21:26 ` Greg KH 2016-05-01 21:26 ` Greg KH
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.