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

From: Al Viro <[email-masked]>
Date: Mon, 11 Dec 2017 05:05:20 +0000

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