On 8/22/25 11:49 PM, Niklas Cassel wrote:
> On Fri, Aug 22, 2025 at 02:34:54PM +0200, Niklas Cassel wrote:
>
> (snip)
>
>> But do we really want a introduce a module parameter for this?
>> I know that commit 4edf1505b76d ("ata: ahci: Disallow LPM policy control for
>> external ports")
>> if external, both:
>> 1) sets initial lpm policy to MAX_POWER (so that hot plug works by default)
>> 2) sets ATA_FLAG_NO_LPM, so that you cannot change the LPM policy using sysfs
>>
>> I think that 1) is good.
>>
>> However, why should we forbid the user to override to policy via sysfs just
>> because the port is external?
>> If a system admin has installed a udev rule or similar to set a lpm policy,
>> why should we not respect that?
>>
>> Yes, I know that many distros supply a rule that just enables LPM
>> unconditionally for all disks...
>>
>> But... instead of forbidding the user to change to policy using sysfs, perhaps
>> a better way would be for the system admin/distros to improve their udev rules?
>>
>> We have a sysfs property that says if the port supports LPM.
>> Perhaps we should have a sysfs attribute that says if the port is external.
>>
>> The udev rules can then be smarter and just set the LPM policy if the port is
>> not external. But the user would still have the option to set a LPM policy
>> (using the same interface), if they don't care about hotplug.
>>
>> It seems more user friendly for a user that has a laptop with a docking
>> station with hotplug capable ports, to install a udev rule to set an LPM
>> policy, than to set a kernel module param.
>>
>> What do you think?
>
> Another idea: perhaps we could add something like:
> "hotplug_supported" and "hotplug_enable" to sysfs.
Yes, that would be nice to have, but... see below.
> For ports marked as external/hotplug capable, we set
> "hotplug_supported" to true, and for ports that support
> hotplug, we set "hotplug_enable" to true by default.
>
> We can then continue to disallow (return -EOPNOTSUPP) when the user tries
> to write to /sys/class/scsi_host/hostX/link_power_management_policy
> for a port that has "hotplug_enable" == true.
>
> If a user sets "hotplug_enable" = false, we allow writing to
> /sys/class/scsi_host/hostX/link_power_management_policy
>
> What do you think? Better or worse idea?
It would be better, but:
1) When setting hotplug_enable to false, we need to set NO_LPM for the port and
change the target policy to max-performance. Not too hard to do, but then...
2) When setting hotplug_enable to true, we need to clear NO_LPM for the port
and set the target policy to the default. However, we actually have no idea
what set NO_LPM. It may be that the device/adapter/port actually do not support
LPM, so in that case, we should not clear it.
So because of (2), unless we cleanup the mess of port/device flags to have
different flags to differentiate between "HW supports X" and "X is
enabled/disabled", I do not think we can do the sysfs trick easily.
--
Damien Le Moal
Western Digital Research
