On 9/5/23 05:42, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@xxxxxxx>
>
> In AHCI 1.3.1, the register description for CAP.SSC:
> "When cleared to ‘0’, software must not allow the HBA to initiate
> transitions to the Slumber state via agressive link power management nor
> the PxCMD.ICC field in each port, and the PxSCTL.IPM field in each port
> must be programmed to disallow device initiated Slumber requests."
>
> In AHCI 1.3.1, the register description for CAP.PSC:
> "When cleared to ‘0’, software must not allow the HBA to initiate
> transitions to the Partial state via agressive link power management nor
> the PxCMD.ICC field in each port, and the PxSCTL.IPM field in each port
> must be programmed to disallow device initiated Partial requests."
>
> Ensure that we always set the corresponding bits in PxSCTL.IPM, such that
> a device is not allowed to initiate transitions to power states which are
> unsupported by the HBA.
>
> DevSleep is always initiated by the HBA, however, for completeness, set the
> corresponding bit in PxSCTL.IPM such that agressive link power management
> cannot transition to DevSleep if DevSleep is not supported.
>
> sata_link_scr_lpm() is used by libahci, ata_piix and libata-pmp.
> However, only libahci has the ability to read the CAP/CAP2 register to see
> if these features are supported. Therefore, in order to not introduce any
> regressions on ata_piix or libata-pmp, create flags that indicate that the
> respective feature is NOT supported. This way, the behavior for ata_piix
> and libata-pmp should remain unchanged.
>
> This change is based on a patch originally submitted by Runa Guo-oc.
>
> Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
Looks good, but don't we need a Fixes tag for this ?
> ---
> drivers/ata/ahci.c | 9 +++++++++
> drivers/ata/libata-sata.c | 19 ++++++++++++++++---
> include/linux/libata.h | 4 ++++
> 3 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index abb5911c9d09..08745e7db820 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -1883,6 +1883,15 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> else
> dev_info(&pdev->dev, "SSS flag set, parallel bus scan disabled\n");
>
> + if (!(hpriv->cap & HOST_CAP_PART))
> + host->flags |= ATA_HOST_NO_PART;
> +
> + if (!(hpriv->cap & HOST_CAP_SSC))
> + host->flags |= ATA_HOST_NO_SSC;
> +
> + if (!(hpriv->cap2 & HOST_CAP2_SDS))
> + host->flags |= ATA_HOST_NO_DEVSLP;
> +
> if (pi.flags & ATA_FLAG_EM)
> ahci_reset_em(host);
>
> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
> index 5d31c08be013..a701e1538482 100644
> --- a/drivers/ata/libata-sata.c
> +++ b/drivers/ata/libata-sata.c
> @@ -396,10 +396,23 @@ int sata_link_scr_lpm(struct ata_link *link, enum ata_lpm_policy policy,
> case ATA_LPM_MED_POWER_WITH_DIPM:
> case ATA_LPM_MIN_POWER_WITH_PARTIAL:
> case ATA_LPM_MIN_POWER:
> - if (ata_link_nr_enabled(link) > 0)
> - /* no restrictions on LPM transitions */
> + if (ata_link_nr_enabled(link) > 0) {
> + /* assume no restrictions on LPM transitions */
> scontrol &= ~(0x7 << 8);
> - else {
> +
> + /*
> + * If the controller does not support partial, slumber,
> + * or devsleep, then disallow these transitions.
> + */
> + if (link->ap->host->flags & ATA_HOST_NO_PART)
> + scontrol |= (0x1 << 8);
> +
> + if (link->ap->host->flags & ATA_HOST_NO_SSC)
> + scontrol |= (0x2 << 8);
> +
> + if (link->ap->host->flags & ATA_HOST_NO_DEVSLP)
> + scontrol |= (0x4 << 8);
> + } else {
> /* empty port, power off */
> scontrol &= ~0xf;
> scontrol |= (0x1 << 2);
> diff --git a/include/linux/libata.h b/include/linux/libata.h
> index 52d58b13e5ee..bf4913f4d7ac 100644
> --- a/include/linux/libata.h
> +++ b/include/linux/libata.h
> @@ -222,6 +222,10 @@ enum {
> ATA_HOST_PARALLEL_SCAN = (1 << 2), /* Ports on this host can be scanned in parallel */
> ATA_HOST_IGNORE_ATA = (1 << 3), /* Ignore ATA devices on this host. */
>
> + ATA_HOST_NO_PART = (1 << 4), /* Host does not support partial */
> + ATA_HOST_NO_SSC = (1 << 5), /* Host does not support slumber */
> + ATA_HOST_NO_DEVSLP = (1 << 6), /* Host does not support devslp */
> +
> /* bits 24:31 of host->flags are reserved for LLD specific flags */
>
> /* various lengths of time */
--
Damien Le Moal
Western Digital Research
