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 ? > > But probably not a problem with this patch, so: > > Reviewed-by: Hannes Reinecke <hare@xxxxxxx> > > Cheers, > > Hannes -- Damien Le Moal Western Digital Research
