On 2022/02/28 23: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)) {
Wouldn't it be safer to have the HOST_CAP_SSC check inside sata_link_scr_lpm() ?
There are other caller sites...
> + struct pci_dev *pdev = to_pci_dev(ap->host->dev);
> +
> + dev_warn(&pdev->dev,
> + "This drive doesn't support slumber; ignoring SATA policy\n"> + } else {
> + sata_link_scr_lpm(link, policy, false);
> + }
> }
>
> if (hpriv->cap & HOST_CAP_ALPM) {
--
Damien Le Moal
Western Digital Research
