stern at rowland
Dec 16, 2005, 7:24 PM
Post #2 of 3
(402 views)
Permalink
|
|
Re: [PATCH] UHCI: add missing memory barriers
[In reply to]
|
|
On Sat, 17 Dec 2005, Benjamin Herrenschmidt wrote:
> > This patch (as617) adds a couple of memory barriers that Ben H. forgot in
> > his recent suspend/resume fix.
>
> I didn't think they were necessary but they certainly won't hurt and
> it's not a hot code path...
True.
> > pci_write_config_word(to_pci_dev(uhci_dev(uhci)), USBLEGSUP, 0);
> > + mb();
>
> Isn't pci config space access always fully synchronous ?
If it is, it's not documented.
Looking at the PCI code, I see that the accesses are protected by a
spinlock. Does that guarantee in-order execution of writes to
configuration space with respect to writes to regular memory? On all
platforms? If yes, then this barrier is not needed.
> > @@ -738,6 +739,7 @@ static int uhci_resume(struct usb_hcd *h
> > * really don't want to keep a stale HCD_FLAG_HW_ACCESSIBLE=0
> > */
> > set_bit(HCD_FLAG_HW_ACCESSIBLE, &hcd->flags);
> > + mb();
>
> I don't think that one matters much but it won't hurt for sure.
Actually this one only needs to be smp_mb(), although the reasoning is a
bit subtle. Anyway, as you said, leaving the barriers in certainly won't
hurt anything.
Alan Stern
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
|
