On 27/09/2022 00:05, Damien Le Moal wrote:
ata_scsi_find_dev() returns NULL, turning
__ata_change_queue_depth() into a nop, which prevents the user from
setting the maximum queue depth of ATA devices used with libsas based
HBAs.
Fix this by renaming __ata_change_queue_depth() to
ata_change_queue_depth() and adding a pointer to the ata_device
structure of the target device as argument. This pointer is provided by
ata_scsi_change_queue_depth() using ata_scsi_find_dev() in the case of
a libata managed device and by sas_change_queue_depth() using
sas_to_ata_dev() in the case of a libsas managed ata device.
This seems ok. But could you alternatively use ata_for_each_dev() and
match by ata_device.sdev pointer? That pointer is set quite late in the
probe, though, so maybe it would not work.
Not sure I understand why we should search for the ata device again using
ata_for_each_dev() when sas_to_ata_dev() gives us directly what we need
for the libsas controlled device... Can you clarify ?
Sure, we can use sas_to_ata_dev() to get the ata_device.
I am just suggesting my way such that we can have a consistent method to
get the ata_device between all libata users and we don't need to change
the ata_change_queue_depth() interface. It would be something like:
struct ata_device *ata_scsi_find_dev(struct ata_port *ap, const struct
scsi_device *scsidev)
{
struct ata_link *link;
struct ata_device *dev;
ata_for_each_link(link, ap, EDGE) {
ata_for_each_dev(dev, link, ENABLED) {
if (scsidev == dev->sdev)
return dev;
}
}
// todo: check pmp
return NULL;
}
Thanks,
John
