linux-kernel - Re: [PATCH] arm64: dts: mt8173: add clock

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 03 Jul 2015 10:38:09 +0200
From:	Heiko Stübner <heiko@...ech.de>
To:	James Liao <jamesjj.liao@...iatek.com>
Cc:	Daniel Kurtz <djkurtz@...omium.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	"open list:OPEN FIRMWARE AND..." <devicetree@...r.kernel.org>,
	Mike Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	linux-mediatek@...ts.infradead.org, ted.lin@...iatek.com,
	Matthias Brugger <matthias.bgg@...il.com>,
	Eddie Huang <eddie.huang@...iatek.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH] arm64: dts: mt8173: add clock_null

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@...iatek.com> 
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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/