Re: [U-Boot] [PATCH 2/6] arch: bcm281xx: Initial commit of bcm281xx architecture code
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
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
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
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
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
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++,