Skip to contentSkip to conversation

clk: iproc: add support for BCM NS, NSP, and NS2

PATCH Superseded Jon Mason 3 participants 18 messages Oct 13, 2015 – Oct 15, 2015

Subsystems (6)
Device tree bindings Arm (32-bit) Arm sub-architectures Common clk +2 more subsystems

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Jon Mason

author
#0 lore raw

v2 cover clk: iproc: add support for BCM NS, NSP, and NS2

v2 - Added a patch to change the Cygnus clk macros to uppercase, removed the device tree changes from the series (will send out those as an RFC), and other minor changes that Stephen Boyd requested.

This patch series adds support for the Broadcom Northstar, Northstar Plus, and Northstar 2 clocks. Some slight modifications were necessary to clk-iproc-pll to get Northstar and Northstar Plus working, due to differences in register layout. This is the reason why the first patch is necessary. Some more modifications were necessary to clk-iproc-pll to get Northstar 2 working, due to differences in register layout (and resulting fallout in Cygnus and NSP). This is the reason why the sixth and seventh patches are necessary. The fifth patch is clean-up to prevent accidentally forgetting to adjust for the base write errata (which happened a few times, but was caught in internal review).

Thanks,
Jon

author
#1 lore raw

PATCH v2 1/7 clk: cygnus: Convert all macros to all caps

The macros that are being used to initialize the values of the clk structures should be all caps. Find and replace all of them with their relevant counterparts.

Signed-off-by Jon Mason <[email protected]>
---
 drivers/clk/bcm/clk-cygnus.c | 146 +++++++++++++++++++++----------------------
 1 file changed, 73 insertions(+), 73 deletions(-)
drivers/clk/bcm/clk-cygnus.c +73 -73
index 316c603..aac82c6 100644
@@ -23,28 +23,28 @@
 #include <dt-bindings/clock/bcm-cygnus.h>
 #include "clk-iproc.h"
 
-#define reg_val(o, s, w) { .offset = o, .shift = s, .width = w, }
+#define REG_VAL(o, s, w) { .offset = o, .shift = s, .width = w, }
 
-#define aon_val(o, pw, ps, is) { .offset = o, .pwr_width = pw, \
+#define AON_VAL(o, pw, ps, is) { .offset = o, .pwr_width = pw, \
 	.pwr_shift = ps, .iso_shift = is }
 
-#define sw_ctrl_val(o, s) { .offset = o, .shift = s, }
+#define SW_CTRL_VAL(o, s) { .offset = o, .shift = s, }
 
-#define asiu_div_val(o, es, hs, hw, ls, lw) \
+#define ASIU_DIV_VAL(o, es, hs, hw, ls, lw) \
 		{ .offset = o, .en_shift = es, .high_shift = hs, \
 		.high_width = hw, .low_shift = ls, .low_width = lw }
 
-#define reset_val(o, rs, prs, kis, kiw, kps, kpw, kas, kaw) { .offset = o, \
+#define RESET_VAL(o, rs, prs, kis, kiw, kps, kpw, kas, kaw) { .offset = o, \
 	.reset_shift = rs, .p_reset_shift = prs, .ki_shift = kis, \
 	.ki_width = kiw, .kp_shift = kps, .kp_width = kpw, .ka_shift = kas, \
 	.ka_width = kaw }
 
-#define vco_ctrl_val(uo, lo) { .u_offset = uo, .l_offset = lo }
+#define VCO_CTRL_VAL(uo, lo) { .u_offset = uo, .l_offset = lo }
 
-#define enable_val(o, es, hs, bs) { .offset = o, .enable_shift = es, \
+#define ENABLE_VAL(o, es, hs, bs) { .offset = o, .enable_shift = es, \
 	.hold_shift = hs, .bypass_shift = bs }
 
-#define asiu_gate_val(o, es) { .offset = o, .en_shift = es }
+#define ASIU_GATE_VAL(o, es) { .offset = o, .en_shift = es }
 
 static void __init cygnus_armpll_init(struct device_node *node)
 {

@@ -55,52 +55,52 @@ CLK_OF_DECLARE(cygnus_armpll, "brcm,cygnus-armpll", cygnus_armpll_init);
 static const struct iproc_pll_ctrl genpll = {
 	.flags = IPROC_CLK_AON | IPROC_CLK_PLL_HAS_NDIV_FRAC |
 		IPROC_CLK_PLL_NEEDS_SW_CFG,
-	.aon = aon_val(0x0, 2, 1, 0),
-	.reset = reset_val(0x0, 11, 10, 4, 3, 0, 4, 7, 3),
-	.sw_ctrl = sw_ctrl_val(0x10, 31),
-	.ndiv_int = reg_val(0x10, 20, 10),
-	.ndiv_frac = reg_val(0x10, 0, 20),
-	.pdiv = reg_val(0x14, 0, 4),
-	.vco_ctrl = vco_ctrl_val(0x18, 0x1c),
-	.status = reg_val(0x28, 12, 1),
+	.aon = AON_VAL(0x0, 2, 1, 0),
+	.reset = RESET_VAL(0x0, 11, 10, 4, 3, 0, 4, 7, 3),
+	.sw_ctrl = SW_CTRL_VAL(0x10, 31),
+	.ndiv_int = REG_VAL(0x10, 20, 10),
+	.ndiv_frac = REG_VAL(0x10, 0, 20),
+	.pdiv = REG_VAL(0x14, 0, 4),
+	.vco_ctrl = VCO_CTRL_VAL(0x18, 0x1c),
+	.status = REG_VAL(0x28, 12, 1),
 };
 
 static const struct iproc_clk_ctrl genpll_clk[] = {
 	[BCM_CYGNUS_GENPLL_AXI21_CLK] = {
 		.channel = BCM_CYGNUS_GENPLL_AXI21_CLK,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x4, 6, 0, 12),
-		.mdiv = reg_val(0x20, 0, 8),
+		.enable = ENABLE_VAL(0x4, 6, 0, 12),
+		.mdiv = REG_VAL(0x20, 0, 8),
 	},
 	[BCM_CYGNUS_GENPLL_250MHZ_CLK] = {
 		.channel = BCM_CYGNUS_GENPLL_250MHZ_CLK,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x4, 7, 1, 13),
-		.mdiv = reg_val(0x20, 10, 8),
+		.enable = ENABLE_VAL(0x4, 7, 1, 13),
+		.mdiv = REG_VAL(0x20, 10, 8),
 	},
 	[BCM_CYGNUS_GENPLL_IHOST_SYS_CLK] = {
 		.channel = BCM_CYGNUS_GENPLL_IHOST_SYS_CLK,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x4, 8, 2, 14),
-		.mdiv = reg_val(0x20, 20, 8),
+		.enable = ENABLE_VAL(0x4, 8, 2, 14),
+		.mdiv = REG_VAL(0x20, 20, 8),
 	},
 	[BCM_CYGNUS_GENPLL_ENET_SW_CLK] = {
 		.channel = BCM_CYGNUS_GENPLL_ENET_SW_CLK,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x4, 9, 3, 15),
-		.mdiv = reg_val(0x24, 0, 8),
+		.enable = ENABLE_VAL(0x4, 9, 3, 15),
+		.mdiv = REG_VAL(0x24, 0, 8),
 	},
 	[BCM_CYGNUS_GENPLL_AUDIO_125_CLK] = {
 		.channel = BCM_CYGNUS_GENPLL_AUDIO_125_CLK,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x4, 10, 4, 16),
-		.mdiv = reg_val(0x24, 10, 8),
+		.enable = ENABLE_VAL(0x4, 10, 4, 16),
+		.mdiv = REG_VAL(0x24, 10, 8),
 	},
 	[BCM_CYGNUS_GENPLL_CAN_CLK] = {
 		.channel = BCM_CYGNUS_GENPLL_CAN_CLK,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x4, 11, 5, 17),
-		.mdiv = reg_val(0x24, 20, 8),
+		.enable = ENABLE_VAL(0x4, 11, 5, 17),
+		.mdiv = REG_VAL(0x24, 20, 8),
 	},
 };
 

@@ -113,51 +113,51 @@ CLK_OF_DECLARE(cygnus_genpll, "brcm,cygnus-genpll", cygnus_genpll_clk_init);
 
 static const struct iproc_pll_ctrl lcpll0 = {
 	.flags = IPROC_CLK_AON | IPROC_CLK_PLL_NEEDS_SW_CFG,
-	.aon = aon_val(0x0, 2, 5, 4),
-	.reset = reset_val(0x0, 31, 30, 27, 3, 23, 4, 19, 4),
-	.sw_ctrl = sw_ctrl_val(0x4, 31),
-	.ndiv_int = reg_val(0x4, 16, 10),
-	.pdiv = reg_val(0x4, 26, 4),
-	.vco_ctrl = vco_ctrl_val(0x10, 0x14),
-	.status = reg_val(0x18, 12, 1),
+	.aon = AON_VAL(0x0, 2, 5, 4),
+	.reset = RESET_VAL(0x0, 31, 30, 27, 3, 23, 4, 19, 4),
+	.sw_ctrl = SW_CTRL_VAL(0x4, 31),
+	.ndiv_int = REG_VAL(0x4, 16, 10),
+	.pdiv = REG_VAL(0x4, 26, 4),
+	.vco_ctrl = VCO_CTRL_VAL(0x10, 0x14),
+	.status = REG_VAL(0x18, 12, 1),
 };
 
 static const struct iproc_clk_ctrl lcpll0_clk[] = {
 	[BCM_CYGNUS_LCPLL0_PCIE_PHY_REF_CLK] = {
 		.channel = BCM_CYGNUS_LCPLL0_PCIE_PHY_REF_CLK,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x0, 7, 1, 13),
-		.mdiv = reg_val(0x8, 0, 8),
+		.enable = ENABLE_VAL(0x0, 7, 1, 13),
+		.mdiv = REG_VAL(0x8, 0, 8),
 	},
 	[BCM_CYGNUS_LCPLL0_DDR_PHY_CLK] = {
 		.channel = BCM_CYGNUS_LCPLL0_DDR_PHY_CLK,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x0, 8, 2, 14),
-		.mdiv = reg_val(0x8, 10, 8),
+		.enable = ENABLE_VAL(0x0, 8, 2, 14),
+		.mdiv = REG_VAL(0x8, 10, 8),
 	},
 	[BCM_CYGNUS_LCPLL0_SDIO_CLK] = {
 		.channel = BCM_CYGNUS_LCPLL0_SDIO_CLK,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x0, 9, 3, 15),
-		.mdiv = reg_val(0x8, 20, 8),
+		.enable = ENABLE_VAL(0x0, 9, 3, 15),
+		.mdiv = REG_VAL(0x8, 20, 8),
 	},
 	[BCM_CYGNUS_LCPLL0_USB_PHY_REF_CLK] = {
 		.channel = BCM_CYGNUS_LCPLL0_USB_PHY_REF_CLK,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x0, 10, 4, 16),
-		.mdiv = reg_val(0xc, 0, 8),
+		.enable = ENABLE_VAL(0x0, 10, 4, 16),
+		.mdiv = REG_VAL(0xc, 0, 8),
 	},
 	[BCM_CYGNUS_LCPLL0_SMART_CARD_CLK] = {
 		.channel = BCM_CYGNUS_LCPLL0_SMART_CARD_CLK,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x0, 11, 5, 17),
-		.mdiv = reg_val(0xc, 10, 8),
+		.enable = ENABLE_VAL(0x0, 11, 5, 17),
+		.mdiv = REG_VAL(0xc, 10, 8),
 	},
 	[BCM_CYGNUS_LCPLL0_CH5_UNUSED] = {
 		.channel = BCM_CYGNUS_LCPLL0_CH5_UNUSED,
 		.flags = IPROC_CLK_AON,
-		.enable = enable_val(0x0, 12, 6, 18),
-		.mdiv = reg_val(0xc, 20, 8),
+		.enable = ENABLE_VAL(0x0, 12, 6, 18),
+		.mdiv = REG_VAL(0xc, 20, 8),
 	},
 };
 

@@ -189,52 +189,52 @@ static const struct iproc_pll_vco_param mipipll_vco_params[] = {
 static const struct iproc_pll_ctrl mipipll = {
 	.flags = IPROC_CLK_PLL_ASIU | IPROC_CLK_PLL_HAS_NDIV_FRAC |
 		 IPROC_CLK_NEEDS_READ_BACK,
-	.aon = aon_val(0x0, 4, 17, 16),
-	.asiu = asiu_gate_val(0x0, 3),
-	.reset = reset_val(0x0, 11, 10, 4, 3, 0, 4, 7, 4),
-	.ndiv_int = reg_val(0x10, 20, 10),
-	.ndiv_frac = reg_val(0x10, 0, 20),
-	.pdiv = reg_val(0x14, 0, 4),
-	.vco_ctrl = vco_ctrl_val(0x18, 0x1c),
-	.status = reg_val(0x28, 12, 1),
+	.aon = AON_VAL(0x0, 4, 17, 16),
+	.asiu = ASIU_GATE_VAL(0x0, 3),
+	.reset = RESET_VAL(0x0, 11, 10, 4, 3, 0, 4, 7, 4),
+	.ndiv_int = REG_VAL(0x10, 20, 10),
+	.ndiv_frac = REG_VAL(0x10, 0, 20),
+	.pdiv = REG_VAL(0x14, 0, 4),
+	.vco_ctrl = VCO_CTRL_VAL(0x18, 0x1c),
+	.status = REG_VAL(0x28, 12, 1),
 };
 
 static const struct iproc_clk_ctrl mipipll_clk[] = {
 	[BCM_CYGNUS_MIPIPLL_CH0_UNUSED] = {
 		.channel = BCM_CYGNUS_MIPIPLL_CH0_UNUSED,
 		.flags = IPROC_CLK_NEEDS_READ_BACK,
-		.enable = enable_val(0x4, 12, 6, 18),
-		.mdiv = reg_val(0x20, 0, 8),
+		.enable = ENABLE_VAL(0x4, 12, 6, 18),
+		.mdiv = REG_VAL(0x20, 0, 8),
 	},
 	[BCM_CYGNUS_MIPIPLL_CH1_LCD] = {
 		.channel = BCM_CYGNUS_MIPIPLL_CH1_LCD,
 		.flags = IPROC_CLK_NEEDS_READ_BACK,
-		.enable = enable_val(0x4, 13, 7, 19),
-		.mdiv = reg_val(0x20, 10, 8),
+		.enable = ENABLE_VAL(0x4, 13, 7, 19),
+		.mdiv = REG_VAL(0x20, 10, 8),
 	},
 	[BCM_CYGNUS_MIPIPLL_CH2_V3D] = {
 		.channel = BCM_CYGNUS_MIPIPLL_CH2_V3D,
 		.flags = IPROC_CLK_NEEDS_READ_BACK,
-		.enable = enable_val(0x4, 14, 8, 20),
-		.mdiv = reg_val(0x20, 20, 8),
+		.enable = ENABLE_VAL(0x4, 14, 8, 20),
+		.mdiv = REG_VAL(0x20, 20, 8),
 	},
 	[BCM_CYGNUS_MIPIPLL_CH3_UNUSED] = {
 		.channel = BCM_CYGNUS_MIPIPLL_CH3_UNUSED,
 		.flags = IPROC_CLK_NEEDS_READ_BACK,
-		.enable = enable_val(0x4, 15, 9, 21),
-		.mdiv = reg_val(0x24, 0, 8),
+		.enable = ENABLE_VAL(0x4, 15, 9, 21),
+		.mdiv = REG_VAL(0x24, 0, 8),
 	},
 	[BCM_CYGNUS_MIPIPLL_CH4_UNUSED] = {
 		.channel = BCM_CYGNUS_MIPIPLL_CH4_UNUSED,
 		.flags = IPROC_CLK_NEEDS_READ_BACK,
-		.enable = enable_val(0x4, 16, 10, 22),
-		.mdiv = reg_val(0x24, 10, 8),
+		.enable = ENABLE_VAL(0x4, 16, 10, 22),
+		.mdiv = REG_VAL(0x24, 10, 8),
 	},
 	[BCM_CYGNUS_MIPIPLL_CH5_UNUSED] = {
 		.channel = BCM_CYGNUS_MIPIPLL_CH5_UNUSED,
 		.flags = IPROC_CLK_NEEDS_READ_BACK,
-		.enable = enable_val(0x4, 17, 11, 23),
-		.mdiv = reg_val(0x24, 20, 8),
+		.enable = ENABLE_VAL(0x4, 17, 11, 23),
+		.mdiv = REG_VAL(0x24, 20, 8),
 	},
 };
 

@@ -247,15 +247,15 @@ static void __init cygnus_mipipll_clk_init(struct device_node *node)
 CLK_OF_DECLARE(cygnus_mipipll, "brcm,cygnus-mipipll", cygnus_mipipll_clk_init);
 
 static const struct iproc_asiu_div asiu_div[] = {
-	[BCM_CYGNUS_ASIU_KEYPAD_CLK] = asiu_div_val(0x0, 31, 16, 10, 0, 10),
-	[BCM_CYGNUS_ASIU_ADC_CLK] = asiu_div_val(0x4, 31, 16, 10, 0, 10),
-	[BCM_CYGNUS_ASIU_PWM_CLK] = asiu_div_val(0x8, 31, 16, 10, 0, 10),
+	[BCM_CYGNUS_ASIU_KEYPAD_CLK] = ASIU_DIV_VAL(0x0, 31, 16, 10, 0, 10),
+	[BCM_CYGNUS_ASIU_ADC_CLK] = ASIU_DIV_VAL(0x4, 31, 16, 10, 0, 10),
+	[BCM_CYGNUS_ASIU_PWM_CLK] = ASIU_DIV_VAL(0x8, 31, 16, 10, 0, 10),
 };
 
 static const struct iproc_asiu_gate asiu_gate[] = {
-	[BCM_CYGNUS_ASIU_KEYPAD_CLK] = asiu_gate_val(0x0, 7),
-	[BCM_CYGNUS_ASIU_ADC_CLK] = asiu_gate_val(0x0, 9),
-	[BCM_CYGNUS_ASIU_PWM_CLK] = asiu_gate_val(IPROC_CLK_INVALID_OFFSET, 0),
+	[BCM_CYGNUS_ASIU_KEYPAD_CLK] = ASIU_GATE_VAL(0x0, 7),
+	[BCM_CYGNUS_ASIU_ADC_CLK] = ASIU_GATE_VAL(0x0, 9),
+	[BCM_CYGNUS_ASIU_PWM_CLK] = ASIU_GATE_VAL(IPROC_CLK_INVALID_OFFSET, 0),
 };
 
 static void __init cygnus_asiu_init(struct device_node *node)
Signature · 1 line
1.9.1
author
#2 lore raw

PATCH v2 2/7 clk: iproc: Add PWRCTRL support

In reply to Jon Mason

Some iProc SoC clocks use a different way to control clock power, via the PWRDWN bit in the PLL control register. Since the PLL control register is used to access the PWRDWN bit, there is no need for the pwr_base when this is being used. A new flag, IPROC_CLK_EMBED_PWRCTRL, has been added to identify this usage. We can use the AON interface to write the values to enable/disable PWRDOWN.

Signed-off-by Jon Mason <[email protected]>
---
 drivers/clk/bcm/clk-iproc-pll.c | 55 ++++++++++++++++++++++++++++-------------
 drivers/clk/bcm/clk-iproc.h     |  6 +++++
 2 files changed, 44 insertions(+), 17 deletions(-)
drivers/clk/bcm/clk-iproc-pll.c +38 -17
index 2dda4e8..e029ab3 100644
@@ -148,14 +148,25 @@ static void __pll_disable(struct iproc_pll *pll)
 		writel(val, pll->asiu_base + ctrl->asiu.offset);
 	}
 
-	/* latch input value so core power can be shut down */
-	val = readl(pll->pwr_base + ctrl->aon.offset);
-	val |= (1 << ctrl->aon.iso_shift);
-	writel(val, pll->pwr_base + ctrl->aon.offset);
-
-	/* power down the core */
-	val &= ~(bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift);
-	writel(val, pll->pwr_base + ctrl->aon.offset);
+	if (ctrl->flags & IPROC_CLK_EMBED_PWRCTRL) {
+		val = readl(pll->pll_base + ctrl->aon.offset);
+		val |= (bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift);
+		writel(val, pll->pll_base + ctrl->aon.offset);
+
+		if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
+			readl(pll->pll_base + ctrl->aon.offset);
+	}
+
+	if (pll->pwr_base) {
+		/* latch input value so core power can be shut down */
+		val = readl(pll->pwr_base + ctrl->aon.offset);
+		val |= (1 << ctrl->aon.iso_shift);
+		writel(val, pll->pwr_base + ctrl->aon.offset);
+
+		/* power down the core */
+		val &= ~(bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift);
+		writel(val, pll->pwr_base + ctrl->aon.offset);
+	}
 }
 
 static int __pll_enable(struct iproc_pll *pll)

@@ -163,11 +174,22 @@ static int __pll_enable(struct iproc_pll *pll)
 	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
 	u32 val;
 
-	/* power up the PLL and make sure it's not latched */
-	val = readl(pll->pwr_base + ctrl->aon.offset);
-	val |= bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift;
-	val &= ~(1 << ctrl->aon.iso_shift);
-	writel(val, pll->pwr_base + ctrl->aon.offset);
+	if (ctrl->flags & IPROC_CLK_EMBED_PWRCTRL) {
+		val = readl(pll->pll_base + ctrl->aon.offset);
+		val &= ~(bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift);
+		writel(val, pll->pll_base + ctrl->aon.offset);
+
+		if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
+			readl(pll->pll_base + ctrl->aon.offset);
+	}
+
+	if (pll->pwr_base) {
+		/* power up the PLL and make sure it's not latched */
+		val = readl(pll->pwr_base + ctrl->aon.offset);
+		val |= bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift;
+		val &= ~(1 << ctrl->aon.iso_shift);
+		writel(val, pll->pwr_base + ctrl->aon.offset);
+	}
 
 	/* certain PLLs also need to be ungated from the ASIU top level */
 	if (ctrl->flags & IPROC_CLK_PLL_ASIU) {

@@ -610,9 +632,8 @@ void __init iproc_pll_clk_setup(struct device_node *node,
 	if (WARN_ON(!pll->pll_base))
 		goto err_pll_iomap;
 
+	/* Some SoCs do not require the pwr_base, thus failing is not fatal */
 	pll->pwr_base = of_iomap(node, 1);
-	if (WARN_ON(!pll->pwr_base))
-		goto err_pwr_iomap;
 
 	/* some PLLs require gating control at the top ASIU level */
 	if (pll_ctrl->flags & IPROC_CLK_PLL_ASIU) {

@@ -695,9 +716,9 @@ err_pll_register:
 		iounmap(pll->asiu_base);
 
 err_asiu_iomap:
-	iounmap(pll->pwr_base);
+	if (pll->pwr_base)
+		iounmap(pll->pwr_base);
 
-err_pwr_iomap:
 	iounmap(pll->pll_base);
 
 err_pll_iomap:
drivers/clk/bcm/clk-iproc.h +6 -0
index d834b7a..ff7bfad 100644
@@ -49,6 +49,12 @@
 #define IPROC_CLK_PLL_NEEDS_SW_CFG BIT(4)
 
 /*
+ * Some PLLs use a different way to control clock power, via the PWRDWN bit in
+ * the PLL control register
+ */
+#define IPROC_CLK_EMBED_PWRCTRL BIT(5)
+
+/*
  * Parameters for VCO frequency configuration
  *
  * VCO frequency =
Signature · 1 line
1.9.1
author
#3 lore raw

PATCH v2 3/7 clk: nsp: add clock support for Broadcom Northstar Plus SoC

In reply to Jon Mason

The Broadcom Northstar Plus SoC is architected under the iProc architecture. It has the following PLLs: ARMPLL, GENPLL, LCPLL0, all derived from an onboard crystal.

Signed-off-by Jon Mason <[email protected]>
---
 drivers/clk/bcm/Makefile            |   2 +
 drivers/clk/bcm/clk-nsp.c           | 135 ++++++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/bcm-nsp.h |  51 ++++++++++++++
 3 files changed, 188 insertions(+)
 create mode 100644 drivers/clk/bcm/clk-nsp.c
 create mode 100644 include/dt-bindings/clock/bcm-nsp.h
drivers/clk/bcm/Makefile +2 -0
index 8a7a477..e258b28 100644
@@ -4,3 +4,5 @@ obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm281xx.o
 obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm21664.o
 obj-$(CONFIG_COMMON_CLK_IPROC)	+= clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o
 obj-$(CONFIG_ARCH_BCM_CYGNUS)	+= clk-cygnus.o
+obj-$(CONFIG_ARCH_BCM_NSP)	+= clk-nsp.o
+obj-$(CONFIG_ARCH_BCM_5301X)	+= clk-nsp.o
drivers/clk/bcm/clk-nsp.c new file +135 -0

135 lines not on this page · raw message

include/dt-bindings/clock/bcm-nsp.h new file +51 -0

51 lines not on this page · raw message

Signature · 1 line
1.9.1
author
#4 lore raw

PATCH v2 4/7 clk: iproc: Add PLL base write function

In reply to Jon Mason

All writes to the PLL base address must be flushed if the IPROC_CLK_NEEDS_READ_BACK flag is set. If we add a function to make the necessary write and reads, we can make sure that any future code which makes PLL base writes will do the correct thing.

Signed-off-by Jon Mason <[email protected]>
---
 drivers/clk/bcm/clk-iproc-pll.c | 80 +++++++++++++++++------------------------
 1 file changed, 33 insertions(+), 47 deletions(-)
drivers/clk/bcm/clk-iproc-pll.c +33 -47
index e029ab3..74cdd66 100644
@@ -137,6 +137,18 @@ static int pll_wait_for_lock(struct iproc_pll *pll)
 	return -EIO;
 }
 
+static void iproc_pll_write(const struct iproc_pll *pll, void __iomem *base,
+			    const u32 offset, u32 val)
+{
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+
+	writel(val, base + offset);
+
+	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK &&
+		     base == pll->pll_base))
+		val = readl(base + offset);
+}
+
 static void __pll_disable(struct iproc_pll *pll)
 {
 	const struct iproc_pll_ctrl *ctrl = pll->ctrl;

@@ -145,27 +157,24 @@ static void __pll_disable(struct iproc_pll *pll)
 	if (ctrl->flags & IPROC_CLK_PLL_ASIU) {
 		val = readl(pll->asiu_base + ctrl->asiu.offset);
 		val &= ~(1 << ctrl->asiu.en_shift);
-		writel(val, pll->asiu_base + ctrl->asiu.offset);
+		iproc_pll_write(pll, pll->asiu_base, ctrl->asiu.offset, val);
 	}
 
 	if (ctrl->flags & IPROC_CLK_EMBED_PWRCTRL) {
 		val = readl(pll->pll_base + ctrl->aon.offset);
 		val |= (bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift);
-		writel(val, pll->pll_base + ctrl->aon.offset);
-
-		if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-			readl(pll->pll_base + ctrl->aon.offset);
+		iproc_pll_write(pll, pll->pll_base, ctrl->aon.offset, val);
 	}
 
 	if (pll->pwr_base) {
 		/* latch input value so core power can be shut down */
 		val = readl(pll->pwr_base + ctrl->aon.offset);
 		val |= (1 << ctrl->aon.iso_shift);
-		writel(val, pll->pwr_base + ctrl->aon.offset);
+		iproc_pll_write(pll, pll->pwr_base, ctrl->aon.offset, val);
 
 		/* power down the core */
 		val &= ~(bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift);
-		writel(val, pll->pwr_base + ctrl->aon.offset);
+		iproc_pll_write(pll, pll->pwr_base, ctrl->aon.offset, val);
 	}
 }
 

@@ -177,10 +186,7 @@ static int __pll_enable(struct iproc_pll *pll)
 	if (ctrl->flags & IPROC_CLK_EMBED_PWRCTRL) {
 		val = readl(pll->pll_base + ctrl->aon.offset);
 		val &= ~(bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift);
-		writel(val, pll->pll_base + ctrl->aon.offset);
-
-		if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-			readl(pll->pll_base + ctrl->aon.offset);
+		iproc_pll_write(pll, pll->pll_base, ctrl->aon.offset, val);
 	}
 
 	if (pll->pwr_base) {

@@ -188,14 +194,14 @@ static int __pll_enable(struct iproc_pll *pll)
 		val = readl(pll->pwr_base + ctrl->aon.offset);
 		val |= bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift;
 		val &= ~(1 << ctrl->aon.iso_shift);
-		writel(val, pll->pwr_base + ctrl->aon.offset);
+		iproc_pll_write(pll, pll->pwr_base, ctrl->aon.offset, val);
 	}
 
 	/* certain PLLs also need to be ungated from the ASIU top level */
 	if (ctrl->flags & IPROC_CLK_PLL_ASIU) {
 		val = readl(pll->asiu_base + ctrl->asiu.offset);
 		val |= (1 << ctrl->asiu.en_shift);
-		writel(val, pll->asiu_base + ctrl->asiu.offset);
+		iproc_pll_write(pll, pll->asiu_base, ctrl->asiu.offset, val);
 	}
 
 	return 0;

@@ -209,9 +215,7 @@ static void __pll_put_in_reset(struct iproc_pll *pll)
 
 	val = readl(pll->pll_base + reset->offset);
 	val &= ~(1 << reset->reset_shift | 1 << reset->p_reset_shift);
-	writel(val, pll->pll_base + reset->offset);
-	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-		readl(pll->pll_base + reset->offset);
+	iproc_pll_write(pll, pll->pll_base, reset->offset, val);
 }
 
 static void __pll_bring_out_reset(struct iproc_pll *pll, unsigned int kp,

@@ -228,9 +232,7 @@ static void __pll_bring_out_reset(struct iproc_pll *pll, unsigned int kp,
 	val |=  ki << reset->ki_shift | kp << reset->kp_shift |
 		ka << reset->ka_shift;
 	val |= 1 << reset->reset_shift | 1 << reset->p_reset_shift;
-	writel(val, pll->pll_base + reset->offset);
-	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-		readl(pll->pll_base + reset->offset);
+	iproc_pll_write(pll, pll->pll_base, reset->offset, val);
 }
 
 static int pll_set_rate(struct iproc_clk *clk, unsigned int rate_index,

@@ -285,9 +287,8 @@ static int pll_set_rate(struct iproc_clk *clk, unsigned int rate_index,
 	/* put PLL in reset */
 	__pll_put_in_reset(pll);
 
-	writel(0, pll->pll_base + ctrl->vco_ctrl.u_offset);
-	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-		readl(pll->pll_base + ctrl->vco_ctrl.u_offset);
+	iproc_pll_write(pll, pll->pll_base, ctrl->vco_ctrl.u_offset, 0);
+
 	val = readl(pll->pll_base + ctrl->vco_ctrl.l_offset);
 
 	if (rate >= VCO_LOW && rate < VCO_MID)

@@ -298,17 +299,13 @@ static int pll_set_rate(struct iproc_clk *clk, unsigned int rate_index,
 	else
 		val |= (1 << PLL_VCO_HIGH_SHIFT);
 
-	writel(val, pll->pll_base + ctrl->vco_ctrl.l_offset);
-	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-		readl(pll->pll_base + ctrl->vco_ctrl.l_offset);
+	iproc_pll_write(pll, pll->pll_base, ctrl->vco_ctrl.l_offset, val);
 
 	/* program integer part of NDIV */
 	val = readl(pll->pll_base + ctrl->ndiv_int.offset);
 	val &= ~(bit_mask(ctrl->ndiv_int.width) << ctrl->ndiv_int.shift);
 	val |= vco->ndiv_int << ctrl->ndiv_int.shift;
-	writel(val, pll->pll_base + ctrl->ndiv_int.offset);
-	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-		readl(pll->pll_base + ctrl->ndiv_int.offset);
+	iproc_pll_write(pll, pll->pll_base, ctrl->ndiv_int.offset, val);
 
 	/* program fractional part of NDIV */
 	if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {

@@ -316,18 +313,15 @@ static int pll_set_rate(struct iproc_clk *clk, unsigned int rate_index,
 		val &= ~(bit_mask(ctrl->ndiv_frac.width) <<
 			 ctrl->ndiv_frac.shift);
 		val |= vco->ndiv_frac << ctrl->ndiv_frac.shift;
-		writel(val, pll->pll_base + ctrl->ndiv_frac.offset);
-		if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-			readl(pll->pll_base + ctrl->ndiv_frac.offset);
+		iproc_pll_write(pll, pll->pll_base, ctrl->ndiv_frac.offset,
+				val);
 	}
 
 	/* program PDIV */
 	val = readl(pll->pll_base + ctrl->pdiv.offset);
 	val &= ~(bit_mask(ctrl->pdiv.width) << ctrl->pdiv.shift);
 	val |= vco->pdiv << ctrl->pdiv.shift;
-	writel(val, pll->pll_base + ctrl->pdiv.offset);
-	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-		readl(pll->pll_base + ctrl->pdiv.offset);
+	iproc_pll_write(pll, pll->pll_base, ctrl->pdiv.offset, val);
 
 	__pll_bring_out_reset(pll, kp, ka, ki);
 

@@ -467,14 +461,12 @@ static int iproc_clk_enable(struct clk_hw *hw)
 	/* channel enable is active low */
 	val = readl(pll->pll_base + ctrl->enable.offset);
 	val &= ~(1 << ctrl->enable.enable_shift);
-	writel(val, pll->pll_base + ctrl->enable.offset);
+	iproc_pll_write(pll, pll->pll_base, ctrl->enable.offset, val);
 
 	/* also make sure channel is not held */
 	val = readl(pll->pll_base + ctrl->enable.offset);
 	val &= ~(1 << ctrl->enable.hold_shift);
-	writel(val, pll->pll_base + ctrl->enable.offset);
-	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-		readl(pll->pll_base + ctrl->enable.offset);
+	iproc_pll_write(pll, pll->pll_base, ctrl->enable.offset, val);
 
 	return 0;
 }

@@ -491,9 +483,7 @@ static void iproc_clk_disable(struct clk_hw *hw)
 
 	val = readl(pll->pll_base + ctrl->enable.offset);
 	val |= 1 << ctrl->enable.enable_shift;
-	writel(val, pll->pll_base + ctrl->enable.offset);
-	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-		readl(pll->pll_base + ctrl->enable.offset);
+	iproc_pll_write(pll, pll->pll_base, ctrl->enable.offset, val);
 }
 
 static unsigned long iproc_clk_recalc_rate(struct clk_hw *hw,

@@ -562,9 +552,7 @@ static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 		val &= ~(bit_mask(ctrl->mdiv.width) << ctrl->mdiv.shift);
 		val |= div << ctrl->mdiv.shift;
 	}
-	writel(val, pll->pll_base + ctrl->mdiv.offset);
-	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-		readl(pll->pll_base + ctrl->mdiv.offset);
+	iproc_pll_write(pll, pll->pll_base, ctrl->mdiv.offset, val);
 	clk->rate = parent_rate / div;
 
 	return 0;

@@ -591,9 +579,7 @@ static void iproc_pll_sw_cfg(struct iproc_pll *pll)
 
 		val = readl(pll->pll_base + ctrl->sw_ctrl.offset);
 		val |= BIT(ctrl->sw_ctrl.shift);
-		writel(val, pll->pll_base + ctrl->sw_ctrl.offset);
-		if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
-			readl(pll->pll_base + ctrl->sw_ctrl.offset);
+		iproc_pll_write(pll, pll->pll_base, ctrl->sw_ctrl.offset, val);
 	}
 }
Signature · 1 line
1.9.1
author
#5 lore raw

PATCH v2 5/7 clk: iproc: Split off dig_filter

In reply to Jon Mason

The PLL loop filter/gain can be located in a separate register on some SoCs. Split these off into a separate variable, so that an offset can be added if necessary. Also, make the necessary modifications to the Cygnus and NSP drivers for this change.

Signed-off-by Jon Mason <[email protected]>
---
 drivers/clk/bcm/clk-cygnus.c    | 17 +++++++++++------
 drivers/clk/bcm/clk-iproc-pll.c | 14 +++++++++-----
 drivers/clk/bcm/clk-iproc.h     | 10 +++++++++-
 drivers/clk/bcm/clk-nsp.c       | 14 +++++++++-----
 4 files changed, 38 insertions(+), 17 deletions(-)
drivers/clk/bcm/clk-cygnus.c +11 -6
index aac82c6..3a228b6 100644
@@ -34,9 +34,11 @@
 		{ .offset = o, .en_shift = es, .high_shift = hs, \
 		.high_width = hw, .low_shift = ls, .low_width = lw }
 
-#define RESET_VAL(o, rs, prs, kis, kiw, kps, kpw, kas, kaw) { .offset = o, \
-	.reset_shift = rs, .p_reset_shift = prs, .ki_shift = kis, \
-	.ki_width = kiw, .kp_shift = kps, .kp_width = kpw, .ka_shift = kas, \
+#define RESET_VAL(o, rs, prs) { .offset = o, .reset_shift = rs, \
+	.p_reset_shift = prs }
+
+#define DF_VAL(o, kis, kiw, kps, kpw, kas, kaw) { .offset = o, .ki_shift = kis,\
+	.ki_width = kiw, .kp_shift = kps, .kp_width = kpw, .ka_shift = kas,    \
 	.ka_width = kaw }
 
 #define VCO_CTRL_VAL(uo, lo) { .u_offset = uo, .l_offset = lo }

@@ -56,7 +58,8 @@ static const struct iproc_pll_ctrl genpll = {
 	.flags = IPROC_CLK_AON | IPROC_CLK_PLL_HAS_NDIV_FRAC |
 		IPROC_CLK_PLL_NEEDS_SW_CFG,
 	.aon = AON_VAL(0x0, 2, 1, 0),
-	.reset = RESET_VAL(0x0, 11, 10, 4, 3, 0, 4, 7, 3),
+	.reset = RESET_VAL(0x0, 11, 10),
+	.dig_filter = DF_VAL(0x0, 4, 3, 0, 4, 7, 3),
 	.sw_ctrl = SW_CTRL_VAL(0x10, 31),
 	.ndiv_int = REG_VAL(0x10, 20, 10),
 	.ndiv_frac = REG_VAL(0x10, 0, 20),

@@ -114,7 +117,8 @@ CLK_OF_DECLARE(cygnus_genpll, "brcm,cygnus-genpll", cygnus_genpll_clk_init);
 static const struct iproc_pll_ctrl lcpll0 = {
 	.flags = IPROC_CLK_AON | IPROC_CLK_PLL_NEEDS_SW_CFG,
 	.aon = AON_VAL(0x0, 2, 5, 4),
-	.reset = RESET_VAL(0x0, 31, 30, 27, 3, 23, 4, 19, 4),
+	.reset = RESET_VAL(0x0, 31, 30),
+	.dig_filter = DF_VAL(0x0, 27, 3, 23, 4, 19, 4),
 	.sw_ctrl = SW_CTRL_VAL(0x4, 31),
 	.ndiv_int = REG_VAL(0x4, 16, 10),
 	.pdiv = REG_VAL(0x4, 26, 4),

@@ -191,7 +195,8 @@ static const struct iproc_pll_ctrl mipipll = {
 		 IPROC_CLK_NEEDS_READ_BACK,
 	.aon = AON_VAL(0x0, 4, 17, 16),
 	.asiu = ASIU_GATE_VAL(0x0, 3),
-	.reset = RESET_VAL(0x0, 11, 10, 4, 3, 0, 4, 7, 4),
+	.reset = RESET_VAL(0x0, 11, 10),
+	.dig_filter = DF_VAL(0x0, 4, 3, 0, 4, 7, 4),
 	.ndiv_int = REG_VAL(0x10, 20, 10),
 	.ndiv_frac = REG_VAL(0x10, 0, 20),
 	.pdiv = REG_VAL(0x14, 0, 4),
drivers/clk/bcm/clk-iproc-pll.c +9 -5
index 74cdd66..d4aab4e 100644
@@ -224,13 +224,17 @@ static void __pll_bring_out_reset(struct iproc_pll *pll, unsigned int kp,
 	u32 val;
 	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
 	const struct iproc_pll_reset_ctrl *reset = &ctrl->reset;
+	const struct iproc_pll_dig_filter_ctrl *dig_filter = &ctrl->dig_filter;
+
+	val = readl(pll->pll_base + dig_filter->offset);
+	val &= ~(bit_mask(dig_filter->ki_width) << dig_filter->ki_shift |
+		bit_mask(dig_filter->kp_width) << dig_filter->kp_shift |
+		bit_mask(dig_filter->ka_width) << dig_filter->ka_shift);
+	val |= ki << dig_filter->ki_shift | kp << dig_filter->kp_shift |
+	       ka << dig_filter->ka_shift;
+	iproc_pll_write(pll, pll->pll_base, dig_filter->offset, val);
 
 	val = readl(pll->pll_base + reset->offset);
-	val &= ~(bit_mask(reset->ki_width) << reset->ki_shift |
-		 bit_mask(reset->kp_width) << reset->kp_shift |
-		 bit_mask(reset->ka_width) << reset->ka_shift);
-	val |=  ki << reset->ki_shift | kp << reset->kp_shift |
-		ka << reset->ka_shift;
 	val |= 1 << reset->reset_shift | 1 << reset->p_reset_shift;
 	iproc_pll_write(pll, pll->pll_base, reset->offset, val);
 }
drivers/clk/bcm/clk-iproc.h +9 -1
index ff7bfad..b71c197 100644
@@ -94,12 +94,19 @@ struct iproc_pll_aon_pwr_ctrl {
 };
 
 /*
- * Control of the PLL reset, with Ki, Kp, and Ka parameters
+ * Control of the PLL reset
  */
 struct iproc_pll_reset_ctrl {
 	unsigned int offset;
 	unsigned int reset_shift;
 	unsigned int p_reset_shift;
+};
+
+/*
+ * Control of the Ki, Kp, and Ka parameters
+ */
+struct iproc_pll_dig_filter_ctrl {
+	unsigned int offset;
 	unsigned int ki_shift;
 	unsigned int ki_width;
 	unsigned int kp_shift;

@@ -129,6 +136,7 @@ struct iproc_pll_ctrl {
 	struct iproc_pll_aon_pwr_ctrl aon;
 	struct iproc_asiu_gate asiu;
 	struct iproc_pll_reset_ctrl reset;
+	struct iproc_pll_dig_filter_ctrl dig_filter;
 	struct iproc_pll_sw_ctrl sw_ctrl;
 	struct iproc_clk_reg_op ndiv_int;
 	struct iproc_clk_reg_op ndiv_frac;
drivers/clk/bcm/clk-nsp.c +9 -5
index bc8ebdcd..cf66f64 100644
@@ -26,9 +26,11 @@
 #define AON_VAL(o, pw, ps, is) { .offset = o, .pwr_width = pw, \
 	.pwr_shift = ps, .iso_shift = is }
 
-#define RESET_VAL(o, rs, prs, kis, kiw, kps, kpw, kas, kaw) { .offset = o, \
-	.reset_shift = rs, .p_reset_shift = prs, .ki_shift = kis, \
-	.ki_width = kiw, .kp_shift = kps, .kp_width = kpw, .ka_shift = kas, \
+#define RESET_VAL(o, rs, prs) { .offset = o, .reset_shift = rs, \
+	.p_reset_shift = prs }
+
+#define DF_VAL(o, kis, kiw, kps, kpw, kas, kaw) { .offset = o, .ki_shift = kis,\
+	.ki_width = kiw, .kp_shift = kps, .kp_width = kpw, .ka_shift = kas,    \
 	.ka_width = kaw }
 
 #define ENABLE_VAL(o, es, hs, bs) { .offset = o, .enable_shift = es, \

@@ -43,7 +45,8 @@ CLK_OF_DECLARE(nsp_armpll, "brcm,nsp-armpll", nsp_armpll_init);
 static const struct iproc_pll_ctrl genpll = {
 	.flags = IPROC_CLK_PLL_HAS_NDIV_FRAC | IPROC_CLK_EMBED_PWRCTRL,
 	.aon = AON_VAL(0x0, 1, 12, 0),
-	.reset = RESET_VAL(0x0, 11, 10, 4, 3, 0, 4, 7, 3),
+	.reset = RESET_VAL(0x0, 11, 10),
+	.dig_filter = DF_VAL(0x0, 4, 3, 0, 4, 7, 3),
 	.ndiv_int = REG_VAL(0x14, 20, 10),
 	.ndiv_frac = REG_VAL(0x14, 0, 20),
 	.pdiv = REG_VAL(0x18, 24, 3),

@@ -99,7 +102,8 @@ CLK_OF_DECLARE(nsp_genpll_clk, "brcm,nsp-genpll", nsp_genpll_clk_init);
 static const struct iproc_pll_ctrl lcpll0 = {
 	.flags = IPROC_CLK_PLL_HAS_NDIV_FRAC | IPROC_CLK_EMBED_PWRCTRL,
 	.aon = AON_VAL(0x0, 1, 24, 0),
-	.reset = RESET_VAL(0x0, 23, 22, 16, 3, 12, 4, 19, 4),
+	.reset = RESET_VAL(0x0, 23, 22),
+	.dig_filter = DF_VAL(0x0, 16, 3, 12, 4, 19, 4),
 	.ndiv_int = REG_VAL(0x4, 20, 8),
 	.ndiv_frac = REG_VAL(0x4, 0, 20),
 	.pdiv = REG_VAL(0x4, 28, 3),
Signature · 1 line
1.9.1
author
#6 lore raw

PATCH v2 6/7 clk: iproc: Separate status and control variables

In reply to Jon Mason

Some PLLs have separate registers for Status and Control. The means the pll_base needs to be split into 2 new variables, so that those PLLs can specify device tree registers for those independently. Also, add a new driver flag to identify this presence of the split, and let the driver know that additional registers need to be used.

Signed-off-by Jon Mason <[email protected]>
---
 drivers/clk/bcm/clk-iproc-pll.c | 96 ++++++++++++++++++++++++-----------------
 drivers/clk/bcm/clk-iproc.h     |  6 +++
 2 files changed, 62 insertions(+), 40 deletions(-)
drivers/clk/bcm/clk-iproc-pll.c +56 -40
index d4aab4e..f128a9d 100644
@@ -74,7 +74,8 @@ struct iproc_clk {
 };
 
 struct iproc_pll {
-	void __iomem *pll_base;
+	void __iomem *status_base;
+	void __iomem *control_base;
 	void __iomem *pwr_base;
 	void __iomem *asiu_base;
 

@@ -127,7 +128,7 @@ static int pll_wait_for_lock(struct iproc_pll *pll)
 	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
 
 	for (i = 0; i < LOCK_DELAY; i++) {
-		u32 val = readl(pll->pll_base + ctrl->status.offset);
+		u32 val = readl(pll->status_base + ctrl->status.offset);
 
 		if (val & (1 << ctrl->status.shift))
 			return 0;

@@ -145,7 +146,7 @@ static void iproc_pll_write(const struct iproc_pll *pll, void __iomem *base,
 	writel(val, base + offset);
 
 	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK &&
-		     base == pll->pll_base))
+		     (base == pll->status_base || base == pll->control_base)))
 		val = readl(base + offset);
 }
 

@@ -161,9 +162,9 @@ static void __pll_disable(struct iproc_pll *pll)
 	}
 
 	if (ctrl->flags & IPROC_CLK_EMBED_PWRCTRL) {
-		val = readl(pll->pll_base + ctrl->aon.offset);
+		val = readl(pll->control_base + ctrl->aon.offset);
 		val |= (bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift);
-		iproc_pll_write(pll, pll->pll_base, ctrl->aon.offset, val);
+		iproc_pll_write(pll, pll->control_base, ctrl->aon.offset, val);
 	}
 
 	if (pll->pwr_base) {

@@ -184,9 +185,9 @@ static int __pll_enable(struct iproc_pll *pll)
 	u32 val;
 
 	if (ctrl->flags & IPROC_CLK_EMBED_PWRCTRL) {
-		val = readl(pll->pll_base + ctrl->aon.offset);
+		val = readl(pll->control_base + ctrl->aon.offset);
 		val &= ~(bit_mask(ctrl->aon.pwr_width) << ctrl->aon.pwr_shift);
-		iproc_pll_write(pll, pll->pll_base, ctrl->aon.offset, val);
+		iproc_pll_write(pll, pll->control_base, ctrl->aon.offset, val);
 	}
 
 	if (pll->pwr_base) {

@@ -213,9 +214,9 @@ static void __pll_put_in_reset(struct iproc_pll *pll)
 	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
 	const struct iproc_pll_reset_ctrl *reset = &ctrl->reset;
 
-	val = readl(pll->pll_base + reset->offset);
+	val = readl(pll->control_base + reset->offset);
 	val &= ~(1 << reset->reset_shift | 1 << reset->p_reset_shift);
-	iproc_pll_write(pll, pll->pll_base, reset->offset, val);
+	iproc_pll_write(pll, pll->control_base, reset->offset, val);
 }
 
 static void __pll_bring_out_reset(struct iproc_pll *pll, unsigned int kp,

@@ -226,17 +227,17 @@ static void __pll_bring_out_reset(struct iproc_pll *pll, unsigned int kp,
 	const struct iproc_pll_reset_ctrl *reset = &ctrl->reset;
 	const struct iproc_pll_dig_filter_ctrl *dig_filter = &ctrl->dig_filter;
 
-	val = readl(pll->pll_base + dig_filter->offset);
+	val = readl(pll->control_base + dig_filter->offset);
 	val &= ~(bit_mask(dig_filter->ki_width) << dig_filter->ki_shift |
 		bit_mask(dig_filter->kp_width) << dig_filter->kp_shift |
 		bit_mask(dig_filter->ka_width) << dig_filter->ka_shift);
 	val |= ki << dig_filter->ki_shift | kp << dig_filter->kp_shift |
 	       ka << dig_filter->ka_shift;
-	iproc_pll_write(pll, pll->pll_base, dig_filter->offset, val);
+	iproc_pll_write(pll, pll->control_base, dig_filter->offset, val);
 
-	val = readl(pll->pll_base + reset->offset);
+	val = readl(pll->control_base + reset->offset);
 	val |= 1 << reset->reset_shift | 1 << reset->p_reset_shift;
-	iproc_pll_write(pll, pll->pll_base, reset->offset, val);
+	iproc_pll_write(pll, pll->control_base, reset->offset, val);
 }
 
 static int pll_set_rate(struct iproc_clk *clk, unsigned int rate_index,

@@ -291,9 +292,9 @@ static int pll_set_rate(struct iproc_clk *clk, unsigned int rate_index,
 	/* put PLL in reset */
 	__pll_put_in_reset(pll);
 
-	iproc_pll_write(pll, pll->pll_base, ctrl->vco_ctrl.u_offset, 0);
+	iproc_pll_write(pll, pll->control_base, ctrl->vco_ctrl.u_offset, 0);
 
-	val = readl(pll->pll_base + ctrl->vco_ctrl.l_offset);
+	val = readl(pll->control_base + ctrl->vco_ctrl.l_offset);
 
 	if (rate >= VCO_LOW && rate < VCO_MID)
 		val |= (1 << PLL_VCO_LOW_SHIFT);

@@ -303,29 +304,29 @@ static int pll_set_rate(struct iproc_clk *clk, unsigned int rate_index,
 	else
 		val |= (1 << PLL_VCO_HIGH_SHIFT);
 
-	iproc_pll_write(pll, pll->pll_base, ctrl->vco_ctrl.l_offset, val);
+	iproc_pll_write(pll, pll->control_base, ctrl->vco_ctrl.l_offset, val);
 
 	/* program integer part of NDIV */
-	val = readl(pll->pll_base + ctrl->ndiv_int.offset);
+	val = readl(pll->control_base + ctrl->ndiv_int.offset);
 	val &= ~(bit_mask(ctrl->ndiv_int.width) << ctrl->ndiv_int.shift);
 	val |= vco->ndiv_int << ctrl->ndiv_int.shift;
-	iproc_pll_write(pll, pll->pll_base, ctrl->ndiv_int.offset, val);
+	iproc_pll_write(pll, pll->control_base, ctrl->ndiv_int.offset, val);
 
 	/* program fractional part of NDIV */
 	if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
-		val = readl(pll->pll_base + ctrl->ndiv_frac.offset);
+		val = readl(pll->control_base + ctrl->ndiv_frac.offset);
 		val &= ~(bit_mask(ctrl->ndiv_frac.width) <<
 			 ctrl->ndiv_frac.shift);
 		val |= vco->ndiv_frac << ctrl->ndiv_frac.shift;
-		iproc_pll_write(pll, pll->pll_base, ctrl->ndiv_frac.offset,
+		iproc_pll_write(pll, pll->control_base, ctrl->ndiv_frac.offset,
 				val);
 	}
 
 	/* program PDIV */
-	val = readl(pll->pll_base + ctrl->pdiv.offset);
+	val = readl(pll->control_base + ctrl->pdiv.offset);
 	val &= ~(bit_mask(ctrl->pdiv.width) << ctrl->pdiv.shift);
 	val |= vco->pdiv << ctrl->pdiv.shift;
-	iproc_pll_write(pll, pll->pll_base, ctrl->pdiv.offset, val);
+	iproc_pll_write(pll, pll->control_base, ctrl->pdiv.offset, val);
 
 	__pll_bring_out_reset(pll, kp, ka, ki);
 

@@ -372,7 +373,7 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
 		return 0;
 
 	/* PLL needs to be locked */
-	val = readl(pll->pll_base + ctrl->status.offset);
+	val = readl(pll->status_base + ctrl->status.offset);
 	if ((val & (1 << ctrl->status.shift)) == 0) {
 		clk->rate = 0;
 		return 0;

@@ -383,13 +384,13 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
 	 *
 	 * ((ndiv_int + ndiv_frac / 2^20) * (parent clock rate / pdiv)
 	 */
-	val = readl(pll->pll_base + ctrl->ndiv_int.offset);
+	val = readl(pll->control_base + ctrl->ndiv_int.offset);
 	ndiv_int = (val >> ctrl->ndiv_int.shift) &
 		bit_mask(ctrl->ndiv_int.width);
 	ndiv = (u64)ndiv_int << ctrl->ndiv_int.shift;
 
 	if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
-		val = readl(pll->pll_base + ctrl->ndiv_frac.offset);
+		val = readl(pll->control_base + ctrl->ndiv_frac.offset);
 		ndiv_frac = (val >> ctrl->ndiv_frac.shift) &
 			bit_mask(ctrl->ndiv_frac.width);
 

@@ -398,7 +399,7 @@ static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
 				ndiv_frac;
 	}
 
-	val = readl(pll->pll_base + ctrl->pdiv.offset);
+	val = readl(pll->control_base + ctrl->pdiv.offset);
 	pdiv = (val >> ctrl->pdiv.shift) & bit_mask(ctrl->pdiv.width);
 
 	clk->rate = (ndiv * parent_rate) >> ctrl->ndiv_int.shift;

@@ -463,14 +464,14 @@ static int iproc_clk_enable(struct clk_hw *hw)
 	u32 val;
 
 	/* channel enable is active low */
-	val = readl(pll->pll_base + ctrl->enable.offset);
+	val = readl(pll->control_base + ctrl->enable.offset);
 	val &= ~(1 << ctrl->enable.enable_shift);
-	iproc_pll_write(pll, pll->pll_base, ctrl->enable.offset, val);
+	iproc_pll_write(pll, pll->control_base, ctrl->enable.offset, val);
 
 	/* also make sure channel is not held */
-	val = readl(pll->pll_base + ctrl->enable.offset);
+	val = readl(pll->control_base + ctrl->enable.offset);
 	val &= ~(1 << ctrl->enable.hold_shift);
-	iproc_pll_write(pll, pll->pll_base, ctrl->enable.offset, val);
+	iproc_pll_write(pll, pll->control_base, ctrl->enable.offset, val);
 
 	return 0;
 }

@@ -485,9 +486,9 @@ static void iproc_clk_disable(struct clk_hw *hw)
 	if (ctrl->flags & IPROC_CLK_AON)
 		return;
 
-	val = readl(pll->pll_base + ctrl->enable.offset);
+	val = readl(pll->control_base + ctrl->enable.offset);
 	val |= 1 << ctrl->enable.enable_shift;
-	iproc_pll_write(pll, pll->pll_base, ctrl->enable.offset, val);
+	iproc_pll_write(pll, pll->control_base, ctrl->enable.offset, val);
 }
 
 static unsigned long iproc_clk_recalc_rate(struct clk_hw *hw,

@@ -502,7 +503,7 @@ static unsigned long iproc_clk_recalc_rate(struct clk_hw *hw,
 	if (parent_rate == 0)
 		return 0;
 
-	val = readl(pll->pll_base + ctrl->mdiv.offset);
+	val = readl(pll->control_base + ctrl->mdiv.offset);
 	mdiv = (val >> ctrl->mdiv.shift) & bit_mask(ctrl->mdiv.width);
 	if (mdiv == 0)
 		mdiv = 256;

@@ -549,14 +550,14 @@ static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	if (div > 256)
 		return -EINVAL;
 
-	val = readl(pll->pll_base + ctrl->mdiv.offset);
+	val = readl(pll->control_base + ctrl->mdiv.offset);
 	if (div == 256) {
 		val &= ~(bit_mask(ctrl->mdiv.width) << ctrl->mdiv.shift);
 	} else {
 		val &= ~(bit_mask(ctrl->mdiv.width) << ctrl->mdiv.shift);
 		val |= div << ctrl->mdiv.shift;
 	}
-	iproc_pll_write(pll, pll->pll_base, ctrl->mdiv.offset, val);
+	iproc_pll_write(pll, pll->control_base, ctrl->mdiv.offset, val);
 	clk->rate = parent_rate / div;
 
 	return 0;

@@ -581,9 +582,10 @@ static void iproc_pll_sw_cfg(struct iproc_pll *pll)
 	if (ctrl->flags & IPROC_CLK_PLL_NEEDS_SW_CFG) {
 		u32 val;
 
-		val = readl(pll->pll_base + ctrl->sw_ctrl.offset);
+		val = readl(pll->control_base + ctrl->sw_ctrl.offset);
 		val |= BIT(ctrl->sw_ctrl.shift);
-		iproc_pll_write(pll, pll->pll_base, ctrl->sw_ctrl.offset, val);
+		iproc_pll_write(pll, pll->control_base, ctrl->sw_ctrl.offset,
+				val);
 	}
 }
 

@@ -618,8 +620,8 @@ void __init iproc_pll_clk_setup(struct device_node *node,
 	if (WARN_ON(!pll->clks))
 		goto err_clks;
 
-	pll->pll_base = of_iomap(node, 0);
-	if (WARN_ON(!pll->pll_base))
+	pll->control_base = of_iomap(node, 0);
+	if (WARN_ON(!pll->control_base))
 		goto err_pll_iomap;
 
 	/* Some SoCs do not require the pwr_base, thus failing is not fatal */

@@ -632,6 +634,16 @@ void __init iproc_pll_clk_setup(struct device_node *node,
 			goto err_asiu_iomap;
 	}
 
+	if (pll_ctrl->flags & IPROC_CLK_PLL_SPLIT_STAT_CTRL) {
+		/* Some SoCs have a split status/control.  If this does not
+		 * exist, assume they are unified.
+		 */
+		pll->status_base = of_iomap(node, 2);
+		if (!pll->status_base)
+			goto err_status_iomap;
+	} else
+		pll->status_base = pll->control_base;
+
 	/* initialize and register the PLL itself */
 	pll->ctrl = pll_ctrl;
 

@@ -702,6 +714,10 @@ err_clk_register:
 		clk_unregister(pll->clk_data.clks[i]);
 
 err_pll_register:
+	if (pll->status_base != pll->control_base)
+		iounmap(pll->status_base);
+
+err_status_iomap:
 	if (pll->asiu_base)
 		iounmap(pll->asiu_base);
 

@@ -709,7 +725,7 @@ err_asiu_iomap:
 	if (pll->pwr_base)
 		iounmap(pll->pwr_base);
 
-	iounmap(pll->pll_base);
+	iounmap(pll->control_base);
 
 err_pll_iomap:
 	kfree(pll->clks);
drivers/clk/bcm/clk-iproc.h +6 -0
index b71c197..8988de7 100644
@@ -55,6 +55,12 @@
 #define IPROC_CLK_EMBED_PWRCTRL BIT(5)
 
 /*
+ * Some PLLs have separate registers for Status and Control.  Identify this to
+ * let the driver know if additional registers need to be used
+ */
+#define IPROC_CLK_PLL_SPLIT_STAT_CTRL BIT(6)
+
+/*
  * Parameters for VCO frequency configuration
  *
  * VCO frequency =
Signature · 1 line
1.9.1
author
#7 lore raw

PATCH v2 7/7 clk: ns2: add clock support for Broadcom Northstar 2 SoC

In reply to Jon Mason

The Broadcom Northstar 2 SoC is architected under the iProc architecture. It has the following PLLs: GENPLL SCR, GENPLL SW, LCPLL DDR, LCPLL Ports, all derived from an onboard crystal.

Signed-off-by Jon Mason <[email protected]>
---
 drivers/clk/Makefile                |   2 +-
 drivers/clk/bcm/Makefile            |   1 +
 drivers/clk/bcm/clk-ns2.c           | 288 ++++++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/bcm-ns2.h |  72 +++++++++
 4 files changed, 362 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/bcm/clk-ns2.c
 create mode 100644 include/dt-bindings/clock/bcm-ns2.h
drivers/clk/Makefile +1 -1
index d08b3e5..6124bd3 100644
@@ -47,7 +47,7 @@ obj-$(CONFIG_COMMON_CLK_WM831X)		+= clk-wm831x.o
 obj-$(CONFIG_COMMON_CLK_XGENE)		+= clk-xgene.o
 obj-$(CONFIG_COMMON_CLK_PWM)		+= clk-pwm.o
 obj-$(CONFIG_COMMON_CLK_AT91)		+= at91/
-obj-$(CONFIG_ARCH_BCM)			+= bcm/
+obj-y					+= bcm/
 obj-$(CONFIG_ARCH_BERLIN)		+= berlin/
 obj-$(CONFIG_ARCH_HISI)			+= hisilicon/
 obj-$(CONFIG_ARCH_MXC)			+= imx/
drivers/clk/bcm/Makefile +1 -0
index e258b28..2d1cbc5 100644
@@ -3,6 +3,7 @@ obj-$(CONFIG_CLK_BCM_KONA)	+= clk-kona-setup.o
 obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm281xx.o
 obj-$(CONFIG_CLK_BCM_KONA)	+= clk-bcm21664.o
 obj-$(CONFIG_COMMON_CLK_IPROC)	+= clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o
+obj-$(CONFIG_COMMON_CLK_IPROC)	+= clk-ns2.o
 obj-$(CONFIG_ARCH_BCM_CYGNUS)	+= clk-cygnus.o
 obj-$(CONFIG_ARCH_BCM_NSP)	+= clk-nsp.o
 obj-$(CONFIG_ARCH_BCM_5301X)	+= clk-nsp.o
drivers/clk/bcm/clk-ns2.c new file +288 -0

288 lines not on this page · raw message

include/dt-bindings/clock/bcm-ns2.h new file +72 -0

72 lines not on this page · raw message

Signature · 1 line
1.9.1
#8 lore raw

Re: [PATCH v2 7/7] clk: ns2: add clock support for Broadcom Northstar 2 SoC

drivers/clk/bcm/Makefile obj-$(CONFIG_CLK_BCM_KONA) += clk-kona-setup.o · line 9

Quoted 8 lines · On Tuesday 13 October 2015 17:08:05 Jon Mason wrote:
drivers/clk/bcm/Makefile +1 -0
@@ -3,6 +3,7 @@ obj-$(CONFIG_CLK_BCM_KONA)      += clk-kona-setup.o
 obj-$(CONFIG_CLK_BCM_KONA)     += clk-bcm281xx.o
 obj-$(CONFIG_CLK_BCM_KONA)     += clk-bcm21664.o
 obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o
+obj-$(CONFIG_COMMON_CLK_IPROC) += clk-ns2.o
 obj-$(CONFIG_ARCH_BCM_CYGNUS)  += clk-cygnus.o
 obj-$(CONFIG_ARCH_BCM_NSP)     += clk-nsp.o
 obj-$(CONFIG_ARCH_BCM_5301X)   += clk-nsp.o

If I understand this right, both CYGNUS and NS2 are IPROC based, but it looks like you now require building the clk-ns2 file for both?

On a related note, I'm seeing problems when CONFIG_CYGNUS is set but CONFIG_COMMON_CLK_IPROC is disabled, as that currently leads to a link failure.

Arnd

author
#9 lore raw

Re: [PATCH v2 7/7] clk: ns2: add clock support for Broadcom Northstar 2 SoC

Quoted 12 lines · Arnd Bergmann

drivers/clk/bcm/Makefile obj-$(CONFIG_CLK_BCM_KONA) += clk-kona-setup.o · line 9

Quoted 8 lines · On Tuesday 13 October 2015 17:08:05 Jon Mason wrote:
drivers/clk/bcm/Makefile +1 -0
@@ -3,6 +3,7 @@ obj-$(CONFIG_CLK_BCM_KONA)      += clk-kona-setup.o
 obj-$(CONFIG_CLK_BCM_KONA)     += clk-bcm281xx.o
 obj-$(CONFIG_CLK_BCM_KONA)     += clk-bcm21664.o
 obj-$(CONFIG_COMMON_CLK_IPROC) += clk-iproc-armpll.o clk-iproc-pll.o clk-iproc-asiu.o
+obj-$(CONFIG_COMMON_CLK_IPROC) += clk-ns2.o
 obj-$(CONFIG_ARCH_BCM_CYGNUS)  += clk-cygnus.o
 obj-$(CONFIG_ARCH_BCM_NSP)     += clk-nsp.o
 obj-$(CONFIG_ARCH_BCM_5301X)   += clk-nsp.o

If I understand this right, both CYGNUS and NS2 are IPROC based, but it looks like you now require building the clk-ns2 file for both?

There is no need for the NS2 clk code to be used by anything other than NS2. There is no unique CONFIG_ identifier for NS2 in the code that was accepted upstream. If I can add one for NS2, then I can split this off similar to Cygnus or NSP. If not, then it has to be lumped in with all of iProc. :(

Quoted 3 lines

On a related note, I'm seeing problems when CONFIG_CYGNUS is set but CONFIG_COMMON_CLK_IPROC is disabled, as that currently leads to a link failure.

I can double check, but it should be on by default when Cygnus is enabled. If you send me the error, I'll be happy to fix it.

Thanks,
Jon

Quoted 2 lines

Arnd

#10 lore raw

Re: [PATCH v2 7/7] clk: ns2: add clock support for Broadcom Northstar 2 SoC

Quoted 7 lines · On Tuesday 13 October 2015 18:04:50 Jon Mason wrote:
Quoted 4 lines

On a related note, I'm seeing problems when CONFIG_CYGNUS is set but CONFIG_COMMON_CLK_IPROC is disabled, as that currently leads to a link failure.

I can double check, but it should be on by default when Cygnus is enabled. If you send me the error, I'll be happy to fix it.

The problem is not that it's off by default but that it can be disabled, so it breaks some 'make randconfig' builds with this message:

drivers/built-in.o: In function `cygnus_armpll_init':
:(.init.text+0x1d290): undefined reference to iproc_armpll_setup' drivers/built-in.o: In function cygnus_genpll_clk_init':
:(.init.text+0x1d2c4): undefined reference to iproc_pll_clk_setup' drivers/built-in.o: In function cygnus_lcpll0_clk_init':
:(.init.text+0x1d304): undefined reference to iproc_pll_clk_setup' drivers/built-in.o: In function cygnus_mipipll_clk_init':
:(.init.text+0x1d344): undefined reference to iproc_pll_clk_setup' drivers/built-in.o: In function cygnus_asiu_init':
:(.init.text+0x1d370): undefined reference to `iproc_asiu_setup'

My patch fixes it by always selecting COMMON_CLK_IPROC from ARCH_BCM_CYGNUS. I wasn't sure whether you want COMMON_CLK_IPROC to still be user-selectable, so I left that in place. Normally I'd expect it to be a silent option though, that just gets implicitly enabled whenever a platform that needs it is built into the kernel.

Signed-off-by Arnd Bergmann <[email protected]>
arch/arm/mach-bcm/Kconfig +1 -0
index 1319c3c14327..35234e563cd8 100644
@@ -29,6 +29,7 @@ config ARCH_BCM_IPROC
 config ARCH_BCM_CYGNUS
 	bool "Broadcom Cygnus Support" if ARCH_MULTI_V7
 	select ARCH_BCM_IPROC
+	select COMMON_CLK_IPROC
 	help
 	  Enable support for the Cygnus family,
 	  which includes the following variants:
maintainer Acked-by
#11 lore raw

Re: [PATCH v2 7/7] clk: ns2: add clock support for Broadcom Northstar 2 SoC

Quoted 25 lines · Arnd Bergmann
Quoted 7 lines · On Tuesday 13 October 2015 18:04:50 Jon Mason wrote:
Quoted 4 lines

On a related note, I'm seeing problems when CONFIG_CYGNUS is set but CONFIG_COMMON_CLK_IPROC is disabled, as that currently leads to a link failure.

I can double check, but it should be on by default when Cygnus is enabled. If you send me the error, I'll be happy to fix it.

The problem is not that it's off by default but that it can be disabled, so it breaks some 'make randconfig' builds with this message:

drivers/built-in.o: In function `cygnus_armpll_init':
:(.init.text+0x1d290): undefined reference to iproc_armpll_setup' drivers/built-in.o: In function cygnus_genpll_clk_init':
:(.init.text+0x1d2c4): undefined reference to iproc_pll_clk_setup' drivers/built-in.o: In function cygnus_lcpll0_clk_init':
:(.init.text+0x1d304): undefined reference to iproc_pll_clk_setup' drivers/built-in.o: In function cygnus_mipipll_clk_init':
:(.init.text+0x1d344): undefined reference to iproc_pll_clk_setup' drivers/built-in.o: In function cygnus_asiu_init':
:(.init.text+0x1d370): undefined reference to `iproc_asiu_setup'

Quoted 6 lines

My patch fixes it by always selecting COMMON_CLK_IPROC from ARCH_BCM_CYGNUS. I wasn't sure whether you want COMMON_CLK_IPROC to still be user-selectable, so I left that in place. Normally I'd expect it to be a silent option though, that just gets implicitly enabled whenever a platform that needs it is built into the kernel.

COMMON_CLK_IPROC can be a silent option and selected as Arnd has done below.

Quoted 2 lines
Signed-off-by Arnd Bergmann <[email protected]>
Acked-by Scott Branden <[email protected]>
Quoted 14 lines
arch/arm/mach-bcm/Kconfig +1 -0
index 1319c3c14327..35234e563cd8 100644
@@ -29,6 +29,7 @@ config ARCH_BCM_IPROC
  config ARCH_BCM_CYGNUS
  	bool "Broadcom Cygnus Support" if ARCH_MULTI_V7
  	select ARCH_BCM_IPROC
+	select COMMON_CLK_IPROC
  	help
  	  Enable support for the Cygnus family,
  	  which includes the following variants:
#12 lore raw

Re: [PATCH v2 7/7] clk: ns2: add clock support for Broadcom Northstar 2 SoC

Quoted 8 lines · On Wednesday 14 October 2015 12:46:19 Scott Branden wrote:
Quoted 7 lines

My patch fixes it by always selecting COMMON_CLK_IPROC from ARCH_BCM_CYGNUS. I wasn't sure whether you want COMMON_CLK_IPROC to still be user-selectable, so I left that in place. Normally I'd expect it to be a silent option though, that just gets implicitly enabled whenever a platform that needs it is built into the kernel.

COMMON_CLK_IPROC can be a silent option and selected as Arnd has done below.

Ok, please do that then. If you want to apply my patch directly, you can remove that last paragraph from the description.

Quoted 2 lines
Quoted 1 line
Signed-off-by Arnd Bergmann <[email protected]>
Acked-by Scott Branden <[email protected]>

Thanks for taking a look!

Arnd

maintainer
#13 lore raw

Re: [PATCH v2 7/7] clk: ns2: add clock support for Broadcom Northstar 2 SoC

Jon,

Can you add this to your patchset and change COMMON_CLK_IPROC to a silent option?

Quoted 20 lines · Arnd Bergmann
Quoted 8 lines · On Wednesday 14 October 2015 12:46:19 Scott Branden wrote:
Quoted 7 lines

My patch fixes it by always selecting COMMON_CLK_IPROC from ARCH_BCM_CYGNUS. I wasn't sure whether you want COMMON_CLK_IPROC to still be user-selectable, so I left that in place. Normally I'd expect it to be a silent option though, that just gets implicitly enabled whenever a platform that needs it is built into the kernel.

COMMON_CLK_IPROC can be a silent option and selected as Arnd has done below.

Ok, please do that then. If you want to apply my patch directly, you can remove that last paragraph from the description.

Quoted 2 lines
Quoted 1 line
Signed-off-by Arnd Bergmann <[email protected]>
Acked-by Scott Branden <[email protected]>

Thanks for taking a look!

Arnd

author
#14 lore raw

Re: [PATCH v2 7/7] clk: ns2: add clock support for Broadcom Northstar 2 SoC

Quoted 4 lines · Scott Branden

Jon,

Can you add this to your patchset and change COMMON_CLK_IPROC to a silent option?

Sure, I'll add this (and make it silent), as well as adding the documentation patches back to this series (per Ray's suggestion), and will resend.

Thanks,
Jon

Quoted 23 lines · Arnd Bergmann
Quoted 20 lines · On Wednesday 14 October 2015 12:46:19 Scott Branden wrote:
Quoted 8 lines
Quoted 7 lines

My patch fixes it by always selecting COMMON_CLK_IPROC from ARCH_BCM_CYGNUS. I wasn't sure whether you want COMMON_CLK_IPROC to still be user-selectable, so I left that in place. Normally I'd expect it to be a silent option though, that just gets implicitly enabled whenever a platform that needs it is built into the kernel.

COMMON_CLK_IPROC can be a silent option and selected as Arnd has done below.

Ok, please do that then. If you want to apply my patch directly, you can remove that last paragraph from the description.

Quoted 2 lines
Quoted 1 line
Signed-off-by Arnd Bergmann <[email protected]>
Acked-by Scott Branden <[email protected]>

Thanks for taking a look!

Arnd

#15 lore raw

Re: [PATCH v2 7/7] clk: ns2: add clock support for Broadcom Northstar 2 SoC

Quoted 10 lines · On Thursday 15 October 2015 14:43:45 Jon Mason wrote:
Quoted 4 lines · Scott Branden

Jon,

Can you add this to your patchset and change COMMON_CLK_IPROC to a silent option?

Sure, I'll add this (and make it silent), as well as adding the documentation patches back to this series (per Ray's suggestion), and will resend.

Thanks!

Arnd

author
#16 lore raw

Re: [PATCH v2 7/7] clk: ns2: add clock support for Broadcom Northstar 2 SoC

Quoted 7 lines · Arnd Bergmann
Quoted 6 lines · On Thursday 15 October 2015 14:43:45 Jon Mason wrote:
Quoted 4 lines · Scott Branden

Jon,

Can you add this to your patchset and change COMMON_CLK_IPROC to a silent option?

Quoted 8 lines
Quoted 4 lines

Sure, I'll add this (and make it silent), as well as adding the documentation patches back to this series (per Ray's suggestion), and will resend.

Thanks!

Arnd

I'm tweaking the patch slightly. So, I'll remove Scott's ack and let him re-ack if he still likes it.

I moved the selection of the COMMON_CLK_IPROC to ARCH_BCM_IPROC, as all iProc SoCs will soon have clk drivers (and enabling it without a SoC driver won't hurt anything). Also, the slient enable option is added.

Thanks,
Jon

author
#17 lore raw
In reply to Jon Mason

Oops, this should be 0/7 not 0/10

Quoted 18 lines · Jon Mason

v2 - Added a patch to change the Cygnus clk macros to uppercase, removed the device tree changes from the series (will send out those as an RFC), and other minor changes that Stephen Boyd requested.

This patch series adds support for the Broadcom Northstar, Northstar Plus, and Northstar 2 clocks. Some slight modifications were necessary to clk-iproc-pll to get Northstar and Northstar Plus working, due to differences in register layout. This is the reason why the first patch is necessary. Some more modifications were necessary to clk-iproc-pll to get Northstar 2 working, due to differences in register layout (and resulting fallout in Cygnus and NSP). This is the reason why the sixth and seventh patches are necessary. The fifth patch is clean-up to prevent accidentally forgetting to adjust for the base write errata (which happened a few times, but was caught in internal review).

Thanks,
Jon