[PATCH] arm64: dts: mt8173: add clock_null

[Daniel Kurtz]

On Thu, Jul 2, 2015 at 11:06 AM, James Liao <jamesjj.liao at mediatek.com> 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 <s.hauer at pengutronix.de> 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