[PATCH 0/3] clk: Add APM X-Gene SoC clock driver

[PATCH 0/3] clk: Add APM X-Gene SoC clock driver

[Loc Ho]

This patch adds APM X-Gene SoC clock driver support. Reference,
PCP PLL, SoC PCP, and Ethernet clocks are defined via the DTS.

Loc Ho (3):
  clk: Add APM X-Gene SoC clock driver with reference, PLL, and device
    clocks.
  clk: arm64: Add DTS clock entry for APM X-Gene Storm SoC.
  Documentation: Add documentation for APM X-Gene clock binding.

 Documentation/devicetree/bindings/clock/xgene.txt |   90 ++++
 arch/arm64/boot/dts/apm-storm.dtsi                |   75 +++
 drivers/clk/Kconfig                               |    7 +
 drivers/clk/Makefile                              |    1 +
 drivers/clk/clk-xgene.c                           |  538 +++++++++++++++++++++
 5 files changed, 711 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/xgene.txt
 create mode 100644 drivers/clk/clk-xgene.c

[PATCH 1/3] clk: Add APM X-Gene SoC clock driver for reference, PLL, and device clocks.

[Loc Ho]

clk: Add APM X-Gene SoC clock driver for reference, PLL, and device clocks.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Kumar Sankaran <ksankaran@apm.com>
Signed-off-by: Vinayak Kale <vkale@apm.com>
Signed-off-by: Feng Kan <fkan@apm.com>
---
 drivers/clk/Kconfig     |    7 +
 drivers/clk/Makefile    |    1 +
 drivers/clk/clk-xgene.c |  538 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 546 insertions(+), 0 deletions(-)
 create mode 100644 drivers/clk/clk-xgene.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 0357ac4..aa4d8b6 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -81,6 +81,13 @@ config COMMON_CLK_AXI_CLKGEN
 	  Support for the Analog Devices axi-clkgen pcore clock generator for Xilinx
 	  FPGAs. It is commonly used in Analog Devices' reference designs.
 
+config COMMON_CLK_XGENE
+	bool "Clock driver for APM XGene SoC"
+	default y
+	depends on ARCH_XGENE
+	---help---
+	  Sypport for the APM X-Gene SoC reference, PLL, and device clocks.
+
 endmenu
 
 source "drivers/clk/mvebu/Kconfig"
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index 137d3e7..4c96337 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -39,3 +39,4 @@ obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o
 obj-$(CONFIG_COMMON_CLK_MAX77686) += clk-max77686.o
 obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o
 obj-$(CONFIG_CLK_TWL6040)	+= clk-twl6040.o
+obj-$(CONFIG_COMMON_CLK_XGENE)  += clk-xgene.o
diff --git a/drivers/clk/clk-xgene.c b/drivers/clk/clk-xgene.c
new file mode 100644
index 0000000..8b81e89
--- /dev/null
+++ b/drivers/clk/clk-xgene.c
@@ -0,0 +1,538 @@
+/*
+ * clk-xgene.c - AppliedMicro X-Gene Clock Interface
+ *
+ * Copyright (c) 2013, Applied Micro Circuits Corporation
+ * Author: Loc Ho <lho@apm.com>
+ *
+ * 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; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ *
+ */
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/clkdev.h>
+#include <linux/clk-provider.h>
+#include <linux/of_address.h>
+#include <asm/setup.h>
+
+/* Register SCU_PCPPLL bit fields */
+#define N_DIV_RD(src)			(((src) & 0x000001ff))
+
+/* Register SCU_SOCPLL bit fields */
+#define CLKR_RD(src)			(((src) & 0x07000000)>>24)
+#define CLKOD_RD(src)			(((src) & 0x00300000)>>20)
+#define REGSPEC_RESET_F1_MASK		0x00010000
+#define CLKF_RD(src)			(((src) & 0x000001ff))
+
+#define XGENE_CLK_DRIVER_VER		"0.1"
+
+static DEFINE_SPINLOCK(clk_lock);
+
+static inline u32 xgene_clk_read(void *csr)
+{
+	return readl_relaxed(csr);
+}
+
+static inline void xgene_clk_write(u32 data, void *csr)
+{
+	return writel_relaxed(data, csr);
+}
+
+/* PLL Clock */
+enum xgene_pll_type {
+	PLL_TYPE_PCP = 0,
+	PLL_TYPE_SOC = 1,
+};
+
+struct xgene_clk_pll {
+	struct clk_hw	hw;
+	const char	*name;
+	void __iomem	*reg;
+	spinlock_t	*lock;
+	u32		pll_offset;
+	enum xgene_pll_type	type;
+};
+
+#define to_xgene_clk_pll(_hw) container_of(_hw, struct xgene_clk_pll, hw)
+
+static int xgene_clk_pll_is_enabled(struct clk_hw *hw)
+{
+	struct xgene_clk_pll *pllclk = to_xgene_clk_pll(hw);
+	u32 data;
+
+	data = xgene_clk_read(pllclk->reg + pllclk->pll_offset);
+	pr_debug("%s pll %s\n", pllclk->name,
+		data & REGSPEC_RESET_F1_MASK ? "disabled" : "enabled");
+
+	return data & REGSPEC_RESET_F1_MASK ? 0 : 1;
+}
+
+static unsigned long xgene_clk_pll_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	struct xgene_clk_pll *pllclk = to_xgene_clk_pll(hw);
+	unsigned long fref;
+	unsigned long fvco;
+	u32 pll;
+	u32 nref;
+	u32 nout;
+	u32 nfb;
+
+	pll = xgene_clk_read(pllclk->reg + pllclk->pll_offset);
+
+	if (pllclk->type == PLL_TYPE_PCP) {
+		/*
+		 * PLL VCO = Reference clock * NF
+		 * PCP PLL = PLL_VCO / 2
+		 */
+		nout = 2;
+		fvco = parent_rate * (N_DIV_RD(pll) + 4);
+	} else {
+		/*
+		 * Fref = Reference Clock / NREF;
+		 * Fvco = Fref * NFB;
+		 * Fout = Fvco / NOUT;
+		 */
+		nref = CLKR_RD(pll) + 1;
+		nout = CLKOD_RD(pll) + 1;
+		nfb = CLKF_RD(pll);
+		fref = parent_rate / nref;
+		fvco = fref * nfb;
+	}
+	pr_debug("%s pll recalc rate %ld parent %ld\n", pllclk->name,
+		fvco / nout, parent_rate);
+
+	return fvco / nout;
+}
+
+const struct clk_ops xgene_clk_pll_ops = {
+	.is_enabled = xgene_clk_pll_is_enabled,
+	.recalc_rate = xgene_clk_pll_recalc_rate,
+};
+
+static struct clk *xgene_register_clk_pll(struct device *dev,
+	const char *name, const char *parent_name,
+	unsigned long flags, void __iomem *reg, u32 pll_offset,
+	u32 type, spinlock_t *lock)
+{
+	struct xgene_clk_pll *apmclk;
+	struct clk *clk;
+	struct clk_init_data init;
+
+	/* allocate the APM clock structure */
+	apmclk = kzalloc(sizeof(struct xgene_clk_pll), GFP_KERNEL);
+	if (!apmclk) {
+		pr_err("%s: could not allocate APM clk\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	init.name = name;
+	init.ops = &xgene_clk_pll_ops;
+	init.flags = flags;
+	init.parent_names = parent_name ? &parent_name : NULL;
+	init.num_parents = parent_name ? 1 : 0;
+
+	apmclk->name = name;
+	apmclk->reg = reg;
+	apmclk->lock = lock;
+	apmclk->pll_offset = pll_offset;
+	apmclk->type = type;
+	apmclk->hw.init = &init;
+
+	/* Register the clock */
+	clk = clk_register(dev, &apmclk->hw);
+	if (IS_ERR(clk)) {
+		pr_err("%s: could not register clk %s\n", __func__, name);
+		kfree(apmclk);
+		return NULL;
+	}
+	return clk;
+}
+
+static void xgene_pllclk_init(struct device_node *np)
+{
+	struct resource res;
+	const char *clk_name = np->full_name;
+	struct clk *clk;
+	void *reg;
+	int rc;
+	u32 type;
+
+	rc = of_address_to_resource(np, 0, &res);
+	if (rc != 0) {
+		pr_err("no DTS CSR register for %s\n", np->full_name);
+		return;
+	}
+	reg = ioremap(res.start, resource_size(&res));
+	if (reg == NULL) {
+		pr_err("Unable to map CSR register for %s\n", np->full_name);
+		return;
+	}
+	of_property_read_string(np, "clock-output-names", &clk_name);
+	if (of_property_read_u32(np, "type", &type))
+		type = 0;
+
+	clk = xgene_register_clk_pll(NULL,
+			clk_name, of_clk_get_parent_name(np, 0),
+			CLK_IS_ROOT, reg, 0, type, &clk_lock);
+	if (!IS_ERR(clk)) {
+		of_clk_add_provider(np, of_clk_src_simple_get, clk);
+		clk_register_clkdev(clk, clk_name, NULL);
+		pr_debug("Add %s clock PLL\n", clk_name);
+	}
+}
+
+/* IP Clock */
+struct xgene_dev_parameters {
+	u32  flags;			/* Any flags to the clock framework */
+	void __iomem *csr_reg;		/* CSR for IP clock */
+	u32 reg_clk_offset;		/* Offset to clock enable CSR */
+	u32 reg_clk_mask;		/* Mask bit for clock enable */
+	u32 reg_csr_offset;		/* Offset to CSR reset */
+	u32 reg_csr_mask;		/* Mask bit for disable CSR reset */
+	void __iomem *divider_reg;	/* CSR for divider */
+	u32 reg_divider_offset;		/* Offset to divider register */
+	u32 reg_divider_shift;		/* Bit shift to divider field */
+	u32 reg_divider_width;		/* Width of the bit to divider field */
+};
+
+struct xgene_clk {
+	struct clk_hw	hw;
+	const char	*name;
+	spinlock_t	*lock;
+	struct xgene_dev_parameters	param;
+};
+
+#define to_xgene_clk(_hw) container_of(_hw, struct xgene_clk, hw)
+
+static int xgene_clk_enable(struct clk_hw *hw)
+{
+	struct xgene_clk *pclk = to_xgene_clk(hw);
+	unsigned long flags = 0;
+	u32 data;
+
+	if (pclk->lock)
+		spin_lock_irqsave(pclk->lock, flags);
+
+	if (pclk->param.csr_reg != NULL) {
+		pr_debug("%s clock enabled\n", pclk->name);
+		/* First enable the clock */
+		data = xgene_clk_read(pclk->param.csr_reg +
+					pclk->param.reg_clk_offset);
+		data |= pclk->param.reg_clk_mask;
+		xgene_clk_write(data, pclk->param.csr_reg +
+					pclk->param.reg_clk_offset);
+		pr_debug("%s clock PADDR base 0x%016LX clk offset 0x%08X mask 0x%08X value 0x%08X\n",
+			pclk->name, __pa(pclk->param.csr_reg),
+			pclk->param.reg_clk_offset, pclk->param.reg_clk_mask,
+			data);
+
+		/* Second enable the CSR */
+		data = xgene_clk_read(pclk->param.csr_reg +
+					pclk->param.reg_csr_offset);
+		data &= ~pclk->param.reg_csr_mask;
+		xgene_clk_write(data, pclk->param.csr_reg +
+					pclk->param.reg_csr_offset);
+		pr_debug("%s CSR RESET PADDR base 0x%016LX csr offset 0x%08X mask 0x%08X value 0x%08X\n",
+			pclk->name, __pa(pclk->param.csr_reg),
+			pclk->param.reg_csr_offset, pclk->param.reg_csr_mask,
+			data);
+	}
+
+	if (pclk->lock)
+		spin_unlock_irqrestore(pclk->lock, flags);
+
+	return 0;
+}
+
+static void xgene_clk_disable(struct clk_hw *hw)
+{
+	struct xgene_clk *pclk = to_xgene_clk(hw);
+	unsigned long flags = 0;
+	u32 data;
+
+	if (pclk->lock)
+		spin_lock_irqsave(pclk->lock, flags);
+
+	if (pclk->param.csr_reg != NULL) {
+		pr_debug("%s clock disabled\n", pclk->name);
+		/* First put the CSR in reset */
+		data = xgene_clk_read(pclk->param.csr_reg +
+					pclk->param.reg_csr_offset);
+		data |= pclk->param.reg_csr_mask;
+		xgene_clk_write(data, pclk->param.csr_reg +
+					pclk->param.reg_csr_offset);
+
+		/* Second disable the clock */
+		data = xgene_clk_read(pclk->param.csr_reg +
+					pclk->param.reg_clk_offset);
+		data &= ~pclk->param.reg_clk_mask;
+		xgene_clk_write(data, pclk->param.csr_reg +
+					pclk->param.reg_clk_offset);
+	}
+
+	if (pclk->lock)
+		spin_unlock_irqrestore(pclk->lock, flags);
+}
+
+static int xgene_clk_is_enabled(struct clk_hw *hw)
+{
+	struct xgene_clk *pclk = to_xgene_clk(hw);
+	u32 data = 0;
+
+	if (pclk->param.csr_reg != NULL) {
+		pr_debug("%s clock checking\n", pclk->name);
+		data = xgene_clk_read(pclk->param.csr_reg +
+					pclk->param.reg_clk_offset);
+		pr_debug("%s clock is %s\n", pclk->name,
+			data & pclk->param.reg_clk_mask ? "enabled" :
+							"disabled");
+	}
+
+	if (pclk->param.csr_reg == NULL)
+		return 1;
+	return data & pclk->param.reg_clk_mask ? 1 : 0;
+}
+
+static unsigned long xgene_clk_recalc_rate(struct clk_hw *hw,
+				unsigned long parent_rate)
+{
+	struct xgene_clk *pclk = to_xgene_clk(hw);
+	u32 data;
+
+	if (pclk->param.divider_reg) {
+		data = xgene_clk_read(pclk->param.divider_reg +
+					pclk->param.reg_divider_offset);
+		data >>= pclk->param.reg_divider_shift;
+		data &= (1 << pclk->param.reg_divider_width) - 1;
+
+		pr_debug("%s clock recalc rate %ld parent %ld\n",
+			pclk->name, parent_rate / data, parent_rate);
+		return parent_rate / data;
+	} else {
+		pr_debug("%s clock recalc rate %ld parent %ld\n",
+			pclk->name, parent_rate, parent_rate);
+		return parent_rate;
+	}
+}
+
+static int xgene_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long parent_rate)
+{
+	struct xgene_clk *pclk = to_xgene_clk(hw);
+	unsigned long flags = 0;
+	u32 data;
+	u32 divider;
+	u32 divider_save;
+
+	if (pclk->lock)
+		spin_lock_irqsave(pclk->lock, flags);
+
+	if (pclk->param.divider_reg) {
+		/* Let's compute the divider */
+		if (rate > parent_rate)
+			rate = parent_rate;
+		divider_save = divider = parent_rate / rate; /* Rounded down */
+		divider &= (1 << pclk->param.reg_divider_width) - 1;
+		divider <<= pclk->param.reg_divider_shift;
+
+		/* Set new divider */
+		data = xgene_clk_read(pclk->param.divider_reg +
+				pclk->param.reg_divider_offset);
+		data &= ~((1 << pclk->param.reg_divider_width) - 1);
+		data |= divider;
+		xgene_clk_write(data, pclk->param.divider_reg +
+					pclk->param.reg_divider_offset);
+		pr_debug("%s clock set rate %ld\n", pclk->name,
+			parent_rate / divider_save);
+	} else {
+		divider_save = 1;
+	}
+
+	if (pclk->lock)
+		spin_unlock_irqrestore(pclk->lock, flags);
+
+	return parent_rate / divider_save;
+}
+
+static long xgene_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+				unsigned long *prate)
+{
+	struct xgene_clk *pclk = to_xgene_clk(hw);
+	unsigned long parent_rate = *prate;
+	u32 divider;
+
+	if (pclk->param.divider_reg) {
+		/* Let's compute the divider */
+		if (rate > parent_rate)
+			rate = parent_rate;
+		divider = parent_rate / rate;   /* Rounded down */
+	} else {
+		divider = 1;
+	}
+
+	return parent_rate / divider;
+}
+
+const struct clk_ops xgene_clk_ops = {
+	.enable = xgene_clk_enable,
+	.disable = xgene_clk_disable,
+	.is_enabled = xgene_clk_is_enabled,
+	.recalc_rate = xgene_clk_recalc_rate,
+	.set_rate = xgene_clk_set_rate,
+	.round_rate = xgene_clk_round_rate,
+};
+
+static struct clk *xgene_register_clk(struct device *dev,
+		const char *name, const char *parent_name,
+		struct xgene_dev_parameters *parameters, spinlock_t *lock)
+{
+	struct xgene_clk *apmclk;
+	struct clk *clk;
+	struct clk_init_data init;
+	int rc;
+
+	/* allocate the APM clock structure */
+	apmclk = kzalloc(sizeof(struct xgene_clk), GFP_KERNEL);
+	if (!apmclk) {
+		pr_err("%s: could not allocate APM clk\n", __func__);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	init.name = name;
+	init.ops = &xgene_clk_ops;
+	init.flags = parameters->flags;
+	init.parent_names = parent_name ? &parent_name : NULL;
+	init.num_parents = parent_name ? 1 : 0;
+
+	apmclk->name = name;
+	apmclk->lock = lock;
+	apmclk->hw.init = &init;
+	apmclk->param = *parameters;
+
+	/* Register the clock */
+	clk = clk_register(dev, &apmclk->hw);
+	if (IS_ERR(clk)) {
+		pr_err("%s: could not register clk %s\n", __func__, name);
+		kfree(apmclk);
+		return NULL;
+	}
+
+	/* Register the clock for lookup */
+	rc = clk_register_clkdev(clk, name, NULL);
+	if (rc != 0) {
+		pr_err("%s: could not register lookup clk %s\n",
+			__func__, name);
+	}
+	return clk;
+}
+
+static void __init xgene_devclk_init(struct device_node *np)
+{
+	const char *clk_name = np->full_name;
+	struct clk *clk;
+	struct resource res;
+	int rc;
+	struct xgene_dev_parameters parameters;
+
+	rc = of_address_to_resource(np, 0, &res);
+	if (rc != 0) {
+		pr_err("no DTS CSR register for %s\n", np->full_name);
+		return;
+	}
+	if (resource_size(&res)) {
+		parameters.csr_reg = ioremap(res.start, resource_size(&res));
+		if (parameters.csr_reg == NULL) {
+			pr_err("Unable to map CSR register for %s\n",
+				np->full_name);
+			return;
+		}
+	} else {
+		parameters.csr_reg = NULL;
+	}
+
+	rc = of_address_to_resource(np, 1, &res);
+	if (rc != 0) {
+		pr_err("no DTS DIV register for %s\n", np->full_name);
+		return;
+	}
+	if (resource_size(&res)) {
+		parameters.divider_reg = ioremap(res.start,
+						resource_size(&res));
+		if (parameters.divider_reg == NULL) {
+			pr_err("Unable to map DIV register for %s\n",
+				np->full_name);
+			if (parameters.csr_reg)
+				iounmap(parameters.csr_reg);
+			return;
+		}
+	} else {
+		parameters.divider_reg = NULL;
+	}
+	if (of_property_read_u32(np, "flags", &parameters.flags))
+		parameters.flags = 0;
+	if (of_property_read_u32(np, "csr-offset", &parameters.reg_csr_offset))
+		parameters.reg_csr_offset = 0;
+	if (of_property_read_u32(np, "csr-mask", &parameters.reg_csr_mask))
+		parameters.reg_csr_mask = 0xF;
+	if (of_property_read_u32(np, "enable-offset",
+				&parameters.reg_clk_offset))
+		parameters.reg_clk_offset = 0x8;
+	if (of_property_read_u32(np, "enable-mask", &parameters.reg_clk_mask))
+		parameters.reg_clk_mask = 0xF;
+	if (of_property_read_u32(np, "divider-offset",
+				&parameters.reg_divider_offset))
+		parameters.reg_divider_offset = 0;
+	if (of_property_read_u32(np, "divider-width",
+				&parameters.reg_divider_width))
+		parameters.reg_divider_width = 0;
+	if (of_property_read_u32(np, "divider-shift",
+				&parameters.reg_divider_shift))
+		parameters.reg_divider_shift = 0;
+	of_property_read_string(np, "clock-output-names", &clk_name);
+
+	clk = xgene_register_clk(NULL, clk_name,
+		of_clk_get_parent_name(np, 0), &parameters, &clk_lock);
+	if (IS_ERR(clk)) {
+		if (parameters.csr_reg)
+			iounmap(parameters.csr_reg);
+		if (parameters.divider_reg)
+			iounmap(parameters.divider_reg);
+		return;
+	}
+	pr_debug("Add %s clock\n", clk_name);
+	rc = of_clk_add_provider(np, of_clk_src_simple_get, clk);
+	if (rc != 0) {
+		pr_err("%s: could register provider clk %s\n", __func__,
+			np->full_name);
+		return;
+	}
+}
+
+CLK_OF_DECLARE(fixed_clock, "fixed-clock", of_fixed_clk_setup);
+CLK_OF_DECLARE(fixed_factor_clock, "fixed-factor-clock",
+		of_fixed_factor_clk_setup);
+CLK_OF_DECLARE(xgene_pll_clock, "apm,xgene-pll-clock", xgene_pllclk_init);
+CLK_OF_DECLARE(xgene_dev_clock, "apm,xgene-device-clock", xgene_devclk_init);
+
+static int __init xgene_clk_init(void)
+{
+	pr_info("XGene clock driver v%s\n", XGENE_CLK_DRIVER_VER);
+	of_clk_init(NULL);
+	return 0;
+}
+arch_initcall(xgene_clk_init);
-- 
1.5.5

[PATCH 2/3] clk: arm64: Add DTS clock entry for APM X-Gene Storm SoC.

[Loc Ho]

clk: arm64: Add DTS clock entry for APM X-Gene Storm SoC with reference to 
reference, PCP PLL, SoC PLL, and Ethernet clocks.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Kumar Sankaran <ksankaran@apm.com>
Signed-off-by: Vinayak Kale <vkale@apm.com>
Signed-off-by: Feng Kan <fkan@apm.com>
---
 arch/arm64/boot/dts/apm-storm.dtsi |   75 ++++++++++++++++++++++++++++++++++++
 1 files changed, 75 insertions(+), 0 deletions(-)

diff --git a/arch/arm64/boot/dts/apm-storm.dtsi b/arch/arm64/boot/dts/apm-storm.dtsi
index bfdc578..d994806 100644
--- a/arch/arm64/boot/dts/apm-storm.dtsi
+++ b/arch/arm64/boot/dts/apm-storm.dtsi
@@ -103,6 +103,81 @@
 		#size-cells = <2>;
 		ranges;
 
+		clocks {
+			#address-cells = <2>;
+			#size-cells = <2>;
+			ranges;
+			refclk: refclk {
+				compatible = "fixed-clock";
+				#clock-cells = <1>;
+				clock-frequency = <100000000>;
+				clock-output-names = "refclk";
+			};
+
+			pcppll: pcppll at 17000100 {
+				compatible = "apm,xgene-pll-clock";
+				#clock-cells = <1>;
+				clocks = <&refclk 0>;
+				clock-names = "pcppll";
+				reg = <0x0 0x17000100 0x0 0x1000>;
+				clock-output-names = "pcppll";
+				type = <0>;
+			};
+
+			socpll: socpll at 17000120 {
+				compatible = "apm,xgene-pll-clock";
+				#clock-cells = <1>;
+				clocks = <&refclk 0>;
+				clock-names = "socpll";
+				reg = <0x0 0x17000120 0x0 0x1000>;
+				clock-output-names = "socpll";
+				type = <1>;
+			};
+
+			socplldiv2: socplldiv2  {
+				compatible = "fixed-factor-clock";
+				#clock-cells = <1>;
+				clocks = <&socpll 0>;
+				clock-names = "socplldiv2";
+				clock-mult = <1>;
+				clock-div = <2>;
+				clock-output-names = "socplldiv2";
+			};
+
+			qmlclk: qmlclk {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				clock-names = "qmlclk";
+				reg = <0x0 0x1703C000 0x0 0x1000
+					0x0 0x00000000 0x0 0x0000>;
+				clock-output-names = "qmlclk";
+			};
+
+			ethclk: ethclk {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&socplldiv2 0>;
+				clock-names = "ethclk";
+				reg = <0x0 0x00000000 0x0 0x0000
+					0x0 0x17000000 0x0 0x1000>;
+				divider-offset = <0x238>;
+				divider-width = <0x9>;
+				divider-shift = <0x0>;
+				clock-output-names = "ethclk";
+			};
+
+			eth8clk: eth8clk {
+				compatible = "apm,xgene-device-clock";
+				#clock-cells = <1>;
+				clocks = <&ethclk 0>;
+				clock-names = "eth8clk";
+				reg = <0x0 0x1702C000 0x0 0x1000
+					0x0 0x00000000 0x0 0x0000>;
+				clock-output-names = "eth8clk";
+			};
+		};
+
 		serial0: serial at 1c020000 {
 			device_type = "serial";
 			compatible = "ns16550";
-- 
1.5.5

[PATCH 3/3] Documentation: Add documentation for APM X-Gene clock binding.

[Loc Ho]

Documentation: Add documentation for APM X-Gene clock binding with PLL and 
device clocks.

Signed-off-by: Loc Ho <lho@apm.com>
Signed-off-by: Kumar Sankaran <ksankaran@apm.com>
Signed-off-by: Vinayak Kale <vkale@apm.com>
Signed-off-by: Feng Kan <fkan@apm.com>
---
 Documentation/devicetree/bindings/clock/xgene.txt |   90 +++++++++++++++++++++
 1 files changed, 90 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/xgene.txt

diff --git a/Documentation/devicetree/bindings/clock/xgene.txt b/Documentation/devicetree/bindings/clock/xgene.txt
new file mode 100644
index 0000000..fcdee79
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/xgene.txt
@@ -0,0 +1,90 @@
+Device Tree Clock bindings for APM X-Gene
+
+This binding uses the common clock binding[1].
+
+[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : shall be one of the following:
+	"apm,xgene-pll-clock" - for a X-Gene PLL clock
+	"apm,xgene-device-clock" - for a X-Gene device clock
+
+Required properties for PLL clocks:
+- reg : shall be the physical PLL register address for the pll clock.
+- clocks : shall be the input parent clock phandle for the clock. This should
+	be the reference clock.
+- #clock-cells : shall be set to 1.
+- clock-output-names : shall be the name of the PLL referenced by derive
+  clock.
+- type : shall be 1 for SoC PLL and 0 for PCP PLL.
+Optional properties for PLL clocks:
+- clock-names : shall be the name of the PLL. If missing, use the device name.
+
+Required properties for device clocks:
+- reg : shall be the physical CSR reset address base and physical CSR divider
+        address base. If one does not exist, specify 0 for address and 0 for 
+        size.
+- clocks : shall be the input parent clock phandle for the clock.
+- #clock-cells : shall be set to 1.
+- clock-output-names : shall be the name of the device referenced.
+Optional properties for device clocks:
+- clock-names : shall be the name of the device clock. If missing, use the
+                device name.
+- flags : Any clock flags. ie. use 0x8 to leave clock un-touch if not
+          referenced. Default is 0.
+- csr-offset : Offset to the CSR reset register from the reset address base.
+               Default is 0.
+- csr-mask : CSR reset mask bit. Default is 0xF.
+- enable-offset : Offset to the enable register from the reset address base.
+                  Default is 0x8.
+- enable-mask : CSR enable mask bit. Default is 0xF.
+- divider-offset : Offset to the divider CSR register from the divider base.
+                   Default is 0x0.
+- divider-width : Width of the divider register. Default is 0.
+- divider-shift : Bit shift of the divider register. Default is 0.
+
+For example:
+
+	pcppll: pcppll at 17000100 {
+		compatible = "apm,xgene-pll-clock";
+		#clock-cells = <1>;
+		clocks = <&refclk 0>;
+		clock-names = "pcppll";
+		reg = <0x0 0x17000100 0x0 0x1000>;
+		clock-output-names = "pcppll";
+		type = <0>;
+	};
+
+	socpll: socpll at 17000120 {
+		compatible = "apm,xgene-pll-clock";
+		#clock-cells = <1>;
+		clocks = <&refclk 0>;
+		clock-names = "socpll";
+		reg = <0x0 0x17000120 0x0 0x1000>;
+		clock-output-names = "socpll";
+		type = <1>;
+	};
+
+	qmlclk: qmlclk {
+		compatible = "apm,xgene-device-clock";
+		#clock-cells = <1>;
+		clocks = <&socplldiv2 0>;
+		clock-names = "qmlclk";
+		reg = <0x0 0x1703C000 0x0 0x1000
+			0x0 0x00000000 0x0 0x0000>;
+		clock-output-names = "qmlclk";
+	};
+
+	ethclk: ethclk {
+		compatible = "apm,xgene-device-clock";
+		#clock-cells = <1>;
+		clocks = <&socplldiv2 0>;
+		clock-names = "ethclk";
+		reg = <0x0 0x00000000 0x0 0x0000
+			0x0 0x17000000 0x0 0x1000>;
+			divider-offset = <0x238>;
+		divider-width = <0x9>;
+		divider-shift = <0x0>;
+		clock-output-names = "ethclk";
+	};
+
-- 
1.5.5

[PATCH 0/3] clk: Add APM X-Gene SoC clock driver

[Catalin Marinas]

On Fri, Jun 21, 2013 at 04:30:02PM +0100, Loc Ho wrote:
> This patch adds APM X-Gene SoC clock driver support. Reference,
> PCP PLL, SoC PCP, and Ethernet clocks are defined via the DTS.
> 
> Loc Ho (3):
>   clk: Add APM X-Gene SoC clock driver with reference, PLL, and device
>     clocks.
>   clk: arm64: Add DTS clock entry for APM X-Gene Storm SoC.
>   Documentation: Add documentation for APM X-Gene clock binding.
> 
>  Documentation/devicetree/bindings/clock/xgene.txt |   90 ++++
>  arch/arm64/boot/dts/apm-storm.dtsi                |   75 +++
>  drivers/clk/Kconfig                               |    7 +
>  drivers/clk/Makefile                              |    1 +
>  drivers/clk/clk-xgene.c                           |  538 +++++++++++++++++++++
>  5 files changed, 711 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/xgene.txt
>  create mode 100644 drivers/clk/clk-xgene.c

You'll have to send these patches to Mike Turquette (cc'ed), they are
not being merged via the arm64 tree.

Thanks.

-- 
Catalin

[PATCH 1/3] clk: Add APM X-Gene SoC clock driver for reference, PLL, and device clocks.

[Catalin Marinas]

On Fri, Jun 21, 2013 at 04:30:03PM +0100, Loc Ho wrote:
> clk: Add APM X-Gene SoC clock driver for reference, PLL, and device clocks.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Kumar Sankaran <ksankaran@apm.com>
> Signed-off-by: Vinayak Kale <vkale@apm.com>
> Signed-off-by: Feng Kan <fkan@apm.com>
> ---
>  drivers/clk/Kconfig     |    7 +
>  drivers/clk/Makefile    |    1 +
>  drivers/clk/clk-xgene.c |  538 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 546 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/clk/clk-xgene.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 0357ac4..aa4d8b6 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -81,6 +81,13 @@ config COMMON_CLK_AXI_CLKGEN
>           Support for the Analog Devices axi-clkgen pcore clock generator for Xilinx
>           FPGAs. It is commonly used in Analog Devices' reference designs.
> 
> +config COMMON_CLK_XGENE
> +       bool "Clock driver for APM XGene SoC"
> +       default y
> +       depends on ARCH_XGENE
> +       ---help---
> +         Sypport for the APM X-Gene SoC reference, PLL, and device clocks.

Like the recent vexpress changes, you can make this depend on ARM64
only.

-- 
Catalin

[PATCH 1/3] clk: Add APM X-Gene SoC clock driver for reference, PLL, and device clocks.

[Mark Rutland]

Hi,

I have some general comments, mostly on the devicetree side of things.

On Fri, Jun 21, 2013 at 04:30:03PM +0100, Loc Ho wrote:
> clk: Add APM X-Gene SoC clock driver for reference, PLL, and device clocks.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Kumar Sankaran <ksankaran@apm.com>
> Signed-off-by: Vinayak Kale <vkale@apm.com>
> Signed-off-by: Feng Kan <fkan@apm.com>
> ---
>  drivers/clk/Kconfig     |    7 +
>  drivers/clk/Makefile    |    1 +
>  drivers/clk/clk-xgene.c |  538 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 546 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/clk/clk-xgene.c
> 
> diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> index 0357ac4..aa4d8b6 100644
> --- a/drivers/clk/Kconfig
> +++ b/drivers/clk/Kconfig
> @@ -81,6 +81,13 @@ config COMMON_CLK_AXI_CLKGEN
>           Support for the Analog Devices axi-clkgen pcore clock generator for Xilinx
>           FPGAs. It is commonly used in Analog Devices' reference designs.
> 
> +config COMMON_CLK_XGENE
> +       bool "Clock driver for APM XGene SoC"
> +       default y
> +       depends on ARCH_XGENE
> +       ---help---
> +         Sypport for the APM X-Gene SoC reference, PLL, and device clocks.
> +
>  endmenu
> 
>  source "drivers/clk/mvebu/Kconfig"
> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
> index 137d3e7..4c96337 100644
> --- a/drivers/clk/Makefile
> +++ b/drivers/clk/Makefile
> @@ -39,3 +39,4 @@ obj-$(CONFIG_COMMON_CLK_WM831X) += clk-wm831x.o
>  obj-$(CONFIG_COMMON_CLK_MAX77686) += clk-max77686.o
>  obj-$(CONFIG_COMMON_CLK_SI5351) += clk-si5351.o
>  obj-$(CONFIG_CLK_TWL6040)      += clk-twl6040.o
> +obj-$(CONFIG_COMMON_CLK_XGENE)  += clk-xgene.o
> diff --git a/drivers/clk/clk-xgene.c b/drivers/clk/clk-xgene.c
> new file mode 100644
> index 0000000..8b81e89
> --- /dev/null
> +++ b/drivers/clk/clk-xgene.c
> @@ -0,0 +1,538 @@
> +/*
> + * clk-xgene.c - AppliedMicro X-Gene Clock Interface
> + *
> + * Copyright (c) 2013, Applied Micro Circuits Corporation
> + * Author: Loc Ho <lho@apm.com>
> + *
> + * 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; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + *
> + */
> +#include <linux/module.h>
> +#include <linux/spinlock.h>
> +#include <linux/io.h>
> +#include <linux/of.h>
> +#include <linux/clkdev.h>
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
> +#include <asm/setup.h>
> +
> +/* Register SCU_PCPPLL bit fields */
> +#define N_DIV_RD(src)                  (((src) & 0x000001ff))
> +
> +/* Register SCU_SOCPLL bit fields */
> +#define CLKR_RD(src)                   (((src) & 0x07000000)>>24)
> +#define CLKOD_RD(src)                  (((src) & 0x00300000)>>20)
> +#define REGSPEC_RESET_F1_MASK          0x00010000
> +#define CLKF_RD(src)                   (((src) & 0x000001ff))
> +
> +#define XGENE_CLK_DRIVER_VER           "0.1"
> +
> +static DEFINE_SPINLOCK(clk_lock);
> +
> +static inline u32 xgene_clk_read(void *csr)
> +{
> +       return readl_relaxed(csr);
> +}
> +
> +static inline void xgene_clk_write(u32 data, void *csr)
> +{
> +       return writel_relaxed(data, csr);
> +}
> +
> +/* PLL Clock */
> +enum xgene_pll_type {
> +       PLL_TYPE_PCP = 0,
> +       PLL_TYPE_SOC = 1,
> +};
> +
> +struct xgene_clk_pll {
> +       struct clk_hw   hw;
> +       const char      *name;
> +       void __iomem    *reg;
> +       spinlock_t      *lock;
> +       u32             pll_offset;
> +       enum xgene_pll_type     type;
> +};
> +
> +#define to_xgene_clk_pll(_hw) container_of(_hw, struct xgene_clk_pll, hw)
> +
> +static int xgene_clk_pll_is_enabled(struct clk_hw *hw)
> +{
> +       struct xgene_clk_pll *pllclk = to_xgene_clk_pll(hw);
> +       u32 data;
> +
> +       data = xgene_clk_read(pllclk->reg + pllclk->pll_offset);
> +       pr_debug("%s pll %s\n", pllclk->name,
> +               data & REGSPEC_RESET_F1_MASK ? "disabled" : "enabled");
> +
> +       return data & REGSPEC_RESET_F1_MASK ? 0 : 1;
> +}
> +
> +static unsigned long xgene_clk_pll_recalc_rate(struct clk_hw *hw,
> +                               unsigned long parent_rate)
> +{
> +       struct xgene_clk_pll *pllclk = to_xgene_clk_pll(hw);
> +       unsigned long fref;
> +       unsigned long fvco;
> +       u32 pll;
> +       u32 nref;
> +       u32 nout;
> +       u32 nfb;
> +
> +       pll = xgene_clk_read(pllclk->reg + pllclk->pll_offset);
> +
> +       if (pllclk->type == PLL_TYPE_PCP) {
> +               /*
> +                * PLL VCO = Reference clock * NF
> +                * PCP PLL = PLL_VCO / 2
> +                */
> +               nout = 2;
> +               fvco = parent_rate * (N_DIV_RD(pll) + 4);
> +       } else {
> +               /*
> +                * Fref = Reference Clock / NREF;
> +                * Fvco = Fref * NFB;
> +                * Fout = Fvco / NOUT;
> +                */
> +               nref = CLKR_RD(pll) + 1;
> +               nout = CLKOD_RD(pll) + 1;
> +               nfb = CLKF_RD(pll);
> +               fref = parent_rate / nref;
> +               fvco = fref * nfb;
> +       }
> +       pr_debug("%s pll recalc rate %ld parent %ld\n", pllclk->name,
> +               fvco / nout, parent_rate);
> +
> +       return fvco / nout;
> +}
> +
> +const struct clk_ops xgene_clk_pll_ops = {
> +       .is_enabled = xgene_clk_pll_is_enabled,
> +       .recalc_rate = xgene_clk_pll_recalc_rate,
> +};
> +
> +static struct clk *xgene_register_clk_pll(struct device *dev,
> +       const char *name, const char *parent_name,
> +       unsigned long flags, void __iomem *reg, u32 pll_offset,
> +       u32 type, spinlock_t *lock)
> +{
> +       struct xgene_clk_pll *apmclk;
> +       struct clk *clk;
> +       struct clk_init_data init;
> +
> +       /* allocate the APM clock structure */
> +       apmclk = kzalloc(sizeof(struct xgene_clk_pll), GFP_KERNEL);

You can do apmclk = kzalloc(sizeof(*apmclk), GFP_KERNEL) here, which is more
robust with respect to type changes in future revisions of the driver.

> +       if (!apmclk) {
> +               pr_err("%s: could not allocate APM clk\n", __func__);
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       init.name = name;
> +       init.ops = &xgene_clk_pll_ops;
> +       init.flags = flags;
> +       init.parent_names = parent_name ? &parent_name : NULL;
> +       init.num_parents = parent_name ? 1 : 0;
> +
> +       apmclk->name = name;
> +       apmclk->reg = reg;
> +       apmclk->lock = lock;
> +       apmclk->pll_offset = pll_offset;
> +       apmclk->type = type;
> +       apmclk->hw.init = &init;
> +
> +       /* Register the clock */
> +       clk = clk_register(dev, &apmclk->hw);
> +       if (IS_ERR(clk)) {
> +               pr_err("%s: could not register clk %s\n", __func__, name);
> +               kfree(apmclk);
> +               return NULL;
> +       }
> +       return clk;
> +}
> +
> +static void xgene_pllclk_init(struct device_node *np)
> +{
> +       struct resource res;
> +       const char *clk_name = np->full_name;
> +       struct clk *clk;
> +       void *reg;
> +       int rc;
> +       u32 type;
> +
> +       rc = of_address_to_resource(np, 0, &res);
> +       if (rc != 0) {
> +               pr_err("no DTS CSR register for %s\n", np->full_name);
> +               return;
> +       }
> +       reg = ioremap(res.start, resource_size(&res));

You could just use reg = of_iomap(np, 0) here.

> +       if (reg == NULL) {
> +               pr_err("Unable to map CSR register for %s\n", np->full_name);
> +               return;
> +       }
> +       of_property_read_string(np, "clock-output-names", &clk_name);
> +       if (of_property_read_u32(np, "type", &type))
> +               type = 0;
> +
> +       clk = xgene_register_clk_pll(NULL,
> +                       clk_name, of_clk_get_parent_name(np, 0),
> +                       CLK_IS_ROOT, reg, 0, type, &clk_lock);
> +       if (!IS_ERR(clk)) {
> +               of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +               clk_register_clkdev(clk, clk_name, NULL);
> +               pr_debug("Add %s clock PLL\n", clk_name);
> +       }
> +}
> +
> +/* IP Clock */
> +struct xgene_dev_parameters {
> +       u32  flags;                     /* Any flags to the clock framework */
> +       void __iomem *csr_reg;          /* CSR for IP clock */
> +       u32 reg_clk_offset;             /* Offset to clock enable CSR */
> +       u32 reg_clk_mask;               /* Mask bit for clock enable */
> +       u32 reg_csr_offset;             /* Offset to CSR reset */
> +       u32 reg_csr_mask;               /* Mask bit for disable CSR reset */
> +       void __iomem *divider_reg;      /* CSR for divider */
> +       u32 reg_divider_offset;         /* Offset to divider register */
> +       u32 reg_divider_shift;          /* Bit shift to divider field */
> +       u32 reg_divider_width;          /* Width of the bit to divider field */
> +};
> +
> +struct xgene_clk {
> +       struct clk_hw   hw;
> +       const char      *name;
> +       spinlock_t      *lock;
> +       struct xgene_dev_parameters     param;
> +};
> +
> +#define to_xgene_clk(_hw) container_of(_hw, struct xgene_clk, hw)
> +
> +static int xgene_clk_enable(struct clk_hw *hw)
> +{
> +       struct xgene_clk *pclk = to_xgene_clk(hw);
> +       unsigned long flags = 0;
> +       u32 data;
> +
> +       if (pclk->lock)
> +               spin_lock_irqsave(pclk->lock, flags);
> +
> +       if (pclk->param.csr_reg != NULL) {
> +               pr_debug("%s clock enabled\n", pclk->name);
> +               /* First enable the clock */
> +               data = xgene_clk_read(pclk->param.csr_reg +
> +                                       pclk->param.reg_clk_offset);
> +               data |= pclk->param.reg_clk_mask;
> +               xgene_clk_write(data, pclk->param.csr_reg +
> +                                       pclk->param.reg_clk_offset);
> +               pr_debug("%s clock PADDR base 0x%016LX clk offset 0x%08X mask 0x%08X value 0x%08X\n",
> +                       pclk->name, __pa(pclk->param.csr_reg),
> +                       pclk->param.reg_clk_offset, pclk->param.reg_clk_mask,
> +                       data);
> +
> +               /* Second enable the CSR */
> +               data = xgene_clk_read(pclk->param.csr_reg +
> +                                       pclk->param.reg_csr_offset);
> +               data &= ~pclk->param.reg_csr_mask;
> +               xgene_clk_write(data, pclk->param.csr_reg +
> +                                       pclk->param.reg_csr_offset);
> +               pr_debug("%s CSR RESET PADDR base 0x%016LX csr offset 0x%08X mask 0x%08X value 0x%08X\n",
> +                       pclk->name, __pa(pclk->param.csr_reg),
> +                       pclk->param.reg_csr_offset, pclk->param.reg_csr_mask,
> +                       data);
> +       }
> +
> +       if (pclk->lock)
> +               spin_unlock_irqrestore(pclk->lock, flags);
> +
> +       return 0;
> +}
> +
> +static void xgene_clk_disable(struct clk_hw *hw)
> +{
> +       struct xgene_clk *pclk = to_xgene_clk(hw);
> +       unsigned long flags = 0;
> +       u32 data;
> +
> +       if (pclk->lock)
> +               spin_lock_irqsave(pclk->lock, flags);
> +
> +       if (pclk->param.csr_reg != NULL) {
> +               pr_debug("%s clock disabled\n", pclk->name);
> +               /* First put the CSR in reset */
> +               data = xgene_clk_read(pclk->param.csr_reg +
> +                                       pclk->param.reg_csr_offset);
> +               data |= pclk->param.reg_csr_mask;
> +               xgene_clk_write(data, pclk->param.csr_reg +
> +                                       pclk->param.reg_csr_offset);
> +
> +               /* Second disable the clock */
> +               data = xgene_clk_read(pclk->param.csr_reg +
> +                                       pclk->param.reg_clk_offset);
> +               data &= ~pclk->param.reg_clk_mask;
> +               xgene_clk_write(data, pclk->param.csr_reg +
> +                                       pclk->param.reg_clk_offset);
> +       }
> +
> +       if (pclk->lock)
> +               spin_unlock_irqrestore(pclk->lock, flags);
> +}
> +
> +static int xgene_clk_is_enabled(struct clk_hw *hw)
> +{
> +       struct xgene_clk *pclk = to_xgene_clk(hw);
> +       u32 data = 0;
> +
> +       if (pclk->param.csr_reg != NULL) {
> +               pr_debug("%s clock checking\n", pclk->name);
> +               data = xgene_clk_read(pclk->param.csr_reg +
> +                                       pclk->param.reg_clk_offset);
> +               pr_debug("%s clock is %s\n", pclk->name,
> +                       data & pclk->param.reg_clk_mask ? "enabled" :
> +                                                       "disabled");
> +       }
> +
> +       if (pclk->param.csr_reg == NULL)
> +               return 1;
> +       return data & pclk->param.reg_clk_mask ? 1 : 0;
> +}
> +
> +static unsigned long xgene_clk_recalc_rate(struct clk_hw *hw,
> +                               unsigned long parent_rate)
> +{
> +       struct xgene_clk *pclk = to_xgene_clk(hw);
> +       u32 data;
> +
> +       if (pclk->param.divider_reg) {
> +               data = xgene_clk_read(pclk->param.divider_reg +
> +                                       pclk->param.reg_divider_offset);
> +               data >>= pclk->param.reg_divider_shift;
> +               data &= (1 << pclk->param.reg_divider_width) - 1;
> +
> +               pr_debug("%s clock recalc rate %ld parent %ld\n",
> +                       pclk->name, parent_rate / data, parent_rate);
> +               return parent_rate / data;
> +       } else {
> +               pr_debug("%s clock recalc rate %ld parent %ld\n",
> +                       pclk->name, parent_rate, parent_rate);
> +               return parent_rate;
> +       }
> +}
> +
> +static int xgene_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> +                               unsigned long parent_rate)
> +{
> +       struct xgene_clk *pclk = to_xgene_clk(hw);
> +       unsigned long flags = 0;
> +       u32 data;
> +       u32 divider;
> +       u32 divider_save;
> +
> +       if (pclk->lock)
> +               spin_lock_irqsave(pclk->lock, flags);
> +
> +       if (pclk->param.divider_reg) {
> +               /* Let's compute the divider */
> +               if (rate > parent_rate)
> +                       rate = parent_rate;
> +               divider_save = divider = parent_rate / rate; /* Rounded down */
> +               divider &= (1 << pclk->param.reg_divider_width) - 1;
> +               divider <<= pclk->param.reg_divider_shift;
> +
> +               /* Set new divider */
> +               data = xgene_clk_read(pclk->param.divider_reg +
> +                               pclk->param.reg_divider_offset);
> +               data &= ~((1 << pclk->param.reg_divider_width) - 1);
> +               data |= divider;
> +               xgene_clk_write(data, pclk->param.divider_reg +
> +                                       pclk->param.reg_divider_offset);
> +               pr_debug("%s clock set rate %ld\n", pclk->name,
> +                       parent_rate / divider_save);
> +       } else {
> +               divider_save = 1;
> +       }
> +
> +       if (pclk->lock)
> +               spin_unlock_irqrestore(pclk->lock, flags);
> +
> +       return parent_rate / divider_save;
> +}
> +
> +static long xgene_clk_round_rate(struct clk_hw *hw, unsigned long rate,
> +                               unsigned long *prate)
> +{
> +       struct xgene_clk *pclk = to_xgene_clk(hw);
> +       unsigned long parent_rate = *prate;
> +       u32 divider;
> +
> +       if (pclk->param.divider_reg) {
> +               /* Let's compute the divider */
> +               if (rate > parent_rate)
> +                       rate = parent_rate;
> +               divider = parent_rate / rate;   /* Rounded down */
> +       } else {
> +               divider = 1;
> +       }
> +
> +       return parent_rate / divider;
> +}
> +
> +const struct clk_ops xgene_clk_ops = {
> +       .enable = xgene_clk_enable,
> +       .disable = xgene_clk_disable,
> +       .is_enabled = xgene_clk_is_enabled,
> +       .recalc_rate = xgene_clk_recalc_rate,
> +       .set_rate = xgene_clk_set_rate,
> +       .round_rate = xgene_clk_round_rate,
> +};
> +
> +static struct clk *xgene_register_clk(struct device *dev,
> +               const char *name, const char *parent_name,
> +               struct xgene_dev_parameters *parameters, spinlock_t *lock)
> +{
> +       struct xgene_clk *apmclk;
> +       struct clk *clk;
> +       struct clk_init_data init;
> +       int rc;
> +
> +       /* allocate the APM clock structure */
> +       apmclk = kzalloc(sizeof(struct xgene_clk), GFP_KERNEL);

Again, you could use kzalloc(sizeof(*apmclk), GFP_KERNEL).

> +       if (!apmclk) {
> +               pr_err("%s: could not allocate APM clk\n", __func__);
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       init.name = name;
> +       init.ops = &xgene_clk_ops;
> +       init.flags = parameters->flags;
> +       init.parent_names = parent_name ? &parent_name : NULL;
> +       init.num_parents = parent_name ? 1 : 0;
> +
> +       apmclk->name = name;
> +       apmclk->lock = lock;
> +       apmclk->hw.init = &init;
> +       apmclk->param = *parameters;
> +
> +       /* Register the clock */
> +       clk = clk_register(dev, &apmclk->hw);
> +       if (IS_ERR(clk)) {
> +               pr_err("%s: could not register clk %s\n", __func__, name);
> +               kfree(apmclk);
> +               return NULL;

As I understand it, NULL is a valid clock. Should you not return clk (which is
an error value) here?

> +       }
> +
> +       /* Register the clock for lookup */
> +       rc = clk_register_clkdev(clk, name, NULL);
> +       if (rc != 0) {
> +               pr_err("%s: could not register lookup clk %s\n",
> +                       __func__, name);
> +       }
> +       return clk;
> +}
> +
> +static void __init xgene_devclk_init(struct device_node *np)
> +{
> +       const char *clk_name = np->full_name;
> +       struct clk *clk;
> +       struct resource res;
> +       int rc;
> +       struct xgene_dev_parameters parameters;
> +
> +       rc = of_address_to_resource(np, 0, &res);
> +       if (rc != 0) {
> +               pr_err("no DTS CSR register for %s\n", np->full_name);
> +               return;
> +       }
> +       if (resource_size(&res)) {
> +               parameters.csr_reg = ioremap(res.start, resource_size(&res));
> +               if (parameters.csr_reg == NULL) {
> +                       pr_err("Unable to map CSR register for %s\n",
> +                               np->full_name);
> +                       return;
> +               }

Use of_iomap(np, 0) here.

> +       } else {
> +               parameters.csr_reg = NULL;
> +       }

Are you able to continue if this is the case?

> +
> +       rc = of_address_to_resource(np, 1, &res);
> +       if (rc != 0) {
> +               pr_err("no DTS DIV register for %s\n", np->full_name);
> +               return;
> +       }
> +       if (resource_size(&res)) {
> +               parameters.divider_reg = ioremap(res.start,
> +                                               resource_size(&res));
> +               if (parameters.divider_reg == NULL) {
> +                       pr_err("Unable to map DIV register for %s\n",
> +                               np->full_name);
> +                       if (parameters.csr_reg)
> +                               iounmap(parameters.csr_reg);
> +                       return;
> +               }

Use of_iomap(np, 1) here.

> +       } else {
> +               parameters.divider_reg = NULL;
> +       }

Similarly, are you able to continue with this being the case?

> +       if (of_property_read_u32(np, "flags", &parameters.flags))
> +               parameters.flags = 0;
> +       if (of_property_read_u32(np, "csr-offset", &parameters.reg_csr_offset))
> +               parameters.reg_csr_offset = 0;
> +       if (of_property_read_u32(np, "csr-mask", &parameters.reg_csr_mask))
> +               parameters.reg_csr_mask = 0xF;
> +       if (of_property_read_u32(np, "enable-offset",
> +                               &parameters.reg_clk_offset))
> +               parameters.reg_clk_offset = 0x8;
> +       if (of_property_read_u32(np, "enable-mask", &parameters.reg_clk_mask))
> +               parameters.reg_clk_mask = 0xF;
> +       if (of_property_read_u32(np, "divider-offset",
> +                               &parameters.reg_divider_offset))
> +               parameters.reg_divider_offset = 0;
> +       if (of_property_read_u32(np, "divider-width",
> +                               &parameters.reg_divider_width))
> +               parameters.reg_divider_width = 0;
> +       if (of_property_read_u32(np, "divider-shift",
> +                               &parameters.reg_divider_shift))
> +               parameters.reg_divider_shift = 0;
> +       of_property_read_string(np, "clock-output-names", &clk_name);
> +
> +       clk = xgene_register_clk(NULL, clk_name,
> +               of_clk_get_parent_name(np, 0), &parameters, &clk_lock);
> +       if (IS_ERR(clk)) {
> +               if (parameters.csr_reg)
> +                       iounmap(parameters.csr_reg);
> +               if (parameters.divider_reg)
> +                       iounmap(parameters.divider_reg);
> +               return;
> +       }
> +       pr_debug("Add %s clock\n", clk_name);
> +       rc = of_clk_add_provider(np, of_clk_src_simple_get, clk);
> +       if (rc != 0) {
> +               pr_err("%s: could register provider clk %s\n", __func__,
> +                       np->full_name);
> +               return;
> +       }
> +}
> +
> +CLK_OF_DECLARE(fixed_clock, "fixed-clock", of_fixed_clk_setup);
> +CLK_OF_DECLARE(fixed_factor_clock, "fixed-factor-clock",
> +               of_fixed_factor_clk_setup);

Why are these declared in your driver? Both fixed-clock and fixed-factor-clock
are standard clock bindings with existing drivers which already have
CLK_OF_DECLARE. Having a duplicate CLK_OF_DECLARE for those compatible strings
is only going to cause problems.

See drivers/clk/clk-fixed-rate.c and drivers/clk/clk-fixed-factor.c.

> +CLK_OF_DECLARE(xgene_pll_clock, "apm,xgene-pll-clock", xgene_pllclk_init);
> +CLK_OF_DECLARE(xgene_dev_clock, "apm,xgene-device-clock", xgene_devclk_init);
> +
> +static int __init xgene_clk_init(void)
> +{
> +       pr_info("XGene clock driver v%s\n", XGENE_CLK_DRIVER_VER);
> +       of_clk_init(NULL);

As of_clk_init is called in arm64_device_init, why do you call it here? It
looks very odd and I suspect you don't need it.

> +       return 0;
> +}
> +arch_initcall(xgene_clk_init);

I think this initcall can go.

Thanks,
Mark

[PATCH 3/3] Documentation: Add documentation for APM X-Gene clock binding.

[Mark Rutland]

On Fri, Jun 21, 2013 at 04:30:05PM +0100, Loc Ho wrote:
> Documentation: Add documentation for APM X-Gene clock binding with PLL and 
> device clocks.
> 
> Signed-off-by: Loc Ho <lho@apm.com>
> Signed-off-by: Kumar Sankaran <ksankaran@apm.com>
> Signed-off-by: Vinayak Kale <vkale@apm.com>
> Signed-off-by: Feng Kan <fkan@apm.com>
> ---
>  Documentation/devicetree/bindings/clock/xgene.txt |   90 +++++++++++++++++++++
>  1 files changed, 90 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/xgene.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/xgene.txt b/Documentation/devicetree/bindings/clock/xgene.txt
> new file mode 100644
> index 0000000..fcdee79
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/xgene.txt
> @@ -0,0 +1,90 @@
> +Device Tree Clock bindings for APM X-Gene
> +
> +This binding uses the common clock binding[1].
> +
> +[1] Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible : shall be one of the following:
> +	"apm,xgene-pll-clock" - for a X-Gene PLL clock
> +	"apm,xgene-device-clock" - for a X-Gene device clock
> +
> +Required properties for PLL clocks:
> +- reg : shall be the physical PLL register address for the pll clock.
> +- clocks : shall be the input parent clock phandle for the clock. This should
> +	be the reference clock.
> +- #clock-cells : shall be set to 1.
> +- clock-output-names : shall be the name of the PLL referenced by derive
> +  clock.
> +- type : shall be 1 for SoC PLL and 0 for PCP PLL.

That makes the binding very difficult for a human to read, could this not be
part of the compatible string? e.g. "apm,xgene-pcp-pll-clock".

> +Optional properties for PLL clocks:
> +- clock-names : shall be the name of the PLL. If missing, use the device name.
> +
> +Required properties for device clocks:
> +- reg : shall be the physical CSR reset address base and physical CSR divider
> +        address base. If one does not exist, specify 0 for address and 0 for 
> +        size.
> +- clocks : shall be the input parent clock phandle for the clock.
> +- #clock-cells : shall be set to 1.
> +- clock-output-names : shall be the name of the device referenced.
> +Optional properties for device clocks:
> +- clock-names : shall be the name of the device clock. If missing, use the
> +                device name.
> +- flags : Any clock flags. ie. use 0x8 to leave clock un-touch if not
> +          referenced. Default is 0.

What flags are these? We should *not* be exporting Linux-internal flags as
devicetree bindings.

> +- csr-offset : Offset to the CSR reset register from the reset address base.
> +               Default is 0.
> +- csr-mask : CSR reset mask bit. Default is 0xF.
> +- enable-offset : Offset to the enable register from the reset address base.
> +                  Default is 0x8.
> +- enable-mask : CSR enable mask bit. Default is 0xF.
> +- divider-offset : Offset to the divider CSR register from the divider base.
> +                   Default is 0x0.
> +- divider-width : Width of the divider register. Default is 0.
> +- divider-shift : Bit shift of the divider register. Default is 0.
> +
> +For example:
> +
> +	pcppll: pcppll at 17000100 {
> +		compatible = "apm,xgene-pll-clock";
> +		#clock-cells = <1>;
> +		clocks = <&refclk 0>;
> +		clock-names = "pcppll";
> +		reg = <0x0 0x17000100 0x0 0x1000>;
> +		clock-output-names = "pcppll";
> +		type = <0>;
> +	};
> +
> +	socpll: socpll at 17000120 {
> +		compatible = "apm,xgene-pll-clock";
> +		#clock-cells = <1>;
> +		clocks = <&refclk 0>;
> +		clock-names = "socpll";
> +		reg = <0x0 0x17000120 0x0 0x1000>;
> +		clock-output-names = "socpll";
> +		type = <1>;
> +	};
> +
> +	qmlclk: qmlclk {
> +		compatible = "apm,xgene-device-clock";
> +		#clock-cells = <1>;
> +		clocks = <&socplldiv2 0>;
> +		clock-names = "qmlclk";
> +		reg = <0x0 0x1703C000 0x0 0x1000
> +			0x0 0x00000000 0x0 0x0000>;
> +		clock-output-names = "qmlclk";
> +	};
> +
> +	ethclk: ethclk {
> +		compatible = "apm,xgene-device-clock";
> +		#clock-cells = <1>;
> +		clocks = <&socplldiv2 0>;
> +		clock-names = "ethclk";
> +		reg = <0x0 0x00000000 0x0 0x0000
> +			0x0 0x17000000 0x0 0x1000>;
> +			divider-offset = <0x238>;

Extraneous tab here.

Thanks,
Mark.

[PATCH 1/3] clk: Add APM X-Gene SoC clock driver for reference, PLL, and device clocks.

[Loc Ho]

Hi,

I will fix them. With regard to continue where the CSR is NULL, it is
possible to continue as long as both is NOT NULL.

> Use of_iomap(np, 0) here.
>
>> +       } else {
>> +               parameters.csr_reg = NULL;
>> +       }
>
> Are you able to continue if this is the case?
>
>> +
>> +       rc = of_address_to_resource(np, 1, &res);
>> +       if (rc != 0) {
>> +               pr_err("no DTS DIV register for %s\n", np->full_name);
>> +               return;
>> +       }
>> +       if (resource_size(&res)) {
>> +               parameters.divider_reg = ioremap(res.start,
>> +                                               resource_size(&res));
>> +               if (parameters.divider_reg == NULL) {
>> +                       pr_err("Unable to map DIV register for %s\n",
>> +                               np->full_name);
>> +                       if (parameters.csr_reg)
>> +                               iounmap(parameters.csr_reg);
>> +                       return;
>> +               }
>
> Use of_iomap(np, 1) here.
>
>> +       } else {
>> +               parameters.divider_reg = NULL;
>> +       }
>
> Similarly, are you able to continue with this being the case?

-Loc

[PATCH 3/3] Documentation: Add documentation for APM X-Gene clock binding.

[Loc Ho]

HI,


>> +Required properties for PLL clocks:
>> +- reg : shall be the physical PLL register address for the pll clock.
>> +- clocks : shall be the input parent clock phandle for the clock. This should
>> +     be the reference clock.
>> +- #clock-cells : shall be set to 1.
>> +- clock-output-names : shall be the name of the PLL referenced by derive
>> +  clock.
>> +- type : shall be 1 for SoC PLL and 0 for PCP PLL.
>
> That makes the binding very difficult for a human to read, could this not be
> part of the compatible string? e.g. "apm,xgene-pcp-pll-clock".
[Loc Ho]
I guess I could by they will only differ by one function in term of
rate computation.

>
>> +Optional properties for PLL clocks:
>> +- clock-names : shall be the name of the PLL. If missing, use the device name.
>> +
>> +Required properties for device clocks:
>> +- reg : shall be the physical CSR reset address base and physical CSR divider
>> +        address base. If one does not exist, specify 0 for address and 0 for
>> +        size.
>> +- clocks : shall be the input parent clock phandle for the clock.
>> +- #clock-cells : shall be set to 1.
>> +- clock-output-names : shall be the name of the device referenced.
>> +Optional properties for device clocks:
>> +- clock-names : shall be the name of the device clock. If missing, use the
>> +                device name.
>> +- flags : Any clock flags. ie. use 0x8 to leave clock un-touch if not
>> +          referenced. Default is 0.
>
> What flags are these? We should *not* be exporting Linux-internal flags as
> devicetree bindings.
[Loc Ho]
I need the CLK ignore if un-used. This is needed in case an subsystem
- such as PCIE is NOT powered on. With different board design, some IP
may not have power. Accessing the clock registers will cause exception
in the kernel. I guess I could just add the clock IGNORE by default
and not require this.

-Loc

[PATCH 3/3] Documentation: Add documentation for APM X-Gene clock binding.

[Mark Rutland]

On Mon, Jun 24, 2013 at 06:57:14PM +0100, Loc Ho wrote:
> HI,
> 
> 
> >> +Required properties for PLL clocks:
> >> +- reg : shall be the physical PLL register address for the pll clock.
> >> +- clocks : shall be the input parent clock phandle for the clock. This should
> >> +     be the reference clock.
> >> +- #clock-cells : shall be set to 1.
> >> +- clock-output-names : shall be the name of the PLL referenced by derive
> >> +  clock.
> >> +- type : shall be 1 for SoC PLL and 0 for PCP PLL.
> >
> > That makes the binding very difficult for a human to read, could this not be
> > part of the compatible string? e.g. "apm,xgene-pcp-pll-clock".
> [Loc Ho]
> I guess I could by they will only differ by one function in term of
> rate computation.

Having a clear binding is useful, and it's just as easy to do a
comparison on the compatible string as it is to deal with the an opaque
type parameter. Another option would be to make the type parameter a
string (i.e type = "soc" or type = "pcp"), if you feel that they are
similar enough to not warrant separate bindings.

> 
> >
> >> +Optional properties for PLL clocks:
> >> +- clock-names : shall be the name of the PLL. If missing, use the device name.
> >> +
> >> +Required properties for device clocks:
> >> +- reg : shall be the physical CSR reset address base and physical CSR divider
> >> +        address base. If one does not exist, specify 0 for address and 0 for
> >> +        size.
> >> +- clocks : shall be the input parent clock phandle for the clock.
> >> +- #clock-cells : shall be set to 1.
> >> +- clock-output-names : shall be the name of the device referenced.
> >> +Optional properties for device clocks:
> >> +- clock-names : shall be the name of the device clock. If missing, use the
> >> +                device name.
> >> +- flags : Any clock flags. ie. use 0x8 to leave clock un-touch if not
> >> +          referenced. Default is 0.
> >
> > What flags are these? We should *not* be exporting Linux-internal flags as
> > devicetree bindings.
> [Loc Ho]
> I need the CLK ignore if un-used. This is needed in case an subsystem
> - such as PCIE is NOT powered on. With different board design, some IP
> may not have power. Accessing the clock registers will cause exception
> in the kernel. I guess I could just add the clock IGNORE by default
> and not require this.

In cases where the clocks (or any devices) are unusable on a system for
some reason (e.g.  they are unpowered), you could instead set their
status property to "disabled" in a board-specific dts to describe this
(or deafualt them to "disabled" and override this to "okay" in a board
file). That's the conventional way of doing it, and there's already
framework for this in the kernel that prevents them from getting probed
(though I'm not enitrely sure if this is the case when using
*_OF_DECLARE).

Thanks,
Mark.

[PATCH 1/3] clk: Add APM X-Gene SoC clock driver for reference, PLL, and device clocks.

[Mark Rutland]

On Mon, Jun 24, 2013 at 06:48:56PM +0100, Loc Ho wrote:
> Hi,
> 
> I will fix them. With regard to continue where the CSR is NULL, it is
> possible to continue as long as both is NOT NULL.

Ah, so one bank of registers might not be present? Given that, it might
be worth using interrupt-names to describe which is present, so that you
don't need to describe a non-existent register bank.

Thanks,
Mark.

> 
> > Use of_iomap(np, 0) here.
> >
> >> +       } else {
> >> +               parameters.csr_reg = NULL;
> >> +       }
> >
> > Are you able to continue if this is the case?
> >
> >> +
> >> +       rc = of_address_to_resource(np, 1, &res);
> >> +       if (rc != 0) {
> >> +               pr_err("no DTS DIV register for %s\n", np->full_name);
> >> +               return;
> >> +       }
> >> +       if (resource_size(&res)) {
> >> +               parameters.divider_reg = ioremap(res.start,
> >> +                                               resource_size(&res));
> >> +               if (parameters.divider_reg == NULL) {
> >> +                       pr_err("Unable to map DIV register for %s\n",
> >> +                               np->full_name);
> >> +                       if (parameters.csr_reg)
> >> +                               iounmap(parameters.csr_reg);
> >> +                       return;
> >> +               }
> >
> > Use of_iomap(np, 1) here.
> >
> >> +       } else {
> >> +               parameters.divider_reg = NULL;
> >> +       }
> >
> > Similarly, are you able to continue with this being the case?
> 
> -Loc
> 

[PATCH 3/3] Documentation: Add documentation for APM X-Gene clock binding.

[Loc Ho]

Hi,


>> >> +Required properties for PLL clocks:
>> >> +- reg : shall be the physical PLL register address for the pll clock.
>> >> +- clocks : shall be the input parent clock phandle for the clock. This should
>> >> +     be the reference clock.
>> >> +- #clock-cells : shall be set to 1.
>> >> +- clock-output-names : shall be the name of the PLL referenced by derive
>> >> +  clock.
>> >> +- type : shall be 1 for SoC PLL and 0 for PCP PLL.
>> >
>> > That makes the binding very difficult for a human to read, could this not be
>> > part of the compatible string? e.g. "apm,xgene-pcp-pll-clock".
>> [Loc Ho]
>> I guess I could by they will only differ by one function in term of
>> rate computation.
>
> Having a clear binding is useful, and it's just as easy to do a
> comparison on the compatible string as it is to deal with the an opaque
> type parameter. Another option would be to make the type parameter a
> string (i.e type = "soc" or type = "pcp"), if you feel that they are
> similar enough to not warrant separate bindings.
[Loc Ho]
I just create two binding type. And just set the type parameter based
on binding.

>> >> +Optional properties for PLL clocks:
>> >> +- clock-names : shall be the name of the PLL. If missing, use the device name.
>> >> +
>> >> +Required properties for device clocks:
>> >> +- reg : shall be the physical CSR reset address base and physical CSR divider
>> >> +        address base. If one does not exist, specify 0 for address and 0 for
>> >> +        size.
>> >> +- clocks : shall be the input parent clock phandle for the clock.
>> >> +- #clock-cells : shall be set to 1.
>> >> +- clock-output-names : shall be the name of the device referenced.
>> >> +Optional properties for device clocks:
>> >> +- clock-names : shall be the name of the device clock. If missing, use the
>> >> +                device name.
>> >> +- flags : Any clock flags. ie. use 0x8 to leave clock un-touch if not
>> >> +          referenced. Default is 0.
>> >
>> > What flags are these? We should *not* be exporting Linux-internal flags as
>> > devicetree bindings.
>> [Loc Ho]
>> I need the CLK ignore if un-used. This is needed in case an subsystem
>> - such as PCIE is NOT powered on. With different board design, some IP
>> may not have power. Accessing the clock registers will cause exception
>> in the kernel. I guess I could just add the clock IGNORE by default
>> and not require this.
>
> In cases where the clocks (or any devices) are unusable on a system for
> some reason (e.g.  they are unpowered), you could instead set their
> status property to "disabled" in a board-specific dts to describe this
> (or deafualt them to "disabled" and override this to "okay" in a board
> file). That's the conventional way of doing it, and there's already
> framework for this in the kernel that prevents them from getting probed
> (though I'm not enitrely sure if this is the case when using
> *_OF_DECLARE).
[Loc Ho]
We are doing this. Though, this requires two set of DTS file and/or
patching the DTS from U-Boot or BIOS. I am trying to avoid having
multiple DTS file and having to write patching code.

-Loc