linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Aloni <da-x@monatomic.org>
To: Mark Lord <lkml@rtr.ca>
Cc: IDE/ATA development list <linux-ide@vger.kernel.org>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	Jens Axboe <axboe@suse.de>, Jeff Garzik <jgarzik@pobox.com>,
	sander@humilis.net, dror@xiv.co.il
Subject: Re: [PATCH 2.6.16] sata_mv.c :: three bug fixes
Date: Wed, 29 Mar 2006 19:06:28 +0200	[thread overview]
Message-ID: <20060329170628.GA25640@localdomain> (raw)
In-Reply-To: <200603290950.32219.lkml@rtr.ca>

On Wed, Mar 29, 2006 at 09:50:31AM -0500, Mark Lord wrote:
> This patch is against 2.6.16, and fixes three serious bugs in sata_mv.c.
> The same fixes are applicable to 2.6.16-git*.
> 
> (1) A DMA transfer size of 0x10000 was not being written
> as 0x0000 in the PRDs.  Fixed.
> 
> (1) The DEV_IRQ interrupt cause bit happens spuriously
> during EDMA operation, and was not being ignored by the driver. 
> This led to various "drive busy" errors being reported,
> with associated unpredictable behaviour.  Fixed.

I saw that happening too, thanks for the fix.
 
> (2) If a SATA or PCI interrupt was received with no outstanding
> command, the interrupt handler still attempted to invoke
> ata_qc_complete(), triggering assert()/BUG_ON() behaviour
> elsewhere in libata.  Fixed.

Dito. Thanks again.

> The driver still has issues with confusion after error-recovery,
> but should now  be reliable in the absence of drive errors.
> I will be looking more into the error-handling bugs next.
> 
> Signed-Off-By: Mark Lord <mlord@pobox.com>
> ---
>[...]
>  	/* we'll need the HC success int register in most cases */
>  	hc_irq_cause = readl(hc_mmio + HC_IRQ_CAUSE_OFS);
>  	if (hc_irq_cause) {
>  		writelfl(~hc_irq_cause, hc_mmio + HC_IRQ_CAUSE_OFS);
>  	}
>  
>  	VPRINTK("ENTER, hc%u relevant=0x%08x HC IRQ cause=0x%08x\n",
>  		hc,relevant,hc_irq_cause);
>  
>  	for (port = port0; port < port0 + MV_PORTS_PER_HC; port++) {
> -		ap = host_set->ports[port];
> +		struct ata_port *ap = host_set->ports[port];
> +		struct mv_port_priv *pp = ap->private_data;

The original code checks ap for NULL, are you sure it is safe to
remove this?

-- 
Dan Aloni
da-x@monatomic.org, da-x@colinux.org, da-x@gmx.net, dan@xiv.co.il

  reply	other threads:[~2006-03-29 17:05 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-03-29 14:50 [PATCH 2.6.16] sata_mv.c :: three bug fixes Mark Lord
2006-03-29 17:06 ` Dan Aloni [this message]
2006-03-29 17:37   ` Mark Lord
2006-03-30  0:46 ` Jeff Garzik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060329170628.GA25640@localdomain \
    --to=da-x@monatomic.org \
    --cc=axboe@suse.de \
    --cc=dror@xiv.co.il \
    --cc=jgarzik@pobox.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkml@rtr.ca \
    --cc=sander@humilis.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).