diff mbox series

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

Message ID 20230605013212.573489-4-dlemoal@kernel.org
State Superseded
Headers show
Series Cleanups and improvements | expand

Commit Message

Damien Le Moal June 5, 2023, 1:32 a.m. UTC 
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@kernel.org>
---
 drivers/ata/libata-scsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Hannes Reinecke June 5, 2023, 8:04 a.m. UTC | #1 
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@kernel.org>
> ---
>   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.

But probably not a problem with this patch, so:

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
Sergei Shtylyov June 5, 2023, 9:47 a.m. UTC | #2 
On 6/5/23 4:32 AM, Damien Le Moal wrote:

> In ata_scsi_dev_config(), instead of hardconing the test to check if

   Again, hardcoding?

> an ATA device supports NCQ by looking at the ATA_DFLAG_NCQ flag, use
> ata_ncq_supported().
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
[...]

MBR, Sergey
Damien Le Moal June 5, 2023, 9:48 a.m. UTC | #3 
On 6/5/23 18:47, Sergei Shtylyov wrote:
> On 6/5/23 4:32 AM, Damien Le Moal wrote:
> 
>> In ata_scsi_dev_config(), instead of hardconing the test to check if
> 
>    Again, hardcoding?

Yes... Square fingers today :)

> 
>> an ATA device supports NCQ by looking at the ATA_DFLAG_NCQ flag, use
>> ata_ncq_supported().
>>
>> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> [...]
> 
> MBR, Sergey
diff mbox series

Patch

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);