Re: [PATCH] comedi: Fix driver module dependencies since HAS_IOPORT changes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 )=-




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux