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

[Darwin Rambo]

On 14-01-31 09:54 AM, Tom Rini wrote:
> On Thu, Jan 30, 2014 at 02:03:41PM -0800, Darwin Rambo wrote:
>>
>>
>> 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.
> 
> With Matt chiming in on where this is within the kernel, lets go with
> creating a include/bitfield.h here.  Thanks!
Will implement this and will try to remove the u32's everywhere. Thanks.

>