On Mon, Dec 11, 2017 at 12:33:42PM +0800, Jie Deng wrote: > Hi AI Viro, > > @@ -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); \