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

[Tom Rini <trini@ti.com>]

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!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140131/951f3268/attachment.pgp>