On 2024/09/10 17:52, Sergey Shtylyov wrote: > On 9/10/24 7:50 AM, Damien Le Moal wrote: > [...] > >>> Replace now gone out of fashion defined(CONFIG_PATA_TOSHIBA[_MODULE]) >>> with the new-fangled IS_ENABLED() macro in the ata_generic[] definition. >> >> Please mention that CONFIG_PATA_TOSHIBA_MODULE actually does not exist at all >> and so can be removed. > > Huh? =) > CONFIG_PATA_TOSHIBA is a tristate option, so CONFIG_PATA_TOSHIBA_MODULE > does exist; else there would be no point in using IS_ENABLED() at all... Oops... Indeed. Got confused with something else :) >>> Signed-off-by: Sergey Shtylyov <s.shtylyov@xxxxxx> > > [...[ > >>> Index: linux/drivers/ata/ata_generic.c >>> =================================================================== >>> --- linux.orig/drivers/ata/ata_generic.c >>> +++ linux/drivers/ata/ata_generic.c >>> @@ -220,7 +220,7 @@ static struct pci_device_id ata_generic[ >>> { PCI_DEVICE(PCI_VENDOR_ID_OPTI, PCI_DEVICE_ID_OPTI_82C558), }, >>> { PCI_DEVICE(PCI_VENDOR_ID_CENATEK,PCI_DEVICE_ID_CENATEK_IDE), >>> .driver_data = ATA_GEN_FORCE_DMA }, >>> -#if !defined(CONFIG_PATA_TOSHIBA) && !defined(CONFIG_PATA_TOSHIBA_MODULE) >>> +#if !IS_ENABLED(CONFIG_PATA_TOSHIBA) >> >> I do not understand the negation here... It seems very wrong. If the driver is >> indeed enabled, we need to add its PCI ID, no ? and the reverse when not defined... > > The separate driver was added by Alan Cox in 2009, before that > Toshiba Piccolo controllers were handled by this generic driver... OK, makes sense now. Maybe we should add a comment above that IS_ENABLED() to say so ? > > [...] > > MBR, Sergey > -- Damien Le Moal Western Digital Research