| Message ID | 20240104223940.339290-2-phill@thesusis.net |
|---|---|
| State | New |
| Headers | show |
| Series | [1/4] libata: only wake a drive once on system resume | expand |
On 1/5/24 07:39, Phillip Susi wrote: > When suspending the system, libata puts the drive in standby mode to > prepare it to lose power. If the drive is in SLEEP mode, this spins it up > only to spin it back down again. Don't do that. > --- > drivers/ata/libata-core.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index a2d8cc0097a8..1244da8f77e2 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -2030,6 +2030,10 @@ void ata_dev_power_set_standby(struct ata_device *dev) > system_entering_hibernation()) > return; > > + /* no need to standby if it is alreqady sleeping */ s/alreqady/already The comment should also be improved. It is more than a "no need" given that a sleeping disk will not respond to any command... So something like: /* * If the devices is in SLEEP state, issuing a STANDBY IMMEDIATE * command will fail. But given that the drive is already in a low * power state, we do not need to do anything. */ > + if (dev->flags & ATA_DFLAG_SLEEPING) > + return; > + > /* Issue STANDBY IMMEDIATE command only if supported by the device */ > if (!ata_dev_power_init_tf(dev, &tf, false)) > return; Other than the above comments, this looks OK. And this probably should go first in the series with a fixes tag.
Damien Le Moal <dlemoal@kernel.org> writes: > The comment should also be improved. It is more than a "no need" given that a > sleeping disk will not respond to any command... So something like: Good point. > /* > * If the devices is in SLEEP state, issuing a STANDBY IMMEDIATE > * command will fail. But given that the drive is already in a low > * power state, we do not need to do anything. > */ It didn't fail, it just caused the drive to spin up, only to spin right back down again. > Other than the above comments, this looks OK. And this probably should go first > in the series with a fixes tag. I'm not sure what I'd point a fixes tag at. I think it's been this way forever. Well, at least as long as SLEEP support has been in, which is basically forever.
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index a2d8cc0097a8..1244da8f77e2 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -2030,6 +2030,10 @@ void ata_dev_power_set_standby(struct ata_device *dev) system_entering_hibernation()) return; + /* no need to standby if it is alreqady sleeping */ + if (dev->flags & ATA_DFLAG_SLEEPING) + return; + /* Issue STANDBY IMMEDIATE command only if supported by the device */ if (!ata_dev_power_init_tf(dev, &tf, false)) return;