On 9/11/24 1:22 AM, Damien Le Moal wrote:
[...]
>>>>>> Replace now gone out of fashion defined(CONFIG_PATA_TOSHIBA[_MODULE])
>>
>> I'll probably rephrase this a bit in v2...
>>
[...]
>>
>>>>>> 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 ?
>>
>> Makes sense, indeed. Do you think this is acceptable to be done in v2 of this
>> patch?
>
> Yep, that is fine and would fit with the config option renaming.
I started respinnig this patch and decided to add the Piccolo comment in
a separate patch, while deferring the Kconfig entry rename until/iff it becomes
truly necessary, as per Niklas' comment).
Unfortunately, I seem to be late for the coming merge window... :-/
MBR, Sergey
