On 03/09/2023 16:49, Arnd Bergmann wrote:
On Fri, Sep 1, 2023, at 15:26, Ian Abbott wrote:
Commit b5c75b68b7de ("comedi: add HAS_IOPORT dependencies") changed the "select" directives to "depend on" directives for several config stanzas, but the options they depended on could not be selected, breaking previously selected options.
Right, I think that correctly describes the regression, sorry I didn't catch that during the submission.
Change them back to "select" directives and add "depends on HAS_IOPORT" to config entries for modules that either use inb()/outb() and friends directly, or (recursively) depend on modules that do so.
This also describes a correct solution to the problem, but from looking at your patch, I think it's not exactly what you do.
config COMEDI_PCL711 tristate "Advantech PCL-711/711b and ADlink ACL-8112 ISA card support"
- depends on HAS_IOPORT
- depends on COMEDI_8254
- select COMEDI_8254
If COMEDI_8254 depends on HAS_IOPORT, you must not drop the 'depends on' here, otherwise you get build failures from missing dependencies.
Same thing for a lot of the ones below. You should only change the select, but not remove the 'depends on HAS_IOPORT' in any of these, unless the entire Kconfig file already has this.
I assumed it was OK because it is only selectable if 'ISA' is selected and all the other ISA card drivers that use inb()/outb() and friends do not depend on 'HAS_IOPORT' either.
@@ -512,7 +500,7 @@ config COMEDI_NI_ATMIO16D
config COMEDI_NI_LABPC_ISA tristate "NI Lab-PC and compatibles ISA support"
- depends on COMEDI_NI_LABPC
- select COMEDI_NI_LABPC help Enable support for National Instruments Lab-PC and compatibles Lab-PC-1200, Lab-PC-1200AI, Lab-PC+.
I was confused a bit by this, as the changelog doesn't mention COMEDI_NI_LABPC, but I saw that this needs the same change recursively, same as COMEDI_DAS08.
@@ -576,7 +564,7 @@ endif # COMEDI_ISA_DRIVERS
menuconfig COMEDI_PCI_DRIVERS tristate "Comedi PCI drivers"
- depends on PCI && HAS_IOPORT
- depends on PCI help Enable support for comedi PCI drivers.
@@ -587,6 +575,7 @@ if COMEDI_PCI_DRIVERS
config COMEDI_8255_PCI tristate "Generic PCI based 8255 digital i/o board support"
- depends on HAS_IOPORT select COMEDI_8255 help Enable support for PCI based 8255 digital i/o boards. This driver
This change looks unrelated to both your description and the bug, as you are just moving around the dependencies, though I might be missing something.
I'm just moving the 'HAS_IOPORT' dependency down from 'COMEDI_PCI_DRIVERS' to its dependents. Not all comedi PCI drivers use I/O ports, although some of the drivers that do not use I/O ports do depend on 'COMEDI_8254' and 'COMEDI_8255' which do depend on 'HAS_IOPORT'.
If this addresses another problem for you, maybe split it out into a separate patch and describe why you move the dependencies.
I'm just correcting one patch with one patch, so don't really want to split it. I could improve the patch description though.
Are you trying to make sure that it's possible to build PCI IIO drivers that don't depend on HAS_IOPORT on targets that don't provide it?
Yes (well, PCI comedi drivers rather than IIO drivers). I still need to do something about the PCI drivers that depend on COMEDI_8254 or COMEDI_8255 but don't actually use the I/O port functionality of the comedi_8254 and comedi_8255 modules, but that can be done in other patch that modifies the drivers.
@@ -735,8 +738,8 @@ config COMEDI_ADL_PCI9111
config COMEDI_ADL_PCI9118 tristate "ADLink PCI-9118DG, PCI-9118HG, PCI-9118HR support"
- depends on HAS_IOPORT depends on HAS_DMA
- depends on COMEDI_8254 help Enable support for ADlink PCI-9118DG, PCI-9118HG, PCI-9118HR cards
I don't see why you'd remove the 'depends on COMEDI_8254' here rather than turning it back into 'select' as it was originally.
Oops! That's an error on my part. Thanks for catching it!
It might be easier to revert the original patch, and then follow up with a fixed version.
Will any random config builds break in 6.5 stable if the original patch is reverted, or is the 'HAS_IOPORT' stuff still in preparation for future use?
Arnd
Cheers, Ian