[U-Boot] [PATCH V2 1/6] ARM: tegra: deduplicate MASK_BITS_xxx clock mux enum
From: Tom Warren twarren.nvidia@gmail.com
The enum used to define the set of register bits used to represent a clock's input mux, MUX_BITS_*, is defined separately for each SoC at present. Move this definition to a common location to ease fixing up some issues with the definition, and the code that uses it.
Signed-off-by: Tom Warren twarren@nvidia.com [swarren, extracted from a larger patch by Tom] Signed-off-by: Stephen Warren swarren@nvidia.com Reviewed-by: Thierry Reding treding@nvidia.com Tested-by: Thierry Reding treding@nvidia.com Acked-by: Thierry Reding treding@nvidia.com --- arch/arm/cpu/tegra114-common/clock.c | 6 ------ arch/arm/cpu/tegra30-common/clock.c | 6 ------ arch/arm/include/asm/arch-tegra/clock.h | 6 ++++++ 3 files changed, 6 insertions(+), 12 deletions(-)
diff --git a/arch/arm/cpu/tegra114-common/clock.c b/arch/arm/cpu/tegra114-common/clock.c index 5c4305a418cc..47612e12d262 100644 --- a/arch/arm/cpu/tegra114-common/clock.c +++ b/arch/arm/cpu/tegra114-common/clock.c @@ -61,12 +61,6 @@ enum { CLOCK_MAX_MUX = 8 /* number of source options for each clock */ };
-enum { - MASK_BITS_31_30 = 2, /* num of bits used to specify clock source */ - MASK_BITS_31_29, - MASK_BITS_29_28, -}; - /* * Clock source mux for each clock type. This just converts our enum into * a list of mux sources for use by the code. diff --git a/arch/arm/cpu/tegra30-common/clock.c b/arch/arm/cpu/tegra30-common/clock.c index 74bd22be1aeb..89c3529c885b 100644 --- a/arch/arm/cpu/tegra30-common/clock.c +++ b/arch/arm/cpu/tegra30-common/clock.c @@ -60,12 +60,6 @@ enum { CLOCK_MAX_MUX = 8 /* number of source options for each clock */ };
-enum { - MASK_BITS_31_30 = 2, /* num of bits used to specify clock source */ - MASK_BITS_31_29, - MASK_BITS_29_28, -}; - /* * Clock source mux for each clock type. This just converts our enum into * a list of mux sources for use by the code. diff --git a/arch/arm/include/asm/arch-tegra/clock.h b/arch/arm/include/asm/arch-tegra/clock.h index e7d0fd45ee1d..052c0208b18a 100644 --- a/arch/arm/include/asm/arch-tegra/clock.h +++ b/arch/arm/include/asm/arch-tegra/clock.h @@ -20,6 +20,12 @@ enum clock_osc_freq { CLOCK_OSC_FREQ_COUNT, };
+enum { + MASK_BITS_31_30 = 2, /* num of bits used to specify clock source */ + MASK_BITS_31_29, + MASK_BITS_29_28, +}; + #include <asm/arch/clock-tables.h> /* PLL stabilization delay in usec */ #define CLOCK_PLL_STABLE_DELAY_US 300
From: Stephen Warren swarren@nvidia.com
The only place where the MASK_BITS_* values are used is in adjust_periph_pll(), which interprets the value 4 (old MASK_BITS_29_28, new MASK_BITS_31_28) as being associated with mask OUT_CLK_SOURCE4_MASK, i.e. bits 31:28. Rename the MASK_BITS_ macro to reflect how it's actually implemented.
Note that no Tegra clock register actually uses all of bits 31:28 as the mux field. Rather, bits 30:28, 29:28, or 28 are used. However, in those cases, nothing is stored in the bits above the mux field, so it's safe to pretend that the mux field extends all the way to the end of the register. As such, the U-Boot clock driver is currently a bit lazy, and doesn't distinguish between 31:28, 30:28, 29:28 and 28; it just lumps them all together and pretends they're all 31:28. This patch doesn't cause this issue; it was pre-existing. Hopefully, future patches will clean this up.
Signed-off-by: Stephen Warren swarren@nvidia.com Reviewed-by: Thierry Reding treding@nvidia.com Tested-by: Thierry Reding treding@nvidia.com Acked-by: Thierry Reding treding@nvidia.com --- v2: Fixed typo in comment & patch description --- arch/arm/cpu/tegra114-common/clock.c | 2 +- arch/arm/cpu/tegra30-common/clock.c | 2 +- arch/arm/include/asm/arch-tegra/clock.h | 11 ++++++++++- 3 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/arch/arm/cpu/tegra114-common/clock.c b/arch/arm/cpu/tegra114-common/clock.c index 47612e12d262..3bede71a7a1f 100644 --- a/arch/arm/cpu/tegra114-common/clock.c +++ b/arch/arm/cpu/tegra114-common/clock.c @@ -103,7 +103,7 @@ static enum clock_id clock_source[CLOCK_TYPE_COUNT][CLOCK_MAX_MUX+1] = { MASK_BITS_31_29}, { CLK(PERIPH), CLK(CGENERAL), CLK(SFROM32KHZ), CLK(OSC), CLK(NONE), CLK(NONE), CLK(NONE), CLK(NONE), - MASK_BITS_29_28} + MASK_BITS_31_28} };
/* diff --git a/arch/arm/cpu/tegra30-common/clock.c b/arch/arm/cpu/tegra30-common/clock.c index 89c3529c885b..33528702185e 100644 --- a/arch/arm/cpu/tegra30-common/clock.c +++ b/arch/arm/cpu/tegra30-common/clock.c @@ -102,7 +102,7 @@ static enum clock_id clock_source[CLOCK_TYPE_COUNT][CLOCK_MAX_MUX+1] = { MASK_BITS_31_29}, { CLK(PERIPH), CLK(CGENERAL), CLK(SFROM32KHZ), CLK(OSC), CLK(NONE), CLK(NONE), CLK(NONE), CLK(NONE), - MASK_BITS_29_28} + MASK_BITS_31_28} };
/* diff --git a/arch/arm/include/asm/arch-tegra/clock.h b/arch/arm/include/asm/arch-tegra/clock.h index 052c0208b18a..80825e30f8e3 100644 --- a/arch/arm/include/asm/arch-tegra/clock.h +++ b/arch/arm/include/asm/arch-tegra/clock.h @@ -20,10 +20,19 @@ enum clock_osc_freq { CLOCK_OSC_FREQ_COUNT, };
+/* + * Note that no Tegra clock register actually uses all of bits 31:28 as + * the mux field. Rather, bits 30:28, 29:28, or 28 are used. However, in + * those cases, nothing is stored in the bits about the mux field, so it's + * safe to pretend that the mux field extends all the way to the end of the + * register. As such, the U-Boot clock driver is currently a bit lazy, and + * doesn't distinguish between 31:28, 30:28, 29:28 and 28; it just lumps + * them all together and pretends they're all 31:28. + */ enum { MASK_BITS_31_30 = 2, /* num of bits used to specify clock source */ MASK_BITS_31_29, - MASK_BITS_29_28, + MASK_BITS_31_28, };
#include <asm/arch/clock-tables.h>
From: Stephen Warren swarren@nvidia.com
OUT_CLK_SOURCE_ are currently named after the number of bits the mask they represent includes. However, bit count is not the only possible variable; bit position may also vary. Rename OUT_CLK_SOURCE_ to OUT_CLK_SOURCE_31_30_ and OUT_CLK_SOURCE4_ to OUT_CLK_SOURCE_31_28 to more completely describe exactly what they represent, without having to go look up the definitions.
Signed-off-by: Stephen Warren swarren@nvidia.com Reviewed-by: Thierry Reding treding@nvidia.com Tested-by: Thierry Reding treding@nvidia.com Acked-by: Thierry Reding treding@nvidia.com --- arch/arm/cpu/tegra-common/clock.c | 16 ++++++++-------- arch/arm/include/asm/arch-tegra/clk_rst.h | 9 +++++---- 2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/arch/arm/cpu/tegra-common/clock.c b/arch/arm/cpu/tegra-common/clock.c index 268fb912b502..d9f2c767d5d1 100644 --- a/arch/arm/cpu/tegra-common/clock.c +++ b/arch/arm/cpu/tegra-common/clock.c @@ -142,8 +142,8 @@ void clock_ll_set_source_divisor(enum periph_id periph_id, unsigned source,
value = readl(reg);
- value &= ~OUT_CLK_SOURCE_MASK; - value |= source << OUT_CLK_SOURCE_SHIFT; + value &= ~OUT_CLK_SOURCE_31_30_MASK; + value |= source << OUT_CLK_SOURCE_31_30_SHIFT;
value &= ~OUT_CLK_DIVISOR_MASK; value |= divisor << OUT_CLK_DIVISOR_SHIFT; @@ -155,8 +155,8 @@ void clock_ll_set_source(enum periph_id periph_id, unsigned source) { u32 *reg = get_periph_source_reg(periph_id);
- clrsetbits_le32(reg, OUT_CLK_SOURCE_MASK, - source << OUT_CLK_SOURCE_SHIFT); + clrsetbits_le32(reg, OUT_CLK_SOURCE_31_30_MASK, + source << OUT_CLK_SOURCE_31_30_SHIFT); }
/** @@ -305,11 +305,11 @@ static int adjust_periph_pll(enum periph_id periph_id, int source, if (source < 0) return -1; if (mux_bits == 4) { - clrsetbits_le32(reg, OUT_CLK_SOURCE4_MASK, - source << OUT_CLK_SOURCE4_SHIFT); + clrsetbits_le32(reg, OUT_CLK_SOURCE_31_28_MASK, + source << OUT_CLK_SOURCE_31_28_SHIFT); } else { - clrsetbits_le32(reg, OUT_CLK_SOURCE_MASK, - source << OUT_CLK_SOURCE_SHIFT); + clrsetbits_le32(reg, OUT_CLK_SOURCE_31_30_MASK, + source << OUT_CLK_SOURCE_31_30_SHIFT); } udelay(2); return 0; diff --git a/arch/arm/include/asm/arch-tegra/clk_rst.h b/arch/arm/include/asm/arch-tegra/clk_rst.h index 074b3bca0b4f..9f81237d2865 100644 --- a/arch/arm/include/asm/arch-tegra/clk_rst.h +++ b/arch/arm/include/asm/arch-tegra/clk_rst.h @@ -233,11 +233,12 @@ enum { #define OUT_CLK_DIVISOR_SHIFT 0 #define OUT_CLK_DIVISOR_MASK (0xffff << OUT_CLK_DIVISOR_SHIFT)
-#define OUT_CLK_SOURCE_SHIFT 30 -#define OUT_CLK_SOURCE_MASK (3U << OUT_CLK_SOURCE_SHIFT) +#define OUT_CLK_SOURCE_31_30_SHIFT 30 +#define OUT_CLK_SOURCE_31_30_MASK (3U << OUT_CLK_SOURCE_31_30_SHIFT)
-#define OUT_CLK_SOURCE4_SHIFT 28 -#define OUT_CLK_SOURCE4_MASK (15U << OUT_CLK_SOURCE4_SHIFT) +/* Note: See comment for MASK_BITS_31_28 in arch-tegra/clock.h */ +#define OUT_CLK_SOURCE_31_28_SHIFT 28 +#define OUT_CLK_SOURCE_31_28_MASK (15U << OUT_CLK_SOURCE_31_28_SHIFT)
/* CLK_RST_CONTROLLER_SCLK_BURST_POLICY */ #define SCLK_SYS_STATE_SHIFT 28U
From: Stephen Warren swarren@nvidia.com
Not all code that set or interpreted "mux_bits" was using the named macros, but rather some was simply using hard-coded integer constants. This makes it hard to determine which pieces of code are affected by changes to those constants.
Replace the integer constants with the equivalent macro definitions so that everything is nicely tied together.
Note that I'm not convinced all the code was using the correct integer constants, and hence I'm not convinced that all the code is now using the desired macros. However, this change is a purely mechanical replacement and should have no functional change. Fixing any bugs will come later, separately.
Signed-off-by: Stephen Warren swarren@nvidia.com Reviewed-by: Thierry Reding treding@nvidia.com Tested-by: Thierry Reding treding@nvidia.com Acked-by: Thierry Reding treding@nvidia.com --- arch/arm/cpu/tegra-common/clock.c | 2 +- arch/arm/cpu/tegra20-common/clock.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/cpu/tegra-common/clock.c b/arch/arm/cpu/tegra-common/clock.c index d9f2c767d5d1..96b705f2f6a4 100644 --- a/arch/arm/cpu/tegra-common/clock.c +++ b/arch/arm/cpu/tegra-common/clock.c @@ -304,7 +304,7 @@ static int adjust_periph_pll(enum periph_id periph_id, int source, /* work out the source clock and set it */ if (source < 0) return -1; - if (mux_bits == 4) { + if (mux_bits == MASK_BITS_31_28) { clrsetbits_le32(reg, OUT_CLK_SOURCE_31_28_MASK, source << OUT_CLK_SOURCE_31_28_SHIFT); } else { diff --git a/arch/arm/cpu/tegra20-common/clock.c b/arch/arm/cpu/tegra20-common/clock.c index 34124f9bbac3..0c4f5fb288a0 100644 --- a/arch/arm/cpu/tegra20-common/clock.c +++ b/arch/arm/cpu/tegra20-common/clock.c @@ -412,9 +412,9 @@ int get_periph_clock_source(enum periph_id periph_id, * with its 16-bit divisor */ if (type == CLOCK_TYPE_PCXTS) - *mux_bits = 4; + *mux_bits = MASK_BITS_31_28; else - *mux_bits = 2; + *mux_bits = MASK_BITS_31_30; if (type == CLOCK_TYPE_PCMT16) *divider_bits = 16; else
From: Stephen Warren swarren@nvidia.com
Since all code that sets or interprets MASK_BITS_* now uses the enums to define/compare the values, there is no need for MASK_BITS_* to have a specific integer value. In fact, having a specific integer value may encourage people to hard-code those values, or interpret the values in incorrect ways.
As such, remove the logic that assigns a specific value to the enum values in order to make it completely clear that it's just an enum, not something that directly represents some integer value.
Signed-off-by: Stephen Warren swarren@nvidia.com Reviewed-by: Thierry Reding treding@nvidia.com Tested-by: Thierry Reding treding@nvidia.com Acked-by: Thierry Reding treding@nvidia.com --- v2: Remove comment that described the removed assignment of numbers to MASK_BITS_* --- arch/arm/include/asm/arch-tegra/clock.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/arch-tegra/clock.h b/arch/arm/include/asm/arch-tegra/clock.h index 80825e30f8e3..2f85696a5854 100644 --- a/arch/arm/include/asm/arch-tegra/clock.h +++ b/arch/arm/include/asm/arch-tegra/clock.h @@ -30,7 +30,7 @@ enum clock_osc_freq { * them all together and pretends they're all 31:28. */ enum { - MASK_BITS_31_30 = 2, /* num of bits used to specify clock source */ + MASK_BITS_31_30, MASK_BITS_31_29, MASK_BITS_31_28, };
From: Tom Warren twarren.nvidia@gmail.com
Some clock sources have 3-bit muxes in bits 31:29. Implement core support for this mux field.
Signed-off-by: Tom Warren twarren@nvidia.com [swarren, extracted from a larger patch by Tom] Signed-off-by: Stephen Warren swarren@nvidia.com Reviewed-by: Thierry Reding treding@nvidia.com Tested-by: Thierry Reding treding@nvidia.com Acked-by: Thierry Reding treding@nvidia.com --- arch/arm/cpu/tegra-common/clock.c | 22 ++++++++++++++++++---- arch/arm/include/asm/arch-tegra/clk_rst.h | 3 +++ 2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/arch/arm/cpu/tegra-common/clock.c b/arch/arm/cpu/tegra-common/clock.c index 96b705f2f6a4..33bb19084b8c 100644 --- a/arch/arm/cpu/tegra-common/clock.c +++ b/arch/arm/cpu/tegra-common/clock.c @@ -304,13 +304,27 @@ static int adjust_periph_pll(enum periph_id periph_id, int source, /* work out the source clock and set it */ if (source < 0) return -1; - if (mux_bits == MASK_BITS_31_28) { - clrsetbits_le32(reg, OUT_CLK_SOURCE_31_28_MASK, - source << OUT_CLK_SOURCE_31_28_SHIFT); - } else { + + switch (mux_bits) { + case MASK_BITS_31_30: clrsetbits_le32(reg, OUT_CLK_SOURCE_31_30_MASK, source << OUT_CLK_SOURCE_31_30_SHIFT); + break; + + case MASK_BITS_31_29: + clrsetbits_le32(reg, OUT_CLK_SOURCE_31_29_MASK, + source << OUT_CLK_SOURCE_31_29_SHIFT); + break; + + case MASK_BITS_31_28: + clrsetbits_le32(reg, OUT_CLK_SOURCE_31_28_MASK, + source << OUT_CLK_SOURCE_31_28_SHIFT); + break; + + default: + return -1; } + udelay(2); return 0; } diff --git a/arch/arm/include/asm/arch-tegra/clk_rst.h b/arch/arm/include/asm/arch-tegra/clk_rst.h index 9f81237d2865..f07b83d26af4 100644 --- a/arch/arm/include/asm/arch-tegra/clk_rst.h +++ b/arch/arm/include/asm/arch-tegra/clk_rst.h @@ -236,6 +236,9 @@ enum { #define OUT_CLK_SOURCE_31_30_SHIFT 30 #define OUT_CLK_SOURCE_31_30_MASK (3U << OUT_CLK_SOURCE_31_30_SHIFT)
+#define OUT_CLK_SOURCE_31_29_SHIFT 29 +#define OUT_CLK_SOURCE_31_29_MASK (7U << OUT_CLK_SOURCE_31_29_SHIFT) + /* Note: See comment for MASK_BITS_31_28 in arch-tegra/clock.h */ #define OUT_CLK_SOURCE_31_28_SHIFT 28 #define OUT_CLK_SOURCE_31_28_MASK (15U << OUT_CLK_SOURCE_31_28_SHIFT)
participants (1)
-
Stephen Warren