Re: [PATCH 01/12] rtwlan: main files

[Stanislaw Gruszka <sgruszka@redhat.com>]

On Thu, Sep 27, 2018 at 10:40:44AM -0500, Larry Finger wrote:
> On 9/27/18 8:50 AM, Stanislaw Gruszka wrote:
> --snip
> 
> > 
> > > +#define BIT_LEN_MASK_32(__bitlen) (0xFFFFFFFF >> (32 - (__bitlen)))
> > > +#define BIT_OFFSET_LEN_MASK_32(__bitoffset, __bitlen)                          \
> > > +	(BIT_LEN_MASK_32(__bitlen) << (__bitoffset))
> > > +#define LE_P4BYTE_TO_HOST_4BYTE(__start) (le32_to_cpu(*((__le32 *)(__start))))
> > > +#define LE_BITS_CLEARED_TO_4BYTE(__start, __bitoffset, __bitlen)               \
> > > +	(LE_P4BYTE_TO_HOST_4BYTE(__start) &                                    \
> > > +	 (~BIT_OFFSET_LEN_MASK_32(__bitoffset, __bitlen)))
> > > +#define LE_BITS_TO_4BYTE(__start, __bitoffset, __bitlen)                       \
> > > +	((LE_P4BYTE_TO_HOST_4BYTE(__start) >> (__bitoffset)) &                 \
> > > +	 BIT_LEN_MASK_32(__bitlen))
> > > +#define SET_BITS_TO_LE_4BYTE(__start, __bitoffset, __bitlen, __value)          \
> > > +	do {                                                                   \
> > > +		*((__le32 *)(__start)) = \
> > > +		cpu_to_le32( \
> > > +		LE_BITS_CLEARED_TO_4BYTE(__start, __bitoffset, __bitlen) |     \
> > > +		((((u32)__value) & BIT_LEN_MASK_32(__bitlen)) << (__bitoffset))\
> > > +		);                                                             \
> > > +	} while (0)
> 
> Stanislaw,
> 
> I have never loved these macros, and it took a lot of time to get them to be
> endian correct. Could you point me to a method that would overwrite a
> portion of a 32-bit little-endian word that would be correct for both
> little- and big-endian machines? Keep in mind that Kalle hates the use of
> compile tests on __LITTLE_ENDIAN.

Maybe something like this (not tested)

#define SET_LE32(reg, off, len, val) \
	((reg & cpu_to_le32(~GENMASK(off + len - 1, off))) | cpu_to_le32(val << off))

?

There are plenty of bitops and endian primitives in kernel, it's
very very unlikly you need custom macros for handle that.

Thanks
Stanislaw