Stanislaw Gruszka <[email protected]> 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

Reply via email to