lkml.org 
[lkml]   [2008]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: RESEND: [PATCH] libata-sff: Fix oops reported in kerneloops.org for pnp devices with no ctl
> 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?

Ok the problem is we have three cases to distinguish

- altstatus must be used - in which case we want to blow up anyway if we
touch it (the usual case)
- altstatus should be used if available - shared IRQ check
- altstatus being used for flushing - altstatus irrelevant, it just has
to flush somehow.

So the _maybe naming sucks, but the reasoning I think is sound. The other
way to do it would be to replace it and the bit of irq handler logic with
an ata_sff_busy() that did the status checks correctly for both ctl and
non-ctl capable devices.

> 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.

The ndelay(400) is correct. The IO-read is Jeff being paranoid and
actually hurts us materially for the usual PIO case (bus PIO not disk
PIO) to the tune of about 1mS a command in many cases, but is needed for
MMIO (which we almost never do for any SFF hardware). That itself is a
different problem that can be fixed later (and not in -rc5). It wants
fixing as its a key reason that old IDE is still faster for PATA.
maybe_altstatus is crap naming but simply making ata_sff_altstatus fake a
reply in arbitary cases risks not catching mistakes and could mean we
don't catch corrupting mistakes which would be very bad indeed.



\
 
 \ /
  Last update: 2008-05-29 20:21    [W:0.103 / U:0.400 seconds]
©2003-2011 Jasper Spaans