On Thu, Oct 20, 2005 at 12:01:01PM -0700, Jesse Brandeburg wrote:
> > @@ -584,6 +584,7 @@ static inline void e100_write_flush(stru
> > {
> > /* Flush previous PCI writes through intermediate bridges
> > * by doing a benign read */
> > + wmb();
> > (void)readb(&nic->csr->scb.status);
> > }
>
> I find it odd that this is needed, the readb is meant to flush all
> posted writes on the pci bus, if your bus is conforming to pci
> specifications, this must succeed. wmb is for host side (processor
> memory) writes to complete, and since we're usually only try to force
> a writeX command to execute immediately with the readb (otherwise lazy
> writes work okay) we shouldn't need a wmb *here*. not to say it might
> not be missing somewhere else.
wmb is defined as a sync instruction which will only complete once the
write has actually left the CPU, that is citing the spec "has become
globally visible". Uncached stores such as writeX() may be held in a
writeback buffers potencially infinitely, until this buffer is needed
by another write operation. The real surprise is to see such behaviour
in a modern piece of silicon; the only that I knew of were the R3000-class
processors and that era has ended over a decade ago, so ATI seems to have
done something funny here.
Ralf
