Re: [PATCH 3/3] ata: libata-scsi: Use ata_ncq_supported in ata_scsi_dev_config()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/5/23 11:37, Damien Le Moal wrote:
On 6/5/23 17:04, Hannes Reinecke wrote:
On 6/5/23 03:32, Damien Le Moal wrote:
In ata_scsi_dev_config(), instead of hardconing the test to check if
an ATA device supports NCQ by looking at the ATA_DFLAG_NCQ flag, use
ata_ncq_supported().

Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
---
   drivers/ata/libata-scsi.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 8ce90284eb34..22e2e9ab6b60 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -1122,7 +1122,7 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct ata_device *dev)
   	if (dev->flags & ATA_DFLAG_AN)
   		set_bit(SDEV_EVT_MEDIA_CHANGE, sdev->supported_events);
- if (dev->flags & ATA_DFLAG_NCQ)
+	if (ata_ncq_supported(dev))
   		depth = min(sdev->host->can_queue, ata_id_queue_depth(dev->id));
   	depth = min(ATA_MAX_QUEUE, depth);
   	scsi_change_queue_depth(sdev, depth);

Argh. ATA NCQ flags. We have ATA_DFLAG_NCQ, ATA_DFLAG_PIO,
ATA_DFLAG_NCQ_OFF (and maybe even more which I forgot about).
Can we please move them into some more descriptive, ie which flags
are for the drive capabilities (ie _can_ the drive do NCQ) and
the current current drive status (ie _does_ the drive do NCQ)?
As it stands it's quite confusing.

In include/linux/libata.h, we have:

ATA_DFLAG_NCQ		= (1 << 3), /* device supports NCQ */
ATA_DFLAG_PIO		= (1 << 13), /* device limited to PIO mode */
ATA_DFLAG_NCQ_OFF	= (1 << 14), /* device limited to non-NCQ mode */

So there are some description. Not enough ?

Well. Guess my point is that ATA_DFLAG_PIO is a device status (which might change during runtime), whereas ATA_DFLAG_NCQ is a device setting
(either it does or does not support NCQ).
And ATA_DFLAG_NCQ_OFF is again a device status (device supports NCQ, but
it's disabled for whatever reason).
I'd rather have a more descriptive naming like
ATA_DFLAG_NCQ_SUPPORTED
to clearly indicate that this flag is not about what the driver _currently_ is using (as opposed to ATA_DFLAG_PIO), but rather a static
device configuration which won't change whatever I do.

Cheers,

Hannes




[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux