Re: Patch: ATI Xilleon port 2/11 net/e100 Memory barriers and write flushing

[Jesse Brandeburg]

On 10/19/05, David Daney <ddaney@xxxxxxxxxx> 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.


> @@ -807,9 +808,13 @@ static inline int e100_exec_cmd(struct n
>                 goto err_unlock;
>         }
>
> -       if(unlikely(cmd != cuc_resume))
> +       wmb();
> +       if(unlikely(cmd != cuc_resume)) {
>                 writel(dma_addr, &nic->csr->scb.gen_ptr);
> +               e100_write_flush(nic);
> +       }
>         writeb(cmd, &nic->csr->scb.cmd_lo);
> +       e100_write_flush(nic);

wouldn't the last e100_write_flush be all that is needed?  e100 only
needs them to come in order, they don't need to be flushed one at a
time.

I can see how this change might be needed in a true write posting environment.

jesse
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/