On 6/27/25 15:04, Hannes Reinecke wrote:
> On 6/27/25 03:11, Damien Le Moal wrote:
>> Improve the comments describing the different values of enum
>> ata_lpm_policy in include/linux/libata.h. The comments match the
>> description given for the CONFIG_SATA_MOBILE_LPM_POLICY config
>> parameter.
>>
>> No functional changes.
>>
>> Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
>> ---
>> include/linux/libata.h | 17 ++++++++++++-----
>> 1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/libata.h b/include/linux/libata.h
>> index 721f0805b6c9..f8bdf167bad9 100644
>> --- a/include/linux/libata.h
>> +++ b/include/linux/libata.h
>> @@ -499,16 +499,23 @@ enum ata_completion_errors {
>> };
>>
>> /*
>> - * Link power management policy: If you alter this, you also need to
>> - * alter libata-sata.c (for the ascii descriptions)
>> + * Link power management policy: If you alter this, you also need to alter
>> + * the policy names used with the sysfs attribute link_power_management_policy
>> + * defined in libata-sata.c
>> */
>> enum ata_lpm_policy {
>> + /* 0 => Keep firmware settings */
>> ATA_LPM_UNKNOWN,
>> + /* 1 => No power savings (maximum performance) */
>> ATA_LPM_MAX_POWER,
>> + /* 2 => HIPM (Partial) */
>> ATA_LPM_MED_POWER,
>> - ATA_LPM_MED_POWER_WITH_DIPM, /* Med power + DIPM as win IRST does */
>> - ATA_LPM_MIN_POWER_WITH_PARTIAL, /* Min Power + partial and slumber */
>> - ATA_LPM_MIN_POWER, /* Min power + no partial (slumber only) */
>> + /* 3 => HIPM (Partial) and DIPM (Partial and Slumber) */
>> + ATA_LPM_MED_POWER_WITH_DIPM,
>> + /* 4 => HIPM (Partial and DevSleep) and DIPM (Partial and Slumber) */
>> + ATA_LPM_MIN_POWER_WITH_PARTIAL,
>> + /* 5 => HIPM (Slumber and DevSleep) and DIPM (Partial and Slumber) */
>> + ATA_LPM_MIN_POWER,
>> };
>>
>> enum ata_lpm_hints {
>
> It feels really weird to have the values for the enum in the _comment_.
> I'd rather drop them and just have the comments without values.
That is fair. The intent here was to link with CONFIG_SATA_MOBILE_LPM_POLICY
which defines the default LPM policy to use. The possible values of this enum
are what goes into that config option. But I can state that in the comment above
the enum.
>
> Cheers,
>
> Hannes
--
Damien Le Moal
Western Digital Research
