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

2014-01-31 Thread Matt Porter
On Wed, Jan 29, 2014 at 05:32:30PM -0500, 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?

To add to what Darwin mentioned about bitops.h being insufficient...

The equivalent kernel implementations are wrapped up in the
regmap/regmap-mmio helpers. That implementation is *very* heavyweight,
and IMHO simply not appropriate for U-Boot (and often not useful in the
kernel as well).  A homegrown generic set of inline ops would seem to be
ideal for U-Boot.

-Matt
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-01-31 Thread Tom Rini
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


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-01-31 Thread 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.

 
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-01-30 Thread Darwin Rambo


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
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-01-29 Thread Tom Rini
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?

 + clk_dbg(%s: %s\n, __func__, c-name);

Please juse use debug(...), here and elsewhere.

 +#ifndef offsetof
 +#define offsetof(TYPE, MEMBER) ((size_t) ((TYPE *)0)-MEMBER)
 +#endif
 +
 +#ifndef container_of
 +#define container_of(ptr, type, member) ({ \
 + const typeof(((type *)0)-member) * __mptr = (ptr); \
 + (type *)((char *)__mptr - offsetof(type, member)); })
 +#endif

We already have these..

-- 
Tom


signature.asc
Description: Digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2014-01-27 Thread Darwin Rambo
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.

Signed-off-by: Darwin Rambo dra...@broadcom.com
Reviewed-by: Steve Rae s...@broadcom.com
Reviewed-by: Tim Kryger tkry...@linaro.org
---
 arch/arm/cpu/armv7/bcm281xx/Makefile|   11 +
 arch/arm/cpu/armv7/bcm281xx/clk-bcm281xx.c  |  525 ++
 arch/arm/cpu/armv7/bcm281xx/clk-bsc.c   |   54 +++
 arch/arm/cpu/armv7/bcm281xx/clk-core.c  |  536 +++
 arch/arm/cpu/armv7/bcm281xx/clk-core.h  |  510 +
 arch/arm/cpu/armv7/bcm281xx/clk-sdio.c  |   75 
 arch/arm/cpu/armv7/bcm281xx/reset.c |   29 ++
 arch/arm/include/asm/arch-bcm281xx/gpio.h   |   17 +
 arch/arm/include/asm/arch-bcm281xx/sysmap.h |   27 ++
 9 files changed, 1784 insertions(+)
 create mode 100644 arch/arm/cpu/armv7/bcm281xx/Makefile
 create mode 100644 arch/arm/cpu/armv7/bcm281xx/clk-bcm281xx.c
 create mode 100644 arch/arm/cpu/armv7/bcm281xx/clk-bsc.c
 create mode 100644 arch/arm/cpu/armv7/bcm281xx/clk-core.c
 create mode 100644 arch/arm/cpu/armv7/bcm281xx/clk-core.h
 create mode 100644 arch/arm/cpu/armv7/bcm281xx/clk-sdio.c
 create mode 100644 arch/arm/cpu/armv7/bcm281xx/reset.c
 create mode 100644 arch/arm/include/asm/arch-bcm281xx/gpio.h
 create mode 100644 arch/arm/include/asm/arch-bcm281xx/sysmap.h

diff --git a/arch/arm/cpu/armv7/bcm281xx/Makefile 
b/arch/arm/cpu/armv7/bcm281xx/Makefile
new file mode 100644
index 000..46c4943
--- /dev/null
+++ b/arch/arm/cpu/armv7/bcm281xx/Makefile
@@ -0,0 +1,11 @@
+#
+# Copyright 2013 Broadcom Corporation. All rights reserved.
+#
+# SPDX-License-Identifier: GPL-2.0+
+#
+
+obj-y  += reset.o
+obj-y  += clk-core.o
+obj-y  += clk-bcm281xx.o
+obj-y  += clk-sdio.o
+obj-y  += clk-bsc.o
diff --git a/arch/arm/cpu/armv7/bcm281xx/clk-bcm281xx.c 
b/arch/arm/cpu/armv7/bcm281xx/clk-bcm281xx.c
new file mode 100644
index 000..2726cfb
--- /dev/null
+++ b/arch/arm/cpu/armv7/bcm281xx/clk-bcm281xx.c
@@ -0,0 +1,525 @@
+/*
+*
+* Copyright 2013 Broadcom Corporation.  All rights reserved.
+*
+* SPDX-License-Identifier:  GPL-2.0+
+*
+*/
+
+/*
+*
+* bcm281xx-specific clock tables
+*
+*/
+
+#include common.h
+#include asm/io.h
+#include asm/errno.h
+#include asm/arch/sysmap.h
+#include asm/kona-common/clk.h
+#include clk-core.h
+
+#defineCLOCK_1K1000
+#defineCLOCK_1M(CLOCK_1K * 1000)
+
+/* declare a reference clock */
+#define DECLARE_REF_CLK(clk_name, clk_parent, clk_rate, clk_div) \
+static struct refclk clk_name = { \
+   .clk=   { \
+   .name   =   #clk_name, \
+   .parent =   clk_parent, \
+   .rate   =   clk_rate, \
+   .div=   clk_div, \
+   .ops=   ref_clk_ops, \
+   }, \
+}
+
+/*
+   Reference clocks
+*/
+/* Declare a list of reference clocks */
+DECLARE_REF_CLK(ref_crystal,   0,  26  * CLOCK_1M, 1);
+DECLARE_REF_CLK(var_96m,   0,  96  * CLOCK_1M, 1);
+DECLARE_REF_CLK(ref_96m,   0,  96  * CLOCK_1M, 1);
+DECLARE_REF_CLK(ref_312m,  0,  312 * CLOCK_1M, 0);
+DECLARE_REF_CLK(ref_104m,  ref_312m.clk,  104 * CLOCK_1M, 3);
+DECLARE_REF_CLK(ref_52m,   ref_104m.clk,  52  * CLOCK_1M, 2);
+DECLARE_REF_CLK(ref_13m,   ref_52m.clk,   13  * CLOCK_1M, 4);
+DECLARE_REF_CLK(var_312m,  0,  312 * CLOCK_1M, 0);
+DECLARE_REF_CLK(var_104m,  var_312m.clk,  104 * CLOCK_1M, 3);
+DECLARE_REF_CLK(var_52m,   var_104m.clk,  52  * CLOCK_1M, 2);
+DECLARE_REF_CLK(var_13m,   var_52m.clk,   13  * CLOCK_1M, 4);
+
+struct refclk_lkup {
+   struct refclk *procclk;
+   const char *name;
+};
+
+/* Lookup table for string to clk tranlation */
+#define MKSTR(x) {x, #x}
+static struct refclk_lkup refclk_str_tbl[] = {
+   MKSTR(ref_crystal), MKSTR(var_96m), MKSTR(ref_96m),
+   MKSTR(ref_312m), MKSTR(ref_104m), MKSTR(ref_52m),
+   MKSTR(ref_13m), MKSTR(var_312m), MKSTR(var_104m),
+   MKSTR(var_52m), MKSTR(var_13m),
+};
+
+int refclk_entries = sizeof(refclk_str_tbl)/sizeof(refclk_str_tbl[0]);
+
+/* convert ref clock string to clock structure pointer */
+struct refclk *refclk_str_to_clk(const char *name)
+{
+   int i;
+   struct refclk_lkup *tblp = refclk_str_tbl;
+   for (i = 0; i  refclk_entries; i++,