On Thu, Oct 12, 2023 at 04:18:00PM +0900, Damien Le Moal wrote:
> Improve the function ata_dev_power_set_active() by having it do nothing
> for a disk that is already in the active power state. To do that,
> introduce the function ata_dev_power_is_active() to test the current
> power state of the disk and return true if the disk is in the PM0:
> active or PM1: idle state (0xff value for the count field of the CHECK
> POWER MODE command output).
>
> To preserve the existing behavior, if the CHECK POWER MODE command
> issued in ata_dev_power_is_active() fails, the drive is assumed to be in
> standby mode and false is returned.
>
> With this change, issuing the VERIFY command to access the disk media to
> spin it up becomes unnecessary most of the time during system resume as
> the port reset done by libata-eh on resume often result in the drive to
> spin-up (this behavior is not clearly defined by the ACS specifications
> and may thus vary between disk models).
>
> Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> ---
> drivers/ata/libata-core.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 83613280928b..6fb4e8dc8c3c 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2042,6 +2042,33 @@ void ata_dev_power_set_standby(struct ata_device *dev)
> err_mask);
> }
>
> +static bool ata_dev_power_is_active(struct ata_device *dev)
> +{
> + struct ata_taskfile tf;
> + unsigned int err_mask;
> +
> + ata_tf_init(dev, &tf);
> + tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR;
> + tf.protocol = ATA_PROT_NODATA;
> + tf.command = ATA_CMD_CHK_POWER;
> +
> + err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0);
> + if (err_mask) {
> + ata_dev_err(dev, "Check power mode failed (err_mask=0x%x)\n",
> + err_mask);
> + /*
> + * Assume we are in standby mode so that we always force a
> + * spinup in ata_dev_power_set_active().
> + */
> + return false;
> + }
> +
> + ata_dev_dbg(dev, "Power mode: 0x%02x\n", tf.nsect);
> +
> + /* Active or idle */
> + return tf.nsect == 0xff;
> +}
> +
> /**
> * ata_dev_power_set_active - Set a device power mode to active
> * @dev: target device
> @@ -2065,6 +2092,13 @@ void ata_dev_power_set_active(struct ata_device *dev)
> if (!ata_dev_power_init_tf(dev, &tf, true))
> return;
>
> + /*
> + * Check the device power state & condition and force a spinup with
> + * VERIFY command only if the drive is not already ACTIVE or IDLE.
> + */
> + if (ata_dev_power_is_active(dev))
> + return;
> +
> ata_dev_notice(dev, "Entering active power mode\n");
>
> err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0);
> --
> 2.41.0
>
Hello Damien,
For a disk that is already spun up, is a READ VERIFY SECTOR(S) – 40h command
with LBA == 0, NSECTORS == 1, really that much slower than a ATA_CMD_CHK_POWER
command?
Kind regards,
Niklas
