Message-ID: <5681F121.3070307@rock-chips.com>
Date:	Tue, 29 Dec 2015 10:34:09 +0800
From:	Xing Zheng <zhengxing@...k-chips.com>
To:	Yakir Yang <ykk@...k-chips.com>
CC:	Heiko Stübner <heiko@...ech.de>,
	Mark Rutland <mark.rutland@....com>,
	devicetree@...r.kernel.org, Pawel Moll <pawel.moll@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Michael Turquette <mturquette@...libre.com>,
	Kumar Gala <galak@...eaurora.org>,
	Stephen Boyd <sboyd@...eaurora.org>,
	linux-kernel@...r.kernel.org, linux-rockchip@...ts.infradead.org,
	Rob Herring <robh+dt@...nel.org>,
	linux-arm-kernel@...ts.infradead.org, keescook@...gle.com,
	linux-clk@...r.kernel.org, leozwang@...gle.com
Subject: Re: [RESEND PATCH v1 4/4] clk: rockchip: rk3036: fix and add node
 id for emac clock

On 2015年12月29日 09:59, Yakir Yang wrote:
> Hi Heiko,
>
> On 12/28/2015 08:41 PM, Heiko Stübner wrote:
>> Hi,
>>
>> Am Montag, 28. Dezember 2015, 17:03:53 schrieb Xing Zheng:
>>> Due to referred old version TRM, there is incorrect emac clock node,
>>> we should fix it. The SEL_21_9 is the parent of SEL_21_4.
>>>
>>> In the emac driver, we need to refer HCLK_MAC, and because There are
>>> only 3PLLs (APLL/GPLL/DPLL) on the rk3036, most clock are under the
>>> GPLL, and it is unable to provide the accurate rate for mac_ref which
>>> need to 50MHz probability, we should let it under the APLL and are
>>> able to set the freq which integer multiples of 50MHz, so we add these
>>> emac node for reference.
>> I don't really follow here. While I do understand that the emac needs 
>> 50MHz, I
>> don't think using the APLL as source is helpful.
>>
>> The APLL is the main clocksource for the cpu-cores, including frequency
>> scaling, and while it currently only lists 816MHz as sole frequency, 
>> you're
>> pretty much guaranteed to not get your correct multiple of 50MHz from 
>> there
>> either. And limiting the cpu to just do 600MHz to get the mac working 
>> sounds
>> pretty bad ;-) .
>>
>>
>> In the rk3036 cru-node the gpll gets set to 594MHz. Is there a 
>> special reason
>> why it needs to be 594MHz and cannot be a round 600MHz? Because that 
>> would
>> also provide your 50MHz-multiple nicely.
>
> Yes, this magic 594MHz would help to support the standard HDMI 
> resolutions, here are the math:
>
> 1920x1080-60Hz DCLK = 148.5MHz = 594MHz / 4
> 1280x720-60Hz DCLK = 74.25MHz = 594MHz / 8
> 720x480-60Hz DCLK = 27MHz = 594MHz / 22
>
> Thanks,
> - Yakir
Thanks Yakir.

Hi Heiko,
 From the above, do you have better idea for the RK3036's emac without 
ext-oscillator?

Thanks. :-)
>
>>> Signed-off-by: Xing Zheng <zhengxing@...k-chips.com>
>>> ---
>>>
>>> drivers/clk/rockchip/clk-rk3036.c | 11 ++++++-----
>>> include/dt-bindings/clock/rk3036-cru.h | 2 ++
>>> 2 files changed, 8 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/clk/rockchip/clk-rk3036.c
>>> b/drivers/clk/rockchip/clk-rk3036.c index 7420cbe..7863c4d 100644
>>> --- a/drivers/clk/rockchip/clk-rk3036.c
>>> +++ b/drivers/clk/rockchip/clk-rk3036.c
>>> @@ -328,13 +328,14 @@ static struct rockchip_clk_branch
>>> rk3036_clk_branches[] __initdata = { RK2928_CLKSEL_CON(16), 0, 2, 
>>> MFLAGS,
>>> 2, 5, DFLAGS,
>>> RK2928_CLKGATE_CON(10), 5, GFLAGS),
>>>
>>> - COMPOSITE_NOGATE(0, "mac_pll_src", mux_pll_src_3plls_p, 0,
>>> - RK2928_CLKSEL_CON(21), 0, 2, MFLAGS, 4, 5, DFLAGS),
>>> + MUX(SCLK_MACPLL, "mac_pll_pre", mux_pll_src_3plls_p, 0,
>>> + RK2928_CLKSEL_CON(21), 0, 2, MFLAGS),
>>> + DIV(0, "mac_pll_src", "mac_pll_pre", 0,
>>> + RK2928_CLKSEL_CON(21), 9, 5, DFLAGS),
>>> MUX(SCLK_MACREF, "mac_clk_ref", mux_mac_p, CLK_SET_RATE_PARENT,
>>> RK2928_CLKSEL_CON(21), 3, 1, MFLAGS),
>>> -
>>> COMPOSITE_NOMUX(SCLK_MAC, "mac_clk", "mac_clk_ref", 0,
>>> - RK2928_CLKSEL_CON(21), 9, 5, DFLAGS,
>>> + RK2928_CLKSEL_CON(21), 4, 5, DFLAGS,
>>> RK2928_CLKGATE_CON(2), 6, GFLAGS),
>>>
>>> MUX(SCLK_HDMI, "dclk_hdmi", mux_dclk_p, 0,
>>> @@ -389,7 +390,7 @@ static struct rockchip_clk_branch 
>>> rk3036_clk_branches[]
>>> __initdata = { GATE(HCLK_OTG1, "hclk_otg1", "hclk_peri", 
>>> CLK_IGNORE_UNUSED,
>>> RK2928_CLKGATE_CON(7), 3, GFLAGS), GATE(HCLK_I2S, "hclk_i2s", 
>>> "hclk_peri",
>>> 0, RK2928_CLKGATE_CON(7), 2, GFLAGS), GATE(0, "hclk_sfc", "hclk_peri",
>>> CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(3), 14, GFLAGS), - GATE(0,
>>> "hclk_mac", "hclk_peri", CLK_IGNORE_UNUSED, RK2928_CLKGATE_CON(3), 15,
>>> GFLAGS), + GATE(HCLK_MAC, "hclk_mac", "hclk_peri", 0,
>>> RK2928_CLKGATE_CON(3), 5, GFLAGS),
>>>
>>> /* pclk_peri gates */
>>> GATE(0, "pclk_peri_matrix", "pclk_peri", CLK_IGNORE_UNUSED,
>>> RK2928_CLKGATE_CON(4), 1, GFLAGS), diff --git
>>> a/include/dt-bindings/clock/rk3036-cru.h
>>> b/include/dt-bindings/clock/rk3036-cru.h index ebc7a7b..de44109 100644
>>> --- a/include/dt-bindings/clock/rk3036-cru.h
>>> +++ b/include/dt-bindings/clock/rk3036-cru.h
>>> @@ -54,6 +54,7 @@
>>> #define SCLK_PVTM_VIDEO 125
>>> #define SCLK_MAC 151
>>> #define SCLK_MACREF 152
>>> +#define SCLK_MACPLL 153
>>> #define SCLK_SFC 160
>>>
>>> /* aclk gates */
>>> @@ -92,6 +93,7 @@
>> please separate the hclk addition into two separate patches:
>> patch1: add the clock-id to the dt-binding header
>> patch2: use the clock in the clock-driver
Done.
>>
>>> #define HCLK_SDMMC 456
>>> #define HCLK_SDIO 457
>>> #define HCLK_EMMC 459
>>> +#define HCLK_MAC 460
>>> #define HCLK_I2S 462
>>> #define HCLK_LCDC 465
>>> #define HCLK_ROM 467
>>
>> Thanks
>> Heiko
>>
>> _______________________________________________
>> Linux-rockchip mailing list
>> Linux-rockchip@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>>
>>
>>
>
>
>


--
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/

Powered by blists - more mailing lists