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