[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5E775D64.8080008@wanyeetech.com>
Date: Sun, 22 Mar 2020 20:43:16 +0800
From: Zhou Yanjie <zhouyanjie@...yeetech.com>
To: Paul Cercueil <paul@...pouillou.net>
Cc: linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, sboyd@...nel.org,
mturquette@...libre.com, robh+dt@...nel.org, mark.rutland@....com,
dongsheng.qiu@...enic.com, yanfei.li@...enic.com,
sernia.zhou@...mail.com, zhenwenjin@...il.com
Subject: Re: [PATCH v6 6/6] clk: X1000: Add FIXDIV for SSI clock of X1000.
Hi Paul,
On 2020年03月22日 20:12, Paul Cercueil wrote:
> Hi Zhou,
>
> Le dim. 22 mars 2020 à 16:21, 周琰杰 (Zhou Yanjie)
> <zhouyanjie@...yeetech.com> a écrit :
>> 1.The SSI clock of X1000 not like JZ4770 and JZ4780, they are not
>> directly derived from the output of SSIPLL, but from the clock
>> obtained by dividing the frequency by 2. "X1000_CLK_SSIPLL_DIV2"
>> is added for this purpose, and ensure that it initialized before
>> "X1000_CLK_SSIMUX" when initializing the clocks.
>> 2.Clocks of LCD, OTG, EMC, EFUSE, OST, and gates of CPU, PCLK
>> are also added.
>>
>> Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@...yeetech.com>
>> ---
>>
>> Notes:
>> v5:
>> New patch.
>>
>> V5->v6:
>> Add missing part of X1000's CGU.
>>
>> drivers/clk/ingenic/x1000-cgu.c | 56
>> ++++++++++++++++++++++++++++++++++++-----
>> 1 file changed, 50 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/clk/ingenic/x1000-cgu.c
>> b/drivers/clk/ingenic/x1000-cgu.c
>> index 6f0ec9d..7e3ef0d 100644
>> --- a/drivers/clk/ingenic/x1000-cgu.c
>> +++ b/drivers/clk/ingenic/x1000-cgu.c
>> @@ -1,7 +1,7 @@
>> // SPDX-License-Identifier: GPL-2.0
>> /*
>> * X1000 SoC CGU driver
>> - * Copyright (c) 2019 Zhou Yanjie <zhouyanjie@...o.com>
>> + * Copyright (c) 2019 周琰杰 (Zhou Yanjie) <zhouyanjie@...yeetech.com>
>> */
>>
>> #include <linux/clk-provider.h>
>> @@ -18,6 +18,7 @@
>> #define CGU_REG_CLKGR 0x20
>> #define CGU_REG_OPCR 0x24
>> #define CGU_REG_DDRCDR 0x2c
>> +#define CGU_REG_USBCDR 0x50
>> #define CGU_REG_MACCDR 0x54
>> #define CGU_REG_I2SCDR 0x60
>> #define CGU_REG_LPCDR 0x64
>> @@ -114,9 +115,10 @@ static const struct ingenic_cgu_clk_info
>> x1000_cgu_clocks[] = {
>> },
>>
>> [X1000_CLK_CPU] = {
>> - "cpu", CGU_CLK_DIV,
>> + "cpu", CGU_CLK_DIV | CGU_CLK_GATE,
>> .parents = { X1000_CLK_CPUMUX, -1, -1, -1 },
>> .div = { CGU_REG_CPCCR, 0, 1, 4, 22, -1, -1 },
>> + .gate = { CGU_REG_CLKGR, 30 },
>> },
>>
>> [X1000_CLK_L2CACHE] = {
>> @@ -145,9 +147,10 @@ static const struct ingenic_cgu_clk_info
>> x1000_cgu_clocks[] = {
>> },
>>
>> [X1000_CLK_PCLK] = {
>> - "pclk", CGU_CLK_DIV,
>> + "pclk", CGU_CLK_DIV | CGU_CLK_GATE,
>> .parents = { X1000_CLK_AHB2PMUX, -1, -1, -1 },
>> .div = { CGU_REG_CPCCR, 16, 1, 4, 20, -1, -1 },
>> + .gate = { CGU_REG_CLKGR, 28 },
>> },
>>
>> [X1000_CLK_DDR] = {
>> @@ -160,12 +163,20 @@ static const struct ingenic_cgu_clk_info
>> x1000_cgu_clocks[] = {
>>
>> [X1000_CLK_MAC] = {
>> "mac", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE,
>> - .parents = { X1000_CLK_SCLKA, X1000_CLK_MPLL},
>> + .parents = { X1000_CLK_SCLKA, X1000_CLK_MPLL },
>> .mux = { CGU_REG_MACCDR, 31, 1 },
>> .div = { CGU_REG_MACCDR, 0, 1, 8, 29, 28, 27 },
>> .gate = { CGU_REG_CLKGR, 25 },
>> },
>>
>> + [X1000_CLK_LCD] = {
>> + "lcd", CGU_CLK_MUX | CGU_CLK_DIV | CGU_CLK_GATE,
>> + .parents = { X1000_CLK_SCLKA, X1000_CLK_MPLL },
>> + .mux = { CGU_REG_LPCDR, 31, 1 },
>> + .div = { CGU_REG_LPCDR, 0, 1, 8, 28, 27, 26 },
>> + .gate = { CGU_REG_CLKGR, 23 },
>> + },
>> +
>> [X1000_CLK_MSCMUX] = {
>> "msc_mux", CGU_CLK_MUX,
>> .parents = { X1000_CLK_SCLKA, X1000_CLK_MPLL},
>> @@ -186,6 +197,15 @@ static const struct ingenic_cgu_clk_info
>> x1000_cgu_clocks[] = {
>> .gate = { CGU_REG_CLKGR, 5 },
>> },
>>
>> + [X1000_CLK_OTG] = {
>> + "otg", CGU_CLK_DIV | CGU_CLK_GATE | CGU_CLK_MUX,
>> + .parents = { X1000_CLK_EXCLK, -1,
>> + X1000_CLK_APLL, X1000_CLK_MPLL },
>> + .mux = { CGU_REG_USBCDR, 30, 2 },
>> + .div = { CGU_REG_USBCDR, 0, 1, 8, 29, 28, 27 },
>> + .gate = { CGU_REG_CLKGR, 3 },
>> + },
>> +
>> [X1000_CLK_SSIPLL] = {
>> "ssi_pll", CGU_CLK_MUX | CGU_CLK_DIV,
>> .parents = { X1000_CLK_SCLKA, X1000_CLK_MPLL, -1, -1 },
>> @@ -193,14 +213,32 @@ static const struct ingenic_cgu_clk_info
>> x1000_cgu_clocks[] = {
>> .div = { CGU_REG_SSICDR, 0, 1, 8, 29, 28, 27 },
>> },
>>
>> + [X1000_CLK_SSIPLL_DIV2] = {
>> + "ssi_pll_div2", CGU_CLK_FIXDIV,
>> + .parents = { X1000_CLK_SSIPLL },
>> + .fixdiv = { 2 },
>> + },
>> +
>> [X1000_CLK_SSIMUX] = {
>> "ssi_mux", CGU_CLK_MUX,
>> - .parents = { X1000_CLK_EXCLK, X1000_CLK_SSIPLL, -1, -1 },
>> + .parents = { X1000_CLK_EXCLK, X1000_CLK_SSIPLL_DIV2, -1, -1 },
>> .mux = { CGU_REG_SSICDR, 30, 1 },
>> },
>>
>> /* Gate-only clocks */
>>
>> + [X1000_CLK_EMC] = {
>> + "emc", CGU_CLK_GATE,
>> + .parents = { X1000_CLK_AHB2, -1, -1, -1 },
>> + .gate = { CGU_REG_CLKGR, 0 },
>> + },
>> +
>> + [X1000_CLK_EFUSE] = {
>> + "efuse", CGU_CLK_GATE,
>> + .parents = { X1000_CLK_AHB2, -1, -1, -1 },
>> + .gate = { CGU_REG_CLKGR, 1 },
>> + },
>> +
>> [X1000_CLK_SFC] = {
>> "sfc", CGU_CLK_GATE,
>> .parents = { X1000_CLK_SSIPLL, -1, -1, -1 },
>> @@ -249,6 +287,12 @@ static const struct ingenic_cgu_clk_info
>> x1000_cgu_clocks[] = {
>> .gate = { CGU_REG_CLKGR, 19 },
>> },
>>
>> + [X1000_CLK_OST] = {
>> + "ost", CGU_CLK_GATE,
>> + .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
>> + .gate = { CGU_REG_CLKGR, 20 },
>> + },
>> +
>> [X1000_CLK_PDMA] = {
>> "pdma", CGU_CLK_GATE,
>> .parents = { X1000_CLK_EXCLK, -1, -1, -1 },
>> @@ -275,4 +319,4 @@ static void __init x1000_cgu_init(struct
>> device_node *np)
>>
>> ingenic_cgu_register_syscore_ops(cgu);
>> }
>> -CLK_OF_DECLARE(x1000_cgu, "ingenic,x1000-cgu", x1000_cgu_init);
>> +CLK_OF_DECLARE_DRIVER(x1000_cgu, "ingenic,x1000-cgu", x1000_cgu_init);
>
> This change is not listed in the commit message, so it's not obvious
> why it's here. Please explain why it's changed in the commit message.
>
This should be that when x1000-cgu.c was first submitted the old way was
used. In earlier cgu drivers CLK_OF_DECLARE was used here, but other cgu
drivers have now been changed to CLK_OF_DECLARE_DRIVER., and this one
should also be changed. I will mention this in the commit message in the
next version.
Thanks and best regards!
> Cheers,
> -Paul
>
>> --
>> 2.7.4
>>
>
Powered by blists - more mailing lists