Re: [RFC][PATCH] apparent big-endian bugs in dwc-xlgmac
flat view
From: Al Viro <viro@ZenIV.linux.org.uk>
Date: 2017-12-11 05:05:22
On Mon, Dec 11, 2017 at 12:33:42PM +0800, Jie Deng wrote:
quoted hunk ↗ jump to hunk
Hi AI Viro,quoted
@@ -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;Make sense. But I think what you want is fix as follows. Right ?@@ -125,8 +125,8 @@ typeof(len) _len = (len); \ - typeof(val) _val = (val); \ + u32 _var = le32_to_cpu((var)); \ _val = (_val << _pos) & GENMASK(_pos + _len - 1, _pos); \ - _var = (_var & ~GENMASK(_pos + _len - 1, _pos)) | _val; \ - cpu_to_le32(_var); \ + (cpu_to_le32(_var) & ~cpu_to_le32(GENMASK(_pos + _len - 1, _pos))) | \ + cpu_to_le32(_val); \ })
What for? Sure, this variant will work, but why bother with a = le32_to_cpu(b); (cpu_to_le32(a) & ....) | .... and how is that better than (b & ...) | ... IDGI... Mind you, I'm not sure if there is any point keeping _var in that thing, seeing that we use var only once - might be better off with ((var) & ~cpu_to_le32(GENMASK(_pos + _len - 1, _pos))) | \ cpu_to_le32(_val); \