On Thu, 29 May 2008, Alan Cox wrote: > > (Jeff would you please take a look at this: Its #4 or #5 top OOPS on Arjan's > oops tracker, and it generally causes the boot to fail. First sent 20th May) Quite frankly, if I was Jeff, I'd have refused to apply this patch as "too damn ugly to live". Why the *hell* doesn't it just fix "ata_sff_altstatus()" instead? Why does it introduce a ludicrously named stupid "maybe" version of it that doesn't oops? In other words: in *any* case where the old "ata_sff_altstatus()" function worked correctly, the new "ata_sff_maybe_altstatus()" function does THE EXACT SAME THING. And in any case where the old "ata_sff_altstatus()" function oopsed, the new "maybe" version at least is _better_. In other words: there is absolutely no excuse for keeping the old (and known-to-be-broken) "ata_sff_altstatus()" function at all. It should be removed, not left around with an alternate function that works. I also think your "ata_sff_sync()" thing is buggy. It has a "ndelay(400)" that is almost certainly buggy (it's the one that is already in ata_sff_pause()). It may be that you meant to make it an "else if" case, ie if there was no IO-read, then you do a ndelay(400) as a last desperate case, but that's not how your ata_sdd_sync() is actually written. Linus -- 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
