Hi,
On 2/28/22 22:33, Mario Limonciello wrote:
> As the default low power policy applies to more chipsets and drives, it's
> important to make sure that drives actually support the policy that a user
> selected in their kernel configuration.
>
> If the drive doesn't support slumber, don't let the default policies
> dependent upon slumber (`min_power` or `min_power_with_partial`) affect the
> disk.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@xxxxxxx>
> ---
> Changes from RFC v1 -> PATCH v1:
> * Move the warning and protection from drives that don't support slumber further
> into the calling path.
> drivers/ata/libahci.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> index 0ed484e04fd6..9dee721ed1fe 100644
> --- a/drivers/ata/libahci.c
> +++ b/drivers/ata/libahci.c
> @@ -785,7 +785,15 @@ static int ahci_set_lpm(struct ata_link *link, enum ata_lpm_policy policy,
> pp->intr_mask &= ~PORT_IRQ_PHYRDY;
> writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
>
> - sata_link_scr_lpm(link, policy, false);
> + if (policy >= ATA_LPM_MIN_POWER_WITH_PARTIAL &&
> + !(hpriv->cap & HOST_CAP_SSC)) {
> + struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> +
> + dev_warn(&pdev->dev,
> + "This drive doesn't support slumber; ignoring SATA policy\n");
Not sure if completely ignoring the policy here is a good idea. IMHO it would
be better to fallback to MED_POWER_WITH_DIPM at least for the haswell - skylake(++++)
generations having MED_POWER_WITH_DIPM is important to allow the package to
go the higher PC states.
> + } else {
> + sata_link_scr_lpm(link, policy, false);
> + }
> }
>
> if (hpriv->cap & HOST_CAP_ALPM) {
Regards,
Hans
