2015-07-10 09:17:24

by James Liao

[permalink] [raw]
Subject: [PATCH 0/2] Add clk_null to be the dummy root of MT8173 clocks

This patchset is based on 4.2-rc1 and adds a dummy clock "clk_null"
to be the root clock of clocks whose parents are not contained in
CCF clock tree.

In previous patch [1], it seems not suitable to declare clk_null in
device tree because it's not a clock comes form outside of SoC. So we
move clk_null into clock driver.

[1] https://lkml.org/lkml/2015/6/18/24

James Liao (2):
clk: mediatek: Add root clocks support for Mediatek SoC.
clk: mediatek: Add clk_null to be the dummy root clock for MT8173

drivers/clk/mediatek/clk-mt8173.c | 5 +++++
drivers/clk/mediatek/clk-mtk.c | 23 +++++++++++++++++++++++
drivers/clk/mediatek/clk-mtk.h | 17 ++++++++++++++++-
include/dt-bindings/clock/mt8173-clk.h | 1 +
4 files changed, 45 insertions(+), 1 deletion(-)

--
1.8.1.1.dirty


2015-07-10 09:17:46

by James Liao

[permalink] [raw]
Subject: [PATCH 1/2] clk: mediatek: Add root clocks support for Mediatek SoC.

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

Signed-off-by: James Liao <[email protected]>
---
drivers/clk/mediatek/clk-mtk.c | 23 +++++++++++++++++++++++
drivers/clk/mediatek/clk-mtk.h | 17 ++++++++++++++++-
2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/mediatek/clk-mtk.c b/drivers/clk/mediatek/clk-mtk.c
index 18444ae..4ac80d3 100644
--- a/drivers/clk/mediatek/clk-mtk.c
+++ b/drivers/clk/mediatek/clk-mtk.c
@@ -49,6 +49,29 @@ err_out:
return NULL;
}

+void mtk_clk_register_root_clks(const struct mtk_root_clk *clks, int num,
+ struct clk_onecell_data *clk_data)
+{
+ int i;
+ struct clk *clk;
+
+ for (i = 0; i < num; i++) {
+ const struct mtk_root_clk *rc = &clks[i];
+
+ clk = clk_register_fixed_rate(NULL, rc->name, NULL,
+ 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 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 9dda9d8..d8d8b06 100644
--- a/drivers/clk/mediatek/clk-mtk.h
+++ b/drivers/clk/mediatek/clk-mtk.h
@@ -25,6 +25,21 @@

#define MHZ (1000 * 1000)

+struct mtk_root_clk {
+ int id;
+ const char *name;
+ unsigned long rate;
+};
+
+#define ROOT_CLK(_id, _name, _rate) { \
+ .id = _id, \
+ .name = _name, \
+ .rate = _rate, \
+ }
+
+void mtk_clk_register_root_clks(const struct mtk_root_clk *clks,
+ int num, struct clk_onecell_data *clk_data);
+
struct mtk_fixed_factor {
int id;
const char *name;
@@ -41,7 +56,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 {
--
1.8.1.1.dirty

2015-07-10 09:17:08

by James Liao

[permalink] [raw]
Subject: [PATCH 2/2] clk: mediatek: Add clk_null to be the dummy root clock for MT8173

This patch add a dummy clock "clk_null" to be the root clock of
clocks whose parents are not contained in CCF clock tree.

Signed-off-by: James Liao <[email protected]>
---
drivers/clk/mediatek/clk-mt8173.c | 5 +++++
include/dt-bindings/clock/mt8173-clk.h | 1 +
2 files changed, 6 insertions(+)

diff --git a/drivers/clk/mediatek/clk-mt8173.c b/drivers/clk/mediatek/clk-mt8173.c
index 4b9e04c..c81bd72 100644
--- a/drivers/clk/mediatek/clk-mt8173.c
+++ b/drivers/clk/mediatek/clk-mt8173.c
@@ -24,6 +24,10 @@

static DEFINE_SPINLOCK(mt8173_clk_lock);

+static const struct mtk_root_clk root_clks[] __initconst = {
+ ROOT_CLK(CLK_TOP_NULL, "clk_null", 0),
+};
+
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),
@@ -714,6 +718,7 @@ static void __init mtk_topckgen_init(struct device_node *node)

clk_data = mtk_alloc_clk_data(CLK_TOP_NR_CLK);

+ mtk_clk_register_root_clks(root_clks, ARRAY_SIZE(root_clks), clk_data);
mtk_clk_register_factors(root_clk_alias, ARRAY_SIZE(root_clk_alias), 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,
diff --git a/include/dt-bindings/clock/mt8173-clk.h b/include/dt-bindings/clock/mt8173-clk.h
index 4ad76ed..e91e9b8 100644
--- a/include/dt-bindings/clock/mt8173-clk.h
+++ b/include/dt-bindings/clock/mt8173-clk.h
@@ -17,6 +17,7 @@

/* TOPCKGEN */

+#define CLK_TOP_NULL 0
#define CLK_TOP_CLKPH_MCK_O 1
#define CLK_TOP_DPI 2
#define CLK_TOP_USB_SYSPLL_125M 3
--
1.8.1.1.dirty

2015-07-10 16:03:39

by Daniel Kurtz

[permalink] [raw]
Subject: Re: [PATCH 0/2] Add clk_null to be the dummy root of MT8173 clocks

On Fri, Jul 10, 2015 at 5:16 PM, James Liao <[email protected]> wrote:
> This patchset is based on 4.2-rc1 and adds a dummy clock "clk_null"
> to be the root clock of clocks whose parents are not contained in
> CCF clock tree.
>
> In previous patch [1], it seems not suitable to declare clk_null in
> device tree because it's not a clock comes form outside of SoC. So we
> move clk_null into clock driver.

NAK. I still do not see why we need to introduce clk_null. Just
model the real clock tree.

Every clock listed with "clk_null" as its parent actually has a real
parent clock.
As mentioned elsewhere, it is not enough to just enable/disable a clock gate.
Clock enables/disables propagate up to parent clocks, and ensure that
parents themselves are enabled when any child is enabled, and disabled
when all of its children are disabled.
So, we must properly configure each clock's parent to ensure that its
parent will be enabled when needed, and that it can be disabled to
save power when no longer required.

Creating a bogus clk_null breaks the clock tree model, potentially
causing real issues when enabling/disabling clocks.

Please ask your hardware team to specify the real parent clock for
each of the clocks below so we can model the clock tree accurately.

In addition, these four need not be defined at all. We can just use
their parent clocks directly.
As Sascha eloquently put it: "There's no need to model pieces of wire in CCF."

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),


For these "clk_null as parent" gate clocks, please specify their real
parent clock:

GATE_ICG(CLK_INFRA_CPUM, "infra_cpum", "clk_null", 15),
GATE_MM1(CLK_MM_DSI0_DIGITAL, "mm_dsi0_digital", "clk_null", 5),
GATE_MM1(CLK_MM_DSI1_DIGITAL, "mm_dsi1_digital", "clk_null", 7),
GATE_MM1(CLK_MM_DPI1_PIXEL, "mm_dpi1_pixel", "clk_null", 10),
GATE_MM1(CLK_MM_LVDS_PIXEL, "mm_lvds_pixel", "clk_null", 16),
GATE_MM1(CLK_MM_LVDS_CTS, "mm_lvds_cts", "clk_null", 17),

Thanks,
-Dan

> [1] https://lkml.org/lkml/2015/6/18/24
>
> James Liao (2):
> clk: mediatek: Add root clocks support for Mediatek SoC.
> clk: mediatek: Add clk_null to be the dummy root clock for MT8173
>
> drivers/clk/mediatek/clk-mt8173.c | 5 +++++
> drivers/clk/mediatek/clk-mtk.c | 23 +++++++++++++++++++++++
> drivers/clk/mediatek/clk-mtk.h | 17 ++++++++++++++++-
> include/dt-bindings/clock/mt8173-clk.h | 1 +
> 4 files changed, 45 insertions(+), 1 deletion(-)
>
> --
> 1.8.1.1.dirty