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
--
-=( Ian Abbott <abbotti@xxxxxxxxx> || MEV Ltd. is a company )=-
-=( registered in England & Wales. Regd. number: 02862268. )=-
-=( Regd. addr.: S11 & 12 Building 67, Europa Business Park, )=-
-=( Bird Hall Lane, STOCKPORT, SK3 0XA, UK. || www.mev.co.uk )=-
