Re: [PATCH] ata: libata-transport: fix {dma|pio|xfer}_mode sysfs files

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

Hello!

On 6/8/22 6:14 AM, Damien Le Moal wrote:
[...]
>>>>>> The {dma|pio|xfer}_mode sysfs files are incorrectly handled by the
>>>>>> ata_bitfield_name_match() macro which leads to reading such kind of
>>>>>> nonsense from them:
>>>>>>
>>>>>> $ cat /sys/class/ata_device/dev3.0/pio_mode
>>>>>> XFER_UDMA_7, XFER_UDMA_6, XFER_UDMA_5, XFER_UDMA_4, XFER_MW_DMA_4,
>>>>>> XFER_PIO_6, XFER_PIO_5, XFER_PIO_4, XFER_PIO_3, XFER_PIO_2, XFER_PIO_1,
>>>>>> XFER_PIO_0
>>>>>>
>>>>>> Using the correct ata_bitfield_name_search() macro fixes that:
>>>>>>
>>>>>> $ cat /sys/class/ata_device/dev3.0/pio_mode
>>>>>> XFER_PIO_4
>>>>>
>>>>> Looks good, but Documentation/ABI/testing/sysfs-ata says:
>>>>
>>>>    Completely forgot that the sysfs files are documented as ABIs... :-(
>>>>    Hm, shouldn't that file be added to the libata's entry in MAINTAINERS?
>>
>>    So what's your opinion on that idea?

   ???

>>>>> pio_mode:       (RO) Transfer modes supported by the device when
>>>>>                 in PIO mode. Mostly used by PATA device.
>>>>>
>>>>> xfer_mode:      (RO) Current transfer mode
>>>>>
>>>>> dma_mode:       (RO) Transfer modes supported by the device when
>>>>>                 in DMA mode. Mostly used by PATA device.
>>>>>
>>>>> which seems incorrect/badly worded for pio_mode and dma_mode. Since these
>>>>> 2 sysfs attributes do not actually device the pio mask (list of supported
>>>>
>>>>    Device?
>>>
>>> advertise :)
>>
>>    Makes sense now. :-)
>>
>>>>> pio modes) but the pio mode that will be used for that device, we should
>>>>> reword, no ?
>>>>
>>>>    Yes, of course. :-)
>>>>
>>>>> What about:
>>>>>
>>>>> pio_mode:       (RO) Transfer mode used by the device when
>>>>>                 in PIO mode. Mostly used by PATA device.
>>>>>
>>>>> xfer_mode:      (RO) Current transfer mode
>>>>>
>>>>> dma_mode:       (RO) Transfer mode used by the device when
>>>>>                 in DMA mode. Mostly used by PATA device.
>>>>
>>>>    Sounds quite tautological... :-)
>>>>    What about:
>>>>
>>>> {dma|pio}_mode: (RO) {DMA|PIO} transfer mode used by the device.
>>>>                 Mostly used by PATA devices.
>>>>
>>>>    I think this should be done in the same patch. Or would you prefer 2 patches?
>>>
>>> Let's do 2 patches. Not sure if you can find a fixes tag for the doc update
>>
>>    It'll be the same tag.
> 
> OK. Then let's do code and doc fixes in one patch, not 2.

   Doh! Just when I did 2 patches... :-/

>>> though. But we should not aggregate the 2 attributes as you did. These doc files
>>> have a defined format and may not be happy with that merged syntax.
>>
>>    Sorry about that -- I did that just for the mail... :-)

MBR, Sergey