[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Jagan Teki]

This is revised version of previous i.MX6 clock management [1]. 

The main difference between previous version is
- Group the i.MX6 ccm clocks into gates and tree instead of handling the 
  clocks in simple way using case statement.
- use gate clocks for enable/disable management.
- use tree clocks for get/set rate or parent traverse management.
- parent clock handling via clock type.
- traverse the parent clock using recursive functionlaity.

The main motive behind this tree framework is to make the clock tree 
management simple and useful for U-Boot requirements instead of garbing 
Linux clock management code.

We are trying to manage the Allwinner clocks with similar kind, so having 
this would really help i.MX6 as well.

Added simple names for clock macros, but will update it in future
version.

I have skipped ENET clocks from previous series, will add it in future
patches.

Changes for v2:
- changed framework patches.
- add support for imx6qdl and imx6ul boards
- add clock gates, tree.

[1] https://patchwork.ozlabs.org/cover/950964/

Any inputs?
Jagan.

Jagan Teki (10):
  clk: imx: Kconfig: Make CONFIG_CLK available for selection
  clk: imx: Add i.MX6Q clock driver
  clk: imx: Add i.MX6UL clock driver
  clk: Add clk_div_mask helper
  clk: imx: Add imx6q clock tree support
  clk: imx6: Add imx6ul clock tree support
  ARM: dts: i.MX6QDL: Add u-boot,dm-spl for clks
  ARM: dts: i.MX6UL: Add u-boot,dm-spl for clks
  configs: icore_mipi: Enable CLK
  ARM: imx6: Enable CLK for Engicam i.MX6UL boards

 arch/arm/dts/imx6qdl-u-boot.dtsi      |   4 +
 arch/arm/dts/imx6ul-u-boot.dtsi       |   4 +
 arch/arm/include/asm/arch-mx6/clock.h | 109 ++++++++++++++++
 arch/arm/mach-imx/mx6/Kconfig         |   2 +
 configs/imx6qdl_icore_mipi_defconfig  |   2 +
 configs/imx8qxp_mek_defconfig         |   2 +-
 drivers/clk/imx/Kconfig               |  29 ++++-
 drivers/clk/imx/Makefile              |   6 +
 drivers/clk/imx/clk-imx6-common.c     | 172 ++++++++++++++++++++++++++
 drivers/clk/imx/clk-imx6q.c           | 109 ++++++++++++++++
 drivers/clk/imx/clk-imx6ul.c          |  85 +++++++++++++
 include/clk-uclass.h                  |   2 +
 12 files changed, 523 insertions(+), 3 deletions(-)
 create mode 100644 drivers/clk/imx/clk-imx6-common.c
 create mode 100644 drivers/clk/imx/clk-imx6q.c
 create mode 100644 drivers/clk/imx/clk-imx6ul.c

-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v2 01/10] clk: imx: Kconfig: Make CONFIG_CLK available for selection

[Jagan Teki]

Adjust the Kconfig to hide architecture and other internal configs
so-that generic CONFIG_CLK is available for selection.

This would make easy for configuration selections between SoC
families with in the same SoC and even more readable.

Cc: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 configs/imx8qxp_mek_defconfig |  2 +-
 drivers/clk/imx/Kconfig       | 15 +++++++++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/configs/imx8qxp_mek_defconfig b/configs/imx8qxp_mek_defconfig
index a94998b8b5..8ad7ef4c04 100644
--- a/configs/imx8qxp_mek_defconfig
+++ b/configs/imx8qxp_mek_defconfig
@@ -40,7 +40,7 @@ CONFIG_DEFAULT_DEVICE_TREE="fsl-imx8qxp-mek"
 CONFIG_ENV_IS_IN_MMC=y
 CONFIG_SPL_DM=y
 CONFIG_SPL_CLK=y
-CONFIG_CLK_IMX8=y
+CONFIG_CLK=y
 CONFIG_CPU=y
 CONFIG_DM_GPIO=y
 CONFIG_MXC_GPIO=y
diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index a6fb58d6cf..a125c23a19 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -1,6 +1,17 @@
+config CLK_IMX
+	bool "Clock support for i.MX SoCs"
+	depends on CLK && ARCH_IMX8
+	default y
+	help
+	  This enables support for common clock driver API on i.MX
+	  SoCs.
+
+if CLK_IMX
+
 config CLK_IMX8
 	bool "Clock support for i.MX8"
-	depends on ARCH_IMX8
-	select CLK
+	default ARCH_IMX8
 	help
 	  This enables support clock driver for i.MX8 platforms.
+
+endif # CLK_IMX
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v2 02/10] clk: imx: Add i.MX6Q clock driver

[Jagan Teki]

i.MX6 clock control module comprise of parent clocks, gates, multiplexers,
dividers, PODF, PLL, fixed rate and etc.

This patch add i.MX6Q USDHC clocks via gate clock which would eventually
handle enable/disable operations via imx6_clock_gate.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/include/asm/arch-mx6/clock.h | 44 +++++++++++++++++
 drivers/clk/imx/Kconfig               |  9 +++-
 drivers/clk/imx/Makefile              |  5 ++
 drivers/clk/imx/clk-imx6-common.c     | 69 +++++++++++++++++++++++++++
 drivers/clk/imx/clk-imx6q.c           | 39 +++++++++++++++
 5 files changed, 165 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/imx/clk-imx6-common.c
 create mode 100644 drivers/clk/imx/clk-imx6q.c

diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h
index a9481a5fea..fa921a9f08 100644
--- a/arch/arm/include/asm/arch-mx6/clock.h
+++ b/arch/arm/include/asm/arch-mx6/clock.h
@@ -21,6 +21,50 @@
 #define MXC_CLK32	32768
 #endif
 
+/**
+ * struct imx6_clk_gate - imx6 ccm clock gate
+ *
+ * @off:	ccm gate offset
+ * @bit:	ccm gate bit
+ */
+struct imx6_clk_gate {
+	u16 off;
+	u32 bit;
+};
+
+#define GATE(_off, _bit) {			\
+	.off = _off,				\
+	.bit = _bit,				\
+}
+
+/**
+ * struct imx6_clk_desc - imx6 clock control module descriptor
+ *
+ * @gates:	ccm clock gates
+ */
+struct imx6_clk_desc {
+	const struct imx6_clk_gate *gates;
+};
+
+/**
+ * struct imx6_clk_priv - imx6 clock control module
+ *
+ * @base:	ccm base address
+ * @desc:	ccm descriptor
+ */
+struct imx6_clk_priv {
+	void *base;
+	const struct imx6_clk_desc *desc;
+};
+
+/**
+ * imx6_clk_probe - common imx6 clock probe
+ * @dev:	clock device
+ */
+int imx6_clk_probe(struct udevice *dev);
+
+extern struct clk_ops imx6_clk_ops;
+
 enum mxc_clock {
 	MXC_ARM_CLK = 0,
 	MXC_PER_CLK,
diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index a125c23a19..f81903dcf4 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -1,6 +1,6 @@
 config CLK_IMX
 	bool "Clock support for i.MX SoCs"
-	depends on CLK && ARCH_IMX8
+	depends on CLK && (ARCH_MX6 || ARCH_IMX8)
 	default y
 	help
 	  This enables support for common clock driver API on i.MX
@@ -8,6 +8,13 @@ config CLK_IMX
 
 if CLK_IMX
 
+config CLK_IMX6Q
+	bool "Clock driver for i.MX6QDL"
+	default MX6QDL
+	help
+	  This enables common clock driver support for platforms based
+	  on i.MX6 QDL SoC.
+
 config CLK_IMX8
 	bool "Clock support for i.MX8"
 	default ARCH_IMX8
diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index 5505ae52e2..a4cb2bbf3b 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -2,4 +2,9 @@
 #
 # SPDX-License-Identifier: GPL-2.0
 
+ifdef CONFIG_ARCH_MX6
+obj-$(CONFIG_CLK_IMX) += clk-imx6-common.o
+endif
+
+obj-$(CONFIG_CLK_IMX6Q) += clk-imx6q.o
 obj-$(CONFIG_CLK_IMX8) += clk-imx8.o
diff --git a/drivers/clk/imx/clk-imx6-common.c b/drivers/clk/imx/clk-imx6-common.c
new file mode 100644
index 0000000000..1d38f51f7e
--- /dev/null
+++ b/drivers/clk/imx/clk-imx6-common.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Amarula Solutions.
+ * Author: Jagan Teki <jagan@amarulasolutions.com>
+ */
+
+#include <common.h>
+#include <clk-uclass.h>
+#include <dm.h>
+#include <errno.h>
+#include <asm/io.h>
+#include <asm/arch/clock.h>
+#include <linux/log2.h>
+
+static const struct imx6_clk_gate *priv_to_gate(struct imx6_clk_priv *priv,
+						unsigned long id)
+{
+	return &priv->desc->gates[id];
+}
+
+static int imx6_set_gate(struct clk *clk, bool on)
+{
+	struct imx6_clk_priv *priv = dev_get_priv(clk->dev);
+	const struct imx6_clk_gate *gate = priv_to_gate(priv, clk->id);
+	u32 reg;
+
+	debug("%s: (CLK#%ld) off#0x%x, BIT(%d)\n", __func__,
+	      clk->id, gate->off, ilog2(gate->bit));
+
+	reg = readl(priv->base + gate->off);
+	if (on)
+		reg |= gate->bit;
+	else
+		reg &= ~gate->bit;
+
+	writel(reg, priv->base + gate->off);
+
+	return 0;
+}
+
+static int imx6_clk_enable(struct clk *clk)
+{
+	return imx6_set_gate(clk, true);
+}
+
+static int imx6_clk_disable(struct clk *clk)
+{
+	return imx6_set_gate(clk, false);
+}
+
+struct clk_ops imx6_clk_ops = {
+	.enable = imx6_clk_enable,
+	.disable = imx6_clk_disable,
+};
+
+int imx6_clk_probe(struct udevice *dev)
+{
+	struct imx6_clk_priv *priv = dev_get_priv(dev);
+
+	priv->base = dev_read_addr_ptr(dev);
+	if (!priv->base)
+		return -ENOMEM;
+
+	priv->desc = (const struct imx6_clk_desc *)dev_get_driver_data(dev);
+	if (!priv->desc)
+		return -EINVAL;
+
+	return 0;
+}
diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
new file mode 100644
index 0000000000..8ec713298d
--- /dev/null
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Amarula Solutions.
+ * Author: Jagan Teki <jagan@amarulasolutions.com>
+ */
+
+#include <common.h>
+#include <clk-uclass.h>
+#include <dm.h>
+#include <asm/arch/clock.h>
+#include <dt-bindings/clock/imx6qdl-clock.h>
+
+static const struct imx6_clk_gate imx6q_gates[] = {
+	[IMX6QDL_CLK_USDHC1]		= GATE(0x080, GENMASK(3, 2)),
+	[IMX6QDL_CLK_USDHC2]		= GATE(0x080, GENMASK(5, 4)),
+	[IMX6QDL_CLK_USDHC3]		= GATE(0x080, GENMASK(7, 6)),
+	[IMX6QDL_CLK_USDHC4]		= GATE(0x080, GENMASK(9, 8)),
+};
+
+static const struct imx6_clk_desc imx6q_clk_desc = {
+	.gates = imx6q_gates,
+};
+
+static const struct udevice_id clk_imx6q_ids[] = {
+	{
+	  .compatible = "fsl,imx6q-ccm",
+	  .data = (ulong)&imx6q_clk_desc
+	},
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(clk_imx6q) = {
+	.name		= "clk_imx6q",
+	.id		= UCLASS_CLK,
+	.of_match	= clk_imx6q_ids,
+	.priv_auto_alloc_size	= sizeof(struct imx6_clk_priv),
+	.ops		= &imx6_clk_ops,
+	.probe		= imx6_clk_probe,
+};
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v2 03/10] clk: imx: Add i.MX6UL clock driver

[Jagan Teki]

i.MX6 clock control module comprise of parent clocks, gates, multiplexers,
dividers, PODF, PLL, fixed rate and etc.

This patch add i.MX6UL USDHC clocks via gate clock which would eventually
handle enable/disable operations via imx6_clock_gate.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/clk/imx/Kconfig      |  7 +++++++
 drivers/clk/imx/Makefile     |  1 +
 drivers/clk/imx/clk-imx6ul.c | 37 ++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+)
 create mode 100644 drivers/clk/imx/clk-imx6ul.c

diff --git a/drivers/clk/imx/Kconfig b/drivers/clk/imx/Kconfig
index f81903dcf4..952cec8bff 100644
--- a/drivers/clk/imx/Kconfig
+++ b/drivers/clk/imx/Kconfig
@@ -15,6 +15,13 @@ config CLK_IMX6Q
 	  This enables common clock driver support for platforms based
 	  on i.MX6 QDL SoC.
 
+config CLK_IMX6UL
+	bool "Clock driver for i.MX6UL"
+	default MX6UL
+	help
+	 This enables common clock driver support for platforms based
+	 on i.MX6 UL SoC.
+
 config CLK_IMX8
 	bool "Clock support for i.MX8"
 	default ARCH_IMX8
diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile
index a4cb2bbf3b..999de1dd6f 100644
--- a/drivers/clk/imx/Makefile
+++ b/drivers/clk/imx/Makefile
@@ -7,4 +7,5 @@ obj-$(CONFIG_CLK_IMX) += clk-imx6-common.o
 endif
 
 obj-$(CONFIG_CLK_IMX6Q) += clk-imx6q.o
+obj-$(CONFIG_CLK_IMX6UL) += clk-imx6ul.o
 obj-$(CONFIG_CLK_IMX8) += clk-imx8.o
diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
new file mode 100644
index 0000000000..f5250e8b72
--- /dev/null
+++ b/drivers/clk/imx/clk-imx6ul.c
@@ -0,0 +1,37 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2019 Amarula Solutions.
+ * Author: Jagan Teki <jagan@amarulasolutions.com>
+ */
+
+#include <common.h>
+#include <clk-uclass.h>
+#include <dm.h>
+#include <asm/arch/clock.h>
+#include <dt-bindings/clock/imx6ul-clock.h>
+
+static const struct imx6_clk_gate imx6ul_gates[] = {
+	[IMX6UL_CLK_USDHC1]		= GATE(0x080, GENMASK(3, 2)),
+	[IMX6UL_CLK_USDHC2]		= GATE(0x080, GENMASK(5, 4)),
+};
+
+static const struct imx6_clk_desc imx6ul_clk_desc = {
+	.gates = imx6ul_gates,
+};
+
+static const struct udevice_id clk_imx6ul_ids[] = {
+	{
+	  .compatible = "fsl,imx6ul-ccm",
+	  .data = (ulong)&imx6ul_clk_desc
+	},
+	{ /* sentinel */ }
+};
+
+U_BOOT_DRIVER(clk_imx6ul) = {
+	.name		= "clk_imx6ul",
+	.id		= UCLASS_CLK,
+	.of_match	= clk_imx6ul_ids,
+	.priv_auto_alloc_size	= sizeof(struct imx6_clk_priv),
+	.ops		= &imx6_clk_ops,
+	.probe		= imx6_clk_probe,
+};
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v2 04/10] clk: Add clk_div_mask helper

[Jagan Teki]

Add clock helper to compute the clock divider mask when use invoke
with clock width.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 include/clk-uclass.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/clk-uclass.h b/include/clk-uclass.h
index e76d98e2f6..15ac8867a7 100644
--- a/include/clk-uclass.h
+++ b/include/clk-uclass.h
@@ -100,4 +100,6 @@ struct clk_ops {
 	int (*disable)(struct clk *clk);
 };
 
+#define clk_div_mask(width)		((1 << (width)) - 1)
+
 #endif
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v2 05/10] clk: imx: Add imx6q clock tree support

[Jagan Teki]

i.MX6 clock control module comprise of parent clocks, gates, multiplexers,
dividers, PODF, PLL, fixed rate and etc.

So, the U-Boot implementation of ccm has divided into gates and tree.

1) gate clocks are generic clock configuration of enable/disable bit management
   which can be handle via imx6_clock_gate.
2) tree clocks are handle via tree clock management where it link the clocks
   based on the parent clock which usually required to get and set the
   clock rates.

This patch add tree clock management for imx6q USDHC clocks, so the mmc driver
from imx6 can eventually use this so getting the USDHC clock rates.

Unlike Linux, U-Boot implementation may not require to maintain exact clock tree
due to various constrains and use cases. So here is how the clock tree differs
between them.

usdhc clock tree in Linux:
-------------------------
USDHC1 => USDHC1_PODF => USDHC1_SEL => PLL2_PFD2 => PLL2_BUS => PLL2_BYPASS => PLL2 => OSC

usdhc clock tree in U-Boot:
---------------------------
USDHC1 => USDHC1_PODF => USDHC1_SEL => PLL2_PFD2 => PLL2_BUS => OSC

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/include/asm/arch-mx6/clock.h |  65 ++++++++++++++++
 drivers/clk/imx/clk-imx6-common.c     | 103 ++++++++++++++++++++++++++
 drivers/clk/imx/clk-imx6q.c           |  70 +++++++++++++++++
 3 files changed, 238 insertions(+)

diff --git a/arch/arm/include/asm/arch-mx6/clock.h b/arch/arm/include/asm/arch-mx6/clock.h
index fa921a9f08..424231c691 100644
--- a/arch/arm/include/asm/arch-mx6/clock.h
+++ b/arch/arm/include/asm/arch-mx6/clock.h
@@ -21,6 +21,67 @@
 #define MXC_CLK32	32768
 #endif
 
+#define OSC_24M_ULL	24000000ULL
+
+enum imx6_clk_type {
+	IMX6_CLK_TYPE_SIMPLE	= 0,
+	IMX6_CLK_TYPE_FIXED,
+	IMX6_CLK_TYPE_DIV,
+	IMX6_CLK_TYPE_MUX,
+	IMX6_CLK_TYPE_PLL_PFD,
+	IMX6_CLK_TYPE_PLL_DIV,
+};
+
+/**
+ * struct imx6_clk_tree - imx6 ccm clock tree
+ *
+ * @parent:		parent clock tree
+ * @type:		clock type
+ * @off:		register offset of the specified clock
+ * @shift:		number of bits to shift the bitfield
+ * @width:		width of the bitfield
+ * @idx:		index of the specified clock
+ * @fixed_rate:		fixed clock rate
+ */
+struct imx6_clk_tree {
+	const unsigned long *parent;
+	enum imx6_clk_type type;
+	u16 off;
+
+	u8 shift;
+	u8 width;
+	u8 idx;
+	ulong fixed_rate;
+};
+
+#define TREE(_parent, _type, _off, _shift, _width, _idx, _fixed_rate) {	\
+	.parent = _parent,						\
+	.type = _type,							\
+	.off = _off,							\
+	.shift = _shift,						\
+	.width = _width,						\
+	.idx = _idx,							\
+	.fixed_rate = _fixed_rate,					\
+}
+
+#define SIMPLE(_parent)							\
+	TREE(_parent, IMX6_CLK_TYPE_SIMPLE, 0, 0, 0, 0, 0)
+
+#define FIXED(_fixed_rate)						\
+	TREE(NULL, IMX6_CLK_TYPE_FIXED, 0, 0, 0, 0, _fixed_rate)
+
+#define DIV(_parent, _off, _shift, _width)				\
+	TREE(_parent, IMX6_CLK_TYPE_DIV, _off, _shift, _width, 0, 0)
+
+#define MUX(_parent, _off, _shift, _width)				\
+	TREE(_parent, IMX6_CLK_TYPE_MUX, _off, _shift, _width, 0, 0)
+
+#define PLL_PFD(_parent, _off, _width, _idx)				\
+	TREE(_parent, IMX6_CLK_TYPE_PLL_PFD, _off, 0, _width, _idx, 0)
+
+#define PLL_DIV(_parent, _off, _shift, _width)				\
+	TREE(_parent, IMX6_CLK_TYPE_PLL_DIV, _off, _shift, _width, 0, 0)
+
 /**
  * struct imx6_clk_gate - imx6 ccm clock gate
  *
@@ -41,19 +102,23 @@ struct imx6_clk_gate {
  * struct imx6_clk_desc - imx6 clock control module descriptor
  *
  * @gates:	ccm clock gates
+ * @tree:	ccm clock tree
  */
 struct imx6_clk_desc {
 	const struct imx6_clk_gate *gates;
+	const struct imx6_clk_tree *tree;
 };
 
 /**
  * struct imx6_clk_priv - imx6 clock control module
  *
  * @base:	ccm base address
+ * @anatop:	anatop base address
  * @desc:	ccm descriptor
  */
 struct imx6_clk_priv {
 	void *base;
+	void *anatop;
 	const struct imx6_clk_desc *desc;
 };
 
diff --git a/drivers/clk/imx/clk-imx6-common.c b/drivers/clk/imx/clk-imx6-common.c
index 1d38f51f7e..d21facf2e5 100644
--- a/drivers/clk/imx/clk-imx6-common.c
+++ b/drivers/clk/imx/clk-imx6-common.c
@@ -6,18 +6,117 @@
 
 #include <common.h>
 #include <clk-uclass.h>
+#include <div64.h>
 #include <dm.h>
 #include <errno.h>
 #include <asm/io.h>
 #include <asm/arch/clock.h>
 #include <linux/log2.h>
 
+#include <dt-bindings/clock/imx6qdl-clock.h>
+
 static const struct imx6_clk_gate *priv_to_gate(struct imx6_clk_priv *priv,
 						unsigned long id)
 {
 	return &priv->desc->gates[id];
 }
 
+static const struct imx6_clk_tree *priv_to_tree(struct imx6_clk_priv *priv,
+						unsigned long id)
+{
+	return &priv->desc->tree[id];
+}
+
+static u8 get_bitfield(void *base, u8 shift, u8 width)
+{
+	return (readl(base) >> shift) & clk_div_mask(width);
+}
+
+static u8 get_mux_parent(const struct imx6_clk_tree *tree, void *base)
+{
+	u8 idx = get_bitfield(base + tree->off, tree->shift, tree->width);
+
+	return tree->parent[idx];
+}
+
+static ulong get_pll2_bus_rate(struct imx6_clk_priv *priv, unsigned long id,
+			       ulong parent_rate)
+{
+	const struct imx6_clk_tree *tree = priv_to_tree(priv, id);
+	u8 div;
+
+	div = get_bitfield(priv->anatop + tree->off, tree->shift, tree->width);
+	return (div == 1) ? parent_rate * 22 : parent_rate * 20;
+}
+
+static ulong get_pfd_rate(struct imx6_clk_priv *priv, unsigned long id,
+			  ulong parent_rate)
+{
+	const struct imx6_clk_tree *tree = priv_to_tree(priv, id);
+	u64 tmp = parent_rate;
+	u8 frac;
+
+	frac = get_bitfield(priv->anatop + tree->off,
+			    tree->idx * 8, tree->width);
+	tmp *= 18;
+	do_div(tmp, frac);
+
+	return tmp;
+}
+
+static ulong get_mux_rate(ulong parent_rate)
+{
+	/* derive clock from respective parent */
+	return parent_rate;
+}
+
+static ulong get_div_rate(struct imx6_clk_priv *priv, unsigned long id,
+			  ulong parent_rate)
+{
+	const struct imx6_clk_tree *tree = priv_to_tree(priv, id);
+	u8 podf;
+
+	podf = get_bitfield(priv->base + tree->off, tree->shift, tree->width);
+	return parent_rate / (podf + 1);
+}
+
+static ulong imx6_calc_clk_rate(struct imx6_clk_priv *priv, unsigned long id)
+{
+	const struct imx6_clk_tree *tree = priv_to_tree(priv, id);
+	ulong rate = 0;
+
+	switch (tree->type) {
+	case IMX6_CLK_TYPE_FIXED:
+		return tree->fixed_rate;
+	case IMX6_CLK_TYPE_PLL_DIV:
+		rate = imx6_calc_clk_rate(priv, tree->parent[0]);
+		return get_pll2_bus_rate(priv, id, rate);
+	case IMX6_CLK_TYPE_PLL_PFD:
+		rate = imx6_calc_clk_rate(priv, tree->parent[0]);
+		return get_pfd_rate(priv, id, rate);
+	case IMX6_CLK_TYPE_MUX:
+		rate = imx6_calc_clk_rate(priv,
+					  get_mux_parent(tree, priv->base));
+		return get_mux_rate(rate);
+	case IMX6_CLK_TYPE_DIV:
+		rate = imx6_calc_clk_rate(priv, tree->parent[0]);
+		return get_div_rate(priv, id, rate);
+	case IMX6_CLK_TYPE_SIMPLE:
+		return imx6_calc_clk_rate(priv, tree->parent[0]);
+	default:
+		printf("%s: (TYPE#%d) unhandled\n", __func__, tree->type);
+	}
+
+	return rate;
+}
+
+static ulong imx6_clk_get_rate(struct clk *clk)
+{
+	struct imx6_clk_priv *priv = dev_get_priv(clk->dev);
+
+	return imx6_calc_clk_rate(priv, clk->id);
+}
+
 static int imx6_set_gate(struct clk *clk, bool on)
 {
 	struct imx6_clk_priv *priv = dev_get_priv(clk->dev);
@@ -51,6 +150,7 @@ static int imx6_clk_disable(struct clk *clk)
 struct clk_ops imx6_clk_ops = {
 	.enable = imx6_clk_enable,
 	.disable = imx6_clk_disable,
+	.get_rate = imx6_clk_get_rate,
 };
 
 int imx6_clk_probe(struct udevice *dev)
@@ -61,6 +161,9 @@ int imx6_clk_probe(struct udevice *dev)
 	if (!priv->base)
 		return -ENOMEM;
 
+	/* FIXME get the anatop base via OF_LIVE */
+	priv->anatop = priv->base + 0x4000;
+
 	priv->desc = (const struct imx6_clk_desc *)dev_get_driver_data(dev);
 	if (!priv->desc)
 		return -EINVAL;
diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
index 8ec713298d..a46d2f6f88 100644
--- a/drivers/clk/imx/clk-imx6q.c
+++ b/drivers/clk/imx/clk-imx6q.c
@@ -10,6 +10,75 @@
 #include <asm/arch/clock.h>
 #include <dt-bindings/clock/imx6qdl-clock.h>
 
+static const unsigned long pll2_bus[] = {
+	IMX6QDL_CLK_OSC,
+};
+
+static const unsigned long pfd_352m[] = {
+	IMX6QDL_CLK_PLL2_BUS,
+};
+
+static const unsigned long usdhc_sel[] = {
+	IMX6QDL_CLK_PLL2_PFD2_396M,
+	IMX6QDL_CLK_PLL2_PFD0_352M,
+};
+
+static const unsigned long usdhc1_podf[] = {
+	IMX6QDL_CLK_USDHC1_SEL,
+};
+
+static const unsigned long usdhc2_podf[] = {
+	IMX6QDL_CLK_USDHC2_SEL,
+};
+
+static const unsigned long usdhc3_podf[] = {
+	IMX6QDL_CLK_USDHC3_SEL,
+};
+
+static const unsigned long usdhc4_podf[] = {
+	IMX6QDL_CLK_USDHC4_SEL,
+};
+
+static const unsigned long usdhc1[] = {
+	IMX6QDL_CLK_USDHC1_PODF,
+};
+
+static const unsigned long usdhc2[] = {
+	IMX6QDL_CLK_USDHC2_PODF,
+};
+
+static const unsigned long usdhc3[] = {
+	IMX6QDL_CLK_USDHC3_PODF,
+};
+
+static const unsigned long usdhc4[] = {
+	IMX6QDL_CLK_USDHC4_PODF,
+};
+
+static const struct imx6_clk_tree imx6q_tree[] = {
+	[IMX6QDL_CLK_OSC]		= FIXED(OSC_24M_ULL),
+
+	[IMX6QDL_CLK_PLL2_BUS]		= PLL_DIV(pll2_bus, 0x30, 13, 1),
+
+	[IMX6QDL_CLK_PLL2_PFD0_352M]	= PLL_PFD(pfd_352m, 0x100, 6, 0),
+	[IMX6QDL_CLK_PLL2_PFD2_396M]	= PLL_PFD(pfd_352m, 0x100, 6, 2),
+
+	[IMX6QDL_CLK_USDHC1_SEL]	= MUX(usdhc_sel, 0x01c, 16, 1),
+	[IMX6QDL_CLK_USDHC2_SEL]	= MUX(usdhc_sel, 0x01c, 17, 1),
+	[IMX6QDL_CLK_USDHC3_SEL]	= MUX(usdhc_sel, 0x01c, 18, 1),
+	[IMX6QDL_CLK_USDHC4_SEL]	= MUX(usdhc_sel, 0x01c, 19, 1),
+
+	[IMX6QDL_CLK_USDHC1_PODF]	= DIV(usdhc1_podf, 0x024, 11, 3),
+	[IMX6QDL_CLK_USDHC2_PODF]	= DIV(usdhc2_podf, 0x024, 16, 3),
+	[IMX6QDL_CLK_USDHC3_PODF]	= DIV(usdhc3_podf, 0x024, 19, 3),
+	[IMX6QDL_CLK_USDHC4_PODF]	= DIV(usdhc4_podf, 0x024, 22, 3),
+
+	[IMX6QDL_CLK_USDHC1]		= SIMPLE(usdhc1),
+	[IMX6QDL_CLK_USDHC2]		= SIMPLE(usdhc2),
+	[IMX6QDL_CLK_USDHC3]		= SIMPLE(usdhc3),
+	[IMX6QDL_CLK_USDHC4]		= SIMPLE(usdhc4),
+};
+
 static const struct imx6_clk_gate imx6q_gates[] = {
 	[IMX6QDL_CLK_USDHC1]		= GATE(0x080, GENMASK(3, 2)),
 	[IMX6QDL_CLK_USDHC2]		= GATE(0x080, GENMASK(5, 4)),
@@ -19,6 +88,7 @@ static const struct imx6_clk_gate imx6q_gates[] = {
 
 static const struct imx6_clk_desc imx6q_clk_desc = {
 	.gates = imx6q_gates,
+	.tree = imx6q_tree,
 };
 
 static const struct udevice_id clk_imx6q_ids[] = {
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v2 05/10] clk: imx: Add imx6q clock tree support

[Lukasz Majewski]

Hi Jagan,

> i.MX6 clock control module comprise of parent clocks, gates,
> multiplexers, dividers, PODF, PLL, fixed rate and etc.
> 
> So, the U-Boot implementation of ccm has divided into gates and tree.
> 
> 1) gate clocks are generic clock configuration of enable/disable bit
> management which can be handle via imx6_clock_gate.
> 2) tree clocks are handle via tree clock management where it link the
> clocks based on the parent clock which usually required to get and
> set the clock rates.

If I understood the patch series:

The clock hierarchy has been flattened to only two kinds of elements:
gates and "tree".

I'm not sure if we will not find this not enough in the future when one
wants to implement/use video/USB/other clocks.

For example only pllv3 type has several variants:

+enum imx_pllv3_type {
+	IMX_PLLV3_GENERIC,
+	IMX_PLLV3_SYS,
+	IMX_PLLV3_USB,
+	IMX_PLLV3_USB_VF610,
+	IMX_PLLV3_AV,
+	IMX_PLLV3_ENET,
+	IMX_PLLV3_ENET_IMX7,
+	IMX_PLLV3_SYS_VF610,
+	IMX_PLLV3_DDR_IMX7,

USB, USB_VF610 (vybrid), Generic, ENET (which is not implemented by
this series).

With porting Linux approach I'm pretty sure that I can add support for
vybrid in the future when e.g. gadget is converted to DM.

Is this also so predictable with your approach ?

> 
> This patch add tree clock management for imx6q USDHC clocks, so the
> mmc driver from imx6 can eventually use this so getting the USDHC
> clock rates.

But for USDHC you only need gate and read the clock value (which is not
so deepen hidden in the hierarchy).

> 
> Unlike Linux, U-Boot implementation may not require to maintain exact
> clock tree due to various constrains and use cases.

Another use case - ECSPI / ENET.

> So here is how
> the clock tree differs between them.
> 
> usdhc clock tree in Linux:
> -------------------------
> USDHC1 => USDHC1_PODF => USDHC1_SEL => PLL2_PFD2 => PLL2_BUS =>
> PLL2_BYPASS => PLL2 => OSC
> 
> usdhc clock tree in U-Boot:
> ---------------------------
> USDHC1 => USDHC1_PODF => USDHC1_SEL => PLL2_PFD2 => PLL2_BUS => OSC
> 
> Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
> ---
>  arch/arm/include/asm/arch-mx6/clock.h |  65 ++++++++++++++++
>  drivers/clk/imx/clk-imx6-common.c     | 103
> ++++++++++++++++++++++++++ drivers/clk/imx/clk-imx6q.c           |
> 70 +++++++++++++++++ 3 files changed, 238 insertions(+)
> 
> diff --git a/arch/arm/include/asm/arch-mx6/clock.h
> b/arch/arm/include/asm/arch-mx6/clock.h index fa921a9f08..424231c691
> 100644 --- a/arch/arm/include/asm/arch-mx6/clock.h
> +++ b/arch/arm/include/asm/arch-mx6/clock.h
> @@ -21,6 +21,67 @@
>  #define MXC_CLK32	32768
>  #endif
>  
> +#define OSC_24M_ULL	24000000ULL
> +
> +enum imx6_clk_type {
> +	IMX6_CLK_TYPE_SIMPLE	= 0,
> +	IMX6_CLK_TYPE_FIXED,
> +	IMX6_CLK_TYPE_DIV,
> +	IMX6_CLK_TYPE_MUX,
> +	IMX6_CLK_TYPE_PLL_PFD,
> +	IMX6_CLK_TYPE_PLL_DIV,
> +};
> +
> +/**
> + * struct imx6_clk_tree - imx6 ccm clock tree
> + *
> + * @parent:		parent clock tree
> + * @type:		clock type
> + * @off:		register offset of the specified clock
> + * @shift:		number of bits to shift the bitfield
> + * @width:		width of the bitfield
> + * @idx:		index of the specified clock
> + * @fixed_rate:		fixed clock rate
> + */
> +struct imx6_clk_tree {
> +	const unsigned long *parent;
> +	enum imx6_clk_type type;
> +	u16 off;
> +
> +	u8 shift;
> +	u8 width;
> +	u8 idx;
> +	ulong fixed_rate;
> +};
> +
> +#define TREE(_parent, _type, _off, _shift, _width, _idx,
> _fixed_rate) {	\
> +	.parent =
> _parent,						\
> +	.type =
> _type,							\
> +	.off =
> _off,							\
> +	.shift =
> _shift,						\
> +	.width =
> _width,						\
> +	.idx =
> _idx,							\
> +	.fixed_rate =
> _fixed_rate,					\ +}
> +
> +#define
> SIMPLE(_parent)
> \
> +	TREE(_parent, IMX6_CLK_TYPE_SIMPLE, 0, 0, 0, 0, 0)
> +
> +#define
> FIXED(_fixed_rate)						\
> +	TREE(NULL, IMX6_CLK_TYPE_FIXED, 0, 0, 0, 0, _fixed_rate)
> +
> +#define DIV(_parent, _off, _shift,
> _width)				\
> +	TREE(_parent, IMX6_CLK_TYPE_DIV, _off, _shift, _width, 0, 0)
> +
> +#define MUX(_parent, _off, _shift,
> _width)				\
> +	TREE(_parent, IMX6_CLK_TYPE_MUX, _off, _shift, _width, 0, 0)
> +
> +#define PLL_PFD(_parent, _off, _width,
> _idx)				\
> +	TREE(_parent, IMX6_CLK_TYPE_PLL_PFD, _off, 0, _width, _idx,
> 0) +
> +#define PLL_DIV(_parent, _off, _shift,
> _width)				\
> +	TREE(_parent, IMX6_CLK_TYPE_PLL_DIV, _off, _shift, _width,
> 0, 0) +
>  /**
>   * struct imx6_clk_gate - imx6 ccm clock gate
>   *
> @@ -41,19 +102,23 @@ struct imx6_clk_gate {
>   * struct imx6_clk_desc - imx6 clock control module descriptor
>   *
>   * @gates:	ccm clock gates
> + * @tree:	ccm clock tree
>   */
>  struct imx6_clk_desc {
>  	const struct imx6_clk_gate *gates;
> +	const struct imx6_clk_tree *tree;
>  };
>  
>  /**
>   * struct imx6_clk_priv - imx6 clock control module
>   *
>   * @base:	ccm base address
> + * @anatop:	anatop base address
>   * @desc:	ccm descriptor
>   */
>  struct imx6_clk_priv {
>  	void *base;
> +	void *anatop;
>  	const struct imx6_clk_desc *desc;
>  };
>  
> diff --git a/drivers/clk/imx/clk-imx6-common.c
> b/drivers/clk/imx/clk-imx6-common.c index 1d38f51f7e..d21facf2e5
> 100644 --- a/drivers/clk/imx/clk-imx6-common.c
> +++ b/drivers/clk/imx/clk-imx6-common.c
> @@ -6,18 +6,117 @@
>  
>  #include <common.h>
>  #include <clk-uclass.h>
> +#include <div64.h>
>  #include <dm.h>
>  #include <errno.h>
>  #include <asm/io.h>
>  #include <asm/arch/clock.h>
>  #include <linux/log2.h>
>  
> +#include <dt-bindings/clock/imx6qdl-clock.h>
> +
>  static const struct imx6_clk_gate *priv_to_gate(struct imx6_clk_priv
> *priv, unsigned long id)
>  {
>  	return &priv->desc->gates[id];
>  }
>  
> +static const struct imx6_clk_tree *priv_to_tree(struct imx6_clk_priv
> *priv,
> +						unsigned long id)
> +{
> +	return &priv->desc->tree[id];
> +}
> +
> +static u8 get_bitfield(void *base, u8 shift, u8 width)
> +{
> +	return (readl(base) >> shift) & clk_div_mask(width);
> +}
> +
> +static u8 get_mux_parent(const struct imx6_clk_tree *tree, void
> *base) +{
> +	u8 idx = get_bitfield(base + tree->off, tree->shift,
> tree->width); +
> +	return tree->parent[idx];
> +}
> +
> +static ulong get_pll2_bus_rate(struct imx6_clk_priv *priv, unsigned
> long id,
> +			       ulong parent_rate)
> +{
> +	const struct imx6_clk_tree *tree = priv_to_tree(priv, id);
> +	u8 div;
> +
> +	div = get_bitfield(priv->anatop + tree->off, tree->shift,
> tree->width);
> +	return (div == 1) ? parent_rate * 22 : parent_rate * 20;
> +}
> +
> +static ulong get_pfd_rate(struct imx6_clk_priv *priv, unsigned long
> id,
> +			  ulong parent_rate)
> +{
> +	const struct imx6_clk_tree *tree = priv_to_tree(priv, id);
> +	u64 tmp = parent_rate;
> +	u8 frac;
> +
> +	frac = get_bitfield(priv->anatop + tree->off,
> +			    tree->idx * 8, tree->width);
> +	tmp *= 18;
> +	do_div(tmp, frac);
> +
> +	return tmp;
> +}
> +
> +static ulong get_mux_rate(ulong parent_rate)
> +{
> +	/* derive clock from respective parent */
> +	return parent_rate;
> +}
> +
> +static ulong get_div_rate(struct imx6_clk_priv *priv, unsigned long
> id,
> +			  ulong parent_rate)
> +{
> +	const struct imx6_clk_tree *tree = priv_to_tree(priv, id);
> +	u8 podf;
> +
> +	podf = get_bitfield(priv->base + tree->off, tree->shift,
> tree->width);
> +	return parent_rate / (podf + 1);
> +}
> +
> +static ulong imx6_calc_clk_rate(struct imx6_clk_priv *priv, unsigned
> long id) +{
> +	const struct imx6_clk_tree *tree = priv_to_tree(priv, id);
> +	ulong rate = 0;
> +
> +	switch (tree->type) {
> +	case IMX6_CLK_TYPE_FIXED:
> +		return tree->fixed_rate;
> +	case IMX6_CLK_TYPE_PLL_DIV:
> +		rate = imx6_calc_clk_rate(priv, tree->parent[0]);
> +		return get_pll2_bus_rate(priv, id, rate);
> +	case IMX6_CLK_TYPE_PLL_PFD:
> +		rate = imx6_calc_clk_rate(priv, tree->parent[0]);
> +		return get_pfd_rate(priv, id, rate);
> +	case IMX6_CLK_TYPE_MUX:
> +		rate = imx6_calc_clk_rate(priv,
> +					  get_mux_parent(tree,
> priv->base));
> +		return get_mux_rate(rate);
> +	case IMX6_CLK_TYPE_DIV:
> +		rate = imx6_calc_clk_rate(priv, tree->parent[0]);
> +		return get_div_rate(priv, id, rate);
> +	case IMX6_CLK_TYPE_SIMPLE:
> +		return imx6_calc_clk_rate(priv, tree->parent[0]);
> +	default:
> +		printf("%s: (TYPE#%d) unhandled\n", __func__,
> tree->type);
> +	}
> +
> +	return rate;
> +}
> +
> +static ulong imx6_clk_get_rate(struct clk *clk)
> +{
> +	struct imx6_clk_priv *priv = dev_get_priv(clk->dev);
> +
> +	return imx6_calc_clk_rate(priv, clk->id);
> +}
> +
>  static int imx6_set_gate(struct clk *clk, bool on)
>  {
>  	struct imx6_clk_priv *priv = dev_get_priv(clk->dev);
> @@ -51,6 +150,7 @@ static int imx6_clk_disable(struct clk *clk)
>  struct clk_ops imx6_clk_ops = {
>  	.enable = imx6_clk_enable,
>  	.disable = imx6_clk_disable,
> +	.get_rate = imx6_clk_get_rate,
>  };
>  
>  int imx6_clk_probe(struct udevice *dev)
> @@ -61,6 +161,9 @@ int imx6_clk_probe(struct udevice *dev)
>  	if (!priv->base)
>  		return -ENOMEM;
>  
> +	/* FIXME get the anatop base via OF_LIVE */
> +	priv->anatop = priv->base + 0x4000;
> +
>  	priv->desc = (const struct imx6_clk_desc
> *)dev_get_driver_data(dev); if (!priv->desc)
>  		return -EINVAL;
> diff --git a/drivers/clk/imx/clk-imx6q.c b/drivers/clk/imx/clk-imx6q.c
> index 8ec713298d..a46d2f6f88 100644
> --- a/drivers/clk/imx/clk-imx6q.c
> +++ b/drivers/clk/imx/clk-imx6q.c
> @@ -10,6 +10,75 @@
>  #include <asm/arch/clock.h>
>  #include <dt-bindings/clock/imx6qdl-clock.h>
>  
> +static const unsigned long pll2_bus[] = {
> +	IMX6QDL_CLK_OSC,
> +};
> +
> +static const unsigned long pfd_352m[] = {
> +	IMX6QDL_CLK_PLL2_BUS,
> +};
> +
> +static const unsigned long usdhc_sel[] = {
> +	IMX6QDL_CLK_PLL2_PFD2_396M,
> +	IMX6QDL_CLK_PLL2_PFD0_352M,
> +};
> +
> +static const unsigned long usdhc1_podf[] = {
> +	IMX6QDL_CLK_USDHC1_SEL,
> +};
> +
> +static const unsigned long usdhc2_podf[] = {
> +	IMX6QDL_CLK_USDHC2_SEL,
> +};
> +
> +static const unsigned long usdhc3_podf[] = {
> +	IMX6QDL_CLK_USDHC3_SEL,
> +};
> +
> +static const unsigned long usdhc4_podf[] = {
> +	IMX6QDL_CLK_USDHC4_SEL,
> +};
> +
> +static const unsigned long usdhc1[] = {
> +	IMX6QDL_CLK_USDHC1_PODF,
> +};
> +
> +static const unsigned long usdhc2[] = {
> +	IMX6QDL_CLK_USDHC2_PODF,
> +};
> +
> +static const unsigned long usdhc3[] = {
> +	IMX6QDL_CLK_USDHC3_PODF,
> +};
> +
> +static const unsigned long usdhc4[] = {
> +	IMX6QDL_CLK_USDHC4_PODF,
> +};
> +
> +static const struct imx6_clk_tree imx6q_tree[] = {
> +	[IMX6QDL_CLK_OSC]		= FIXED(OSC_24M_ULL),
> +
> +	[IMX6QDL_CLK_PLL2_BUS]		= PLL_DIV(pll2_bus,
> 0x30, 13, 1), +
> +	[IMX6QDL_CLK_PLL2_PFD0_352M]	= PLL_PFD(pfd_352m,
> 0x100, 6, 0),
> +	[IMX6QDL_CLK_PLL2_PFD2_396M]	= PLL_PFD(pfd_352m,
> 0x100, 6, 2), +
> +	[IMX6QDL_CLK_USDHC1_SEL]	= MUX(usdhc_sel, 0x01c, 16,
> 1),
> +	[IMX6QDL_CLK_USDHC2_SEL]	= MUX(usdhc_sel, 0x01c, 17,
> 1),
> +	[IMX6QDL_CLK_USDHC3_SEL]	= MUX(usdhc_sel, 0x01c, 18,
> 1),
> +	[IMX6QDL_CLK_USDHC4_SEL]	= MUX(usdhc_sel, 0x01c, 19,
> 1), +
> +	[IMX6QDL_CLK_USDHC1_PODF]	= DIV(usdhc1_podf, 0x024,
> 11, 3),
> +	[IMX6QDL_CLK_USDHC2_PODF]	= DIV(usdhc2_podf, 0x024,
> 16, 3),
> +	[IMX6QDL_CLK_USDHC3_PODF]	= DIV(usdhc3_podf, 0x024,
> 19, 3),
> +	[IMX6QDL_CLK_USDHC4_PODF]	= DIV(usdhc4_podf, 0x024,
> 22, 3), +
> +	[IMX6QDL_CLK_USDHC1]		= SIMPLE(usdhc1),
> +	[IMX6QDL_CLK_USDHC2]		= SIMPLE(usdhc2),
> +	[IMX6QDL_CLK_USDHC3]		= SIMPLE(usdhc3),
> +	[IMX6QDL_CLK_USDHC4]		= SIMPLE(usdhc4),
> +};
> +
>  static const struct imx6_clk_gate imx6q_gates[] = {
>  	[IMX6QDL_CLK_USDHC1]		= GATE(0x080, GENMASK(3,
> 2)), [IMX6QDL_CLK_USDHC2]		= GATE(0x080, GENMASK(5, 4)),
> @@ -19,6 +88,7 @@ static const struct imx6_clk_gate imx6q_gates[] = {
>  
>  static const struct imx6_clk_desc imx6q_clk_desc = {
>  	.gates = imx6q_gates,
> +	.tree = imx6q_tree,
>  };
>  
>  static const struct udevice_id clk_imx6q_ids[] = {




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190419/fbfc5259/attachment.sig>

[U-Boot] [PATCH v2 06/10] clk: imx6: Add imx6ul clock tree support

[Jagan Teki]

i.MX6 clock control module comprise of parent clocks, gates, multiplexers,
dividers, PODF, PLL, fixed rate and etc.

So, the U-Boot implementation of ccm has divided into gates and tree.

1) gate clocks are generic clock configuration of enable/disable bit management
   which can be handle via imx6_clock_gate.
2) tree clocks are handle via tree clock management where it link the clocks
   based on the parent clock which usually required to get and set the
   clock rates.

This patch add tree clock management for imx6ul USDHC clocks, so the mmc driver
from imx6 can eventually use this so getting the USDHC clock rates.

Unlike Linux, U-Boot implementation may not require to maintain exact clock tree
due to various constrains and use cases. So here is how the clock tree differs
between them.

usdhc clock tree in Linux:
-------------------------
USDHC1 => USDHC1_PODF => USDHC1_SEL => PLL2_PFD2 => PLL2_BUS => PLL2_BYPASS => PLL2 => OSC

usdhc clock tree in U-Boot:
---------------------------
USDHC1 => USDHC1_PODF => USDHC1_SEL => PLL2_PFD2 => PLL2_BUS => OSC

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 drivers/clk/imx/clk-imx6ul.c | 48 ++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/drivers/clk/imx/clk-imx6ul.c b/drivers/clk/imx/clk-imx6ul.c
index f5250e8b72..8528176eec 100644
--- a/drivers/clk/imx/clk-imx6ul.c
+++ b/drivers/clk/imx/clk-imx6ul.c
@@ -10,6 +10,53 @@
 #include <asm/arch/clock.h>
 #include <dt-bindings/clock/imx6ul-clock.h>
 
+static const unsigned long pll2_bus[] = {
+	IMX6UL_CLK_OSC,
+};
+
+static const unsigned long pfd_352m[] = {
+	IMX6UL_CLK_PLL2_BUS,
+};
+
+static const unsigned long usdhc_sel[] = {
+	IMX6UL_CLK_PLL2_PFD2,
+	IMX6UL_CLK_PLL2_PFD0,
+};
+
+static const unsigned long usdhc1_podf[] = {
+	IMX6UL_CLK_USDHC1_SEL,
+};
+
+static const unsigned long usdhc2_podf[] = {
+	IMX6UL_CLK_USDHC2_SEL,
+};
+
+static const unsigned long usdhc1[] = {
+	IMX6UL_CLK_USDHC1_PODF,
+};
+
+static const unsigned long usdhc2[] = {
+	IMX6UL_CLK_USDHC2_PODF,
+};
+
+static const struct imx6_clk_tree imx6ul_tree[] = {
+	[IMX6UL_CLK_OSC]		= FIXED(OSC_24M_ULL),
+
+	[IMX6UL_CLK_PLL2_BUS]		= PLL_DIV(pll2_bus, 0x30, 13, 1),
+
+	[IMX6UL_CLK_PLL2_PFD0]		= PLL_PFD(pfd_352m, 0x100, 6, 0),
+	[IMX6UL_CLK_PLL2_PFD2]		= PLL_PFD(pfd_352m, 0x100, 6, 2),
+
+	[IMX6UL_CLK_USDHC2_SEL]		= MUX(usdhc_sel, 0x01c, 17, 1),
+	[IMX6UL_CLK_USDHC1_SEL]		= MUX(usdhc_sel, 0x01c, 16, 1),
+
+	[IMX6UL_CLK_USDHC2_PODF]	= DIV(usdhc2_podf, 0x024, 16, 3),
+	[IMX6UL_CLK_USDHC1_PODF]	= DIV(usdhc1_podf, 0x024, 11, 3),
+
+	[IMX6UL_CLK_USDHC2]		= SIMPLE(usdhc2),
+	[IMX6UL_CLK_USDHC1]		= SIMPLE(usdhc1),
+};
+
 static const struct imx6_clk_gate imx6ul_gates[] = {
 	[IMX6UL_CLK_USDHC1]		= GATE(0x080, GENMASK(3, 2)),
 	[IMX6UL_CLK_USDHC2]		= GATE(0x080, GENMASK(5, 4)),
@@ -17,6 +64,7 @@ static const struct imx6_clk_gate imx6ul_gates[] = {
 
 static const struct imx6_clk_desc imx6ul_clk_desc = {
 	.gates = imx6ul_gates,
+	.tree = imx6ul_tree,
 };
 
 static const struct udevice_id clk_imx6ul_ids[] = {
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v2 07/10] ARM: dts: i.MX6QDL: Add u-boot, dm-spl for clks

[Jagan Teki]

Add clks node available for SPL in imx6qdl SoC.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/dts/imx6qdl-u-boot.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/dts/imx6qdl-u-boot.dtsi b/arch/arm/dts/imx6qdl-u-boot.dtsi
index 0aa29e38b8..912cb4281c 100644
--- a/arch/arm/dts/imx6qdl-u-boot.dtsi
+++ b/arch/arm/dts/imx6qdl-u-boot.dtsi
@@ -24,6 +24,10 @@
 	u-boot,dm-spl;
 };
 
+&clks {
+	u-boot,dm-spl;
+};
+
 &iomuxc {
 	u-boot,dm-spl;
 };
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v2 08/10] ARM: dts: i.MX6UL: Add u-boot, dm-spl for clks

[Jagan Teki]

Add clks node available for SPL in imx6ul SoC.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/dts/imx6ul-u-boot.dtsi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/dts/imx6ul-u-boot.dtsi b/arch/arm/dts/imx6ul-u-boot.dtsi
index eb190cf8c8..b25f77cb44 100644
--- a/arch/arm/dts/imx6ul-u-boot.dtsi
+++ b/arch/arm/dts/imx6ul-u-boot.dtsi
@@ -21,6 +21,10 @@
 	u-boot,dm-spl;
 };
 
+&clks {
+	u-boot,dm-spl;
+};
+
 &iomuxc {
 	u-boot,dm-spl;
 };
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v2 09/10] configs: icore_mipi: Enable CLK

[Jagan Teki]

Now, clock driver available for i.MX6Q so enable the CLK
in SPL and U-Boot proper for Engicam i.CoreM6 MIPI boards.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 configs/imx6qdl_icore_mipi_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configs/imx6qdl_icore_mipi_defconfig b/configs/imx6qdl_icore_mipi_defconfig
index 7a0e5169b6..bb5e0d3abd 100644
--- a/configs/imx6qdl_icore_mipi_defconfig
+++ b/configs/imx6qdl_icore_mipi_defconfig
@@ -8,6 +8,7 @@ CONFIG_SYS_MALLOC_F_LEN=0x2000
 CONFIG_TARGET_MX6Q_ENGICAM=y
 CONFIG_SPL_SERIAL_SUPPORT=y
 CONFIG_SPL=y
+CONFIG_SPL_CLK=y
 CONFIG_DEBUG_UART_BASE=0x021f0000
 CONFIG_DEBUG_UART_CLOCK=24000000
 CONFIG_SPL_LIBDISK_SUPPORT=y
@@ -44,6 +45,7 @@ CONFIG_DEFAULT_DEVICE_TREE="imx6q-icore-mipi"
 CONFIG_OF_LIST="imx6q-icore-mipi imx6dl-icore-mipi"
 CONFIG_ENV_IS_IN_MMC=y
 CONFIG_SYS_I2C_MXC=y
+CONFIG_CLK=y
 CONFIG_FSL_ESDHC=y
 CONFIG_PHYLIB=y
 CONFIG_PHY_SMSC=y
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v2 10/10] ARM: imx6: Enable CLK for Engicam i.MX6UL boards

[Jagan Teki]

Now, clock driver available for i.MX6UL so enable the CLK
in SPL and U-Boot proper for Engicam i.MX6UL boards.

Signed-off-by: Jagan Teki <jagan@amarulasolutions.com>
---
 arch/arm/mach-imx/mx6/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/mach-imx/mx6/Kconfig b/arch/arm/mach-imx/mx6/Kconfig
index e782859b1e..851cabc6fa 100644
--- a/arch/arm/mach-imx/mx6/Kconfig
+++ b/arch/arm/mach-imx/mx6/Kconfig
@@ -361,6 +361,7 @@ config TARGET_MX6UL_14X14_EVK
 config TARGET_MX6UL_ENGICAM
 	bool "Support Engicam GEAM6UL/Is.IoT"
 	select BOARD_LATE_INIT
+	select CLK
 	select DM
 	select DM_ETH
 	select DM_GPIO
@@ -369,6 +370,7 @@ config TARGET_MX6UL_ENGICAM
 	select DM_THERMAL
 	select MX6UL
 	select OF_CONTROL
+	select SPL_CLK if SPL
 	select SPL_DM if SPL
 	select SPL_OF_CONTROL if SPL
 	select SPL_PINCTRL if SPL
-- 
2.18.0.321.gffc6fa0e3

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Lukasz Majewski]

On Tue,  2 Apr 2019 16:58:33 +0530
Jagan Teki <jagan@amarulasolutions.com> wrote:

> This is revised version of previous i.MX6 clock management [1]. 
> 
> The main difference between previous version is
> - Group the i.MX6 ccm clocks into gates and tree instead of handling
> the clocks in simple way using case statement.
> - use gate clocks for enable/disable management.
> - use tree clocks for get/set rate or parent traverse management.
> - parent clock handling via clock type.
> - traverse the parent clock using recursive functionlaity.
> 
> The main motive behind this tree framework is to make the clock tree 
> management simple and useful for U-Boot requirements instead of
> garbing Linux clock management code.
> 
> We are trying to manage the Allwinner clocks with similar kind, so
> having this would really help i.MX6 as well.
> 
> Added simple names for clock macros, but will update it in future
> version.
> 
> I have skipped ENET clocks from previous series, will add it in future
> patches.
> 
> Changes for v2:
> - changed framework patches.
> - add support for imx6qdl and imx6ul boards
> - add clock gates, tree.
> 
> [1] https://patchwork.ozlabs.org/cover/950964/
> 
> Any inputs?

Hmm.... It looks like we are doing some development in parallel.

Please look into following commit [1]:
https://patchwork.ozlabs.org/patch/1034051/

It ports from Linux 5.0 the CCF framework for iMX6Q, which IMHO in the
long term is a better approach.
The code is kept simple and resembles the code from Barebox.

Please correct me if I'm wrong, but the code from your work is not
modeling muxes, gates and other components from Linux CCF.


Unfortunately for [1] - I did not have time recently to finish it ...
(address Simon's comments about uclass).


> Jagan.
> 
> Jagan Teki (10):
>   clk: imx: Kconfig: Make CONFIG_CLK available for selection
>   clk: imx: Add i.MX6Q clock driver
>   clk: imx: Add i.MX6UL clock driver
>   clk: Add clk_div_mask helper
>   clk: imx: Add imx6q clock tree support
>   clk: imx6: Add imx6ul clock tree support
>   ARM: dts: i.MX6QDL: Add u-boot,dm-spl for clks
>   ARM: dts: i.MX6UL: Add u-boot,dm-spl for clks
>   configs: icore_mipi: Enable CLK
>   ARM: imx6: Enable CLK for Engicam i.MX6UL boards
> 
>  arch/arm/dts/imx6qdl-u-boot.dtsi      |   4 +
>  arch/arm/dts/imx6ul-u-boot.dtsi       |   4 +
>  arch/arm/include/asm/arch-mx6/clock.h | 109 ++++++++++++++++
>  arch/arm/mach-imx/mx6/Kconfig         |   2 +
>  configs/imx6qdl_icore_mipi_defconfig  |   2 +
>  configs/imx8qxp_mek_defconfig         |   2 +-
>  drivers/clk/imx/Kconfig               |  29 ++++-
>  drivers/clk/imx/Makefile              |   6 +
>  drivers/clk/imx/clk-imx6-common.c     | 172
> ++++++++++++++++++++++++++ drivers/clk/imx/clk-imx6q.c           |
> 109 ++++++++++++++++ drivers/clk/imx/clk-imx6ul.c          |  85
> +++++++++++++ include/clk-uclass.h                  |   2 +
>  12 files changed, 523 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/clk/imx/clk-imx6-common.c
>  create mode 100644 drivers/clk/imx/clk-imx6q.c
>  create mode 100644 drivers/clk/imx/clk-imx6ul.c
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190404/3272c64e/attachment.sig>

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Jagan Teki]

On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> On Tue,  2 Apr 2019 16:58:33 +0530
> Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> > This is revised version of previous i.MX6 clock management [1].
> >
> > The main difference between previous version is
> > - Group the i.MX6 ccm clocks into gates and tree instead of handling
> > the clocks in simple way using case statement.
> > - use gate clocks for enable/disable management.
> > - use tree clocks for get/set rate or parent traverse management.
> > - parent clock handling via clock type.
> > - traverse the parent clock using recursive functionlaity.
> >
> > The main motive behind this tree framework is to make the clock tree
> > management simple and useful for U-Boot requirements instead of
> > garbing Linux clock management code.
> >
> > We are trying to manage the Allwinner clocks with similar kind, so
> > having this would really help i.MX6 as well.
> >
> > Added simple names for clock macros, but will update it in future
> > version.
> >
> > I have skipped ENET clocks from previous series, will add it in future
> > patches.
> >
> > Changes for v2:
> > - changed framework patches.
> > - add support for imx6qdl and imx6ul boards
> > - add clock gates, tree.
> >
> > [1] https://patchwork.ozlabs.org/cover/950964/
> >
> > Any inputs?
>
> Hmm.... It looks like we are doing some development in parallel.
>
> Please look into following commit [1]:
> https://patchwork.ozlabs.org/patch/1034051/
>
> It ports from Linux 5.0 the CCF framework for iMX6Q, which IMHO in the
> long term is a better approach.
> The code is kept simple and resembles the code from Barebox.
>
> Please correct me if I'm wrong, but the code from your work is not
> modeling muxes, gates and other components from Linux CCF.

The U-Boot implementation of CLK would require as minimal and simple
as possible due to requirement of U-Boot itself. Hope you agree this
point? if yes having CCF stack code to handle all clock with
respective separate drivers management is may not require as of now,
IMHO.

This series is using recursive calls for handling parenting stuff to
handle get or set rates, which is fine for handling clock tree
management as far as U-Boot point-of-view. We have faced similar
situation as I explained in commit message about Allwinner clocks [2]
and we ended up going this way.

The patches where I get introduced clock tree is based on muxes, gates
which were similar like Linux but I've managed to update according to
U-Boot need. I have tried enet, enet_ref clocks as well and those are
working out-of-box.

Feel free to comments, I have no intention to block anything. let's
have a proper discussion.

[2] https://patchwork.ozlabs.org/patch/1019673/

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Lukasz Majewski]

On Thu, 4 Apr 2019 14:56:36 +0530
Jagan Teki <jagan@amarulasolutions.com> wrote:

> On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > On Tue,  2 Apr 2019 16:58:33 +0530
> > Jagan Teki <jagan@amarulasolutions.com> wrote:
> >  
> > > This is revised version of previous i.MX6 clock management [1].
> > >
> > > The main difference between previous version is
> > > - Group the i.MX6 ccm clocks into gates and tree instead of
> > > handling the clocks in simple way using case statement.
> > > - use gate clocks for enable/disable management.
> > > - use tree clocks for get/set rate or parent traverse management.
> > > - parent clock handling via clock type.
> > > - traverse the parent clock using recursive functionlaity.
> > >
> > > The main motive behind this tree framework is to make the clock
> > > tree management simple and useful for U-Boot requirements instead
> > > of garbing Linux clock management code.
> > >
> > > We are trying to manage the Allwinner clocks with similar kind, so
> > > having this would really help i.MX6 as well.
> > >
> > > Added simple names for clock macros, but will update it in future
> > > version.
> > >
> > > I have skipped ENET clocks from previous series, will add it in
> > > future patches.
> > >
> > > Changes for v2:
> > > - changed framework patches.
> > > - add support for imx6qdl and imx6ul boards
> > > - add clock gates, tree.
> > >
> > > [1] https://patchwork.ozlabs.org/cover/950964/
> > >
> > > Any inputs?  
> >
> > Hmm.... It looks like we are doing some development in parallel.
> >
> > Please look into following commit [1]:
> > https://patchwork.ozlabs.org/patch/1034051/
> >
> > It ports from Linux 5.0 the CCF framework for iMX6Q, which IMHO in
> > the long term is a better approach.
> > The code is kept simple and resembles the code from Barebox.
> >
> > Please correct me if I'm wrong, but the code from your work is not
> > modeling muxes, gates and other components from Linux CCF.  
> 
> The U-Boot implementation of CLK would require as minimal and simple
> as possible due to requirement of U-Boot itself. Hope you agree this
> point? 

Now i.MX6 is using clock.c CLK implementation. If we decide to
replace it - we shall do it in a way, which would allow us to follow
Linux kernel. (the barebox implementation is a stripped CCF from
Linux, the same is in patch [1]).

> if yes having CCF stack code to handle all clock with
> respective separate drivers management is may not require as of now,
> IMHO.

I do have a gut feeling, that we will end up with the need to have the
CCF framework ported anyway. As for example imx7/8 can re-use muxes,
gates code.

However, those are only my "feelings" after a glimpse look - I will look
into your code more thoroughly and provide feedback.

> 
> This series is using recursive calls for handling parenting stuff to
> handle get or set rates, which is fine for handling clock tree
> management as far as U-Boot point-of-view. We have faced similar
> situation as I explained in commit message about Allwinner clocks [2]
> and we ended up going this way.

I'm not Allwinner expert - but if I may ask - how far away is this
implementation from mainline Linux kernel?

How difficult is it to port the new code (or update it)?

> 
> The patches where I get introduced clock tree is based on muxes, gates
> which were similar like Linux but I've managed to update according to
> U-Boot need.
> I have tried enet, enet_ref clocks as well and those are
> working out-of-box.
> 
> Feel free to comments, I have no intention to block anything. let's
> have a proper discussion.

Fabio, Stefano what do you think?

As we change the clock.c code, IMHO we shall do the new port properly.

> 
> [2] https://patchwork.ozlabs.org/patch/1019673/




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190404/40973719/attachment.sig>

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Fabio Estevam]

On Thu, Apr 4, 2019 at 7:01 AM Lukasz Majewski <lukma@denx.de> wrote:

> Fabio, Stefano what do you think?
>
> As we change the clock.c code, IMHO we shall do the new port properly.

I think the CCF solution proposed by Lukasz looks good and it will be
easier to maintain and sync with the kernel.

Thanks

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Tom Rini]

On Thu, Apr 04, 2019 at 12:48:58PM -0300, Fabio Estevam wrote:
> On Thu, Apr 4, 2019 at 7:01 AM Lukasz Majewski <lukma@denx.de> wrote:
> 
> > Fabio, Stefano what do you think?
> >
> > As we change the clock.c code, IMHO we shall do the new port properly.
> 
> I think the CCF solution proposed by Lukasz looks good and it will be
> easier to maintain and sync with the kernel.

This sounds like an important goal as well, to me.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190404/f0a7b19a/attachment.sig>

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Jagan Teki]

On Thu, Apr 4, 2019 at 9:26 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Thu, Apr 04, 2019 at 12:48:58PM -0300, Fabio Estevam wrote:
> > On Thu, Apr 4, 2019 at 7:01 AM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > > Fabio, Stefano what do you think?
> > >
> > > As we change the clock.c code, IMHO we shall do the new port properly.
> >
> > I think the CCF solution proposed by Lukasz looks good and it will be
> > easier to maintain and sync with the kernel.
>
> This sounds like an important goal as well, to me.  Thanks!

I don't know why we rely too-much on Linux to import the big stack
code, since the requirement of U-Boot here is to handle the clocks as
minimum(as required) as compared to what OS is looking for.

Are we looking for handling clock tree management for a whole or
looking as required (or as simple) is the main criteria to think
about.

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Tom Rini]

On Thu, Apr 04, 2019 at 09:35:43PM +0530, Jagan Teki wrote:
> On Thu, Apr 4, 2019 at 9:26 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Thu, Apr 04, 2019 at 12:48:58PM -0300, Fabio Estevam wrote:
> > > On Thu, Apr 4, 2019 at 7:01 AM Lukasz Majewski <lukma@denx.de> wrote:
> > >
> > > > Fabio, Stefano what do you think?
> > > >
> > > > As we change the clock.c code, IMHO we shall do the new port properly.
> > >
> > > I think the CCF solution proposed by Lukasz looks good and it will be
> > > easier to maintain and sync with the kernel.
> >
> > This sounds like an important goal as well, to me.  Thanks!
> 
> I don't know why we rely too-much on Linux to import the big stack
> code, since the requirement of U-Boot here is to handle the clocks as
> minimum(as required) as compared to what OS is looking for.
> 
> Are we looking for handling clock tree management for a whole or
> looking as required (or as simple) is the main criteria to think
> about.

We rely on leveraging Linux when possible for a lot of reasons.  First,
it's generally going to have to solve most of the same problems we have
to solve.  Second, it's what most folks are going to be familiar with.
So if we can strip down that same framework to work for us, it'll make
life easier on everyone involved.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190404/a40e3bc8/attachment.sig>

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Jagan Teki]

On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski <lukma@denx.de> wrote:
>
> On Thu, 4 Apr 2019 14:56:36 +0530
> Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski <lukma@denx.de> wrote:
> > >
> > > On Tue,  2 Apr 2019 16:58:33 +0530
> > > Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > > This is revised version of previous i.MX6 clock management [1].
> > > >
> > > > The main difference between previous version is
> > > > - Group the i.MX6 ccm clocks into gates and tree instead of
> > > > handling the clocks in simple way using case statement.
> > > > - use gate clocks for enable/disable management.
> > > > - use tree clocks for get/set rate or parent traverse management.
> > > > - parent clock handling via clock type.
> > > > - traverse the parent clock using recursive functionlaity.
> > > >
> > > > The main motive behind this tree framework is to make the clock
> > > > tree management simple and useful for U-Boot requirements instead
> > > > of garbing Linux clock management code.
> > > >
> > > > We are trying to manage the Allwinner clocks with similar kind, so
> > > > having this would really help i.MX6 as well.
> > > >
> > > > Added simple names for clock macros, but will update it in future
> > > > version.
> > > >
> > > > I have skipped ENET clocks from previous series, will add it in
> > > > future patches.
> > > >
> > > > Changes for v2:
> > > > - changed framework patches.
> > > > - add support for imx6qdl and imx6ul boards
> > > > - add clock gates, tree.
> > > >
> > > > [1] https://patchwork.ozlabs.org/cover/950964/
> > > >
> > > > Any inputs?
> > >
> > > Hmm.... It looks like we are doing some development in parallel.
> > >
> > > Please look into following commit [1]:
> > > https://patchwork.ozlabs.org/patch/1034051/
> > >
> > > It ports from Linux 5.0 the CCF framework for iMX6Q, which IMHO in
> > > the long term is a better approach.
> > > The code is kept simple and resembles the code from Barebox.
> > >
> > > Please correct me if I'm wrong, but the code from your work is not
> > > modeling muxes, gates and other components from Linux CCF.
> >
> > The U-Boot implementation of CLK would require as minimal and simple
> > as possible due to requirement of U-Boot itself. Hope you agree this
> > point?
>
> Now i.MX6 is using clock.c CLK implementation. If we decide to
> replace it - we shall do it in a way, which would allow us to follow
> Linux kernel. (the barebox implementation is a stripped CCF from
> Linux, the same is in patch [1]).
>
> > if yes having CCF stack code to handle all clock with
> > respective separate drivers management is may not require as of now,
> > IMHO.
>
> I do have a gut feeling, that we will end up with the need to have the
> CCF framework ported anyway. As for example imx7/8 can re-use muxes,
> gates code.

As per my experience the main the over-ahead to handle clocks in
U-Boot if we go with separate clock drivers is for Video and Ethernet
peripherals. these are key IP's which use more clocks from U-Boot
point-of-view, others can be handle pretty straight-forward unless if
they don't have too much tree chain.

On this series, the tree management is already supported ENET in
i.MX6, and Allwinner platforms.

As of now, I'm thinking I can handle reset of the clocks with similar way.

>
> However, those are only my "feelings" after a glimpse look - I will look
> into your code more thoroughly and provide feedback.

Please have a look, if possible check even the code size by adding USDHC clocks.

>
> >
> > This series is using recursive calls for handling parenting stuff to
> > handle get or set rates, which is fine for handling clock tree
> > management as far as U-Boot point-of-view. We have faced similar
> > situation as I explained in commit message about Allwinner clocks [2]
> > and we ended up going this way.
>
> I'm not Allwinner expert - but if I may ask - how far away is this
> implementation from mainline Linux kernel?
>
> How difficult is it to port the new code (or update it)?

Allwinner clocks also has similar gates, muxs, and with other platform
stuff which has too much scope in Linux to use CCM.

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Lukasz Majewski]

Hi Jagan,

> On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > On Thu, 4 Apr 2019 14:56:36 +0530
> > Jagan Teki <jagan@amarulasolutions.com> wrote:
> >  
> > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski <lukma@denx.de>
> > > wrote:  
> > > >
> > > > On Tue,  2 Apr 2019 16:58:33 +0530
> > > > Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > >  
> > > > > This is revised version of previous i.MX6 clock management
> > > > > [1].
> > > > >
> > > > > The main difference between previous version is
> > > > > - Group the i.MX6 ccm clocks into gates and tree instead of
> > > > > handling the clocks in simple way using case statement.
> > > > > - use gate clocks for enable/disable management.
> > > > > - use tree clocks for get/set rate or parent traverse
> > > > > management.
> > > > > - parent clock handling via clock type.
> > > > > - traverse the parent clock using recursive functionlaity.
> > > > >
> > > > > The main motive behind this tree framework is to make the
> > > > > clock tree management simple and useful for U-Boot
> > > > > requirements instead of garbing Linux clock management code.
> > > > >
> > > > > We are trying to manage the Allwinner clocks with similar
> > > > > kind, so having this would really help i.MX6 as well.
> > > > >
> > > > > Added simple names for clock macros, but will update it in
> > > > > future version.
> > > > >
> > > > > I have skipped ENET clocks from previous series, will add it
> > > > > in future patches.
> > > > >
> > > > > Changes for v2:
> > > > > - changed framework patches.
> > > > > - add support for imx6qdl and imx6ul boards
> > > > > - add clock gates, tree.
> > > > >
> > > > > [1] https://patchwork.ozlabs.org/cover/950964/
> > > > >
> > > > > Any inputs?  
> > > >
> > > > Hmm.... It looks like we are doing some development in parallel.
> > > >
> > > > Please look into following commit [1]:
> > > > https://patchwork.ozlabs.org/patch/1034051/
> > > >
> > > > It ports from Linux 5.0 the CCF framework for iMX6Q, which IMHO
> > > > in the long term is a better approach.
> > > > The code is kept simple and resembles the code from Barebox.
> > > >
> > > > Please correct me if I'm wrong, but the code from your work is
> > > > not modeling muxes, gates and other components from Linux CCF.  
> > >
> > > The U-Boot implementation of CLK would require as minimal and
> > > simple as possible due to requirement of U-Boot itself. Hope you
> > > agree this point?  
> >
> > Now i.MX6 is using clock.c CLK implementation. If we decide to
> > replace it - we shall do it in a way, which would allow us to follow
> > Linux kernel. (the barebox implementation is a stripped CCF from
> > Linux, the same is in patch [1]).
> >  
> > > if yes having CCF stack code to handle all clock with
> > > respective separate drivers management is may not require as of
> > > now, IMHO.  
> >
> > I do have a gut feeling, that we will end up with the need to have
> > the CCF framework ported anyway. As for example imx7/8 can re-use
> > muxes, gates code.  
> 
> As per my experience the main the over-ahead to handle clocks in
> U-Boot if we go with separate clock drivers is for Video and Ethernet
> peripherals. these are key IP's which use more clocks from U-Boot
> point-of-view, others can be handle pretty straight-forward unless if
> they don't have too much tree chain.
> 
> On this series, the tree management is already supported ENET in
> i.MX6, and Allwinner platforms.
> 
> As of now, I'm thinking I can handle reset of the clocks with similar
> way.

But this code also supports ENET and ESDHCI clocks on i.MX6Q (as
supporting those was the motivator for this work).

One important thing to be aware of - the problem with SPL's footprint.
The implementation with clock.c is small and simple, but doesn't scale
well.

> 
> >
> > However, those are only my "feelings" after a glimpse look - I will
> > look into your code more thoroughly and provide feedback.  
> 
> Please have a look, if possible check even the code size by adding
> USDHC clocks.

Yes, code size (especially in SPL) is an _important_ factor here.

> 
> >  
> > >
> > > This series is using recursive calls for handling parenting stuff
> > > to handle get or set rates, which is fine for handling clock tree
> > > management as far as U-Boot point-of-view. We have faced similar
> > > situation as I explained in commit message about Allwinner clocks
> > > [2] and we ended up going this way.  
> >
> > I'm not Allwinner expert - but if I may ask - how far away is this
> > implementation from mainline Linux kernel?
> >
> > How difficult is it to port the new code (or update it)?  
> 
> Allwinner clocks also has similar gates, muxs, and with other platform
> stuff which has too much scope in Linux to use CCM.

For example the barebox managed to get subset of Linux CCF ported,
without loosing the CCF similarity.


Important factors/requirements for the i.MX clock code:

1. Easy maintenance in long-term

2. Reusing the code in SPL (with a very important factor of
_code_size_).

3. Reuse the code for other i.MX SoCs (imx7, imx8)

4. Effort needed to use DM with this code 



Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190404/60e47a2d/attachment.sig>

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Jagan Teki]

On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski <lukma@denx.de> wrote:
>
> Hi Jagan,
>
> > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski <lukma@denx.de> wrote:
> > >
> > > On Thu, 4 Apr 2019 14:56:36 +0530
> > > Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski <lukma@denx.de>
> > > > wrote:
> > > > >
> > > > > On Tue,  2 Apr 2019 16:58:33 +0530
> > > > > Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > > >
> > > > > > This is revised version of previous i.MX6 clock management
> > > > > > [1].
> > > > > >
> > > > > > The main difference between previous version is
> > > > > > - Group the i.MX6 ccm clocks into gates and tree instead of
> > > > > > handling the clocks in simple way using case statement.
> > > > > > - use gate clocks for enable/disable management.
> > > > > > - use tree clocks for get/set rate or parent traverse
> > > > > > management.
> > > > > > - parent clock handling via clock type.
> > > > > > - traverse the parent clock using recursive functionlaity.
> > > > > >
> > > > > > The main motive behind this tree framework is to make the
> > > > > > clock tree management simple and useful for U-Boot
> > > > > > requirements instead of garbing Linux clock management code.
> > > > > >
> > > > > > We are trying to manage the Allwinner clocks with similar
> > > > > > kind, so having this would really help i.MX6 as well.
> > > > > >
> > > > > > Added simple names for clock macros, but will update it in
> > > > > > future version.
> > > > > >
> > > > > > I have skipped ENET clocks from previous series, will add it
> > > > > > in future patches.
> > > > > >
> > > > > > Changes for v2:
> > > > > > - changed framework patches.
> > > > > > - add support for imx6qdl and imx6ul boards
> > > > > > - add clock gates, tree.
> > > > > >
> > > > > > [1] https://patchwork.ozlabs.org/cover/950964/
> > > > > >
> > > > > > Any inputs?
> > > > >
> > > > > Hmm.... It looks like we are doing some development in parallel.
> > > > >
> > > > > Please look into following commit [1]:
> > > > > https://patchwork.ozlabs.org/patch/1034051/
> > > > >
> > > > > It ports from Linux 5.0 the CCF framework for iMX6Q, which IMHO
> > > > > in the long term is a better approach.
> > > > > The code is kept simple and resembles the code from Barebox.
> > > > >
> > > > > Please correct me if I'm wrong, but the code from your work is
> > > > > not modeling muxes, gates and other components from Linux CCF.
> > > >
> > > > The U-Boot implementation of CLK would require as minimal and
> > > > simple as possible due to requirement of U-Boot itself. Hope you
> > > > agree this point?
> > >
> > > Now i.MX6 is using clock.c CLK implementation. If we decide to
> > > replace it - we shall do it in a way, which would allow us to follow
> > > Linux kernel. (the barebox implementation is a stripped CCF from
> > > Linux, the same is in patch [1]).
> > >
> > > > if yes having CCF stack code to handle all clock with
> > > > respective separate drivers management is may not require as of
> > > > now, IMHO.
> > >
> > > I do have a gut feeling, that we will end up with the need to have
> > > the CCF framework ported anyway. As for example imx7/8 can re-use
> > > muxes, gates code.
> >
> > As per my experience the main the over-ahead to handle clocks in
> > U-Boot if we go with separate clock drivers is for Video and Ethernet
> > peripherals. these are key IP's which use more clocks from U-Boot
> > point-of-view, others can be handle pretty straight-forward unless if
> > they don't have too much tree chain.
> >
> > On this series, the tree management is already supported ENET in
> > i.MX6, and Allwinner platforms.
> >
> > As of now, I'm thinking I can handle reset of the clocks with similar
> > way.
>
> But this code also supports ENET and ESDHCI clocks on i.MX6Q (as
> supporting those was the motivator for this work).
>
> One important thing to be aware of - the problem with SPL's footprint.
> The implementation with clock.c is small and simple, but doesn't scale
> well.
>
> >
> > >
> > > However, those are only my "feelings" after a glimpse look - I will
> > > look into your code more thoroughly and provide feedback.
> >
> > Please have a look, if possible check even the code size by adding
> > USDHC clocks.
>
> Yes, code size (especially in SPL) is an _important_ factor here.
>
> >
> > >
> > > >
> > > > This series is using recursive calls for handling parenting stuff
> > > > to handle get or set rates, which is fine for handling clock tree
> > > > management as far as U-Boot point-of-view. We have faced similar
> > > > situation as I explained in commit message about Allwinner clocks
> > > > [2] and we ended up going this way.
> > >
> > > I'm not Allwinner expert - but if I may ask - how far away is this
> > > implementation from mainline Linux kernel?
> > >
> > > How difficult is it to port the new code (or update it)?
> >
> > Allwinner clocks also has similar gates, muxs, and with other platform
> > stuff which has too much scope in Linux to use CCM.
>
> For example the barebox managed to get subset of Linux CCF ported,
> without loosing the CCF similarity.
>
>
> Important factors/requirements for the i.MX clock code:
>
> 1. Easy maintenance in long-term
>
> 2. Reusing the code in SPL (with a very important factor of
> _code_size_).
>
> 3. Reuse the code for other i.MX SoCs (imx7, imx8)
>
> 4. Effort needed to use DM with this code

I understand your points, I was managed this series based on these
requirements as well. We even consider the foot-print, atleast for
recursive calls of handling parenting scale well. May be we can
consider to design based on this as per U-Boot.

Let me come-back with another series or do you have any inputs or
questions, please post it.

Jagan.

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Lukasz Majewski]

On Fri, 19 Apr 2019 11:56:25 +0530
Jagan Teki <jagan@amarulasolutions.com> wrote:

> On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski <lukma@denx.de> wrote:
> >
> > Hi Jagan,
> >  
> > > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski <lukma@denx.de>
> > > wrote:  
> > > >
> > > > On Thu, 4 Apr 2019 14:56:36 +0530
> > > > Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > >  
> > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski <lukma@denx.de>
> > > > > wrote:  
> > > > > >
> > > > > > On Tue,  2 Apr 2019 16:58:33 +0530
> > > > > > Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > > > >  
> > > > > > > This is revised version of previous i.MX6 clock management
> > > > > > > [1].
> > > > > > >
> > > > > > > The main difference between previous version is
> > > > > > > - Group the i.MX6 ccm clocks into gates and tree instead
> > > > > > > of handling the clocks in simple way using case statement.
> > > > > > > - use gate clocks for enable/disable management.
> > > > > > > - use tree clocks for get/set rate or parent traverse
> > > > > > > management.
> > > > > > > - parent clock handling via clock type.
> > > > > > > - traverse the parent clock using recursive functionlaity.
> > > > > > >
> > > > > > > The main motive behind this tree framework is to make the
> > > > > > > clock tree management simple and useful for U-Boot
> > > > > > > requirements instead of garbing Linux clock management
> > > > > > > code.
> > > > > > >
> > > > > > > We are trying to manage the Allwinner clocks with similar
> > > > > > > kind, so having this would really help i.MX6 as well.
> > > > > > >
> > > > > > > Added simple names for clock macros, but will update it in
> > > > > > > future version.
> > > > > > >
> > > > > > > I have skipped ENET clocks from previous series, will add
> > > > > > > it in future patches.
> > > > > > >
> > > > > > > Changes for v2:
> > > > > > > - changed framework patches.
> > > > > > > - add support for imx6qdl and imx6ul boards
> > > > > > > - add clock gates, tree.
> > > > > > >
> > > > > > > [1] https://patchwork.ozlabs.org/cover/950964/
> > > > > > >
> > > > > > > Any inputs?  
> > > > > >
> > > > > > Hmm.... It looks like we are doing some development in
> > > > > > parallel.
> > > > > >
> > > > > > Please look into following commit [1]:
> > > > > > https://patchwork.ozlabs.org/patch/1034051/
> > > > > >
> > > > > > It ports from Linux 5.0 the CCF framework for iMX6Q, which
> > > > > > IMHO in the long term is a better approach.
> > > > > > The code is kept simple and resembles the code from Barebox.
> > > > > >
> > > > > > Please correct me if I'm wrong, but the code from your work
> > > > > > is not modeling muxes, gates and other components from
> > > > > > Linux CCF.  
> > > > >
> > > > > The U-Boot implementation of CLK would require as minimal and
> > > > > simple as possible due to requirement of U-Boot itself. Hope
> > > > > you agree this point?  
> > > >
> > > > Now i.MX6 is using clock.c CLK implementation. If we decide to
> > > > replace it - we shall do it in a way, which would allow us to
> > > > follow Linux kernel. (the barebox implementation is a stripped
> > > > CCF from Linux, the same is in patch [1]).
> > > >  
> > > > > if yes having CCF stack code to handle all clock with
> > > > > respective separate drivers management is may not require as
> > > > > of now, IMHO.  
> > > >
> > > > I do have a gut feeling, that we will end up with the need to
> > > > have the CCF framework ported anyway. As for example imx7/8 can
> > > > re-use muxes, gates code.  
> > >
> > > As per my experience the main the over-ahead to handle clocks in
> > > U-Boot if we go with separate clock drivers is for Video and
> > > Ethernet peripherals. these are key IP's which use more clocks
> > > from U-Boot point-of-view, others can be handle pretty
> > > straight-forward unless if they don't have too much tree chain.
> > >
> > > On this series, the tree management is already supported ENET in
> > > i.MX6, and Allwinner platforms.
> > >
> > > As of now, I'm thinking I can handle reset of the clocks with
> > > similar way.  
> >
> > But this code also supports ENET and ESDHCI clocks on i.MX6Q (as
> > supporting those was the motivator for this work).
> >
> > One important thing to be aware of - the problem with SPL's
> > footprint. The implementation with clock.c is small and simple, but
> > doesn't scale well.
> >  
> > >  
> > > >
> > > > However, those are only my "feelings" after a glimpse look - I
> > > > will look into your code more thoroughly and provide feedback.  
> > >
> > > Please have a look, if possible check even the code size by adding
> > > USDHC clocks.  
> >
> > Yes, code size (especially in SPL) is an _important_ factor here.
> >  
> > >  
> > > >  
> > > > >
> > > > > This series is using recursive calls for handling parenting
> > > > > stuff to handle get or set rates, which is fine for handling
> > > > > clock tree management as far as U-Boot point-of-view. We have
> > > > > faced similar situation as I explained in commit message
> > > > > about Allwinner clocks [2] and we ended up going this way.  
> > > >
> > > > I'm not Allwinner expert - but if I may ask - how far away is
> > > > this implementation from mainline Linux kernel?
> > > >
> > > > How difficult is it to port the new code (or update it)?  
> > >
> > > Allwinner clocks also has similar gates, muxs, and with other
> > > platform stuff which has too much scope in Linux to use CCM.  
> >
> > For example the barebox managed to get subset of Linux CCF ported,
> > without loosing the CCF similarity.
> >
> >
> > Important factors/requirements for the i.MX clock code:
> >
> > 1. Easy maintenance in long-term
> >
> > 2. Reusing the code in SPL (with a very important factor of
> > _code_size_).
> >
> > 3. Reuse the code for other i.MX SoCs (imx7, imx8)
> >
> > 4. Effort needed to use DM with this code  
> 
> I understand your points, I was managed this series based on these
> requirements as well. 

Ok.

Could you share the delta of footprint size (u-boot.img/SPL) with and
without your patch (on imx6q) ?

In my case the CCF caused increase of u-boot.img proper (as it was not
yet adapted to SPL):

415KiB -> 421KiB = 6KiB increase of size (< 2%).

(This can be further reduced by using OF_PLATDATA).

This CCF code hasn't been ported to SPL (yet)

> We even consider the foot-print, atleast for
> recursive calls of handling parenting scale well.

With CCF porting v3 I'm going to provide some caching, so the
descending would be done at most once.

> May be we can
> consider to design based on this as per U-Boot.
> 

Please look into point 1. Having code ported from Linux is IMHO better
in the long term.

> Let me come-back with another series or do you have any inputs or
> questions, please post it.

I will post CCF port for imx6q v3 in a few days.

> 
> Jagan.




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190419/5c84bf95/attachment.sig>

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Peng Fan]

> On Fri, 19 Apr 2019 11:56:25 +0530
> Jagan Teki <jagan@amarulasolutions.com> wrote:
> 
> > On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski <lukma@denx.de> wrote:
> > >
> > > Hi Jagan,
> > >
> > > > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski <lukma@denx.de>
> > > > wrote:
> > > > >
> > > > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki
> > > > > <jagan@amarulasolutions.com> wrote:
> > > > >
> > > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski <lukma@denx.de>
> > > > > > wrote:
> > > > > > >
> > > > > > > On Tue,  2 Apr 2019 16:58:33 +0530 Jagan Teki
> > > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > > >
> > > > > > > > This is revised version of previous i.MX6 clock management
> > > > > > > > [1].
> > > > > > > >
> > > > > > > > The main difference between previous version is
> > > > > > > > - Group the i.MX6 ccm clocks into gates and tree instead
> > > > > > > > of handling the clocks in simple way using case statement.
> > > > > > > > - use gate clocks for enable/disable management.
> > > > > > > > - use tree clocks for get/set rate or parent traverse
> > > > > > > > management.
> > > > > > > > - parent clock handling via clock type.
> > > > > > > > - traverse the parent clock using recursive functionlaity.
> > > > > > > >
> > > > > > > > The main motive behind this tree framework is to make the
> > > > > > > > clock tree management simple and useful for U-Boot
> > > > > > > > requirements instead of garbing Linux clock management
> > > > > > > > code.
> > > > > > > >
> > > > > > > > We are trying to manage the Allwinner clocks with similar
> > > > > > > > kind, so having this would really help i.MX6 as well.
> > > > > > > >
> > > > > > > > Added simple names for clock macros, but will update it in
> > > > > > > > future version.
> > > > > > > >
> > > > > > > > I have skipped ENET clocks from previous series, will add
> > > > > > > > it in future patches.
> > > > > > > >
> > > > > > > > Changes for v2:
> > > > > > > > - changed framework patches.
> > > > > > > > - add support for imx6qdl and imx6ul boards
> > > > > > > > - add clock gates, tree.
> > > > > > > >
> > > > > > > > [1] https://patchwork.ozlabs.org/cover/950964/
> > > > > > > >
> > > > > > > > Any inputs?
> > > > > > >
> > > > > > > Hmm.... It looks like we are doing some development in
> > > > > > > parallel.
> > > > > > >
> > > > > > > Please look into following commit [1]:
> > > > > > > https://patchwork.ozlabs.org/patch/1034051/
> > > > > > >
> > > > > > > It ports from Linux 5.0 the CCF framework for iMX6Q, which
> > > > > > > IMHO in the long term is a better approach.
> > > > > > > The code is kept simple and resembles the code from Barebox.
> > > > > > >
> > > > > > > Please correct me if I'm wrong, but the code from your work
> > > > > > > is not modeling muxes, gates and other components from Linux
> > > > > > > CCF.
> > > > > >
> > > > > > The U-Boot implementation of CLK would require as minimal and
> > > > > > simple as possible due to requirement of U-Boot itself. Hope
> > > > > > you agree this point?
> > > > >
> > > > > Now i.MX6 is using clock.c CLK implementation. If we decide to
> > > > > replace it - we shall do it in a way, which would allow us to
> > > > > follow Linux kernel. (the barebox implementation is a stripped
> > > > > CCF from Linux, the same is in patch [1]).
> > > > >
> > > > > > if yes having CCF stack code to handle all clock with
> > > > > > respective separate drivers management is may not require as
> > > > > > of now, IMHO.
> > > > >
> > > > > I do have a gut feeling, that we will end up with the need to
> > > > > have the CCF framework ported anyway. As for example imx7/8 can
> > > > > re-use muxes, gates code.
> > > >
> > > > As per my experience the main the over-ahead to handle clocks in
> > > > U-Boot if we go with separate clock drivers is for Video and
> > > > Ethernet peripherals. these are key IP's which use more clocks
> > > > from U-Boot point-of-view, others can be handle pretty
> > > > straight-forward unless if they don't have too much tree chain.
> > > >
> > > > On this series, the tree management is already supported ENET in
> > > > i.MX6, and Allwinner platforms.
> > > >
> > > > As of now, I'm thinking I can handle reset of the clocks with
> > > > similar way.
> > >
> > > But this code also supports ENET and ESDHCI clocks on i.MX6Q (as
> > > supporting those was the motivator for this work).
> > >
> > > One important thing to be aware of - the problem with SPL's
> > > footprint. The implementation with clock.c is small and simple, but
> > > doesn't scale well.
> > >
> > > >
> > > > >
> > > > > However, those are only my "feelings" after a glimpse look - I
> > > > > will look into your code more thoroughly and provide feedback.
> > > >
> > > > Please have a look, if possible check even the code size by adding
> > > > USDHC clocks.
> > >
> > > Yes, code size (especially in SPL) is an _important_ factor here.
> > >
> > > >
> > > > >
> > > > > >
> > > > > > This series is using recursive calls for handling parenting
> > > > > > stuff to handle get or set rates, which is fine for handling
> > > > > > clock tree management as far as U-Boot point-of-view. We have
> > > > > > faced similar situation as I explained in commit message about
> > > > > > Allwinner clocks [2] and we ended up going this way.
> > > > >
> > > > > I'm not Allwinner expert - but if I may ask - how far away is
> > > > > this implementation from mainline Linux kernel?
> > > > >
> > > > > How difficult is it to port the new code (or update it)?
> > > >
> > > > Allwinner clocks also has similar gates, muxs, and with other
> > > > platform stuff which has too much scope in Linux to use CCM.
> > >
> > > For example the barebox managed to get subset of Linux CCF ported,
> > > without loosing the CCF similarity.
> > >
> > >
> > > Important factors/requirements for the i.MX clock code:
> > >
> > > 1. Easy maintenance in long-term
> > >
> > > 2. Reusing the code in SPL (with a very important factor of
> > > _code_size_).
> > >
> > > 3. Reuse the code for other i.MX SoCs (imx7, imx8)
> > >
> > > 4. Effort needed to use DM with this code
> >
> > I understand your points, I was managed this series based on these
> > requirements as well.
> 
> Ok.
> 
> Could you share the delta of footprint size (u-boot.img/SPL) with and without
> your patch (on imx6q) ?
> 
> In my case the CCF caused increase of u-boot.img proper (as it was not yet
> adapted to SPL):
> 
> 415KiB -> 421KiB = 6KiB increase of size (< 2%).
> 
> (This can be further reduced by using OF_PLATDATA).
> 
> This CCF code hasn't been ported to SPL (yet)
> 
> > We even consider the foot-print, atleast for recursive calls of
> > handling parenting scale well.
> 
> With CCF porting v3 I'm going to provide some caching, so the descending
> would be done at most once.
> 
> > May be we can
> > consider to design based on this as per U-Boot.
> >
> 
> Please look into point 1. Having code ported from Linux is IMHO better in the
> long term.

Agree.

> 
> > Let me come-back with another series or do you have any inputs or
> > questions, please post it.
> 
> I will post CCF port for imx6q v3 in a few days.

Looking forward your new patchset.
Working on enabling i.MX8MM CCF support.

Regards,
Peng.

> 
> >
> > Jagan.
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma at denx.de

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Lukasz Majewski]

On Fri, 19 Apr 2019 08:52:28 +0000
Peng Fan <peng.fan@nxp.com> wrote:

> > On Fri, 19 Apr 2019 11:56:25 +0530
> > Jagan Teki <jagan@amarulasolutions.com> wrote:
> >   
> > > On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski <lukma@denx.de>
> > > wrote:  
> > > >
> > > > Hi Jagan,
> > > >  
> > > > > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski <lukma@denx.de>
> > > > > wrote:  
> > > > > >
> > > > > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki
> > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > >  
> > > > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski
> > > > > > > <lukma@denx.de> wrote:  
> > > > > > > >
> > > > > > > > On Tue,  2 Apr 2019 16:58:33 +0530 Jagan Teki
> > > > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > > > >  
> > > > > > > > > This is revised version of previous i.MX6 clock
> > > > > > > > > management [1].
> > > > > > > > >
> > > > > > > > > The main difference between previous version is
> > > > > > > > > - Group the i.MX6 ccm clocks into gates and tree
> > > > > > > > > instead of handling the clocks in simple way using
> > > > > > > > > case statement.
> > > > > > > > > - use gate clocks for enable/disable management.
> > > > > > > > > - use tree clocks for get/set rate or parent traverse
> > > > > > > > > management.
> > > > > > > > > - parent clock handling via clock type.
> > > > > > > > > - traverse the parent clock using recursive
> > > > > > > > > functionlaity.
> > > > > > > > >
> > > > > > > > > The main motive behind this tree framework is to make
> > > > > > > > > the clock tree management simple and useful for U-Boot
> > > > > > > > > requirements instead of garbing Linux clock management
> > > > > > > > > code.
> > > > > > > > >
> > > > > > > > > We are trying to manage the Allwinner clocks with
> > > > > > > > > similar kind, so having this would really help i.MX6
> > > > > > > > > as well.
> > > > > > > > >
> > > > > > > > > Added simple names for clock macros, but will update
> > > > > > > > > it in future version.
> > > > > > > > >
> > > > > > > > > I have skipped ENET clocks from previous series, will
> > > > > > > > > add it in future patches.
> > > > > > > > >
> > > > > > > > > Changes for v2:
> > > > > > > > > - changed framework patches.
> > > > > > > > > - add support for imx6qdl and imx6ul boards
> > > > > > > > > - add clock gates, tree.
> > > > > > > > >
> > > > > > > > > [1] https://patchwork.ozlabs.org/cover/950964/
> > > > > > > > >
> > > > > > > > > Any inputs?  
> > > > > > > >
> > > > > > > > Hmm.... It looks like we are doing some development in
> > > > > > > > parallel.
> > > > > > > >
> > > > > > > > Please look into following commit [1]:
> > > > > > > > https://patchwork.ozlabs.org/patch/1034051/
> > > > > > > >
> > > > > > > > It ports from Linux 5.0 the CCF framework for iMX6Q,
> > > > > > > > which IMHO in the long term is a better approach.
> > > > > > > > The code is kept simple and resembles the code from
> > > > > > > > Barebox.
> > > > > > > >
> > > > > > > > Please correct me if I'm wrong, but the code from your
> > > > > > > > work is not modeling muxes, gates and other components
> > > > > > > > from Linux CCF.  
> > > > > > >
> > > > > > > The U-Boot implementation of CLK would require as minimal
> > > > > > > and simple as possible due to requirement of U-Boot
> > > > > > > itself. Hope you agree this point?  
> > > > > >
> > > > > > Now i.MX6 is using clock.c CLK implementation. If we decide
> > > > > > to replace it - we shall do it in a way, which would allow
> > > > > > us to follow Linux kernel. (the barebox implementation is a
> > > > > > stripped CCF from Linux, the same is in patch [1]).
> > > > > >  
> > > > > > > if yes having CCF stack code to handle all clock with
> > > > > > > respective separate drivers management is may not require
> > > > > > > as of now, IMHO.  
> > > > > >
> > > > > > I do have a gut feeling, that we will end up with the need
> > > > > > to have the CCF framework ported anyway. As for example
> > > > > > imx7/8 can re-use muxes, gates code.  
> > > > >
> > > > > As per my experience the main the over-ahead to handle clocks
> > > > > in U-Boot if we go with separate clock drivers is for Video
> > > > > and Ethernet peripherals. these are key IP's which use more
> > > > > clocks from U-Boot point-of-view, others can be handle pretty
> > > > > straight-forward unless if they don't have too much tree
> > > > > chain.
> > > > >
> > > > > On this series, the tree management is already supported ENET
> > > > > in i.MX6, and Allwinner platforms.
> > > > >
> > > > > As of now, I'm thinking I can handle reset of the clocks with
> > > > > similar way.  
> > > >
> > > > But this code also supports ENET and ESDHCI clocks on i.MX6Q (as
> > > > supporting those was the motivator for this work).
> > > >
> > > > One important thing to be aware of - the problem with SPL's
> > > > footprint. The implementation with clock.c is small and simple,
> > > > but doesn't scale well.
> > > >  
> > > > >  
> > > > > >
> > > > > > However, those are only my "feelings" after a glimpse look
> > > > > > - I will look into your code more thoroughly and provide
> > > > > > feedback.  
> > > > >
> > > > > Please have a look, if possible check even the code size by
> > > > > adding USDHC clocks.  
> > > >
> > > > Yes, code size (especially in SPL) is an _important_ factor
> > > > here. 
> > > > >  
> > > > > >  
> > > > > > >
> > > > > > > This series is using recursive calls for handling
> > > > > > > parenting stuff to handle get or set rates, which is fine
> > > > > > > for handling clock tree management as far as U-Boot
> > > > > > > point-of-view. We have faced similar situation as I
> > > > > > > explained in commit message about Allwinner clocks [2]
> > > > > > > and we ended up going this way.  
> > > > > >
> > > > > > I'm not Allwinner expert - but if I may ask - how far away
> > > > > > is this implementation from mainline Linux kernel?
> > > > > >
> > > > > > How difficult is it to port the new code (or update it)?  
> > > > >
> > > > > Allwinner clocks also has similar gates, muxs, and with other
> > > > > platform stuff which has too much scope in Linux to use CCM.  
> > > >
> > > > For example the barebox managed to get subset of Linux CCF
> > > > ported, without loosing the CCF similarity.
> > > >
> > > >
> > > > Important factors/requirements for the i.MX clock code:
> > > >
> > > > 1. Easy maintenance in long-term
> > > >
> > > > 2. Reusing the code in SPL (with a very important factor of
> > > > _code_size_).
> > > >
> > > > 3. Reuse the code for other i.MX SoCs (imx7, imx8)
> > > >
> > > > 4. Effort needed to use DM with this code  
> > >
> > > I understand your points, I was managed this series based on these
> > > requirements as well.  
> > 
> > Ok.
> > 
> > Could you share the delta of footprint size (u-boot.img/SPL) with
> > and without your patch (on imx6q) ?
> > 
> > In my case the CCF caused increase of u-boot.img proper (as it was
> > not yet adapted to SPL):
> > 
> > 415KiB -> 421KiB = 6KiB increase of size (< 2%).
> > 
> > (This can be further reduced by using OF_PLATDATA).
> > 
> > This CCF code hasn't been ported to SPL (yet)
> >   
> > > We even consider the foot-print, atleast for recursive calls of
> > > handling parenting scale well.  
> > 
> > With CCF porting v3 I'm going to provide some caching, so the
> > descending would be done at most once.
> >   
> > > May be we can
> > > consider to design based on this as per U-Boot.
> > >  
> > 
> > Please look into point 1. Having code ported from Linux is IMHO
> > better in the long term.  
> 
> Agree.
> 
> >   
> > > Let me come-back with another series or do you have any inputs or
> > > questions, please post it.  
> > 
> > I will post CCF port for imx6q v3 in a few days.  
> 
> Looking forward your new patchset.
> Working on enabling i.MX8MM CCF support.

Output of 'dm tree' on imx6q:

 clk          1  [   ]   fixed_rate_clock      |-- ckil
 clk          2  [   ]   fixed_rate_clock      |-- ckih1
 clk          3  [ + ]   fixed_rate_clock      `-- osc
 clk          4  [ + ]   imx_clk_pllv3             |-- pll2_bus
 clk          7  [   ]   imx_clk_pfd               |   |-- pll2_pfd0_352m
 clk          8  [ + ]   imx_clk_pfd               |   `-- pll2_pfd2_396m
 clk          9  [ + ]   imx_clk_mux               |       |-- usdhc1_sel
 clk         13  [ + ]   imx_clk_divider           |       |   `-- usdhc1_podf
 clk         22  [   ]   imx_clk_gate2             |       |       `-- usdhc1
 clk         10  [ + ]   imx_clk_mux               |       |-- usdhc2_sel
 clk         14  [ + ]   imx_clk_divider           |       |   `-- usdhc2_podf
 clk         23  [   ]   imx_clk_gate2             |       |       `-- usdhc2
 clk         11  [ + ]   imx_clk_mux               |       |-- usdhc3_sel
 clk         15  [ + ]   imx_clk_divider           |       |   `-- usdhc3_podf
 clk         24  [   ]   imx_clk_gate2             |       |       `-- usdhc3
 clk         12  [ + ]   imx_clk_mux               |       `-- usdhc4_sel
 clk         16  [ + ]   imx_clk_divider           |           `-- usdhc4_podf
 clk         25  [   ]   imx_clk_gate2             |               `-- usdhc4
 clk          5  [ + ]   imx_clk_pllv3             `-- pll3_usb_otg
 clk          6  [ + ]   imx_clk_fixed_factor          `-- pll3_60m
 clk         17  [ + ]   imx_clk_divider                   `-- ecspi_root
 clk         18  [   ]   imx_clk_gate2                         |-- ecspi1
 clk         19  [   ]   imx_clk_gate2                         |-- ecspi2
 clk         20  [   ]   imx_clk_gate2                         |-- ecspi3
 clk         21  [   ]   imx_clk_gate2                         `-- ecspi4 



> 
> Regards,
> Peng.
> 
> >   
> > >
> > > Jagan.  
> > 
> > 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma at denx.de  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190420/e9d9ff42/attachment.sig>

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Peng Fan]

Hi Lukasz,

> -----Original Message-----
> From: Lukasz Majewski [mailto:lukma at denx.de]
> Sent: 2019年4月20日 6:18
> To: Peng Fan <peng.fan@nxp.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>; Stefano Babic
> <sbabic@denx.de>; Fabio Estevam <fabio.estevam@nxp.com>; Simon Glass
> <sjg@chromium.org>; Tom Rini <trini@konsulko.com>; Marek Vasut
> <marek.vasut+renesas@gmail.com>; Neil Armstrong
> <narmstrong@baylibre.com>; Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com>; Maxime Ripard
> <maxime.ripard@bootlin.com>; Michael Trimarchi
> <michael@amarulasolutions.com>; Andre Przywara
> <andre.przywara@arm.com>; U-Boot-Denx <u-boot@lists.denx.de>;
> linux-amarula at amarulasolutions.com; dl-uboot-imx <uboot-imx@nxp.com>
> Subject: Re: [U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support
> 
> On Fri, 19 Apr 2019 08:52:28 +0000
> Peng Fan <peng.fan@nxp.com> wrote:
> 
> > > On Fri, 19 Apr 2019 11:56:25 +0530
> > > Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > > On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski <lukma@denx.de>
> > > > wrote:
> > > > >
> > > > > Hi Jagan,
> > > > >
> > > > > > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski <lukma@denx.de>
> > > > > > wrote:
> > > > > > >
> > > > > > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki
> > > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > > >
> > > > > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski
> > > > > > > > <lukma@denx.de> wrote:
> > > > > > > > >
> > > > > > > > > On Tue,  2 Apr 2019 16:58:33 +0530 Jagan Teki
> > > > > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > > > > >
> > > > > > > > > > This is revised version of previous i.MX6 clock
> > > > > > > > > > management [1].
> > > > > > > > > >
> > > > > > > > > > The main difference between previous version is
> > > > > > > > > > - Group the i.MX6 ccm clocks into gates and tree
> > > > > > > > > > instead of handling the clocks in simple way using
> > > > > > > > > > case statement.
> > > > > > > > > > - use gate clocks for enable/disable management.
> > > > > > > > > > - use tree clocks for get/set rate or parent traverse
> > > > > > > > > > management.
> > > > > > > > > > - parent clock handling via clock type.
> > > > > > > > > > - traverse the parent clock using recursive
> > > > > > > > > > functionlaity.
> > > > > > > > > >
> > > > > > > > > > The main motive behind this tree framework is to make
> > > > > > > > > > the clock tree management simple and useful for U-Boot
> > > > > > > > > > requirements instead of garbing Linux clock management
> > > > > > > > > > code.
> > > > > > > > > >
> > > > > > > > > > We are trying to manage the Allwinner clocks with
> > > > > > > > > > similar kind, so having this would really help i.MX6
> > > > > > > > > > as well.
> > > > > > > > > >
> > > > > > > > > > Added simple names for clock macros, but will update
> > > > > > > > > > it in future version.
> > > > > > > > > >
> > > > > > > > > > I have skipped ENET clocks from previous series, will
> > > > > > > > > > add it in future patches.
> > > > > > > > > >
> > > > > > > > > > Changes for v2:
> > > > > > > > > > - changed framework patches.
> > > > > > > > > > - add support for imx6qdl and imx6ul boards
> > > > > > > > > > - add clock gates, tree.
> > > > > > > > > >
> > > > > > > > > > [1] https://patchwork.ozlabs.org/cover/950964/
> > > > > > > > > >
> > > > > > > > > > Any inputs?
> > > > > > > > >
> > > > > > > > > Hmm.... It looks like we are doing some development in
> > > > > > > > > parallel.
> > > > > > > > >
> > > > > > > > > Please look into following commit [1]:
> > > > > > > > > https://patchwork.ozlabs.org/patch/1034051/
> > > > > > > > >
> > > > > > > > > It ports from Linux 5.0 the CCF framework for iMX6Q,
> > > > > > > > > which IMHO in the long term is a better approach.
> > > > > > > > > The code is kept simple and resembles the code from
> > > > > > > > > Barebox.
> > > > > > > > >
> > > > > > > > > Please correct me if I'm wrong, but the code from your
> > > > > > > > > work is not modeling muxes, gates and other components
> > > > > > > > > from Linux CCF.
> > > > > > > >
> > > > > > > > The U-Boot implementation of CLK would require as minimal
> > > > > > > > and simple as possible due to requirement of U-Boot
> > > > > > > > itself. Hope you agree this point?
> > > > > > >
> > > > > > > Now i.MX6 is using clock.c CLK implementation. If we decide
> > > > > > > to replace it - we shall do it in a way, which would allow
> > > > > > > us to follow Linux kernel. (the barebox implementation is a
> > > > > > > stripped CCF from Linux, the same is in patch [1]).
> > > > > > >
> > > > > > > > if yes having CCF stack code to handle all clock with
> > > > > > > > respective separate drivers management is may not require
> > > > > > > > as of now, IMHO.
> > > > > > >
> > > > > > > I do have a gut feeling, that we will end up with the need
> > > > > > > to have the CCF framework ported anyway. As for example
> > > > > > > imx7/8 can re-use muxes, gates code.
> > > > > >
> > > > > > As per my experience the main the over-ahead to handle clocks
> > > > > > in U-Boot if we go with separate clock drivers is for Video
> > > > > > and Ethernet peripherals. these are key IP's which use more
> > > > > > clocks from U-Boot point-of-view, others can be handle pretty
> > > > > > straight-forward unless if they don't have too much tree
> > > > > > chain.
> > > > > >
> > > > > > On this series, the tree management is already supported ENET
> > > > > > in i.MX6, and Allwinner platforms.
> > > > > >
> > > > > > As of now, I'm thinking I can handle reset of the clocks with
> > > > > > similar way.
> > > > >
> > > > > But this code also supports ENET and ESDHCI clocks on i.MX6Q (as
> > > > > supporting those was the motivator for this work).
> > > > >
> > > > > One important thing to be aware of - the problem with SPL's
> > > > > footprint. The implementation with clock.c is small and simple,
> > > > > but doesn't scale well.
> > > > >
> > > > > >
> > > > > > >
> > > > > > > However, those are only my "feelings" after a glimpse look
> > > > > > > - I will look into your code more thoroughly and provide
> > > > > > > feedback.
> > > > > >
> > > > > > Please have a look, if possible check even the code size by
> > > > > > adding USDHC clocks.
> > > > >
> > > > > Yes, code size (especially in SPL) is an _important_ factor
> > > > > here.
> > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > This series is using recursive calls for handling
> > > > > > > > parenting stuff to handle get or set rates, which is fine
> > > > > > > > for handling clock tree management as far as U-Boot
> > > > > > > > point-of-view. We have faced similar situation as I
> > > > > > > > explained in commit message about Allwinner clocks [2] and
> > > > > > > > we ended up going this way.
> > > > > > >
> > > > > > > I'm not Allwinner expert - but if I may ask - how far away
> > > > > > > is this implementation from mainline Linux kernel?
> > > > > > >
> > > > > > > How difficult is it to port the new code (or update it)?
> > > > > >
> > > > > > Allwinner clocks also has similar gates, muxs, and with other
> > > > > > platform stuff which has too much scope in Linux to use CCM.
> > > > >
> > > > > For example the barebox managed to get subset of Linux CCF
> > > > > ported, without loosing the CCF similarity.
> > > > >
> > > > >
> > > > > Important factors/requirements for the i.MX clock code:
> > > > >
> > > > > 1. Easy maintenance in long-term
> > > > >
> > > > > 2. Reusing the code in SPL (with a very important factor of
> > > > > _code_size_).
> > > > >
> > > > > 3. Reuse the code for other i.MX SoCs (imx7, imx8)
> > > > >
> > > > > 4. Effort needed to use DM with this code
> > > >
> > > > I understand your points, I was managed this series based on these
> > > > requirements as well.
> > >
> > > Ok.
> > >
> > > Could you share the delta of footprint size (u-boot.img/SPL) with
> > > and without your patch (on imx6q) ?
> > >
> > > In my case the CCF caused increase of u-boot.img proper (as it was
> > > not yet adapted to SPL):
> > >
> > > 415KiB -> 421KiB = 6KiB increase of size (< 2%).
> > >
> > > (This can be further reduced by using OF_PLATDATA).
> > >
> > > This CCF code hasn't been ported to SPL (yet)
> > >
> > > > We even consider the foot-print, atleast for recursive calls of
> > > > handling parenting scale well.
> > >
> > > With CCF porting v3 I'm going to provide some caching, so the
> > > descending would be done at most once.
> > >
> > > > May be we can
> > > > consider to design based on this as per U-Boot.
> > > >
> > >
> > > Please look into point 1. Having code ported from Linux is IMHO
> > > better in the long term.
> >
> > Agree.
> >
> > >
> > > > Let me come-back with another series or do you have any inputs or
> > > > questions, please post it.
> > >
> > > I will post CCF port for imx6q v3 in a few days.
> >
> > Looking forward your new patchset.
> > Working on enabling i.MX8MM CCF support.
> 
> Output of 'dm tree' on imx6q:
> 
>  clk          1  [   ]   fixed_rate_clock      |-- ckil
>  clk          2  [   ]   fixed_rate_clock      |-- ckih1
>  clk          3  [ + ]   fixed_rate_clock      `-- osc
>  clk          4  [ + ]   imx_clk_pllv3             |-- pll2_bus
>  clk          7  [   ]   imx_clk_pfd               |   |--
> pll2_pfd0_352m
>  clk          8  [ + ]   imx_clk_pfd               |   `--
> pll2_pfd2_396m
>  clk          9  [ + ]   imx_clk_mux               |       |--
> usdhc1_sel
>  clk         13  [ + ]   imx_clk_divider           |       |   `--
> usdhc1_podf
>  clk         22  [   ]   imx_clk_gate2             |       |
> `-- usdhc1
>  clk         10  [ + ]   imx_clk_mux               |       |--
> usdhc2_sel
>  clk         14  [ + ]   imx_clk_divider           |       |   `--
> usdhc2_podf
>  clk         23  [   ]   imx_clk_gate2             |       |
> `-- usdhc2
>  clk         11  [ + ]   imx_clk_mux               |       |--
> usdhc3_sel
>  clk         15  [ + ]   imx_clk_divider           |       |   `--
> usdhc3_podf
>  clk         24  [   ]   imx_clk_gate2             |       |
> `-- usdhc3
>  clk         12  [ + ]   imx_clk_mux               |       `--
> usdhc4_sel
>  clk         16  [ + ]   imx_clk_divider           |           `--
> usdhc4_podf
>  clk         25  [   ]   imx_clk_gate2             |
> `-- usdhc4
>  clk          5  [ + ]   imx_clk_pllv3             `-- pll3_usb_otg
>  clk          6  [ + ]   imx_clk_fixed_factor          `-- pll3_60m
>  clk         17  [ + ]   imx_clk_divider                   `--
> ecspi_root
>  clk         18  [   ]   imx_clk_gate2                         |--
> ecspi1
>  clk         19  [   ]   imx_clk_gate2                         |--
> ecspi2
>  clk         20  [   ]   imx_clk_gate2                         |--
> ecspi3
>  clk         21  [   ]   imx_clk_gate2                         `--
> ecspi4
> 

Do you have a public tree/branch for CCF? I am adding imx8mm clk and would like
to base on your tree. I think need to extend clk_ops to support mux/divider,
but not just get rate. To avoid conflict with you work, if you have a public tree,
that could be good.

Thanks,
Peng.


> 
> 
> >
> > Regards,
> > Peng.
> >
> > >
> > > >
> > > > Jagan.
> > >
> > >
> > >
> > >
> > > Best regards,
> > >
> > > Lukasz Majewski
> > >
> > > --
> > >
> > > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > > lukma at denx.de
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma at denx.de

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Lukasz Majewski]

On Tue, 23 Apr 2019 07:47:38 +0000
Peng Fan <peng.fan@nxp.com> wrote:

> Hi Lukasz,
> 
> > -----Original Message-----
> > From: Lukasz Majewski [mailto:lukma at denx.de]
> > Sent: 2019年4月20日 6:18
> > To: Peng Fan <peng.fan@nxp.com>
> > Cc: Jagan Teki <jagan@amarulasolutions.com>; Stefano Babic
> > <sbabic@denx.de>; Fabio Estevam <fabio.estevam@nxp.com>; Simon Glass
> > <sjg@chromium.org>; Tom Rini <trini@konsulko.com>; Marek Vasut
> > <marek.vasut+renesas@gmail.com>; Neil Armstrong
> > <narmstrong@baylibre.com>; Philipp Tomsich
> > <philipp.tomsich@theobroma-systems.com>; Maxime Ripard
> > <maxime.ripard@bootlin.com>; Michael Trimarchi
> > <michael@amarulasolutions.com>; Andre Przywara
> > <andre.przywara@arm.com>; U-Boot-Denx <u-boot@lists.denx.de>;
> > linux-amarula at amarulasolutions.com; dl-uboot-imx <uboot-imx@nxp.com>
> > Subject: Re: [U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK
> > support
> > 
> > On Fri, 19 Apr 2019 08:52:28 +0000
> > Peng Fan <peng.fan@nxp.com> wrote:
> >   
> > > > On Fri, 19 Apr 2019 11:56:25 +0530
> > > > Jagan Teki <jagan@amarulasolutions.com> wrote:
> > > >  
> > > > > On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski <lukma@denx.de>
> > > > > wrote:  
> > > > > >
> > > > > > Hi Jagan,
> > > > > >  
> > > > > > > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski
> > > > > > > <lukma@denx.de> wrote:  
> > > > > > > >
> > > > > > > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki
> > > > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > > > >  
> > > > > > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski
> > > > > > > > > <lukma@denx.de> wrote:  
> > > > > > > > > >
> > > > > > > > > > On Tue,  2 Apr 2019 16:58:33 +0530 Jagan Teki
> > > > > > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > > > > > >  
> > > > > > > > > > > This is revised version of previous i.MX6 clock
> > > > > > > > > > > management [1].
> > > > > > > > > > >
> > > > > > > > > > > The main difference between previous version is
> > > > > > > > > > > - Group the i.MX6 ccm clocks into gates and tree
> > > > > > > > > > > instead of handling the clocks in simple way using
> > > > > > > > > > > case statement.
> > > > > > > > > > > - use gate clocks for enable/disable management.
> > > > > > > > > > > - use tree clocks for get/set rate or parent
> > > > > > > > > > > traverse management.
> > > > > > > > > > > - parent clock handling via clock type.
> > > > > > > > > > > - traverse the parent clock using recursive
> > > > > > > > > > > functionlaity.
> > > > > > > > > > >
> > > > > > > > > > > The main motive behind this tree framework is to
> > > > > > > > > > > make the clock tree management simple and useful
> > > > > > > > > > > for U-Boot requirements instead of garbing Linux
> > > > > > > > > > > clock management code.
> > > > > > > > > > >
> > > > > > > > > > > We are trying to manage the Allwinner clocks with
> > > > > > > > > > > similar kind, so having this would really help
> > > > > > > > > > > i.MX6 as well.
> > > > > > > > > > >
> > > > > > > > > > > Added simple names for clock macros, but will
> > > > > > > > > > > update it in future version.
> > > > > > > > > > >
> > > > > > > > > > > I have skipped ENET clocks from previous series,
> > > > > > > > > > > will add it in future patches.
> > > > > > > > > > >
> > > > > > > > > > > Changes for v2:
> > > > > > > > > > > - changed framework patches.
> > > > > > > > > > > - add support for imx6qdl and imx6ul boards
> > > > > > > > > > > - add clock gates, tree.
> > > > > > > > > > >
> > > > > > > > > > > [1] https://patchwork.ozlabs.org/cover/950964/
> > > > > > > > > > >
> > > > > > > > > > > Any inputs?  
> > > > > > > > > >
> > > > > > > > > > Hmm.... It looks like we are doing some development
> > > > > > > > > > in parallel.
> > > > > > > > > >
> > > > > > > > > > Please look into following commit [1]:
> > > > > > > > > > https://patchwork.ozlabs.org/patch/1034051/
> > > > > > > > > >
> > > > > > > > > > It ports from Linux 5.0 the CCF framework for iMX6Q,
> > > > > > > > > > which IMHO in the long term is a better approach.
> > > > > > > > > > The code is kept simple and resembles the code from
> > > > > > > > > > Barebox.
> > > > > > > > > >
> > > > > > > > > > Please correct me if I'm wrong, but the code from
> > > > > > > > > > your work is not modeling muxes, gates and other
> > > > > > > > > > components from Linux CCF.  
> > > > > > > > >
> > > > > > > > > The U-Boot implementation of CLK would require as
> > > > > > > > > minimal and simple as possible due to requirement of
> > > > > > > > > U-Boot itself. Hope you agree this point?  
> > > > > > > >
> > > > > > > > Now i.MX6 is using clock.c CLK implementation. If we
> > > > > > > > decide to replace it - we shall do it in a way, which
> > > > > > > > would allow us to follow Linux kernel. (the barebox
> > > > > > > > implementation is a stripped CCF from Linux, the same
> > > > > > > > is in patch [1]). 
> > > > > > > > > if yes having CCF stack code to handle all clock with
> > > > > > > > > respective separate drivers management is may not
> > > > > > > > > require as of now, IMHO.  
> > > > > > > >
> > > > > > > > I do have a gut feeling, that we will end up with the
> > > > > > > > need to have the CCF framework ported anyway. As for
> > > > > > > > example imx7/8 can re-use muxes, gates code.  
> > > > > > >
> > > > > > > As per my experience the main the over-ahead to handle
> > > > > > > clocks in U-Boot if we go with separate clock drivers is
> > > > > > > for Video and Ethernet peripherals. these are key IP's
> > > > > > > which use more clocks from U-Boot point-of-view, others
> > > > > > > can be handle pretty straight-forward unless if they
> > > > > > > don't have too much tree chain.
> > > > > > >
> > > > > > > On this series, the tree management is already supported
> > > > > > > ENET in i.MX6, and Allwinner platforms.
> > > > > > >
> > > > > > > As of now, I'm thinking I can handle reset of the clocks
> > > > > > > with similar way.  
> > > > > >
> > > > > > But this code also supports ENET and ESDHCI clocks on
> > > > > > i.MX6Q (as supporting those was the motivator for this
> > > > > > work).
> > > > > >
> > > > > > One important thing to be aware of - the problem with SPL's
> > > > > > footprint. The implementation with clock.c is small and
> > > > > > simple, but doesn't scale well.
> > > > > >  
> > > > > > >  
> > > > > > > >
> > > > > > > > However, those are only my "feelings" after a glimpse
> > > > > > > > look
> > > > > > > > - I will look into your code more thoroughly and provide
> > > > > > > > feedback.  
> > > > > > >
> > > > > > > Please have a look, if possible check even the code size
> > > > > > > by adding USDHC clocks.  
> > > > > >
> > > > > > Yes, code size (especially in SPL) is an _important_ factor
> > > > > > here.  
> > > > > > >  
> > > > > > > >  
> > > > > > > > >
> > > > > > > > > This series is using recursive calls for handling
> > > > > > > > > parenting stuff to handle get or set rates, which is
> > > > > > > > > fine for handling clock tree management as far as
> > > > > > > > > U-Boot point-of-view. We have faced similar situation
> > > > > > > > > as I explained in commit message about Allwinner
> > > > > > > > > clocks [2] and we ended up going this way.  
> > > > > > > >
> > > > > > > > I'm not Allwinner expert - but if I may ask - how far
> > > > > > > > away is this implementation from mainline Linux kernel?
> > > > > > > >
> > > > > > > > How difficult is it to port the new code (or update
> > > > > > > > it)?  
> > > > > > >
> > > > > > > Allwinner clocks also has similar gates, muxs, and with
> > > > > > > other platform stuff which has too much scope in Linux to
> > > > > > > use CCM.  
> > > > > >
> > > > > > For example the barebox managed to get subset of Linux CCF
> > > > > > ported, without loosing the CCF similarity.
> > > > > >
> > > > > >
> > > > > > Important factors/requirements for the i.MX clock code:
> > > > > >
> > > > > > 1. Easy maintenance in long-term
> > > > > >
> > > > > > 2. Reusing the code in SPL (with a very important factor of
> > > > > > _code_size_).
> > > > > >
> > > > > > 3. Reuse the code for other i.MX SoCs (imx7, imx8)
> > > > > >
> > > > > > 4. Effort needed to use DM with this code  
> > > > >
> > > > > I understand your points, I was managed this series based on
> > > > > these requirements as well.  
> > > >
> > > > Ok.
> > > >
> > > > Could you share the delta of footprint size (u-boot.img/SPL)
> > > > with and without your patch (on imx6q) ?
> > > >
> > > > In my case the CCF caused increase of u-boot.img proper (as it
> > > > was not yet adapted to SPL):
> > > >
> > > > 415KiB -> 421KiB = 6KiB increase of size (< 2%).
> > > >
> > > > (This can be further reduced by using OF_PLATDATA).
> > > >
> > > > This CCF code hasn't been ported to SPL (yet)
> > > >  
> > > > > We even consider the foot-print, atleast for recursive calls
> > > > > of handling parenting scale well.  
> > > >
> > > > With CCF porting v3 I'm going to provide some caching, so the
> > > > descending would be done at most once.
> > > >  
> > > > > May be we can
> > > > > consider to design based on this as per U-Boot.
> > > > >  
> > > >
> > > > Please look into point 1. Having code ported from Linux is IMHO
> > > > better in the long term.  
> > >
> > > Agree.
> > >  
> > > >  
> > > > > Let me come-back with another series or do you have any
> > > > > inputs or questions, please post it.  
> > > >
> > > > I will post CCF port for imx6q v3 in a few days.  
> > >
> > > Looking forward your new patchset.
> > > Working on enabling i.MX8MM CCF support.  
> > 
> > Output of 'dm tree' on imx6q:
> > 
> >  clk          1  [   ]   fixed_rate_clock      |-- ckil
> >  clk          2  [   ]   fixed_rate_clock      |-- ckih1
> >  clk          3  [ + ]   fixed_rate_clock      `-- osc
> >  clk          4  [ + ]   imx_clk_pllv3             |-- pll2_bus
> >  clk          7  [   ]   imx_clk_pfd               |   |--
> > pll2_pfd0_352m
> >  clk          8  [ + ]   imx_clk_pfd               |   `--
> > pll2_pfd2_396m
> >  clk          9  [ + ]   imx_clk_mux               |       |--
> > usdhc1_sel
> >  clk         13  [ + ]   imx_clk_divider           |       |   `--
> > usdhc1_podf
> >  clk         22  [   ]   imx_clk_gate2             |       |
> > `-- usdhc1
> >  clk         10  [ + ]   imx_clk_mux               |       |--
> > usdhc2_sel
> >  clk         14  [ + ]   imx_clk_divider           |       |   `--
> > usdhc2_podf
> >  clk         23  [   ]   imx_clk_gate2             |       |
> > `-- usdhc2
> >  clk         11  [ + ]   imx_clk_mux               |       |--
> > usdhc3_sel
> >  clk         15  [ + ]   imx_clk_divider           |       |   `--
> > usdhc3_podf
> >  clk         24  [   ]   imx_clk_gate2             |       |
> > `-- usdhc3
> >  clk         12  [ + ]   imx_clk_mux               |       `--
> > usdhc4_sel
> >  clk         16  [ + ]   imx_clk_divider           |           `--
> > usdhc4_podf
> >  clk         25  [   ]   imx_clk_gate2             |
> > `-- usdhc4
> >  clk          5  [ + ]   imx_clk_pllv3             `-- pll3_usb_otg
> >  clk          6  [ + ]   imx_clk_fixed_factor          `-- pll3_60m
> >  clk         17  [ + ]   imx_clk_divider                   `--
> > ecspi_root
> >  clk         18  [   ]   imx_clk_gate2                         |--
> > ecspi1
> >  clk         19  [   ]   imx_clk_gate2                         |--
> > ecspi2
> >  clk         20  [   ]   imx_clk_gate2                         |--
> > ecspi3
> >  clk         21  [   ]   imx_clk_gate2                         `--
> > ecspi4
> >   
> 
> Do you have a public tree/branch for CCF? 

Please find the CCF devel work:
https://github.com/lmajewski/u-boot-dfu/commits/CCF-devel

The CCF starts from: c792297e1a47ead02ff2baa4f162de8782b29910

(below you will find imx6q DM/DTS conversion code).

What is added when compared to the original one:

- SPL support

- Some fixes for v2019.04+

What is on the TO DO list:

- OF_PLATDATA for SPL (as I did not used any optimisations yet).


>I am adding imx8mm clk and
> would like to base on your tree. I think need to extend clk_ops to
> support mux/divider, but not just get rate.

Some mux/divider is provided (clk-mux.c / clk-divider.c)

> To avoid conflict with
> you work, if you have a public tree, that could be good.

No problem. Thanks for the interest.

> 
> Thanks,
> Peng.
> 
> 
> > 
> >   
> > >
> > > Regards,
> > > Peng.
> > >  
> > > >  
> > > > >
> > > > > Jagan.  
> > > >
> > > >
> > > >
> > > >
> > > > Best regards,
> > > >
> > > > Lukasz Majewski
> > > >
> > > > --
> > > >
> > > > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194
> > > > Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax:
> > > > (+49)-8142-66989-80 Email: lukma at denx.de  
> > 
> > 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma at denx.de  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190423/f13f6c87/attachment.sig>

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Peng Fan]

> -----Original Message-----
> From: Lukasz Majewski [mailto:lukma at denx.de]
> Sent: 2019年4月23日 16:46
> To: Peng Fan <peng.fan@nxp.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>; Stefano Babic
> <sbabic@denx.de>; Fabio Estevam <fabio.estevam@nxp.com>; Simon Glass
> <sjg@chromium.org>; Tom Rini <trini@konsulko.com>; Marek Vasut
> <marek.vasut+renesas@gmail.com>; Neil Armstrong
> <narmstrong@baylibre.com>; Philipp Tomsich
> <philipp.tomsich@theobroma-systems.com>; Maxime Ripard
> <maxime.ripard@bootlin.com>; Michael Trimarchi
> <michael@amarulasolutions.com>; Andre Przywara
> <andre.przywara@arm.com>; U-Boot-Denx <u-boot@lists.denx.de>;
> linux-amarula at amarulasolutions.com; dl-uboot-imx <uboot-imx@nxp.com>
> Subject: Re: [U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support
> 
> On Tue, 23 Apr 2019 07:47:38 +0000
> Peng Fan <peng.fan@nxp.com> wrote:
> 
> > Hi Lukasz,
> >
> > > -----Original Message-----
> > > From: Lukasz Majewski [mailto:lukma at denx.de]
> > > Sent: 2019年4月20日 6:18
> > > To: Peng Fan <peng.fan@nxp.com>
> > > Cc: Jagan Teki <jagan@amarulasolutions.com>; Stefano Babic
> > > <sbabic@denx.de>; Fabio Estevam <fabio.estevam@nxp.com>; Simon
> Glass
> > > <sjg@chromium.org>; Tom Rini <trini@konsulko.com>; Marek Vasut
> > > <marek.vasut+renesas@gmail.com>; Neil Armstrong
> > > <narmstrong@baylibre.com>; Philipp Tomsich
> > > <philipp.tomsich@theobroma-systems.com>; Maxime Ripard
> > > <maxime.ripard@bootlin.com>; Michael Trimarchi
> > > <michael@amarulasolutions.com>; Andre Przywara
> > > <andre.przywara@arm.com>; U-Boot-Denx <u-boot@lists.denx.de>;
> > > linux-amarula at amarulasolutions.com; dl-uboot-imx
> <uboot-imx@nxp.com>
> > > Subject: Re: [U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK
> > > support
> > >
> > > On Fri, 19 Apr 2019 08:52:28 +0000
> > > Peng Fan <peng.fan@nxp.com> wrote:
> > >
> > > > > On Fri, 19 Apr 2019 11:56:25 +0530 Jagan Teki
> > > > > <jagan@amarulasolutions.com> wrote:
> > > > >
> > > > > > On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski <lukma@denx.de>
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Jagan,
> > > > > > >
> > > > > > > > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski
> > > > > > > > <lukma@denx.de> wrote:
> > > > > > > > >
> > > > > > > > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki
> > > > > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > > > > >
> > > > > > > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski
> > > > > > > > > > <lukma@denx.de> wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Tue,  2 Apr 2019 16:58:33 +0530 Jagan Teki
> > > > > > > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > > This is revised version of previous i.MX6 clock
> > > > > > > > > > > > management [1].
> > > > > > > > > > > >
> > > > > > > > > > > > The main difference between previous version is
> > > > > > > > > > > > - Group the i.MX6 ccm clocks into gates and tree
> > > > > > > > > > > > instead of handling the clocks in simple way using
> > > > > > > > > > > > case statement.
> > > > > > > > > > > > - use gate clocks for enable/disable management.
> > > > > > > > > > > > - use tree clocks for get/set rate or parent
> > > > > > > > > > > > traverse management.
> > > > > > > > > > > > - parent clock handling via clock type.
> > > > > > > > > > > > - traverse the parent clock using recursive
> > > > > > > > > > > > functionlaity.
> > > > > > > > > > > >
> > > > > > > > > > > > The main motive behind this tree framework is to
> > > > > > > > > > > > make the clock tree management simple and useful
> > > > > > > > > > > > for U-Boot requirements instead of garbing Linux
> > > > > > > > > > > > clock management code.
> > > > > > > > > > > >
> > > > > > > > > > > > We are trying to manage the Allwinner clocks with
> > > > > > > > > > > > similar kind, so having this would really help
> > > > > > > > > > > > i.MX6 as well.
> > > > > > > > > > > >
> > > > > > > > > > > > Added simple names for clock macros, but will
> > > > > > > > > > > > update it in future version.
> > > > > > > > > > > >
> > > > > > > > > > > > I have skipped ENET clocks from previous series,
> > > > > > > > > > > > will add it in future patches.
> > > > > > > > > > > >
> > > > > > > > > > > > Changes for v2:
> > > > > > > > > > > > - changed framework patches.
> > > > > > > > > > > > - add support for imx6qdl and imx6ul boards
> > > > > > > > > > > > - add clock gates, tree.
> > > > > > > > > > > >
> > > > > > > > > > > > [1] https://patchwork.ozlabs.org/cover/950964/
> > > > > > > > > > > >
> > > > > > > > > > > > Any inputs?
> > > > > > > > > > >
> > > > > > > > > > > Hmm.... It looks like we are doing some development
> > > > > > > > > > > in parallel.
> > > > > > > > > > >
> > > > > > > > > > > Please look into following commit [1]:
> > > > > > > > > > > https://patchwork.ozlabs.org/patch/1034051/
> > > > > > > > > > >
> > > > > > > > > > > It ports from Linux 5.0 the CCF framework for iMX6Q,
> > > > > > > > > > > which IMHO in the long term is a better approach.
> > > > > > > > > > > The code is kept simple and resembles the code from
> > > > > > > > > > > Barebox.
> > > > > > > > > > >
> > > > > > > > > > > Please correct me if I'm wrong, but the code from
> > > > > > > > > > > your work is not modeling muxes, gates and other
> > > > > > > > > > > components from Linux CCF.
> > > > > > > > > >
> > > > > > > > > > The U-Boot implementation of CLK would require as
> > > > > > > > > > minimal and simple as possible due to requirement of
> > > > > > > > > > U-Boot itself. Hope you agree this point?
> > > > > > > > >
> > > > > > > > > Now i.MX6 is using clock.c CLK implementation. If we
> > > > > > > > > decide to replace it - we shall do it in a way, which
> > > > > > > > > would allow us to follow Linux kernel. (the barebox
> > > > > > > > > implementation is a stripped CCF from Linux, the same is
> > > > > > > > > in patch [1]).
> > > > > > > > > > if yes having CCF stack code to handle all clock with
> > > > > > > > > > respective separate drivers management is may not
> > > > > > > > > > require as of now, IMHO.
> > > > > > > > >
> > > > > > > > > I do have a gut feeling, that we will end up with the
> > > > > > > > > need to have the CCF framework ported anyway. As for
> > > > > > > > > example imx7/8 can re-use muxes, gates code.
> > > > > > > >
> > > > > > > > As per my experience the main the over-ahead to handle
> > > > > > > > clocks in U-Boot if we go with separate clock drivers is
> > > > > > > > for Video and Ethernet peripherals. these are key IP's
> > > > > > > > which use more clocks from U-Boot point-of-view, others
> > > > > > > > can be handle pretty straight-forward unless if they don't
> > > > > > > > have too much tree chain.
> > > > > > > >
> > > > > > > > On this series, the tree management is already supported
> > > > > > > > ENET in i.MX6, and Allwinner platforms.
> > > > > > > >
> > > > > > > > As of now, I'm thinking I can handle reset of the clocks
> > > > > > > > with similar way.
> > > > > > >
> > > > > > > But this code also supports ENET and ESDHCI clocks on i.MX6Q
> > > > > > > (as supporting those was the motivator for this work).
> > > > > > >
> > > > > > > One important thing to be aware of - the problem with SPL's
> > > > > > > footprint. The implementation with clock.c is small and
> > > > > > > simple, but doesn't scale well.
> > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > However, those are only my "feelings" after a glimpse
> > > > > > > > > look
> > > > > > > > > - I will look into your code more thoroughly and provide
> > > > > > > > > feedback.
> > > > > > > >
> > > > > > > > Please have a look, if possible check even the code size
> > > > > > > > by adding USDHC clocks.
> > > > > > >
> > > > > > > Yes, code size (especially in SPL) is an _important_ factor
> > > > > > > here.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > This series is using recursive calls for handling
> > > > > > > > > > parenting stuff to handle get or set rates, which is
> > > > > > > > > > fine for handling clock tree management as far as
> > > > > > > > > > U-Boot point-of-view. We have faced similar situation
> > > > > > > > > > as I explained in commit message about Allwinner
> > > > > > > > > > clocks [2] and we ended up going this way.
> > > > > > > > >
> > > > > > > > > I'm not Allwinner expert - but if I may ask - how far
> > > > > > > > > away is this implementation from mainline Linux kernel?
> > > > > > > > >
> > > > > > > > > How difficult is it to port the new code (or update it)?
> > > > > > > >
> > > > > > > > Allwinner clocks also has similar gates, muxs, and with
> > > > > > > > other platform stuff which has too much scope in Linux to
> > > > > > > > use CCM.
> > > > > > >
> > > > > > > For example the barebox managed to get subset of Linux CCF
> > > > > > > ported, without loosing the CCF similarity.
> > > > > > >
> > > > > > >
> > > > > > > Important factors/requirements for the i.MX clock code:
> > > > > > >
> > > > > > > 1. Easy maintenance in long-term
> > > > > > >
> > > > > > > 2. Reusing the code in SPL (with a very important factor of
> > > > > > > _code_size_).
> > > > > > >
> > > > > > > 3. Reuse the code for other i.MX SoCs (imx7, imx8)
> > > > > > >
> > > > > > > 4. Effort needed to use DM with this code
> > > > > >
> > > > > > I understand your points, I was managed this series based on
> > > > > > these requirements as well.
> > > > >
> > > > > Ok.
> > > > >
> > > > > Could you share the delta of footprint size (u-boot.img/SPL)
> > > > > with and without your patch (on imx6q) ?
> > > > >
> > > > > In my case the CCF caused increase of u-boot.img proper (as it
> > > > > was not yet adapted to SPL):
> > > > >
> > > > > 415KiB -> 421KiB = 6KiB increase of size (< 2%).
> > > > >
> > > > > (This can be further reduced by using OF_PLATDATA).
> > > > >
> > > > > This CCF code hasn't been ported to SPL (yet)
> > > > >
> > > > > > We even consider the foot-print, atleast for recursive calls
> > > > > > of handling parenting scale well.
> > > > >
> > > > > With CCF porting v3 I'm going to provide some caching, so the
> > > > > descending would be done at most once.
> > > > >
> > > > > > May be we can
> > > > > > consider to design based on this as per U-Boot.
> > > > > >
> > > > >
> > > > > Please look into point 1. Having code ported from Linux is IMHO
> > > > > better in the long term.
> > > >
> > > > Agree.
> > > >
> > > > >
> > > > > > Let me come-back with another series or do you have any inputs
> > > > > > or questions, please post it.
> > > > >
> > > > > I will post CCF port for imx6q v3 in a few days.
> > > >
> > > > Looking forward your new patchset.
> > > > Working on enabling i.MX8MM CCF support.
> > >
> > > Output of 'dm tree' on imx6q:
> > >
> > >  clk          1  [   ]   fixed_rate_clock      |-- ckil
> > >  clk          2  [   ]   fixed_rate_clock      |-- ckih1
> > >  clk          3  [ + ]   fixed_rate_clock      `-- osc
> > >  clk          4  [ + ]   imx_clk_pllv3             |-- pll2_bus
> > >  clk          7  [   ]   imx_clk_pfd               |   |--
> > > pll2_pfd0_352m
> > >  clk          8  [ + ]   imx_clk_pfd               |   `--
> > > pll2_pfd2_396m
> > >  clk          9  [ + ]   imx_clk_mux               |       |--
> > > usdhc1_sel
> > >  clk         13  [ + ]   imx_clk_divider           |       |
> `--
> > > usdhc1_podf
> > >  clk         22  [   ]   imx_clk_gate2             |       |
> > > `-- usdhc1
> > >  clk         10  [ + ]   imx_clk_mux               |       |--
> > > usdhc2_sel
> > >  clk         14  [ + ]   imx_clk_divider           |       |
> `--
> > > usdhc2_podf
> > >  clk         23  [   ]   imx_clk_gate2             |       |
> > > `-- usdhc2
> > >  clk         11  [ + ]   imx_clk_mux               |       |--
> > > usdhc3_sel
> > >  clk         15  [ + ]   imx_clk_divider           |       |
> `--
> > > usdhc3_podf
> > >  clk         24  [   ]   imx_clk_gate2             |       |
> > > `-- usdhc3
> > >  clk         12  [ + ]   imx_clk_mux               |       `--
> > > usdhc4_sel
> > >  clk         16  [ + ]   imx_clk_divider           |
> `--
> > > usdhc4_podf
> > >  clk         25  [   ]   imx_clk_gate2             |
> > > `-- usdhc4
> > >  clk          5  [ + ]   imx_clk_pllv3             `-- pll3_usb_otg
> > >  clk          6  [ + ]   imx_clk_fixed_factor          `-- pll3_60m
> > >  clk         17  [ + ]   imx_clk_divider                   `--
> > > ecspi_root
> > >  clk         18  [   ]   imx_clk_gate2
> |--
> > > ecspi1
> > >  clk         19  [   ]   imx_clk_gate2
> |--
> > > ecspi2
> > >  clk         20  [   ]   imx_clk_gate2
> |--
> > > ecspi3
> > >  clk         21  [   ]   imx_clk_gate2
> `--
> > > ecspi4
> > >
> >
> > Do you have a public tree/branch for CCF?
> 
> Please find the CCF devel work:
> https://github.com/lmajewski/u-boot-dfu/commits/CCF-devel
> 
> The CCF starts from: c792297e1a47ead02ff2baa4f162de8782b29910

Thanks.

> 
> (below you will find imx6q DM/DTS conversion code).
> 
> What is added when compared to the original one:
> 
> - SPL support
> 
> - Some fixes for v2019.04+
> 
> What is on the TO DO list:
> 
> - OF_PLATDATA for SPL (as I did not used any optimisations yet).
> 
> 
> >I am adding imx8mm clk and
> > would like to base on your tree. I think need to extend clk_ops to
> >support mux/divider, but not just get rate.
> 
> Some mux/divider is provided (clk-mux.c / clk-divider.c)

After reading into details, there is no clk core logic.
mux not support reparenting.
Divider not support rate setting.
no composite clk support.
We could not directly reuse Linux vendor clk driver, need adapt to U-Boot,
should not be hard from your code.

When multiple devices sources from one PLL, one device might would
like to set pll value that break other devices, there is no logic to protect
if support freq changing.

Do you have plan work on the upper items?
i.MX8MM use composite heavily in Linux, so I have started.

Thanks,
Peng.

> 
> > To avoid conflict with
> > you work, if you have a public tree, that could be good.
> 
> No problem. Thanks for the interest.
> 
> >
> > Thanks,
> > Peng.
> >
> >
> > >
> > >
> > > >
> > > > Regards,
> > > > Peng.
> > > >
> > > > >
> > > > > >
> > > > > > Jagan.
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > Best regards,
> > > > >
> > > > > Lukasz Majewski
> > > > >
> > > > > --
> > > > >
> > > > > DENX Software Engineering GmbH,      Managing Director:
> Wolfgang
> > > > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194
> > > > > Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax:
> > > > > (+49)-8142-66989-80 Email: lukma at denx.de
> > >
> > >
> > >
> > >
> > > Best regards,
> > >
> > > Lukasz Majewski
> > >
> > > --
> > >
> > > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > > lukma at denx.de
> 
> 
> 
> 
> Best regards,
> 
> Lukasz Majewski
> 
> --
> 
> DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> lukma at denx.de

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Lukasz Majewski]

Hi Peng,

> > -----Original Message-----
> > From: Lukasz Majewski [mailto:lukma at denx.de]
> > Sent: 2019年4月23日 16:46
> > To: Peng Fan <peng.fan@nxp.com>
> > Cc: Jagan Teki <jagan@amarulasolutions.com>; Stefano Babic
> > <sbabic@denx.de>; Fabio Estevam <fabio.estevam@nxp.com>; Simon Glass
> > <sjg@chromium.org>; Tom Rini <trini@konsulko.com>; Marek Vasut
> > <marek.vasut+renesas@gmail.com>; Neil Armstrong
> > <narmstrong@baylibre.com>; Philipp Tomsich
> > <philipp.tomsich@theobroma-systems.com>; Maxime Ripard
> > <maxime.ripard@bootlin.com>; Michael Trimarchi
> > <michael@amarulasolutions.com>; Andre Przywara
> > <andre.przywara@arm.com>; U-Boot-Denx <u-boot@lists.denx.de>;
> > linux-amarula at amarulasolutions.com; dl-uboot-imx <uboot-imx@nxp.com>
> > Subject: Re: [U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK
> > support
> > 
> > On Tue, 23 Apr 2019 07:47:38 +0000
> > Peng Fan <peng.fan@nxp.com> wrote:
> >   
> > > Hi Lukasz,
> > >  
> > > > -----Original Message-----
> > > > From: Lukasz Majewski [mailto:lukma at denx.de]
> > > > Sent: 2019年4月20日 6:18
> > > > To: Peng Fan <peng.fan@nxp.com>
> > > > Cc: Jagan Teki <jagan@amarulasolutions.com>; Stefano Babic
> > > > <sbabic@denx.de>; Fabio Estevam <fabio.estevam@nxp.com>; Simon  
> > Glass  
> > > > <sjg@chromium.org>; Tom Rini <trini@konsulko.com>; Marek Vasut
> > > > <marek.vasut+renesas@gmail.com>; Neil Armstrong
> > > > <narmstrong@baylibre.com>; Philipp Tomsich
> > > > <philipp.tomsich@theobroma-systems.com>; Maxime Ripard
> > > > <maxime.ripard@bootlin.com>; Michael Trimarchi
> > > > <michael@amarulasolutions.com>; Andre Przywara
> > > > <andre.przywara@arm.com>; U-Boot-Denx <u-boot@lists.denx.de>;
> > > > linux-amarula at amarulasolutions.com; dl-uboot-imx  
> > <uboot-imx@nxp.com>  
> > > > Subject: Re: [U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK
> > > > support
> > > >
> > > > On Fri, 19 Apr 2019 08:52:28 +0000
> > > > Peng Fan <peng.fan@nxp.com> wrote:
> > > >  
> > > > > > On Fri, 19 Apr 2019 11:56:25 +0530 Jagan Teki
> > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > >  
> > > > > > > On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski
> > > > > > > <lukma@denx.de> wrote:  
> > > > > > > >
> > > > > > > > Hi Jagan,
> > > > > > > >  
> > > > > > > > > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski
> > > > > > > > > <lukma@denx.de> wrote:  
> > > > > > > > > >
> > > > > > > > > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki
> > > > > > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > > > > > >  
> > > > > > > > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski
> > > > > > > > > > > <lukma@denx.de> wrote:  
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue,  2 Apr 2019 16:58:33 +0530 Jagan Teki
> > > > > > > > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > > > > > > > >  
> > > > > > > > > > > > > This is revised version of previous i.MX6
> > > > > > > > > > > > > clock management [1].
> > > > > > > > > > > > >
> > > > > > > > > > > > > The main difference between previous version
> > > > > > > > > > > > > is
> > > > > > > > > > > > > - Group the i.MX6 ccm clocks into gates and
> > > > > > > > > > > > > tree instead of handling the clocks in simple
> > > > > > > > > > > > > way using case statement.
> > > > > > > > > > > > > - use gate clocks for enable/disable
> > > > > > > > > > > > > management.
> > > > > > > > > > > > > - use tree clocks for get/set rate or parent
> > > > > > > > > > > > > traverse management.
> > > > > > > > > > > > > - parent clock handling via clock type.
> > > > > > > > > > > > > - traverse the parent clock using recursive
> > > > > > > > > > > > > functionlaity.
> > > > > > > > > > > > >
> > > > > > > > > > > > > The main motive behind this tree framework is
> > > > > > > > > > > > > to make the clock tree management simple and
> > > > > > > > > > > > > useful for U-Boot requirements instead of
> > > > > > > > > > > > > garbing Linux clock management code.
> > > > > > > > > > > > >
> > > > > > > > > > > > > We are trying to manage the Allwinner clocks
> > > > > > > > > > > > > with similar kind, so having this would
> > > > > > > > > > > > > really help i.MX6 as well.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Added simple names for clock macros, but will
> > > > > > > > > > > > > update it in future version.
> > > > > > > > > > > > >
> > > > > > > > > > > > > I have skipped ENET clocks from previous
> > > > > > > > > > > > > series, will add it in future patches.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Changes for v2:
> > > > > > > > > > > > > - changed framework patches.
> > > > > > > > > > > > > - add support for imx6qdl and imx6ul boards
> > > > > > > > > > > > > - add clock gates, tree.
> > > > > > > > > > > > >
> > > > > > > > > > > > > [1] https://patchwork.ozlabs.org/cover/950964/
> > > > > > > > > > > > >
> > > > > > > > > > > > > Any inputs?  
> > > > > > > > > > > >
> > > > > > > > > > > > Hmm.... It looks like we are doing some
> > > > > > > > > > > > development in parallel.
> > > > > > > > > > > >
> > > > > > > > > > > > Please look into following commit [1]:
> > > > > > > > > > > > https://patchwork.ozlabs.org/patch/1034051/
> > > > > > > > > > > >
> > > > > > > > > > > > It ports from Linux 5.0 the CCF framework for
> > > > > > > > > > > > iMX6Q, which IMHO in the long term is a better
> > > > > > > > > > > > approach. The code is kept simple and resembles
> > > > > > > > > > > > the code from Barebox.
> > > > > > > > > > > >
> > > > > > > > > > > > Please correct me if I'm wrong, but the code
> > > > > > > > > > > > from your work is not modeling muxes, gates and
> > > > > > > > > > > > other components from Linux CCF.  
> > > > > > > > > > >
> > > > > > > > > > > The U-Boot implementation of CLK would require as
> > > > > > > > > > > minimal and simple as possible due to requirement
> > > > > > > > > > > of U-Boot itself. Hope you agree this point?  
> > > > > > > > > >
> > > > > > > > > > Now i.MX6 is using clock.c CLK implementation. If we
> > > > > > > > > > decide to replace it - we shall do it in a way,
> > > > > > > > > > which would allow us to follow Linux kernel. (the
> > > > > > > > > > barebox implementation is a stripped CCF from
> > > > > > > > > > Linux, the same is in patch [1]).  
> > > > > > > > > > > if yes having CCF stack code to handle all clock
> > > > > > > > > > > with respective separate drivers management is
> > > > > > > > > > > may not require as of now, IMHO.  
> > > > > > > > > >
> > > > > > > > > > I do have a gut feeling, that we will end up with
> > > > > > > > > > the need to have the CCF framework ported anyway.
> > > > > > > > > > As for example imx7/8 can re-use muxes, gates
> > > > > > > > > > code.  
> > > > > > > > >
> > > > > > > > > As per my experience the main the over-ahead to handle
> > > > > > > > > clocks in U-Boot if we go with separate clock drivers
> > > > > > > > > is for Video and Ethernet peripherals. these are key
> > > > > > > > > IP's which use more clocks from U-Boot point-of-view,
> > > > > > > > > others can be handle pretty straight-forward unless
> > > > > > > > > if they don't have too much tree chain.
> > > > > > > > >
> > > > > > > > > On this series, the tree management is already
> > > > > > > > > supported ENET in i.MX6, and Allwinner platforms.
> > > > > > > > >
> > > > > > > > > As of now, I'm thinking I can handle reset of the
> > > > > > > > > clocks with similar way.  
> > > > > > > >
> > > > > > > > But this code also supports ENET and ESDHCI clocks on
> > > > > > > > i.MX6Q (as supporting those was the motivator for this
> > > > > > > > work).
> > > > > > > >
> > > > > > > > One important thing to be aware of - the problem with
> > > > > > > > SPL's footprint. The implementation with clock.c is
> > > > > > > > small and simple, but doesn't scale well.
> > > > > > > >  
> > > > > > > > >  
> > > > > > > > > >
> > > > > > > > > > However, those are only my "feelings" after a
> > > > > > > > > > glimpse look
> > > > > > > > > > - I will look into your code more thoroughly and
> > > > > > > > > > provide feedback.  
> > > > > > > > >
> > > > > > > > > Please have a look, if possible check even the code
> > > > > > > > > size by adding USDHC clocks.  
> > > > > > > >
> > > > > > > > Yes, code size (especially in SPL) is an _important_
> > > > > > > > factor here.  
> > > > > > > > >  
> > > > > > > > > >  
> > > > > > > > > > >
> > > > > > > > > > > This series is using recursive calls for handling
> > > > > > > > > > > parenting stuff to handle get or set rates, which
> > > > > > > > > > > is fine for handling clock tree management as far
> > > > > > > > > > > as U-Boot point-of-view. We have faced similar
> > > > > > > > > > > situation as I explained in commit message about
> > > > > > > > > > > Allwinner clocks [2] and we ended up going this
> > > > > > > > > > > way.  
> > > > > > > > > >
> > > > > > > > > > I'm not Allwinner expert - but if I may ask - how
> > > > > > > > > > far away is this implementation from mainline Linux
> > > > > > > > > > kernel?
> > > > > > > > > >
> > > > > > > > > > How difficult is it to port the new code (or update
> > > > > > > > > > it)?  
> > > > > > > > >
> > > > > > > > > Allwinner clocks also has similar gates, muxs, and
> > > > > > > > > with other platform stuff which has too much scope in
> > > > > > > > > Linux to use CCM.  
> > > > > > > >
> > > > > > > > For example the barebox managed to get subset of Linux
> > > > > > > > CCF ported, without loosing the CCF similarity.
> > > > > > > >
> > > > > > > >
> > > > > > > > Important factors/requirements for the i.MX clock code:
> > > > > > > >
> > > > > > > > 1. Easy maintenance in long-term
> > > > > > > >
> > > > > > > > 2. Reusing the code in SPL (with a very important
> > > > > > > > factor of _code_size_).
> > > > > > > >
> > > > > > > > 3. Reuse the code for other i.MX SoCs (imx7, imx8)
> > > > > > > >
> > > > > > > > 4. Effort needed to use DM with this code  
> > > > > > >
> > > > > > > I understand your points, I was managed this series based
> > > > > > > on these requirements as well.  
> > > > > >
> > > > > > Ok.
> > > > > >
> > > > > > Could you share the delta of footprint size (u-boot.img/SPL)
> > > > > > with and without your patch (on imx6q) ?
> > > > > >
> > > > > > In my case the CCF caused increase of u-boot.img proper (as
> > > > > > it was not yet adapted to SPL):
> > > > > >
> > > > > > 415KiB -> 421KiB = 6KiB increase of size (< 2%).
> > > > > >
> > > > > > (This can be further reduced by using OF_PLATDATA).
> > > > > >
> > > > > > This CCF code hasn't been ported to SPL (yet)
> > > > > >  
> > > > > > > We even consider the foot-print, atleast for recursive
> > > > > > > calls of handling parenting scale well.  
> > > > > >
> > > > > > With CCF porting v3 I'm going to provide some caching, so
> > > > > > the descending would be done at most once.
> > > > > >  
> > > > > > > May be we can
> > > > > > > consider to design based on this as per U-Boot.
> > > > > > >  
> > > > > >
> > > > > > Please look into point 1. Having code ported from Linux is
> > > > > > IMHO better in the long term.  
> > > > >
> > > > > Agree.
> > > > >  
> > > > > >  
> > > > > > > Let me come-back with another series or do you have any
> > > > > > > inputs or questions, please post it.  
> > > > > >
> > > > > > I will post CCF port for imx6q v3 in a few days.  
> > > > >
> > > > > Looking forward your new patchset.
> > > > > Working on enabling i.MX8MM CCF support.  
> > > >
> > > > Output of 'dm tree' on imx6q:
> > > >
> > > >  clk          1  [   ]   fixed_rate_clock      |-- ckil
> > > >  clk          2  [   ]   fixed_rate_clock      |-- ckih1
> > > >  clk          3  [ + ]   fixed_rate_clock      `-- osc
> > > >  clk          4  [ + ]   imx_clk_pllv3             |-- pll2_bus
> > > >  clk          7  [   ]   imx_clk_pfd               |   |--
> > > > pll2_pfd0_352m
> > > >  clk          8  [ + ]   imx_clk_pfd               |   `--
> > > > pll2_pfd2_396m
> > > >  clk          9  [ + ]   imx_clk_mux               |       |--
> > > > usdhc1_sel
> > > >  clk         13  [ + ]   imx_clk_divider           |       |  
> > `--  
> > > > usdhc1_podf
> > > >  clk         22  [   ]   imx_clk_gate2             |       |
> > > > `-- usdhc1
> > > >  clk         10  [ + ]   imx_clk_mux               |       |--
> > > > usdhc2_sel
> > > >  clk         14  [ + ]   imx_clk_divider           |       |  
> > `--  
> > > > usdhc2_podf
> > > >  clk         23  [   ]   imx_clk_gate2             |       |
> > > > `-- usdhc2
> > > >  clk         11  [ + ]   imx_clk_mux               |       |--
> > > > usdhc3_sel
> > > >  clk         15  [ + ]   imx_clk_divider           |       |  
> > `--  
> > > > usdhc3_podf
> > > >  clk         24  [   ]   imx_clk_gate2             |       |
> > > > `-- usdhc3
> > > >  clk         12  [ + ]   imx_clk_mux               |       `--
> > > > usdhc4_sel
> > > >  clk         16  [ + ]   imx_clk_divider           |  
> > `--  
> > > > usdhc4_podf
> > > >  clk         25  [   ]   imx_clk_gate2             |
> > > > `-- usdhc4
> > > >  clk          5  [ + ]   imx_clk_pllv3             `--
> > > > pll3_usb_otg clk          6  [ + ]
> > > > imx_clk_fixed_factor          `-- pll3_60m clk         17  [ +
> > > > ]   imx_clk_divider                   `-- ecspi_root
> > > >  clk         18  [   ]   imx_clk_gate2  
> > |--  
> > > > ecspi1
> > > >  clk         19  [   ]   imx_clk_gate2  
> > |--  
> > > > ecspi2
> > > >  clk         20  [   ]   imx_clk_gate2  
> > |--  
> > > > ecspi3
> > > >  clk         21  [   ]   imx_clk_gate2  
> > `--  
> > > > ecspi4
> > > >  
> > >
> > > Do you have a public tree/branch for CCF?  
> > 
> > Please find the CCF devel work:
> > https://github.com/lmajewski/u-boot-dfu/commits/CCF-devel
> > 
> > The CCF starts from: c792297e1a47ead02ff2baa4f162de8782b29910  
> 
> Thanks.
> 
> > 
> > (below you will find imx6q DM/DTS conversion code).
> > 
> > What is added when compared to the original one:
> > 
> > - SPL support
> > 
> > - Some fixes for v2019.04+
> > 
> > What is on the TO DO list:
> > 
> > - OF_PLATDATA for SPL (as I did not used any optimisations yet).
> > 
> >   
> > >I am adding imx8mm clk and
> > > would like to base on your tree. I think need to extend clk_ops to
> > >support mux/divider, but not just get rate.  
> > 
> > Some mux/divider is provided (clk-mux.c / clk-divider.c)  
> 
> After reading into details, there is no clk core logic.
> mux not support reparenting.
> Divider not support rate setting.
> no composite clk support.

I've stripped out the code, which I couldn't test/needed on imx6q.

This version only has the "must have" for imx6q (including SPL) with
some generic code for clock framework in u-boot.

> We could not directly reuse Linux vendor clk driver,
> need adapt to
> U-Boot, 

Yes, you would need to adapt the code (as it was done in barebox and
this patch set).

> should not be hard from your code.

I wanted to keep the structure of the code untouched from Linux.
However, for example, we can't afford in imx6q the
clk[IMX6QDL_<clock_name>] table - it is simply too big for SPL.

> 
> When multiple devices sources from one PLL, one device might would
> like to set pll value that break other devices, there is no logic to
> protect if support freq changing.

Ok, so in your use-case you need the frequency re-calc code? (Which
changes - updates - the clocks up in the tree when root PLL is
changed) ?

No, it was not added.

> 
> Do you have plan work on the upper items?

For the imx6q use case - in which I need some muxing and gating only - I
do not need to recalc/reparent the frequency.

The CCF porting patch only brought:

- the smallest subset of CCF functions for my use case

- directory (and source files) structure from Linux

- some clock uclass enhancement for u-boot

- the clock generic (register) code (clk/clk.c)


The plan was to allow others to port their subset of code when needed.
In that way we only get the code which is really used and tested.

> i.MX8MM use composite heavily in Linux, so I have started.

Ok. I see. Your patches/review is more than welcome.

> 
> Thanks,
> Peng.
> 
> >   
> > > To avoid conflict with
> > > you work, if you have a public tree, that could be good.  
> > 
> > No problem. Thanks for the interest.
> >   
> > >
> > > Thanks,
> > > Peng.
> > >
> > >  
> > > >
> > > >  
> > > > >
> > > > > Regards,
> > > > > Peng.
> > > > >  
> > > > > >  
> > > > > > >
> > > > > > > Jagan.  
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > Best regards,
> > > > > >
> > > > > > Lukasz Majewski
> > > > > >
> > > > > > --
> > > > > >
> > > > > > DENX Software Engineering GmbH,      Managing Director:  
> > Wolfgang  
> > > > > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194
> > > > > > Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax:
> > > > > > (+49)-8142-66989-80 Email: lukma at denx.de  
> > > >
> > > >
> > > >
> > > >
> > > > Best regards,
> > > >
> > > > Lukasz Majewski
> > > >
> > > > --
> > > >
> > > > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194
> > > > Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax:
> > > > (+49)-8142-66989-80 Email: lukma at denx.de  
> > 
> > 
> > 
> > 
> > Best regards,
> > 
> > Lukasz Majewski
> > 
> > --
> > 
> > DENX Software Engineering GmbH,      Managing Director: Wolfgang
> > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell,
> > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email:
> > lukma at denx.de  




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190423/6c7dd406/attachment.sig>

[U-Boot] [PATCH v2 00/10] clk: imx: Add i.MX6 CLK support

[Tom Rini]

On Fri, Apr 19, 2019 at 08:52:28AM +0000, Peng Fan wrote:
> 
> > On Fri, 19 Apr 2019 11:56:25 +0530
> > Jagan Teki <jagan@amarulasolutions.com> wrote:
> > 
> > > On Fri, Apr 5, 2019 at 2:20 AM Lukasz Majewski <lukma@denx.de> wrote:
> > > >
> > > > Hi Jagan,
> > > >
> > > > > On Thu, Apr 4, 2019 at 3:31 PM Lukasz Majewski <lukma@denx.de>
> > > > > wrote:
> > > > > >
> > > > > > On Thu, 4 Apr 2019 14:56:36 +0530 Jagan Teki
> > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > >
> > > > > > > On Thu, Apr 4, 2019 at 2:31 PM Lukasz Majewski <lukma@denx.de>
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Tue,  2 Apr 2019 16:58:33 +0530 Jagan Teki
> > > > > > > > <jagan@amarulasolutions.com> wrote:
> > > > > > > >
> > > > > > > > > This is revised version of previous i.MX6 clock management
> > > > > > > > > [1].
> > > > > > > > >
> > > > > > > > > The main difference between previous version is
> > > > > > > > > - Group the i.MX6 ccm clocks into gates and tree instead
> > > > > > > > > of handling the clocks in simple way using case statement.
> > > > > > > > > - use gate clocks for enable/disable management.
> > > > > > > > > - use tree clocks for get/set rate or parent traverse
> > > > > > > > > management.
> > > > > > > > > - parent clock handling via clock type.
> > > > > > > > > - traverse the parent clock using recursive functionlaity.
> > > > > > > > >
> > > > > > > > > The main motive behind this tree framework is to make the
> > > > > > > > > clock tree management simple and useful for U-Boot
> > > > > > > > > requirements instead of garbing Linux clock management
> > > > > > > > > code.
> > > > > > > > >
> > > > > > > > > We are trying to manage the Allwinner clocks with similar
> > > > > > > > > kind, so having this would really help i.MX6 as well.
> > > > > > > > >
> > > > > > > > > Added simple names for clock macros, but will update it in
> > > > > > > > > future version.
> > > > > > > > >
> > > > > > > > > I have skipped ENET clocks from previous series, will add
> > > > > > > > > it in future patches.
> > > > > > > > >
> > > > > > > > > Changes for v2:
> > > > > > > > > - changed framework patches.
> > > > > > > > > - add support for imx6qdl and imx6ul boards
> > > > > > > > > - add clock gates, tree.
> > > > > > > > >
> > > > > > > > > [1] https://patchwork.ozlabs.org/cover/950964/
> > > > > > > > >
> > > > > > > > > Any inputs?
> > > > > > > >
> > > > > > > > Hmm.... It looks like we are doing some development in
> > > > > > > > parallel.
> > > > > > > >
> > > > > > > > Please look into following commit [1]:
> > > > > > > > https://patchwork.ozlabs.org/patch/1034051/
> > > > > > > >
> > > > > > > > It ports from Linux 5.0 the CCF framework for iMX6Q, which
> > > > > > > > IMHO in the long term is a better approach.
> > > > > > > > The code is kept simple and resembles the code from Barebox.
> > > > > > > >
> > > > > > > > Please correct me if I'm wrong, but the code from your work
> > > > > > > > is not modeling muxes, gates and other components from Linux
> > > > > > > > CCF.
> > > > > > >
> > > > > > > The U-Boot implementation of CLK would require as minimal and
> > > > > > > simple as possible due to requirement of U-Boot itself. Hope
> > > > > > > you agree this point?
> > > > > >
> > > > > > Now i.MX6 is using clock.c CLK implementation. If we decide to
> > > > > > replace it - we shall do it in a way, which would allow us to
> > > > > > follow Linux kernel. (the barebox implementation is a stripped
> > > > > > CCF from Linux, the same is in patch [1]).
> > > > > >
> > > > > > > if yes having CCF stack code to handle all clock with
> > > > > > > respective separate drivers management is may not require as
> > > > > > > of now, IMHO.
> > > > > >
> > > > > > I do have a gut feeling, that we will end up with the need to
> > > > > > have the CCF framework ported anyway. As for example imx7/8 can
> > > > > > re-use muxes, gates code.
> > > > >
> > > > > As per my experience the main the over-ahead to handle clocks in
> > > > > U-Boot if we go with separate clock drivers is for Video and
> > > > > Ethernet peripherals. these are key IP's which use more clocks
> > > > > from U-Boot point-of-view, others can be handle pretty
> > > > > straight-forward unless if they don't have too much tree chain.
> > > > >
> > > > > On this series, the tree management is already supported ENET in
> > > > > i.MX6, and Allwinner platforms.
> > > > >
> > > > > As of now, I'm thinking I can handle reset of the clocks with
> > > > > similar way.
> > > >
> > > > But this code also supports ENET and ESDHCI clocks on i.MX6Q (as
> > > > supporting those was the motivator for this work).
> > > >
> > > > One important thing to be aware of - the problem with SPL's
> > > > footprint. The implementation with clock.c is small and simple, but
> > > > doesn't scale well.
> > > >
> > > > >
> > > > > >
> > > > > > However, those are only my "feelings" after a glimpse look - I
> > > > > > will look into your code more thoroughly and provide feedback.
> > > > >
> > > > > Please have a look, if possible check even the code size by adding
> > > > > USDHC clocks.
> > > >
> > > > Yes, code size (especially in SPL) is an _important_ factor here.
> > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > This series is using recursive calls for handling parenting
> > > > > > > stuff to handle get or set rates, which is fine for handling
> > > > > > > clock tree management as far as U-Boot point-of-view. We have
> > > > > > > faced similar situation as I explained in commit message about
> > > > > > > Allwinner clocks [2] and we ended up going this way.
> > > > > >
> > > > > > I'm not Allwinner expert - but if I may ask - how far away is
> > > > > > this implementation from mainline Linux kernel?
> > > > > >
> > > > > > How difficult is it to port the new code (or update it)?
> > > > >
> > > > > Allwinner clocks also has similar gates, muxs, and with other
> > > > > platform stuff which has too much scope in Linux to use CCM.
> > > >
> > > > For example the barebox managed to get subset of Linux CCF ported,
> > > > without loosing the CCF similarity.
> > > >
> > > >
> > > > Important factors/requirements for the i.MX clock code:
> > > >
> > > > 1. Easy maintenance in long-term
> > > >
> > > > 2. Reusing the code in SPL (with a very important factor of
> > > > _code_size_).
> > > >
> > > > 3. Reuse the code for other i.MX SoCs (imx7, imx8)
> > > >
> > > > 4. Effort needed to use DM with this code
> > >
> > > I understand your points, I was managed this series based on these
> > > requirements as well.
> > 
> > Ok.
> > 
> > Could you share the delta of footprint size (u-boot.img/SPL) with and without
> > your patch (on imx6q) ?
> > 
> > In my case the CCF caused increase of u-boot.img proper (as it was not yet
> > adapted to SPL):
> > 
> > 415KiB -> 421KiB = 6KiB increase of size (< 2%).
> > 
> > (This can be further reduced by using OF_PLATDATA).
> > 
> > This CCF code hasn't been ported to SPL (yet)
> > 
> > > We even consider the foot-print, atleast for recursive calls of
> > > handling parenting scale well.
> > 
> > With CCF porting v3 I'm going to provide some caching, so the descending
> > would be done at most once.
> > 
> > > May be we can
> > > consider to design based on this as per U-Boot.
> > >
> > 
> > Please look into point 1. Having code ported from Linux is IMHO better in the
> > long term.
> 
> Agree.

I just want to re-iterate my support again here that we should be
looking at adapting and stripping down frameworks from the kernel.  They
are:
- Familiar to a large subset of our developers as most folks also work
  on the Linux kernel.
- Already (generally) well designed to take into account the various
  designs of vastly different SoCs that we also want to support.
- Likely to already be fed by device tree and we can just leverage
  what's already in the dts* files.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20190422/b0d88689/attachment.sig>