Re: [PATCH v2 0/5] drop low power policy board type

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/6/2024 15:13, 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@xxxxxxx/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@xxxxxxxxxx/

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.



For the series:
Reviewed-by: Mario Limonciello <mario.limonciello@xxxxxxx>

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(-)






[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux