[RFC][PATCH] apparent big-endian bugs in dwc-xlgmac

[Al Viro <viro@ZenIV.linux.org.uk>]

In xlgmac_dev_xmit():

                        /* Mark it as a CONTEXT descriptor */
                        dma_desc->desc3 = XLGMAC_SET_REG_BITS_LE(
                                                dma_desc->desc3,
                                                TX_CONTEXT_DESC3_CTXT_POS,
                                                TX_CONTEXT_DESC3_CTXT_LEN,
                                                1);



Looking at XLGMAC_SET_REG_BITS_LE() we see this:
#define XLGMAC_SET_REG_BITS_LE(var, pos, len, val) ({                   \
        typeof(var) _var = (var);                                       \
        typeof(pos) _pos = (pos);                                       \
        typeof(len) _len = (len);                                       \
        typeof(val) _val = (val);                                       \
        _val = (_val << _pos) & GENMASK(_pos + _len - 1, _pos);         \
        _var = (_var & ~GENMASK(_pos + _len - 1, _pos)) | _val;         \
        cpu_to_le32(_var);                                              \
})

That thing assumes var to be host-endian and has a little-endian result.
Unfortunately, we feed it a little-endian and store the result back into
the same place.  That might work if the original values *was* host-endian
and we wanted to end up with little-endian.  However, that is immediately
followed by
                        /* Indicate this descriptor contains the MSS */
                        dma_desc->desc3 = XLGMAC_SET_REG_BITS_LE(
                                                dma_desc->desc3,
                                                TX_CONTEXT_DESC3_TCMSSV_POS,
                                                TX_CONTEXT_DESC3_TCMSSV_LEN,
                                                1);

where we operate on the now definitely little-endian value.  That really
can't be right.  I don't have the hardware in question, so I can't test
that, but it smells like this needs something like diff below, making
XLGMAC_SET_REG_BITS_LE take le32 and return le32.  GET side of things
is le32 -> u32; definition looks correct, but slightly misannotated.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac.h b/drivers/net/ethernet/synopsys/dwc-xlgmac.h
index cab3e40a86b9..e95c4c250e16 100644
--- a/drivers/net/ethernet/synopsys/dwc-xlgmac.h
+++ b/drivers/net/ethernet/synopsys/dwc-xlgmac.h
@@ -106,7 +106,7 @@
 #define XLGMAC_GET_REG_BITS_LE(var, pos, len) ({			\
 	typeof(pos) _pos = (pos);					\
 	typeof(len) _len = (len);					\
-	typeof(var) _var = le32_to_cpu((var));				\
+	u32 _var = le32_to_cpu((var));				\
 	((_var) & GENMASK(_pos + _len - 1, _pos)) >> (_pos);		\
 })
 
@@ -125,8 +125,8 @@
 	typeof(len) _len = (len);					\
 	typeof(val) _val = (val);					\
 	_val = (_val << _pos) & GENMASK(_pos + _len - 1, _pos);		\
-	_var = (_var & ~GENMASK(_pos + _len - 1, _pos)) | _val;		\
-	cpu_to_le32(_var);						\
+	(_var & ~cpu_to_le32(GENMASK(_pos + _len - 1, _pos))) | 	\
+		cpu_to_le32(_val);					\
 })
 
 struct xlgmac_pdata;