[PATCH i-g-t] lib/igt_pm: Skip LPM policy control on unsupported SATA hosts

[Damien Le Moal]

On 7/31/25 4:41 PM, Chaitanya Kumar Borah wrote:
> With [1], LPM policy control is now disabled for external SATA ports.
> This led to a regression in our CI runs [2]. Discussions on the regression
> resulted in patch [3] which introduces a new sysfs entry, querying
> which we can determine if LPM policy control is allowed on a host or not.
> 
> Store/Re-store LPM policy only when it is allowed. If the sysfs entry is
> not present assume LPM policy control is allowed. This  maintains backward
> compatibility.

Note: allowed, yes, but that does not mean it will actually work or do
anything, since the port or the device may not be supporting LPN at all.

> 
> [1] https://lore.kernel.org/linux-ide/20250701125321.69496-8-dlemoal@kernel.org/
> [2] https://lore.kernel.org/linux-ide/07563042-6576-41cd-9a95-de83cfc95de1@intel.com/
> [3] https://lore.kernel.org/linux-ide/20250730001947.332661-1-dlemoal@kernel.org/
> 
> Signed-off-by: Chaitanya Kumar Borah <chaitanya.kumar.borah at intel.com>

Patch looks OK to me (nit below though).

Reviewed-by: Damien Le Moal <dlemoal at kernel.org>


> ---
>  lib/igt_pm.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/lib/igt_pm.c b/lib/igt_pm.c
> index cf1c442f8..1ffcdcef3 100644
> --- a/lib/igt_pm.c
> +++ b/lib/igt_pm.c
> @@ -321,6 +321,35 @@ void igt_pm_enable_audio_runtime_pm(void)
>  		igt_debug("Failed to enable audio runtime PM! (%d)\n", -err);
>  }
>  
> +static bool lpm_supported_by_sata_host(int host_num)
> +{
> +	int fd;
> +	ssize_t len;
> +	char buf[2];
> +	char file_name[PATH_MAX];
> +
> +	snprintf(file_name, PATH_MAX,
> +		 "/sys/class/scsi_host/host%d/link_power_management_supported",
> +		 host_num);
> +
> +	fd = open(file_name, O_RDONLY);
> +
> +	/* assume LPM is supported if sysfs entry is absent. This preserves default behaviour */
> +	if (fd < 0)
> +		return true;
> +
> +	len = read(fd, buf, 1);
> +
> +	close(fd);

Nit: this would all be a lot simpler using scanf:
	
	snprintf(file_name, PATH_MAX,
		 "/sys/class/scsi_host/host%d/link_power_management_supported",
		 host_num);

	/*
	 * If sysfs entry is absent, assume that LPM is supported to
	 * preserve the default behaviour.
	 */
	if (scanf(file_name, "%d", &supported) != 1)
		return true;

	return supported;

> +
> +	if (len <= 0)
> +		return true;
> +
> +	buf[len] = '\0';
> +
> +	return buf[0] == '1';
> +}
> +
>  static void __igt_pm_enable_sata_link_power_management(void)
>  {
>  	int fd, i;
> @@ -372,6 +401,9 @@ static void __igt_pm_enable_sata_link_power_management(void)
>  	igt_install_exit_handler(__igt_pm_sata_link_pm_exit_handler);
>  
>  	for (i = 0; i < __scsi_host_cnt; i++) {
> +		if (!lpm_supported_by_sata_host(i))
> +			continue;
> +
>  		snprintf(file_name, PATH_MAX,
>  			 "/sys/class/scsi_host/host%d/link_power_management_policy",
>  			 i);
> @@ -412,6 +444,9 @@ static void __igt_pm_restore_sata_link_power_management(void)
>  	for (i = 0; i < __scsi_host_cnt; i++) {
>  		int8_t policy;
>  
> +		if (!lpm_supported_by_sata_host(i))
> +			continue;
> +
>  		if (__sata_pm_policies[i] == POLICY_UNKNOWN)
>  			continue;
>  		else


-- 
Damien Le Moal
Western Digital Research