From: Al Viro <viro@ZenIV•linux.org.uk>
To: Jie Deng <Jie.Deng1@synopsys•com>
Cc: netdev@vger•kernel.org
Subject: [RFC][PATCH] apparent big-endian bugs in dwc-xlgmac
Date: Sun, 10 Dec 2017 04:53:36 +0000 [thread overview]
Message-ID: <20171210045326.GO21978@ZenIV.linux.org.uk> (raw)
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;
next reply other threads:[~2017-12-10 4:54 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-10 4:53 Al Viro [this message]
2017-12-11 4:33 ` [RFC][PATCH] apparent big-endian bugs in dwc-xlgmac Jie Deng
2017-12-11 5:05 ` Al Viro
2017-12-11 5:38 ` Al Viro
2017-12-11 6:46 ` Jie Deng
2017-12-11 15:54 ` [RFC][PATCH] new byteorder primitives - ..._{replace,get}_bits() Al Viro
2017-12-12 4:02 ` Jakub Kicinski
2017-12-12 6:20 ` Al Viro
2017-12-12 19:45 ` Al Viro
2017-12-12 20:04 ` Jakub Kicinski
2017-12-12 23:48 ` Al Viro
2017-12-12 23:59 ` Jakub Kicinski
2017-12-13 0:36 ` Al Viro
2017-12-13 1:04 ` Jakub Kicinski
2017-12-13 1:30 ` Al Viro
2017-12-13 1:35 ` Jakub Kicinski
2017-12-13 1:51 ` Al Viro
2017-12-13 2:44 ` Jakub Kicinski
2017-12-13 14:22 ` Al Viro
2017-12-13 17:45 ` Al Viro
2017-12-15 2:33 ` [RFC][PATCH] Add primitives for manipulating bitfields both in host- and fixed-endian Al Viro
2017-12-15 5:07 ` Jakub Kicinski
2017-12-15 5:34 ` Al Viro
2017-12-15 16:48 ` [RFC][PATCH v2] " Al Viro
2017-12-13 19:04 ` [RFC][PATCH] new byteorder primitives - ..._{replace,get}_bits() Jakub Kicinski
2017-12-11 6:18 ` [RFC][PATCH] apparent big-endian bugs in dwc-xlgmac Jie Deng
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171210045326.GO21978@ZenIV.linux.org.uk \
--to=viro@zeniv$(echo .)linux.org.uk \
--cc=Jie.Deng1@synopsys$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox