Re: [PATCH #upstream 2/2] libata: implement spurious irq handling for SFF and apply it to piix

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 01/18/2010 08:49 PM, Tejun Heo wrote:
Traditional IDE interface sucks in that it doesn't have a reliable IRQ
pending bit, so if the controller raises IRQ while the driver is
expecting it not to, the IRQ won't be cleared and eventually the IRQ
line will be killed by interrupt subsystem.  Some controllers have
non-standard mechanism to indicate IRQ pending so that this condition
can be detected and worked around.

This patch adds an optional operation ->sff_irq_check() which will be
called for each port from the ata_sff_interrupt() if an unexpected
interrupt is received.  If the operation returns %true,
->sff_check_status() and ->sff_irq_clear() will be cleared for the
port.  Note that this doesn't mark the interrupt as handled so it
won't prevent IRQ subsystem from killing the IRQ if this mechanism
fails to clear the spurious IRQ.

This patch also implements ->sff_irq_check() for ata_piix.  Note that
this adds slight overhead to shared IRQ operation as IRQs which are
destined for other controllers will trigger extra register accesses to
check whether IDE interrupt is pending but this solves rare screaming
IRQ cases and for some curious reason also helps weird BIOS related
glitch on Samsung n130 as reported in bko#14314.

   http://bugzilla.kernel.org/show_bug.cgi?id=14314

* piix_base_ops dropped as suggested by Sergei.

* Spurious IRQ detection doesn't kick in anymore if polling qc is in
   progress.  This provides less protection but some controllers have
   possible data corruption issues if the wrong register is accessed
   while a command is in progress.

Signed-off-by: Tejun Heo<tj@xxxxxxxxxx>
Reported-by: Johannes Stezenbach<js@xxxxxxxxx>
Reported-by: Hans Werner<hwerner4@xxxxxx>
Cc: Alan Cox<alan@xxxxxxxxxxxxxxxxxxx>
Cc: Sergei Shtylyov<sshtylyov@xxxxxxxxxxxxx>
---
Jeff, these two are for #upstream but generated on top of
#upstream-fixes because the piix sata 32bit bmdma patch is currently
only in #upstream-fixes.  Please pull -fixes into #upstream before
applying these two.  Thanks.

  drivers/ata/ata_piix.c   |   20 +++++++++++++++-----
  drivers/ata/libata-sff.c |   35 ++++++++++++++++++++++++++++++++---
  include/linux/libata.h   |    1 +
  3 files changed, 48 insertions(+), 8 deletions(-)

applied, with comment:

Overall, as long as the drive is in Bus-Idle mode, it should be safe to go ahead and read Status, for pretty much every controller and drive.

I would make exception only for the new SATA FIS-based controllers, where we know that hitting Status is likely both pointless and wasteful, as well as being superfluous because the newer FIS-based controllers all have irq status registers.

Additionally, I think we should have a "fast-timeout" and "slow-timeout", whereby we check Status after a short period (5 seconds?) to make sure we did not lose an interrupt. If Status is !BSY, then we can proceed with handling qc success/failure immediately.

	Jeff



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

[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux