On Mon, 9 May 2011, Jan Andersson wrote:
> > Why did all this stuff get moved up in the header file?
> >
>
> I moved struct uhci_hcd up as the cpu_to_hc32() and hc32_to_cpu()
> functions depend on it, particularly via uhci_big_endian_desc(uhci) that
> expands to ((uhci)->big_endian_desc). enum uhci_rh_state is used in
> struct uhci_hcd so I moved that part too. cpu_to_hc32() and
> hc32_to_cpu() are used in several of the defines that were previously
> located before struct uhci_hcd ..
Things appearing in a #define don't have to come before the #define
itself. Things appearing in an inline function _do_ have to come
before the function -- but as far as I can see there are only two
inlines here: qh_element() and td_status(). Both of those should be
rewritten anyway; I've been meaning to do it for some time.
You can build your patch on top of the one below, and they can all be
submitted together. Then things won't need to be moved around so much.
Alan Stern
-------------------------------------------------------------------------
This patch (as1462) updates the special accessor functions defined in
uhci-hcd.h. Rather than using a full compiler barrier, all we really
need is the ACCESS_ONCE() mechanism, because the idea is to prevent
the compiler from caching a value that can change at any time.
Signed-off-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
---
drivers/usb/host/uhci-hcd.h | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
Index: usb-2.6/drivers/usb/host/uhci-hcd.h
===================================================================
--- usb-2.6.orig/drivers/usb/host/uhci-hcd.h
+++ usb-2.6/drivers/usb/host/uhci-hcd.h
@@ -168,12 +168,7 @@ struct uhci_qh {
* We need a special accessor for the element pointer because it is
* subject to asynchronous updates by the controller.
*/
-static inline __le32 qh_element(struct uhci_qh *qh) {
- __le32 element = qh->element;
-
- barrier();
- return element;
-}
+#define qh_element(qh) ACCESS_ONCE((qh)->element)
#define LINK_TO_QH(qh) (UHCI_PTR_QH | cpu_to_le32((qh)->dma_handle))
@@ -263,12 +258,7 @@ struct uhci_td {
* We need a special accessor for the control/status word because it is
* subject to asynchronous updates by the controller.
*/
-static inline u32 td_status(struct uhci_td *td) {
- __le32 status = td->status;
-
- barrier();
- return le32_to_cpu(status);
-}
+#define td_status(td) le32_to_cpu(ACCESS_ONCE((td)->status))
#define LINK_TO_TD(td) (cpu_to_le32((td)->dma_handle))
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
