Hello! On 5/16/23 1:59 PM, Niklas Schnelle wrote: > In a future patch HAS_IOPORT=n will result in inb()/outb() and friends > not being declared. We thus need to add HAS_IOPORT as dependency for > those drivers using them. > > Co-developed-by: Arnd Bergmann <arnd@xxxxxxxxxx> > Signed-off-by: Arnd Bergmann <arnd@xxxxxxxxxx> > Acked-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx> > Signed-off-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx> > --- > Note: The HAS_IOPORT Kconfig option was added in v6.4-rc1 so > per-subsystem patches may be applied independently > > drivers/ata/Kconfig | 28 ++++++++++++++-------------- > drivers/ata/ata_generic.c | 2 ++ > drivers/ata/libata-sff.c | 2 ++ > include/linux/libata.h | 2 ++ > 4 files changed, 20 insertions(+), 14 deletions(-) > > diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig > index 42b51c9812a0..c521cdc51f8c 100644 > --- a/drivers/ata/Kconfig > +++ b/drivers/ata/Kconfig [...] Shouldn't there be an entry for the ATIIXP driver here? It doesn't call in*/out*() but it does call ata_bmdma_{start|stop}() that call ioread*/iowrite*()... And shouldn't there be an entry for APT867x driver too? It does call ioread*/iowrite*()... [...] Shouldn't there be an entry for the HPT3x3 driver too? It does call ioread*/iowrite*()... and also for the IT821x driver? And the Marvall driver? > @@ -819,7 +819,7 @@ config PATA_MPC52xx > > config PATA_NETCELL > tristate "NETCELL Revolution RAID support" > - depends on PCI > + depends on PCI && HAS_IOPORT Not clear why -- because it calls ata_pci_bmdma_clear_simplex()? [...] Shouldn't there be an entry for the NS87415 driver too? It does call ioread*/iowrite*()... [...] > @@ -919,7 +919,7 @@ config PATA_SCH > > config PATA_SERVERWORKS > tristate "SERVERWORKS OSB4/CSB5/CSB6/HT1000 PATA support" > - depends on PCI > + depends on PCI && HAS_IOPORT Not clear why -- because it calls ata_pci_bmdma_clear_simplex()? [...] Shouldn't there be an entry for the VIA driver too? It does call ioread*/iowrite*()... and SiL680 driver too... and Winbond SL82C105 driver too... and OPTi PIO driver too... and PCMCIA driver too... [...] > @@ -1183,7 +1183,7 @@ config ATA_GENERIC > > config PATA_LEGACY > tristate "Legacy ISA PATA support (Experimental)" > - depends on (ISA || PCI) > + depends on (ISA || PCI) && HAS_IOPORT > select PATA_TIMINGS Hm, won't it override the HAS_IOPORT dependency, if you enable PATA_QDI or PATA_WINBOD_VLB? > help > This option enables support for ISA/VLB/PCI bus legacy PATA > diff --git a/drivers/ata/ata_generic.c b/drivers/ata/ata_generic.c > index 2f57ec00ab82..2d391d117f74 100644 > --- a/drivers/ata/ata_generic.c > +++ b/drivers/ata/ata_generic.c This driver calls ioread8() as well... > @@ -197,8 +197,10 @@ static int ata_generic_init_one(struct pci_dev *dev, const struct pci_device_id > if (!(command & PCI_COMMAND_IO)) > return -ENODEV; > > +#ifdef CONFIG_PATA_ALI This #ifdef doesn't make sense to me -- pata_ali.c will call the below function anyway, no? > if (dev->vendor == PCI_VENDOR_ID_AL) > ata_pci_bmdma_clear_simplex(dev); > +#endif /* CONFIG_PATA_ALI */ > if (dev->vendor == PCI_VENDOR_ID_ATI) { > int rc = pcim_enable_device(dev); > diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c > index 9d28badfe41d..80137edb7ebf 100644 > --- a/drivers/ata/libata-sff.c > +++ b/drivers/ata/libata-sff.c > @@ -3031,6 +3031,7 @@ EXPORT_SYMBOL_GPL(ata_bmdma_port_start32); > > #ifdef CONFIG_PCI > > +#ifdef CONFIG_HAS_IOPORT > /** > * ata_pci_bmdma_clear_simplex - attempt to kick device out of simplex > * @pdev: PCI device > @@ -3056,6 +3057,7 @@ int ata_pci_bmdma_clear_simplex(struct pci_dev *pdev) > return 0; > } > EXPORT_SYMBOL_GPL(ata_pci_bmdma_clear_simplex); > +#endif /* CONFIG_HAS_IOPORT */ > > static void ata_bmdma_nodma(struct ata_host *host, const char *reason) > { > diff --git a/include/linux/libata.h b/include/linux/libata.h > index 311cd93377c7..90002d4a785b 100644 > --- a/include/linux/libata.h > +++ b/include/linux/libata.h > @@ -2012,7 +2012,9 @@ extern int ata_bmdma_port_start(struct ata_port *ap); > extern int ata_bmdma_port_start32(struct ata_port *ap); > > #ifdef CONFIG_PCI > +#ifdef CONFIG_HAS_IOPORT > extern int ata_pci_bmdma_clear_simplex(struct pci_dev *pdev); > +#endif /* CONFIG_HAS_IOPORT */ Hm, wouldn't it be better if you used #else and declare an inline variant of this function simply retirning an error? [...] MBR, Sergey