The idea was that we only need to enqueue an unpark request if there is
no other request on the queue. The check is rather silly though because
other requests may be enqueued later anyway. In libata, we don't do a
similar check, so I dropped it here too. The assumption is, of course,
that a check power command is cheap, otherwise we should think again
whether we really should use it unconditionally.
For the sake of consistency, I've always tried to make ide and libata
behave alike (or as close to it as possible). However, the final version
of the libata patch is very hard to mimc in ide. Therefore, I wonder
whether we can do in ide what we'd really like to do in libata
eventually. The patch below is a real per-device implementation of the
unload feature. However, I'd like you to confirm the crucial assumption
underlying this patch: a port reset is the only way a device can
interfere with another device on the same port. In particular, I haven't
made an effort to understand pnp and similar stuff completely, but from
a first glance I got the impression that these things are done per-port
rather than per-device and that nothing sinister will happen behind our
back. In short, can you confirm the following:
Condition: device A on a port is parked (implies there is at least one
request on the queue of that device, i.e we hold a
reference to the device and thus to the port).
Assumption: nothing will disturb the device because resets due to
command failure / timeouts on device B are deferred (see my
patch) and spurious commands like IDENTIFY (or whatever
actions may be related to pnp and the like) are not
performed while the device is sleeping and a request is
Yes, of course, thanks for the hint.
From: Elias Oltmanns <firstname.lastname@example.org>
Subject: [PATCH] ide: Implement disk shock protection support
On user request (through sysfs), the IDLE IMMEDIATE command with ...
Sorry for spamming you again, but I forgot to mention one more thing: In
my hardware environment, I cannot easily test the code in do_reset1()
which is supposed to defer resets if necessary. Since we have something
very similar in libata (which I have tested), I'm quite confident that
everything will work out nicely. Still, you may want to pay special
attention to this piece of code.
This wake_up_all() has to go outside the if clause. I'll change this the
next time round to be called right after drive->sleep has been set. The
two nested if clauses will be unified and the conditions &&'ed.
I've just audited your latest patch and it all seems good (the assumptions
taken are valid and all concerns were addressed) so you may add my ACK to
the next-time-round patch. Big thanks for patiently improving this patch,
the final version looks so much better than the initial/draft one. :)