[U-Boot] [PATCH v3 02/10] armv7: add miscellaneous utility macros

[Wolfgang Denk]

Dear Aneesh V,

In message <1305202276-27784-3-git-send-email-aneesh at ti.com> you wrote:
> add utility macros for:
>  * bit field operations
>  * log2n functions
...

> +/* extract a bit field from a bit vector */
> +#define get_bit_field(nr, start, mask)\
> +	(((nr) & (mask)) >> (start))
> +
> +/* Set a field in a bit vector */
> +#define set_bit_field(nr, start, mask, val)\
> +	do { \
> +		(nr) = ((nr) & ~(mask)) | (((val) << (start)) & (mask));\
> +	} while (0);

I really dislike such "useful" helpers, because they make the code
unreadable.  Being nonstandrd, nbody knows what they are doing, so you
always will have to look up the implementation before you can continue
reading the code.  It's a waste of time an resources.

> +/*
> + * Utility macro for read-modify-write of a hardware register
> + *	addr - address of the register
> + *	shift - starting bit position of the field to be modified
> + *	msk - mask for the field
> + *	val - value to be shifted masked and written to the field
> + */
> +#define modify_reg_32(addr, shift, msk, val) \
> +	do {\
> +		writel(((readl(addr) & ~(msk))|(((val) << (shift)) & (msk))),\
> +		       (addr));\
> +	} while (0);

NAK again, for the same reasons.

Note that there are some semi-standardized I/O accessro macros
available, at least for some architectures, like clrbits.*(),
setbits_(), or clrsetbits().

See for example "arch/arm/include/asm/io.h",
"arch/powerpc/include/asm/io.h" for reference.

Instead of reinventing the wheel (just differently shaped) we should
rather try and use a single, standardized set of such helpers.

So please use the existing macros instead of inventing new,
non-standard ones.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
No journaling file system can recover your data if the disk dies.
                                 - Steve Rago in <D4Cw1p.L9E at plc.com>