Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751876AbbGBEYH (ORCPT ); Thu, 2 Jul 2015 00:24:07 -0400 Received: from mail-yk0-f173.google.com ([209.85.160.173]:33589 "EHLO mail-yk0-f173.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750768AbbGBEYA (ORCPT ); Thu, 2 Jul 2015 00:24:00 -0400 MIME-Version: 1.0 In-Reply-To: <1435806379.3526.31.camel@mtksdaap41> References: <1434605351-64592-1-git-send-email-eddie.huang@mediatek.com> <8860488.JuX1EN5tWB@diego> <1435132455.28866.21.camel@mtksdaap41> <1510058.fTE6dlSRsH@diego> <1435655229.19330.15.camel@mtksdaap41> <20150701064935.GC18611@pengutronix.de> <1435806379.3526.31.camel@mtksdaap41> From: Daniel Kurtz Date: Thu, 2 Jul 2015 12:23:40 +0800 X-Google-Sender-Auth: kunyOf3Bo55REZOppiSvgvDNNWM Message-ID: Subject: Re: [PATCH] arm64: dts: mt8173: add clock_null To: James Liao Cc: Sascha Hauer , =?UTF-8?Q?Heiko_St=C3=BCbner?= , "open list:OPEN FIRMWARE AND..." , Mike Turquette , Stephen Boyd , "linux-kernel@vger.kernel.org" , linux-mediatek@lists.infradead.org, ted.lin@mediatek.com, Matthias Brugger , Eddie Huang , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4098 Lines: 97 On Thu, Jul 2, 2015 at 11:06 AM, James Liao wrote: > Hi Daniel, > > On Wed, 2015-07-01 at 19:54 +0800, Daniel Kurtz wrote: >> On Wed, Jul 1, 2015 at 2:49 PM, Sascha Hauer wrote: >> > The problem is not that you use fixed clocks for non software >> > controllable clocks of unknwown rates, but that you try to use a single >> > clock for all of them and name it 'dummy' or 'null'. Name it >> > >> > dpi_ck { >> > compatible = "fixed-clock"; >> > rate = <0>; /* unknown, generated by some Analog block */ >> > }; >> >> It would be nice, though, to use the real clock rates. >> Otherwise, we end up with a bunch of unknown clock rates, like this: >> >> clock enable_cnt prepare_cnt rate >> accuracy phase >> ---------------------------------------------------------------------------------------- >> clk_null 2 2 0 >> 0 0 >> mm_lvds_cts 0 0 0 >> 0 0 >> mm_lvds_pixel 0 0 0 >> 0 0 >> mm_dpi1_pixel 0 0 0 >> 0 0 > >> Furthermore, at least some of these children clocks are possible >> source clocks for other clocks for which we do want to know the >> resulting frequency. For example, the "dmpll_*" clocks are mux inputs >> for many of the subsystem clocks. > > 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? 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. 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. -Dan > So we can't > use a hard-coded rate for them. And we also don't care the actual rate > of these clocks, so assign a dummy rate such as 0 to them should be a > better way. > > > Best regards, > > james -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/