[PATCH v4 0/2] CREG clk driver for NXP LPC18xx family

[PATCH v4 0/2] CREG clk driver for NXP LPC18xx family

[Joachim Eastwood]

This patch set adds a clk driver for the low power clocks found in
the CREG block on lpc18xx. CREG is a collection of miscellaneous
configuration registers that can be accessed through a syscon
regmap interface. The clk driver makes it possible to setup and
enabled these two clocks.

This need to support peripherals like the internal RTC on the
lpc18xx platform.

The driver has one early init using CLK_OF_DECLARE and a standard
platform driver. The early init is used to register the 32KHZ clk
which may be needed early on. The other clock (1KHZ) is registered
when the platform driver probe runs.

This version splits clk array into clk_creg_early and clk_creg. I
think this will work as long as clk_creg_early is _not_ used after
the clk_creg is registered.

This seems to work, but I currently don't have a setup that requires
the 32khz clock early so that part isn't really tested.

The probe deferral seems to work for the RTC:
[    3.386861] lpc24xx-rtc 40046000.rtc: error getting rtc clock
[    6.737615] lpc24xx-rtc 40046000.rtc: rtc core: registered lpc24xx-rtc as rtc0


Changes since v3:
 - Add .is_enabled function to both creg clks.
 - Remove unit address from bindings as requested by Rob Herring.

Changes since v2:
 - Use two clk arrays for registration.
 - Couple of minor cleanups.

Changes since v1:
 - Split driver with one early init and one platform driver
   probe.
 - Rebase on v4.5-rc1.

Joachim Eastwood (2):
  clk: add lpc18xx creg clk driver
  doc: dt: add documentation for lpc1850-creg-clk driver

 .../devicetree/bindings/clock/lpc1850-creg-clk.txt |  52 +++++
 drivers/clk/nxp/Makefile                           |   1 +
 drivers/clk/nxp/clk-lpc18xx-creg.c                 | 222 +++++++++++++++++++++
 3 files changed, 275 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt
 create mode 100644 drivers/clk/nxp/clk-lpc18xx-creg.c

-- 
1.8.0

[PATCH v4 1/2] clk: add lpc18xx creg clk driver

[Joachim Eastwood]

The CREG block on lpc18xx contains configuration register
for two low power clocks. Support enabling of these two
clocks with a clk driver that access CREG trough the
syscon regmap interface.

These clocks are needed to support peripherals like the
internal RTC on lpc18xx.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 drivers/clk/nxp/Makefile           |   1 +
 drivers/clk/nxp/clk-lpc18xx-creg.c | 222 +++++++++++++++++++++++++++++++++++++
 2 files changed, 223 insertions(+)
 create mode 100644 drivers/clk/nxp/clk-lpc18xx-creg.c

diff --git a/drivers/clk/nxp/Makefile b/drivers/clk/nxp/Makefile
index 607bd48c6563..d456ee6cc3d3 100644
--- a/drivers/clk/nxp/Makefile
+++ b/drivers/clk/nxp/Makefile
@@ -1,3 +1,4 @@
 obj-$(CONFIG_ARCH_LPC18XX)	+= clk-lpc18xx-cgu.o
 obj-$(CONFIG_ARCH_LPC18XX)	+= clk-lpc18xx-ccu.o
+obj-$(CONFIG_ARCH_LPC18XX)	+= clk-lpc18xx-creg.o
 obj-$(CONFIG_ARCH_LPC32XX)	+= clk-lpc32xx.o
diff --git a/drivers/clk/nxp/clk-lpc18xx-creg.c b/drivers/clk/nxp/clk-lpc18xx-creg.c
new file mode 100644
index 000000000000..a7c8f966d925
--- /dev/null
+++ b/drivers/clk/nxp/clk-lpc18xx-creg.c
@@ -0,0 +1,222 @@
+/*
+ * Clk driver for NXP LPC18xx/43xx Configuration Registers (CREG)
+ *
+ * Copyright (C) 2015 Joachim Eastwood <manabian@gmail.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/mfd/syscon.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+
+#define LPC18XX_CREG_CREG0			0x004
+#define  LPC18XX_CREG_CREG0_EN1KHZ		BIT(0)
+#define  LPC18XX_CREG_CREG0_EN32KHZ		BIT(1)
+#define  LPC18XX_CREG_CREG0_RESET32KHZ		BIT(2)
+#define  LPC18XX_CREG_CREG0_PD32KHZ		BIT(3)
+
+#define to_clk_creg(_hw) container_of(_hw, struct clk_creg_data, hw)
+
+enum {
+	CREG_CLK_1KHZ,
+	CREG_CLK_32KHZ,
+	CREG_CLK_MAX,
+};
+
+struct clk_creg_data {
+	struct clk_hw hw;
+	const char *name;
+	struct regmap *reg;
+	unsigned int en_mask;
+	const struct clk_ops *ops;
+};
+
+#define CREG_CLK(_name, _emask, _ops)		\
+{						\
+	.name = _name,				\
+	.en_mask = LPC18XX_CREG_CREG0_##_emask,	\
+	.ops = &_ops,				\
+}
+
+static int clk_creg_32k_prepare(struct clk_hw *hw)
+{
+	struct clk_creg_data *creg = to_clk_creg(hw);
+	int ret;
+
+	ret = regmap_update_bits(creg->reg, LPC18XX_CREG_CREG0,
+				 LPC18XX_CREG_CREG0_PD32KHZ |
+				 LPC18XX_CREG_CREG0_RESET32KHZ, 0);
+
+	/*
+	 * Powering up the 32k oscillator takes a long while
+	 * and sadly there aren't any status bit to poll.
+	 */
+	msleep(2500);
+
+	return ret;
+}
+
+static void clk_creg_32k_unprepare(struct clk_hw *hw)
+{
+	struct clk_creg_data *creg = to_clk_creg(hw);
+
+	regmap_update_bits(creg->reg, LPC18XX_CREG_CREG0,
+			   LPC18XX_CREG_CREG0_PD32KHZ,
+			   LPC18XX_CREG_CREG0_PD32KHZ);
+}
+
+static int clk_creg_32k_is_prepared(struct clk_hw *hw)
+{
+	struct clk_creg_data *creg = to_clk_creg(hw);
+	u32 reg;
+
+	regmap_read(creg->reg, LPC18XX_CREG_CREG0, &reg);
+
+	return !(reg & LPC18XX_CREG_CREG0_PD32KHZ) &&
+	       !(reg & LPC18XX_CREG_CREG0_RESET32KHZ);
+}
+
+static unsigned long clk_creg_1k_recalc_rate(struct clk_hw *hw,
+					     unsigned long parent_rate)
+{
+	return parent_rate / 32;
+}
+
+static int clk_creg_enable(struct clk_hw *hw)
+{
+	struct clk_creg_data *creg = to_clk_creg(hw);
+
+	return regmap_update_bits(creg->reg, LPC18XX_CREG_CREG0,
+				  creg->en_mask, creg->en_mask);
+}
+
+static void clk_creg_disable(struct clk_hw *hw)
+{
+	struct clk_creg_data *creg = to_clk_creg(hw);
+
+	regmap_update_bits(creg->reg, LPC18XX_CREG_CREG0,
+			   creg->en_mask, 0);
+}
+
+static int clk_creg_is_enabled(struct clk_hw *hw)
+{
+	struct clk_creg_data *creg = to_clk_creg(hw);
+	u32 reg;
+
+	regmap_read(creg->reg, LPC18XX_CREG_CREG0, &reg);
+
+	return !!(reg & creg->en_mask);
+}
+
+static const struct clk_ops clk_creg_32k = {
+	.enable		= clk_creg_enable,
+	.disable	= clk_creg_disable,
+	.is_enabled	= clk_creg_is_enabled,
+	.prepare	= clk_creg_32k_prepare,
+	.unprepare	= clk_creg_32k_unprepare,
+	.is_prepared	= clk_creg_32k_is_prepared,
+};
+
+static const struct clk_ops clk_creg_1k = {
+	.enable		= clk_creg_enable,
+	.disable	= clk_creg_disable,
+	.is_enabled	= clk_creg_is_enabled,
+	.recalc_rate	= clk_creg_1k_recalc_rate,
+};
+
+static struct clk_creg_data clk_creg_clocks[] = {
+	[CREG_CLK_1KHZ]  = CREG_CLK("1khz_clk",  EN1KHZ,  clk_creg_1k),
+	[CREG_CLK_32KHZ] = CREG_CLK("32khz_clk", EN32KHZ, clk_creg_32k),
+};
+
+static struct clk *clk_register_creg_clk(struct clk_creg_data *creg_clk,
+					 const char **parent_name,
+					 struct regmap *syscon)
+{
+	struct clk_init_data init;
+
+	init.ops = creg_clk->ops;
+	init.name = creg_clk->name;
+	init.parent_names = parent_name;
+	init.num_parents = 1;
+
+	creg_clk->reg = syscon;
+	creg_clk->hw.init = &init;
+
+	return clk_register(NULL, &creg_clk->hw);
+}
+
+static struct clk *clk_creg_early[CREG_CLK_MAX];
+static struct clk_onecell_data clk_creg_early_data = {
+	.clks = clk_creg_early,
+	.clk_num = CREG_CLK_MAX,
+};
+
+static void __init lpc18xx_creg_clk_init(struct device_node *np)
+{
+	const char *clk_32khz_parent;
+	struct regmap *syscon;
+
+	syscon = syscon_node_to_regmap(np->parent);
+	if (IS_ERR(syscon)) {
+		pr_err("%s: syscon lookup failed\n", __func__);
+		return;
+	}
+
+	clk_32khz_parent = of_clk_get_parent_name(np, 0);
+
+	clk_creg_early[CREG_CLK_32KHZ] =
+		clk_register_creg_clk(&clk_creg_clocks[CREG_CLK_32KHZ],
+				      &clk_32khz_parent, syscon);
+	clk_creg_early[CREG_CLK_1KHZ] = ERR_PTR(-EPROBE_DEFER);
+
+	of_clk_add_provider(np, of_clk_src_onecell_get, &clk_creg_early_data);
+}
+CLK_OF_DECLARE(lpc18xx_creg_clk, "nxp,lpc1850-creg-clk", lpc18xx_creg_clk_init);
+
+static struct clk *clk_creg[CREG_CLK_MAX];
+static struct clk_onecell_data clk_creg_data = {
+	.clks = clk_creg,
+	.clk_num = CREG_CLK_MAX,
+};
+
+static int lpc18xx_creg_clk_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct regmap *syscon;
+
+	syscon = syscon_node_to_regmap(np->parent);
+	if (IS_ERR(syscon)) {
+		dev_err(&pdev->dev, "syscon lookup failed\n");
+		return PTR_ERR(syscon);
+	}
+
+	clk_creg[CREG_CLK_32KHZ] = clk_creg_early[CREG_CLK_32KHZ];
+	clk_creg[CREG_CLK_1KHZ] =
+		clk_register_creg_clk(&clk_creg_clocks[CREG_CLK_1KHZ],
+				      &clk_creg_clocks[CREG_CLK_32KHZ].name,
+				      syscon);
+
+	return of_clk_add_provider(np, of_clk_src_onecell_get, &clk_creg_data);
+}
+
+static const struct of_device_id lpc18xx_creg_clk_of_match[] = {
+	{ .compatible = "nxp,lpc1850-creg-clk" },
+	{},
+};
+
+static struct platform_driver lpc18xx_creg_clk_driver = {
+	.probe = lpc18xx_creg_clk_probe,
+	.driver = {
+		.name = "lpc18xx-creg-clk",
+		.of_match_table = lpc18xx_creg_clk_of_match,
+	},
+};
+builtin_platform_driver(lpc18xx_creg_clk_driver);
-- 
1.8.0

[PATCH v4 2/2] doc: dt: add documentation for lpc1850-creg-clk driver

[Joachim Eastwood]

Add DT binding documentation for lpc1850-creg-clk driver.

Signed-off-by: Joachim Eastwood <manabian@gmail.com>
---
 .../devicetree/bindings/clock/lpc1850-creg-clk.txt | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt

diff --git a/Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt b/Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt
new file mode 100644
index 000000000000..4e13586d083c
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt
@@ -0,0 +1,52 @@
+* NXP LPC1850 CREG clocks
+
+The NXP LPC18xx/43xx CREG (Configuration Registers) block contains
+control registers for two low speed clocks. One of the clocks is a
+32 kHz oscillator driver with power up/down and clock gating. Next
+is a fixed divider that creates a 1 kHz clock from the 32 kHz osc.
+
+These clocks are used by the RTC and the Event Router peripherials.
+The 32 kHz can also be routed to other peripherials to enable low
+power modes.
+
+This binding uses the common clock binding:
+    Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible:
+	Should be "nxp,lpc1850-creg-clk"
+- #clock-cells:
+	Shall have value <1>.
+- clocks:
+	Shall contain a phandle to the fixed 32 kHz crystall.
+
+The creg-clk node must be a child of the creg syscon node.
+
+The following clocks are available from the clock node.
+
+Clock ID	Name
+   0		 1 kHz clock
+   1		32 kHz Oscillator
+
+Example:
+soc {
+	creg: syscon@40043000 {
+		compatible = "nxp,lpc1850-creg", "syscon", "simple-mfd";
+		reg = <0x40043000 0x1000>;
+
+		creg_clk: clock-controller {
+			compatible = "nxp,lpc1850-creg-clk";
+			clocks = <&xtal32>;
+			#clock-cells = <1>;
+		};
+
+		...
+	};
+
+	rtc: rtc@40046000 {
+		...
+		clocks = <&creg_clk 0>, <&ccu1 CLK_CPU_BUS>;
+		clock-names = "rtc", "reg";
+		...
+	};
+};
-- 
1.8.0

Re: [PATCH v4 2/2] doc: dt: add documentation for lpc1850-creg-clk driver

[Rob Herring]

On Wed, Feb 24, 2016 at 11:04:20PM +0100, Joachim Eastwood wrote:
> Add DT binding documentation for lpc1850-creg-clk driver.
> 
> Signed-off-by: Joachim Eastwood <manabian@gmail.com>
> ---
>  .../devicetree/bindings/clock/lpc1850-creg-clk.txt | 52 ++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt

One typo, otherwise:

Acked-by: Rob Herring <robh@kernel.org>

> 
> diff --git a/Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt b/Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt
> new file mode 100644
> index 000000000000..4e13586d083c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/lpc1850-creg-clk.txt
> @@ -0,0 +1,52 @@
> +* NXP LPC1850 CREG clocks
> +
> +The NXP LPC18xx/43xx CREG (Configuration Registers) block contains
> +control registers for two low speed clocks. One of the clocks is a
> +32 kHz oscillator driver with power up/down and clock gating. Next
> +is a fixed divider that creates a 1 kHz clock from the 32 kHz osc.
> +
> +These clocks are used by the RTC and the Event Router peripherials.
> +The 32 kHz can also be routed to other peripherials to enable low
> +power modes.
> +
> +This binding uses the common clock binding:
> +    Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible:
> +	Should be "nxp,lpc1850-creg-clk"
> +- #clock-cells:
> +	Shall have value <1>.
> +- clocks:
> +	Shall contain a phandle to the fixed 32 kHz crystall.

s/crystall/crystal/

> +
> +The creg-clk node must be a child of the creg syscon node.
> +
> +The following clocks are available from the clock node.
> +
> +Clock ID	Name
> +   0		 1 kHz clock
> +   1		32 kHz Oscillator
> +
> +Example:
> +soc {
> +	creg: syscon@40043000 {
> +		compatible = "nxp,lpc1850-creg", "syscon", "simple-mfd";
> +		reg = <0x40043000 0x1000>;
> +
> +		creg_clk: clock-controller {
> +			compatible = "nxp,lpc1850-creg-clk";
> +			clocks = <&xtal32>;
> +			#clock-cells = <1>;
> +		};
> +
> +		...
> +	};
> +
> +	rtc: rtc@40046000 {
> +		...
> +		clocks = <&creg_clk 0>, <&ccu1 CLK_CPU_BUS>;
> +		clock-names = "rtc", "reg";
> +		...
> +	};
> +};
> -- 
> 1.8.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v4 1/2] clk: add lpc18xx creg clk driver

[Michael Turquette]

Hi Joachim,

Quoting Joachim Eastwood (2016-02-24 14:04:19)
> +static struct clk *clk_register_creg_clk(struct clk_creg_data *creg_clk,
> +                                        const char **parent_name,
> +                                        struct regmap *syscon)
> +{
> +       struct clk_init_data init;
> +
> +       init.ops = creg_clk->ops;
> +       init.name = creg_clk->name;
> +       init.parent_names = parent_name;
> +       init.num_parents = 1;
> +
> +       creg_clk->reg = syscon;
> +       creg_clk->hw.init = &init;
> +
> +       return clk_register(NULL, &creg_clk->hw);

This should be devm_clk_register with your struct device *dev passed
into it.

Otherwise this patch looks good for merge.

Regards,
Mike

Re: [PATCH v4 1/2] clk: add lpc18xx creg clk driver

[Joachim Eastwood]

On 3 March 2016 at 00:01, Michael Turquette <mturquette@baylibre.com> wrote:
> Hi Joachim,
>
> Quoting Joachim Eastwood (2016-02-24 14:04:19)
>> +static struct clk *clk_register_creg_clk(struct clk_creg_data *creg_clk,
>> +                                        const char **parent_name,
>> +                                        struct regmap *syscon)
>> +{
>> +       struct clk_init_data init;
>> +
>> +       init.ops = creg_clk->ops;
>> +       init.name = creg_clk->name;
>> +       init.parent_names = parent_name;
>> +       init.num_parents = 1;
>> +
>> +       creg_clk->reg = syscon;
>> +       creg_clk->hw.init = &init;
>> +
>> +       return clk_register(NULL, &creg_clk->hw);
>
> This should be devm_clk_register with your struct device *dev passed
> into it.

Since clk_register_creg_clk() is used both from early init and probe
I'll need to change the function a bit.

I'll pass dev in as a parameter and either do clk_register() or
devm_clk_register() depending on dev being NULL or not.
Doesn't look like devm_clk_register() would like to be passed NULL for dev.


I send out a new version tomorrow.

btw, this driver is a builtin_platform_driver. So is there a point in
using devm_clk_register?


regards,
Joachim Eastwood

Re: [PATCH v4 1/2] clk: add lpc18xx creg clk driver

[Stephen Boyd]

On 03/03, Joachim Eastwood wrote:
> On 3 March 2016 at 00:01, Michael Turquette <mturquette@baylibre.com> wrote:
> >
> > This should be devm_clk_register with your struct device *dev passed
> > into it.
> 
> Since clk_register_creg_clk() is used both from early init and probe
> I'll need to change the function a bit.
> 
> I'll pass dev in as a parameter and either do clk_register() or
> devm_clk_register() depending on dev being NULL or not.
> Doesn't look like devm_clk_register() would like to be passed NULL for dev.
> 
> 
> I send out a new version tomorrow.
> 
> btw, this driver is a builtin_platform_driver. So is there a point in
> using devm_clk_register?
> 

Builtin drivers can still probe defer or fail for other reasons,
so just in case that happens it's better to use
devm_clk_register() to make sure we cleanup on the error path. It
may never actually happen in practice, but it's better to write
safe drivers.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project