Re: [PATCH] arm64: dts: mt8173: add clock_null

["Heiko Stübner" <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>]

Am Freitag, 3. Juli 2015, 15:45:12 schrieb James Liao:
> On Thu, 2015-07-02 at 12:23 +0800, Daniel Kurtz wrote:
> > On Thu, Jul 2, 2015 at 11:06 AM, James Liao <jamesjj.liao-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org> 
wrote:
> > > These clocks such as clkph_mck_o are configured by other modules before
> > > kernel init, and their rates may different among platforms.
> > 
> > What other modules?
> > Do you mean the rates are configured by firmware?
> > How are the rates set?
> > Are there registers that configure its rate?
> > If so, why can't the kernel read these registers and compute the current
> > rate?
> CLKPH_MCK_O for example, it's a DRAM related clock, and initialized in
> preloader (before kernel init). It's setting may be different because
> using different DRAM module. And the setting may be changed dynamically
> in runtime.

If it's set in the preloader (and maybe changed at runtime) this means, its 
setting can also be read back to determine its current clock rate, so this is 
a non-argument here ;-)


> We don't care CLKPH_MCK_O's rate in CCF because we don't need to change
> mem_sel's setting in kernel, and there is not driver need to know
> mem_sel's rate.

As Daniel said, some of these clocks may be parents of other clocks, and not 
knowing their rate might hurt in the future - especially when it's easy after 
all to get its actual value like in your CLKPH_MCK_O example above.


Heiko


> > For mt8173, we are essentially discussing the following clocks (whose
> > sole parent is clk_null):
> > 
> > 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),
> > 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),
> > 
> > clkph_mck_o - This is the parent for dmpll_*, which are themselves
> > (potential) parent clocks for nearly every subsystem.
> > In fact, as shown above, the dmpll_* is the selected parent for
> > several other clocks, which all end up with an unknown rate.
> > So, I think it is worth investigating a little more how to properly
> > read or otherwise specify the rate for clkph_mck_o.
> 
> Please see above.
> 
> > dpi_ck, infra_cpum, mm_dsi0_digital, mm_dsi1_digital, mm_lvds_cts -
> > These are a dead-end (internal?) clock.
> > It is probably fine if their rates are unknown (0 Hz).
> > 
> > usb_syspll_125m - This sounds like a fixed 125 MHz clock.  It is also
> > a possible parent usb30 clock, so its value will propagate.
> > 
> > hdmitx_dig_cts - This is the root clock for the tree leading to
> > mm_hdmi_pllck, which includes hdmitxpll_d* and hdmi_sel.
> > However, I don't know how "mm_hdmi_pllck" is used.
> > 
> > mm_dpi1_pixel, mm_lvds_pixel - These two look very suspicious.  The
> > similar "mm_dpi0_pixel" and "mm_hdmi_pixel" have parent dpi0_sel.
> > It looks like maybe they should have "dpi1_sel" or "dpilvds_sel" as
> > parents, but the _sel are not hooked up.
> 
> Subsystem clocks with parent clk_null may have different reasons. I'll
> get back to you later.
> 
> 
> Best regards,
> 
> James

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html