Re: [PATCH 2/2] ata: libata-core: Revert "ata: libata-core: Fix ata_pci_shutdown_one()"

[Sergei Shtylyov <sergei.shtylyov@gmail.com>]

On 1/11/24 2:51 PM, Damien Le Moal wrote:

> This reverts commit fd3a6837d8e18cb7be80dcca1283276290336a7a.
> 
> Several users have signaled issues with commit fd3a6837d8e1 ("ata:
> libata-core: Fix ata_pci_shutdown_one()") which causes failure of the
> system SoC to go to a low power state. The reason for this problem
> is not well understood but given that this patch is harmless with the
> improvements to ata_dev_power_set_standby(), restore it to allow system
> lower power state transitions.
> 
> For regular system shutdown, ata_dev_power_set_standby() will be
> executed twice: once the scsi device is removed and another when
> ata_pci_shutdown_one() executes and EH completes unloading the devices.
> Make the second call to ata_dev_power_set_standby() do nothing by using
> ata_dev_power_is_active() and return if the device is already in
> standby.
> 
> Fixes: fd3a6837d8e1 ("ata: libata-core: Fix ata_pci_shutdown_one()")
> Cc: stable@vger.kernel.org
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>  drivers/ata/libata-core.c | 75 +++++++++++++++++++++++----------------
>  1 file changed, 45 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index d9f80f4f70f5..20a366942626 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2001,6 +2001,33 @@ bool ata_dev_power_init_tf(struct ata_device *dev, struct ata_taskfile *tf,
>  	return true;
>  }
>  
> +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;

   Why set ATA_TFLAG_ISADDR, BTW? This command doesn't use any taskfile
regs but the device/head reg. Material for a fix, I guess... :-)

> +	tf.protocol = ATA_PROT_NODATA;
> +	tf.command = ATA_CMD_CHK_POWER;
> +
[...]

MBR, Sergey