+
On 12/09/2023 10:00, Damien Le Moal wrote:
+int sas_ata_slave_configure(struct scsi_device *sdev)
+{
+ struct domain_device *dev = sdev_to_domain_dev(sdev);
+ struct ata_port *ap = dev->sata_dev.ap;
+ struct device *sdev_dev = &sdev->sdev_gendev;
+ struct device_link *link;
+
+ ata_sas_slave_configure(sdev, ap);
+
+ link = device_link_add(sdev_dev, &ap->tdev,
+ DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE);
I am not sure what is the point of this. For libsas/pm8001 driver, there
is no ata_port -> .. -> host dev link, right? If so, it just seems that
you are are adding a dependency to a device (&ap->tdev) which has no
dependency to a real device itself.
For libata, the point is to have PM order suspend resume operations correctly:
suspend: scsi dev first, then ata_port
resume: ata_port first then scsi dev
Strictly speaking, we should do that with ata_dev and scsi dev, but libata PM
operations are defined for ata_port, not ata_dev.
For libsas, the PM operations come from scsi, so scsi bus operations for
devices. And given that:
1) we should already have the dev chain:
hba dev -> scsi host ->scsi target -> scsi dev
That would seem right, since scsi_add_host() is passed the host dev and
the ancestry would be created naturally, but for hisi_sas a link was
still required. Let me check why that is again ...
2) libsas does its own ata PM control when suspending & resuming the HBA (if I
understand things correctly)
we should thus not need anything special for libsas. ata devices suspend/resume
will be done in the right order without an extra link. I do not really
understand why hisi_sas need to create that link. If it is indeed needed, then
we probably should have it created always by libsas generic code...
Thanks,
John
