[U-Boot] [RESEND PATCH v2 1/5] Tegra2: Add macros to calculate bitfield shifts and masks

[Simon Glass]

Hi Albert,

On Sat, Jul 9, 2011 at 6:56 AM, Albert ARIBAUD <albert.u.boot at aribaud.net>wrote:

> Hi Simon,
>
> Le 05/07/2011 18:49, Simon Glass a écrit :
>
>  Signed-off-by: Simon Glass<sjg at chromium.org>
>> ---
>> Changes in v2:
>> - Removed all bitfield access macros
>>
>
> Not sure I follow you: the added lines below do indeed add bitfield access
> macros, don't they?
>
> No these are just for defining the shifts and masks. There is no access
going on through macros, either IO or normal variables. Everything uses
manual shifts and adds, and the IO uses writel/readl/clrsetbits_le32().
Please check the follow-on patches to see what I mean.

>
>   arch/arm/include/asm/arch-**tegra2/bitfield.h |   96
>> +++++++++++++++++++++++++++
>>  1 files changed, 96 insertions(+), 0 deletions(-)
>>  create mode 100644 arch/arm/include/asm/arch-**tegra2/bitfield.h
>>
>> diff --git a/arch/arm/include/asm/arch-**tegra2/bitfield.h
>> b/arch/arm/include/asm/arch-**tegra2/bitfield.h
>> new file mode 100644
>> index 0000000..494087c
>> --- /dev/null
>> +++ b/arch/arm/include/asm/arch-**tegra2/bitfield.h
>> @@ -0,0 +1,96 @@
>> +/*
>> + * Copyright (c) 2011 The Chromium OS Authors.
>> + * See file CREDITS for list of people who contributed to this
>> + * project.
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation; either version 2 of
>> + * the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
>> + * MA 02111-1307 USA
>> + */
>> +
>> +#ifndef __TEGRA2_BITFIELD_H
>> +#define __TEGRA2_BITFIELD_H
>> +
>> +/*
>> + * Basic macros for easily getting mask and shift values for bit fields
>> on
>> + * ARM.
>> + *
>> + * You use these to reliably create shifts and masks from a bit field
>> + * definition. Bit fields are defined like this:
>> + *
>> + * #define NAME_BITS   MSB : LSB
>> + *
>> + * where MSB is the most significant bit, and LSB the least sig, bit.
>> This
>> + * notation is chosen since it is commonly used in CPU / SOC datasheets.
>> + *
>> + * For example:
>> + *
>> + * #define UART_FBCON_BITS  5:3                Bit range for the FBCON
>> field
>> + *
>> + * Note that if you are using a big-endian machine there is no consistent
>> + * notion of big numbers, since bit 3 is a different bit depending on
>> whether
>> + * the access is 32-bits or 64-bits. For this reason these macros should
>> not
>> + * be used as it would probably be too confusing to have to specify your
>> + * access width all the time.
>> + *
>> + * This defines a bit field extending between bits 3 and 5.
>> + *
>> + * Then in your header file you can set up the shift and mask like this:
>> + *
>> + *      #define UART_FBCON_SHIFT       bf_shift(UART_FBCON)
>> + *      #define UART_FBCON_MASK        bf_mask(UART_FBCON)
>> + *
>> + * Then you can use these macros in your code (there is no bitfield
>> support
>> + * in the C file and these macros MUST NOT be used directly in C code).
>> + *
>> + * To write, overwriting other fields too:
>> + *     writel(3<<  UART_FBCON_SHIFT,&uart->fbcon)**;
>> + *
>> + * To read:
>> + *     int fbcon = (readl(&uart->fbcon)&  UART_FBCON_MASK)>>
>> + *             UART_FBCON_SHIFT;
>> + *
>> + * To update just this field (for example):
>> + *     clrsetbits_le32(&uart->fbcon, UART_FBCON_MASK, 3<<
>>  UART_FBCON_SHIFT);
>> + */
>>
>
> What's the benefit of this over directly specifying a shift and mask value,
> e.g. #define UART_FBCON_SHIFT 3 and #define UART_FBCON_MASK 0x7 and doing
> shifts and ANDs? I don't see this as simpler or more intuitive.


The only benefit is to avoid having to calculate all of these masks and
shifts in your head. It is basically just a shortcut and assists with
checking code against datasheets. Of course I would prefer to have access
through macros also but that was shot down so the code is now full of manual
shifts and ANDs. I hope that at least this small convenience will be
acceptable.

Regards,
Simon


>
>
>  +#include "compiler.h"
>> +
>> +#if __BYTE_ORDER == __LITTLE_ENDIAN
>> +
>> +/* Returns the bit number of the LSB */
>> +#define _LSB(range)    ((0 ? range)&  0x1f)
>> +
>> +/* Returns the bit number of the MSB */
>> +#define _MSB(range)    (1 ? range)
>> +
>> +/* Returns the width of the bitfield (in bits) */
>> +#define _BITFIELD_WIDTH(range) (_MSB(range) - _LSB(range) + 1)
>> +
>> +
>> +/*
>> + * Returns the number of bits the bitfield needs to be shifted left to
>> pack it.
>> + * This is just the same as the low bit.
>> + */
>> +#define bf_shift(field)                _LSB(field)
>> +
>> +/* Returns the unshifted mask for the field (i.e. LSB of mask is bit 0)
>> */
>> +#define bf_rawmask(field)      ((1UL<<  _BITFIELD_WIDTH(field)) - 1)
>> +
>> +/* Returns the mask for a field. Clear these bits to zero the field */
>> +#define bf_mask(field)         (bf_rawmask(field)<<  (bf_shift(field)))
>> +
>> +#endif /* __BYTE_ORDER == __LITTLE_ENDIAN */
>> +
>> +#endif
>>
>
> Amicalement,
> --
> Albert.
>