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

[Damien Le Moal <damien.lemoal@opensource.wdc.com>]

On 6/5/22 05:53, Sergey Shtylyov 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:

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
pio modes) but the pio mode that will be used for that device, we should
reword, no ?

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.


> 
> Fixes: d9027470b886 ("[libata] Add ATA transport class")
> Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru>
> Cc: stable@vger.kernel.org
> 
> ---
> This patch is against the 'master' branch of Damien's 'libata.git' repo.
> 
>  drivers/ata/libata-transport.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> Index: libata/drivers/ata/libata-transport.c
> ===================================================================
> --- libata.orig/drivers/ata/libata-transport.c
> +++ libata/drivers/ata/libata-transport.c
> @@ -196,7 +196,7 @@ static struct {
>  	{ XFER_PIO_0,			"XFER_PIO_0" },
>  	{ XFER_PIO_SLOW,		"XFER_PIO_SLOW" }
>  };
> -ata_bitfield_name_match(xfer,ata_xfer_names)
> +ata_bitfield_name_search(xfer, ata_xfer_names)
>  
>  /*
>   * ATA Port attributes


-- 
Damien Le Moal
Western Digital Research