Re: [PATCH] ata: ata_generic: use IS_ENABLED() macro

[Sergey Shtylyov <s.shtylyov@omp.ru>]

On 9/11/24 1:22 AM, Damien Le Moal wrote:
[...]
>> [Resending after adding the missed test, please ignore the previus reply.)

   Oops, some more typos! :-)

[...]
>>>>>> 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 :)
>>
>>    There's something to be confused about this driver vs its Kconfig option
>> naming: the driver is called pata_piccolo.c and its option CONFIG_PATA_TOSHIBA.
>> However, Toshiba seemingly has more than one family of the PATA controllers:
>> there's also TC86C001 PCI multi-function chip (dubbed GOKU-S by Toshiba) which
>> supports up to UDMA66 and doesn't seem compatible with Piccolo, judging by the
>> driver code and Toshiba GOKU-S datasheet I have: the timing regs are mapped @
>> AR5 and not in the PCI config space, like with the Piccolo chips.

   I'm sure I typed BAR5 but apparently B went somewhere with further editing... :-)

>>    If somebody like me (it was me who submitted the reworked Toshiba's TC86C001
>> driver for drivers/ide/ back in 2007) added TC86C001 libata driver, the confusion
>> would probably worsen... :-/ Luckily, the chip is a bit tricky (I had to somewhat

   If you want to see the original patch:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=33dced2ea5ed03dda10e7f9f41f0910f32e02eaa

   Some fragments of the patch lived thru the drivers/ide/ removal, see e.g.:

https://elixir.bootlin.com/linux/v6.11-rc1/source/drivers/pci/quirks.c#L2319

>> abuse drivers/ide/ to work around some "limitations", as Toshiba calls their errata)
>> and I don't have access to the chip to properly test the driver anymore.  Obviously, there should be a little interest now in adding the "new" PATA drivers... :-)

   The interesting fact is that the TC86C001 (GOUKU-S) USB device controller (PCI function #2) is still supported by its own driver (drivers/usb/gadget/udc/goku_udc.c), mereg back in 2004... :-)

>>    Any thoughts on the naming confusion?
> 
> Maybe rename the option to CONFIG_PATA_TOSHIBA_PICCCOLO ?

   Nah, that doesn't make much sense to me; if we rename it, we should match the driver's name, i.e. make it CONFIG_PATA_PICCOLO.  I'm mainly concerned about the
Linux distros which would have to handle such rename somehow, IIUC...

[...[

MBR, Sergey