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); \
