[U-Boot] [PATCH 2/6] arch: bcm281xx: Initial commit of bcm281xx architecture code

[Darwin Rambo <drambo@broadcom.com>]

On 14-01-29 02:32 PM, Tom Rini wrote:
> On Mon, Jan 27, 2014 at 10:53:26AM -0800, Darwin Rambo wrote:
> 
>> Add bcm281xx architecture support code including a clock framework and
>> chip reset.  Define register block base addresses for the bcm281xx
>> architecture and create an empty gpio header file required when
>> CONFIG_CMD_GPIO is set.
> [snip]
>> +/* Bitfield operations */
>> +
>> +/* Produces a mask of set bits covering a range of a 32-bit value */
>> +static inline u32 bitfield_mask(u32 shift, u32 width)
>> +{
>> +	return ((1 << width) - 1) << shift;
>> +}
>> +
>> +/* Extract the value of a bitfield found within a given register value */
>> +static inline u32 bitfield_extract(u32 reg_val, u32 shift, u32 width)
>> +{
>> +	return (reg_val & bitfield_mask(shift, width)) >> shift;
>> +}
>> +
>> +/* Replace the value of a bitfield found within a given register value */
>> +static inline u32 bitfield_replace(u32 reg_val, u32 shift, u32 width, u32 val)
>> +{
>> +	u32 mask = bitfield_mask(shift, width);
>> +
>> +	return (reg_val & ~mask) | (val << shift);
>> +}
> 
> This all feels horribly generic, isn't there some linux header we've
> already got that I can't think off of the top of my head that gives us
> these kind of functions?
Hi Tom,

I had a similar feeling. There are files such as include/linux/bitops.h
and arch/arm/include/asm/bitops.h and a host of others, but these seem
single bit oriented, and don't provide the functionality here. The
bcm281xx clock registers are a myriad of bit fields of different widths
and positions, and the driver code is simpler because it uses these
generic bitfield functions and data tables to describe the bitfields.
Perhaps the bcm281xx clock register hardware has revealed the need for
more functions like this now. I've searched through the tree for
equivalent functions and they don't seem to exist, but I could be wrong.
We could create include/bitfield.h with functions specifically for
bitfield operations if it were warranted. But if it only ever got used
by one driver, it might be wrong to make it generic. But my gut feel is
that if we did create include/bitfield.h it probably would be used by
others who wanted to take a similar data-driven approach to register
fields. We would also have to make it non-u32 specific I imagine,
possibly just 'int' types. Thanks.

Darwin

<snip>