LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Alan Cox <alan@lxorguk.ukuu.org.uk>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>,
	Tim Ellis <tim@ngndg.com>,
	linux-kernel@vger.kernel.org, jeff@garzik.org
Subject: Re: [PATCH] libata: Add MMIO support to pata_sil680
Date: Sat, 16 Feb 2008 08:45:56 +1100	[thread overview]
Message-ID: <1203111956.22915.24.camel@pasglop> (raw)
In-Reply-To: <20080215155332.2b89c429@core>


On Fri, 2008-02-15 at 15:53 +0000, Alan Cox wrote:
> > > That's strange though. Somebody with knowledge of that HW (or specs) who
> > > can spot something ? Could it be an issue with timing ?
> > > 
> > > I don't have HW access to this machine. If somebody could send one to me
> > > I could do more investigation.
> > 
> > Ben, would an ssh access to such a machine and to a terminal server 
> > suffice?
> 
> It says clearly in the code where to start. See the FIXME notes in both
> libata-sff and libata-core about MMIO. Neither the DMA transfer start or
> the probe SRST sequence are correct with MMIO posting and this hasn't
> been fixed as I pointed out was needed when I originally NAKked the
> change.
> 
> Without those being fixed (especially SRST) on any device with heavy PCI
> posting of mmio your controller *wont work*.

The dbdma start is mostly harmless (things don't get posted for -that-
long), though I suppose it's worth fixing. Would reading back dmactl do
in that case or do you foresee any kind of side effect ? (Maybe only
doing it for MMIO ?)

As for SRST, I'm not totally confident how safe it is to read back
there while doing the reset sequence, so I'm tempted to really only
do it for MMIO and use altstat rather than ctl/stat (the later tends
to have side effects which we don't want here).

What do you think ?

The main problem from here is that I don't know whether we are using
MMIO or PIO from libata-core. Maybe I can add a host flag indicate
that such flushing is needed ?

In the meantime, Guennadi, can you check if that patch helps for you
(to see if that is indeed the problem):


diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 004dae4..1451a52 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -3461,10 +3461,13 @@ static int ata_bus_softreset(struct ata_port *ap, unsigned int devmask,
 
 	/* software reset.  causes dev0 to be selected */
 	iowrite8(ap->ctl, ioaddr->ctl_addr);
+	ioread16(ioaddr->nsect_addr);
 	udelay(20);	/* FIXME: flush */
 	iowrite8(ap->ctl | ATA_SRST, ioaddr->ctl_addr);
+	ioread16(ioaddr->nsect_addr);
 	udelay(20);	/* FIXME: flush */
 	iowrite8(ap->ctl, ioaddr->ctl_addr);
+	ioread16(ioaddr->nsect_addr);
 
 	/* wait a while before checking status */
 	ata_wait_after_reset(ap, deadline);
diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 60cd4b1..81d5828 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -273,6 +273,7 @@ void ata_bmdma_start(struct ata_queued_cmd *qc)
 	 * FIXME: The posting of this write means I/O starts are
 	 * unneccessarily delayed for MMIO
 	 */
+	ioread8(ap->ioaddr.bmdma_addr + ATA_DMA_CMD);
 }
 
 /**

Cheers,
Ben.



  reply	other threads:[~2008-02-15 21:46 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-02-12 15:58 Tim Ellis
2008-02-12 21:02 ` Benjamin Herrenschmidt
2008-02-12 21:42   ` Alan Cox
2008-02-15 15:52   ` Guennadi Liakhovetski
2008-02-15 15:53     ` Alan Cox
2008-02-15 21:45       ` Benjamin Herrenschmidt [this message]
2008-02-15 22:27         ` Alan Cox
2008-02-15 22:55           ` Benjamin Herrenschmidt
2008-02-15 23:56         ` Tim Ellis
2008-02-25 22:57           ` Jeff Garzik
2008-02-25 23:06             ` Guennadi Liakhovetski
2008-02-26  0:58             ` Benjamin Herrenschmidt
2008-03-25 23:31               ` [PATCH] Work around breakage introduced in pata_sil680 by switching it to MMIO Guennadi Liakhovetski
2008-03-25 23:36                 ` Alan Cox
2008-03-26  8:20                   ` Benjamin Herrenschmidt
2008-02-15 21:36     ` [PATCH] libata: Add MMIO support to pata_sil680 Benjamin Herrenschmidt

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=1203111956.22915.24.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=g.liakhovetski@gmx.de \
    --cc=jeff@garzik.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tim@ngndg.com \
    --subject='Re: [PATCH] libata: Add MMIO support to pata_sil680' \
    /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

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