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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]


On 2023/09/21 14:45, Bart Van Assche wrote:
> 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().

Sure, but then we need to get the sdkp for the struct device and use
sdkp->device in both sd_suspend_runtime() and sd_suspend_system(). This is in my
opinion not as clean as the little inline helper. But if you insist, I can make
that change.

>> -		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.

There is a comment already (unchanged) above the call to sd_start_stop_device(),
one line above the "if".

> Otherwise this patch looks good to me.
> Thanks,
> Bart.

Damien Le Moal
Western Digital Research

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]

  Powered by Linux