From: Niklas Cassel <cassel@kernel.org>
To: Damien Le Moal <dlemoal@kernel.org>
Cc: Werner Fischer <devlists@wefi.net>,
Daniel Drake <drake@endlessos.org>,
Mika Westerberg <mika.westerberg@linux.intel.com>,
Jian-Hong Pan <jhp@endlessos.org>,
Dieter Mummenschanz <dmummenschanz@web.de>,
Mario Limonciello <mario.limonciello@amd.com>,
linux-ide@vger.kernel.org
Subject: Re: [PATCH v2 0/5] drop low power policy board type
Date: Fri, 9 Feb 2024 11:01:00 +0100 [thread overview]
Message-ID: <ZcX33BLsEnDGez0z@x1-carbon> (raw)
In-Reply-To: <20240206211352.1664816-1-cassel@kernel.org>
On Tue, Feb 06, 2024 at 10:13:41PM +0100, Niklas Cassel wrote:
> The series is based on top of:
> https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/log/?h=for-next
>
>
> Hello all,
>
> This revives a patch sent out almost two years ago from Mario Limonciello:
> https://lore.kernel.org/linux-ide/20220524170508.563-1-mario.limonciello@amd.com/T/#u
>
> The reason why we did not merge it back then, is because LPM and hotplug
> events are mutually exclusive.
>
> The difference with this series compared to what was sent out back then:
> I've added a patch that checks if the port is external, i.e. either
> hotplug capable or eSATA. For external ports, we never enable LPM, as
> that will break hotplug.
>
> For ports that do not advertise themselves as external (typically laptops),
> we set the LPM policy as requested.
>
> This matches how Microsoft Windows does things, see:
> https://studylib.net/doc/10034428/esata---microsoft-center
>
> Thanks to Werner Fischer for suggesting something like this at last year's
> ALPSS conference.
>
> There might of course be some platform firmware that e.g. incorrectly marks
> its port as internal, even though it is external, but if we find any such
> platforms we will need to deal with them using quirks.
>
>
> Also note that we even if the user requested a certain policy, there is
> no guarantee that he will get all the features for that policy, see:
> https://github.com/torvalds/linux/blob/master/drivers/ata/libata-sata.c#L403-L414
>
> However, I'd rather we not try to map all the combinations of
> partial/slumber/devsleep in to a single policy represented by a single
> integer, thus I do not try to "change" the requested policy.
> The user will get all the features that are included in the requested
> policy AND supported by the HBA.
>
> Another difference (compared to an earlier version of Mario's series)
> is that we do not try to change the default CONFIG_SATA_MOBILE_LPM_POLICY
> value from 0 to 3, it will continue to be 0.
> If you really don't want LPM even if your HBA supports it, and your port
> is internal, one option is to leave the Kconfig set to the default value.
>
> Damien: considering that the Intel VMD + ahci_intel_pcs_quirk() bug turned
> out to have nothing to do with LPM, it was simply the fact that the
> ahci_intel_pcs_quirk() was only applied if there was an explicit entry in
> ahci_pci_tbl. So since that bug is totally unrelated to LPM, I no longer
> think that this series need to wait for a fix for that bug.
>
>
> Link to v1:
> https://lore.kernel.org/linux-ide/20240201161507.1147521-1-cassel@kernel.org/
>
> Changes since v1:
> -Picked up tags from Damien.
> -Moved the comment in front of ahci_mark_external_port() to inside the
> function.
> -Modified the comment in patch 4/5 to more clearly state hotplug removal
> events.
> -Rewrote the commit message for patch 4/5 to be more detailed.
>
>
> Kind regards,
> Niklas
>
>
> Mario Limonciello (1):
> ata: ahci: Drop low power policy board type
>
> Niklas Cassel (4):
> ata: ahci: move marking of external port earlier
> ata: ahci: a hotplug capable port is an external port
> ata: ahci: drop hpriv param from ahci_update_initial_lpm_policy()
> ata: ahci: do not enable LPM on external ports
>
> drivers/ata/Kconfig | 5 +-
> drivers/ata/ahci.c | 135 +++++++++++++++++++++++-------------------
> drivers/ata/ahci.h | 9 +--
> drivers/ata/libahci.c | 7 ---
> 4 files changed, 78 insertions(+), 78 deletions(-)
>
> --
> 2.43.0
>
Applied:
https://git.kernel.org/pub/scm/linux/kernel/git/libata/linux.git/log/?h=for-6.9
prev parent reply other threads:[~2024-02-09 10:01 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-06 21:13 [PATCH v2 0/5] drop low power policy board type Niklas Cassel
2024-02-06 21:13 ` [PATCH v2 1/5] ata: ahci: move marking of external port earlier Niklas Cassel
2024-02-06 21:13 ` [PATCH v2 2/5] ata: ahci: a hotplug capable port is an external port Niklas Cassel
2024-02-06 21:13 ` [PATCH v2 3/5] ata: ahci: drop hpriv param from ahci_update_initial_lpm_policy() Niklas Cassel
2024-02-07 4:19 ` Jian-Hong Pan
2024-02-06 21:13 ` [PATCH v2 4/5] ata: ahci: do not enable LPM on external ports Niklas Cassel
2024-02-08 23:34 ` Damien Le Moal
2024-02-06 21:13 ` [PATCH v2 5/5] ata: ahci: Drop low power policy board type Niklas Cassel
2024-02-07 4:19 ` Jian-Hong Pan
2024-02-06 21:54 ` [PATCH v2 0/5] drop " Mario Limonciello
2024-02-07 4:30 ` Jian-Hong Pan
2024-02-07 6:35 ` Mika Westerberg
2024-02-08 23:43 ` Damien Le Moal
2024-02-09 10:01 ` Niklas Cassel [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZcX33BLsEnDGez0z@x1-carbon \
--to=cassel@kernel.org \
--cc=devlists@wefi.net \
--cc=dlemoal@kernel.org \
--cc=dmummenschanz@web.de \
--cc=drake@endlessos.org \
--cc=jhp@endlessos.org \
--cc=linux-ide@vger.kernel.org \
--cc=mario.limonciello@amd.com \
--cc=mika.westerberg@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).