Re: [PATCH 01/10] ata: libata-core: Introduce ata_dev_config_lpm()

[Niklas Cassel <cassel@kernel.org>]

On Mon, Jun 30, 2025 at 03:26:28PM +0900, Damien Le Moal wrote:
> If the port of a device does not support Device Initiated Power
> Management (DIPM), that is, the port is flagged with ATA_FLAG_NO_DIPM,
> the DIPM feature of a device should be disabled when the device is first
> configured.
> 
> Introduce the function ata_dev_config_lpm() to disable DIPM on a
> device that supports this feature if the port of the device is flagged
> with ATA_FLAG_NO_DIPM. ata_dev_config_lpm() is called from
> ata_dev_configure(), ensuring that a device DIPM feature is disabled
> when it cannot be used.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>
> ---
>  drivers/ata/libata-core.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 3918ea624e0b..0d85474f6640 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -2790,6 +2790,27 @@ static void ata_dev_config_cpr(struct ata_device *dev)
>  	kfree(buf);
>  }
>  
> +/*
> + * Configure features related to link power management.
> + */
> +static void ata_dev_config_lpm(struct ata_device *dev)
> +{
> +	struct ata_port *ap = dev->link->ap;
> +	unsigned int err_mask;
> +
> +	/*
> +	 * If the device port does not support Device Initiated Power Management
> +	 * (DIPM), and the device supports this feature, disable it.

Perhaps mention in the comment that even though DIPM is disabled by default,
because of the "Software Settings Preservation feature", DIPM can be enabled
(even across a COMRESET) when using LPM policy "Keep FW settings".


> +	 */
> +	if (ap->flags & ATA_FLAG_NO_DIPM && ata_id_has_dipm(dev->id)) {
> +		err_mask = ata_dev_set_feature(dev,
> +					SETFEATURES_SATA_DISABLE, SATA_DIPM);
> +		if (err_mask && err_mask != AC_ERR_DEV)
> +			ata_dev_err(dev, "Disable DIPM failed, Emask 0x%x\n",
> +				    err_mask);
> +	}
> +}
> +
>  static void ata_dev_print_features(struct ata_device *dev)
>  {
>  	if (!(dev->flags & ATA_DFLAG_FEATURES_MASK))
> @@ -2963,6 +2984,7 @@ int ata_dev_configure(struct ata_device *dev)
>  			ata_dev_config_chs(dev);
>  		}
>  
> +		ata_dev_config_lpm(dev);
>  		ata_dev_config_fua(dev);
>  		ata_dev_config_devslp(dev);
>  		ata_dev_config_sense_reporting(dev);
> -- 
> 2.50.0
>