All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jeff@garzik.org>
To: Mikael Pettersson <mikpe@it.uu.se>
Cc: htejun@gmail.com, linux-ide@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2.6.19 2/3] sata_promise: new EH conversion
Date: Wed, 06 Dec 2006 04:13:41 -0500	[thread overview]
Message-ID: <457689C5.5030201@garzik.org> (raw)
In-Reply-To: <200612060853.kB68r0Gg024641@harpo.it.uu.se>

Mikael Pettersson wrote:
> On Sun, 03 Dec 2006 22:00:42 +0900, Tejun Heo wrote:
>> Mikael Pettersson wrote:
>>> +}
>>> +
>>> +static void pdc_error_handler(struct ata_port *ap)
>>> +{
>>> +	struct ata_eh_context *ehc = &ap->eh_context;
>>> +	ata_reset_fn_t hardreset;
>>> +
>>> +	/* stop DMA, mask IRQ, don't clobber anything else */
>>> +	ata_eh_freeze_port(ap);
>> Don't freeze port unconditionally.  You'll end up hardresetting on every
>> error.  Just make sure DMA engine is stopped and the controller is in a
>> sane state.  If that fails, then, the port should be frozen.
> 
> I'm looking into this now, but so far it seems only a reset
> (what Promise calls software reset, I don't know if libata
> considers it a soft or hard reset) of the ATA channel will do.
> 
>>> +	hardreset = NULL;
>>> +	if (sata_scr_valid(ap)) {
>>> +		ehc->i.action |= ATA_EH_HARDRESET;
>> Why always force HARDRESET?
> 
> I based that on sata_sil24:
> 
> 	if (sil24_init_port(ap)) {
> 		ata_eh_freeze_port(ap);
> 		ehc->i.action |= ATA_EH_HARDRESET;
> 	}
> 
> I interpreted the ATA_EH_HARDRESET as being required due to
> the ata_eh_freeze_port(), but perhaps it's only there because
> sil24_init_port() returned failure?
> 
> A different issue, but of practical importance, is which
> libata branch I should base the EH conversion on: #upstream
> or #ALL? Andrew Morton's -mm kernels include the ALL patches,
> but they in turn include the promise-sata-pata patches, and
> there is a conflict between the PATA patch and the EH conversion.
> Currently my EH conversion is based on #upstream, and I've ported
> the PATA patch to apply on top of it.

It's a tiered system ;-)

* if at all possible, provide patches against the latest linux-2.6.git

* if there are dependencies in #upstream-fixes or #upstream (i.e. I 
already applied some of your patches), provide patches against 
#upstream-fixes or #upstream

#ALL is a branch that is blown away at will, and is really more of a 
testing and akpm sync point.  Don't worry about conflicts with 
promise-sata-pata, I take care of those when I merge the #ALL branch 
together.

	Jeff




  reply	other threads:[~2006-12-06  9:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-12-06  8:53 [PATCH 2.6.19 2/3] sata_promise: new EH conversion Mikael Pettersson
2006-12-06  9:13 ` Jeff Garzik [this message]
2006-12-06  9:19 ` Tejun Heo
  -- strict thread matches above, loose matches on Subject: below --
2006-12-06  8:52 Mikael Pettersson
2006-12-06  9:40 ` Tejun Heo
2006-12-01  9:58 Mikael Pettersson
2006-12-03 13:00 ` Tejun Heo
2006-12-03 13:03   ` Jeff Garzik
2006-12-03 13:19     ` Tejun Heo
2006-12-03 14:16       ` Arjan van de Ven
2006-12-06  9:38         ` Tejun Heo

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=457689C5.5030201@garzik.org \
    --to=jeff@garzik.org \
    --cc=htejun@gmail.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikpe@it.uu.se \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.