[PATCH v6 0/9] Fixes and new clocks support for Mediatek MT8173

[PATCH v6 0/9] Fixes and new clocks support for Mediatek MT8173

[James Liao]

This patchset is based on 4.2-rc2 and [1], and contains minor fixes and
subsystem clocks support for Mediatek MT8173.

The previous reviews can be found in [2]. The most different from previous
patchset is rebasing to 4.2-rc2.

changes since v5:
- Rebase to 4.2-rc2.
- Patch "Fix enabling of critical clocks" had been included in 4.2-rc2,
  so it need not be included in this patchset.

changes since v4:
- Add a separated patch to remove unused dpi_ck.
- Add separated patches to fix clk/mediatek in release 4.2-rc1.
- Add __init* for initialization data and functions.
- Rename dummy_clk with oscillator in DT.
- Remove mtk_clk_register_apmixed_ex(), call mtk_clk_register_ref2usb_tx()
  directly to register USB clock.

changes since v3:
- Remove clk_null dependency from root clocks.
- Use fixed-rate clocks with typical rate to replace clk_null.
- Separate ref2usb_tx implementation to clk-apmixed.c
- Use clock-controller as the name of clock providers.

changes since v2:
- Rebase to 4.2-rc1.
- Add device tree nodes for subsystem clocks.
- Fine tune comments of patches.
- Add __init, __initconst and const to init data and functions.
- Removed unused code from init functions of subsystem clocks.

changes since v1:
- Add CA7PLL and CA15PLL as critical clocks.
- Use the same register descriptor for imgsys, vensys and vencltsys.
- Generalize apmixedsys special clocks registration.

[1] https://patchwork.kernel.org/patch/6446341/
[2] http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001800.html

James Liao (9):
  clk: mediatek: Removed unused dpi_ck clock from MT8173
  clk: mediatek: Remove unused code from MT8173.
  clk: mediatek: Add __initdata and __init for data and functions
  clk: mediatek: Add fixed clocks support for Mediatek SoC.
  clk: mediatek: Fix rate and dependency of MT8173 clocks
  dt-bindings: ARM: Mediatek: Document devicetree bindings for clock
    controllers
  clk: mediatek: Add subsystem clocks of MT8173
  clk: mediatek: Add USB clock support in MT8173 APMIXEDSYS
  arm64: dts: mt8173: Add subsystem clock controller device nodes

 .../bindings/arm/mediatek/mediatek,imgsys.txt      |  22 ++
 .../bindings/arm/mediatek/mediatek,mmsys.txt       |  22 ++
 .../bindings/arm/mediatek/mediatek,vdecsys.txt     |  22 ++
 .../bindings/arm/mediatek/mediatek,vencltsys.txt   |  22 ++
 .../bindings/arm/mediatek/mediatek,vencsys.txt     |  22 ++
 arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  37 +++
 drivers/clk/mediatek/Makefile                      |   2 +-
 drivers/clk/mediatek/clk-apmixed.c                 | 107 +++++++
 drivers/clk/mediatek/clk-gate.c                    |   2 +-
 drivers/clk/mediatek/clk-mt8173.c                  | 335 ++++++++++++++++++++-
 drivers/clk/mediatek/clk-mtk.c                     |  36 ++-
 drivers/clk/mediatek/clk-mtk.h                     |  24 +-
 drivers/clk/mediatek/clk-pll.c                     |   7 +-
 include/dt-bindings/clock/mt8173-clk.h             | 101 ++++++-
 14 files changed, 728 insertions(+), 33 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,imgsys.txt
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt
 create mode 100644 drivers/clk/mediatek/clk-apmixed.c

--
1.8.1.1.dirty

[PATCH v6 1/9] clk: mediatek: Removed unused dpi_ck clock from MT8173

[James Liao]

The dpi_ck clock can be removed because it not actually used
in topckgen and subsystems.

Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
---
 drivers/clk/mediatek/clk-mt8173.c      | 1 -
 include/dt-bindings/clock/mt8173-clk.h | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 8b6523d..80871e2 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -26,7 +26,6 @@ static DEFINE_SPINLOCK(mt8173_clk_lock);
 
 static const struct mtk_fixed_factor root_clk_alias[] __initconst = {
 	FACTOR(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk_null", 1, 1),
-	FACTOR(CLK_TOP_DPI, "dpi_ck", "clk_null", 1, 1),
 	FACTOR(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk_null", 1, 1),
 	FACTOR(CLK_TOP_HDMITX_DIG_CTS, "hdmitx_dig_cts", "clk_null", 1, 1),
 };
diff --git a/include/dt-bindings/clock/mt8173-clk.h b/include/dt-bindings/clock/mt8173-clk.h
index 4ad76ed..7230c38 100644
--- a/include/dt-bindings/clock/mt8173-clk.h
+++ b/include/dt-bindings/clock/mt8173-clk.h
@@ -18,7 +18,6 @@
 /* TOPCKGEN */
 
 #define CLK_TOP_CLKPH_MCK_O		1
-#define CLK_TOP_DPI			2
 #define CLK_TOP_USB_SYSPLL_125M		3
 #define CLK_TOP_HDMITX_DIG_CTS		4
 #define CLK_TOP_ARMCA7PLL_754M		5
-- 
1.8.1.1.dirty

[PATCH v6 2/9] clk: mediatek: Remove unused code from MT8173.

[James Liao]

Remove unused header files from MT8173, and remove unused
keywords from function declaration.

Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
---
 drivers/clk/mediatek/clk-mt8173.c | 2 --
 drivers/clk/mediatek/clk-mtk.h    | 4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 80871e2..13aa432 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -14,8 +14,6 @@
 
 #include <linux/of.h>
 #include <linux/of_address.h>
-#include <linux/slab.h>
-#include <linux/mfd/syscon.h>
 
 #include "clk-mtk.h"
 #include "clk-gate.h"
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index 9dda9d8..740fe5e 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -41,7 +41,7 @@ struct mtk_fixed_factor {
 		.div = _div,				\
 	}
 
-extern void mtk_clk_register_factors(const struct mtk_fixed_factor *clks,
+void mtk_clk_register_factors(const struct mtk_fixed_factor *clks,
 		int num, struct clk_onecell_data *clk_data);
 
 struct mtk_composite {
@@ -152,7 +152,7 @@ struct mtk_pll_data {
 	int pcw_shift;
 };
 
-void __init mtk_clk_register_plls(struct device_node *node,
+void mtk_clk_register_plls(struct device_node *node,
 		const struct mtk_pll_data *plls, int num_plls,
 		struct clk_onecell_data *clk_data);
 
-- 
1.8.1.1.dirty

[PATCH v6 3/9] clk: mediatek: Add __initdata and __init for data and functions

[James Liao]

Add __init for clock registration functions, and add __initdata for
mtk_gate_regs initial structures.

Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
---
 drivers/clk/mediatek/clk-gate.c   |  2 +-
 drivers/clk/mediatek/clk-mt8173.c |  6 +++---
 drivers/clk/mediatek/clk-mtk.c    | 13 +++++++------
 3 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/mediatek/clk-gate.c b/drivers/clk/mediatek/clk-gate.c
index 5702036..576bdb7 100644
--- a/drivers/clk/mediatek/clk-gate.c
+++ b/drivers/clk/mediatek/clk-gate.c
@@ -97,7 +97,7 @@ const struct clk_ops mtk_clk_gate_ops_setclr_inv = {
 	.disable	= mtk_cg_disable_inv,
 };
 
-struct clk *mtk_clk_register_gate(
+struct clk * __init mtk_clk_register_gate(
 		const char *name,
 		const char *parent_name,
 		struct regmap *regmap,
diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 13aa432..361fe32 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -586,7 +586,7 @@ static const struct mtk_composite top_muxes[] __initconst = {
 	MUX(CLK_TOP_I2S3_B_SEL, "i2s3_b_ck_sel", i2s3_b_ck_parents, 0x120, 8, 1),
 };
 
-static const struct mtk_gate_regs infra_cg_regs = {
+static const struct mtk_gate_regs infra_cg_regs __initconst = {
 	.set_ofs = 0x0040,
 	.clr_ofs = 0x0044,
 	.sta_ofs = 0x0048,
@@ -615,13 +615,13 @@ static const struct mtk_gate infra_clks[] __initconst = {
 	GATE_ICG(CLK_INFRA_PMICWRAP, "infra_pmicwrap", "axi_sel", 23),
 };
 
-static const struct mtk_gate_regs peri0_cg_regs = {
+static const struct mtk_gate_regs peri0_cg_regs __initconst = {
 	.set_ofs = 0x0008,
 	.clr_ofs = 0x0010,
 	.sta_ofs = 0x0018,
 };
 
-static const struct mtk_gate_regs peri1_cg_regs = {
+static const struct mtk_gate_regs peri1_cg_regs __initconst = {
 	.set_ofs = 0x000c,
 	.clr_ofs = 0x0014,
 	.sta_ofs = 0x001c,
diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 18444ae..268b6ff 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -24,7 +24,7 @@
 #include "clk-mtk.h"
 #include "clk-gate.h"
 
-struct clk_onecell_data *mtk_alloc_clk_data(unsigned int clk_num)
+struct clk_onecell_data * __init mtk_alloc_clk_data(unsigned int clk_num)
 {
 	int i;
 	struct clk_onecell_data *clk_data;
@@ -49,8 +49,8 @@ err_out:
 	return NULL;
 }
 
-void mtk_clk_register_factors(const struct mtk_fixed_factor *clks, int num,
-		struct clk_onecell_data *clk_data)
+void __init mtk_clk_register_factors(const struct mtk_fixed_factor *clks,
+		int num, struct clk_onecell_data *clk_data)
 {
 	int i;
 	struct clk *clk;
@@ -72,7 +72,8 @@ void mtk_clk_register_factors(const struct mtk_fixed_factor *clks, int num,
 	}
 }
 
-int mtk_clk_register_gates(struct device_node *node, const struct mtk_gate *clks,
+int __init mtk_clk_register_gates(struct device_node *node,
+		const struct mtk_gate *clks,
 		int num, struct clk_onecell_data *clk_data)
 {
 	int i;
@@ -111,7 +112,7 @@ int mtk_clk_register_gates(struct device_node *node, const struct mtk_gate *clks
 	return 0;
 }
 
-struct clk *mtk_clk_register_composite(const struct mtk_composite *mc,
+struct clk * __init mtk_clk_register_composite(const struct mtk_composite *mc,
 		void __iomem *base, spinlock_t *lock)
 {
 	struct clk *clk;
@@ -196,7 +197,7 @@ err_out:
 	return ERR_PTR(ret);
 }
 
-void mtk_clk_register_composites(const struct mtk_composite *mcs,
+void __init mtk_clk_register_composites(const struct mtk_composite *mcs,
 		int num, void __iomem *base, spinlock_t *lock,
 		struct clk_onecell_data *clk_data)
 {
-- 
1.8.1.1.dirty

[PATCH v6 4/9] clk: mediatek: Add fixed clocks support for Mediatek SoC.

[James Liao]

This patch adds fixed clocks support by using CCF fixed-rate
clock implementation.

Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
---
 drivers/clk/mediatek/clk-mtk.c | 23 +++++++++++++++++++++++
 drivers/clk/mediatek/clk-mtk.h | 17 +++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 268b6ff..cf08db6 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -49,6 +49,29 @@ err_out:
 	return NULL;
 }
 
+void __init mtk_clk_register_fixed_clks(const struct mtk_fixed_clk *clks,
+		int num, struct clk_onecell_data *clk_data)
+{
+	int i;
+	struct clk *clk;
+
+	for (i = 0; i < num; i++) {
+		const struct mtk_fixed_clk *rc = &clks[i];
+
+		clk = clk_register_fixed_rate(NULL, rc->name, rc->parent,
+				rc->parent ? 0 : CLK_IS_ROOT, rc->rate);
+
+		if (IS_ERR(clk)) {
+			pr_err("Failed to register clk %s: %ld\n",
+					rc->name, PTR_ERR(clk));
+			continue;
+		}
+
+		if (clk_data)
+			clk_data->clks[rc->id] = clk;
+	}
+}
+
 void __init mtk_clk_register_factors(const struct mtk_fixed_factor *clks,
 		int num, struct clk_onecell_data *clk_data)
 {
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index 740fe5e..32d4f55 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -25,6 +25,23 @@
 
 #define MHZ (1000 * 1000)
 
+struct mtk_fixed_clk {
+	int id;
+	const char *name;
+	const char *parent;
+	unsigned long rate;
+};
+
+#define FIXED_CLK(_id, _name, _parent, _rate) {		\
+		.id = _id,				\
+		.name = _name,				\
+		.parent = _parent,			\
+		.rate = _rate,				\
+	}
+
+void mtk_clk_register_fixed_clks(const struct mtk_fixed_clk *clks,
+		int num, struct clk_onecell_data *clk_data);
+
 struct mtk_fixed_factor {
 	int id;
 	const char *name;
-- 
1.8.1.1.dirty

[PATCH v6 5/9] clk: mediatek: Fix rate and dependency of MT8173 clocks

[James Liao]

Remove the dependency from clk_null, and give all root clocks a
typical rate, include clkph_mck_o, usb_syspll_125m and hdmitx_dig_cts.

dpi_ck was removed due to no clock reference to it.

Replace parent clock of infra_cpum with cpum_ck, which is an external
clock and can be defined in the device tree.

Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
---
 drivers/clk/mediatek/clk-mt8173.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 361fe32..f37ace6 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -22,10 +22,9 @@
 
 static DEFINE_SPINLOCK(mt8173_clk_lock);
 
-static const struct mtk_fixed_factor root_clk_alias[] __initconst = {
-	FACTOR(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk_null", 1, 1),
-	FACTOR(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk_null", 1, 1),
-	FACTOR(CLK_TOP_HDMITX_DIG_CTS, "hdmitx_dig_cts", "clk_null", 1, 1),
+static const struct mtk_fixed_clk fixed_clks[] __initconst = {
+	FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
+	FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
 };
 
 static const struct mtk_fixed_factor top_divs[] __initconst = {
@@ -50,6 +49,7 @@ static const struct mtk_fixed_factor top_divs[] __initconst = {
 	FACTOR(CLK_TOP_CLKRTC_INT, "clkrtc_int", "clk26m", 1, 793),
 	FACTOR(CLK_TOP_FPC, "fpc_ck", "clk26m", 1, 1),
 
+	FACTOR(CLK_TOP_HDMITX_DIG_CTS, "hdmitx_dig_cts", "tvdpll_445p5m", 1, 3),
 	FACTOR(CLK_TOP_HDMITXPLL_D2, "hdmitxpll_d2", "hdmitx_dig_cts", 1, 2),
 	FACTOR(CLK_TOP_HDMITXPLL_D3, "hdmitxpll_d3", "hdmitx_dig_cts", 1, 3),
 
@@ -608,7 +608,7 @@ static const struct mtk_gate infra_clks[] __initconst = {
 	GATE_ICG(CLK_INFRA_GCE, "infra_gce", "axi_sel", 6),
 	GATE_ICG(CLK_INFRA_L2C_SRAM, "infra_l2c_sram", "axi_sel", 7),
 	GATE_ICG(CLK_INFRA_M4U, "infra_m4u", "mem_sel", 8),
-	GATE_ICG(CLK_INFRA_CPUM, "infra_cpum", "clk_null", 15),
+	GATE_ICG(CLK_INFRA_CPUM, "infra_cpum", "cpum_ck", 15),
 	GATE_ICG(CLK_INFRA_KP, "infra_kp", "axi_sel", 16),
 	GATE_ICG(CLK_INFRA_CEC, "infra_cec", "clk26m", 18),
 	GATE_ICG(CLK_INFRA_PMICSPI, "infra_pmicspi", "pmicspi_sel", 22),
@@ -727,7 +727,7 @@ static void __init mtk_topckgen_init(struct device_node *node)
 
 	mt8173_top_clk_data = clk_data = mtk_alloc_clk_data(CLK_TOP_NR_CLK);
 
-	mtk_clk_register_factors(root_clk_alias, ARRAY_SIZE(root_clk_alias), clk_data);
+	mtk_clk_register_fixed_clks(fixed_clks, ARRAY_SIZE(fixed_clks), clk_data);
 	mtk_clk_register_factors(top_divs, ARRAY_SIZE(top_divs), clk_data);
 	mtk_clk_register_composites(top_muxes, ARRAY_SIZE(top_muxes), base,
 			&mt8173_clk_lock, clk_data);
-- 
1.8.1.1.dirty

[PATCH v6 5/9] clk: mediatek: Fix rate and dependency of MT8173 clocks

[Sascha Hauer]

On Tue, Aug 04, 2015 at 04:16:54PM +0800, James Liao wrote:
> Remove the dependency from clk_null, and give all root clocks a
> typical rate, include clkph_mck_o, usb_syspll_125m and hdmitx_dig_cts.
> 
> dpi_ck was removed due to no clock reference to it.
> 
> Replace parent clock of infra_cpum with cpum_ck, which is an external
> clock and can be defined in the device tree.
> 
> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
> ---
>  drivers/clk/mediatek/clk-mt8173.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
> index 361fe32..f37ace6 100644
> --- a/drivers/clk/mediatek/clk-mt8173.c
> +++ b/drivers/clk/mediatek/clk-mt8173.c
> @@ -22,10 +22,9 @@
>  
>  static DEFINE_SPINLOCK(mt8173_clk_lock);
>  
> -static const struct mtk_fixed_factor root_clk_alias[] __initconst = {
> -	FACTOR(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk_null", 1, 1),
> -	FACTOR(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk_null", 1, 1),
> -	FACTOR(CLK_TOP_HDMITX_DIG_CTS, "hdmitx_dig_cts", "clk_null", 1, 1),
> +static const struct mtk_fixed_clk fixed_clks[] __initconst = {
> +	FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
> +	FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),

Hm, it seems you hide PLLs in fixed factor clock. Are you sure that
there is a PLL in the system generating 125MHz from 26MHz which is in no
way configurable? Or is this really some clock derived from the syspll
as the clock name suggests?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v6 5/9] clk: mediatek: Fix rate and dependency of MT8173 clocks

[James Liao]

Hi Sascha,

On Wed, 2015-08-05 at 08:53 +0200, Sascha Hauer wrote:
> On Tue, Aug 04, 2015 at 04:16:54PM +0800, James Liao wrote:
> > -static const struct mtk_fixed_factor root_clk_alias[] __initconst = {
> > -	FACTOR(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk_null", 1, 1),
> > -	FACTOR(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk_null", 1, 1),
> > -	FACTOR(CLK_TOP_HDMITX_DIG_CTS, "hdmitx_dig_cts", "clk_null", 1, 1),
> > +static const struct mtk_fixed_clk fixed_clks[] __initconst = {
> > +	FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
> > +	FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
> 
> Hm, it seems you hide PLLs in fixed factor clock. Are you sure that
> there is a PLL in the system generating 125MHz from 26MHz which is in no
> way configurable? Or is this really some clock derived from the syspll
> as the clock name suggests?

According to the datasheet from our clock designer, usb_syspll_125m is
the output clock of an analog macro which is named SSUSB_PHY, and its
input clock is AD_CLK26M_CK.

SSUSB_PHY is not the same as general PLLs such as MAINPLL. So I don't
treat it as a configurable PLL but a fixed clock with the typical rate
125 MHz.


Best regards,

James

[PATCH v6 5/9] clk: mediatek: Fix rate and dependency of MT8173 clocks

[Sascha Hauer]

On Thu, Aug 06, 2015 at 04:35:32PM +0800, James Liao wrote:
> Hi Sascha,
> 
> On Wed, 2015-08-05 at 08:53 +0200, Sascha Hauer wrote:
> > On Tue, Aug 04, 2015 at 04:16:54PM +0800, James Liao wrote:
> > > -static const struct mtk_fixed_factor root_clk_alias[] __initconst = {
> > > -	FACTOR(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk_null", 1, 1),
> > > -	FACTOR(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk_null", 1, 1),
> > > -	FACTOR(CLK_TOP_HDMITX_DIG_CTS, "hdmitx_dig_cts", "clk_null", 1, 1),
> > > +static const struct mtk_fixed_clk fixed_clks[] __initconst = {
> > > +	FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
> > > +	FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
> > 
> > Hm, it seems you hide PLLs in fixed factor clock. Are you sure that
> > there is a PLL in the system generating 125MHz from 26MHz which is in no
> > way configurable? Or is this really some clock derived from the syspll
> > as the clock name suggests?
> 
> According to the datasheet from our clock designer, usb_syspll_125m is
> the output clock of an analog macro which is named SSUSB_PHY, and its
> input clock is AD_CLK26M_CK.
> 
> SSUSB_PHY is not the same as general PLLs such as MAINPLL. So I don't
> treat it as a configurable PLL but a fixed clock with the typical rate
> 125 MHz.

Ok then.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v6 6/9] dt-bindings: ARM: Mediatek: Document devicetree bindings for clock controllers

[James Liao]

This adds the binding documentation for the mmsys, imgsys, vdecsys,
vencsys and vencltsys controllers found on Mediatek SoCs.

Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
---
 .../bindings/arm/mediatek/mediatek,imgsys.txt      | 22 ++++++++++++++++++++++
 .../bindings/arm/mediatek/mediatek,mmsys.txt       | 22 ++++++++++++++++++++++
 .../bindings/arm/mediatek/mediatek,vdecsys.txt     | 22 ++++++++++++++++++++++
 .../bindings/arm/mediatek/mediatek,vencltsys.txt   | 22 ++++++++++++++++++++++
 .../bindings/arm/mediatek/mediatek,vencsys.txt     | 22 ++++++++++++++++++++++
 5 files changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,imgsys.txt
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
 create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt

diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,imgsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imgsys.txt
new file mode 100644
index 0000000..b1f2ce1
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,imgsys.txt
@@ -0,0 +1,22 @@
+Mediatek imgsys controller
+============================
+
+The Mediatek imgsys controller provides various clocks to the system.
+
+Required Properties:
+
+- compatible: Should be:
+	- "mediatek,mt8173-imgsys", "syscon"
+- #clock-cells: Must be 1
+
+The imgsys controller uses the common clk binding from
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+The available clocks are defined in dt-bindings/clock/mt*-clk.h.
+
+Example:
+
+imgsys: clock-controller at 15000000 {
+	compatible = "mediatek,mt8173-imgsys", "syscon";
+	reg = <0 0x15000000 0 0x1000>;
+	#clock-cells = <1>;
+};
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
new file mode 100644
index 0000000..4385946
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
@@ -0,0 +1,22 @@
+Mediatek mmsys controller
+============================
+
+The Mediatek mmsys controller provides various clocks to the system.
+
+Required Properties:
+
+- compatible: Should be:
+	- "mediatek,mt8173-mmsys", "syscon"
+- #clock-cells: Must be 1
+
+The mmsys controller uses the common clk binding from
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+The available clocks are defined in dt-bindings/clock/mt*-clk.h.
+
+Example:
+
+mmsys: clock-controller at 14000000 {
+	compatible = "mediatek,mt8173-mmsys", "syscon";
+	reg = <0 0x14000000 0 0x1000>;
+	#clock-cells = <1>;
+};
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
new file mode 100644
index 0000000..1faacf1
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
@@ -0,0 +1,22 @@
+Mediatek vdecsys controller
+============================
+
+The Mediatek vdecsys controller provides various clocks to the system.
+
+Required Properties:
+
+- compatible: Should be:
+	- "mediatek,mt8173-vdecsys", "syscon"
+- #clock-cells: Must be 1
+
+The vdecsys controller uses the common clk binding from
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+The available clocks are defined in dt-bindings/clock/mt*-clk.h.
+
+Example:
+
+vdecsys: clock-controller at 16000000 {
+	compatible = "mediatek,mt8173-vdecsys", "syscon";
+	reg = <0 0x16000000 0 0x1000>;
+	#clock-cells = <1>;
+};
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
new file mode 100644
index 0000000..3cc299f
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
@@ -0,0 +1,22 @@
+Mediatek vencltsys controller
+============================
+
+The Mediatek vencltsys controller provides various clocks to the system.
+
+Required Properties:
+
+- compatible: Should be:
+	- "mediatek,mt8173-vencltsys", "syscon"
+- #clock-cells: Must be 1
+
+The vencltsys controller uses the common clk binding from
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+The available clocks are defined in dt-bindings/clock/mt*-clk.h.
+
+Example:
+
+vencltsys: clock-controller at 19000000 {
+	compatible = "mediatek,mt8173-vencltsys", "syscon";
+	reg = <0 0x19000000 0 0x1000>;
+	#clock-cells = <1>;
+};
diff --git a/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt
new file mode 100644
index 0000000..5bb2866
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt
@@ -0,0 +1,22 @@
+Mediatek vencsys controller
+============================
+
+The Mediatek vencsys controller provides various clocks to the system.
+
+Required Properties:
+
+- compatible: Should be:
+	- "mediatek,mt8173-vencsys", "syscon"
+- #clock-cells: Must be 1
+
+The vencsys controller uses the common clk binding from
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+The available clocks are defined in dt-bindings/clock/mt*-clk.h.
+
+Example:
+
+vencsys: clock-controller at 18000000 {
+	compatible = "mediatek,mt8173-vencsys", "syscon";
+	reg = <0 0x18000000 0 0x1000>;
+	#clock-cells = <1>;
+};
-- 
1.8.1.1.dirty

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[James Liao]

Most multimedia subsystem clocks will be accessed by multiple
drivers, so it's a better way to manage these clocks in CCF.
This patch adds clock support for MM, IMG, VDEC, VENC and VENC_LT
subsystems.

Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
---
 drivers/clk/mediatek/clk-mt8173.c      | 267 +++++++++++++++++++++++++++++++++
 include/dt-bindings/clock/mt8173-clk.h |  97 +++++++++++-
 2 files changed, 361 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index f37ace6..05335e5 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -25,6 +25,10 @@ static DEFINE_SPINLOCK(mt8173_clk_lock);
 static const struct mtk_fixed_clk fixed_clks[] __initconst = {
 	FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
 	FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
+	FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
+	FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
+	FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
+	FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
 };
 
 static const struct mtk_fixed_factor top_divs[] __initconst = {
@@ -697,6 +701,183 @@ static const struct mtk_composite peri_clks[] __initconst = {
 	MUX(CLK_PERI_UART3_SEL, "uart3_ck_sel", uart_ck_sel_parents, 0x40c, 3, 1),
 };
 
+static const struct mtk_gate_regs cg_regs_4_8_0 __initconst = {
+	.set_ofs = 0x0004,
+	.clr_ofs = 0x0008,
+	.sta_ofs = 0x0000,
+};
+
+#define GATE_IMG(_id, _name, _parent, _shift) {			\
+		.id = _id,					\
+		.name = _name,					\
+		.parent_name = _parent,				\
+		.regs = &cg_regs_4_8_0,				\
+		.shift = _shift,				\
+		.ops = &mtk_clk_gate_ops_setclr,		\
+	}
+
+static const struct mtk_gate img_clks[] __initconst = {
+	GATE_IMG(CLK_IMG_LARB2_SMI, "img_larb2_smi", "mm_sel", 0),
+	GATE_IMG(CLK_IMG_CAM_SMI, "img_cam_smi", "mm_sel", 5),
+	GATE_IMG(CLK_IMG_CAM_CAM, "img_cam_cam", "mm_sel", 6),
+	GATE_IMG(CLK_IMG_SEN_TG, "img_sen_tg", "camtg_sel", 7),
+	GATE_IMG(CLK_IMG_SEN_CAM, "img_sen_cam", "mm_sel", 8),
+	GATE_IMG(CLK_IMG_CAM_SV, "img_cam_sv", "mm_sel", 9),
+	GATE_IMG(CLK_IMG_FD, "img_fd", "mm_sel", 11),
+};
+
+static const struct mtk_gate_regs mm0_cg_regs __initconst = {
+	.set_ofs = 0x0104,
+	.clr_ofs = 0x0108,
+	.sta_ofs = 0x0100,
+};
+
+static const struct mtk_gate_regs mm1_cg_regs __initconst = {
+	.set_ofs = 0x0114,
+	.clr_ofs = 0x0118,
+	.sta_ofs = 0x0110,
+};
+
+#define GATE_MM0(_id, _name, _parent, _shift) {			\
+		.id = _id,					\
+		.name = _name,					\
+		.parent_name = _parent,				\
+		.regs = &mm0_cg_regs,				\
+		.shift = _shift,				\
+		.ops = &mtk_clk_gate_ops_setclr,		\
+	}
+
+#define GATE_MM1(_id, _name, _parent, _shift) {			\
+		.id = _id,					\
+		.name = _name,					\
+		.parent_name = _parent,				\
+		.regs = &mm1_cg_regs,				\
+		.shift = _shift,				\
+		.ops = &mtk_clk_gate_ops_setclr,		\
+	}
+
+static const struct mtk_gate mm_clks[] __initconst = {
+	/* MM0 */
+	GATE_MM0(CLK_MM_SMI_COMMON, "mm_smi_common", "mm_sel", 0),
+	GATE_MM0(CLK_MM_SMI_LARB0, "mm_smi_larb0", "mm_sel", 1),
+	GATE_MM0(CLK_MM_CAM_MDP, "mm_cam_mdp", "mm_sel", 2),
+	GATE_MM0(CLK_MM_MDP_RDMA0, "mm_mdp_rdma0", "mm_sel", 3),
+	GATE_MM0(CLK_MM_MDP_RDMA1, "mm_mdp_rdma1", "mm_sel", 4),
+	GATE_MM0(CLK_MM_MDP_RSZ0, "mm_mdp_rsz0", "mm_sel", 5),
+	GATE_MM0(CLK_MM_MDP_RSZ1, "mm_mdp_rsz1", "mm_sel", 6),
+	GATE_MM0(CLK_MM_MDP_RSZ2, "mm_mdp_rsz2", "mm_sel", 7),
+	GATE_MM0(CLK_MM_MDP_TDSHP0, "mm_mdp_tdshp0", "mm_sel", 8),
+	GATE_MM0(CLK_MM_MDP_TDSHP1, "mm_mdp_tdshp1", "mm_sel", 9),
+	GATE_MM0(CLK_MM_MDP_WDMA, "mm_mdp_wdma", "mm_sel", 11),
+	GATE_MM0(CLK_MM_MDP_WROT0, "mm_mdp_wrot0", "mm_sel", 12),
+	GATE_MM0(CLK_MM_MDP_WROT1, "mm_mdp_wrot1", "mm_sel", 13),
+	GATE_MM0(CLK_MM_FAKE_ENG, "mm_fake_eng", "mm_sel", 14),
+	GATE_MM0(CLK_MM_MUTEX_32K, "mm_mutex_32k", "rtc_sel", 15),
+	GATE_MM0(CLK_MM_DISP_OVL0, "mm_disp_ovl0", "mm_sel", 16),
+	GATE_MM0(CLK_MM_DISP_OVL1, "mm_disp_ovl1", "mm_sel", 17),
+	GATE_MM0(CLK_MM_DISP_RDMA0, "mm_disp_rdma0", "mm_sel", 18),
+	GATE_MM0(CLK_MM_DISP_RDMA1, "mm_disp_rdma1", "mm_sel", 19),
+	GATE_MM0(CLK_MM_DISP_RDMA2, "mm_disp_rdma2", "mm_sel", 20),
+	GATE_MM0(CLK_MM_DISP_WDMA0, "mm_disp_wdma0", "mm_sel", 21),
+	GATE_MM0(CLK_MM_DISP_WDMA1, "mm_disp_wdma1", "mm_sel", 22),
+	GATE_MM0(CLK_MM_DISP_COLOR0, "mm_disp_color0", "mm_sel", 23),
+	GATE_MM0(CLK_MM_DISP_COLOR1, "mm_disp_color1", "mm_sel", 24),
+	GATE_MM0(CLK_MM_DISP_AAL, "mm_disp_aal", "mm_sel", 25),
+	GATE_MM0(CLK_MM_DISP_GAMMA, "mm_disp_gamma", "mm_sel", 26),
+	GATE_MM0(CLK_MM_DISP_UFOE, "mm_disp_ufoe", "mm_sel", 27),
+	GATE_MM0(CLK_MM_DISP_SPLIT0, "mm_disp_split0", "mm_sel", 28),
+	GATE_MM0(CLK_MM_DISP_SPLIT1, "mm_disp_split1", "mm_sel", 29),
+	GATE_MM0(CLK_MM_DISP_MERGE, "mm_disp_merge", "mm_sel", 30),
+	GATE_MM0(CLK_MM_DISP_OD, "mm_disp_od", "mm_sel", 31),
+	/* MM1 */
+	GATE_MM1(CLK_MM_DISP_PWM0MM, "mm_disp_pwm0mm", "mm_sel", 0),
+	GATE_MM1(CLK_MM_DISP_PWM026M, "mm_disp_pwm026m", "pwm_sel", 1),
+	GATE_MM1(CLK_MM_DISP_PWM1MM, "mm_disp_pwm1mm", "mm_sel", 2),
+	GATE_MM1(CLK_MM_DISP_PWM126M, "mm_disp_pwm126m", "pwm_sel", 3),
+	GATE_MM1(CLK_MM_DSI0_ENGINE, "mm_dsi0_engine", "mm_sel", 4),
+	GATE_MM1(CLK_MM_DSI0_DIGITAL, "mm_dsi0_digital", "dsi0_dig", 5),
+	GATE_MM1(CLK_MM_DSI1_ENGINE, "mm_dsi1_engine", "mm_sel", 6),
+	GATE_MM1(CLK_MM_DSI1_DIGITAL, "mm_dsi1_digital", "dsi1_dig", 7),
+	GATE_MM1(CLK_MM_DPI_PIXEL, "mm_dpi_pixel", "dpi0_sel", 8),
+	GATE_MM1(CLK_MM_DPI_ENGINE, "mm_dpi_engine", "mm_sel", 9),
+	GATE_MM1(CLK_MM_DPI1_PIXEL, "mm_dpi1_pixel", "lvds_pxl", 10),
+	GATE_MM1(CLK_MM_DPI1_ENGINE, "mm_dpi1_engine", "mm_sel", 11),
+	GATE_MM1(CLK_MM_HDMI_PIXEL, "mm_hdmi_pixel", "dpi0_sel", 12),
+	GATE_MM1(CLK_MM_HDMI_PLLCK, "mm_hdmi_pllck", "hdmi_sel", 13),
+	GATE_MM1(CLK_MM_HDMI_AUDIO, "mm_hdmi_audio", "apll1", 14),
+	GATE_MM1(CLK_MM_HDMI_SPDIF, "mm_hdmi_spdif", "apll2", 15),
+	GATE_MM1(CLK_MM_LVDS_PIXEL, "mm_lvds_pixel", "lvds_pxl", 16),
+	GATE_MM1(CLK_MM_LVDS_CTS, "mm_lvds_cts", "lvds_cts", 17),
+	GATE_MM1(CLK_MM_SMI_LARB4, "mm_smi_larb4", "mm_sel", 18),
+	GATE_MM1(CLK_MM_HDMI_HDCP, "mm_hdmi_hdcp", "hdcp_sel", 19),
+	GATE_MM1(CLK_MM_HDMI_HDCP24M, "mm_hdmi_hdcp24m", "hdcp_24m_sel", 20),
+};
+
+static const struct mtk_gate_regs vdec0_cg_regs __initconst = {
+	.set_ofs = 0x0000,
+	.clr_ofs = 0x0004,
+	.sta_ofs = 0x0000,
+};
+
+static const struct mtk_gate_regs vdec1_cg_regs __initconst = {
+	.set_ofs = 0x0008,
+	.clr_ofs = 0x000c,
+	.sta_ofs = 0x0008,
+};
+
+#define GATE_VDEC0(_id, _name, _parent, _shift) {		\
+		.id = _id,					\
+		.name = _name,					\
+		.parent_name = _parent,				\
+		.regs = &vdec0_cg_regs,				\
+		.shift = _shift,				\
+		.ops = &mtk_clk_gate_ops_setclr_inv,		\
+	}
+
+#define GATE_VDEC1(_id, _name, _parent, _shift) {		\
+		.id = _id,					\
+		.name = _name,					\
+		.parent_name = _parent,				\
+		.regs = &vdec1_cg_regs,				\
+		.shift = _shift,				\
+		.ops = &mtk_clk_gate_ops_setclr_inv,		\
+	}
+
+static const struct mtk_gate vdec_clks[] __initconst = {
+	GATE_VDEC0(CLK_VDEC_CKEN, "vdec_cken", "vdec_sel", 0),
+	GATE_VDEC1(CLK_VDEC_LARB_CKEN, "vdec_larb_cken", "mm_sel", 0),
+};
+
+#define GATE_VENC(_id, _name, _parent, _shift) {		\
+		.id = _id,					\
+		.name = _name,					\
+		.parent_name = _parent,				\
+		.regs = &cg_regs_4_8_0,				\
+		.shift = _shift,				\
+		.ops = &mtk_clk_gate_ops_setclr_inv,		\
+	}
+
+static const struct mtk_gate venc_clks[] __initconst = {
+	GATE_VENC(CLK_VENC_CKE0, "venc_cke0", "mm_sel", 0),
+	GATE_VENC(CLK_VENC_CKE1, "venc_cke1", "venc_sel", 4),
+	GATE_VENC(CLK_VENC_CKE2, "venc_cke2", "venc_sel", 8),
+	GATE_VENC(CLK_VENC_CKE3, "venc_cke3", "venc_sel", 12),
+};
+
+#define GATE_VENCLT(_id, _name, _parent, _shift) {		\
+		.id = _id,					\
+		.name = _name,					\
+		.parent_name = _parent,				\
+		.regs = &cg_regs_4_8_0,				\
+		.shift = _shift,				\
+		.ops = &mtk_clk_gate_ops_setclr_inv,		\
+	}
+
+static const struct mtk_gate venclt_clks[] __initconst = {
+	GATE_VENCLT(CLK_VENCLT_CKE0, "venclt_cke0", "mm_sel", 0),
+	GATE_VENCLT(CLK_VENCLT_CKE1, "venclt_cke1", "venclt_sel", 4),
+};
+
 static struct clk_onecell_data *mt8173_top_clk_data __initdata;
 static struct clk_onecell_data *mt8173_pll_clk_data __initdata;
 
@@ -841,3 +1022,89 @@ static void __init mtk_apmixedsys_init(struct device_node *node)
 }
 CLK_OF_DECLARE(mtk_apmixedsys, "mediatek,mt8173-apmixedsys",
 		mtk_apmixedsys_init);
+
+static void __init mtk_imgsys_init(struct device_node *node)
+{
+	struct clk_onecell_data *clk_data;
+	int r;
+
+	clk_data = mtk_alloc_clk_data(CLK_IMG_NR_CLK);
+
+	mtk_clk_register_gates(node, img_clks, ARRAY_SIZE(img_clks),
+						clk_data);
+
+	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+
+	if (r)
+		pr_err("%s(): could not register clock provider: %d\n",
+			__func__, r);
+}
+CLK_OF_DECLARE(mtk_imgsys, "mediatek,mt8173-imgsys", mtk_imgsys_init);
+
+static void __init mtk_mmsys_init(struct device_node *node)
+{
+	struct clk_onecell_data *clk_data;
+	int r;
+
+	clk_data = mtk_alloc_clk_data(CLK_MM_NR_CLK);
+
+	mtk_clk_register_gates(node, mm_clks, ARRAY_SIZE(mm_clks),
+						clk_data);
+
+	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+	if (r)
+		pr_err("%s(): could not register clock provider: %d\n",
+			__func__, r);
+}
+CLK_OF_DECLARE(mtk_mmsys, "mediatek,mt8173-mmsys", mtk_mmsys_init);
+
+static void __init mtk_vdecsys_init(struct device_node *node)
+{
+	struct clk_onecell_data *clk_data;
+	int r;
+
+	clk_data = mtk_alloc_clk_data(CLK_VDEC_NR_CLK);
+
+	mtk_clk_register_gates(node, vdec_clks, ARRAY_SIZE(vdec_clks),
+						clk_data);
+
+	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+	if (r)
+		pr_err("%s(): could not register clock provider: %d\n",
+			__func__, r);
+}
+CLK_OF_DECLARE(mtk_vdecsys, "mediatek,mt8173-vdecsys", mtk_vdecsys_init);
+
+static void __init mtk_vencsys_init(struct device_node *node)
+{
+	struct clk_onecell_data *clk_data;
+	int r;
+
+	clk_data = mtk_alloc_clk_data(CLK_VENC_NR_CLK);
+
+	mtk_clk_register_gates(node, venc_clks, ARRAY_SIZE(venc_clks),
+						clk_data);
+
+	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+	if (r)
+		pr_err("%s(): could not register clock provider: %d\n",
+			__func__, r);
+}
+CLK_OF_DECLARE(mtk_vencsys, "mediatek,mt8173-vencsys", mtk_vencsys_init);
+
+static void __init mtk_vencltsys_init(struct device_node *node)
+{
+	struct clk_onecell_data *clk_data;
+	int r;
+
+	clk_data = mtk_alloc_clk_data(CLK_VENCLT_NR_CLK);
+
+	mtk_clk_register_gates(node, venclt_clks, ARRAY_SIZE(venclt_clks),
+						clk_data);
+
+	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+	if (r)
+		pr_err("%s(): could not register clock provider: %d\n",
+			__func__, r);
+}
+CLK_OF_DECLARE(mtk_vencltsys, "mediatek,mt8173-vencltsys", mtk_vencltsys_init);
diff --git a/include/dt-bindings/clock/mt8173-clk.h b/include/dt-bindings/clock/mt8173-clk.h
index 7230c38..87820ee 100644
--- a/include/dt-bindings/clock/mt8173-clk.h
+++ b/include/dt-bindings/clock/mt8173-clk.h
@@ -153,12 +153,16 @@
 #define CLK_TOP_I2S2_M_SEL		135
 #define CLK_TOP_I2S3_M_SEL		136
 #define CLK_TOP_I2S3_B_SEL		137
-#define CLK_TOP_NR_CLK			138
+#define CLK_TOP_DSI0_DIG		138
+#define CLK_TOP_DSI1_DIG		139
+#define CLK_TOP_LVDS_PXL		140
+#define CLK_TOP_LVDS_CTS		141
+#define CLK_TOP_NR_CLK			142
 
 /* APMIXED_SYS */
 
-#define CLK_APMIXED_ARMCA15PLL	1
-#define CLK_APMIXED_ARMCA7PLL	2
+#define CLK_APMIXED_ARMCA15PLL		1
+#define CLK_APMIXED_ARMCA7PLL		2
 #define CLK_APMIXED_MAINPLL		3
 #define CLK_APMIXED_UNIVPLL		4
 #define CLK_APMIXED_MMPLL		5
@@ -231,4 +235,91 @@
 #define CLK_PERI_UART3_SEL		39
 #define CLK_PERI_NR_CLK			40
 
+/* IMG_SYS */
+
+#define CLK_IMG_LARB2_SMI		1
+#define CLK_IMG_CAM_SMI			2
+#define CLK_IMG_CAM_CAM			3
+#define CLK_IMG_SEN_TG			4
+#define CLK_IMG_SEN_CAM			5
+#define CLK_IMG_CAM_SV			6
+#define CLK_IMG_FD			7
+#define CLK_IMG_NR_CLK			8
+
+/* MM_SYS */
+
+#define CLK_MM_SMI_COMMON		1
+#define CLK_MM_SMI_LARB0		2
+#define CLK_MM_CAM_MDP			3
+#define CLK_MM_MDP_RDMA0		4
+#define CLK_MM_MDP_RDMA1		5
+#define CLK_MM_MDP_RSZ0			6
+#define CLK_MM_MDP_RSZ1			7
+#define CLK_MM_MDP_RSZ2			8
+#define CLK_MM_MDP_TDSHP0		9
+#define CLK_MM_MDP_TDSHP1		10
+#define CLK_MM_MDP_WDMA			11
+#define CLK_MM_MDP_WROT0		12
+#define CLK_MM_MDP_WROT1		13
+#define CLK_MM_FAKE_ENG			14
+#define CLK_MM_MUTEX_32K		15
+#define CLK_MM_DISP_OVL0		16
+#define CLK_MM_DISP_OVL1		17
+#define CLK_MM_DISP_RDMA0		18
+#define CLK_MM_DISP_RDMA1		19
+#define CLK_MM_DISP_RDMA2		20
+#define CLK_MM_DISP_WDMA0		21
+#define CLK_MM_DISP_WDMA1		22
+#define CLK_MM_DISP_COLOR0		23
+#define CLK_MM_DISP_COLOR1		24
+#define CLK_MM_DISP_AAL			25
+#define CLK_MM_DISP_GAMMA		26
+#define CLK_MM_DISP_UFOE		27
+#define CLK_MM_DISP_SPLIT0		28
+#define CLK_MM_DISP_SPLIT1		29
+#define CLK_MM_DISP_MERGE		30
+#define CLK_MM_DISP_OD			31
+#define CLK_MM_DISP_PWM0MM		32
+#define CLK_MM_DISP_PWM026M		33
+#define CLK_MM_DISP_PWM1MM		34
+#define CLK_MM_DISP_PWM126M		35
+#define CLK_MM_DSI0_ENGINE		36
+#define CLK_MM_DSI0_DIGITAL		37
+#define CLK_MM_DSI1_ENGINE		38
+#define CLK_MM_DSI1_DIGITAL		39
+#define CLK_MM_DPI_PIXEL		40
+#define CLK_MM_DPI_ENGINE		41
+#define CLK_MM_DPI1_PIXEL		42
+#define CLK_MM_DPI1_ENGINE		43
+#define CLK_MM_HDMI_PIXEL		44
+#define CLK_MM_HDMI_PLLCK		45
+#define CLK_MM_HDMI_AUDIO		46
+#define CLK_MM_HDMI_SPDIF		47
+#define CLK_MM_LVDS_PIXEL		48
+#define CLK_MM_LVDS_CTS			49
+#define CLK_MM_SMI_LARB4		50
+#define CLK_MM_HDMI_HDCP		51
+#define CLK_MM_HDMI_HDCP24M		52
+#define CLK_MM_NR_CLK			53
+
+/* VDEC_SYS */
+
+#define CLK_VDEC_CKEN			1
+#define CLK_VDEC_LARB_CKEN		2
+#define CLK_VDEC_NR_CLK			3
+
+/* VENC_SYS */
+
+#define CLK_VENC_CKE0			1
+#define CLK_VENC_CKE1			2
+#define CLK_VENC_CKE2			3
+#define CLK_VENC_CKE3			4
+#define CLK_VENC_NR_CLK			5
+
+/* VENCLT_SYS */
+
+#define CLK_VENCLT_CKE0			1
+#define CLK_VENCLT_CKE1			2
+#define CLK_VENCLT_NR_CLK		3
+
 #endif /* _DT_BINDINGS_CLK_MT8173_H */
-- 
1.8.1.1.dirty

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[Sascha Hauer]

On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
> Most multimedia subsystem clocks will be accessed by multiple
> drivers, so it's a better way to manage these clocks in CCF.
> This patch adds clock support for MM, IMG, VDEC, VENC and VENC_LT
> subsystems.
> 
> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
> ---
>  drivers/clk/mediatek/clk-mt8173.c      | 267 +++++++++++++++++++++++++++++++++
>  include/dt-bindings/clock/mt8173-clk.h |  97 +++++++++++-
>  2 files changed, 361 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
> index f37ace6..05335e5 100644
> --- a/drivers/clk/mediatek/clk-mt8173.c
> +++ b/drivers/clk/mediatek/clk-mt8173.c
> @@ -25,6 +25,10 @@ static DEFINE_SPINLOCK(mt8173_clk_lock);
>  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
>  	FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
>  	FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
> +	FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
> +	FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
> +	FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
> +	FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),

I would expect 51975 * KHZ here to avoid fractional numbers. Probably
gcc calculates that during compile time so this will work as expected,
still I'm not sure this is good style to use fractional numbers here.

Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
a clock derived from this running at 148.5MHz? Is it really correct to
use a fixed clock here or should it rather be lvdspll directly?

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[Daniel Kurtz]

On Wed, Aug 5, 2015 at 2:46 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
>> Most multimedia subsystem clocks will be accessed by multiple
>> drivers, so it's a better way to manage these clocks in CCF.
>> This patch adds clock support for MM, IMG, VDEC, VENC and VENC_LT
>> subsystems.
>>
>> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
>> ---
>>  drivers/clk/mediatek/clk-mt8173.c      | 267 +++++++++++++++++++++++++++++++++
>>  include/dt-bindings/clock/mt8173-clk.h |  97 +++++++++++-
>>  2 files changed, 361 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
>> index f37ace6..05335e5 100644
>> --- a/drivers/clk/mediatek/clk-mt8173.c
>> +++ b/drivers/clk/mediatek/clk-mt8173.c
>> @@ -25,6 +25,10 @@ static DEFINE_SPINLOCK(mt8173_clk_lock);
>>  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
>>       FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
>>       FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
>> +     FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
>> +     FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
>> +     FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
>> +     FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
>
> I would expect 51975 * KHZ here to avoid fractional numbers. Probably
> gcc calculates that during compile time so this will work as expected,
> still I'm not sure this is good style to use fractional numbers here.

I thought this looked a bit strange too, but for what its worth, these
two evaluate correctly:

localhost ~ # cat /sys/kernel/debug/clk/clk_summary  | grep lvds
    lvdspll                               0            0   149999878
       0 0
       lvds_pxl                           0            0   148500000
       0 0
       lvds_cts                           0            0    51975000
       0 0

>
> Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
> a clock derived from this running at 148.5MHz? Is it really correct to
> use a fixed clock here or should it rather be lvdspll directly?

I agree it does look strange to have a 51.975 MHz and 148.5 MHz clocks
with a 150 MHz PLL as their parent...  However, I'm not sure how much
this matters?  I think the idea here was that these frequencies are
best effort "nominal" clock values provided by Mediatek "designers".
The important point is that for the hardware to generate these either
of these clocks, lvdspll must be enabled.

-Dan


> Sascha
>
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[Sascha Hauer]

On Wed, Aug 05, 2015 at 03:26:29PM +0800, Daniel Kurtz wrote:
> On Wed, Aug 5, 2015 at 2:46 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
> >> Most multimedia subsystem clocks will be accessed by multiple
> >> drivers, so it's a better way to manage these clocks in CCF.
> >> This patch adds clock support for MM, IMG, VDEC, VENC and VENC_LT
> >> subsystems.
> >>
> >> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
> >> ---
> >>  drivers/clk/mediatek/clk-mt8173.c      | 267 +++++++++++++++++++++++++++++++++
> >>  include/dt-bindings/clock/mt8173-clk.h |  97 +++++++++++-
> >>  2 files changed, 361 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
> >> index f37ace6..05335e5 100644
> >> --- a/drivers/clk/mediatek/clk-mt8173.c
> >> +++ b/drivers/clk/mediatek/clk-mt8173.c
> >> @@ -25,6 +25,10 @@ static DEFINE_SPINLOCK(mt8173_clk_lock);
> >>  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
> >>       FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
> >>       FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
> >> +     FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
> >> +     FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
> >> +     FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
> >> +     FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
> >
> > I would expect 51975 * KHZ here to avoid fractional numbers. Probably
> > gcc calculates that during compile time so this will work as expected,
> > still I'm not sure this is good style to use fractional numbers here.
> 
> I thought this looked a bit strange too, but for what its worth, these
> two evaluate correctly:
> 
> localhost ~ # cat /sys/kernel/debug/clk/clk_summary  | grep lvds
>     lvdspll                               0            0   149999878
>        0 0
>        lvds_pxl                           0            0   148500000
>        0 0
>        lvds_cts                           0            0    51975000
>        0 0
> 
> >
> > Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
> > a clock derived from this running at 148.5MHz? Is it really correct to
> > use a fixed clock here or should it rather be lvdspll directly?
> 
> I agree it does look strange to have a 51.975 MHz and 148.5 MHz clocks
> with a 150 MHz PLL as their parent...  However, I'm not sure how much
> this matters?  I think the idea here was that these frequencies are
> best effort "nominal" clock values provided by Mediatek "designers".
> The important point is that for the hardware to generate these either
> of these clocks, lvdspll must be enabled.

This assumes that the lvdspll always runs at frequency the designers
thought that might be a good value. Should we really provide wrong clock
values when on some board for whatever reason the lvdspll is configured
for a different frequency?

sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[Daniel Kurtz]

On Wed, Aug 5, 2015 at 3:36 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, Aug 05, 2015 at 03:26:29PM +0800, Daniel Kurtz wrote:
>> On Wed, Aug 5, 2015 at 2:46 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
>> >> Most multimedia subsystem clocks will be accessed by multiple
>> >> drivers, so it's a better way to manage these clocks in CCF.
>> >> This patch adds clock support for MM, IMG, VDEC, VENC and VENC_LT
>> >> subsystems.
>> >>
>> >> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
>> >> ---
>> >>  drivers/clk/mediatek/clk-mt8173.c      | 267 +++++++++++++++++++++++++++++++++
>> >>  include/dt-bindings/clock/mt8173-clk.h |  97 +++++++++++-
>> >>  2 files changed, 361 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
>> >> index f37ace6..05335e5 100644
>> >> --- a/drivers/clk/mediatek/clk-mt8173.c
>> >> +++ b/drivers/clk/mediatek/clk-mt8173.c
>> >> @@ -25,6 +25,10 @@ static DEFINE_SPINLOCK(mt8173_clk_lock);
>> >>  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
>> >>       FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
>> >>       FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
>> >> +     FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
>> >> +     FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
>> >> +     FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
>> >> +     FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
>> >
>> > I would expect 51975 * KHZ here to avoid fractional numbers. Probably
>> > gcc calculates that during compile time so this will work as expected,
>> > still I'm not sure this is good style to use fractional numbers here.
>>
>> I thought this looked a bit strange too, but for what its worth, these
>> two evaluate correctly:
>>
>> localhost ~ # cat /sys/kernel/debug/clk/clk_summary  | grep lvds
>>     lvdspll                               0            0   149999878
>>        0 0
>>        lvds_pxl                           0            0   148500000
>>        0 0
>>        lvds_cts                           0            0    51975000
>>        0 0
>>
>> >
>> > Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
>> > a clock derived from this running at 148.5MHz? Is it really correct to
>> > use a fixed clock here or should it rather be lvdspll directly?
>>
>> I agree it does look strange to have a 51.975 MHz and 148.5 MHz clocks
>> with a 150 MHz PLL as their parent...  However, I'm not sure how much
>> this matters?  I think the idea here was that these frequencies are
>> best effort "nominal" clock values provided by Mediatek "designers".
>> The important point is that for the hardware to generate these either
>> of these clocks, lvdspll must be enabled.
>
> This assumes that the lvdspll always runs at frequency the designers
> thought that might be a good value. Should we really provide wrong clock
> values when on some board for whatever reason the lvdspll is configured
> for a different frequency?

Do you have an alternative suggestion for estimating the frequency of
a non-software controllable or measurable hardware clock?

Or can we land this series and update the frequency via some future
patch if and when that other board arrives which may have a different
frequency - and there is some reason to care what that frequency
actually is?

-Dan

>
> sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[Sascha Hauer]

On Wed, Aug 05, 2015 at 03:41:49PM +0800, Daniel Kurtz wrote:
> On Wed, Aug 5, 2015 at 3:36 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Wed, Aug 05, 2015 at 03:26:29PM +0800, Daniel Kurtz wrote:
> >> On Wed, Aug 5, 2015 at 2:46 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >> > On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
> >> >> Most multimedia subsystem clocks will be accessed by multiple
> >> >> drivers, so it's a better way to manage these clocks in CCF.
> >> >> This patch adds clock support for MM, IMG, VDEC, VENC and VENC_LT
> >> >> subsystems.
> >> >>
> >> >> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
> >> >> ---
> >> >>  drivers/clk/mediatek/clk-mt8173.c      | 267 +++++++++++++++++++++++++++++++++
> >> >>  include/dt-bindings/clock/mt8173-clk.h |  97 +++++++++++-
> >> >>  2 files changed, 361 insertions(+), 3 deletions(-)
> >> >>
> >> >> diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
> >> >> index f37ace6..05335e5 100644
> >> >> --- a/drivers/clk/mediatek/clk-mt8173.c
> >> >> +++ b/drivers/clk/mediatek/clk-mt8173.c
> >> >> @@ -25,6 +25,10 @@ static DEFINE_SPINLOCK(mt8173_clk_lock);
> >> >>  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
> >> >>       FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
> >> >>       FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
> >> >> +     FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
> >> >> +     FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
> >> >> +     FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
> >> >> +     FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
> >> >
> >> > I would expect 51975 * KHZ here to avoid fractional numbers. Probably
> >> > gcc calculates that during compile time so this will work as expected,
> >> > still I'm not sure this is good style to use fractional numbers here.
> >>
> >> I thought this looked a bit strange too, but for what its worth, these
> >> two evaluate correctly:
> >>
> >> localhost ~ # cat /sys/kernel/debug/clk/clk_summary  | grep lvds
> >>     lvdspll                               0            0   149999878
> >>        0 0
> >>        lvds_pxl                           0            0   148500000
> >>        0 0
> >>        lvds_cts                           0            0    51975000
> >>        0 0
> >>
> >> >
> >> > Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
> >> > a clock derived from this running at 148.5MHz? Is it really correct to
> >> > use a fixed clock here or should it rather be lvdspll directly?
> >>
> >> I agree it does look strange to have a 51.975 MHz and 148.5 MHz clocks
> >> with a 150 MHz PLL as their parent...  However, I'm not sure how much
> >> this matters?  I think the idea here was that these frequencies are
> >> best effort "nominal" clock values provided by Mediatek "designers".
> >> The important point is that for the hardware to generate these either
> >> of these clocks, lvdspll must be enabled.
> >
> > This assumes that the lvdspll always runs at frequency the designers
> > thought that might be a good value. Should we really provide wrong clock
> > values when on some board for whatever reason the lvdspll is configured
> > for a different frequency?
> 
> Do you have an alternative suggestion for estimating the frequency of
> a non-software controllable or measurable hardware clock?

What's the problem with using lvdspll directly? The consumers of these
clocks should be able to cope with the deviation between nomial
frequency and actual frequency.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[Daniel Kurtz]

On Wed, Aug 5, 2015 at 3:50 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, Aug 05, 2015 at 03:41:49PM +0800, Daniel Kurtz wrote:
>> On Wed, Aug 5, 2015 at 3:36 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> > On Wed, Aug 05, 2015 at 03:26:29PM +0800, Daniel Kurtz wrote:
>> >> On Wed, Aug 5, 2015 at 2:46 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
>> >> > On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
>> >> >> Most multimedia subsystem clocks will be accessed by multiple
>> >> >> drivers, so it's a better way to manage these clocks in CCF.
>> >> >> This patch adds clock support for MM, IMG, VDEC, VENC and VENC_LT
>> >> >> subsystems.
>> >> >>
>> >> >> Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
>> >> >> ---
>> >> >>  drivers/clk/mediatek/clk-mt8173.c      | 267 +++++++++++++++++++++++++++++++++
>> >> >>  include/dt-bindings/clock/mt8173-clk.h |  97 +++++++++++-
>> >> >>  2 files changed, 361 insertions(+), 3 deletions(-)
>> >> >>
>> >> >> diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
>> >> >> index f37ace6..05335e5 100644
>> >> >> --- a/drivers/clk/mediatek/clk-mt8173.c
>> >> >> +++ b/drivers/clk/mediatek/clk-mt8173.c
>> >> >> @@ -25,6 +25,10 @@ static DEFINE_SPINLOCK(mt8173_clk_lock);
>> >> >>  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
>> >> >>       FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
>> >> >>       FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
>> >> >> +     FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
>> >> >> +     FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
>> >> >> +     FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
>> >> >> +     FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
>> >> >
>> >> > I would expect 51975 * KHZ here to avoid fractional numbers. Probably
>> >> > gcc calculates that during compile time so this will work as expected,
>> >> > still I'm not sure this is good style to use fractional numbers here.
>> >>
>> >> I thought this looked a bit strange too, but for what its worth, these
>> >> two evaluate correctly:
>> >>
>> >> localhost ~ # cat /sys/kernel/debug/clk/clk_summary  | grep lvds
>> >>     lvdspll                               0            0   149999878
>> >>        0 0
>> >>        lvds_pxl                           0            0   148500000
>> >>        0 0
>> >>        lvds_cts                           0            0    51975000
>> >>        0 0
>> >>
>> >> >
>> >> > Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
>> >> > a clock derived from this running at 148.5MHz? Is it really correct to
>> >> > use a fixed clock here or should it rather be lvdspll directly?
>> >>
>> >> I agree it does look strange to have a 51.975 MHz and 148.5 MHz clocks
>> >> with a 150 MHz PLL as their parent...  However, I'm not sure how much
>> >> this matters?  I think the idea here was that these frequencies are
>> >> best effort "nominal" clock values provided by Mediatek "designers".
>> >> The important point is that for the hardware to generate these either
>> >> of these clocks, lvdspll must be enabled.
>> >
>> > This assumes that the lvdspll always runs at frequency the designers
>> > thought that might be a good value. Should we really provide wrong clock
>> > values when on some board for whatever reason the lvdspll is configured
>> > for a different frequency?
>>
>> Do you have an alternative suggestion for estimating the frequency of
>> a non-software controllable or measurable hardware clock?
>
> What's the problem with using lvdspll directly? The consumers of these
> clocks should be able to cope with the deviation between nomial
> frequency and actual frequency.

Oh, I'm totally fine with that too.  Although I think the way James
has it better models the hardware.

> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Industrial Linux Solutions                 | http://www.pengutronix.de/  |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[James Liao]

Hi Sascha,

On Wed, 2015-08-05 at 08:46 +0200, Sascha Hauer wrote:
> On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
> >  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
> >  	FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
> >  	FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
> > +	FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
> > +	FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
> > +	FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
> > +	FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
> 
> I would expect 51975 * KHZ here to avoid fractional numbers. Probably
> gcc calculates that during compile time so this will work as expected,
> still I'm not sure this is good style to use fractional numbers here.

As I know all constants will be calculated in compile time, so there
should be no difference between 51.975 * MHZ and 51975 * KHz. 

> Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
> a clock derived from this running at 148.5MHz? Is it really correct to
> use a fixed clock here or should it rather be lvdspll directly?

Here is the clock hierarchy between lvdspll and lvds_pxl:

            --------       AD_VPLL_DPIX_CK  --------   lvds_pxl  -----
           |        |--------------------->|        |---------->|
           |        |                      | cksys  |           |
LVDSPLL -->| LVDSTX |                      | buffer |           | MMSYS
           |        | AD_LVDSTX_CLKDIG_CTS | test   |  lvds_cts |
           |        |--------------------->|        |---------->|
            --------                        --------             -----

Some clocks and blocks are not modeled into CCF. But we prefer to enable
lvdspll before enabling lvds_pxl. So I modeled lvds_pxl (and lvds_cts)
as a fixed-rate clock with a source from lvdspll.

The frequency of these fixed-rate clocks (such as 148.5 MHz) are typical
rate. In fact, we don't care about the actual rate of these clocks. We
just care about the enable / disable sequence of them.


Best regards,

James

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[Sascha Hauer]

On Thu, Aug 06, 2015 at 04:23:51PM +0800, James Liao wrote:
> Hi Sascha,
> 
> On Wed, 2015-08-05 at 08:46 +0200, Sascha Hauer wrote:
> > On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
> > >  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
> > >  	FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
> > >  	FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
> > > +	FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
> > > +	FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
> > > +	FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
> > > +	FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
> > 
> > I would expect 51975 * KHZ here to avoid fractional numbers. Probably
> > gcc calculates that during compile time so this will work as expected,
> > still I'm not sure this is good style to use fractional numbers here.
> 
> As I know all constants will be calculated in compile time, so there
> should be no difference between 51.975 * MHZ and 51975 * KHz. 
> 
> > Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
> > a clock derived from this running at 148.5MHz? Is it really correct to
> > use a fixed clock here or should it rather be lvdspll directly?
> 
> Here is the clock hierarchy between lvdspll and lvds_pxl:
> 
>             --------       AD_VPLL_DPIX_CK  --------   lvds_pxl  -----
>            |        |--------------------->|        |---------->|
>            |        |                      | cksys  |           |
> LVDSPLL -->| LVDSTX |                      | buffer |           | MMSYS
>            |        | AD_LVDSTX_CLKDIG_CTS | test   |  lvds_cts |
>            |        |--------------------->|        |---------->|
>             --------                        --------             -----
> 
> Some clocks and blocks are not modeled into CCF. But we prefer to enable
> lvdspll before enabling lvds_pxl. So I modeled lvds_pxl (and lvds_cts)
> as a fixed-rate clock with a source from lvdspll.
> 
> The frequency of these fixed-rate clocks (such as 148.5 MHz) are typical
> rate. In fact, we don't care about the actual rate of these clocks. We
> just care about the enable / disable sequence of them.

Please either use the real rate or 0 (along with a explaining why). Using
a frequency with four to five significant digits makes me think that the
actual rate is very important.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[James Liao]

Hi Sascha,

On Thu, 2015-08-06 at 10:53 +0200, Sascha Hauer wrote:
> On Thu, Aug 06, 2015 at 04:23:51PM +0800, James Liao wrote:
> > On Wed, 2015-08-05 at 08:46 +0200, Sascha Hauer wrote:
> > > On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
> > > >  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
> > > >  	FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
> > > >  	FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
> > > > +	FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
> > > > +	FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
> > > > +	FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
> > > > +	FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
> > > 
> > > I would expect 51975 * KHZ here to avoid fractional numbers. Probably
> > > gcc calculates that during compile time so this will work as expected,
> > > still I'm not sure this is good style to use fractional numbers here.
> > 
> > As I know all constants will be calculated in compile time, so there
> > should be no difference between 51.975 * MHZ and 51975 * KHz. 
> > 
> > > Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
> > > a clock derived from this running at 148.5MHz? Is it really correct to
> > > use a fixed clock here or should it rather be lvdspll directly?
> > 
> > Here is the clock hierarchy between lvdspll and lvds_pxl:
> > 
> >             --------       AD_VPLL_DPIX_CK  --------   lvds_pxl  -----
> >            |        |--------------------->|        |---------->|
> >            |        |                      | cksys  |           |
> > LVDSPLL -->| LVDSTX |                      | buffer |           | MMSYS
> >            |        | AD_LVDSTX_CLKDIG_CTS | test   |  lvds_cts |
> >            |        |--------------------->|        |---------->|
> >             --------                        --------             -----
> > 
> > Some clocks and blocks are not modeled into CCF. But we prefer to enable
> > lvdspll before enabling lvds_pxl. So I modeled lvds_pxl (and lvds_cts)
> > as a fixed-rate clock with a source from lvdspll.
> > 
> > The frequency of these fixed-rate clocks (such as 148.5 MHz) are typical
> > rate. In fact, we don't care about the actual rate of these clocks. We
> > just care about the enable / disable sequence of them.
> 
> Please either use the real rate or 0 (along with a explaining why). Using
> a frequency with four to five significant digits makes me think that the
> actual rate is very important.

Oops, your suggestion is much different from Daniel's.

Daniel, could you help to comment about how we model these clocks?


Best regards,

James

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[Daniel Kurtz]

On Thu, Aug 6, 2015 at 5:00 PM, James Liao <jamesjj.liao@mediatek.com> wrote:
> Hi Sascha,
>
> On Thu, 2015-08-06 at 10:53 +0200, Sascha Hauer wrote:
>> On Thu, Aug 06, 2015 at 04:23:51PM +0800, James Liao wrote:
>> > On Wed, 2015-08-05 at 08:46 +0200, Sascha Hauer wrote:
>> > > On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
>> > > >  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
>> > > >         FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
>> > > >         FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
>> > > > +       FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
>> > > > +       FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
>> > > > +       FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
>> > > > +       FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
>> > >
>> > > I would expect 51975 * KHZ here to avoid fractional numbers. Probably
>> > > gcc calculates that during compile time so this will work as expected,
>> > > still I'm not sure this is good style to use fractional numbers here.
>> >
>> > As I know all constants will be calculated in compile time, so there
>> > should be no difference between 51.975 * MHZ and 51975 * KHz.
>> >
>> > > Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
>> > > a clock derived from this running at 148.5MHz? Is it really correct to
>> > > use a fixed clock here or should it rather be lvdspll directly?
>> >
>> > Here is the clock hierarchy between lvdspll and lvds_pxl:
>> >
>> >             --------       AD_VPLL_DPIX_CK  --------   lvds_pxl  -----
>> >            |        |--------------------->|        |---------->|
>> >            |        |                      | cksys  |           |
>> > LVDSPLL -->| LVDSTX |                      | buffer |           | MMSYS
>> >            |        | AD_LVDSTX_CLKDIG_CTS | test   |  lvds_cts |
>> >            |        |--------------------->|        |---------->|
>> >             --------                        --------             -----
>> >
>> > Some clocks and blocks are not modeled into CCF. But we prefer to enable
>> > lvdspll before enabling lvds_pxl. So I modeled lvds_pxl (and lvds_cts)
>> > as a fixed-rate clock with a source from lvdspll.
>> >
>> > The frequency of these fixed-rate clocks (such as 148.5 MHz) are typical
>> > rate. In fact, we don't care about the actual rate of these clocks. We
>> > just care about the enable / disable sequence of them.
>>
>> Please either use the real rate or 0 (along with a explaining why). Using
>> a frequency with four to five significant digits makes me think that the
>> actual rate is very important.
>
> Oops, your suggestion is much different from Daniel's.
>
> Daniel, could you help to comment about how we model these clocks?

First of all, for clocks where the rate doesn't matter, it doesn't
matters to what rate we set the clock.

As for the color of our shed, "the designer says these are the typical
rates" sounds good enough to me for a "real rate", so I prefer using
the rates in James' patch.

If not sure what Sascha's concern is, but if he insists on 0, I'm fine
with that too.

-Dan

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[Sascha Hauer]

On Thu, Aug 06, 2015 at 05:13:21PM +0800, Daniel Kurtz wrote:
> On Thu, Aug 6, 2015 at 5:00 PM, James Liao <jamesjj.liao@mediatek.com> wrote:
> > Hi Sascha,
> >
> > On Thu, 2015-08-06 at 10:53 +0200, Sascha Hauer wrote:
> >> On Thu, Aug 06, 2015 at 04:23:51PM +0800, James Liao wrote:
> >> > On Wed, 2015-08-05 at 08:46 +0200, Sascha Hauer wrote:
> >> > > On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
> >> > > >  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
> >> > > >         FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
> >> > > >         FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
> >> > > > +       FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
> >> > > > +       FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
> >> > > > +       FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
> >> > > > +       FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
> >> > >
> >> > > I would expect 51975 * KHZ here to avoid fractional numbers. Probably
> >> > > gcc calculates that during compile time so this will work as expected,
> >> > > still I'm not sure this is good style to use fractional numbers here.
> >> >
> >> > As I know all constants will be calculated in compile time, so there
> >> > should be no difference between 51.975 * MHZ and 51975 * KHz.
> >> >
> >> > > Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
> >> > > a clock derived from this running at 148.5MHz? Is it really correct to
> >> > > use a fixed clock here or should it rather be lvdspll directly?
> >> >
> >> > Here is the clock hierarchy between lvdspll and lvds_pxl:
> >> >
> >> >             --------       AD_VPLL_DPIX_CK  --------   lvds_pxl  -----
> >> >            |        |--------------------->|        |---------->|
> >> >            |        |                      | cksys  |           |
> >> > LVDSPLL -->| LVDSTX |                      | buffer |           | MMSYS
> >> >            |        | AD_LVDSTX_CLKDIG_CTS | test   |  lvds_cts |
> >> >            |        |--------------------->|        |---------->|
> >> >             --------                        --------             -----
> >> >
> >> > Some clocks and blocks are not modeled into CCF. But we prefer to enable
> >> > lvdspll before enabling lvds_pxl. So I modeled lvds_pxl (and lvds_cts)
> >> > as a fixed-rate clock with a source from lvdspll.
> >> >
> >> > The frequency of these fixed-rate clocks (such as 148.5 MHz) are typical
> >> > rate. In fact, we don't care about the actual rate of these clocks. We
> >> > just care about the enable / disable sequence of them.
> >>
> >> Please either use the real rate or 0 (along with a explaining why). Using
> >> a frequency with four to five significant digits makes me think that the
> >> actual rate is very important.
> >
> > Oops, your suggestion is much different from Daniel's.
> >
> > Daniel, could you help to comment about how we model these clocks?
> 
> First of all, for clocks where the rate doesn't matter, it doesn't
> matters to what rate we set the clock.
> 
> As for the color of our shed, "the designer says these are the typical
> rates" sounds good enough to me for a "real rate", so I prefer using
> the rates in James' patch.
> 
> If not sure what Sascha's concern is, but if he insists on 0, I'm fine
> with that too.

I only find it confusing. I'd expect either the correct rate or an
obviously dummy rate. Whatever we choose a comment explaining the
background would really help here. Otherwise we won't know later
whether this 148.5 MHz rate was introduced because a) The consumers
depend on this rate being reported, b) It really is the correct rate or
c) we don't care about the rate.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[James Liao]

Hi Sascha,

On Thu, 2015-08-06 at 12:20 +0200, Sascha Hauer wrote:
> On Thu, Aug 06, 2015 at 05:13:21PM +0800, Daniel Kurtz wrote:
> > On Thu, Aug 6, 2015 at 5:00 PM, James Liao <jamesjj.liao@mediatek.com> wrote:
> > > Hi Sascha,
> > >
> > > On Thu, 2015-08-06 at 10:53 +0200, Sascha Hauer wrote:
> > >> On Thu, Aug 06, 2015 at 04:23:51PM +0800, James Liao wrote:
> > >> > On Wed, 2015-08-05 at 08:46 +0200, Sascha Hauer wrote:
> > >> > > On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
> > >> > > >  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
> > >> > > >         FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
> > >> > > >         FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
> > >> > > > +       FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
> > >> > > > +       FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
> > >> > > > +       FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
> > >> > > > +       FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
> > >> > >
> > >> > > I would expect 51975 * KHZ here to avoid fractional numbers. Probably
> > >> > > gcc calculates that during compile time so this will work as expected,
> > >> > > still I'm not sure this is good style to use fractional numbers here.
> > >> >
> > >> > As I know all constants will be calculated in compile time, so there
> > >> > should be no difference between 51.975 * MHZ and 51975 * KHz.
> > >> >
> > >> > > Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
> > >> > > a clock derived from this running at 148.5MHz? Is it really correct to
> > >> > > use a fixed clock here or should it rather be lvdspll directly?
> > >> >
> > >> > Here is the clock hierarchy between lvdspll and lvds_pxl:
> > >> >
> > >> >             --------       AD_VPLL_DPIX_CK  --------   lvds_pxl  -----
> > >> >            |        |--------------------->|        |---------->|
> > >> >            |        |                      | cksys  |           |
> > >> > LVDSPLL -->| LVDSTX |                      | buffer |           | MMSYS
> > >> >            |        | AD_LVDSTX_CLKDIG_CTS | test   |  lvds_cts |
> > >> >            |        |--------------------->|        |---------->|
> > >> >             --------                        --------             -----
> > >> >
> > >> > Some clocks and blocks are not modeled into CCF. But we prefer to enable
> > >> > lvdspll before enabling lvds_pxl. So I modeled lvds_pxl (and lvds_cts)
> > >> > as a fixed-rate clock with a source from lvdspll.
> > >> >
> > >> > The frequency of these fixed-rate clocks (such as 148.5 MHz) are typical
> > >> > rate. In fact, we don't care about the actual rate of these clocks. We
> > >> > just care about the enable / disable sequence of them.
> > >>
> > >> Please either use the real rate or 0 (along with a explaining why). Using
> > >> a frequency with four to five significant digits makes me think that the
> > >> actual rate is very important.
> > >
> > > Oops, your suggestion is much different from Daniel's.
> > >
> > > Daniel, could you help to comment about how we model these clocks?
> > 
> > First of all, for clocks where the rate doesn't matter, it doesn't
> > matters to what rate we set the clock.
> > 
> > As for the color of our shed, "the designer says these are the typical
> > rates" sounds good enough to me for a "real rate", so I prefer using
> > the rates in James' patch.
> > 
> > If not sure what Sascha's concern is, but if he insists on 0, I'm fine
> > with that too.
> 
> I only find it confusing. I'd expect either the correct rate or an
> obviously dummy rate. Whatever we choose a comment explaining the
> background would really help here. Otherwise we won't know later
> whether this 148.5 MHz rate was introduced because a) The consumers
> depend on this rate being reported, b) It really is the correct rate or
> c) we don't care about the rate.

So the proper setting should be:

clk_name,          parent,    rate
-------------------------------------
"clkph_mck_o",     "clk26m",  0
"usb_syspll_125m", "clk26m",  125 * MHZ
"dsi0_dig",        "clk26m",  0
"dsi1_dig",        "clk26m",  0
"lvds_pxl",        "lvdspll", 0
"lvds_cts",        "lvdspll", 0

usb_syspll_125m will keep in 125 MHz event in different products.Others
may be changed by DRAM or display settings.

Daniel, do you think it's OK to model these clocks like above?


Best regards,

James

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[Daniel Kurtz]

On Fri, Aug 7, 2015 at 10:20 AM, James Liao <jamesjj.liao@mediatek.com> wrote:
> Hi Sascha,
>
> On Thu, 2015-08-06 at 12:20 +0200, Sascha Hauer wrote:
>> On Thu, Aug 06, 2015 at 05:13:21PM +0800, Daniel Kurtz wrote:
>> > On Thu, Aug 6, 2015 at 5:00 PM, James Liao <jamesjj.liao@mediatek.com> wrote:
>> > > Hi Sascha,
>> > >
>> > > On Thu, 2015-08-06 at 10:53 +0200, Sascha Hauer wrote:
>> > >> On Thu, Aug 06, 2015 at 04:23:51PM +0800, James Liao wrote:
>> > >> > On Wed, 2015-08-05 at 08:46 +0200, Sascha Hauer wrote:
>> > >> > > On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
>> > >> > > >  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
>> > >> > > >         FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
>> > >> > > >         FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
>> > >> > > > +       FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
>> > >> > > > +       FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
>> > >> > > > +       FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
>> > >> > > > +       FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
>> > >> > >
>> > >> > > I would expect 51975 * KHZ here to avoid fractional numbers. Probably
>> > >> > > gcc calculates that during compile time so this will work as expected,
>> > >> > > still I'm not sure this is good style to use fractional numbers here.
>> > >> >
>> > >> > As I know all constants will be calculated in compile time, so there
>> > >> > should be no difference between 51.975 * MHZ and 51975 * KHz.
>> > >> >
>> > >> > > Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
>> > >> > > a clock derived from this running at 148.5MHz? Is it really correct to
>> > >> > > use a fixed clock here or should it rather be lvdspll directly?
>> > >> >
>> > >> > Here is the clock hierarchy between lvdspll and lvds_pxl:
>> > >> >
>> > >> >             --------       AD_VPLL_DPIX_CK  --------   lvds_pxl  -----
>> > >> >            |        |--------------------->|        |---------->|
>> > >> >            |        |                      | cksys  |           |
>> > >> > LVDSPLL -->| LVDSTX |                      | buffer |           | MMSYS
>> > >> >            |        | AD_LVDSTX_CLKDIG_CTS | test   |  lvds_cts |
>> > >> >            |        |--------------------->|        |---------->|
>> > >> >             --------                        --------             -----
>> > >> >
>> > >> > Some clocks and blocks are not modeled into CCF. But we prefer to enable
>> > >> > lvdspll before enabling lvds_pxl. So I modeled lvds_pxl (and lvds_cts)
>> > >> > as a fixed-rate clock with a source from lvdspll.
>> > >> >
>> > >> > The frequency of these fixed-rate clocks (such as 148.5 MHz) are typical
>> > >> > rate. In fact, we don't care about the actual rate of these clocks. We
>> > >> > just care about the enable / disable sequence of them.
>> > >>
>> > >> Please either use the real rate or 0 (along with a explaining why). Using
>> > >> a frequency with four to five significant digits makes me think that the
>> > >> actual rate is very important.
>> > >
>> > > Oops, your suggestion is much different from Daniel's.
>> > >
>> > > Daniel, could you help to comment about how we model these clocks?
>> >
>> > First of all, for clocks where the rate doesn't matter, it doesn't
>> > matters to what rate we set the clock.
>> >
>> > As for the color of our shed, "the designer says these are the typical
>> > rates" sounds good enough to me for a "real rate", so I prefer using
>> > the rates in James' patch.
>> >
>> > If not sure what Sascha's concern is, but if he insists on 0, I'm fine
>> > with that too.
>>
>> I only find it confusing. I'd expect either the correct rate or an
>> obviously dummy rate. Whatever we choose a comment explaining the
>> background would really help here. Otherwise we won't know later
>> whether this 148.5 MHz rate was introduced because a) The consumers
>> depend on this rate being reported, b) It really is the correct rate or
>> c) we don't care about the rate.
>
> So the proper setting should be:
>
> clk_name,          parent,    rate
> -------------------------------------
> "clkph_mck_o",     "clk26m",  0
> "usb_syspll_125m", "clk26m",  125 * MHZ
> "dsi0_dig",        "clk26m",  0
> "dsi1_dig",        "clk26m",  0
> "lvds_pxl",        "lvdspll", 0
> "lvds_cts",        "lvdspll", 0
>
> usb_syspll_125m will keep in 125 MHz event in different products.Others
> may be changed by DRAM or display settings.
>
> Daniel, do you think it's OK to model these clocks like above?

Yes, I am fine with this.

>
>
> Best regards,
>
> James
>
>

[PATCH v6 7/9] clk: mediatek: Add subsystem clocks of MT8173

[Daniel Kurtz]

On Fri, Aug 7, 2015 at 4:05 PM, Daniel Kurtz <djkurtz@chromium.org> wrote:
> On Fri, Aug 7, 2015 at 10:20 AM, James Liao <jamesjj.liao@mediatek.com> wrote:
>> Hi Sascha,
>>
>> On Thu, 2015-08-06 at 12:20 +0200, Sascha Hauer wrote:
>>> On Thu, Aug 06, 2015 at 05:13:21PM +0800, Daniel Kurtz wrote:
>>> > On Thu, Aug 6, 2015 at 5:00 PM, James Liao <jamesjj.liao@mediatek.com> wrote:
>>> > > Hi Sascha,
>>> > >
>>> > > On Thu, 2015-08-06 at 10:53 +0200, Sascha Hauer wrote:
>>> > >> On Thu, Aug 06, 2015 at 04:23:51PM +0800, James Liao wrote:
>>> > >> > On Wed, 2015-08-05 at 08:46 +0200, Sascha Hauer wrote:
>>> > >> > > On Tue, Aug 04, 2015 at 04:16:56PM +0800, James Liao wrote:
>>> > >> > > >  static const struct mtk_fixed_clk fixed_clks[] __initconst = {
>>> > >> > > >         FIXED_CLK(CLK_TOP_CLKPH_MCK_O, "clkph_mck_o", "clk26m", 400 * MHZ),
>>> > >> > > >         FIXED_CLK(CLK_TOP_USB_SYSPLL_125M, "usb_syspll_125m", "clk26m", 125 * MHZ),
>>> > >> > > > +       FIXED_CLK(CLK_TOP_DSI0_DIG, "dsi0_dig", "clk26m", 130 * MHZ),
>>> > >> > > > +       FIXED_CLK(CLK_TOP_DSI1_DIG, "dsi1_dig", "clk26m", 130 * MHZ),
>>> > >> > > > +       FIXED_CLK(CLK_TOP_LVDS_PXL, "lvds_pxl", "lvdspll", 148.5 * MHZ),
>>> > >> > > > +       FIXED_CLK(CLK_TOP_LVDS_CTS, "lvds_cts", "lvdspll", 51.975 * MHZ),
>>> > >> > >
>>> > >> > > I would expect 51975 * KHZ here to avoid fractional numbers. Probably
>>> > >> > > gcc calculates that during compile time so this will work as expected,
>>> > >> > > still I'm not sure this is good style to use fractional numbers here.
>>> > >> >
>>> > >> > As I know all constants will be calculated in compile time, so there
>>> > >> > should be no difference between 51.975 * MHZ and 51975 * KHz.
>>> > >> >
>>> > >> > > Anyway, on my system lvdspll is running at 150MHz. Are you sure there is
>>> > >> > > a clock derived from this running at 148.5MHz? Is it really correct to
>>> > >> > > use a fixed clock here or should it rather be lvdspll directly?
>>> > >> >
>>> > >> > Here is the clock hierarchy between lvdspll and lvds_pxl:
>>> > >> >
>>> > >> >             --------       AD_VPLL_DPIX_CK  --------   lvds_pxl  -----
>>> > >> >            |        |--------------------->|        |---------->|
>>> > >> >            |        |                      | cksys  |           |
>>> > >> > LVDSPLL -->| LVDSTX |                      | buffer |           | MMSYS
>>> > >> >            |        | AD_LVDSTX_CLKDIG_CTS | test   |  lvds_cts |
>>> > >> >            |        |--------------------->|        |---------->|
>>> > >> >             --------                        --------             -----
>>> > >> >
>>> > >> > Some clocks and blocks are not modeled into CCF. But we prefer to enable
>>> > >> > lvdspll before enabling lvds_pxl. So I modeled lvds_pxl (and lvds_cts)
>>> > >> > as a fixed-rate clock with a source from lvdspll.
>>> > >> >
>>> > >> > The frequency of these fixed-rate clocks (such as 148.5 MHz) are typical
>>> > >> > rate. In fact, we don't care about the actual rate of these clocks. We
>>> > >> > just care about the enable / disable sequence of them.
>>> > >>
>>> > >> Please either use the real rate or 0 (along with a explaining why). Using
>>> > >> a frequency with four to five significant digits makes me think that the
>>> > >> actual rate is very important.
>>> > >
>>> > > Oops, your suggestion is much different from Daniel's.
>>> > >
>>> > > Daniel, could you help to comment about how we model these clocks?
>>> >
>>> > First of all, for clocks where the rate doesn't matter, it doesn't
>>> > matters to what rate we set the clock.
>>> >
>>> > As for the color of our shed, "the designer says these are the typical
>>> > rates" sounds good enough to me for a "real rate", so I prefer using
>>> > the rates in James' patch.
>>> >
>>> > If not sure what Sascha's concern is, but if he insists on 0, I'm fine
>>> > with that too.
>>>
>>> I only find it confusing. I'd expect either the correct rate or an
>>> obviously dummy rate. Whatever we choose a comment explaining the
>>> background would really help here. Otherwise we won't know later
>>> whether this 148.5 MHz rate was introduced because a) The consumers
>>> depend on this rate being reported, b) It really is the correct rate or
>>> c) we don't care about the rate.
>>
>> So the proper setting should be:
>>
>> clk_name,          parent,    rate
>> -------------------------------------
>> "clkph_mck_o",     "clk26m",  0
>> "usb_syspll_125m", "clk26m",  125 * MHZ
>> "dsi0_dig",        "clk26m",  0
>> "dsi1_dig",        "clk26m",  0
>> "lvds_pxl",        "lvdspll", 0
>> "lvds_cts",        "lvdspll", 0
>>
>> usb_syspll_125m will keep in 125 MHz event in different products.Others
>> may be changed by DRAM or display settings.
>>
>> Daniel, do you think it's OK to model these clocks like above?
>
> Yes, I am fine with this.

Oh, but Sascha's main point is that, whatever we choose to do, please
document it to make it clear why we are setting these clocks to the
value we choose.

-Dan

>
>>
>>
>> Best regards,
>>
>> James
>>
>>

[PATCH v6 8/9] clk: mediatek: Add USB clock support in MT8173 APMIXEDSYS

[James Liao]

Add REF2USB_TX clock support into MT8173 APMIXEDSYS. This clock
is needed by USB 3.0.

Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
---
 drivers/clk/mediatek/Makefile          |   2 +-
 drivers/clk/mediatek/clk-apmixed.c     | 107 +++++++++++++++++++++++++++++++++
 drivers/clk/mediatek/clk-mt8173.c      |  47 +++++++++++++++
 drivers/clk/mediatek/clk-mtk.h         |   3 +
 drivers/clk/mediatek/clk-pll.c         |   7 +--
 include/dt-bindings/clock/mt8173-clk.h |   3 +-
 6 files changed, 161 insertions(+), 8 deletions(-)
 create mode 100644 drivers/clk/mediatek/clk-apmixed.c

diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
index 8e4b2a4..95fdfac 100644
--- a/drivers/clk/mediatek/Makefile
+++ b/drivers/clk/mediatek/Makefile
@@ -1,4 +1,4 @@
-obj-y += clk-mtk.o clk-pll.o clk-gate.o
+obj-y += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o
 obj-$(CONFIG_RESET_CONTROLLER) += reset.o
 obj-y += clk-mt8135.o
 obj-y += clk-mt8173.o
diff --git a/drivers/clk/mediatek/clk-apmixed.c b/drivers/clk/mediatek/clk-apmixed.c
new file mode 100644
index 0000000..5303c59
--- /dev/null
+++ b/drivers/clk/mediatek/clk-apmixed.c
@@ -0,0 +1,107 @@
+/*
+ * Copyright (c) 2015 MediaTek Inc.
+ * Author: James Liao <jamesjj.liao@mediatek.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/delay.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+
+#include "clk-mtk.h"
+
+#define REF2USB_TX_EN		BIT(0)
+#define REF2USB_TX_LPF_EN	BIT(1)
+#define REF2USB_TX_OUT_EN	BIT(2)
+#define REF2USB_EN_MASK		(REF2USB_TX_EN | REF2USB_TX_LPF_EN | \
+				 REF2USB_TX_OUT_EN)
+
+struct mtk_ref2usb_tx {
+	struct clk_hw	hw;
+	void __iomem	*base_addr;
+};
+
+static inline struct mtk_ref2usb_tx *to_mtk_ref2usb_tx(struct clk_hw *hw)
+{
+	return container_of(hw, struct mtk_ref2usb_tx, hw);
+}
+
+static int mtk_ref2usb_tx_is_prepared(struct clk_hw *hw)
+{
+	struct mtk_ref2usb_tx *tx = to_mtk_ref2usb_tx(hw);
+
+	return (readl(tx->base_addr) & REF2USB_EN_MASK) == REF2USB_EN_MASK;
+}
+
+static int mtk_ref2usb_tx_prepare(struct clk_hw *hw)
+{
+	struct mtk_ref2usb_tx *tx = to_mtk_ref2usb_tx(hw);
+	u32 val;
+
+	val = readl(tx->base_addr);
+
+	val |= REF2USB_TX_EN;
+	writel(val, tx->base_addr);
+	udelay(100);
+
+	val |= REF2USB_TX_LPF_EN;
+	writel(val, tx->base_addr);
+
+	val |= REF2USB_TX_OUT_EN;
+	writel(val, tx->base_addr);
+
+	return 0;
+}
+
+static void mtk_ref2usb_tx_unprepare(struct clk_hw *hw)
+{
+	struct mtk_ref2usb_tx *tx = to_mtk_ref2usb_tx(hw);
+	u32 val;
+
+	val = readl(tx->base_addr);
+	val &= ~REF2USB_EN_MASK;
+	writel(val, tx->base_addr);
+}
+
+static const struct clk_ops mtk_ref2usb_tx_ops = {
+	.is_prepared	= mtk_ref2usb_tx_is_prepared,
+	.prepare	= mtk_ref2usb_tx_prepare,
+	.unprepare	= mtk_ref2usb_tx_unprepare,
+};
+
+struct clk * __init mtk_clk_register_ref2usb_tx(const char *name,
+			const char *parent_name, void __iomem *reg)
+{
+	struct mtk_ref2usb_tx *tx;
+	struct clk_init_data init = {};
+	struct clk *clk;
+
+	tx = kzalloc(sizeof(*tx), GFP_KERNEL);
+	if (!tx)
+		return ERR_PTR(-ENOMEM);
+
+	tx->base_addr = reg;
+	tx->hw.init = &init;
+
+	init.name = name;
+	init.ops = &mtk_ref2usb_tx_ops;
+	init.parent_names = &parent_name;
+	init.num_parents = 1;
+
+	clk = clk_register(NULL, &tx->hw);
+
+	if (IS_ERR(clk)) {
+		pr_err("Failed to register clk %s: %ld\n", name, PTR_ERR(clk));
+		kfree(tx);
+	}
+
+	return clk;
+}
diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 05335e5..63662d0 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -969,6 +969,24 @@ static void __init mtk_pericfg_init(struct device_node *node)
 }
 CLK_OF_DECLARE(mtk_pericfg, "mediatek,mt8173-pericfg", mtk_pericfg_init);
 
+struct mtk_clk_usb {
+	int id;
+	const char *name;
+	const char *parent;
+	u32 reg_ofs;
+};
+
+#define APMIXED_USB(_id, _name, _parent, _reg_ofs) {			\
+		.id = _id,						\
+		.name = _name,						\
+		.parent = _parent,					\
+		.reg_ofs = _reg_ofs,					\
+	}
+
+static const struct mtk_clk_usb apmixed_usb[] __initconst = {
+	APMIXED_USB(CLK_APMIXED_REF2USB_TX, "ref2usb_tx", "clk26m", 0x8),
+};
+
 #define MT8173_PLL_FMAX		(3000UL * MHZ)
 
 #define CON0_MT8173_RST_BAR	BIT(24)
@@ -1011,6 +1029,15 @@ static const struct mtk_pll_data plls[] = {
 static void __init mtk_apmixedsys_init(struct device_node *node)
 {
 	struct clk_onecell_data *clk_data;
+	void __iomem *base;
+	struct clk *clk;
+	int r, i;
+
+	base = of_iomap(node, 0);
+	if (!base) {
+		pr_err("%s(): ioremap failed\n", __func__);
+		return;
+	}
 
 	mt8173_pll_clk_data = clk_data = mtk_alloc_clk_data(CLK_APMIXED_NR_CLK);
 	if (!clk_data)
@@ -1018,6 +1045,26 @@ static void __init mtk_apmixedsys_init(struct device_node *node)
 
 	mtk_clk_register_plls(node, plls, ARRAY_SIZE(plls), clk_data);
 
+	for (i = 0; i < ARRAY_SIZE(apmixed_usb); i++) {
+		const struct mtk_clk_usb *cku = &apmixed_usb[i];
+
+		clk = mtk_clk_register_ref2usb_tx(cku->name, cku->parent,
+					base + cku->reg_ofs);
+
+		if (IS_ERR(clk)) {
+			pr_err("Failed to register clk %s: %ld\n", cku->name,
+					PTR_ERR(clk));
+			continue;
+		}
+
+		clk_data->clks[cku->id] = clk;
+	}
+
+	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
+	if (r)
+		pr_err("%s(): could not register clock provider: %d\n",
+			__func__, r);
+
 	mtk_clk_enable_critical();
 }
 CLK_OF_DECLARE(mtk_apmixedsys, "mediatek,mt8173-apmixedsys",
diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
index 32d4f55..cf19363 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -173,6 +173,9 @@ void mtk_clk_register_plls(struct device_node *node,
 		const struct mtk_pll_data *plls, int num_plls,
 		struct clk_onecell_data *clk_data);
 
+struct clk *mtk_clk_register_ref2usb_tx(const char *name,
+			const char *parent_name, void __iomem *reg);
+
 #ifdef CONFIG_RESET_CONTROLLER
 void mtk_register_reset_controller(struct device_node *np,
 			unsigned int num_regs, int regofs);
diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c
index 44409e9..813f0c7 100644
--- a/drivers/clk/mediatek/clk-pll.c
+++ b/drivers/clk/mediatek/clk-pll.c
@@ -302,7 +302,7 @@ void __init mtk_clk_register_plls(struct device_node *node,
 		const struct mtk_pll_data *plls, int num_plls, struct clk_onecell_data *clk_data)
 {
 	void __iomem *base;
-	int r, i;
+	int i;
 	struct clk *clk;
 
 	base = of_iomap(node, 0);
@@ -324,9 +324,4 @@ void __init mtk_clk_register_plls(struct device_node *node,
 
 		clk_data->clks[pll->id] = clk;
 	}
-
-	r = of_clk_add_provider(node, of_clk_src_onecell_get, clk_data);
-	if (r)
-		pr_err("%s(): could not register clock provider: %d\n",
-			__func__, r);
 }
diff --git a/include/dt-bindings/clock/mt8173-clk.h b/include/dt-bindings/clock/mt8173-clk.h
index 87820ee..e3db63a 100644
--- a/include/dt-bindings/clock/mt8173-clk.h
+++ b/include/dt-bindings/clock/mt8173-clk.h
@@ -175,7 +175,8 @@
 #define CLK_APMIXED_APLL2		12
 #define CLK_APMIXED_LVDSPLL		13
 #define CLK_APMIXED_MSDCPLL2		14
-#define CLK_APMIXED_NR_CLK		15
+#define CLK_APMIXED_REF2USB_TX		15
+#define CLK_APMIXED_NR_CLK		16
 
 /* INFRA_SYS */
 
-- 
1.8.1.1.dirty

[PATCH v6 9/9] arm64: dts: mt8173: Add subsystem clock controller device nodes

[James Liao]

This patch adds device nodes providing subsystem clocks on MT8173,
includes mmsys, imgsys, vdecsys, vencsys and vencltsys.

Signed-off-by: James Liao <jamesjj.liao@mediatek.com>
---
 arch/arm64/boot/dts/mediatek/mt8173.dtsi | 37 ++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
index a2f63e4..32c85cc 100644
--- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi
@@ -102,6 +102,13 @@
 		clock-output-names = "clk32k";
 	};
 
+	cpum_ck: oscillator at 2 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <0>;
+		clock-output-names = "cpum_ck";
+	};
+
 	timer {
 		compatible = "arm,armv8-timer";
 		interrupt-parent = <&gic>;
@@ -227,6 +234,36 @@
 			clocks = <&uart_clk>;
 			status = "disabled";
 		};
+
+		mmsys: clock-controller at 14000000 {
+			compatible = "mediatek,mt8173-mmsys", "syscon";
+			reg = <0 0x14000000 0 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		imgsys: clock-controller at 15000000 {
+			compatible = "mediatek,mt8173-imgsys", "syscon";
+			reg = <0 0x15000000 0 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		vdecsys: clock-controller at 16000000 {
+			compatible = "mediatek,mt8173-vdecsys", "syscon";
+			reg = <0 0x16000000 0 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		vencsys: clock-controller at 18000000 {
+			compatible = "mediatek,mt8173-vencsys", "syscon";
+			reg = <0 0x18000000 0 0x1000>;
+			#clock-cells = <1>;
+		};
+
+		vencltsys: clock-controller at 19000000 {
+			compatible = "mediatek,mt8173-vencltsys", "syscon";
+			reg = <0 0x19000000 0 0x1000>;
+			#clock-cells = <1>;
+		};
 	};
 };
 
-- 
1.8.1.1.dirty

[PATCH v6 0/9] Fixes and new clocks support for Mediatek MT8173

[Daniel Kurtz]

On Tue, Aug 4, 2015 at 4:16 PM, James Liao <jamesjj.liao@mediatek.com> wrote:
> This patchset is based on 4.2-rc2 and [1], and contains minor fixes and
> subsystem clocks support for Mediatek MT8173.
>
> The previous reviews can be found in [2]. The most different from previous
> patchset is rebasing to 4.2-rc2.

For the whole series:

Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>


>
> changes since v5:
> - Rebase to 4.2-rc2.
> - Patch "Fix enabling of critical clocks" had been included in 4.2-rc2,
>   so it need not be included in this patchset.
>
> changes since v4:
> - Add a separated patch to remove unused dpi_ck.
> - Add separated patches to fix clk/mediatek in release 4.2-rc1.
> - Add __init* for initialization data and functions.
> - Rename dummy_clk with oscillator in DT.
> - Remove mtk_clk_register_apmixed_ex(), call mtk_clk_register_ref2usb_tx()
>   directly to register USB clock.
>
> changes since v3:
> - Remove clk_null dependency from root clocks.
> - Use fixed-rate clocks with typical rate to replace clk_null.
> - Separate ref2usb_tx implementation to clk-apmixed.c
> - Use clock-controller as the name of clock providers.
>
> changes since v2:
> - Rebase to 4.2-rc1.
> - Add device tree nodes for subsystem clocks.
> - Fine tune comments of patches.
> - Add __init, __initconst and const to init data and functions.
> - Removed unused code from init functions of subsystem clocks.
>
> changes since v1:
> - Add CA7PLL and CA15PLL as critical clocks.
> - Use the same register descriptor for imgsys, vensys and vencltsys.
> - Generalize apmixedsys special clocks registration.
>
> [1] https://patchwork.kernel.org/patch/6446341/
> [2] http://lists.infradead.org/pipermail/linux-mediatek/2015-July/001800.html
>
> James Liao (9):
>   clk: mediatek: Removed unused dpi_ck clock from MT8173
>   clk: mediatek: Remove unused code from MT8173.
>   clk: mediatek: Add __initdata and __init for data and functions
>   clk: mediatek: Add fixed clocks support for Mediatek SoC.
>   clk: mediatek: Fix rate and dependency of MT8173 clocks
>   dt-bindings: ARM: Mediatek: Document devicetree bindings for clock
>     controllers
>   clk: mediatek: Add subsystem clocks of MT8173
>   clk: mediatek: Add USB clock support in MT8173 APMIXEDSYS
>   arm64: dts: mt8173: Add subsystem clock controller device nodes
>
>  .../bindings/arm/mediatek/mediatek,imgsys.txt      |  22 ++
>  .../bindings/arm/mediatek/mediatek,mmsys.txt       |  22 ++
>  .../bindings/arm/mediatek/mediatek,vdecsys.txt     |  22 ++
>  .../bindings/arm/mediatek/mediatek,vencltsys.txt   |  22 ++
>  .../bindings/arm/mediatek/mediatek,vencsys.txt     |  22 ++
>  arch/arm64/boot/dts/mediatek/mt8173.dtsi           |  37 +++
>  drivers/clk/mediatek/Makefile                      |   2 +-
>  drivers/clk/mediatek/clk-apmixed.c                 | 107 +++++++
>  drivers/clk/mediatek/clk-gate.c                    |   2 +-
>  drivers/clk/mediatek/clk-mt8173.c                  | 335 ++++++++++++++++++++-
>  drivers/clk/mediatek/clk-mtk.c                     |  36 ++-
>  drivers/clk/mediatek/clk-mtk.h                     |  24 +-
>  drivers/clk/mediatek/clk-pll.c                     |   7 +-
>  include/dt-bindings/clock/mt8173-clk.h             | 101 ++++++-
>  14 files changed, 728 insertions(+), 33 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,imgsys.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,mmsys.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vdecsys.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vencltsys.txt
>  create mode 100644 Documentation/devicetree/bindings/arm/mediatek/mediatek,vencsys.txt
>  create mode 100644 drivers/clk/mediatek/clk-apmixed.c
>
> --
> 1.8.1.1.dirty
>