Search Linux Wireless

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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Stanislaw Gruszka <sgruszka@xxxxxxxxxx> writes:

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

This is horrible.

>> 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.

Indeed, try avoiding reinventing wheel as much as possible.

-- 
Kalle Valo



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux