On 6/7/22 18:49, Sergei Shtylyov wrote:
> On 6/7/22 3:37 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.
>
>> 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
--
Damien Le Moal
Western Digital Research
