On Fri, Jul 10, 2015 at 5:16 PM, James Liao <jamesjj.liao@xxxxxxxxxxxx> 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
