Tejun Heo wrote:
Mark Lord wrote:
Tejun Heo wrote:
Mark Lord wrote:
..
+ /* FIXME:
+ * Except for the outer do-while construct below, this function
+ * is an exact clone of sata_std_hardreset() from libata-core.c.
+ *
+ * Once this driver is stable, we should re-org libata so we
can share
+ * more of that code, rather than duplicating so much of it here
+ * and in other drivers.
+ */
After modularize patchsets, sata_link_hardreset() does all the chores
needed around hardreset and sata_mv should be able to just build a
loop around it.
..
Mmm... I don't see how this helps.
The bulk of mv_sata_hardreset() is from sata_std_hardreset().
The only part those two do *not* have in common, is that
sata_mv needs to do it's own equivalent of sata_link_hardreset(),
so sata_link_hardreset() cannot be reused here. Wrapper or not.
Now, if we had a per-LLD .link_hardreset op, defaulting to
sata_link_hardreset,
then this would be trivial.
The MV specific part is retry-if-offline w/ lower link speed, right? You
can do that just as well by looping outside of sata_link_hardreset().
..
Yes, the code already has a loop "outside of sata_link_hardreset()"
for the speed errata handling. So nothing new there.
And the rest of that routine is a line-by-line clone of ata_std_hardreset().
This is smaller than what other drivers have cloned for these routines,
and a lot better than the old code from sata_mv that it replaces.
The comment in my patch above is just a reminder that someday we could
go back in and address those things. In *all* LLDs, not just sata_mv.
I guess I'd better stop adding such comments in the future.. :)
Still think it needs any changes ?
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
