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
