RE: [PATCH AUTOSEL 5.15 28/39] ata: libahci: Adjust behavior when StorageD3Enable _DSD is set

From: Limonciello, Mario
Date: Mon Nov 29 2021 - 09:48:13 EST




> -----Original Message-----
> From: Sasha Levin <sashal@xxxxxxxxxx>
> Sent: Thursday, November 25, 2021 20:32
> To: linux-kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> Cc: Limonciello, Mario <Mario.Limonciello@xxxxxxx>; Shah, Nehal-
> bakulchandra <Nehal-bakulchandra.Shah@xxxxxxx>; Damien Le Moal
> <damien.lemoal@xxxxxxxxxxxxxxxxxx>; Sasha Levin <sashal@xxxxxxxxxx>;
> linux-ide@xxxxxxxxxxxxxxx
> Subject: [PATCH AUTOSEL 5.15 28/39] ata: libahci: Adjust behavior when
> StorageD3Enable _DSD is set
>
> From: Mario Limonciello <mario.limonciello@xxxxxxx>
>
> [ Upstream commit 7c5f641a5914ce0303b06bcfcd7674ee64aeebe9 ]
>
> The StorageD3Enable _DSD is used for the vendor to indicate that the disk
> should be opted into or out of a different behavior based upon the platform
> design.
>
> For AMD's Renoir and Green Sardine platforms it's important that any
> attached SATA storage has transitioned into DevSlp when s2idle is used.
>
> If the disk is left in active/partial/slumber, then the system is not able
> to resume properly.
>
> When the StorageD3Enable _DSD is detected, check the system is using
> s2idle
> and DevSlp is enabled and if so explicitly wait long enough for the disk to
> enter DevSlp.
>
> Cc: Nehal-bakulchandra Shah <Nehal-bakulchandra.Shah@xxxxxxx>
> BugLink:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugz
> illa.kernel.org%2Fshow_bug.cgi%3Fid%3D214091&amp;data=04%7C01%7Cm
> ario.limonciello%40amd.com%7C15dc139812a0497d31bc08d9b0850f45%7C3d
> d8961fe4884e608e11a82d994e183d%7C0%7C0%7C637734907816859936%7CU
> nknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI
> 6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=%2BX2cfl%2BYeFYWZJ%2
> FPFWX%2FzxnNtneb2er7w%2BeJpVxxBcU%3D&amp;reserved=0
> Link:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs
> .microsoft.com%2Fen-us%2Fwindows-hardware%2Fdesign%2Fcomponent-
> guidelines%2Fpower-management-for-storage-hardware-devices-
> intro&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C15dc139812
> a0497d31bc08d9b0850f45%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C
> 0%7C637734907816859936%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLj
> AwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;
> sdata=ubGKpzMA6EaXugHAanwRUQJ2lvL957wBRFKKMjUBGlw%3D&amp;re
> served=0
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> Signed-off-by: Damien Le Moal <damien.lemoal@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> ---
> drivers/ata/libahci.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index 5b3fa2cbe7223..395772fa39432 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -2305,6 +2305,18 @@ int ahci_port_resume(struct ata_port *ap)
> EXPORT_SYMBOL_GPL(ahci_port_resume);
>
> #ifdef CONFIG_PM
> +static void ahci_handle_s2idle(struct ata_port *ap)
> +{
> + void __iomem *port_mmio = ahci_port_base(ap);
> + u32 devslp;
> +
> + if (pm_suspend_via_firmware())
> + return;
> + devslp = readl(port_mmio + PORT_DEVSLP);
> + if ((devslp & PORT_DEVSLP_ADSE))
> + ata_msleep(ap, devslp_idle_timeout);
> +}
> +
> static int ahci_port_suspend(struct ata_port *ap, pm_message_t mesg)
> {
> const char *emsg = NULL;
> @@ -2318,6 +2330,9 @@ static int ahci_port_suspend(struct ata_port *ap,
> pm_message_t mesg)
> ata_port_freeze(ap);
> }
>
> + if (acpi_storage_d3(ap->host->dev))
> + ahci_handle_s2idle(ap);
> +
> ahci_rpm_put_port(ap);
> return rc;
> }
> --
> 2.33.0

Sasha,

No concerns for me to 5.15 or any of the earlier kernels the autosel picked, but would you mind also sending this to 5.14.y too?

Thanks,