On Mon, Sep 11, 2023 at 01:02:02PM +0900, Damien Le Moal wrote: > The introduction of a device link to create a consumer/supplier > relationship between the scsi device of an ATA device and the ATA port > of the ATA device fixed the ordering of the suspend and resume > operations. For suspend, the scsi device is suspended first and the ata > port after it. This is fine as this allows the synchronize cache and > START STOP UNIT commands issued by the scsi disk driver to be executed > before the ata port is disabled. > > For resume operations, the ata port is resumed first, followed > by the scsi device. This allows having the request queue of the scsi > device to be unfrozen after the ata port restart is scheduled in EH, > thus avoiding to see new requests issued to the ATA device prematurely. > However, since libata sets manage_start_stop to 1, the scsi disk resume > operation also results in issuing a START STOP UNIT command to wakeup > the device. This is too late and that must be done before libata EH > resume handling starts revalidating the drive with IDENTIFY etc > commands. Commit 0a8589055936 ("ata,scsi: do not issue START STOP UNIT > on resume") disabled issuing the START STOP UNIT command to avoid > issues with it. However, this is incorrect as transitioning a device to > the active power mode from the standby power mode set on suspend > requires a media access command. The device link reset and subsequent > SET FEATURES, IDENTIFY and READ LOG commands executed in libata EH > context triggered by the ata port resume operation may thus fail. > > Fix this by handling a device power mode transitions for suspend and > resume in libata EH context without relying on the scsi disk management > triggered with the manage_start_stop flag. > > To do this, the following libata helper functions are introduced: > > 1) ata_dev_power_set_standby(): > > This function issues a STANDBY IMMEDIATE command to transitiom a device > to the standby power mode. For HDDs, this spins down the disks. This > function applies only to ATA and ZAC devices and does nothing otherwise. > This function also does nothing for devices that have the > ATA_FLAG_NO_POWEROFF_SPINDOWN or ATA_FLAG_NO_HIBERNATE_SPINDOWN flag > set. > > For suspend, call ata_dev_power_set_standby() in > ata_eh_handle_port_suspend() before the port is disabled and frozen. > ata_eh_unload() is also modified to transition all enabled devices to > the standby power mode when the system is shutdown or devices removed. > > 2) ata_dev_power_set_active() and > > This function applies to ATA or ZAC devices and issues a VERIFY command > for 1 sector at LBA 0 to transition the device to the active power mode. > For HDDs, since this function will complete only once the disk spin up. > Its execution uses the same timeouts as for reset, to give the drive > enough time to complete spinup without triggering a command timeout. > > For resume, call ata_dev_power_set_active() in > ata_eh_revalidate_and_attach() after the port has been enabled and > before any other command is issued to the device. > > With these changes, the manage_start_stop flag does not need to be set > in ata_scsi_dev_config(). > > Fixes: 0a8589055936 ("ata,scsi: do not issue START STOP UNIT on resume") > Fixes: a19a93e4c6a9 ("scsi: core: pm: Rely on the device driver core for async power management") > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx> Tested-by: Chia-Lin Kao (AceLan) <acelan.kao@xxxxxxxxxxxxx>