| Message ID | 783be62acf68b35f3fe4785a2cedfe017624688b.1460040201.git.vilhelm.gray@gmail.com |
|---|---|
| State | New |
| Headers | show |
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --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
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(-)