linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture
@ 2015-03-18  5:45 Ray Jui
  2015-03-18  5:45 ` [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding Ray Jui
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Ray Jui @ 2015-03-18  5:45 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset contains the initial common clock support for Broadcom's iProc
family of SoCs. The iProc clock architecture comprises of various PLLs, e.g.,
ARMPLL, GENPLL, LCPLL0, MIPIPLL, and etc. An onboard crystal serves as the
basic reference clock for these PLLs. Each PLL may have several leaf clocks.
One special group of clocks is the ASIU clocks, which are dervied directly
from the crystal reference clock.

This patchset also contains the basic clock support for the Broadcom Cygnus
SoC, which implements the iProc clock architecture

Changes from v5:
 - Rebase to v4.0-rc4
 - Drop of_clk_get_parent_rate helper function from the clock framework
 - Get rid of custom "clock-frequency" support in iProc PLL code. Instead, add
   standard clock set_rate and round_rate support and make use of DT properties
   "assigned-clocks" and "assigned-clock-rates" to initialize PLL to the
   desired rate when registering to the clock framework
 - Add SW workaround for ASIC bug on MIPI PLL to always read back the same
   register following a write transaction, to ensure value is written to the
   correct register

Changes from v4:
 - Add of_clk_get_parent_rate helper function into the clock framework
 - Switch to use of_clk_get_parent_rate in the iProc PLL clock driver

Changes from v3:
 - Fix incorrect use of passing in of_clk_src_onecell_get when adding ARM PLL
   and other iProc PLLs as clock provider. These PLLs have zero cells in DT and
   thefore of_clk_src_simple_get should be used instead
 - Rename Cygnus MIPI PLL Channel 2 clock from BCM_CYGNUS_MIPIPLL_CH2_UNUSED
   to BCM_CYGNUS_MIPIPLL_CH2_V3D, since a 3D graphic rendering engine has been
   integrated into Cygnus revision B0 and has its core clock running off
   MIPI PLL Channel 2
 - Changed default MIPI PLL VCO frequency from 1.75 GHz to 2.1 GHz. This allows
   us to derive 300 MHz V3D clock from channel 2 through the post divisor

Changes from v2:
 - Re-arrange Cygnus clock/pll init functions so each init function is right
   next to its clock table
 - Removed #defines for number of clocks in Cygnus. Have the number of clocks
   automatically determined based on array size of the clock table

Changes from v1:
 - Separate drivers/clk/Makefile change for drivers/clk/bcm out to a standalone patch

Ray Jui (6):
  clk: iproc: define Broadcom iProc clock binding
  clk: iproc: add initial common clock support
  clk: Change bcm clocks build dependency
  clk: cygnus: add clock support for Broadcom Cygnus
  ARM: dts: enable clock support for Broadcom Cygnus
  clk: cygnus: remove Cygnus dummy clock binding

 .../devicetree/bindings/clock/bcm-cygnus-clock.txt |   34 --
 .../bindings/clock/brcm,iproc-clocks.txt           |  171 +++++++
 arch/arm/boot/dts/bcm-cygnus-clock.dtsi            |  112 ++++-
 arch/arm/boot/dts/bcm-cygnus.dtsi                  |    2 +-
 drivers/clk/Makefile                               |    2 +-
 drivers/clk/bcm/Kconfig                            |    9 +
 drivers/clk/bcm/Makefile                           |    2 +
 drivers/clk/bcm/clk-cygnus.c                       |  284 ++++++++++++
 drivers/clk/bcm/clk-iproc-armpll.c                 |  282 +++++++++++
 drivers/clk/bcm/clk-iproc-asiu.c                   |  275 +++++++++++
 drivers/clk/bcm/clk-iproc-clk.c                    |  244 ++++++++++
 drivers/clk/bcm/clk-iproc-pll.c                    |  490 ++++++++++++++++++++
 drivers/clk/bcm/clk-iproc.h                        |  164 +++++++
 include/dt-bindings/clock/bcm-cygnus.h             |   65 +++
 14 files changed, 2075 insertions(+), 61 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/bcm-cygnus-clock.txt
 create mode 100644 Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
 create mode 100644 drivers/clk/bcm/clk-cygnus.c
 create mode 100644 drivers/clk/bcm/clk-iproc-armpll.c
 create mode 100644 drivers/clk/bcm/clk-iproc-asiu.c
 create mode 100644 drivers/clk/bcm/clk-iproc-clk.c
 create mode 100644 drivers/clk/bcm/clk-iproc-pll.c
 create mode 100644 drivers/clk/bcm/clk-iproc.h
 create mode 100644 include/dt-bindings/clock/bcm-cygnus.h

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding
  2015-03-18  5:45 [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Ray Jui
@ 2015-03-18  5:45 ` Ray Jui
  2015-04-11  0:12   ` Michael Turquette
  2015-03-18  5:45 ` [PATCH v6 2/6] clk: iproc: add initial common clock support Ray Jui
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Ray Jui @ 2015-03-18  5:45 UTC (permalink / raw)
  To: linux-arm-kernel

Document the device tree binding for Broadcom iProc architecture based
clock controller

Signed-off-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 .../bindings/clock/brcm,iproc-clocks.txt           |  171 ++++++++++++++++++++
 1 file changed, 171 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt

diff --git a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
new file mode 100644
index 0000000..bf2316b
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
@@ -0,0 +1,171 @@
+Broadcom iProc Family Clocks
+
+This binding uses the common clock binding:
+    Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+The iProc clock controller manages clocks that are common to the iProc family.
+An SoC from the iProc family may have several PPLs, e.g., ARMPLL, GENPLL,
+LCPLL0, MIPIPLL, and etc., all derived from an onboard crystal. Each PLL
+comprises of several leaf clocks
+
+Required properties for PLLs:
+- compatible:
+    Should have a value of the form "brcm,<soc>-<pll>". For example, GENPLL on
+Cygnus has a compatible string of "brcm,cygnus-genpll"
+
+- #clock-cells:
+    Must be <0>
+
+- reg:
+    Define the base and range of the I/O address space that contain the iProc
+clock control registers required for the PLL
+
+- clocks:
+    The input parent clock phandle for the PLL. For all iProc PLLs, this is an
+onboard crystal with a fixed rate
+
+Example:
+
+	osc: oscillator {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <25000000>;
+	};
+
+	genpll: genpll {
+		#clock-cells = <0>;
+		compatible = "brcm,cygnus-genpll";
+		reg = <0x0301d000 0x2c>,
+			<0x0301c020 0x4>;
+		clocks = <&osc>;
+	};
+
+Required properties for leaf clocks of a PLL:
+
+- compatible:
+    Should have a value of the form "brcm,<soc>-<pll>-clk". For example, leaf
+clocks derived from the GENPLL on Cygnus SoC have a compatible string of
+"brcm,cygnus-genpll-clk"
+
+- #clock-cells:
+    Have a value of <1> since there are more than 1 leaf clock of a
+given PLL
+
+- reg:
+    Define the base and range of the I/O address space that contain the iProc
+clock control registers required for the PLL leaf clocks
+
+- clocks:
+    The input parent PLL phandle for the leaf clock
+
+- clock-output-names:
+    An ordered list of strings defining the names of the leaf clocks
+
+Example:
+
+	genpll: genpll {
+		#clock-cells = <0>;
+		compatible = "brcm,cygnus-genpll";
+		reg = <0x0301d000 0x2c>,
+			<0x0301c020 0x4>;
+		clocks = <&osc>;
+	};
+
+	genpll_clks: genpll_clks {
+		#clock-cells = <1>;
+		compatible = "brcm,cygnus-genpll-clk";
+		reg = <0x0301d000 0x2c>;
+		clocks = <&genpll>;
+		clock-output-names = "axi21", "250mhz", "ihost_sys",
+			"enet_sw", "audio_125", "can";
+	};
+
+Required properties for ASIU clocks:
+
+ASIU clocks are a special case. These clocks are derived directly from the
+reference clock of the onboard crystal
+
+- compatible:
+    Should have a value of the form "brcm,<soc>-asiu-clk". For example, ASIU
+clocks for Cygnus have a compatible string of "brcm,cygnus-asiu-clk"
+
+- #clock-cells:
+    Have a value of <1> since there are more than 1 ASIU clocks
+
+- reg:
+    Define the base and range of the I/O address space that contain the iProc
+clock control registers required for ASIU clocks
+
+- clocks:
+    The input parent clock phandle for the ASIU clock, i.e., the onboard
+crystal
+
+- clock-output-names:
+    An ordered list of strings defining the names of the ASIU clocks
+
+Example:
+
+	osc: oscillator {
+		#clock-cells = <0>;
+		compatible = "fixed-clock";
+		clock-frequency = <25000000>;
+	};
+
+	asiu_clks: asiu_clks {
+		#clock-cells = <1>;
+		compatible = "brcm,cygnus-asiu-clk";
+		reg = <0x0301d048 0xc>,
+			<0x180aa024 0x4>;
+		clocks = <&osc>;
+		clock-output-names = "keypad", "adc/touch", "pwm";
+	};
+
+Cygnus
+------
+PLL and leaf clock compatible strings for Cygnus are:
+    "brcm,cygnus-armpll"
+    "brcm,cygnus-genpll"
+    "brcm,cygnus-lcpll0"
+    "brcm,cygnus-mipipll"
+    "brcm,cygnus-genpll-clk"
+    "brcm,cygnus-lcpll0-clk"
+    "brcm,cygnus-mipipll-clk"
+    "brcm,cygnus-asiu-clk"
+
+The following table defines the set of PLL/clock index and ID for Cygnus.
+These clock IDs are defined in:
+    "include/dt-bindings/clock/bcm-cygnus.h"
+
+    Clock      Source           Index   ID
+    ---        -----            -----   ---------
+    crystal    N/A              N/A     N/A
+
+    armpll     crystal          N/A     N/A
+    genpll     crystal          N/A     N/A
+    lcpll0     crystal          N/A     N/A
+    mipipll    crystal          N/A     N/A
+
+    keypad     crystal (ASIU)   0       BCM_CYGNUS_ASIU_KEYPAD_CLK
+    adc/tsc    crystal (ASIU)   1       BCM_CYGNUS_ASIU_ADC_CLK
+    pwm        crystal (ASIU)   2       BCM_CYGNUS_ASIU_PWM_CLK
+
+    axi21      genpll           0       BCM_CYGNUS_GENPLL_AXI21_CLK
+    250mhz     genpll           1       BCM_CYGNUS_GENPLL_250MHZ_CLK
+    ihost_sys  genpll           2       BCM_CYGNUS_GENPLL_IHOST_SYS_CLK
+    enet_sw    genpll           3       BCM_CYGNUS_GENPLL_ENET_SW_CLK
+    audio_125  genpll           4       BCM_CYGNUS_GENPLL_AUDIO_125_CLK
+    can        genpll           5       BCM_CYGNUS_GENPLL_CAN_CLK
+
+    pcie_phy   lcpll0           0       BCM_CYGNUS_LCPLL0_PCIE_PHY_REF_CLK
+    ddr_phy    lcpll0           1       BCM_CYGNUS_LCPLL0_DDR_PHY_CLK
+    sdio       lcpll0           2       BCM_CYGNUS_LCPLL0_SDIO_CLK
+    usb_phy    lcpll0           3       BCM_CYGNUS_LCPLL0_USB_PHY_REF_CLK
+    smart_card lcpll0           4       BCM_CYGNUS_LCPLL0_SMART_CARD_CLK
+    ch5_unused lcpll0           5       BCM_CYGNUS_LCPLL0_CH5_UNUSED
+
+    ch0_unused mipipll          0       BCM_CYGNUS_MIPIPLL_CH0_UNUSED
+    ch1_lcd    mipipll          1       BCM_CYGNUS_MIPIPLL_CH1_LCD
+    ch2_v3d    mipipll          2       BCM_CYGNUS_MIPIPLL_CH2_V3D
+    ch3_unused mipipll          3       BCM_CYGNUS_MIPIPLL_CH3_UNUSED
+    ch4_unused mipipll          4       BCM_CYGNUS_MIPIPLL_CH4_UNUSED
+    ch5_unused mipipll          5       BCM_CYGNUS_MIPIPLL_CH5_UNUSED
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 2/6] clk: iproc: add initial common clock support
  2015-03-18  5:45 [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Ray Jui
  2015-03-18  5:45 ` [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding Ray Jui
@ 2015-03-18  5:45 ` Ray Jui
  2015-03-18  5:45 ` [PATCH v6 3/6] clk: Change bcm clocks build dependency Ray Jui
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Ray Jui @ 2015-03-18  5:45 UTC (permalink / raw)
  To: linux-arm-kernel

This adds basic and generic support for various iProc PLLs and clocks
including the ARMPLL, GENPLL, LCPLL, MIPIPLL, and ASIU clocks.

SoCs under the iProc architecture can define their specific register
offsets and clock parameters for their PLL and clock controllers. These
parameters can be passed as arugments into the generic iProc PLL and
clock setup functions

Derived from code originally provided by Jonathan Richardson
<jonathar@broadcom.com>

Signed-off-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 drivers/clk/bcm/Kconfig            |    9 +
 drivers/clk/bcm/Makefile           |    1 +
 drivers/clk/bcm/clk-iproc-armpll.c |  282 +++++++++++++++++++++
 drivers/clk/bcm/clk-iproc-asiu.c   |  275 ++++++++++++++++++++
 drivers/clk/bcm/clk-iproc-clk.c    |  244 ++++++++++++++++++
 drivers/clk/bcm/clk-iproc-pll.c    |  490 ++++++++++++++++++++++++++++++++++++
 drivers/clk/bcm/clk-iproc.h        |  164 ++++++++++++
 7 files changed, 1465 insertions(+)
 create mode 100644 drivers/clk/bcm/clk-iproc-armpll.c
 create mode 100644 drivers/clk/bcm/clk-iproc-asiu.c
 create mode 100644 drivers/clk/bcm/clk-iproc-clk.c
 create mode 100644 drivers/clk/bcm/clk-iproc-pll.c
 create mode 100644 drivers/clk/bcm/clk-iproc.h

diff --git a/drivers/clk/bcm/Kconfig b/drivers/clk/bcm/Kconfig
index 75506e5..131a3af 100644
--- a/drivers/clk/bcm/Kconfig
+++ b/drivers/clk/bcm/Kconfig
@@ -7,3 +7,12 @@ config CLK_BCM_KONA
 	  Enable common clock framework support for Broadcom SoCs
 	  using "Kona" style clock control units, including those
 	  in the BCM281xx and BCM21664 families.
+
+config COMMON_CLK_IPROC
+	bool "Broadcom iProc clock support"
+	depends on ARCH_BCM_IPROC
+	depends on COMMON_CLK
+	default ARCH_BCM_IPROC
+	help
+	  Enable common clock framework support for Broadcom SoCs
+	  based on the "iProc" architecture
diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index 6297d05..6926636 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -2,3 +2,4 @@ obj-$(CONFIG_CLK_BCM_KONA)	+= clk-kona.o
 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-clk.o clk-iproc-asiu.o
diff --git a/drivers/clk/bcm/clk-iproc-armpll.c b/drivers/clk/bcm/clk-iproc-armpll.c
new file mode 100644
index 0000000..965cd4e
--- /dev/null
+++ b/drivers/clk/bcm/clk-iproc-armpll.c
@@ -0,0 +1,282 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/of_address.h>
+
+#define IPROC_CLK_MAX_FREQ_POLICY                    0x3
+#define IPROC_CLK_POLICY_FREQ_OFFSET                 0x008
+#define IPROC_CLK_POLICY_FREQ_POLICY_FREQ_SHIFT      8
+#define IPROC_CLK_POLICY_FREQ_POLICY_FREQ_MASK       0x7
+
+#define IPROC_CLK_PLLARMA_OFFSET                     0xc00
+#define IPROC_CLK_PLLARMA_LOCK_SHIFT                 28
+#define IPROC_CLK_PLLARMA_PDIV_SHIFT                 24
+#define IPROC_CLK_PLLARMA_PDIV_MASK                  0xf
+#define IPROC_CLK_PLLARMA_NDIV_INT_SHIFT             8
+#define IPROC_CLK_PLLARMA_NDIV_INT_MASK              0x3ff
+
+#define IPROC_CLK_PLLARMB_OFFSET                     0xc04
+#define IPROC_CLK_PLLARMB_NDIV_FRAC_MASK             0xfffff
+
+#define IPROC_CLK_PLLARMC_OFFSET                     0xc08
+#define IPROC_CLK_PLLARMC_BYPCLK_EN_SHIFT            8
+#define IPROC_CLK_PLLARMC_MDIV_MASK                  0xff
+
+#define IPROC_CLK_PLLARMCTL5_OFFSET                  0xc20
+#define IPROC_CLK_PLLARMCTL5_H_MDIV_MASK             0xff
+
+#define IPROC_CLK_PLLARM_OFFSET_OFFSET               0xc24
+#define IPROC_CLK_PLLARM_SW_CTL_SHIFT                29
+#define IPROC_CLK_PLLARM_NDIV_INT_OFFSET_SHIFT       20
+#define IPROC_CLK_PLLARM_NDIV_INT_OFFSET_MASK        0xff
+#define IPROC_CLK_PLLARM_NDIV_FRAC_OFFSET_MASK       0xfffff
+
+#define IPROC_CLK_ARM_DIV_OFFSET                     0xe00
+#define IPROC_CLK_ARM_DIV_PLL_SELECT_OVERRIDE_SHIFT  4
+#define IPROC_CLK_ARM_DIV_ARM_PLL_SELECT_MASK        0xf
+
+#define IPROC_CLK_POLICY_DBG_OFFSET                  0xec0
+#define IPROC_CLK_POLICY_DBG_ACT_FREQ_SHIFT          12
+#define IPROC_CLK_POLICY_DBG_ACT_FREQ_MASK           0x7
+
+enum iproc_arm_pll_fid {
+	ARM_PLL_FID_CRYSTAL_CLK   = 0,
+	ARM_PLL_FID_SYS_CLK       = 2,
+	ARM_PLL_FID_CH0_SLOW_CLK  = 6,
+	ARM_PLL_FID_CH1_FAST_CLK  = 7
+};
+
+struct iproc_arm_pll {
+	struct clk_hw hw;
+	void __iomem *base;
+	unsigned long rate;
+};
+
+#define to_iproc_arm_pll(hw) container_of(hw, struct iproc_arm_pll, hw)
+
+static unsigned int __get_fid(struct iproc_arm_pll *pll)
+{
+	u32 val;
+	unsigned int policy, fid, active_fid;
+
+	val = readl(pll->base + IPROC_CLK_ARM_DIV_OFFSET);
+	if (val & (1 << IPROC_CLK_ARM_DIV_PLL_SELECT_OVERRIDE_SHIFT))
+		policy = val & IPROC_CLK_ARM_DIV_ARM_PLL_SELECT_MASK;
+	else
+		policy = 0;
+
+	/* something is seriously wrong */
+	BUG_ON(policy > IPROC_CLK_MAX_FREQ_POLICY);
+
+	val = readl(pll->base + IPROC_CLK_POLICY_FREQ_OFFSET);
+	fid = (val >> (IPROC_CLK_POLICY_FREQ_POLICY_FREQ_SHIFT * policy)) &
+		IPROC_CLK_POLICY_FREQ_POLICY_FREQ_MASK;
+
+	val = readl(pll->base + IPROC_CLK_POLICY_DBG_OFFSET);
+	active_fid = IPROC_CLK_POLICY_DBG_ACT_FREQ_MASK &
+		(val >> IPROC_CLK_POLICY_DBG_ACT_FREQ_SHIFT);
+	if (fid != active_fid) {
+		pr_debug("%s: fid override %u->%u\n", __func__,	fid,
+				active_fid);
+		fid = active_fid;
+	}
+
+	pr_debug("%s: active fid: %u\n", __func__, fid);
+
+	return fid;
+}
+
+/*
+ * Determine the mdiv (post divider) based on the frequency ID being used.
+ * There are 4 sources that can be used to derive the output clock rate:
+ *    - 25 MHz Crystal
+ *    - System clock
+ *    - PLL channel 0 (slow clock)
+ *    - PLL channel 1 (fast clock)
+ */
+static int __get_mdiv(struct iproc_arm_pll *pll)
+{
+	unsigned int fid;
+	int mdiv;
+	u32 val;
+
+	fid = __get_fid(pll);
+
+	switch (fid) {
+	case ARM_PLL_FID_CRYSTAL_CLK:
+	case ARM_PLL_FID_SYS_CLK:
+		mdiv = 1;
+		break;
+
+	case ARM_PLL_FID_CH0_SLOW_CLK:
+		val = readl(pll->base + IPROC_CLK_PLLARMC_OFFSET);
+		mdiv = val & IPROC_CLK_PLLARMC_MDIV_MASK;
+		if (mdiv == 0)
+			mdiv = 256;
+		break;
+
+	case ARM_PLL_FID_CH1_FAST_CLK:
+		val = readl(pll->base +	IPROC_CLK_PLLARMCTL5_OFFSET);
+		mdiv = val & IPROC_CLK_PLLARMCTL5_H_MDIV_MASK;
+		if (mdiv == 0)
+			mdiv = 256;
+		break;
+
+	default:
+		mdiv = -EFAULT;
+	}
+
+	return mdiv;
+}
+
+static unsigned int __get_ndiv(struct iproc_arm_pll *pll)
+{
+	u32 val;
+	unsigned int ndiv_int, ndiv_frac, ndiv;
+
+	val = readl(pll->base + IPROC_CLK_PLLARM_OFFSET_OFFSET);
+	if (val & (1 << IPROC_CLK_PLLARM_SW_CTL_SHIFT)) {
+		/*
+		 * offset mode is active. Read the ndiv from the PLLARM OFFSET
+		 * register
+		 */
+		ndiv_int = (val >> IPROC_CLK_PLLARM_NDIV_INT_OFFSET_SHIFT) &
+			IPROC_CLK_PLLARM_NDIV_INT_OFFSET_MASK;
+		if (ndiv_int == 0)
+			ndiv_int = 256;
+
+		ndiv_frac = val & IPROC_CLK_PLLARM_NDIV_FRAC_OFFSET_MASK;
+	} else {
+		/* offset mode not active */
+		val = readl(pll->base + IPROC_CLK_PLLARMA_OFFSET);
+		ndiv_int = (val >> IPROC_CLK_PLLARMA_NDIV_INT_SHIFT) &
+			IPROC_CLK_PLLARMA_NDIV_INT_MASK;
+		if (ndiv_int == 0)
+			ndiv_int = 1024;
+
+		val = readl(pll->base + IPROC_CLK_PLLARMB_OFFSET);
+		ndiv_frac = val & IPROC_CLK_PLLARMB_NDIV_FRAC_MASK;
+	}
+
+	ndiv = (ndiv_int << 20) | ndiv_frac;
+
+	return ndiv;
+}
+
+/*
+ * The output frequency of the ARM PLL is calculated based on the ARM PLL
+ * divider values:
+ *   pdiv = ARM PLL pre-divider
+ *   ndiv = ARM PLL multiplier
+ *   mdiv = ARM PLL post divider
+ *
+ * The frequency is calculated by:
+ *   ((ndiv * parent clock rate) / pdiv) / mdiv
+ */
+static unsigned long iproc_arm_pll_recalc_rate(struct clk_hw *hw,
+		unsigned long parent_rate)
+{
+	struct iproc_arm_pll *pll = to_iproc_arm_pll(hw);
+	u32 val;
+	int mdiv;
+	u64 ndiv;
+	unsigned int pdiv;
+
+	/* in bypass mode, use parent rate */
+	val = readl(pll->base + IPROC_CLK_PLLARMC_OFFSET);
+	if (val & (1 << IPROC_CLK_PLLARMC_BYPCLK_EN_SHIFT)) {
+		pll->rate = parent_rate;
+		return pll->rate;
+	}
+
+	/* PLL needs to be locked */
+	val = readl(pll->base + IPROC_CLK_PLLARMA_OFFSET);
+	if (!(val & (1 << IPROC_CLK_PLLARMA_LOCK_SHIFT))) {
+		pll->rate = 0;
+		return 0;
+	}
+
+	pdiv = (val >> IPROC_CLK_PLLARMA_PDIV_SHIFT) &
+		IPROC_CLK_PLLARMA_PDIV_MASK;
+	if (pdiv == 0)
+		pdiv = 16;
+
+	ndiv = __get_ndiv(pll);
+	mdiv = __get_mdiv(pll);
+	if (mdiv <= 0) {
+		pll->rate = 0;
+		return 0;
+	}
+	pll->rate = (ndiv * parent_rate) >> 20;
+	pll->rate = (pll->rate / pdiv) / mdiv;
+
+	pr_debug("%s: ARM PLL rate: %lu. parent rate: %lu\n", __func__,
+			pll->rate, parent_rate);
+	pr_debug("%s: ndiv_int: %u, pdiv: %u, mdiv: %d\n", __func__,
+			(unsigned int)(ndiv >> 20), pdiv, mdiv);
+
+	return pll->rate;
+}
+
+static const struct clk_ops iproc_arm_pll_ops = {
+	.recalc_rate = iproc_arm_pll_recalc_rate,
+};
+
+void __init iproc_armpll_setup(struct device_node *node)
+{
+	int ret;
+	struct clk *clk;
+	struct iproc_arm_pll *pll;
+	struct clk_init_data init;
+	const char *parent_name;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (WARN_ON(!pll))
+		return;
+
+	pll->base = of_iomap(node, 0);
+	if (WARN_ON(!pll->base))
+		goto err_free_pll;
+
+	init.name = node->name;
+	init.ops = &iproc_arm_pll_ops;
+	init.flags = 0;
+	parent_name = of_clk_get_parent_name(node, 0);
+	init.parent_names = (parent_name ? &parent_name : NULL);
+	init.num_parents = (parent_name ? 1 : 0);
+	pll->hw.init = &init;
+
+	clk = clk_register(NULL, &pll->hw);
+	if (WARN_ON(IS_ERR(clk)))
+		goto err_iounmap;
+
+	ret = of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	if (WARN_ON(ret))
+		goto err_clk_unregister;
+
+	return;
+
+err_clk_unregister:
+	clk_unregister(clk);
+err_iounmap:
+	iounmap(pll->base);
+err_free_pll:
+	kfree(pll);
+}
diff --git a/drivers/clk/bcm/clk-iproc-asiu.c b/drivers/clk/bcm/clk-iproc-asiu.c
new file mode 100644
index 0000000..ab86b8c
--- /dev/null
+++ b/drivers/clk/bcm/clk-iproc-asiu.c
@@ -0,0 +1,275 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+
+#include "clk-iproc.h"
+
+struct iproc_asiu;
+
+struct iproc_asiu_clk {
+	struct clk_hw hw;
+	const char *name;
+	struct iproc_asiu *asiu;
+	unsigned long rate;
+	struct iproc_asiu_div div;
+	struct iproc_asiu_gate gate;
+};
+
+struct iproc_asiu {
+	void __iomem *div_base;
+	void __iomem *gate_base;
+
+	struct clk_onecell_data clk_data;
+	struct iproc_asiu_clk *clks;
+};
+
+#define to_asiu_clk(hw) container_of(hw, struct iproc_asiu_clk, hw)
+
+static int iproc_asiu_clk_enable(struct clk_hw *hw)
+{
+	struct iproc_asiu_clk *clk = to_asiu_clk(hw);
+	struct iproc_asiu *asiu = clk->asiu;
+	u32 val;
+
+	/* some clocks at the ASIU level are always enabled */
+	if (clk->gate.offset == IPROC_CLK_INVALID_OFFSET)
+		return 0;
+
+	val = readl(asiu->gate_base + clk->gate.offset);
+	val |= (1 << clk->gate.en_shift);
+	writel(val, asiu->gate_base + clk->gate.offset);
+
+	return 0;
+}
+
+static void iproc_asiu_clk_disable(struct clk_hw *hw)
+{
+	struct iproc_asiu_clk *clk = to_asiu_clk(hw);
+	struct iproc_asiu *asiu = clk->asiu;
+	u32 val;
+
+	/* some clocks at the ASIU level are always enabled */
+	if (clk->gate.offset == IPROC_CLK_INVALID_OFFSET)
+		return;
+
+	val = readl(asiu->gate_base + clk->gate.offset);
+	val &= ~(1 << clk->gate.en_shift);
+	writel(val, asiu->gate_base + clk->gate.offset);
+}
+
+static unsigned long iproc_asiu_clk_recalc_rate(struct clk_hw *hw,
+		unsigned long parent_rate)
+{
+	struct iproc_asiu_clk *clk = to_asiu_clk(hw);
+	struct iproc_asiu *asiu = clk->asiu;
+	u32 val;
+	unsigned int div_h, div_l;
+
+	if (parent_rate == 0) {
+		clk->rate = 0;
+		return 0;
+	}
+
+	/* if clock divisor is not enabled, simply return parent rate */
+	val = readl(asiu->div_base + clk->div.offset);
+	if ((val & (1 << clk->div.en_shift)) == 0) {
+		clk->rate = parent_rate;
+		return parent_rate;
+	}
+
+	/* clock rate = parent rate / (high_div + 1) + (low_div + 1) */
+	div_h = (val >> clk->div.high_shift) & bit_mask(clk->div.high_width);
+	div_h++;
+	div_l = (val >> clk->div.low_shift) & bit_mask(clk->div.low_width);
+	div_l++;
+
+	clk->rate = parent_rate / (div_h + div_l);
+	pr_debug("%s: rate: %lu. parent rate: %lu div_h: %u div_l: %u\n",
+		__func__, clk->rate, parent_rate, div_h, div_l);
+
+	return clk->rate;
+}
+
+static long iproc_asiu_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+		unsigned long *parent_rate)
+{
+	unsigned int div;
+
+	if (rate == 0 || *parent_rate == 0)
+		return -EINVAL;
+
+	if (rate == *parent_rate)
+		return *parent_rate;
+
+	div = DIV_ROUND_UP(*parent_rate, rate);
+	if (div < 2)
+		return *parent_rate;
+
+	return *parent_rate / div;
+}
+
+static int iproc_asiu_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+		unsigned long parent_rate)
+{
+	struct iproc_asiu_clk *clk = to_asiu_clk(hw);
+	struct iproc_asiu *asiu = clk->asiu;
+	unsigned int div, div_h, div_l;
+	u32 val;
+
+	if (rate == 0 || parent_rate == 0)
+		return -EINVAL;
+
+	/* simply disable the divisor if one wants the same rate as parent */
+	if (rate == parent_rate) {
+		val = readl(asiu->div_base + clk->div.offset);
+		val &= ~(1 << clk->div.en_shift);
+		writel(val, asiu->div_base + clk->div.offset);
+		return 0;
+	}
+
+	div = DIV_ROUND_UP(parent_rate, rate);
+	if (div < 2)
+		return -EINVAL;
+
+	div_h = div_l = div >> 1;
+	div_h--;
+	div_l--;
+
+	val = readl(asiu->div_base + clk->div.offset);
+	val |= 1 << clk->div.en_shift;
+	if (div_h) {
+		val &= ~(bit_mask(clk->div.high_width)
+				<< clk->div.high_shift);
+		val |= div_h << clk->div.high_shift;
+	} else {
+		val &= ~(bit_mask(clk->div.high_width)
+				<< clk->div.high_shift);
+	}
+	if (div_l) {
+		val &= ~(bit_mask(clk->div.low_width) << clk->div.low_shift);
+		val |= div_l << clk->div.low_shift;
+	} else {
+		val &= ~(bit_mask(clk->div.low_width) << clk->div.low_shift);
+	}
+	writel(val, asiu->div_base + clk->div.offset);
+
+	return 0;
+}
+
+static const struct clk_ops iproc_asiu_ops = {
+	.enable = iproc_asiu_clk_enable,
+	.disable = iproc_asiu_clk_disable,
+	.recalc_rate = iproc_asiu_clk_recalc_rate,
+	.round_rate = iproc_asiu_clk_round_rate,
+	.set_rate = iproc_asiu_clk_set_rate,
+};
+
+void __init iproc_asiu_setup(struct device_node *node,
+		const struct iproc_asiu_div *div,
+		const struct iproc_asiu_gate *gate, unsigned int num_clks)
+{
+	int i, ret;
+	struct iproc_asiu *asiu;
+
+	if (WARN_ON(!gate || !div))
+		return;
+
+	asiu = kzalloc(sizeof(*asiu), GFP_KERNEL);
+	if (WARN_ON(!asiu))
+		return;
+
+	asiu->clk_data.clk_num = num_clks;
+	asiu->clk_data.clks = kcalloc(num_clks, sizeof(*asiu->clk_data.clks),
+			GFP_KERNEL);
+	if (WARN_ON(!asiu->clk_data.clks))
+		goto err_clks;
+
+	asiu->clks = kcalloc(num_clks, sizeof(*asiu->clks), GFP_KERNEL);
+	if (WARN_ON(!asiu->clks))
+		goto err_asiu_clks;
+
+	asiu->div_base = of_iomap(node, 0);
+	if (WARN_ON(!asiu->div_base))
+		goto err_iomap_div;
+
+	asiu->gate_base = of_iomap(node, 1);
+	if (WARN_ON(!asiu->gate_base))
+		goto err_iomap_gate;
+
+	for (i = 0; i < num_clks; i++) {
+		struct clk_init_data init;
+		struct clk *clk;
+		const char *parent_name;
+		struct iproc_asiu_clk *asiu_clk;
+		const char *clk_name;
+
+		clk_name = kzalloc(IPROC_CLK_NAME_LEN, GFP_KERNEL);
+		if (WARN_ON(!clk_name))
+			goto err_clk_register;
+
+		ret = of_property_read_string_index(node, "clock-output-names",
+				i, &clk_name);
+		if (WARN_ON(ret))
+			goto err_clk_register;
+
+		asiu_clk = &asiu->clks[i];
+		asiu_clk->name = clk_name;
+		asiu_clk->asiu = asiu;
+		asiu_clk->div = div[i];
+		asiu_clk->gate = gate[i];
+		init.name = clk_name;
+		init.ops = &iproc_asiu_ops;
+		init.flags = 0;
+		parent_name = of_clk_get_parent_name(node, 0);
+		init.parent_names = (parent_name ? &parent_name : NULL);
+		init.num_parents = (parent_name ? 1 : 0);
+		asiu_clk->hw.init = &init;
+
+		clk = clk_register(NULL, &asiu_clk->hw);
+		if (WARN_ON(IS_ERR(clk)))
+			goto err_clk_register;
+		asiu->clk_data.clks[i] = clk;
+	}
+
+	ret = of_clk_add_provider(node, of_clk_src_onecell_get,
+			&asiu->clk_data);
+	if (WARN_ON(ret))
+		goto err_clk_register;
+
+	return;
+
+err_clk_register:
+	for (i = 0; i < num_clks; i++)
+		kfree(asiu->clks[i].name);
+	iounmap(asiu->gate_base);
+
+err_iomap_gate:
+	iounmap(asiu->div_base);
+
+err_iomap_div:
+	kfree(asiu->clks);
+
+err_asiu_clks:
+	kfree(asiu->clk_data.clks);
+
+err_clks:
+	kfree(asiu);
+}
diff --git a/drivers/clk/bcm/clk-iproc-clk.c b/drivers/clk/bcm/clk-iproc-clk.c
new file mode 100644
index 0000000..3b5a9bb
--- /dev/null
+++ b/drivers/clk/bcm/clk-iproc-clk.c
@@ -0,0 +1,244 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+
+#include "clk-iproc.h"
+
+struct iproc_pll;
+
+struct iproc_clk {
+	struct clk_hw hw;
+	const char *name;
+	struct iproc_pll *pll;
+	unsigned long rate;
+	const struct iproc_clk_ctrl *ctrl;
+};
+
+struct iproc_pll {
+	void __iomem *base;
+	struct clk_onecell_data clk_data;
+	struct iproc_clk *clks;
+};
+
+#define to_iproc_clk(hw) container_of(hw, struct iproc_clk, hw)
+
+static int iproc_clk_enable(struct clk_hw *hw)
+{
+	struct iproc_clk *clk = to_iproc_clk(hw);
+	const struct iproc_clk_ctrl *ctrl = clk->ctrl;
+	struct iproc_pll *pll = clk->pll;
+	u32 val;
+
+	/* channel enable is active low */
+	val = readl(pll->base + ctrl->enable.offset);
+	val &= ~(1 << ctrl->enable.enable_shift);
+	writel(val, pll->base + ctrl->enable.offset);
+
+	/* also make sure channel is not held */
+	val = readl(pll->base + ctrl->enable.offset);
+	val &= ~(1 << ctrl->enable.hold_shift);
+	writel(val, pll->base + ctrl->enable.offset);
+	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
+		readl(pll->base + ctrl->enable.offset);
+
+	return 0;
+}
+
+static void iproc_clk_disable(struct clk_hw *hw)
+{
+	struct iproc_clk *clk = to_iproc_clk(hw);
+	const struct iproc_clk_ctrl *ctrl = clk->ctrl;
+	struct iproc_pll *pll = clk->pll;
+	u32 val;
+
+	if (ctrl->flags & IPROC_CLK_AON)
+		return;
+
+	val = readl(pll->base + ctrl->enable.offset);
+	val |= 1 << ctrl->enable.enable_shift;
+	writel(val, pll->base + ctrl->enable.offset);
+	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
+		readl(pll->base + ctrl->enable.offset);
+}
+
+static unsigned long iproc_clk_recalc_rate(struct clk_hw *hw,
+		unsigned long parent_rate)
+{
+	struct iproc_clk *clk = to_iproc_clk(hw);
+	const struct iproc_clk_ctrl *ctrl = clk->ctrl;
+	struct iproc_pll *pll = clk->pll;
+	u32 val;
+	unsigned int mdiv;
+
+	if (parent_rate == 0)
+		return 0;
+
+	val = readl(pll->base + ctrl->mdiv.offset);
+	mdiv = (val >> ctrl->mdiv.shift) & bit_mask(ctrl->mdiv.width);
+	if (mdiv == 0)
+		mdiv = 256;
+
+	clk->rate = parent_rate / mdiv;
+
+	return clk->rate;
+}
+
+static long iproc_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+		unsigned long *parent_rate)
+{
+	unsigned int div;
+
+	if (rate == 0 || *parent_rate == 0)
+		return -EINVAL;
+
+	if (rate == *parent_rate)
+		return *parent_rate;
+
+	div = DIV_ROUND_UP(*parent_rate, rate);
+	if (div < 2)
+		return *parent_rate;
+
+	if (div > 256)
+		div = 256;
+
+	return *parent_rate / div;
+}
+
+static int iproc_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+		unsigned long parent_rate)
+{
+	struct iproc_clk *clk = to_iproc_clk(hw);
+	const struct iproc_clk_ctrl *ctrl = clk->ctrl;
+	struct iproc_pll *pll = clk->pll;
+	u32 val;
+	unsigned int div;
+
+	if (rate == 0 || parent_rate == 0)
+		return -EINVAL;
+
+	div = DIV_ROUND_UP(parent_rate, rate);
+	if (div > 256)
+		return -EINVAL;
+
+	val = readl(pll->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;
+	}
+	writel(val, pll->base + ctrl->mdiv.offset);
+	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
+		readl(pll->base + ctrl->mdiv.offset);
+	clk->rate = parent_rate / div;
+
+	return 0;
+}
+
+static const struct clk_ops iproc_clk_ops = {
+	.enable = iproc_clk_enable,
+	.disable = iproc_clk_disable,
+	.recalc_rate = iproc_clk_recalc_rate,
+	.round_rate = iproc_clk_round_rate,
+	.set_rate = iproc_clk_set_rate,
+};
+
+void __init iproc_clk_setup(struct device_node *node,
+		const struct iproc_clk_ctrl *ctrl, unsigned int num_clks)
+{
+	int i, ret;
+	struct iproc_pll *pll;
+
+	if (WARN_ON(!ctrl))
+		return;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (WARN_ON(!pll))
+		return;
+
+	pll->clk_data.clk_num = num_clks;
+	pll->clk_data.clks = kcalloc(num_clks, sizeof(*pll->clk_data.clks),
+			GFP_KERNEL);
+	if (WARN_ON(!pll->clk_data.clks))
+		goto err_clks;
+
+	pll->clks = kcalloc(num_clks, sizeof(*pll->clks), GFP_KERNEL);
+	if (WARN_ON(!pll->clks))
+		goto err_pll_clks;
+
+	pll->base = of_iomap(node, 0);
+	if (WARN_ON(!pll->base))
+		goto err_iomap;
+
+	for (i = 0; i < num_clks; i++) {
+		struct clk_init_data init;
+		struct clk *clk;
+		const char *parent_name;
+		struct iproc_clk *iclk;
+		const char *clk_name;
+
+		clk_name = kzalloc(IPROC_CLK_NAME_LEN, GFP_KERNEL);
+		if (WARN_ON(!clk_name))
+			goto err_clk_register;
+
+		ret = of_property_read_string_index(node, "clock-output-names",
+				i, &clk_name);
+		if (WARN_ON(ret))
+			goto err_clk_register;
+
+		iclk = &pll->clks[i];
+		iclk->name = clk_name;
+		iclk->pll = pll;
+		iclk->ctrl = &ctrl[i];
+		init.name = clk_name;
+		init.ops = &iproc_clk_ops;
+		init.flags = 0;
+		parent_name = of_clk_get_parent_name(node, 0);
+		init.parent_names = (parent_name ? &parent_name : NULL);
+		init.num_parents = (parent_name ? 1 : 0);
+		iclk->hw.init = &init;
+
+		clk = clk_register(NULL, &iclk->hw);
+		if (WARN_ON(IS_ERR(clk)))
+			goto err_clk_register;
+		pll->clk_data.clks[i] = clk;
+	}
+
+	ret = of_clk_add_provider(node, of_clk_src_onecell_get, &pll->clk_data);
+	if (WARN_ON(ret))
+		goto err_clk_register;
+
+	return;
+
+err_clk_register:
+	for (i = 0; i < num_clks; i++)
+		kfree(pll->clks[i].name);
+	iounmap(pll->base);
+
+err_iomap:
+	kfree(pll->clks);
+
+err_pll_clks:
+	kfree(pll->clk_data.clks);
+
+err_clks:
+	kfree(pll);
+}
diff --git a/drivers/clk/bcm/clk-iproc-pll.c b/drivers/clk/bcm/clk-iproc-pll.c
new file mode 100644
index 0000000..5111d94
--- /dev/null
+++ b/drivers/clk/bcm/clk-iproc-pll.c
@@ -0,0 +1,490 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+
+#include "clk-iproc.h"
+
+#define PLL_VCO_HIGH_SHIFT 19
+#define PLL_VCO_LOW_SHIFT  30
+
+/* number of delay loops waiting for PLL to lock */
+#define LOCK_DELAY 100
+
+/* number of VCO frequency bands */
+#define NUM_FREQ_BANDS 8
+
+#define NUM_KP_BANDS 3
+enum kp_band {
+	KP_BAND_MID = 0,
+	KP_BAND_HIGH,
+	KP_BAND_HIGH_HIGH
+};
+
+static const unsigned int kp_table[NUM_KP_BANDS][NUM_FREQ_BANDS] = {
+	{ 5, 6, 6, 7, 7, 8, 9, 10 },
+	{ 4, 4, 5, 5, 6, 7, 8, 9  },
+	{ 4, 5, 5, 6, 7, 8, 9, 10 },
+};
+
+static const unsigned long ref_freq_table[NUM_FREQ_BANDS][2] = {
+	{ 10000000,  12500000  },
+	{ 12500000,  15000000  },
+	{ 15000000,  20000000  },
+	{ 20000000,  25000000  },
+	{ 25000000,  50000000  },
+	{ 50000000,  75000000  },
+	{ 75000000,  100000000 },
+	{ 100000000, 125000000 },
+};
+
+enum vco_freq_range {
+	VCO_LOW       = 700000000U,
+	VCO_MID       = 1200000000U,
+	VCO_HIGH      = 2200000000U,
+	VCO_HIGH_HIGH = 3100000000U,
+	VCO_MAX       = 4000000000U,
+};
+
+struct iproc_pll {
+	struct clk_hw hw;
+	void __iomem *pll_base;
+	void __iomem *pwr_base;
+	void __iomem *asiu_base;
+	const char *name;
+	const struct iproc_pll_ctrl *ctrl;
+	const struct iproc_pll_vco_freq_param *vco_param;
+	unsigned int num_vco_entries;
+	unsigned long rate;
+};
+
+#define to_iproc_pll(hw) container_of(hw, struct iproc_pll, hw)
+
+/*
+ * Based on the target frequency, find a match from the VCO frequency parameter
+ * table and return its index
+ */
+static int pll_get_rate_index(struct iproc_pll *pll, unsigned int target_rate)
+{
+	int i;
+
+	for (i = 0; i < pll->num_vco_entries; i++)
+		if (target_rate == pll->vco_param[i].rate)
+			break;
+
+	if (i >= pll->num_vco_entries)
+		return -EINVAL;
+
+	return i;
+}
+
+static int get_kp(unsigned long ref_freq, enum kp_band kp_index)
+{
+	int i;
+
+	if (ref_freq < ref_freq_table[0][0])
+		return -EINVAL;
+
+	for (i = 0; i < NUM_FREQ_BANDS; i++) {
+		if (ref_freq >= ref_freq_table[i][0] &&
+			ref_freq < ref_freq_table[i][1])
+			return kp_table[kp_index][i];
+	}
+	return -EINVAL;
+}
+
+static int pll_wait_for_lock(struct iproc_pll *pll)
+{
+	int i;
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+
+	for (i = 0; i < LOCK_DELAY; i++) {
+		u32 val = readl(pll->pll_base + ctrl->status.offset);
+
+		if (val & (1 << ctrl->status.shift))
+			return 0;
+		udelay(10);
+	}
+
+	return -EIO;
+}
+
+static void __pll_disable(struct iproc_pll *pll)
+{
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+	u32 val;
+
+	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);
+	}
+
+	/* 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)
+{
+	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);
+
+	/* 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);
+	}
+
+	return 0;
+}
+
+static void __pll_put_in_reset(struct iproc_pll *pll)
+{
+	u32 val;
+	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 &= ~(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);
+}
+
+static void __pll_bring_out_reset(struct iproc_pll *pll, unsigned int kp,
+				  unsigned int ka, unsigned int ki)
+{
+	u32 val;
+	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 &= ~(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;
+	writel(val, pll->pll_base + reset->offset);
+	if (unlikely(ctrl->flags & IPROC_CLK_NEEDS_READ_BACK))
+		readl(pll->pll_base + reset->offset);
+}
+
+static int pll_set_rate(struct iproc_pll *pll, unsigned int rate_index,
+			unsigned long parent_rate)
+{
+	const struct iproc_pll_vco_freq_param *vco =
+				&pll->vco_param[rate_index];
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+	int ka = 0, ki, kp, ret;
+	unsigned long rate = vco->rate;
+	u32 val;
+	enum kp_band kp_index;
+	unsigned long ref_freq;
+
+	/*
+	 * reference frequency = parent frequency / PDIV
+	 * If PDIV = 0, then it becomes a multiplier (x2)
+	 */
+	if (vco->pdiv == 0)
+		ref_freq = parent_rate * 2;
+	else
+		ref_freq = parent_rate / vco->pdiv;
+
+	/* determine Ki and Kp index based on target VCO frequency */
+	if (rate >= VCO_LOW && rate < VCO_HIGH) {
+		ki = 4;
+		kp_index = KP_BAND_MID;
+	} else if (rate >= VCO_HIGH && rate && rate < VCO_HIGH_HIGH) {
+		ki = 3;
+		kp_index = KP_BAND_HIGH;
+	} else if (rate >= VCO_HIGH_HIGH && rate < VCO_MAX) {
+		ki = 3;
+		kp_index = KP_BAND_HIGH_HIGH;
+	} else {
+		pr_err("%s: pll: %s has invalid rate: %lu\n", __func__,
+				pll->name, rate);
+		return -EINVAL;
+	}
+
+	kp = get_kp(ref_freq, kp_index);
+	if (kp < 0) {
+		pr_err("%s: pll: %s has invalid kp\n", __func__, pll->name);
+		return kp;
+	}
+
+	ret = __pll_enable(pll);
+	if (ret) {
+		pr_err("%s: pll: %s fails to enable\n", __func__, pll->name);
+		return ret;
+	}
+
+	/* 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);
+	val = readl(pll->pll_base + ctrl->vco_ctrl.l_offset);
+
+	if (rate >= VCO_LOW && rate < VCO_MID)
+		val |= (1 << PLL_VCO_LOW_SHIFT);
+
+	if (rate < VCO_HIGH)
+		val &= ~(1 << PLL_VCO_HIGH_SHIFT);
+	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);
+
+	/* 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);
+
+	/* program fractional part of NDIV */
+	if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
+		val = readl(pll->pll_base + ctrl->ndiv_frac.offset);
+		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);
+	}
+
+	/* 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);
+
+	__pll_bring_out_reset(pll, kp, ka, ki);
+
+	ret = pll_wait_for_lock(pll);
+	if (ret < 0) {
+		pr_err("%s: pll: %s failed to lock\n", __func__, pll->name);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int iproc_pll_enable(struct clk_hw *hw)
+{
+	struct iproc_pll *pll = to_iproc_pll(hw);
+
+	return __pll_enable(pll);
+}
+
+static void iproc_pll_disable(struct clk_hw *hw)
+{
+	struct iproc_pll *pll = to_iproc_pll(hw);
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+
+	if (ctrl->flags & IPROC_CLK_AON)
+		return;
+
+	__pll_disable(pll);
+}
+
+static unsigned long iproc_pll_recalc_rate(struct clk_hw *hw,
+					   unsigned long parent_rate)
+{
+	struct iproc_pll *pll = to_iproc_pll(hw);
+	const struct iproc_pll_ctrl *ctrl = pll->ctrl;
+	u32 val;
+	u64 ndiv;
+	unsigned int ndiv_int, ndiv_frac, pdiv;
+
+	if (parent_rate == 0)
+		return 0;
+
+	/* PLL needs to be locked */
+	val = readl(pll->pll_base + ctrl->status.offset);
+	if ((val & (1 << ctrl->status.shift)) == 0) {
+		pll->rate = 0;
+		return 0;
+	}
+
+	/*
+	 * PLL output frequency =
+	 *
+	 * ((ndiv_int + ndiv_frac / 2^20) * (parent clock rate / pdiv)
+	 */
+	val = readl(pll->pll_base + ctrl->ndiv_int.offset);
+	ndiv_int = (val >> ctrl->ndiv_int.shift) &
+		bit_mask(ctrl->ndiv_int.width);
+	ndiv = ndiv_int << ctrl->ndiv_int.shift;
+
+	if (ctrl->flags & IPROC_CLK_PLL_HAS_NDIV_FRAC) {
+		val = readl(pll->pll_base + ctrl->ndiv_frac.offset);
+		ndiv_frac = (val >> ctrl->ndiv_frac.shift) &
+			bit_mask(ctrl->ndiv_frac.width);
+
+		if (ndiv_frac != 0)
+			ndiv = (ndiv_int << ctrl->ndiv_int.shift) | ndiv_frac;
+	}
+
+	val = readl(pll->pll_base + ctrl->pdiv.offset);
+	pdiv = (val >> ctrl->pdiv.shift) & bit_mask(ctrl->pdiv.width);
+
+	pll->rate = (ndiv * parent_rate) >> ctrl->ndiv_int.shift;
+
+	if (pdiv == 0)
+		pll->rate *= 2;
+	else
+		pll->rate /= pdiv;
+
+	return pll->rate;
+}
+
+static long iproc_pll_round_rate(struct clk_hw *hw, unsigned long rate,
+				 unsigned long *parent_rate)
+{
+	unsigned i;
+	struct iproc_pll *pll = to_iproc_pll(hw);
+
+	if (rate == 0 || *parent_rate == 0 || !pll->vco_param)
+		return -EINVAL;
+
+	for (i = 0; i < pll->num_vco_entries; i++) {
+		if (rate <= pll->vco_param[i].rate)
+			break;
+	}
+
+	if (i == pll->num_vco_entries)
+		i--;
+
+	return pll->vco_param[i].rate;
+}
+
+static int iproc_pll_set_rate(struct clk_hw *hw, unsigned long rate,
+		unsigned long parent_rate)
+{
+	struct iproc_pll *pll = to_iproc_pll(hw);
+	int rate_index, ret;
+
+	rate_index = pll_get_rate_index(pll, rate);
+	if (rate_index < 0)
+		return rate_index;
+
+	ret = pll_set_rate(pll, rate_index, parent_rate);
+	return ret;
+}
+
+static const struct clk_ops iproc_pll_ops = {
+	.enable = iproc_pll_enable,
+	.disable = iproc_pll_disable,
+	.recalc_rate = iproc_pll_recalc_rate,
+	.round_rate = iproc_pll_round_rate,
+	.set_rate = iproc_pll_set_rate,
+};
+
+void __init iproc_pll_setup(struct device_node *node,
+		const struct iproc_pll_ctrl *ctrl,
+		const struct iproc_pll_vco_freq_param *vco_param,
+		unsigned int num_vco_entries)
+{
+	int ret;
+	struct clk *clk;
+	struct iproc_pll *pll;
+	struct clk_init_data init;
+	const char *parent_name;
+
+	if (WARN_ON(!ctrl))
+		return;
+
+	pll = kzalloc(sizeof(*pll), GFP_KERNEL);
+	if (WARN_ON(!pll))
+		return;
+
+	pll->pll_base = of_iomap(node, 0);
+	if (WARN_ON(!pll->pll_base))
+		goto err_pll_iomap;
+
+	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 (ctrl->flags & IPROC_CLK_PLL_ASIU) {
+		pll->asiu_base = of_iomap(node, 2);
+		if (WARN_ON(!pll->asiu_base))
+			goto err_asiu_iomap;
+	}
+
+	pll->ctrl = ctrl;
+	pll->name = node->name;
+	init.name = node->name;
+	init.ops = &iproc_pll_ops;
+	init.flags = 0;
+	parent_name = of_clk_get_parent_name(node, 0);
+	init.parent_names = (parent_name ? &parent_name : NULL);
+	init.num_parents = (parent_name ? 1 : 0);
+	pll->hw.init = &init;
+
+	if (vco_param) {
+		pll->num_vco_entries = num_vco_entries;
+		pll->vco_param = vco_param;
+	}
+
+	clk = clk_register(NULL, &pll->hw);
+	if (WARN_ON(IS_ERR(clk)))
+		goto err_clk_register;
+
+	ret = of_clk_add_provider(node, of_clk_src_simple_get, clk);
+	if (WARN_ON(ret))
+		goto err_clk_add;
+
+	return;
+
+err_clk_add:
+	clk_unregister(clk);
+err_clk_register:
+	if (pll->asiu_base)
+		iounmap(pll->asiu_base);
+err_asiu_iomap:
+	iounmap(pll->pwr_base);
+err_pwr_iomap:
+	iounmap(pll->pll_base);
+err_pll_iomap:
+	kfree(pll);
+}
diff --git a/drivers/clk/bcm/clk-iproc.h b/drivers/clk/bcm/clk-iproc.h
new file mode 100644
index 0000000..43bbfa3
--- /dev/null
+++ b/drivers/clk/bcm/clk-iproc.h
@@ -0,0 +1,164 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef _CLK_IPROC_H
+#define _CLK_IPROC_H
+
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/device.h>
+#include <linux/of.h>
+#include <linux/clk-provider.h>
+
+#define IPROC_CLK_NAME_LEN 25
+#define IPROC_CLK_INVALID_OFFSET 0xffffffff
+#define bit_mask(width) ((1 << (width)) - 1)
+
+/* clock should not be disabled at runtime */
+#define IPROC_CLK_AON BIT(0)
+
+/* PLL requires gating through ASIU */
+#define IPROC_CLK_PLL_ASIU BIT(1)
+
+/* PLL has fractional part of the NDIV */
+#define IPROC_CLK_PLL_HAS_NDIV_FRAC BIT(2)
+
+/*
+ * Some of the iProc PLL/clocks may have an ASIC bug that requires read back
+ * of the same register following the write to flush the write transaction into
+ * the intended register
+ */
+#define IPROC_CLK_NEEDS_READ_BACK BIT(3)
+
+/*
+ * Parameters for VCO frequency configuration
+ *
+ * VCO frequency =
+ * ((ndiv_int + ndiv_frac / 2^20) * (ref freqeuncy  / pdiv)
+ */
+struct iproc_pll_vco_freq_param {
+	unsigned long rate;
+	unsigned int ndiv_int;
+	unsigned int ndiv_frac;
+	unsigned int pdiv;
+};
+
+struct iproc_clk_reg_op {
+	unsigned int offset;
+	unsigned int shift;
+	unsigned int width;
+};
+
+/*
+ * Clock gating control@the top ASIU level
+ */
+struct iproc_asiu_gate {
+	unsigned int offset;
+	unsigned int en_shift;
+};
+
+/*
+ * Control of powering on/off of a PLL
+ *
+ * Before powering off a PLL, input isolation (ISO) needs to be enabled
+ */
+struct iproc_pll_aon_pwr_ctrl {
+	unsigned int offset;
+	unsigned int pwr_width;
+	unsigned int pwr_shift;
+	unsigned int iso_shift;
+};
+
+/*
+ * Control of the PLL reset, with Ki, Kp, and Ka parameters
+ */
+struct iproc_pll_reset_ctrl {
+	unsigned int offset;
+	unsigned int reset_shift;
+	unsigned int p_reset_shift;
+	unsigned int ki_shift;
+	unsigned int ki_width;
+	unsigned int kp_shift;
+	unsigned int kp_width;
+	unsigned int ka_shift;
+	unsigned int ka_width;
+};
+
+struct iproc_pll_vco_ctrl {
+	unsigned int u_offset;
+	unsigned int l_offset;
+};
+
+/*
+ * Main PLL control parameters
+ */
+struct iproc_pll_ctrl {
+	unsigned long flags;
+	struct iproc_pll_aon_pwr_ctrl aon;
+	struct iproc_asiu_gate asiu;
+	struct iproc_pll_reset_ctrl reset;
+	struct iproc_clk_reg_op ndiv_int;
+	struct iproc_clk_reg_op ndiv_frac;
+	struct iproc_clk_reg_op pdiv;
+	struct iproc_pll_vco_ctrl vco_ctrl;
+	struct iproc_clk_reg_op status;
+};
+
+/*
+ * Controls enabling/disabling a PLL derived clock
+ */
+struct iproc_clk_enable_ctrl {
+	unsigned int offset;
+	unsigned int enable_shift;
+	unsigned int hold_shift;
+	unsigned int bypass_shift;
+};
+
+/*
+ * Main clock control parameters for clocks derived from the PLLs
+ */
+struct iproc_clk_ctrl {
+	unsigned int channel;
+	unsigned long flags;
+	struct iproc_clk_enable_ctrl enable;
+	struct iproc_clk_reg_op mdiv;
+};
+
+/*
+ * Divisor of the ASIU clocks
+ */
+struct iproc_asiu_div {
+	unsigned int offset;
+	unsigned int en_shift;
+	unsigned int high_shift;
+	unsigned int high_width;
+	unsigned int low_shift;
+	unsigned int low_width;
+};
+
+void iproc_armpll_setup(struct device_node *node);
+void iproc_pll_setup(struct device_node *node,
+		     const struct iproc_pll_ctrl *ctrl,
+		     const struct iproc_pll_vco_freq_param *vco_param,
+		     unsigned int num_freqs);
+void iproc_clk_setup(struct device_node *node,
+		     const struct iproc_clk_ctrl *ctrl,
+		     unsigned int num_clks);
+void iproc_asiu_setup(struct device_node *node,
+		      const struct iproc_asiu_div *div,
+		      const struct iproc_asiu_gate *gate,
+		      unsigned int num_clks);
+
+#endif /* _CLK_IPROC_H */
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 3/6] clk: Change bcm clocks build dependency
  2015-03-18  5:45 [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Ray Jui
  2015-03-18  5:45 ` [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding Ray Jui
  2015-03-18  5:45 ` [PATCH v6 2/6] clk: iproc: add initial common clock support Ray Jui
@ 2015-03-18  5:45 ` Ray Jui
  2015-03-18  5:45 ` [PATCH v6 4/6] clk: cygnus: add clock support for Broadcom Cygnus Ray Jui
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Ray Jui @ 2015-03-18  5:45 UTC (permalink / raw)
  To: linux-arm-kernel

The clock code under drivers/clk/bcm now contains code for both the
Broadcom mobile SoCs and the iProc SoCs. Change the the makefile
dependency to be under config flag CONFIG_ARCH_BCM that's enabled for
both families of SoCs

Signed-off-by: Ray Jui <rjui@broadcom.com>
---
 drivers/clk/Makefile |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index d478ceb..1dde3c8 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -43,7 +43,7 @@ obj-$(CONFIG_ARCH_VT8500)		+= clk-vt8500.o
 obj-$(CONFIG_COMMON_CLK_WM831X)		+= clk-wm831x.o
 obj-$(CONFIG_COMMON_CLK_XGENE)		+= clk-xgene.o
 obj-$(CONFIG_COMMON_CLK_AT91)		+= at91/
-obj-$(CONFIG_ARCH_BCM_MOBILE)		+= bcm/
+obj-$(CONFIG_ARCH_BCM)			+= bcm/
 obj-$(CONFIG_ARCH_BERLIN)		+= berlin/
 obj-$(CONFIG_ARCH_HI3xxx)		+= hisilicon/
 obj-$(CONFIG_ARCH_HIP04)		+= hisilicon/
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 4/6] clk: cygnus: add clock support for Broadcom Cygnus
  2015-03-18  5:45 [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Ray Jui
                   ` (2 preceding siblings ...)
  2015-03-18  5:45 ` [PATCH v6 3/6] clk: Change bcm clocks build dependency Ray Jui
@ 2015-03-18  5:45 ` Ray Jui
  2015-03-18  5:45 ` [PATCH v6 5/6] ARM: dts: enable " Ray Jui
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Ray Jui @ 2015-03-18  5:45 UTC (permalink / raw)
  To: linux-arm-kernel

The Broadcom Cygnus SoC is architected under the iProc architecture. It
has the following PLLs: ARMPLL, GENPLL, LCPLL0, MIPIPLL, all dervied
from an onboard crystal. Cygnus also has various ASIU clocks that are
derived directly from the onboard crystal.

Signed-off-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 drivers/clk/bcm/Makefile               |    1 +
 drivers/clk/bcm/clk-cygnus.c           |  284 ++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/bcm-cygnus.h |   65 ++++++++
 3 files changed, 350 insertions(+)
 create mode 100644 drivers/clk/bcm/clk-cygnus.c
 create mode 100644 include/dt-bindings/clock/bcm-cygnus.h

diff --git a/drivers/clk/bcm/Makefile b/drivers/clk/bcm/Makefile
index 6926636..afcbe55 100644
--- a/drivers/clk/bcm/Makefile
+++ b/drivers/clk/bcm/Makefile
@@ -3,3 +3,4 @@ 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-clk.o clk-iproc-asiu.o
+obj-$(CONFIG_ARCH_BCM_CYGNUS)	+= clk-cygnus.o
diff --git a/drivers/clk/bcm/clk-cygnus.c b/drivers/clk/bcm/clk-cygnus.c
new file mode 100644
index 0000000..fe3013b
--- /dev/null
+++ b/drivers/clk/bcm/clk-cygnus.c
@@ -0,0 +1,284 @@
+/*
+ * Copyright (C) 2014 Broadcom Corporation
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/clk-provider.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/of_address.h>
+#include <linux/delay.h>
+
+#include <dt-bindings/clock/bcm-cygnus.h>
+#include "clk-iproc.h"
+
+#define reg_val(o, s, w) { .offset = o, .shift = s, .width = w, }
+
+#define aon_val(o, pw, ps, is) { .offset = o, .pwr_width = pw, \
+	.pwr_shift = ps, .iso_shift = is }
+
+#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, \
+	.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 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 }
+
+static void __init cygnus_armpll_init(struct device_node *node)
+{
+	iproc_armpll_setup(node);
+}
+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,
+	.aon = aon_val(0x0, 2, 1, 0),
+	.reset = reset_val(0x0, 11, 10, 4, 3, 0, 4, 7, 3),
+	.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 void __init cygnus_genpll_init(struct device_node *node)
+{
+	iproc_pll_setup(node, &genpll, NULL, 0);
+}
+CLK_OF_DECLARE(cygnus_genpll, "brcm,cygnus-genpll", cygnus_genpll_init);
+
+static const struct iproc_pll_ctrl lcpll0 = {
+	.flags = IPROC_CLK_AON,
+	.aon = aon_val(0x0, 2, 5, 4),
+	.reset = reset_val(0x0, 31, 30, 27, 3, 23, 4, 19, 4),
+	.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 void __init cygnus_lcpll0_init(struct device_node *node)
+{
+	iproc_pll_setup(node, &lcpll0, NULL, 0);
+}
+CLK_OF_DECLARE(cygnus_lcpll0, "brcm,cygnus-lcpll0", cygnus_lcpll0_init);
+
+/*
+ * MIPI PLL VCO frequency parameter table
+ */
+static const struct iproc_pll_vco_freq_param mipipll_vco_params[] = {
+	/* rate (Hz) ndiv_int ndiv_frac pdiv */
+	{ 750000000UL,   30,     0,        1 },
+	{ 1000000000UL,  40,     0,        1 },
+	{ 1350000000ul,  54,     0,        1 },
+	{ 2000000000UL,  80,     0,        1 },
+	{ 2100000000UL,  84,     0,        1 },
+	{ 2250000000UL,  90,     0,        1 },
+	{ 2500000000UL,  100,    0,        1 },
+	{ 2700000000UL,  54,     0,        0 },
+	{ 2975000000UL,  119,    0,        1 },
+	{ 3100000000UL,  124,    0,        1 },
+	{ 3150000000UL,  126,    0,        1 },
+};
+
+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),
+};
+
+static void __init cygnus_mipipll_init(struct device_node *node)
+{
+	iproc_pll_setup(node, &mipipll, mipipll_vco_params,
+			ARRAY_SIZE(mipipll_vco_params));
+}
+CLK_OF_DECLARE(cygnus_mipipll, "brcm,cygnus-mipipll", cygnus_mipipll_init);
+
+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),
+	},
+	[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),
+	},
+	[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),
+	},
+	[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),
+	},
+	[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),
+	},
+	[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),
+	},
+};
+
+static void __init cygnus_genpll_clk_init(struct device_node *node)
+{
+	iproc_clk_setup(node, genpll_clk, ARRAY_SIZE(genpll_clk));
+}
+CLK_OF_DECLARE(cygnus_genpll_clk, "brcm,cygnus-genpll-clk",
+		cygnus_genpll_clk_init);
+
+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),
+	},
+	[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),
+	},
+	[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),
+	},
+	[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),
+	},
+	[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),
+	},
+	[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),
+	},
+};
+
+static void __init cygnus_lcpll0_clk_init(struct device_node *node)
+{
+	iproc_clk_setup(node, lcpll0_clk, ARRAY_SIZE(lcpll0_clk));
+}
+CLK_OF_DECLARE(cygnus_lcpll0_clk, "brcm,cygnus-lcpll0-clk",
+		cygnus_lcpll0_clk_init);
+
+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),
+	},
+	[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),
+	},
+	[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),
+	},
+	[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),
+	},
+	[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),
+	},
+	[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),
+	},
+};
+
+static void __init cygnus_mipipll_clk_init(struct device_node *node)
+{
+	iproc_clk_setup(node, mipipll_clk, ARRAY_SIZE(mipipll_clk));
+}
+CLK_OF_DECLARE(cygnus_mipipll_clk, "brcm,cygnus-mipipll-clk",
+		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),
+};
+
+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),
+};
+
+static void __init cygnus_asiu_init(struct device_node *node)
+{
+	iproc_asiu_setup(node, asiu_div, asiu_gate, ARRAY_SIZE(asiu_div));
+}
+CLK_OF_DECLARE(cygnus_asiu_clk, "brcm,cygnus-asiu-clk", cygnus_asiu_init);
diff --git a/include/dt-bindings/clock/bcm-cygnus.h b/include/dt-bindings/clock/bcm-cygnus.h
new file mode 100644
index 0000000..9d30582
--- /dev/null
+++ b/include/dt-bindings/clock/bcm-cygnus.h
@@ -0,0 +1,65 @@
+/*
+ *  BSD LICENSE
+ *
+ *  Copyright(c) 2014 Broadcom Corporation.  All rights reserved.
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions
+ *  are met:
+ *
+ *    * Redistributions of source code must retain the above copyright
+ *      notice, this list of conditions and the following disclaimer.
+ *    * Redistributions in binary form must reproduce the above copyright
+ *      notice, this list of conditions and the following disclaimer in
+ *      the documentation and/or other materials provided with the
+ *      distribution.
+ *    * Neither the name of Broadcom Corporation nor the names of its
+ *      contributors may be used to endorse or promote products derived
+ *      from this software without specific prior written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *  A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *  OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *  SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *  LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *  DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *  THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *  (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _CLOCK_BCM_CYGNUS_H
+#define _CLOCK_BCM_CYGNUS_H
+
+/* GENPLL clock channel ID */
+#define BCM_CYGNUS_GENPLL_AXI21_CLK          0
+#define BCM_CYGNUS_GENPLL_250MHZ_CLK         1
+#define BCM_CYGNUS_GENPLL_IHOST_SYS_CLK      2
+#define BCM_CYGNUS_GENPLL_ENET_SW_CLK        3
+#define BCM_CYGNUS_GENPLL_AUDIO_125_CLK      4
+#define BCM_CYGNUS_GENPLL_CAN_CLK            5
+
+/* LCPLL0 clock channel ID */
+#define BCM_CYGNUS_LCPLL0_PCIE_PHY_REF_CLK    0
+#define BCM_CYGNUS_LCPLL0_DDR_PHY_CLK         1
+#define BCM_CYGNUS_LCPLL0_SDIO_CLK            2
+#define BCM_CYGNUS_LCPLL0_USB_PHY_REF_CLK     3
+#define BCM_CYGNUS_LCPLL0_SMART_CARD_CLK      4
+#define BCM_CYGNUS_LCPLL0_CH5_UNUSED          5
+
+/* MIPI PLL clock channel ID */
+#define BCM_CYGNUS_MIPIPLL_CH0_UNUSED         0
+#define BCM_CYGNUS_MIPIPLL_CH1_LCD            1
+#define BCM_CYGNUS_MIPIPLL_CH2_V3D            2
+#define BCM_CYGNUS_MIPIPLL_CH3_UNUSED         3
+#define BCM_CYGNUS_MIPIPLL_CH4_UNUSED         4
+#define BCM_CYGNUS_MIPIPLL_CH5_UNUSED         5
+
+/* ASIU clock IDs */
+#define BCM_CYGNUS_ASIU_KEYPAD_CLK    0
+#define BCM_CYGNUS_ASIU_ADC_CLK       1
+#define BCM_CYGNUS_ASIU_PWM_CLK       2
+
+#endif /* _CLOCK_BCM_CYGNUS_H */
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 5/6] ARM: dts: enable clock support for Broadcom Cygnus
  2015-03-18  5:45 [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Ray Jui
                   ` (3 preceding siblings ...)
  2015-03-18  5:45 ` [PATCH v6 4/6] clk: cygnus: add clock support for Broadcom Cygnus Ray Jui
@ 2015-03-18  5:45 ` Ray Jui
  2015-03-18  5:45 ` [PATCH v6 6/6] clk: cygnus: remove Cygnus dummy clock binding Ray Jui
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Ray Jui @ 2015-03-18  5:45 UTC (permalink / raw)
  To: linux-arm-kernel

Replace current device tree dummy clocks with real clock support for
Broadcom Cygnus SoC

Signed-off-by: Ray Jui <rjui@broadcom.com>
Reviewed-by: Scott Branden <sbranden@broadcom.com>
---
 arch/arm/boot/dts/bcm-cygnus-clock.dtsi |  112 ++++++++++++++++++++++++-------
 arch/arm/boot/dts/bcm-cygnus.dtsi       |    2 +-
 2 files changed, 88 insertions(+), 26 deletions(-)

diff --git a/arch/arm/boot/dts/bcm-cygnus-clock.dtsi b/arch/arm/boot/dts/bcm-cygnus-clock.dtsi
index 60d8389..92aab3d 100644
--- a/arch/arm/boot/dts/bcm-cygnus-clock.dtsi
+++ b/arch/arm/boot/dts/bcm-cygnus-clock.dtsi
@@ -36,56 +36,118 @@ clocks {
 	ranges;
 
 	osc: oscillator {
+		#clock-cells = <0>;
 		compatible = "fixed-clock";
-		#clock-cells = <1>;
 		clock-frequency = <25000000>;
 	};
 
-	apb_clk: apb_clk {
-		compatible = "fixed-clock";
+	/* Cygnus ARM PLL */
+	armpll: armpll {
 		#clock-cells = <0>;
-		clock-frequency = <1000000000>;
+		compatible = "brcm,cygnus-armpll";
+		clocks = <&osc>;
+		reg = <0x19000000 0x1000>;
 	};
 
-	periph_clk: periph_clk {
-		compatible = "fixed-clock";
+	/* peripheral clock for system timer */
+	arm_periph_clk: arm_periph_clk {
 		#clock-cells = <0>;
-		clock-frequency = <500000000>;
+		compatible = "fixed-factor-clock";
+		clocks = <&armpll>;
+		clock-div = <2>;
+		clock-mult = <1>;
 	};
 
-	sdio_clk: lcpll_ch2 {
-		compatible = "fixed-clock";
+	/* APB bus clock */
+	apb_clk: apb_clk {
 		#clock-cells = <0>;
-		clock-frequency = <200000000>;
+		compatible = "fixed-factor-clock";
+		clocks = <&armpll>;
+		clock-div = <4>;
+		clock-mult = <1>;
 	};
 
-	axi81_clk: axi81_clk {
-		compatible = "fixed-clock";
+	genpll: genpll {
 		#clock-cells = <0>;
-		clock-frequency = <100000000>;
+		compatible = "brcm,cygnus-genpll";
+		reg = <0x0301d000 0x2c>,
+			<0x0301c020 0x4>;
+		clocks = <&osc>;
 	};
 
-	keypad_clk: keypad_clk {
-		compatible = "fixed-clock";
+	/* various clocks running off the GENPLL */
+	genpll_clks: genpll_clks {
+		#clock-cells = <1>;
+		compatible = "brcm,cygnus-genpll-clk";
+		reg = <0x0301d000 0x2c>;
+		clocks = <&genpll>;
+		clock-output-names = "axi21", "250mhz", "ihost_sys",
+			"enet_sw", "audio_125", "can";
+	};
+
+	/* always 1/2 of the axi21 clock */
+	axi41_clk: axi41_clk {
 		#clock-cells = <0>;
-		clock-frequency = <31806>;
+		compatible = "fixed-factor-clock";
+		clocks = <&genpll_clks 0>;
+		clock-div = <2>;
+		clock-mult = <1>;
 	};
 
-	adc_clk: adc_clk {
-		compatible = "fixed-clock";
+	/* always 1/4 of the axi21 clock */
+	axi81_clk: axi81_clk {
 		#clock-cells = <0>;
-		clock-frequency = <1562500>;
+		compatible = "fixed-factor-clock";
+		clocks = <&genpll_clks 0>;
+		clock-div = <4>;
+		clock-mult = <1>;
 	};
 
-	pwm_clk: pwm_clk {
-		compatible = "fixed-clock";
+	lcpll0: lcpll0 {
 		#clock-cells = <0>;
-		clock-frequency = <1000000>;
+		compatible = "brcm,cygnus-lcpll0";
+		reg = <0x0301d02c 0x1c>,
+			<0x0301c020 0x4>;
+		clocks = <&osc>;
 	};
 
-	lcd_clk: mipipll_ch1 {
-		compatible = "fixed-clock";
+	/* various clocks running off the LCPLL0 */
+	lcpll0_clks: lcpll0_clks {
+		#clock-cells = <1>;
+		compatible = "brcm,cygnus-lcpll0-clk";
+		reg = <0x0301d02c 0x1c>;
+		clocks = <&lcpll0>;
+		clock-output-names = "pcie_phy", "ddr_phy", "sdio",
+			"usb_phy", "smart_card", "ch5";
+	};
+
+	mipipll: mipipll {
 		#clock-cells = <0>;
-		clock-frequency = <100000000>;
+		compatible = "brcm,cygnus-mipipll";
+		reg = <0x180a9800 0x2c>,
+			<0x0301c020 0x4>,
+			<0x180aa024 0x4>;
+		clocks = <&osc>;
+
+		assigned-clocks = <&mipipll>;
+		assigned-clock-rates = <2100000000>;
+	};
+
+	mipipll_clks: mipipll_clks {
+		#clock-cells = <1>;
+		compatible = "brcm,cygnus-mipipll-clk";
+		reg = <0x180a9800 0x2c>;
+		clocks = <&mipipll>;
+		clock-output-names = "ch0_unused", "ch1_lcd", "ch2_v3d",
+			"ch3_unused", "ch4_unused", "ch5_unused";
+	};
+
+	asiu_clks: asiu_clks {
+		#clock-cells = <1>;
+		compatible = "brcm,cygnus-asiu-clk";
+		reg = <0x0301d048 0xc>,
+			<0x180aa024 0x4>;
+		clocks = <&osc>;
+		clock-output-names = "keypad", "adc/touch", "pwm";
 	};
 };
diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi
index ff5fb6a..35a25e4 100644
--- a/arch/arm/boot/dts/bcm-cygnus.dtsi
+++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
@@ -154,7 +154,7 @@
 		compatible = "arm,cortex-a9-global-timer";
 		reg = <0x19020200 0x100>;
 		interrupts = <GIC_PPI 11 IRQ_TYPE_LEVEL_HIGH>;
-		clocks = <&periph_clk>;
+		clocks = <&arm_periph_clk>;
 	};
 
 };
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 6/6] clk: cygnus: remove Cygnus dummy clock binding
  2015-03-18  5:45 [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Ray Jui
                   ` (4 preceding siblings ...)
  2015-03-18  5:45 ` [PATCH v6 5/6] ARM: dts: enable " Ray Jui
@ 2015-03-18  5:45 ` Ray Jui
  2015-03-19  1:42 ` [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Scott Branden
  2015-03-30  5:09 ` Ray Jui
  7 siblings, 0 replies; 16+ messages in thread
From: Ray Jui @ 2015-03-18  5:45 UTC (permalink / raw)
  To: linux-arm-kernel

Remove old Cygnus dummy clock binding document, as it's replaced by
Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt

Signed-off-by: Ray Jui <rjui@broadcom.com>
---
 .../devicetree/bindings/clock/bcm-cygnus-clock.txt |   34 --------------------
 1 file changed, 34 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/bcm-cygnus-clock.txt

diff --git a/Documentation/devicetree/bindings/clock/bcm-cygnus-clock.txt b/Documentation/devicetree/bindings/clock/bcm-cygnus-clock.txt
deleted file mode 100644
index 00d26ed..0000000
--- a/Documentation/devicetree/bindings/clock/bcm-cygnus-clock.txt
+++ /dev/null
@@ -1,34 +0,0 @@
-Broadcom Cygnus Clocks
-
-This binding uses the common clock binding:
-Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Currently various "fixed" clocks are declared for peripheral drivers that use
-the common clock framework to reference their core clocks. Proper support of
-these clocks will be added later
-
-Device tree example:
-
-	clocks {
-		#address-cells = <1>;
-		#size-cells = <1>;
-		ranges;
-
-		osc: oscillator {
-			compatible = "fixed-clock";
-			#clock-cells = <1>;
-			clock-frequency = <25000000>;
-		};
-
-		apb_clk: apb_clk {
-			compatible = "fixed-clock";
-			#clock-cells = <0>;
-			clock-frequency = <1000000000>;
-		};
-
-		periph_clk: periph_clk {
-			compatible = "fixed-clock";
-			#clock-cells = <0>;
-			clock-frequency = <500000000>;
-		};
-	};
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture
  2015-03-18  5:45 [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Ray Jui
                   ` (5 preceding siblings ...)
  2015-03-18  5:45 ` [PATCH v6 6/6] clk: cygnus: remove Cygnus dummy clock binding Ray Jui
@ 2015-03-19  1:42 ` Scott Branden
  2015-03-30  5:09 ` Ray Jui
  7 siblings, 0 replies; 16+ messages in thread
From: Scott Branden @ 2015-03-19  1:42 UTC (permalink / raw)
  To: linux-arm-kernel

Patchset looks good now.

Scott

On 15-03-17 10:45 PM, Ray Jui wrote:
> This patchset contains the initial common clock support for Broadcom's iProc
> family of SoCs. The iProc clock architecture comprises of various PLLs, e.g.,
> ARMPLL, GENPLL, LCPLL0, MIPIPLL, and etc. An onboard crystal serves as the
> basic reference clock for these PLLs. Each PLL may have several leaf clocks.
> One special group of clocks is the ASIU clocks, which are dervied directly
> from the crystal reference clock.
>
> This patchset also contains the basic clock support for the Broadcom Cygnus
> SoC, which implements the iProc clock architecture
>
> Changes from v5:
>   - Rebase to v4.0-rc4
>   - Drop of_clk_get_parent_rate helper function from the clock framework
>   - Get rid of custom "clock-frequency" support in iProc PLL code. Instead, add
>     standard clock set_rate and round_rate support and make use of DT properties
>     "assigned-clocks" and "assigned-clock-rates" to initialize PLL to the
>     desired rate when registering to the clock framework
>   - Add SW workaround for ASIC bug on MIPI PLL to always read back the same
>     register following a write transaction, to ensure value is written to the
>     correct register
>
> Changes from v4:
>   - Add of_clk_get_parent_rate helper function into the clock framework
>   - Switch to use of_clk_get_parent_rate in the iProc PLL clock driver
>
> Changes from v3:
>   - Fix incorrect use of passing in of_clk_src_onecell_get when adding ARM PLL
>     and other iProc PLLs as clock provider. These PLLs have zero cells in DT and
>     thefore of_clk_src_simple_get should be used instead
>   - Rename Cygnus MIPI PLL Channel 2 clock from BCM_CYGNUS_MIPIPLL_CH2_UNUSED
>     to BCM_CYGNUS_MIPIPLL_CH2_V3D, since a 3D graphic rendering engine has been
>     integrated into Cygnus revision B0 and has its core clock running off
>     MIPI PLL Channel 2
>   - Changed default MIPI PLL VCO frequency from 1.75 GHz to 2.1 GHz. This allows
>     us to derive 300 MHz V3D clock from channel 2 through the post divisor
>
> Changes from v2:
>   - Re-arrange Cygnus clock/pll init functions so each init function is right
>     next to its clock table
>   - Removed #defines for number of clocks in Cygnus. Have the number of clocks
>     automatically determined based on array size of the clock table
>
> Changes from v1:
>   - Separate drivers/clk/Makefile change for drivers/clk/bcm out to a standalone patch
>
> Ray Jui (6):
>    clk: iproc: define Broadcom iProc clock binding
>    clk: iproc: add initial common clock support
>    clk: Change bcm clocks build dependency
>    clk: cygnus: add clock support for Broadcom Cygnus
>    ARM: dts: enable clock support for Broadcom Cygnus
>    clk: cygnus: remove Cygnus dummy clock binding
>
>   .../devicetree/bindings/clock/bcm-cygnus-clock.txt |   34 --
>   .../bindings/clock/brcm,iproc-clocks.txt           |  171 +++++++
>   arch/arm/boot/dts/bcm-cygnus-clock.dtsi            |  112 ++++-
>   arch/arm/boot/dts/bcm-cygnus.dtsi                  |    2 +-
>   drivers/clk/Makefile                               |    2 +-
>   drivers/clk/bcm/Kconfig                            |    9 +
>   drivers/clk/bcm/Makefile                           |    2 +
>   drivers/clk/bcm/clk-cygnus.c                       |  284 ++++++++++++
>   drivers/clk/bcm/clk-iproc-armpll.c                 |  282 +++++++++++
>   drivers/clk/bcm/clk-iproc-asiu.c                   |  275 +++++++++++
>   drivers/clk/bcm/clk-iproc-clk.c                    |  244 ++++++++++
>   drivers/clk/bcm/clk-iproc-pll.c                    |  490 ++++++++++++++++++++
>   drivers/clk/bcm/clk-iproc.h                        |  164 +++++++
>   include/dt-bindings/clock/bcm-cygnus.h             |   65 +++
>   14 files changed, 2075 insertions(+), 61 deletions(-)
>   delete mode 100644 Documentation/devicetree/bindings/clock/bcm-cygnus-clock.txt
>   create mode 100644 Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>   create mode 100644 drivers/clk/bcm/clk-cygnus.c
>   create mode 100644 drivers/clk/bcm/clk-iproc-armpll.c
>   create mode 100644 drivers/clk/bcm/clk-iproc-asiu.c
>   create mode 100644 drivers/clk/bcm/clk-iproc-clk.c
>   create mode 100644 drivers/clk/bcm/clk-iproc-pll.c
>   create mode 100644 drivers/clk/bcm/clk-iproc.h
>   create mode 100644 include/dt-bindings/clock/bcm-cygnus.h
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture
  2015-03-18  5:45 [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Ray Jui
                   ` (6 preceding siblings ...)
  2015-03-19  1:42 ` [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Scott Branden
@ 2015-03-30  5:09 ` Ray Jui
  7 siblings, 0 replies; 16+ messages in thread
From: Ray Jui @ 2015-03-30  5:09 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mike,

This is the latest patch series of the Broadcom iProc clock driver that
I was talking about. I believe it has addressed your previous code
review comments by switching to use assigned-clocks/assigned-clock-rates
for configuring PLL clock at the time of clock registration.

Please have a look and let us know if this can be pulled in.

Thanks,

Ray

On 3/17/2015 10:45 PM, Ray Jui wrote:
> This patchset contains the initial common clock support for Broadcom's iProc
> family of SoCs. The iProc clock architecture comprises of various PLLs, e.g.,
> ARMPLL, GENPLL, LCPLL0, MIPIPLL, and etc. An onboard crystal serves as the
> basic reference clock for these PLLs. Each PLL may have several leaf clocks.
> One special group of clocks is the ASIU clocks, which are dervied directly
> from the crystal reference clock.
> 
> This patchset also contains the basic clock support for the Broadcom Cygnus
> SoC, which implements the iProc clock architecture
> 
> Changes from v5:
>  - Rebase to v4.0-rc4
>  - Drop of_clk_get_parent_rate helper function from the clock framework
>  - Get rid of custom "clock-frequency" support in iProc PLL code. Instead, add
>    standard clock set_rate and round_rate support and make use of DT properties
>    "assigned-clocks" and "assigned-clock-rates" to initialize PLL to the
>    desired rate when registering to the clock framework
>  - Add SW workaround for ASIC bug on MIPI PLL to always read back the same
>    register following a write transaction, to ensure value is written to the
>    correct register
> 
> Changes from v4:
>  - Add of_clk_get_parent_rate helper function into the clock framework
>  - Switch to use of_clk_get_parent_rate in the iProc PLL clock driver
> 
> Changes from v3:
>  - Fix incorrect use of passing in of_clk_src_onecell_get when adding ARM PLL
>    and other iProc PLLs as clock provider. These PLLs have zero cells in DT and
>    thefore of_clk_src_simple_get should be used instead
>  - Rename Cygnus MIPI PLL Channel 2 clock from BCM_CYGNUS_MIPIPLL_CH2_UNUSED
>    to BCM_CYGNUS_MIPIPLL_CH2_V3D, since a 3D graphic rendering engine has been
>    integrated into Cygnus revision B0 and has its core clock running off
>    MIPI PLL Channel 2
>  - Changed default MIPI PLL VCO frequency from 1.75 GHz to 2.1 GHz. This allows
>    us to derive 300 MHz V3D clock from channel 2 through the post divisor
> 
> Changes from v2:
>  - Re-arrange Cygnus clock/pll init functions so each init function is right
>    next to its clock table
>  - Removed #defines for number of clocks in Cygnus. Have the number of clocks
>    automatically determined based on array size of the clock table
> 
> Changes from v1:
>  - Separate drivers/clk/Makefile change for drivers/clk/bcm out to a standalone patch
> 
> Ray Jui (6):
>   clk: iproc: define Broadcom iProc clock binding
>   clk: iproc: add initial common clock support
>   clk: Change bcm clocks build dependency
>   clk: cygnus: add clock support for Broadcom Cygnus
>   ARM: dts: enable clock support for Broadcom Cygnus
>   clk: cygnus: remove Cygnus dummy clock binding
> 
>  .../devicetree/bindings/clock/bcm-cygnus-clock.txt |   34 --
>  .../bindings/clock/brcm,iproc-clocks.txt           |  171 +++++++
>  arch/arm/boot/dts/bcm-cygnus-clock.dtsi            |  112 ++++-
>  arch/arm/boot/dts/bcm-cygnus.dtsi                  |    2 +-
>  drivers/clk/Makefile                               |    2 +-
>  drivers/clk/bcm/Kconfig                            |    9 +
>  drivers/clk/bcm/Makefile                           |    2 +
>  drivers/clk/bcm/clk-cygnus.c                       |  284 ++++++++++++
>  drivers/clk/bcm/clk-iproc-armpll.c                 |  282 +++++++++++
>  drivers/clk/bcm/clk-iproc-asiu.c                   |  275 +++++++++++
>  drivers/clk/bcm/clk-iproc-clk.c                    |  244 ++++++++++
>  drivers/clk/bcm/clk-iproc-pll.c                    |  490 ++++++++++++++++++++
>  drivers/clk/bcm/clk-iproc.h                        |  164 +++++++
>  include/dt-bindings/clock/bcm-cygnus.h             |   65 +++
>  14 files changed, 2075 insertions(+), 61 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/clock/bcm-cygnus-clock.txt
>  create mode 100644 Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>  create mode 100644 drivers/clk/bcm/clk-cygnus.c
>  create mode 100644 drivers/clk/bcm/clk-iproc-armpll.c
>  create mode 100644 drivers/clk/bcm/clk-iproc-asiu.c
>  create mode 100644 drivers/clk/bcm/clk-iproc-clk.c
>  create mode 100644 drivers/clk/bcm/clk-iproc-pll.c
>  create mode 100644 drivers/clk/bcm/clk-iproc.h
>  create mode 100644 include/dt-bindings/clock/bcm-cygnus.h
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding
  2015-03-18  5:45 ` [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding Ray Jui
@ 2015-04-11  0:12   ` Michael Turquette
  2015-04-13  4:08     ` Ray Jui
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Turquette @ 2015-04-11  0:12 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Ray Jui (2015-03-17 22:45:17)
> Document the device tree binding for Broadcom iProc architecture based
> clock controller
> 
> Signed-off-by: Ray Jui <rjui@broadcom.com>
> Reviewed-by: Scott Branden <sbranden@broadcom.com>
> ---
>  .../bindings/clock/brcm,iproc-clocks.txt           |  171 ++++++++++++++++++++
>  1 file changed, 171 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
> new file mode 100644
> index 0000000..bf2316b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
> @@ -0,0 +1,171 @@
> +Broadcom iProc Family Clocks
> +
> +This binding uses the common clock binding:
> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +The iProc clock controller manages clocks that are common to the iProc family.
> +An SoC from the iProc family may have several PPLs, e.g., ARMPLL, GENPLL,
> +LCPLL0, MIPIPLL, and etc., all derived from an onboard crystal. Each PLL
> +comprises of several leaf clocks
> +
> +Required properties for PLLs:
> +- compatible:
> +    Should have a value of the form "brcm,<soc>-<pll>". For example, GENPLL on
> +Cygnus has a compatible string of "brcm,cygnus-genpll"
> +
> +- #clock-cells:
> +    Must be <0>
> +
> +- reg:
> +    Define the base and range of the I/O address space that contain the iProc
> +clock control registers required for the PLL
> +
> +- clocks:
> +    The input parent clock phandle for the PLL. For all iProc PLLs, this is an
> +onboard crystal with a fixed rate
> +
> +Example:
> +
> +       osc: oscillator {
> +               #clock-cells = <0>;
> +               compatible = "fixed-clock";
> +               clock-frequency = <25000000>;
> +       };
> +
> +       genpll: genpll {
> +               #clock-cells = <0>;
> +               compatible = "brcm,cygnus-genpll";
> +               reg = <0x0301d000 0x2c>,
> +                       <0x0301c020 0x4>;
> +               clocks = <&osc>;
> +       };
> +
> +Required properties for leaf clocks of a PLL:
> +
> +- compatible:
> +    Should have a value of the form "brcm,<soc>-<pll>-clk". For example, leaf
> +clocks derived from the GENPLL on Cygnus SoC have a compatible string of
> +"brcm,cygnus-genpll-clk"
> +
> +- #clock-cells:
> +    Have a value of <1> since there are more than 1 leaf clock of a
> +given PLL
> +
> +- reg:
> +    Define the base and range of the I/O address space that contain the iProc
> +clock control registers required for the PLL leaf clocks
> +
> +- clocks:
> +    The input parent PLL phandle for the leaf clock
> +
> +- clock-output-names:
> +    An ordered list of strings defining the names of the leaf clocks
> +
> +Example:
> +
> +       genpll: genpll {
> +               #clock-cells = <0>;
> +               compatible = "brcm,cygnus-genpll";
> +               reg = <0x0301d000 0x2c>,
> +                       <0x0301c020 0x4>;
> +               clocks = <&osc>;
> +       };
> +
> +       genpll_clks: genpll_clks {
> +               #clock-cells = <1>;
> +               compatible = "brcm,cygnus-genpll-clk";
> +               reg = <0x0301d000 0x2c>;
> +               clocks = <&genpll>;
> +               clock-output-names = "axi21", "250mhz", "ihost_sys",
> +                       "enet_sw", "audio_125", "can";
> +       };

Hi Ray,

Thanks for submitting the patch. It was nice meeting you at ELC as well.

This binding doesn't conform to the norms for clock bindings. It looks
like for each type of controllable clock node (e.g. pll, leaf clock,
etc) you have a dts node. Looking at the above example it seems that
those two nodes (genpll and genpll_clks) share the same register.

/me checks patch #5

Yup, you re-use the same register address for the *pll and *pll_clks
nodes. I'm not a DT expert but I think this is considered Wrong.

More generally your clock dt binding should be modeling the hardware in
terms of IP blocks. If you have a clock generator IP block it may
control many clock bits and output many clock signals. E.g. for your
hardware (based only on the addresses in patch #5 and not having seen
any data manual) I will hazard a guess that the genpll, lcpll and asiu
clocks are all part of the same IP block.

If my guess is correct then these clocks should all be represented by a
single node int DT. Lets say that the clock controller IP block that
manages the genpll, lcpll and asiu clocks (including their child/leaf
clocks) is called "foobar" in your data manual. You should have a dts
node with a compatible string such as "brcm,cygnus-foobar" or
"brcm,cygnus-foobar-clk".

Your clock driver should be responsible for registering all of the
clocks controlled by this IP block, regardless of the "type" of clock
node.

I think part of the reason for your approach is that the previous
(deprecated) binding/dts had a bunch of fixed-rate clocks in it. This is
a hack that we do every now and then to get a new platform up and
running with a mainline kernel, but we don't do per-clock nodes in dts
any more, we do them by clock controller ip block instead.

There are plenty of good examples of this for Exynos and QCOM SoCs if
you want an example.

Regards,
Mike

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding
  2015-04-11  0:12   ` Michael Turquette
@ 2015-04-13  4:08     ` Ray Jui
  2015-04-13  6:02       ` Michael Turquette
  0 siblings, 1 reply; 16+ messages in thread
From: Ray Jui @ 2015-04-13  4:08 UTC (permalink / raw)
  To: linux-arm-kernel



On 4/10/2015 5:12 PM, Michael Turquette wrote:
> Quoting Ray Jui (2015-03-17 22:45:17)
>> Document the device tree binding for Broadcom iProc architecture based
>> clock controller
>>
>> Signed-off-by: Ray Jui <rjui@broadcom.com>
>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
>> ---
>>  .../bindings/clock/brcm,iproc-clocks.txt           |  171 ++++++++++++++++++++
>>  1 file changed, 171 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>>
>> diff --git a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>> new file mode 100644
>> index 0000000..bf2316b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>> @@ -0,0 +1,171 @@
>> +Broadcom iProc Family Clocks
>> +
>> +This binding uses the common clock binding:
>> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +
>> +The iProc clock controller manages clocks that are common to the iProc family.
>> +An SoC from the iProc family may have several PPLs, e.g., ARMPLL, GENPLL,
>> +LCPLL0, MIPIPLL, and etc., all derived from an onboard crystal. Each PLL
>> +comprises of several leaf clocks
>> +
>> +Required properties for PLLs:
>> +- compatible:
>> +    Should have a value of the form "brcm,<soc>-<pll>". For example, GENPLL on
>> +Cygnus has a compatible string of "brcm,cygnus-genpll"
>> +
>> +- #clock-cells:
>> +    Must be <0>
>> +
>> +- reg:
>> +    Define the base and range of the I/O address space that contain the iProc
>> +clock control registers required for the PLL
>> +
>> +- clocks:
>> +    The input parent clock phandle for the PLL. For all iProc PLLs, this is an
>> +onboard crystal with a fixed rate
>> +
>> +Example:
>> +
>> +       osc: oscillator {
>> +               #clock-cells = <0>;
>> +               compatible = "fixed-clock";
>> +               clock-frequency = <25000000>;
>> +       };
>> +
>> +       genpll: genpll {
>> +               #clock-cells = <0>;
>> +               compatible = "brcm,cygnus-genpll";
>> +               reg = <0x0301d000 0x2c>,
>> +                       <0x0301c020 0x4>;
>> +               clocks = <&osc>;
>> +       };
>> +
>> +Required properties for leaf clocks of a PLL:
>> +
>> +- compatible:
>> +    Should have a value of the form "brcm,<soc>-<pll>-clk". For example, leaf
>> +clocks derived from the GENPLL on Cygnus SoC have a compatible string of
>> +"brcm,cygnus-genpll-clk"
>> +
>> +- #clock-cells:
>> +    Have a value of <1> since there are more than 1 leaf clock of a
>> +given PLL
>> +
>> +- reg:
>> +    Define the base and range of the I/O address space that contain the iProc
>> +clock control registers required for the PLL leaf clocks
>> +
>> +- clocks:
>> +    The input parent PLL phandle for the leaf clock
>> +
>> +- clock-output-names:
>> +    An ordered list of strings defining the names of the leaf clocks
>> +
>> +Example:
>> +
>> +       genpll: genpll {
>> +               #clock-cells = <0>;
>> +               compatible = "brcm,cygnus-genpll";
>> +               reg = <0x0301d000 0x2c>,
>> +                       <0x0301c020 0x4>;
>> +               clocks = <&osc>;
>> +       };
>> +
>> +       genpll_clks: genpll_clks {
>> +               #clock-cells = <1>;
>> +               compatible = "brcm,cygnus-genpll-clk";
>> +               reg = <0x0301d000 0x2c>;
>> +               clocks = <&genpll>;
>> +               clock-output-names = "axi21", "250mhz", "ihost_sys",
>> +                       "enet_sw", "audio_125", "can";
>> +       };
> 
> Hi Ray,
> 
> Thanks for submitting the patch. It was nice meeting you at ELC as well.
> 
> This binding doesn't conform to the norms for clock bindings. It looks
> like for each type of controllable clock node (e.g. pll, leaf clock,
> etc) you have a dts node. Looking at the above example it seems that
> those two nodes (genpll and genpll_clks) share the same register.
> 
> /me checks patch #5
> 
> Yup, you re-use the same register address for the *pll and *pll_clks
> nodes. I'm not a DT expert but I think this is considered Wrong.
> 
> More generally your clock dt binding should be modeling the hardware in
> terms of IP blocks. If you have a clock generator IP block it may
> control many clock bits and output many clock signals. E.g. for your
> hardware (based only on the addresses in patch #5 and not having seen
> any data manual) I will hazard a guess that the genpll, lcpll and asiu
> clocks are all part of the same IP block.

Hi Mike,

In fact, lcpll, genpll, mipipll are similar but DIFFERENT IP blocks, and
asiu is completely different from any of them. All of these plls are
unique and have their own register banks, as you can see from the
bcm-cygnus-clock.dtsi file. Therefore, I think it's totally correct and
actually necessary to represent each of them with a separate device node.

Regarding the relationship between a PLL clock and its leaf clocks:
Taking the genpll as an example, physically they are connected as:

xtal -> genpll -> axi21, 250mhz, ihost_sys, ...

The 25 MHz crystal feeds to the genpll, and the genpll generates 6
different leaf clocks including axi21, 250mhz, ihost_sys, and etc. One
can choose to set the genpll's vco to one frequency, and based on that
frequency, different leaf clock frequencies can be generated with their
own post divider.

Therefore, I think it makes sense to represent xtal, genpll, genpll_clks
(including axi21, 250mhz, ihost_sys, and etc) each with a unique device
node, and genpll is the parent of these leaf clocks. Basically the
device nodes and the way how the "clocks" phandle is used represent the
hierarchy of the clock architecture within Cygnus (and in the future
other iProc SoCs). Does that make sense?

Regarding the register address overlapping, again, taking genpll as an
example, the genpll and the genpll_clks actually never access the same
set of registers, but the registers are sort of scattered within one
bank, i.e., on Cygnus, genpll uses registers at offset 0x0, 0x8 ~ 0x1c,
and genpll_clks uses registers at offset 0x4, 0x20 - 0x24.

Thanks,

Ray

> 
> If my guess is correct then these clocks should all be represented by a
> single node int DT. Lets say that the clock controller IP block that
> manages the genpll, lcpll and asiu clocks (including their child/leaf
> clocks) is called "foobar" in your data manual. You should have a dts
> node with a compatible string such as "brcm,cygnus-foobar" or
> "brcm,cygnus-foobar-clk".
> 
> Your clock driver should be responsible for registering all of the
> clocks controlled by this IP block, regardless of the "type" of clock
> node.
> 
> I think part of the reason for your approach is that the previous
> (deprecated) binding/dts had a bunch of fixed-rate clocks in it. This is
> a hack that we do every now and then to get a new platform up and
> running with a mainline kernel, but we don't do per-clock nodes in dts
> any more, we do them by clock controller ip block instead.
> 
> There are plenty of good examples of this for Exynos and QCOM SoCs if
> you want an example.
> 
> Regards,
> Mike
> 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding
  2015-04-13  4:08     ` Ray Jui
@ 2015-04-13  6:02       ` Michael Turquette
  2015-04-13 19:40         ` Ray Jui
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Turquette @ 2015-04-13  6:02 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Ray Jui (2015-04-12 21:08:32)
> 
> 
> On 4/10/2015 5:12 PM, Michael Turquette wrote:
> > Quoting Ray Jui (2015-03-17 22:45:17)
> >> Document the device tree binding for Broadcom iProc architecture based
> >> clock controller
> >>
> >> Signed-off-by: Ray Jui <rjui@broadcom.com>
> >> Reviewed-by: Scott Branden <sbranden@broadcom.com>
> >> ---
> >>  .../bindings/clock/brcm,iproc-clocks.txt           |  171 ++++++++++++++++++++
> >>  1 file changed, 171 insertions(+)
> >>  create mode 100644 Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
> >> new file mode 100644
> >> index 0000000..bf2316b
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
> >> @@ -0,0 +1,171 @@
> >> +Broadcom iProc Family Clocks
> >> +
> >> +This binding uses the common clock binding:
> >> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
> >> +
> >> +The iProc clock controller manages clocks that are common to the iProc family.
> >> +An SoC from the iProc family may have several PPLs, e.g., ARMPLL, GENPLL,
> >> +LCPLL0, MIPIPLL, and etc., all derived from an onboard crystal. Each PLL
> >> +comprises of several leaf clocks
> >> +
> >> +Required properties for PLLs:
> >> +- compatible:
> >> +    Should have a value of the form "brcm,<soc>-<pll>". For example, GENPLL on
> >> +Cygnus has a compatible string of "brcm,cygnus-genpll"
> >> +
> >> +- #clock-cells:
> >> +    Must be <0>
> >> +
> >> +- reg:
> >> +    Define the base and range of the I/O address space that contain the iProc
> >> +clock control registers required for the PLL
> >> +
> >> +- clocks:
> >> +    The input parent clock phandle for the PLL. For all iProc PLLs, this is an
> >> +onboard crystal with a fixed rate
> >> +
> >> +Example:
> >> +
> >> +       osc: oscillator {
> >> +               #clock-cells = <0>;
> >> +               compatible = "fixed-clock";
> >> +               clock-frequency = <25000000>;
> >> +       };
> >> +
> >> +       genpll: genpll {
> >> +               #clock-cells = <0>;
> >> +               compatible = "brcm,cygnus-genpll";
> >> +               reg = <0x0301d000 0x2c>,
> >> +                       <0x0301c020 0x4>;
> >> +               clocks = <&osc>;
> >> +       };
> >> +
> >> +Required properties for leaf clocks of a PLL:
> >> +
> >> +- compatible:
> >> +    Should have a value of the form "brcm,<soc>-<pll>-clk". For example, leaf
> >> +clocks derived from the GENPLL on Cygnus SoC have a compatible string of
> >> +"brcm,cygnus-genpll-clk"
> >> +
> >> +- #clock-cells:
> >> +    Have a value of <1> since there are more than 1 leaf clock of a
> >> +given PLL
> >> +
> >> +- reg:
> >> +    Define the base and range of the I/O address space that contain the iProc
> >> +clock control registers required for the PLL leaf clocks
> >> +
> >> +- clocks:
> >> +    The input parent PLL phandle for the leaf clock
> >> +
> >> +- clock-output-names:
> >> +    An ordered list of strings defining the names of the leaf clocks
> >> +
> >> +Example:
> >> +
> >> +       genpll: genpll {
> >> +               #clock-cells = <0>;
> >> +               compatible = "brcm,cygnus-genpll";
> >> +               reg = <0x0301d000 0x2c>,
> >> +                       <0x0301c020 0x4>;
> >> +               clocks = <&osc>;
> >> +       };
> >> +
> >> +       genpll_clks: genpll_clks {
> >> +               #clock-cells = <1>;
> >> +               compatible = "brcm,cygnus-genpll-clk";
> >> +               reg = <0x0301d000 0x2c>;
> >> +               clocks = <&genpll>;
> >> +               clock-output-names = "axi21", "250mhz", "ihost_sys",
> >> +                       "enet_sw", "audio_125", "can";
> >> +       };
> > 
> > Hi Ray,
> > 
> > Thanks for submitting the patch. It was nice meeting you at ELC as well.
> > 
> > This binding doesn't conform to the norms for clock bindings. It looks
> > like for each type of controllable clock node (e.g. pll, leaf clock,
> > etc) you have a dts node. Looking at the above example it seems that
> > those two nodes (genpll and genpll_clks) share the same register.
> > 
> > /me checks patch #5
> > 
> > Yup, you re-use the same register address for the *pll and *pll_clks
> > nodes. I'm not a DT expert but I think this is considered Wrong.
> > 
> > More generally your clock dt binding should be modeling the hardware in
> > terms of IP blocks. If you have a clock generator IP block it may
> > control many clock bits and output many clock signals. E.g. for your
> > hardware (based only on the addresses in patch #5 and not having seen
> > any data manual) I will hazard a guess that the genpll, lcpll and asiu
> > clocks are all part of the same IP block.
> 
> Hi Mike,
> 
> In fact, lcpll, genpll, mipipll are similar but DIFFERENT IP blocks, and
> asiu is completely different from any of them. All of these plls are
> unique and have their own register banks, as you can see from the
> bcm-cygnus-clock.dtsi file. Therefore, I think it's totally correct and
> actually necessary to represent each of them with a separate device node.

OK, that makes sense to me, if those registers live in addresses ranges
which correspond to different IP blocks.

> 
> Regarding the relationship between a PLL clock and its leaf clocks:
> Taking the genpll as an example, physically they are connected as:
> 
> xtal -> genpll -> axi21, 250mhz, ihost_sys, ...
> 
> The 25 MHz crystal feeds to the genpll, and the genpll generates 6
> different leaf clocks including axi21, 250mhz, ihost_sys, and etc. One
> can choose to set the genpll's vco to one frequency, and based on that
> frequency, different leaf clock frequencies can be generated with their
> own post divider.
> 
> Therefore, I think it makes sense to represent xtal, genpll, genpll_clks
> (including axi21, 250mhz, ihost_sys, and etc) each with a unique device
> node, and genpll is the parent of these leaf clocks. Basically the
> device nodes and the way how the "clocks" phandle is used represent the
> hierarchy of the clock architecture within Cygnus (and in the future
> other iProc SoCs). Does that make sense?

This doesn't make sense to me. If I understand correctly, the register
range that controls the post-divider clock (e.g. axi21) is the same
register range that control's genpll. This is a reasonable indicator to
me that the leaf clocks are part of the same clock generator IP block as
the PLL controls. As such they should be on node.

> 
> Regarding the register address overlapping, again, taking genpll as an
> example, the genpll and the genpll_clks actually never access the same
> set of registers, but the registers are sort of scattered within one
> bank, i.e., on Cygnus, genpll uses registers at offset 0x0, 0x8 ~ 0x1c,
> and genpll_clks uses registers at offset 0x4, 0x20 - 0x24.

Sure, my argument above doesn't hinge on the fact that the pll and child
clocks use the exact same register. It still looks to me like *pll and
it's child dividers belong in the same IP block. Is there an open data
sheet or technical reference manual I can look at for this part? That is
the best way to put my concerns at ease ;-)

Looking over your binding again, it looks like your nodes are divided
conveniently for the different clock types (e.g. pll versus
post-divider), but our goal with DT is to accurately describe the
hardware, not the C structures.

Regards,
Mike

> 
> Thanks,
> 
> Ray
> 
> > 
> > If my guess is correct then these clocks should all be represented by a
> > single node int DT. Lets say that the clock controller IP block that
> > manages the genpll, lcpll and asiu clocks (including their child/leaf
> > clocks) is called "foobar" in your data manual. You should have a dts
> > node with a compatible string such as "brcm,cygnus-foobar" or
> > "brcm,cygnus-foobar-clk".
> > 
> > Your clock driver should be responsible for registering all of the
> > clocks controlled by this IP block, regardless of the "type" of clock
> > node.
> > 
> > I think part of the reason for your approach is that the previous
> > (deprecated) binding/dts had a bunch of fixed-rate clocks in it. This is
> > a hack that we do every now and then to get a new platform up and
> > running with a mainline kernel, but we don't do per-clock nodes in dts
> > any more, we do them by clock controller ip block instead.
> > 
> > There are plenty of good examples of this for Exynos and QCOM SoCs if
> > you want an example.
> > 
> > Regards,
> > Mike
> > 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding
  2015-04-13  6:02       ` Michael Turquette
@ 2015-04-13 19:40         ` Ray Jui
  2015-04-14 19:10           ` Ray Jui
  0 siblings, 1 reply; 16+ messages in thread
From: Ray Jui @ 2015-04-13 19:40 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mike,

On 4/12/2015 11:02 PM, Michael Turquette wrote:
> Quoting Ray Jui (2015-04-12 21:08:32)
>>
>>
>> On 4/10/2015 5:12 PM, Michael Turquette wrote:
>>> Quoting Ray Jui (2015-03-17 22:45:17)
>>>> Document the device tree binding for Broadcom iProc architecture based
>>>> clock controller
>>>>
>>>> Signed-off-by: Ray Jui <rjui@broadcom.com>
>>>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
>>>> ---
>>>>  .../bindings/clock/brcm,iproc-clocks.txt           |  171 ++++++++++++++++++++
>>>>  1 file changed, 171 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>>>> new file mode 100644
>>>> index 0000000..bf2316b
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>>>> @@ -0,0 +1,171 @@
>>>> +Broadcom iProc Family Clocks
>>>> +
>>>> +This binding uses the common clock binding:
>>>> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>> +
>>>> +The iProc clock controller manages clocks that are common to the iProc family.
>>>> +An SoC from the iProc family may have several PPLs, e.g., ARMPLL, GENPLL,
>>>> +LCPLL0, MIPIPLL, and etc., all derived from an onboard crystal. Each PLL
>>>> +comprises of several leaf clocks
>>>> +
>>>> +Required properties for PLLs:
>>>> +- compatible:
>>>> +    Should have a value of the form "brcm,<soc>-<pll>". For example, GENPLL on
>>>> +Cygnus has a compatible string of "brcm,cygnus-genpll"
>>>> +
>>>> +- #clock-cells:
>>>> +    Must be <0>
>>>> +
>>>> +- reg:
>>>> +    Define the base and range of the I/O address space that contain the iProc
>>>> +clock control registers required for the PLL
>>>> +
>>>> +- clocks:
>>>> +    The input parent clock phandle for the PLL. For all iProc PLLs, this is an
>>>> +onboard crystal with a fixed rate
>>>> +
>>>> +Example:
>>>> +
>>>> +       osc: oscillator {
>>>> +               #clock-cells = <0>;
>>>> +               compatible = "fixed-clock";
>>>> +               clock-frequency = <25000000>;
>>>> +       };
>>>> +
>>>> +       genpll: genpll {
>>>> +               #clock-cells = <0>;
>>>> +               compatible = "brcm,cygnus-genpll";
>>>> +               reg = <0x0301d000 0x2c>,
>>>> +                       <0x0301c020 0x4>;
>>>> +               clocks = <&osc>;
>>>> +       };
>>>> +
>>>> +Required properties for leaf clocks of a PLL:
>>>> +
>>>> +- compatible:
>>>> +    Should have a value of the form "brcm,<soc>-<pll>-clk". For example, leaf
>>>> +clocks derived from the GENPLL on Cygnus SoC have a compatible string of
>>>> +"brcm,cygnus-genpll-clk"
>>>> +
>>>> +- #clock-cells:
>>>> +    Have a value of <1> since there are more than 1 leaf clock of a
>>>> +given PLL
>>>> +
>>>> +- reg:
>>>> +    Define the base and range of the I/O address space that contain the iProc
>>>> +clock control registers required for the PLL leaf clocks
>>>> +
>>>> +- clocks:
>>>> +    The input parent PLL phandle for the leaf clock
>>>> +
>>>> +- clock-output-names:
>>>> +    An ordered list of strings defining the names of the leaf clocks
>>>> +
>>>> +Example:
>>>> +
>>>> +       genpll: genpll {
>>>> +               #clock-cells = <0>;
>>>> +               compatible = "brcm,cygnus-genpll";
>>>> +               reg = <0x0301d000 0x2c>,
>>>> +                       <0x0301c020 0x4>;
>>>> +               clocks = <&osc>;
>>>> +       };
>>>> +
>>>> +       genpll_clks: genpll_clks {
>>>> +               #clock-cells = <1>;
>>>> +               compatible = "brcm,cygnus-genpll-clk";
>>>> +               reg = <0x0301d000 0x2c>;
>>>> +               clocks = <&genpll>;
>>>> +               clock-output-names = "axi21", "250mhz", "ihost_sys",
>>>> +                       "enet_sw", "audio_125", "can";
>>>> +       };
>>>
>>> Hi Ray,
>>>
>>> Thanks for submitting the patch. It was nice meeting you at ELC as well.
>>>
>>> This binding doesn't conform to the norms for clock bindings. It looks
>>> like for each type of controllable clock node (e.g. pll, leaf clock,
>>> etc) you have a dts node. Looking at the above example it seems that
>>> those two nodes (genpll and genpll_clks) share the same register.
>>>
>>> /me checks patch #5
>>>
>>> Yup, you re-use the same register address for the *pll and *pll_clks
>>> nodes. I'm not a DT expert but I think this is considered Wrong.
>>>
>>> More generally your clock dt binding should be modeling the hardware in
>>> terms of IP blocks. If you have a clock generator IP block it may
>>> control many clock bits and output many clock signals. E.g. for your
>>> hardware (based only on the addresses in patch #5 and not having seen
>>> any data manual) I will hazard a guess that the genpll, lcpll and asiu
>>> clocks are all part of the same IP block.
>>
>> Hi Mike,
>>
>> In fact, lcpll, genpll, mipipll are similar but DIFFERENT IP blocks, and
>> asiu is completely different from any of them. All of these plls are
>> unique and have their own register banks, as you can see from the
>> bcm-cygnus-clock.dtsi file. Therefore, I think it's totally correct and
>> actually necessary to represent each of them with a separate device node.
> 
> OK, that makes sense to me, if those registers live in addresses ranges
> which correspond to different IP blocks.
> 
>>
>> Regarding the relationship between a PLL clock and its leaf clocks:
>> Taking the genpll as an example, physically they are connected as:
>>
>> xtal -> genpll -> axi21, 250mhz, ihost_sys, ...
>>
>> The 25 MHz crystal feeds to the genpll, and the genpll generates 6
>> different leaf clocks including axi21, 250mhz, ihost_sys, and etc. One
>> can choose to set the genpll's vco to one frequency, and based on that
>> frequency, different leaf clock frequencies can be generated with their
>> own post divider.
>>
>> Therefore, I think it makes sense to represent xtal, genpll, genpll_clks
>> (including axi21, 250mhz, ihost_sys, and etc) each with a unique device
>> node, and genpll is the parent of these leaf clocks. Basically the
>> device nodes and the way how the "clocks" phandle is used represent the
>> hierarchy of the clock architecture within Cygnus (and in the future
>> other iProc SoCs). Does that make sense?
> 
> This doesn't make sense to me. If I understand correctly, the register
> range that controls the post-divider clock (e.g. axi21) is the same
> register range that control's genpll. This is a reasonable indicator to
> me that the leaf clocks are part of the same clock generator IP block as
> the PLL controls. As such they should be on node.
> 
>>
>> Regarding the register address overlapping, again, taking genpll as an
>> example, the genpll and the genpll_clks actually never access the same
>> set of registers, but the registers are sort of scattered within one
>> bank, i.e., on Cygnus, genpll uses registers at offset 0x0, 0x8 ~ 0x1c,
>> and genpll_clks uses registers at offset 0x4, 0x20 - 0x24.
> 
> Sure, my argument above doesn't hinge on the fact that the pll and child
> clocks use the exact same register. It still looks to me like *pll and
> it's child dividers belong in the same IP block. Is there an open data
> sheet or technical reference manual I can look at for this part? That is
> the best way to put my concerns at ease ;-)
> 
> Looking over your binding again, it looks like your nodes are divided
> conveniently for the different clock types (e.g. pll versus
> post-divider), but our goal with DT is to accurately describe the
> hardware, not the C structures.
> 
> Regards,
> Mike
> 

Yes, the genpll and genpll_clks are controlled by the same IP block. I
can make the change to combine them to use one DT node. But before doing
that, I just need to get one thing clarified with you. In SW, the pll
and its leaf clocks will still be registered as separate two clock
providers, since we need to be able to configure the PLL vco frequency
and its leaf clock frequencies separately. The pll will be the parent of
its leaf clocks, and the leaf clocks will be used by various peripherals.

I plan to have the combined DT looks like this:

genpll_clks: genpll_clks {
    #clock-cells = <1>;
    compatible = "brcm,cygnus-genpll-clk";
    reg = <0x0301d000 0x2c>;

    assigned-clocks = <&genpll_clks>;
    /* this sets the PLL rate at the time when the PLL clock is
registered */
    assigned-clock-rates = <4000000000>;

    clocks = <&osc>;

    clock-output-names = "axi21", "250mhz", "ihost_sys", "enet_sw",
"audio_125", "can";
};

peripheralA: peripheralA {
    /* use the first leaf clock of genpll */
    clocks = <&genpll_clks 0>;
    clock-frequency  = <100000000>;
};

If we register both the genpll and its leaf clocks to the clock
framework as two separate clock providers, would the above DT entries
still work? For example, peripheralA refers to the phandle of
genpll_clks, but how does the clock framework know to link it to the pll
clock or the leaf clock?

Thanks,

Ray

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding
  2015-04-13 19:40         ` Ray Jui
@ 2015-04-14 19:10           ` Ray Jui
  2015-04-16 19:20             ` Michael Turquette
  0 siblings, 1 reply; 16+ messages in thread
From: Ray Jui @ 2015-04-14 19:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mike,

On 4/13/2015 12:40 PM, Ray Jui wrote:
> Hi Mike,
> 
> On 4/12/2015 11:02 PM, Michael Turquette wrote:
>> Quoting Ray Jui (2015-04-12 21:08:32)
>>>
>>>
>>> On 4/10/2015 5:12 PM, Michael Turquette wrote:
>>>> Quoting Ray Jui (2015-03-17 22:45:17)
>>>>> Document the device tree binding for Broadcom iProc architecture based
>>>>> clock controller
>>>>>
>>>>> Signed-off-by: Ray Jui <rjui@broadcom.com>
>>>>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
>>>>> ---
>>>>>  .../bindings/clock/brcm,iproc-clocks.txt           |  171 ++++++++++++++++++++
>>>>>  1 file changed, 171 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>>>>> new file mode 100644
>>>>> index 0000000..bf2316b
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>>>>> @@ -0,0 +1,171 @@
>>>>> +Broadcom iProc Family Clocks
>>>>> +
>>>>> +This binding uses the common clock binding:
>>>>> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>>> +
>>>>> +The iProc clock controller manages clocks that are common to the iProc family.
>>>>> +An SoC from the iProc family may have several PPLs, e.g., ARMPLL, GENPLL,
>>>>> +LCPLL0, MIPIPLL, and etc., all derived from an onboard crystal. Each PLL
>>>>> +comprises of several leaf clocks
>>>>> +
>>>>> +Required properties for PLLs:
>>>>> +- compatible:
>>>>> +    Should have a value of the form "brcm,<soc>-<pll>". For example, GENPLL on
>>>>> +Cygnus has a compatible string of "brcm,cygnus-genpll"
>>>>> +
>>>>> +- #clock-cells:
>>>>> +    Must be <0>
>>>>> +
>>>>> +- reg:
>>>>> +    Define the base and range of the I/O address space that contain the iProc
>>>>> +clock control registers required for the PLL
>>>>> +
>>>>> +- clocks:
>>>>> +    The input parent clock phandle for the PLL. For all iProc PLLs, this is an
>>>>> +onboard crystal with a fixed rate
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> +       osc: oscillator {
>>>>> +               #clock-cells = <0>;
>>>>> +               compatible = "fixed-clock";
>>>>> +               clock-frequency = <25000000>;
>>>>> +       };
>>>>> +
>>>>> +       genpll: genpll {
>>>>> +               #clock-cells = <0>;
>>>>> +               compatible = "brcm,cygnus-genpll";
>>>>> +               reg = <0x0301d000 0x2c>,
>>>>> +                       <0x0301c020 0x4>;
>>>>> +               clocks = <&osc>;
>>>>> +       };
>>>>> +
>>>>> +Required properties for leaf clocks of a PLL:
>>>>> +
>>>>> +- compatible:
>>>>> +    Should have a value of the form "brcm,<soc>-<pll>-clk". For example, leaf
>>>>> +clocks derived from the GENPLL on Cygnus SoC have a compatible string of
>>>>> +"brcm,cygnus-genpll-clk"
>>>>> +
>>>>> +- #clock-cells:
>>>>> +    Have a value of <1> since there are more than 1 leaf clock of a
>>>>> +given PLL
>>>>> +
>>>>> +- reg:
>>>>> +    Define the base and range of the I/O address space that contain the iProc
>>>>> +clock control registers required for the PLL leaf clocks
>>>>> +
>>>>> +- clocks:
>>>>> +    The input parent PLL phandle for the leaf clock
>>>>> +
>>>>> +- clock-output-names:
>>>>> +    An ordered list of strings defining the names of the leaf clocks
>>>>> +
>>>>> +Example:
>>>>> +
>>>>> +       genpll: genpll {
>>>>> +               #clock-cells = <0>;
>>>>> +               compatible = "brcm,cygnus-genpll";
>>>>> +               reg = <0x0301d000 0x2c>,
>>>>> +                       <0x0301c020 0x4>;
>>>>> +               clocks = <&osc>;
>>>>> +       };
>>>>> +
>>>>> +       genpll_clks: genpll_clks {
>>>>> +               #clock-cells = <1>;
>>>>> +               compatible = "brcm,cygnus-genpll-clk";
>>>>> +               reg = <0x0301d000 0x2c>;
>>>>> +               clocks = <&genpll>;
>>>>> +               clock-output-names = "axi21", "250mhz", "ihost_sys",
>>>>> +                       "enet_sw", "audio_125", "can";
>>>>> +       };
>>>>
>>>> Hi Ray,
>>>>
>>>> Thanks for submitting the patch. It was nice meeting you at ELC as well.
>>>>
>>>> This binding doesn't conform to the norms for clock bindings. It looks
>>>> like for each type of controllable clock node (e.g. pll, leaf clock,
>>>> etc) you have a dts node. Looking at the above example it seems that
>>>> those two nodes (genpll and genpll_clks) share the same register.
>>>>
>>>> /me checks patch #5
>>>>
>>>> Yup, you re-use the same register address for the *pll and *pll_clks
>>>> nodes. I'm not a DT expert but I think this is considered Wrong.
>>>>
>>>> More generally your clock dt binding should be modeling the hardware in
>>>> terms of IP blocks. If you have a clock generator IP block it may
>>>> control many clock bits and output many clock signals. E.g. for your
>>>> hardware (based only on the addresses in patch #5 and not having seen
>>>> any data manual) I will hazard a guess that the genpll, lcpll and asiu
>>>> clocks are all part of the same IP block.
>>>
>>> Hi Mike,
>>>
>>> In fact, lcpll, genpll, mipipll are similar but DIFFERENT IP blocks, and
>>> asiu is completely different from any of them. All of these plls are
>>> unique and have their own register banks, as you can see from the
>>> bcm-cygnus-clock.dtsi file. Therefore, I think it's totally correct and
>>> actually necessary to represent each of them with a separate device node.
>>
>> OK, that makes sense to me, if those registers live in addresses ranges
>> which correspond to different IP blocks.
>>
>>>
>>> Regarding the relationship between a PLL clock and its leaf clocks:
>>> Taking the genpll as an example, physically they are connected as:
>>>
>>> xtal -> genpll -> axi21, 250mhz, ihost_sys, ...
>>>
>>> The 25 MHz crystal feeds to the genpll, and the genpll generates 6
>>> different leaf clocks including axi21, 250mhz, ihost_sys, and etc. One
>>> can choose to set the genpll's vco to one frequency, and based on that
>>> frequency, different leaf clock frequencies can be generated with their
>>> own post divider.
>>>
>>> Therefore, I think it makes sense to represent xtal, genpll, genpll_clks
>>> (including axi21, 250mhz, ihost_sys, and etc) each with a unique device
>>> node, and genpll is the parent of these leaf clocks. Basically the
>>> device nodes and the way how the "clocks" phandle is used represent the
>>> hierarchy of the clock architecture within Cygnus (and in the future
>>> other iProc SoCs). Does that make sense?
>>
>> This doesn't make sense to me. If I understand correctly, the register
>> range that controls the post-divider clock (e.g. axi21) is the same
>> register range that control's genpll. This is a reasonable indicator to
>> me that the leaf clocks are part of the same clock generator IP block as
>> the PLL controls. As such they should be on node.
>>
>>>
>>> Regarding the register address overlapping, again, taking genpll as an
>>> example, the genpll and the genpll_clks actually never access the same
>>> set of registers, but the registers are sort of scattered within one
>>> bank, i.e., on Cygnus, genpll uses registers at offset 0x0, 0x8 ~ 0x1c,
>>> and genpll_clks uses registers at offset 0x4, 0x20 - 0x24.
>>
>> Sure, my argument above doesn't hinge on the fact that the pll and child
>> clocks use the exact same register. It still looks to me like *pll and
>> it's child dividers belong in the same IP block. Is there an open data
>> sheet or technical reference manual I can look at for this part? That is
>> the best way to put my concerns at ease ;-)
>>
>> Looking over your binding again, it looks like your nodes are divided
>> conveniently for the different clock types (e.g. pll versus
>> post-divider), but our goal with DT is to accurately describe the
>> hardware, not the C structures.
>>
>> Regards,
>> Mike
>>
> 
> Yes, the genpll and genpll_clks are controlled by the same IP block. I
> can make the change to combine them to use one DT node. But before doing
> that, I just need to get one thing clarified with you. In SW, the pll
> and its leaf clocks will still be registered as separate two clock
> providers, since we need to be able to configure the PLL vco frequency
> and its leaf clock frequencies separately. The pll will be the parent of
> its leaf clocks, and the leaf clocks will be used by various peripherals.
> 
> I plan to have the combined DT looks like this:
> 
> genpll_clks: genpll_clks {
>     #clock-cells = <1>;
>     compatible = "brcm,cygnus-genpll-clk";
>     reg = <0x0301d000 0x2c>;
> 
>     assigned-clocks = <&genpll_clks>;
>     /* this sets the PLL rate at the time when the PLL clock is
> registered */
>     assigned-clock-rates = <4000000000>;
> 
>     clocks = <&osc>;
> 
>     clock-output-names = "axi21", "250mhz", "ihost_sys", "enet_sw",
> "audio_125", "can";
> };
> 
> peripheralA: peripheralA {
>     /* use the first leaf clock of genpll */
>     clocks = <&genpll_clks 0>;
>     clock-frequency  = <100000000>;
> };
> 
> If we register both the genpll and its leaf clocks to the clock
> framework as two separate clock providers, would the above DT entries
> still work? For example, peripheralA refers to the phandle of
> genpll_clks, but how does the clock framework know to link it to the pll
> clock or the leaf clock?
> 
> Thanks,
> 
> Ray
> 

So I've changed the code to merge pll and pll_clks nodes and have played
around with it a bit. Let me try to explain the issue I'm having here,
and see if you can help to shed some light:

As far as I know, the correct way to configure a clock's rate at the
time of registering to the clock framework, as you mentioned previously,
is to use the "assigned-clock" and "assigned-clock-rates" properties.

Assuming we have a pll clock called "pll", with 6 leaf clocks derived
from the pll called "clk-0", "clk-1", etc., and a peripheral using the
first leaf clock: "clk-0".

pll: pll {
   #clock-cell = <1>;
   compatible = "brcm,pll";
   clocks = <&osc>;
   clock-output-names = "clk-0", "clk-1", ...;
};

peripheral: peripheral {
    clocks = <&pll 0>;
};

With your proposed way of constructing the above clock device tree,
i.e., to combine "pll" and "clk-x" to use the same DT node, now how can
one configure the frequency of the "pll" through device tree at the time
of clock registration? There's now no access to "pll" through device
tree but only access to its leaf clocks, e.g., clocks = <&pll 0>.

The requirement comes from the fact that we need the peripheral to run
at a certain clock rate off the leaf clock, and to achieve that we need
the pll to run at a specific rate, since we only have an integer based
post-divider between the pll and its leaf clocks. I was able to achieve
this previously by separating the pll and its leaf clocks into two DT
nodes, but I am really not sure how one can do this when merging pll and
its leaf clocks into the same node.

Any idea how this can be done cleanly with a single DT node representing
the pll and its leaf clocks?

Thanks,

Ray

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding
  2015-04-14 19:10           ` Ray Jui
@ 2015-04-16 19:20             ` Michael Turquette
  2015-04-16 21:01               ` Ray Jui
  0 siblings, 1 reply; 16+ messages in thread
From: Michael Turquette @ 2015-04-16 19:20 UTC (permalink / raw)
  To: linux-arm-kernel

Quoting Ray Jui (2015-04-14 12:10:35)
> Hi Mike,
> 
> On 4/13/2015 12:40 PM, Ray Jui wrote:
> > Hi Mike,
> > 
> > On 4/12/2015 11:02 PM, Michael Turquette wrote:
> >> Quoting Ray Jui (2015-04-12 21:08:32)
> >>>
> >>>
> >>> On 4/10/2015 5:12 PM, Michael Turquette wrote:
> >>>> Quoting Ray Jui (2015-03-17 22:45:17)
> >>>>> Document the device tree binding for Broadcom iProc architecture based
> >>>>> clock controller
> >>>>>
> >>>>> Signed-off-by: Ray Jui <rjui@broadcom.com>
> >>>>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
> >>>>> ---
> >>>>>  .../bindings/clock/brcm,iproc-clocks.txt           |  171 ++++++++++++++++++++
> >>>>>  1 file changed, 171 insertions(+)
> >>>>>  create mode 100644 Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
> >>>>> new file mode 100644
> >>>>> index 0000000..bf2316b
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
> >>>>> @@ -0,0 +1,171 @@
> >>>>> +Broadcom iProc Family Clocks
> >>>>> +
> >>>>> +This binding uses the common clock binding:
> >>>>> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
> >>>>> +
> >>>>> +The iProc clock controller manages clocks that are common to the iProc family.
> >>>>> +An SoC from the iProc family may have several PPLs, e.g., ARMPLL, GENPLL,
> >>>>> +LCPLL0, MIPIPLL, and etc., all derived from an onboard crystal. Each PLL
> >>>>> +comprises of several leaf clocks
> >>>>> +
> >>>>> +Required properties for PLLs:
> >>>>> +- compatible:
> >>>>> +    Should have a value of the form "brcm,<soc>-<pll>". For example, GENPLL on
> >>>>> +Cygnus has a compatible string of "brcm,cygnus-genpll"
> >>>>> +
> >>>>> +- #clock-cells:
> >>>>> +    Must be <0>
> >>>>> +
> >>>>> +- reg:
> >>>>> +    Define the base and range of the I/O address space that contain the iProc
> >>>>> +clock control registers required for the PLL
> >>>>> +
> >>>>> +- clocks:
> >>>>> +    The input parent clock phandle for the PLL. For all iProc PLLs, this is an
> >>>>> +onboard crystal with a fixed rate
> >>>>> +
> >>>>> +Example:
> >>>>> +
> >>>>> +       osc: oscillator {
> >>>>> +               #clock-cells = <0>;
> >>>>> +               compatible = "fixed-clock";
> >>>>> +               clock-frequency = <25000000>;
> >>>>> +       };
> >>>>> +
> >>>>> +       genpll: genpll {
> >>>>> +               #clock-cells = <0>;
> >>>>> +               compatible = "brcm,cygnus-genpll";
> >>>>> +               reg = <0x0301d000 0x2c>,
> >>>>> +                       <0x0301c020 0x4>;
> >>>>> +               clocks = <&osc>;
> >>>>> +       };
> >>>>> +
> >>>>> +Required properties for leaf clocks of a PLL:
> >>>>> +
> >>>>> +- compatible:
> >>>>> +    Should have a value of the form "brcm,<soc>-<pll>-clk". For example, leaf
> >>>>> +clocks derived from the GENPLL on Cygnus SoC have a compatible string of
> >>>>> +"brcm,cygnus-genpll-clk"
> >>>>> +
> >>>>> +- #clock-cells:
> >>>>> +    Have a value of <1> since there are more than 1 leaf clock of a
> >>>>> +given PLL
> >>>>> +
> >>>>> +- reg:
> >>>>> +    Define the base and range of the I/O address space that contain the iProc
> >>>>> +clock control registers required for the PLL leaf clocks
> >>>>> +
> >>>>> +- clocks:
> >>>>> +    The input parent PLL phandle for the leaf clock
> >>>>> +
> >>>>> +- clock-output-names:
> >>>>> +    An ordered list of strings defining the names of the leaf clocks
> >>>>> +
> >>>>> +Example:
> >>>>> +
> >>>>> +       genpll: genpll {
> >>>>> +               #clock-cells = <0>;
> >>>>> +               compatible = "brcm,cygnus-genpll";
> >>>>> +               reg = <0x0301d000 0x2c>,
> >>>>> +                       <0x0301c020 0x4>;
> >>>>> +               clocks = <&osc>;
> >>>>> +       };
> >>>>> +
> >>>>> +       genpll_clks: genpll_clks {
> >>>>> +               #clock-cells = <1>;
> >>>>> +               compatible = "brcm,cygnus-genpll-clk";
> >>>>> +               reg = <0x0301d000 0x2c>;
> >>>>> +               clocks = <&genpll>;
> >>>>> +               clock-output-names = "axi21", "250mhz", "ihost_sys",
> >>>>> +                       "enet_sw", "audio_125", "can";
> >>>>> +       };
> >>>>
> >>>> Hi Ray,
> >>>>
> >>>> Thanks for submitting the patch. It was nice meeting you at ELC as well.
> >>>>
> >>>> This binding doesn't conform to the norms for clock bindings. It looks
> >>>> like for each type of controllable clock node (e.g. pll, leaf clock,
> >>>> etc) you have a dts node. Looking at the above example it seems that
> >>>> those two nodes (genpll and genpll_clks) share the same register.
> >>>>
> >>>> /me checks patch #5
> >>>>
> >>>> Yup, you re-use the same register address for the *pll and *pll_clks
> >>>> nodes. I'm not a DT expert but I think this is considered Wrong.
> >>>>
> >>>> More generally your clock dt binding should be modeling the hardware in
> >>>> terms of IP blocks. If you have a clock generator IP block it may
> >>>> control many clock bits and output many clock signals. E.g. for your
> >>>> hardware (based only on the addresses in patch #5 and not having seen
> >>>> any data manual) I will hazard a guess that the genpll, lcpll and asiu
> >>>> clocks are all part of the same IP block.
> >>>
> >>> Hi Mike,
> >>>
> >>> In fact, lcpll, genpll, mipipll are similar but DIFFERENT IP blocks, and
> >>> asiu is completely different from any of them. All of these plls are
> >>> unique and have their own register banks, as you can see from the
> >>> bcm-cygnus-clock.dtsi file. Therefore, I think it's totally correct and
> >>> actually necessary to represent each of them with a separate device node.
> >>
> >> OK, that makes sense to me, if those registers live in addresses ranges
> >> which correspond to different IP blocks.
> >>
> >>>
> >>> Regarding the relationship between a PLL clock and its leaf clocks:
> >>> Taking the genpll as an example, physically they are connected as:
> >>>
> >>> xtal -> genpll -> axi21, 250mhz, ihost_sys, ...
> >>>
> >>> The 25 MHz crystal feeds to the genpll, and the genpll generates 6
> >>> different leaf clocks including axi21, 250mhz, ihost_sys, and etc. One
> >>> can choose to set the genpll's vco to one frequency, and based on that
> >>> frequency, different leaf clock frequencies can be generated with their
> >>> own post divider.
> >>>
> >>> Therefore, I think it makes sense to represent xtal, genpll, genpll_clks
> >>> (including axi21, 250mhz, ihost_sys, and etc) each with a unique device
> >>> node, and genpll is the parent of these leaf clocks. Basically the
> >>> device nodes and the way how the "clocks" phandle is used represent the
> >>> hierarchy of the clock architecture within Cygnus (and in the future
> >>> other iProc SoCs). Does that make sense?
> >>
> >> This doesn't make sense to me. If I understand correctly, the register
> >> range that controls the post-divider clock (e.g. axi21) is the same
> >> register range that control's genpll. This is a reasonable indicator to
> >> me that the leaf clocks are part of the same clock generator IP block as
> >> the PLL controls. As such they should be on node.
> >>
> >>>
> >>> Regarding the register address overlapping, again, taking genpll as an
> >>> example, the genpll and the genpll_clks actually never access the same
> >>> set of registers, but the registers are sort of scattered within one
> >>> bank, i.e., on Cygnus, genpll uses registers at offset 0x0, 0x8 ~ 0x1c,
> >>> and genpll_clks uses registers at offset 0x4, 0x20 - 0x24.
> >>
> >> Sure, my argument above doesn't hinge on the fact that the pll and child
> >> clocks use the exact same register. It still looks to me like *pll and
> >> it's child dividers belong in the same IP block. Is there an open data
> >> sheet or technical reference manual I can look at for this part? That is
> >> the best way to put my concerns at ease ;-)
> >>
> >> Looking over your binding again, it looks like your nodes are divided
> >> conveniently for the different clock types (e.g. pll versus
> >> post-divider), but our goal with DT is to accurately describe the
> >> hardware, not the C structures.
> >>
> >> Regards,
> >> Mike
> >>
> > 
> > Yes, the genpll and genpll_clks are controlled by the same IP block. I
> > can make the change to combine them to use one DT node. But before doing
> > that, I just need to get one thing clarified with you. In SW, the pll
> > and its leaf clocks will still be registered as separate two clock
> > providers, since we need to be able to configure the PLL vco frequency
> > and its leaf clock frequencies separately. The pll will be the parent of
> > its leaf clocks, and the leaf clocks will be used by various peripherals.
> > 
> > I plan to have the combined DT looks like this:
> > 
> > genpll_clks: genpll_clks {
> >     #clock-cells = <1>;
> >     compatible = "brcm,cygnus-genpll-clk";
> >     reg = <0x0301d000 0x2c>;
> > 
> >     assigned-clocks = <&genpll_clks>;
> >     /* this sets the PLL rate at the time when the PLL clock is
> > registered */
> >     assigned-clock-rates = <4000000000>;
> > 
> >     clocks = <&osc>;
> > 
> >     clock-output-names = "axi21", "250mhz", "ihost_sys", "enet_sw",
> > "audio_125", "can";
> > };
> > 
> > peripheralA: peripheralA {
> >     /* use the first leaf clock of genpll */
> >     clocks = <&genpll_clks 0>;
> >     clock-frequency  = <100000000>;
> > };
> > 
> > If we register both the genpll and its leaf clocks to the clock
> > framework as two separate clock providers, would the above DT entries
> > still work? For example, peripheralA refers to the phandle of
> > genpll_clks, but how does the clock framework know to link it to the pll
> > clock or the leaf clock?

Hi Ray,

There are two options here: using clock-output-names or not using
clock-output-names.

I would recommend against using clock-output-names. If both your
clock driver and the peripheral devices that consume these clocks are
all represented in DT then you can skip clock-output-names and instead
create phandle linkage between the clocks inside of your clock provider
and the consumer devices. String names are still important here, but now
you only need to specify the string name as an *input* to the consumer
device, and not as an *output* from the clock provider node. Let's look
at how the qcom bindings do it:

gcc is a clock generator ip block.

arch/arm/boot/dts/qcom-apq8084.dtsi:
                gcc: clock-controller at fc400000 {
                        compatible = "qcom,gcc-apq8084";
                        #clock-cells = <1>;
                        #reset-cells = <1>;
                        reg = <0xfc400000 0x4000>;
                };

This same file includes this header:
#include <dt-bindings/clock/qcom,gcc-apq8084.h>

That header creates a map of clocks by numbers. An example:
#define GCC_BLSP2_UART2_APPS_CLK                        142

We can see how it used by the serial controller in
arch/arm/boot/dts/qcom-apq8084.dtsi:
                serial at f995e000 {
                        compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
                        reg = <0xf995e000 0x1000>;
                        interrupts = <0 114 0x0>;
                        clocks = <&gcc GCC_BLSP2_UART2_APPS_CLK>, <&gcc GCC_BLSP2_AHB_CLK>;
                        clock-names = "core", "iface";
                        status = "disabled";
                };

Note the 'clock-names' property. This lists a string name which is a property
of the *consuming* device (uart controller), not the *providing* device (gcc
clock generator).

Finally, it all comes together in the driver simply using clk_get:
        msm_port->clk = devm_clk_get(&pdev->dev, "core");
        if (IS_ERR(msm_port->clk))
                return PTR_ERR(msm_port->clk);

        if (msm_port->is_uartdm) {
                msm_port->pclk = devm_clk_get(&pdev->dev, "iface");
                if (IS_ERR(msm_port->pclk))
                        return PTR_ERR(msm_port->pclk);

                clk_set_rate(msm_port->clk, 1843200);
        }

This works because we have created linkage between the clock phandle and
the consuming device node. The shared header is used by the Linux kernel
to look up the clock as well as the consumer node to map the string name
onto a clock input.

Back to clock-output-names:

If you decide to keep using clock-output-names you can still represent
the pll clock in your "clock-output-names". In a perfect world DT nodes
that use clock-output-names would only expose the leaf clocks that your
peripheral devices consume, but there is no technical reason why your
pll can't be a part of the clock-output-names array. It is simply an
array of string names that maps to an index. There is no sense of
parent-child hierarchy in the DT binding, so there is no problem adding
a parent pll clock to the array of clock-output-names. I think that this
will tactically solve your problem in the short term, but I encourage
you to inspect the qcom binding example I gave above to see if it a
better fit for you long term.

Regards,
Mike

> > 
> > Thanks,
> > 
> > Ray
> > 
> 
> So I've changed the code to merge pll and pll_clks nodes and have played
> around with it a bit. Let me try to explain the issue I'm having here,
> and see if you can help to shed some light:
> 
> As far as I know, the correct way to configure a clock's rate at the
> time of registering to the clock framework, as you mentioned previously,
> is to use the "assigned-clock" and "assigned-clock-rates" properties.
> 
> Assuming we have a pll clock called "pll", with 6 leaf clocks derived
> from the pll called "clk-0", "clk-1", etc., and a peripheral using the
> first leaf clock: "clk-0".
> 
> pll: pll {
>    #clock-cell = <1>;
>    compatible = "brcm,pll";
>    clocks = <&osc>;
>    clock-output-names = "clk-0", "clk-1", ...;
> };
> 
> peripheral: peripheral {
>     clocks = <&pll 0>;
> };
> 
> With your proposed way of constructing the above clock device tree,
> i.e., to combine "pll" and "clk-x" to use the same DT node, now how can
> one configure the frequency of the "pll" through device tree at the time
> of clock registration? There's now no access to "pll" through device
> tree but only access to its leaf clocks, e.g., clocks = <&pll 0>.
> 
> The requirement comes from the fact that we need the peripheral to run
> at a certain clock rate off the leaf clock, and to achieve that we need
> the pll to run at a specific rate, since we only have an integer based
> post-divider between the pll and its leaf clocks. I was able to achieve
> this previously by separating the pll and its leaf clocks into two DT
> nodes, but I am really not sure how one can do this when merging pll and
> its leaf clocks into the same node.
> 
> Any idea how this can be done cleanly with a single DT node representing
> the pll and its leaf clocks?
> 
> Thanks,
> 
> Ray

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding
  2015-04-16 19:20             ` Michael Turquette
@ 2015-04-16 21:01               ` Ray Jui
  0 siblings, 0 replies; 16+ messages in thread
From: Ray Jui @ 2015-04-16 21:01 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mike,

On 4/16/2015 12:20 PM, Michael Turquette wrote:
> Quoting Ray Jui (2015-04-14 12:10:35)
>> Hi Mike,
>>
>> On 4/13/2015 12:40 PM, Ray Jui wrote:
>>> Hi Mike,
>>>
>>> On 4/12/2015 11:02 PM, Michael Turquette wrote:
>>>> Quoting Ray Jui (2015-04-12 21:08:32)
>>>>>
>>>>>
>>>>> On 4/10/2015 5:12 PM, Michael Turquette wrote:
>>>>>> Quoting Ray Jui (2015-03-17 22:45:17)
>>>>>>> Document the device tree binding for Broadcom iProc architecture based
>>>>>>> clock controller
>>>>>>>
>>>>>>> Signed-off-by: Ray Jui <rjui@broadcom.com>
>>>>>>> Reviewed-by: Scott Branden <sbranden@broadcom.com>
>>>>>>> ---
>>>>>>>  .../bindings/clock/brcm,iproc-clocks.txt           |  171 ++++++++++++++++++++
>>>>>>>  1 file changed, 171 insertions(+)
>>>>>>>  create mode 100644 Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>>>>>>> new file mode 100644
>>>>>>> index 0000000..bf2316b
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/clock/brcm,iproc-clocks.txt
>>>>>>> @@ -0,0 +1,171 @@
>>>>>>> +Broadcom iProc Family Clocks
>>>>>>> +
>>>>>>> +This binding uses the common clock binding:
>>>>>>> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>>>>> +
>>>>>>> +The iProc clock controller manages clocks that are common to the iProc family.
>>>>>>> +An SoC from the iProc family may have several PPLs, e.g., ARMPLL, GENPLL,
>>>>>>> +LCPLL0, MIPIPLL, and etc., all derived from an onboard crystal. Each PLL
>>>>>>> +comprises of several leaf clocks
>>>>>>> +
>>>>>>> +Required properties for PLLs:
>>>>>>> +- compatible:
>>>>>>> +    Should have a value of the form "brcm,<soc>-<pll>". For example, GENPLL on
>>>>>>> +Cygnus has a compatible string of "brcm,cygnus-genpll"
>>>>>>> +
>>>>>>> +- #clock-cells:
>>>>>>> +    Must be <0>
>>>>>>> +
>>>>>>> +- reg:
>>>>>>> +    Define the base and range of the I/O address space that contain the iProc
>>>>>>> +clock control registers required for the PLL
>>>>>>> +
>>>>>>> +- clocks:
>>>>>>> +    The input parent clock phandle for the PLL. For all iProc PLLs, this is an
>>>>>>> +onboard crystal with a fixed rate
>>>>>>> +
>>>>>>> +Example:
>>>>>>> +
>>>>>>> +       osc: oscillator {
>>>>>>> +               #clock-cells = <0>;
>>>>>>> +               compatible = "fixed-clock";
>>>>>>> +               clock-frequency = <25000000>;
>>>>>>> +       };
>>>>>>> +
>>>>>>> +       genpll: genpll {
>>>>>>> +               #clock-cells = <0>;
>>>>>>> +               compatible = "brcm,cygnus-genpll";
>>>>>>> +               reg = <0x0301d000 0x2c>,
>>>>>>> +                       <0x0301c020 0x4>;
>>>>>>> +               clocks = <&osc>;
>>>>>>> +       };
>>>>>>> +
>>>>>>> +Required properties for leaf clocks of a PLL:
>>>>>>> +
>>>>>>> +- compatible:
>>>>>>> +    Should have a value of the form "brcm,<soc>-<pll>-clk". For example, leaf
>>>>>>> +clocks derived from the GENPLL on Cygnus SoC have a compatible string of
>>>>>>> +"brcm,cygnus-genpll-clk"
>>>>>>> +
>>>>>>> +- #clock-cells:
>>>>>>> +    Have a value of <1> since there are more than 1 leaf clock of a
>>>>>>> +given PLL
>>>>>>> +
>>>>>>> +- reg:
>>>>>>> +    Define the base and range of the I/O address space that contain the iProc
>>>>>>> +clock control registers required for the PLL leaf clocks
>>>>>>> +
>>>>>>> +- clocks:
>>>>>>> +    The input parent PLL phandle for the leaf clock
>>>>>>> +
>>>>>>> +- clock-output-names:
>>>>>>> +    An ordered list of strings defining the names of the leaf clocks
>>>>>>> +
>>>>>>> +Example:
>>>>>>> +
>>>>>>> +       genpll: genpll {
>>>>>>> +               #clock-cells = <0>;
>>>>>>> +               compatible = "brcm,cygnus-genpll";
>>>>>>> +               reg = <0x0301d000 0x2c>,
>>>>>>> +                       <0x0301c020 0x4>;
>>>>>>> +               clocks = <&osc>;
>>>>>>> +       };
>>>>>>> +
>>>>>>> +       genpll_clks: genpll_clks {
>>>>>>> +               #clock-cells = <1>;
>>>>>>> +               compatible = "brcm,cygnus-genpll-clk";
>>>>>>> +               reg = <0x0301d000 0x2c>;
>>>>>>> +               clocks = <&genpll>;
>>>>>>> +               clock-output-names = "axi21", "250mhz", "ihost_sys",
>>>>>>> +                       "enet_sw", "audio_125", "can";
>>>>>>> +       };
>>>>>>
>>>>>> Hi Ray,
>>>>>>
>>>>>> Thanks for submitting the patch. It was nice meeting you at ELC as well.
>>>>>>
>>>>>> This binding doesn't conform to the norms for clock bindings. It looks
>>>>>> like for each type of controllable clock node (e.g. pll, leaf clock,
>>>>>> etc) you have a dts node. Looking at the above example it seems that
>>>>>> those two nodes (genpll and genpll_clks) share the same register.
>>>>>>
>>>>>> /me checks patch #5
>>>>>>
>>>>>> Yup, you re-use the same register address for the *pll and *pll_clks
>>>>>> nodes. I'm not a DT expert but I think this is considered Wrong.
>>>>>>
>>>>>> More generally your clock dt binding should be modeling the hardware in
>>>>>> terms of IP blocks. If you have a clock generator IP block it may
>>>>>> control many clock bits and output many clock signals. E.g. for your
>>>>>> hardware (based only on the addresses in patch #5 and not having seen
>>>>>> any data manual) I will hazard a guess that the genpll, lcpll and asiu
>>>>>> clocks are all part of the same IP block.
>>>>>
>>>>> Hi Mike,
>>>>>
>>>>> In fact, lcpll, genpll, mipipll are similar but DIFFERENT IP blocks, and
>>>>> asiu is completely different from any of them. All of these plls are
>>>>> unique and have their own register banks, as you can see from the
>>>>> bcm-cygnus-clock.dtsi file. Therefore, I think it's totally correct and
>>>>> actually necessary to represent each of them with a separate device node.
>>>>
>>>> OK, that makes sense to me, if those registers live in addresses ranges
>>>> which correspond to different IP blocks.
>>>>
>>>>>
>>>>> Regarding the relationship between a PLL clock and its leaf clocks:
>>>>> Taking the genpll as an example, physically they are connected as:
>>>>>
>>>>> xtal -> genpll -> axi21, 250mhz, ihost_sys, ...
>>>>>
>>>>> The 25 MHz crystal feeds to the genpll, and the genpll generates 6
>>>>> different leaf clocks including axi21, 250mhz, ihost_sys, and etc. One
>>>>> can choose to set the genpll's vco to one frequency, and based on that
>>>>> frequency, different leaf clock frequencies can be generated with their
>>>>> own post divider.
>>>>>
>>>>> Therefore, I think it makes sense to represent xtal, genpll, genpll_clks
>>>>> (including axi21, 250mhz, ihost_sys, and etc) each with a unique device
>>>>> node, and genpll is the parent of these leaf clocks. Basically the
>>>>> device nodes and the way how the "clocks" phandle is used represent the
>>>>> hierarchy of the clock architecture within Cygnus (and in the future
>>>>> other iProc SoCs). Does that make sense?
>>>>
>>>> This doesn't make sense to me. If I understand correctly, the register
>>>> range that controls the post-divider clock (e.g. axi21) is the same
>>>> register range that control's genpll. This is a reasonable indicator to
>>>> me that the leaf clocks are part of the same clock generator IP block as
>>>> the PLL controls. As such they should be on node.
>>>>
>>>>>
>>>>> Regarding the register address overlapping, again, taking genpll as an
>>>>> example, the genpll and the genpll_clks actually never access the same
>>>>> set of registers, but the registers are sort of scattered within one
>>>>> bank, i.e., on Cygnus, genpll uses registers at offset 0x0, 0x8 ~ 0x1c,
>>>>> and genpll_clks uses registers at offset 0x4, 0x20 - 0x24.
>>>>
>>>> Sure, my argument above doesn't hinge on the fact that the pll and child
>>>> clocks use the exact same register. It still looks to me like *pll and
>>>> it's child dividers belong in the same IP block. Is there an open data
>>>> sheet or technical reference manual I can look at for this part? That is
>>>> the best way to put my concerns at ease ;-)
>>>>
>>>> Looking over your binding again, it looks like your nodes are divided
>>>> conveniently for the different clock types (e.g. pll versus
>>>> post-divider), but our goal with DT is to accurately describe the
>>>> hardware, not the C structures.
>>>>
>>>> Regards,
>>>> Mike
>>>>
>>>
>>> Yes, the genpll and genpll_clks are controlled by the same IP block. I
>>> can make the change to combine them to use one DT node. But before doing
>>> that, I just need to get one thing clarified with you. In SW, the pll
>>> and its leaf clocks will still be registered as separate two clock
>>> providers, since we need to be able to configure the PLL vco frequency
>>> and its leaf clock frequencies separately. The pll will be the parent of
>>> its leaf clocks, and the leaf clocks will be used by various peripherals.
>>>
>>> I plan to have the combined DT looks like this:
>>>
>>> genpll_clks: genpll_clks {
>>>     #clock-cells = <1>;
>>>     compatible = "brcm,cygnus-genpll-clk";
>>>     reg = <0x0301d000 0x2c>;
>>>
>>>     assigned-clocks = <&genpll_clks>;
>>>     /* this sets the PLL rate at the time when the PLL clock is
>>> registered */
>>>     assigned-clock-rates = <4000000000>;
>>>
>>>     clocks = <&osc>;
>>>
>>>     clock-output-names = "axi21", "250mhz", "ihost_sys", "enet_sw",
>>> "audio_125", "can";
>>> };
>>>
>>> peripheralA: peripheralA {
>>>     /* use the first leaf clock of genpll */
>>>     clocks = <&genpll_clks 0>;
>>>     clock-frequency  = <100000000>;
>>> };
>>>
>>> If we register both the genpll and its leaf clocks to the clock
>>> framework as two separate clock providers, would the above DT entries
>>> still work? For example, peripheralA refers to the phandle of
>>> genpll_clks, but how does the clock framework know to link it to the pll
>>> clock or the leaf clock?
> 
> Hi Ray,
> 
> There are two options here: using clock-output-names or not using
> clock-output-names.
> 
> I would recommend against using clock-output-names. If both your
> clock driver and the peripheral devices that consume these clocks are
> all represented in DT then you can skip clock-output-names and instead
> create phandle linkage between the clocks inside of your clock provider
> and the consumer devices. String names are still important here, but now
> you only need to specify the string name as an *input* to the consumer
> device, and not as an *output* from the clock provider node. Let's look
> at how the qcom bindings do it:
> 
> gcc is a clock generator ip block.
> 
> arch/arm/boot/dts/qcom-apq8084.dtsi:
>                 gcc: clock-controller at fc400000 {
>                         compatible = "qcom,gcc-apq8084";
>                         #clock-cells = <1>;
>                         #reset-cells = <1>;
>                         reg = <0xfc400000 0x4000>;
>                 };
> 
> This same file includes this header:
> #include <dt-bindings/clock/qcom,gcc-apq8084.h>
> 
> That header creates a map of clocks by numbers. An example:
> #define GCC_BLSP2_UART2_APPS_CLK                        142
> 
> We can see how it used by the serial controller in
> arch/arm/boot/dts/qcom-apq8084.dtsi:
>                 serial at f995e000 {
>                         compatible = "qcom,msm-uartdm-v1.4", "qcom,msm-uartdm";
>                         reg = <0xf995e000 0x1000>;
>                         interrupts = <0 114 0x0>;
>                         clocks = <&gcc GCC_BLSP2_UART2_APPS_CLK>, <&gcc GCC_BLSP2_AHB_CLK>;
>                         clock-names = "core", "iface";
>                         status = "disabled";
>                 };
> 
> Note the 'clock-names' property. This lists a string name which is a property
> of the *consuming* device (uart controller), not the *providing* device (gcc
> clock generator).
> 
> Finally, it all comes together in the driver simply using clk_get:
>         msm_port->clk = devm_clk_get(&pdev->dev, "core");
>         if (IS_ERR(msm_port->clk))
>                 return PTR_ERR(msm_port->clk);
> 
>         if (msm_port->is_uartdm) {
>                 msm_port->pclk = devm_clk_get(&pdev->dev, "iface");
>                 if (IS_ERR(msm_port->pclk))
>                         return PTR_ERR(msm_port->pclk);
> 
>                 clk_set_rate(msm_port->clk, 1843200);
>         }
> 
> This works because we have created linkage between the clock phandle and
> the consuming device node. The shared header is used by the Linux kernel
> to look up the clock as well as the consumer node to map the string name
> onto a clock input.
> 
> Back to clock-output-names:
> 
> If you decide to keep using clock-output-names you can still represent
> the pll clock in your "clock-output-names". In a perfect world DT nodes
> that use clock-output-names would only expose the leaf clocks that your
> peripheral devices consume, but there is no technical reason why your
> pll can't be a part of the clock-output-names array. It is simply an
> array of string names that maps to an index. There is no sense of
> parent-child hierarchy in the DT binding, so there is no problem adding
> a parent pll clock to the array of clock-output-names. I think that this
> will tactically solve your problem in the short term, but I encourage
> you to inspect the qcom binding example I gave above to see if it a
> better fit for you long term.
> 
> Regards,
> Mike
> 

Thanks for your thorough explanation. You clarified that there does not
need to be a clock parent-child hierarchy in the DT binding. I think
that's where I got really confused at. Yes, by exposing both the pll
(parent) and its leaf clocks (children) with individual indices under
the same node, that should solve the problem I have.

I will investigate and work out a new patch.

Thanks,

Ray

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2015-04-16 21:01 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-18  5:45 [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Ray Jui
2015-03-18  5:45 ` [PATCH v6 1/6] clk: iproc: define Broadcom iProc clock binding Ray Jui
2015-04-11  0:12   ` Michael Turquette
2015-04-13  4:08     ` Ray Jui
2015-04-13  6:02       ` Michael Turquette
2015-04-13 19:40         ` Ray Jui
2015-04-14 19:10           ` Ray Jui
2015-04-16 19:20             ` Michael Turquette
2015-04-16 21:01               ` Ray Jui
2015-03-18  5:45 ` [PATCH v6 2/6] clk: iproc: add initial common clock support Ray Jui
2015-03-18  5:45 ` [PATCH v6 3/6] clk: Change bcm clocks build dependency Ray Jui
2015-03-18  5:45 ` [PATCH v6 4/6] clk: cygnus: add clock support for Broadcom Cygnus Ray Jui
2015-03-18  5:45 ` [PATCH v6 5/6] ARM: dts: enable " Ray Jui
2015-03-18  5:45 ` [PATCH v6 6/6] clk: cygnus: remove Cygnus dummy clock binding Ray Jui
2015-03-19  1:42 ` [PATCH v6 0/6] Add common clock support for Broadcom iProc architecture Scott Branden
2015-03-30  5:09 ` Ray Jui

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).