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. 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.
Fixes: b5c75b68b7de ("comedi: add HAS_IOPORT dependencies") Cc: Niklas Schnelle schnelle@linux.ibm.com Cc: Arnd Bergmann arnd@kernel.org Cc: stable@vger.kernel.org # v6.5+ Signed-off-by: Ian Abbott abbotti@mev.co.uk --- drivers/comedi/Kconfig | 119 +++++++++++++++++++++++++---------------- 1 file changed, 72 insertions(+), 47 deletions(-)
diff --git a/drivers/comedi/Kconfig b/drivers/comedi/Kconfig index 7a8d402f05be..094102e5ee1f 100644 --- a/drivers/comedi/Kconfig +++ b/drivers/comedi/Kconfig @@ -103,8 +103,7 @@ if COMEDI_ISA_DRIVERS
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 help Enable support for Advantech PCL-711 and 711b, ADlink ACL-8112
@@ -165,9 +164,8 @@ config COMEDI_PCL730
config COMEDI_PCL812 tristate "Advantech PCL-812/813 and ADlink ACL-8112/8113/8113/8216" - depends on HAS_IOPORT select COMEDI_ISADMA if ISA_DMA_API - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for Advantech PCL-812/PG, PCL-813/B, ADLink ACL-8112DG/HG/PG, ACL-8113, ACL-8216, ICP DAS A-821PGH/PGL/PGL-NDA, @@ -178,9 +176,8 @@ config COMEDI_PCL812
config COMEDI_PCL816 tristate "Advantech PCL-814 and PCL-816 ISA card support" - depends on HAS_IOPORT select COMEDI_ISADMA if ISA_DMA_API - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for Advantech PCL-814 and PCL-816 ISA cards
@@ -189,9 +186,8 @@ config COMEDI_PCL816
config COMEDI_PCL818 tristate "Advantech PCL-718 and PCL-818 ISA card support" - depends on HAS_IOPORT select COMEDI_ISADMA if ISA_DMA_API - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for Advantech PCL-818 ISA cards PCL-818L, PCL-818H, PCL-818HD, PCL-818HG, PCL-818 and PCL-718 @@ -210,7 +206,7 @@ config COMEDI_PCM3724
config COMEDI_AMPLC_DIO200_ISA tristate "Amplicon PC212E/PC214E/PC215E/PC218E/PC272E" - depends on COMEDI_AMPLC_DIO200 + select COMEDI_AMPLC_DIO200 help Enable support for Amplicon PC212E, PC214E, PC215E, PC218E and PC272E ISA DIO boards @@ -262,8 +258,7 @@ config COMEDI_DAC02
config COMEDI_DAS16M1 tristate "MeasurementComputing CIO-DAS16/M1DAS-16 ISA card support" - depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 select COMEDI_8255 help Enable support for Measurement Computing CIO-DAS16/M1 ISA cards. @@ -273,7 +268,7 @@ config COMEDI_DAS16M1
config COMEDI_DAS08_ISA tristate "DAS-08 compatible ISA and PC/104 card support" - depends on COMEDI_DAS08 + select COMEDI_DAS08 help Enable support for Keithley Metrabyte/ComputerBoards DAS08 and compatible ISA and PC/104 cards: @@ -286,9 +281,8 @@ config COMEDI_DAS08_ISA
config COMEDI_DAS16 tristate "DAS-16 compatible ISA and PC/104 card support" - depends on HAS_IOPORT select COMEDI_ISADMA if ISA_DMA_API - depends on COMEDI_8254 + select COMEDI_8254 select COMEDI_8255 help Enable support for Keithley Metrabyte/ComputerBoards DAS16 @@ -305,8 +299,7 @@ config COMEDI_DAS16
config COMEDI_DAS800 tristate "DAS800 and compatible ISA card support" - depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for Keithley Metrabyte DAS800 and compatible ISA cards Keithley Metrabyte DAS-800, DAS-801, DAS-802 @@ -318,9 +311,8 @@ config COMEDI_DAS800
config COMEDI_DAS1800 tristate "DAS1800 and compatible ISA card support" - depends on HAS_IOPORT select COMEDI_ISADMA if ISA_DMA_API - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for DAS1800 and compatible ISA cards Keithley Metrabyte DAS-1701ST, DAS-1701ST-DA, DAS-1701/AO, @@ -334,8 +326,7 @@ config COMEDI_DAS1800
config COMEDI_DAS6402 tristate "DAS6402 and compatible ISA card support" - depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for DAS6402 and compatible ISA cards Computerboards, Keithley Metrabyte DAS6402 and compatibles @@ -414,8 +405,7 @@ config COMEDI_FL512
config COMEDI_AIO_AIO12_8 tristate "I/O Products PC/104 AIO12-8 Analog I/O Board support" - depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 select COMEDI_8255 help Enable support for I/O Products PC/104 AIO12-8 Analog I/O Board @@ -469,9 +459,8 @@ config COMEDI_ADQ12B
config COMEDI_NI_AT_A2150 tristate "NI AT-A2150 ISA card support" - depends on HAS_IOPORT select COMEDI_ISADMA if ISA_DMA_API - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for National Instruments AT-A2150 cards
@@ -480,8 +469,7 @@ config COMEDI_NI_AT_A2150
config COMEDI_NI_AT_AO tristate "NI AT-AO-6/10 EISA card support" - depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for National Instruments AT-AO-6/10 cards
@@ -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+. @@ -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 @@ -604,6 +593,7 @@ config COMEDI_8255_PCI
config COMEDI_ADDI_WATCHDOG tristate + depends on HAS_IOPORT help Provides support for the watchdog subdevice found on many ADDI-DATA boards. This module will be automatically selected when needed. The @@ -611,6 +601,7 @@ config COMEDI_ADDI_WATCHDOG
config COMEDI_ADDI_APCI_1032 tristate "ADDI-DATA APCI_1032 support" + depends on HAS_IOPORT help Enable support for ADDI-DATA APCI_1032 cards
@@ -619,6 +610,7 @@ config COMEDI_ADDI_APCI_1032
config COMEDI_ADDI_APCI_1500 tristate "ADDI-DATA APCI_1500 support" + depends on HAS_IOPORT help Enable support for ADDI-DATA APCI_1500 cards
@@ -627,6 +619,7 @@ config COMEDI_ADDI_APCI_1500
config COMEDI_ADDI_APCI_1516 tristate "ADDI-DATA APCI-1016/1516/2016 support" + depends on HAS_IOPORT select COMEDI_ADDI_WATCHDOG help Enable support for ADDI-DATA APCI-1016, APCI-1516 and APCI-2016 boards. @@ -638,6 +631,7 @@ config COMEDI_ADDI_APCI_1516
config COMEDI_ADDI_APCI_1564 tristate "ADDI-DATA APCI_1564 support" + depends on HAS_IOPORT select COMEDI_ADDI_WATCHDOG help Enable support for ADDI-DATA APCI_1564 cards @@ -647,6 +641,7 @@ config COMEDI_ADDI_APCI_1564
config COMEDI_ADDI_APCI_16XX tristate "ADDI-DATA APCI_16xx support" + depends on HAS_IOPORT help Enable support for ADDI-DATA APCI_16xx cards
@@ -655,6 +650,7 @@ config COMEDI_ADDI_APCI_16XX
config COMEDI_ADDI_APCI_2032 tristate "ADDI-DATA APCI_2032 support" + depends on HAS_IOPORT select COMEDI_ADDI_WATCHDOG help Enable support for ADDI-DATA APCI_2032 cards @@ -664,6 +660,7 @@ config COMEDI_ADDI_APCI_2032
config COMEDI_ADDI_APCI_2200 tristate "ADDI-DATA APCI_2200 support" + depends on HAS_IOPORT select COMEDI_ADDI_WATCHDOG help Enable support for ADDI-DATA APCI_2200 cards @@ -673,6 +670,7 @@ config COMEDI_ADDI_APCI_2200
config COMEDI_ADDI_APCI_3120 tristate "ADDI-DATA APCI_3120/3001 support" + depends on HAS_IOPORT depends on HAS_DMA help Enable support for ADDI-DATA APCI_3120/3001 cards @@ -682,6 +680,7 @@ config COMEDI_ADDI_APCI_3120
config COMEDI_ADDI_APCI_3501 tristate "ADDI-DATA APCI_3501 support" + depends on HAS_IOPORT help Enable support for ADDI-DATA APCI_3501 cards
@@ -690,6 +689,7 @@ config COMEDI_ADDI_APCI_3501
config COMEDI_ADDI_APCI_3XXX tristate "ADDI-DATA APCI_3xxx support" + depends on HAS_IOPORT help Enable support for ADDI-DATA APCI_3xxx cards
@@ -698,6 +698,7 @@ config COMEDI_ADDI_APCI_3XXX
config COMEDI_ADL_PCI6208 tristate "ADLink PCI-6208A support" + depends on HAS_IOPORT help Enable support for ADLink PCI-6208A cards
@@ -706,6 +707,7 @@ config COMEDI_ADL_PCI6208
config COMEDI_ADL_PCI7X3X tristate "ADLink PCI-723X/743X isolated digital i/o board support" + depends on HAS_IOPORT help Enable support for ADlink PCI-723X/743X isolated digital i/o boards. Supported boards include the 32-channel PCI-7230 (16 in/16 out), @@ -717,6 +719,7 @@ config COMEDI_ADL_PCI7X3X
config COMEDI_ADL_PCI8164 tristate "ADLink PCI-8164 4 Axes Motion Control board support" + depends on HAS_IOPORT help Enable support for ADlink PCI-8164 4 Axes Motion Control board
@@ -726,7 +729,7 @@ config COMEDI_ADL_PCI8164 config COMEDI_ADL_PCI9111 tristate "ADLink PCI-9111HR support" depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for ADlink PCI9111 cards
@@ -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
@@ -746,7 +749,7 @@ config COMEDI_ADL_PCI9118 config COMEDI_ADV_PCI1710 tristate "Advantech PCI-171x and PCI-1731 support" depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for Advantech PCI-1710, PCI-1710HG, PCI-1711, PCI-1713 and PCI-1731 @@ -756,6 +759,7 @@ config COMEDI_ADV_PCI1710
config COMEDI_ADV_PCI1720 tristate "Advantech PCI-1720 support" + depends on HAS_IOPORT help Enable support for Advantech PCI-1720 Analog Output board.
@@ -764,6 +768,7 @@ config COMEDI_ADV_PCI1720
config COMEDI_ADV_PCI1723 tristate "Advantech PCI-1723 support" + depends on HAS_IOPORT help Enable support for Advantech PCI-1723 cards
@@ -772,6 +777,7 @@ config COMEDI_ADV_PCI1723
config COMEDI_ADV_PCI1724 tristate "Advantech PCI-1724U support" + depends on HAS_IOPORT help Enable support for Advantech PCI-1724U cards. These are 32-channel analog output cards with voltage and current loop output ranges and @@ -782,6 +788,7 @@ config COMEDI_ADV_PCI1724
config COMEDI_ADV_PCI1760 tristate "Advantech PCI-1760 support" + depends on HAS_IOPORT help Enable support for Advantech PCI-1760 board.
@@ -791,7 +798,7 @@ config COMEDI_ADV_PCI1760 config COMEDI_ADV_PCI_DIO tristate "Advantech PCI DIO card support" depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 select COMEDI_8255 help Enable support for Advantech PCI DIO cards @@ -804,7 +811,8 @@ config COMEDI_ADV_PCI_DIO
config COMEDI_AMPLC_DIO200_PCI tristate "Amplicon PCI215/PCI272/PCIe215/PCIe236/PCIe296 DIO support" - depends on COMEDI_AMPLC_DIO200 + depends on HAS_IOPORT + select COMEDI_AMPLC_DIO200 help Enable support for Amplicon PCI215, PCI272, PCIe215, PCIe236 and PCIe296 DIO boards. @@ -814,6 +822,7 @@ config COMEDI_AMPLC_DIO200_PCI
config COMEDI_AMPLC_PC236_PCI tristate "Amplicon PCI236 DIO board support" + depends on HAS_IOPORT select COMEDI_AMPLC_PC236 help Enable support for Amplicon PCI236 DIO board. @@ -823,6 +832,7 @@ config COMEDI_AMPLC_PC236_PCI
config COMEDI_AMPLC_PC263_PCI tristate "Amplicon PCI263 relay board support" + depends on HAS_IOPORT help Enable support for Amplicon PCI263 relay board. This is a PCI board with 16 reed relay output channels. @@ -833,7 +843,7 @@ config COMEDI_AMPLC_PC263_PCI config COMEDI_AMPLC_PCI224 tristate "Amplicon PCI224 and PCI234 support" depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for Amplicon PCI224 and PCI234 AO boards
@@ -843,7 +853,7 @@ config COMEDI_AMPLC_PCI224 config COMEDI_AMPLC_PCI230 tristate "Amplicon PCI230 and PCI260 support" depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 select COMEDI_8255 help Enable support for Amplicon PCI230 and PCI260 Multifunction I/O @@ -854,6 +864,7 @@ config COMEDI_AMPLC_PCI230
config COMEDI_CONTEC_PCI_DIO tristate "Contec PIO1616L digital I/O board support" + depends on HAS_IOPORT help Enable support for the Contec PIO1616L digital I/O board
@@ -862,7 +873,8 @@ config COMEDI_CONTEC_PCI_DIO
config COMEDI_DAS08_PCI tristate "DAS-08 PCI support" - depends on COMEDI_DAS08 + depends on HAS_IOPORT + select COMEDI_DAS08 help Enable support for PCI DAS-08 cards.
@@ -881,6 +893,7 @@ config COMEDI_DT3000
config COMEDI_DYNA_PCI10XX tristate "Dynalog PCI DAQ series support" + depends on HAS_IOPORT help Enable support for Dynalog PCI DAQ series PCI-1050 @@ -914,6 +927,7 @@ config COMEDI_ICP_MULTI
config COMEDI_DAQBOARD2000 tristate "IOtech DAQboard/2000 support" + depends on HAS_IOPORT select COMEDI_8255 help Enable support for the IOtech DAQboard/2000 @@ -939,6 +953,7 @@ config COMEDI_KE_COUNTER
config COMEDI_CB_PCIDAS64 tristate "MeasurementComputing PCI-DAS 64xx, 60xx, and 4020 support" + depends on HAS_IOPORT select COMEDI_8255 help Enable support for ComputerBoards/MeasurementComputing PCI-DAS 64xx, @@ -950,7 +965,7 @@ config COMEDI_CB_PCIDAS64 config COMEDI_CB_PCIDAS tristate "MeasurementComputing PCI-DAS support" depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 select COMEDI_8255 help Enable support for ComputerBoards/MeasurementComputing PCI-DAS with @@ -963,6 +978,7 @@ config COMEDI_CB_PCIDAS
config COMEDI_CB_PCIDDA tristate "MeasurementComputing PCI-DDA series support" + depends on HAS_IOPORT select COMEDI_8255 help Enable support for ComputerBoards/MeasurementComputing PCI-DDA @@ -975,7 +991,7 @@ config COMEDI_CB_PCIDDA config COMEDI_CB_PCIMDAS tristate "MeasurementComputing PCIM-DAS1602/16, PCIe-DAS1602/16 support" depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 select COMEDI_8255 help Enable support for ComputerBoards/MeasurementComputing PCI Migration @@ -986,6 +1002,7 @@ config COMEDI_CB_PCIMDAS
config COMEDI_CB_PCIMDDA tristate "MeasurementComputing PCIM-DDA06-16 support" + depends on HAS_IOPORT select COMEDI_8255 help Enable support for ComputerBoards/MeasurementComputing PCIM-DDA06-16 @@ -996,7 +1013,7 @@ config COMEDI_CB_PCIMDDA config COMEDI_ME4000 tristate "Meilhaus ME-4000 support" depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for Meilhaus PCI data acquisition cards ME-4650, ME-4670i, ME-4680, ME-4680i and ME-4680is @@ -1054,7 +1071,8 @@ config COMEDI_NI_670X
config COMEDI_NI_LABPC_PCI tristate "NI Lab-PC PCI-1200 support" - depends on COMEDI_NI_LABPC + depends on HAS_IOPORT + select COMEDI_NI_LABPC help Enable support for National Instruments Lab-PC PCI-1200.
@@ -1064,6 +1082,7 @@ config COMEDI_NI_LABPC_PCI config COMEDI_NI_PCIDIO tristate "NI PCI-DIO32HS, PCI-6533, PCI-6534 support" depends on HAS_DMA + depends on HAS_IOPORT select COMEDI_MITE select COMEDI_8255 help @@ -1099,7 +1118,7 @@ config COMEDI_NI_PCIMIO config COMEDI_RTD520 tristate "Real Time Devices PCI4520/DM7520 support" depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for Real Time Devices PCI4520/DM7520
@@ -1140,7 +1159,7 @@ if COMEDI_PCMCIA_DRIVERS config COMEDI_CB_DAS16_CS tristate "CB DAS16 series PCMCIA support" depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 help Enable support for the ComputerBoards/MeasurementComputing PCMCIA cards DAS16/16, PCM-DAS16D/12 and PCM-DAS16s/16 @@ -1150,7 +1169,8 @@ config COMEDI_CB_DAS16_CS
config COMEDI_DAS08_CS tristate "CB DAS08 PCMCIA support" - depends on COMEDI_DAS08 + depends on HAS_IOPORT + select COMEDI_DAS08 help Enable support for the ComputerBoards/MeasurementComputing DAS-08 PCMCIA card @@ -1179,7 +1199,8 @@ config COMEDI_NI_DAQ_DIO24_CS
config COMEDI_NI_LABPC_CS tristate "NI DAQCard-1200 PCMCIA support" - depends on COMEDI_NI_LABPC + depends on HAS_IOPORT + select COMEDI_NI_LABPC help Enable support for the National Instruments PCMCIA DAQCard-1200
@@ -1282,6 +1303,7 @@ config COMEDI_8254
config COMEDI_8255 tristate + depends on HAS_IOPORT
config COMEDI_8255_SA tristate "Standalone 8255 support" @@ -1317,16 +1339,19 @@ config COMEDI_KCOMEDILIB called kcomedilib.
config COMEDI_AMPLC_DIO200 - depends on COMEDI_8254 tristate + depends on HAS_IOPORT + select COMEDI_8254
config COMEDI_AMPLC_PC236 tristate + depends on HAS_IOPORT select COMEDI_8255
config COMEDI_DAS08 tristate - depends on COMEDI_8254 + depends on HAS_IOPORT + select COMEDI_8254 select COMEDI_8255
config COMEDI_ISADMA @@ -1335,7 +1360,7 @@ config COMEDI_ISADMA config COMEDI_NI_LABPC tristate depends on HAS_IOPORT - depends on COMEDI_8254 + select COMEDI_8254 select COMEDI_8255
config COMEDI_NI_LABPC_ISADMA
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.
@@ -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.
If this addresses another problem for you, maybe split it out into a separate patch and describe why you move the dependencies.
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?
@@ -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.
It might be easier to revert the original patch, and then follow up with a fixed version.
Arnd
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
On Mon, 2023-09-04 at 11:10 +0100, Ian Abbott wrote:
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.
---8<---
@@ -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?
The patch that finally compile-time disables I/O port accesses as well as a few others are still not merged. I was away for a few weeks and also still have a few todos. I also and found a few things needed for new changes. So no a revert will not break compiles or anything like that.
Thanks, Niklas
On 04/09/2023 12:23, Niklas Schnelle wrote:
On Mon, 2023-09-04 at 11:10 +0100, Ian Abbott wrote:
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.
---8<---
@@ -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?
The patch that finally compile-time disables I/O port accesses as well as a few others are still not merged. I was away for a few weeks and also still have a few todos. I also and found a few things needed for new changes. So no a revert will not break compiles or anything like that.
Thanks for the confirmation. Will it be safe to assume that anything that selects ISA will also select HAS_IOPORT? That is trivially the case for arch/{alpha,arm,x86}; arch/mips explicitly selects HAS_IOPORT if ISA is selected; arch/powerpc explicitly selects HAS_IOPORT if PCI is selected and it is only possible to configure ISA if PPC_CHRP is configured which selects FORCE_PCI and therefore selects PCI and therefore selects HAS_IOPORT; arch/um does not select HAS_IOPORT and although it has a 'config ISA', nothing appears to select it. None of the remaining arch/* have 'select ISA'.
On 04/09/2023 13:01, Ian Abbott wrote:
On 04/09/2023 12:23, Niklas Schnelle wrote:
On Mon, 2023-09-04 at 11:10 +0100, Ian Abbott wrote:
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.
---8<---
@@ -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?
The patch that finally compile-time disables I/O port accesses as well as a few others are still not merged. I was away for a few weeks and also still have a few todos. I also and found a few things needed for new changes. So no a revert will not break compiles or anything like that.
Thanks for the confirmation. Will it be safe to assume that anything that selects ISA will also select HAS_IOPORT? That is trivially the case for arch/{alpha,arm,x86}; arch/mips explicitly selects HAS_IOPORT if ISA is selected; arch/powerpc explicitly selects HAS_IOPORT if PCI is selected and it is only possible to configure ISA if PPC_CHRP is configured which selects FORCE_PCI and therefore selects PCI and therefore selects HAS_IOPORT; arch/um does not select HAS_IOPORT and although it has a 'config ISA', nothing appears to select it. None of the remaining arch/* have 'select ISA'.
Another quick question: Will dummy versions of request_region(), request_muxed_region() and release_region() (and the devm variants) exist in the future even if HAS_IOPORT is not selected? If so, I guess any drivers that only use inb()/outb() and friends on successfully requested regions can safely replace inb()/outb() and friends with conditionally compiled wrappers.
On Mon, Sep 4, 2023, at 08:01, Ian Abbott wrote:
On 04/09/2023 12:23, Niklas Schnelle wrote:
On Mon, 2023-09-04 at 11:10 +0100, Ian Abbott wrote:
Thanks for the confirmation. Will it be safe to assume that anything that selects ISA will also select HAS_IOPORT? That is trivially the case for arch/{alpha,arm,x86}; arch/mips explicitly selects HAS_IOPORT if ISA is selected; arch/powerpc explicitly selects HAS_IOPORT if PCI is selected and it is only possible to configure ISA if PPC_CHRP is configured which selects FORCE_PCI and therefore selects PCI and therefore selects HAS_IOPORT; arch/um does not select HAS_IOPORT and although it has a 'config ISA', nothing appears to select it. None of the remaining arch/* have 'select ISA'.
Yes, I think that will always be a safe assumption, ISA without port I/O is just not a sensible configuration. A few of the later ISA devices use PCI style memory mapped I/O, but I can't think of any driver that doesn't also require port I/O, and you wouldn't find ISA slots in a system that lacks support for port I/O.
Arnd
On Mon, 4 Sep 2023, Arnd Bergmann wrote:
Yes, I think that will always be a safe assumption, ISA without port I/O is just not a sensible configuration. A few of the later ISA devices use PCI style memory mapped I/O, but I can't think of any driver that doesn't also require port I/O, and you wouldn't find ISA slots in a system that lacks support for port I/O.
FWIW for the 8086 CPU as it was designed back in 1970s (and borrowing from the 8080/8085) and consequently IBM PC systems of 1980s memory and I/O bus cycles were meant for resource accesses as the respective names implied, there was no concept of MMIO for those systems back then, which came later from CPU architectures that only have a single address space.
With those ISA option cards memory bus cycles were typically decoded by an option ROM where implemented (for system BIOS expansion), sometimes socketed (e.g. for a network card's optional boot ROM), and video adapters (graphics or text-only such as the MDA) also decoded memory bus cycles to video RAM. There were also ISA memory expansion cards, exceedingly rare, which decoded memory bus cycles to onboard RAM.
For anything else I/O bus cycles were used, so for the purpose of our consideration ISA pretty much equals I/O and the presence of ISA implies a way to generate 8086 bus I/O cycles in a system, regardless of what native bus protocols the host CPU might implement.
Maciej
On Mon, Sep 4, 2023, at 06:10, Ian Abbott wrote:
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.
Ok, that is probably true if there s an implied 'depends on ISA', but I wouldn't change the logic as part of a bug fix, since the 'depends on HAS_IOPORT' is not wrong.
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.
No, you should always split bugfixes from cleanups, otherwise it's impossible to review the patch. Please either revert the patch and do it correctly the way you want, or send one bugfix patch and a cleanup on top.
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).
sorry, my mistake.
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?
No, the final patches to remove ioport stubs for !HAS_IOPORT configs got delayed for other reasons and isn't part of 6.6-rc1 either, so if you do a revert followed by annother patch in 6.6, backporting just the revert should be fine.
Arnd
On 01/09/2023 20: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. 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.
Fixes: b5c75b68b7de ("comedi: add HAS_IOPORT dependencies") Cc: Niklas Schnelle schnelle@linux.ibm.com Cc: Arnd Bergmann arnd@kernel.org Cc: stable@vger.kernel.org # v6.5+ Signed-off-by: Ian Abbott abbotti@mev.co.uk
drivers/comedi/Kconfig | 119 +++++++++++++++++++++++++---------------- 1 file changed, 72 insertions(+), 47 deletions(-)
Please ignore this patch.
I'll send another patch to revert b5c75b68b7de as recommended in the other replies on this thread by Arnd and Niklas.
linux-stable-mirror@lists.linaro.org
-
Arnd Bergmann -
Ian Abbott -
Maciej W. Rozycki -
Niklas Schnelle