Re: [PATCH v4 04/23] scsi: sd: Differentiate system and runtime start/stop management

[Bart Van Assche <bvanassche@acm.org>]

On 9/20/23 06:54, Damien Le Moal wrote:
> The underlying device and driver of a scsi disk may have different
> system and runtime power mode control requirements. This is because
> runtime power management affects only the scsi disk, while sustem level
> power management affects all devices, including the controller for the
> scsi disk.
> 
> For instance, issuing a START STOP UNIT command when a scsi disk is
> runtime suspended and resumed is fine: the command is translated to a
> STANDBY IMMEDIATE command to spin down the ATA disk and to a VERIFY
> command to wake it up. The scsi disk runtime operations have no effect
> on the ata port device used to connect the ATA disk. However, for
> system suspend/resume operations, the ATA port used to connect the
> device will also be suspended and resumed, with the resum operation

resum -> resume

> requiring re-validating the device link and the device itseld. In this

itseld -> itself

> -static int sd_suspend_common(struct device *dev, bool ignore_stop_errors)
> +static inline bool sd_do_start_stop(struct scsi_device *sdev, bool runtime)
> +{
> +	return (sdev->manage_system_start_stop && !runtime) ||
> +		(sdev->manage_runtime_start_stop && runtime);
> +}

This function wouldn't be necessary if the sd_suspend_common() callers 
would pass sdev->manage_system_start_stop / 
sdev->manage_runtime_start_stop as an additional argument to 
sd_suspend_common().

> -		if (ignore_stop_errors)
> +		if (!runtime)
>   			ret = 0;
>   	}

The old code was self-documenting. If the name of the "runtime" argument 
is retained, a comment above this if-statement that explains why stop 
errors are ignored during a system suspend would be welcome.

Otherwise this patch looks good to me.

Thanks,

Bart.