adaspindown: check disk power mode before sending IDLE command

[avg]

Summary

If a disk is already in STANDBY mode, then setting IDLE mode can
actually spin it up.

[mav]

I generally like it, except one comment inline.

It makes me think whether it makes sense to send standby command if now we know that the state is already standby, and idle if idle. But it is probably not a big deal.

sys/cam/ata/ata_da.c

3657

I am not sure how useful would it be after initial testing. I'd at very least move it lower into the default case.

[avg]

In D33588#758809, @mav wrote:

It makes me think whether it makes sense to send standby command if now we know that the state is already standby, and idle if idle. But it is probably not a big deal.

In my tests, at least, STANDBY_IMMEDIATE did not spin up a disk that was already spun down.
At the same time, we can be a little bit faster if we don't send the redundant command.
I'll make that change.

sys/cam/ata/ata_da.c

3657

Okay, will do.

[mav]

In D33588#758812, @avg wrote:

In my tests, at least, STANDBY_IMMEDIATE did not spin up a disk that was already spun down.
At the same time, we can be a little bit faster if we don't send the redundant command.

I am thinking about two sides: 1) whether disk can be in idle/standby with caches not flushed, and 2) there are some obsolete/reserved/unknown states, in which commands still would better be sent to be sure.

[imp]

sys/cam/ata/ata_da.c

3661

Are there defines for these magic numbers?

[avg]

sys/cam/ata/ata_da.c

3661

I could not find any. camcontrol also uses literals. smartmontools has them that way as well.

[avg]

hide reporting of the current mode under DIAGNOSTIC option

[avg]

So, I decided to not do any optimizations (with respect to sending potentially redundant commands) just to stay on the safer side.
I also kept the reporting of the current mode in case any regressions would need to be debugged.
It's doubly hidden now, under DIAGNOSTIC and bootverbose.

[avg]

@imp , I actually found some relevant definitions but they only cover EPC / Extended Power Conditions.
ATA_CHECK_POWER_MODE can report both normal power modes and EPC modes depending on whether EPC is present and enabled.
I am not sure how to extend and unify those modes:

• have a single set of modes under EPC
• handle a single set of modes under (new) power modes
• add (new) power modes and duplicate common modes with EPC

At the moment I am leaning towards the last option.
Let me create a review request for it.

[avg]

D33646 for a cross-reference.