linux-amlogic.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Jian Hu <jian.hu@amlogic.com>
To: Jerome Brunet <jbrunet@baylibre.com>, Stephen Boyd <sboyd@kernel.org>
Cc: devicetree@vger.kernel.org, Jianxin Pan <jianxin.pan@amlogic.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Michael Turquette <mturquette@baylibre.com>,
	linux-kernel@vger.kernel.org,
	Qiufang Dai <qiufang.dai@amlogic.com>,
	linux-amlogic@lists.infradead.org, linux-clk@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver
Date: Mon, 14 Oct 2019 21:42:32 +0800	[thread overview]
Message-ID: <8ae988c2-68f5-603e-843b-9cd70e4d4349@amlogic.com> (raw)
In-Reply-To: <1jsgnz20jq.fsf@starbuckisacylon.baylibre.com>

Hi, Jerome

sorry, it's my problem.

I did not discribe clearly the issue.

On 2019/10/11 15:39, Jerome Brunet wrote:
> 
> On Tue 08 Oct 2019 at 10:03, Jian Hu <jian.hu@amlogic.com> wrote:
> 
>> Hi, Jerome
>>
>> PLL clocks and peripheral clocks rely on each other.
>>
>> for fixed_pll, we can describe its parent like this:
>>
>> xtal-->xtal_fixpll-->fixed_dco-->fixed_pll
>>
>> xtal fixpll is belong to peripheral region.
>> fixed_pll/fclk_div2/fclk_div3 is belong to PLL region.
>>
>> if PLL clocks probe first, it will fail to get xtal_fixpll.
>> we can not get fixed_dco's parent clock.
>>
>> if peripheral clocks probe first, it will fail to get
>> fixed_pll clocks. A lot of peripheral clocks parent are fclk_div2/3/5/7.
>> and we can not get fclk_div2/3/5/7 clocks.
> 
> What does "fail" mean ?
> I intended to get clock using devm_clk_get API in each driver, In this 
scene,it will get failed because of the clock not being reigstered. In 
fact, we could not use devm_clk_get.
>>
>> I can think of two solutions:
>> 1) Do not describe xtal_fixpll, xtal_hifipll.
>>     that is to say, do not decribe the SYS_OSCIN_CTRL register.
>>
>> 2) Put peripheral and pll clock driver in one driver.
> 
> Those are work arounds. Actually fixing the problem is usually
> preferable.
> 
>   So if rephrase your problem:
> 
>   * We have 2 clock controllers (A and B)
>   * Clock are passed between the controllers using DT
>   * We have a PLL in controller B which is used by clocks in
>     controller A.
>   * the PLL parent clock is in controller A.
> 
Yeah, it is the scene.
> => So if I understand correctly you are saying that it will "fail"
> because there is a circular dependency between controller A and B, right
> ?
> 
> Do you have evidence that your problem comes from this circular
> dependency ?
> 
I have realized the peripheral driver and PLL drivers,

PLL driver probes first, Peripheral clock driver is the second.

In addition,for A1 SoC, it will not work using meson_clk_pll_ops,

it needs strictly sequence,so maybe another ops is required.hifi pll 
will be sent with sys pll and CPU clock driver.
> AFAIK, CCF will orphan the clock and continue if the parent is not
> available. Later, when the parent comes up, the orphan will be
> reparented.
> 
> IOW, the problem you are reporting should already be covered by CCF.
> 
>>
>> And  which sulution is better above two?
> 
> Neither, I'm afraid
> 
>>
>> Or maybe other good ideas for it?
> 
> My bet would be that an important clocks (maybe more than 1) is being
> gated during the init process.
> 
> Maybe you should try the command line parameter "clk_ignore_unused"
> until you get things running with your 2 controllers.
> 
>>
>> On 2019/9/29 17:38, Jian Hu wrote:
>>>
>>>
>>> On 2019/9/27 21:32, Jerome Brunet wrote:
>>>>
>>>> On Fri 27 Sep 2019 at 11:52, Jian Hu <jian.hu@amlogic.com> wrote:
>>>>
>>>>> Hi, Jerome
>>>>>
>>>>> Thank you for review.
>>>>>
>>>>> On 2019/9/25 23:09, Jerome Brunet wrote:
>>>>>> On Wed 25 Sep 2019 at 19:44, Jian Hu <jian.hu@amlogic.com> wrote:
>>>>>>
>>>>>>> The Amlogic A1 clock includes three parts:
>>>>>>> peripheral clocks, pll clocks, CPU clocks.
>>>>>>> sys pll and CPU clocks will be sent in next patch.
>>>>>>>
>>>>>>> Unlike the previous series, there is no EE/AO domain
>>>>>>> in A1 CLK controllers.
>>>>>>>
>>>>>>> Signed-off-by: Jian Hu <jian.hu@amlogic.com>
>>>>>>> Signed-off-by: Jianxin Pan <jianxin.pan@amlogic.com>
>>>>>>> ---
>>>>>>>     arch/arm64/Kconfig.platforms |    1 +
>>>>>>>     drivers/clk/meson/Kconfig    |   10 +
>>>>>>>     drivers/clk/meson/Makefile   |    1 +
>>>>>>>     drivers/clk/meson/a1.c       | 2617
>>>>>>> ++++++++++++++++++++++++++++++++++++++++++
>>>>>>>     drivers/clk/meson/a1.h       |  172 +++
>>>>>>>     5 files changed, 2801 insertions(+)
>>>>>>>     create mode 100644 drivers/clk/meson/a1.c
>>>>>>>     create mode 100644 drivers/clk/meson/a1.h
>>>>>>>
>> [...]
>>>>>>> diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
>>>>>>> new file mode 100644
>>>>>>> index 0000000..26edae0f
>>>>>>> --- /dev/null
>>>>>>> +++ b/drivers/clk/meson/a1.c
>>>>>>> @@ -0,0 +1,2617 @@
>>>>>>> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
>>>>>>> +/*
>>>>>>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
>>>>>>> + */
>>>>>>> +
>>>>>>> +#include <linux/clk-provider.h>
>>>>>>> +#include <linux/init.h>
>>>>>>> +#include <linux/of_device.h>
>>>>>>> +#include <linux/platform_device.h>
>>>>>>> +#include <linux/of_address.h>
>>>>>>> +#include "clk-mpll.h"
>>>>>>> +#include "clk-pll.h"
>>>>>>> +#include "clk-regmap.h"
>>>>>>> +#include "vid-pll-div.h"
>>>>>>> +#include "clk-dualdiv.h"
>>>>>>> +#include "meson-eeclk.h"
>>>>>>> +#include "a1.h"
>>>>>>> +
>>>>>>> +/* PLLs clock in gates, its parent is xtal */
>>>>>>> +static struct clk_regmap a1_xtal_clktree = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = SYS_OSCIN_CTRL,
>>>>>>> +        .bit_idx = 0,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data) {
>>>>>>> +        .name = "xtal_clktree",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_data = &(const struct clk_parent_data) {
>>>>>>> +            .fw_name = "xtal",
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        .flags = CLK_IS_CRITICAL,
>>>>>>
>>>>>> Is CCF even expected to touch this ever ? what about RO ops ?
>>>>>> Please review your other clocks with this in mind
>>>>>>
>>>>> the clock should not be changed at runtime.clk_regmap_gate_ro_ops
>>>>> is a good idea. Set RO ops and remove the CLK_IS_CRITICAL flag.
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_regmap a1_xtal_fixpll = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = SYS_OSCIN_CTRL,
>>>>>>> +        .bit_idx = 1,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data) {
>>>>>>> +        .name = "xtal_fixpll",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_data = &(const struct clk_parent_data) {
>>>>>>> +            .fw_name = "xtal",
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        .flags = CLK_IS_CRITICAL,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_regmap a1_xtal_usb_phy = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = SYS_OSCIN_CTRL,
>>>>>>> +        .bit_idx = 2,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data) {
>>>>>>> +        .name = "xtal_usb_phy",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_data = &(const struct clk_parent_data) {
>>>>>>> +            .fw_name = "xtal",
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        .flags = CLK_IS_CRITICAL,
>>>>>>
>>>>>> How is an USB clock critical to the system ?
>>>>>> Please review your other clocks with comment in mind ...
>>>>> the usb clock does not affect the system,
>>>>> remove the CLK_IS_CRITICAL flag
>>>>>>
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_regmap a1_xtal_usb_ctrl = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = SYS_OSCIN_CTRL,
>>>>>>> +        .bit_idx = 3,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data) {
>>>>>>> +        .name = "xtal_usb_ctrl",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_data = &(const struct clk_parent_data) {
>>>>>>> +            .fw_name = "xtal",
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        .flags = CLK_IS_CRITICAL,
>>>>>>> +    },
>>>>>>> +};
>>>>> the usb clock does not affect the system,
>>>>> remove the CLK_IS_CRITICAL flag
>>>>>>> +
>>>>>>> +static struct clk_regmap a1_xtal_hifipll = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = SYS_OSCIN_CTRL,
>>>>>>> +        .bit_idx = 4,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data) {
>>>>>>> +        .name = "xtal_hifipll",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_data = &(const struct clk_parent_data) {
>>>>>>> +            .fw_name = "xtal",
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        .flags = CLK_IS_CRITICAL,
>>>>>>> +    },
>>>>>>> +};
>>>>> CLK_IS_CRITICAL is need to lock hifi pll.
>>>>
>>>> That's not how CCF works, this falg is not ok here.
>>>> CCF will enable this clock before calling enable on your hifi pll
>>>>
>>> ok, I will remove it.
>>>>>>> +
>>>>>>> +static struct clk_regmap a1_xtal_syspll = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = SYS_OSCIN_CTRL,
>>>>>>> +        .bit_idx = 5,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data) {
>>>>>>> +        .name = "xtal_syspll",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_data = &(const struct clk_parent_data) {
>>>>>>> +            .fw_name = "xtal",
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        .flags = CLK_IS_CRITICAL,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>> when CPU clock is at fixed clock, sys pll
>>>>> will be disabled, xtal_syspll will be disabled too.
>>>>> when setting sys pll, call clk_set_rate to lock
>>>>> sys pll, add RO ops to avoid disabling the clock
>>>>
>>>> Again not Ok.
>>>> If you mechanism to lock the PLL is properly implemented in the enable
>>>> callback of the sys pll, still kind of work around are not needed
>>>>
>>>> This has worked on the pll we had so far.
>>>>
>>> ok, I will remove it.
>>>>>
>>>>>>> +static struct clk_regmap a1_xtal_dds = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = SYS_OSCIN_CTRL,
>>>>>>> +        .bit_idx = 6,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data) {
>>>>>>> +        .name = "xtal_dds",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_data = &(const struct clk_parent_data) {
>>>>>>> +            .fw_name = "xtal",
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        .flags = CLK_IS_CRITICAL,
>>>>>>> +    },
>>>>>>> +};
>>>>> CLK_IS_CRITICAL is need to lock dds
>>>>>>> +
>>>>>>> +/* fixed pll = 1536M
>>>>>>> + *
>>>>>>> + * fixed pll ----- fclk_div2 = 768M
>>>>>>> + *           |
>>>>>>> + *           ----- fclk_div3 = 512M
>>>>>>> + *           |
>>>>>>> + *           ----- fclk_div5 = 307.2M
>>>>>>> + *           |
>>>>>>> + *           ----- fclk_div7 = 219.4M
>>>>>>> + */
>>>>>>
>>>>>> The framework will make those calculation ... you can remove this
>>>>>>
>>>>> ok, I will remote the comment.
>>>>>>> +static struct clk_regmap a1_fixed_pll_dco = {
>>>>>>> +    .data = &(struct meson_clk_pll_data){
>>>>>>> +        .en = {
>>>>>>> +            .reg_off = ANACTRL_FIXPLL_CTRL0,
>>>>>>> +            .shift   = 28,
>>>>>>> +            .width   = 1,
>>>>>>> +        },
>>>>>>> +        .m = {
>>>>>>> +            .reg_off = ANACTRL_FIXPLL_CTRL0,
>>>>>>> +            .shift   = 0,
>>>>>>> +            .width   = 8,
>>>>>>> +        },
>>>>>>> +        .n = {
>>>>>>> +            .reg_off = ANACTRL_FIXPLL_CTRL0,
>>>>>>> +            .shift   = 10,
>>>>>>> +            .width   = 5,
>>>>>>> +        },
>>>>>>> +        .frac = {
>>>>>>> +            .reg_off = ANACTRL_FIXPLL_CTRL1,
>>>>>>> +            .shift   = 0,
>>>>>>> +            .width   = 19,
>>>>>>> +        },
>>>>>>> +        .l = {
>>>>>>> +            .reg_off = ANACTRL_FIXPLL_CTRL0,
>>>>>>> +            .shift   = 31,
>>>>>>> +            .width   = 1,
>>>>>>> +        },
>>>>>>> +        .rst = {
>>>>>>> +            .reg_off = ANACTRL_FIXPLL_CTRL0,
>>>>>>> +            .shift   = 29,
>>>>>>> +            .width   = 1,
>>>>>>> +        },
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>> +        .name = "fixed_pll_dco",
>>>>>>> +        .ops = &meson_clk_pll_ro_ops,
>>>>>>> +        .parent_hws = (const struct clk_hw *[]) {
>>>>>>> +            &a1_xtal_fixpll.hw
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_regmap a1_fixed_pll = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = ANACTRL_FIXPLL_CTRL0,
>>>>>>> +        .bit_idx = 20,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data) {
>>>>>>> +        .name = "fixed_pll",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_hws = (const struct clk_hw *[]) {
>>>>>>> +            &a1_fixed_pll_dco.hw
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        .flags = CLK_IGNORE_UNUSED,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static const struct pll_params_table a1_hifi_pll_params_table[] = {
>>>>>>> +    PLL_PARAMS(128, 5), /* DCO = 614.4M */
>>>>>>> +};
>>>>>>> +
>>>>>>> +static const struct reg_sequence a1_hifi_init_regs[] = {
>>>>>>> +    { .reg = ANACTRL_HIFIPLL_CTRL1,    .def = 0x01800000 },
>>>>>>> +    { .reg = ANACTRL_HIFIPLL_CTRL2,    .def = 0x00001100 },
>>>>>>> +    { .reg = ANACTRL_HIFIPLL_CTRL3,    .def = 0x10022200 },
>>>>>>> +    { .reg = ANACTRL_HIFIPLL_CTRL4,    .def = 0x00301000 },
>>>>>>> +    { .reg = ANACTRL_HIFIPLL_CTRL0, .def = 0x01f19480 },
>>>>>>> +    { .reg = ANACTRL_HIFIPLL_CTRL0, .def = 0x11f19480, .delay_us =
>>>>>>> 10 },
>>>>>>> +    { .reg = ANACTRL_HIFIPLL_CTRL0,    .def = 0x15f11480, .delay_us
>>>>>>> = 40 },
>>>>>>> +    { .reg = ANACTRL_HIFIPLL_CTRL2,    .def = 0x00001140 },
>>>>>>> +    { .reg = ANACTRL_HIFIPLL_CTRL2,    .def = 0x00001100 },
>>>>>>> +};
>>>>>>> +
>>>>>>> +/*
>>>>>>> + * The Meson A1 HIFI PLL is 614.4M, it requires
>>>>>>> + * a strict register sequence to enable the PLL.
>>>>>>> + * set meson_clk_pcie_pll_ops as its ops
>>>>>>> + */
>>>>>>
>>>>>> Could you elaborate on this ? What need to be done to enable the clock
>>>>>> ?
>>>>>> Also the HIFI PLL used to be able to do a *LOT* of different rate which
>>>>>> might be desirable for audio use case. Why is this one restricted to
>>>>>> one
>>>>>> particular rate ?
>>>>>>
>>>>> The audio working frequency are 44.1khz, 48khz and 192khz.
>>>>>
>>>>> 614.4M can meet the three frequency.
>>>>>
>>>>> after the hifi pll, there are two dividers in Audio clock.
>>>>>
>>>>> 614.4M/3200 = 192khz
>>>>>
>>>>> 614.4M/12800 = 48khz
>>>>>
>>>>> 614,4M/13932 = 44.0999khz
>>>>
>>>> It does not really answer my question though.
>>>> You are locking a use case here, which is 32 bit sample width
>>>>
>>>> We have other constraint with the upstream audio driver, and we usually
>>>> looking for base frequency that a multiple of 768 (24*32).
>>>>
>>>> If you need your PLL to be set to a particular rate for a use case, the
>>>> correct way is "assigned-rate" in DT
>>>>
>>>> so the question still stands, the HIFI pll before was pretty easy to set
>>>> at a wide variety of rate (same as GP0) ... is it not the case anymore ?
>>>> If yes, could you decribe the constraints.
>>>>
>>>> All this took us a long time to figure out on our own, which is why I'd
>>>> prefer to get the proper constraints in from the beginning this time
>>>>
>>> ok, I will verify it and  describe the constraints about it
>>>>
>>>>>
>>>>>>> +static struct clk_regmap a1_hifi_pll = {
>>>>>>> +    .data = &(struct meson_clk_pll_data){
>>>>>>> +        .en = {
>>>>>>> +            .reg_off = ANACTRL_HIFIPLL_CTRL0,
>>>>>>> +            .shift   = 28,
>>>>>>> +            .width   = 1,
>>>>>>> +        },
>>>>>>> +        .m = {
>>>>>>> +            .reg_off = ANACTRL_HIFIPLL_CTRL0,
>>>>>>> +            .shift   = 0,
>>>>>>> +            .width   = 8,
>>>>>>> +        },
>>>>>>> +        .n = {
>>>>>>> +            .reg_off = ANACTRL_HIFIPLL_CTRL0,
>>>>>>> +            .shift   = 10,
>>>>>>> +            .width   = 5,
>>>>>>> +        },
>>>>>>> +        .frac = {
>>>>>>> +            .reg_off = ANACTRL_HIFIPLL_CTRL1,
>>>>>>> +            .shift   = 0,
>>>>>>> +            .width   = 19,
>>>>>>> +        },
>>>>>>> +        .l = {
>>>>>>> +            .reg_off = ANACTRL_HIFIPLL_STS,
>>>>>>> +            .shift   = 31,
>>>>>>> +            .width   = 1,
>>>>>>> +        },
>>>>>>> +        .table = a1_hifi_pll_params_table,
>>>>>>> +        .init_regs = a1_hifi_init_regs,
>>>>>>> +        .init_count = ARRAY_SIZE(a1_hifi_init_regs),
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>> +        .name = "hifi_pll",
>>>>>>> +        .ops = &meson_clk_pcie_pll_ops,
>>>>>>> +        .parent_hws = (const struct clk_hw *[]) {
>>>>>>> +            &a1_xtal_hifipll.hw
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_fixed_factor a1_fclk_div2_div = {
>>>>>>> +    .mult = 1,
>>>>>>> +    .div = 2,
>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>> +        .name = "fclk_div2_div",
>>>>>>> +        .ops = &clk_fixed_factor_ops,
>>>>>>> +        .parent_hws = (const struct clk_hw *[]) {
>>>>>>> +            &a1_fixed_pll.hw
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_regmap a1_fclk_div2 = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = ANACTRL_FIXPLL_CTRL0,
>>>>>>> +        .bit_idx = 21,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>> +        .name = "fclk_div2",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_hws = (const struct clk_hw *[]) {
>>>>>>> +            &a1_fclk_div2_div.hw
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        /*
>>>>>>> +         * add CLK_IS_CRITICAL flag to avoid being disabled by clk
>>>>>>> core
>>>>>>> +         * or its children clocks.
>>>>>>> +         */
>>>>>>
>>>>>> The meaning of this flag is already documented in clk-provider.h
>>>>>> The reason why you need this flag is lot more interesting here ...
>>>>>>
>>>>>> Same below
>>>>> ok, I will replace new comments here.
>>>>>>
>>>>>>> +        .flags = CLK_IS_CRITICAL,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_fixed_factor a1_fclk_div3_div = {
>>>>>>> +    .mult = 1,
>>>>>>> +    .div = 3,
>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>> +        .name = "fclk_div3_div",
>>>>>>> +        .ops = &clk_fixed_factor_ops,
>>>>>>> +        .parent_hws = (const struct clk_hw *[]) {
>>>>>>> +            &a1_fixed_pll.hw
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_regmap a1_fclk_div3 = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = ANACTRL_FIXPLL_CTRL0,
>>>>>>> +        .bit_idx = 22,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>> +        .name = "fclk_div3",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_hws = (const struct clk_hw *[]) {
>>>>>>> +            &a1_fclk_div3_div.hw
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        /*
>>>>>>> +         * add CLK_IS_CRITICAL flag to avoid being disabled by clk
>>>>>>> core
>>>>>>> +         * its children clocks.
>>>>>>> +         */
>>>>>>> +        .flags = CLK_IS_CRITICAL,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_fixed_factor a1_fclk_div5_div = {
>>>>>>> +    .mult = 1,
>>>>>>> +    .div = 5,
>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>> +        .name = "fclk_div5_div",
>>>>>>> +        .ops = &clk_fixed_factor_ops,
>>>>>>> +        .parent_hws = (const struct clk_hw *[]) {
>>>>>>> +            &a1_fixed_pll.hw
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_regmap a1_fclk_div5 = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = ANACTRL_FIXPLL_CTRL0,
>>>>>>> +        .bit_idx = 23,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>> +        .name = "fclk_div5",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_hws = (const struct clk_hw *[]) {
>>>>>>> +            &a1_fclk_div5_div.hw
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        /*
>>>>>>> +         * add CLK_IS_CRITICAL flag to avoid being disabled by clk
>>>>>>> core
>>>>>>> +         * its children clocks.
>>>>>>> +         */
>>>>>>> +        .flags = CLK_IS_CRITICAL,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_fixed_factor a1_fclk_div7_div = {
>>>>>>> +    .mult = 1,
>>>>>>> +    .div = 7,
>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>> +        .name = "fclk_div7_div",
>>>>>>> +        .ops = &clk_fixed_factor_ops,
>>>>>>> +        .parent_hws = (const struct clk_hw *[]) {
>>>>>>> +            &a1_fixed_pll.hw
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>>>>>>> +static struct clk_regmap a1_fclk_div7 = {
>>>>>>> +    .data = &(struct clk_regmap_gate_data){
>>>>>>> +        .offset = ANACTRL_FIXPLL_CTRL0,
>>>>>>> +        .bit_idx = 23,
>>>>>>> +    },
>>>>>>> +    .hw.init = &(struct clk_init_data){
>>>>>>> +        .name = "fclk_div7",
>>>>>>> +        .ops = &clk_regmap_gate_ops,
>>>>>>> +        .parent_hws = (const struct clk_hw *[]) {
>>>>>>> +            &a1_fclk_div7_div.hw
>>>>>>> +        },
>>>>>>> +        .num_parents = 1,
>>>>>>> +        /*
>>>>>>> +         * add CLK_IS_CRITICAL flag to avoid being disabled by clk
>>>>>>> core
>>>>>>> +         * or its children clock.
>>>>>>> +         */
>>>>>>> +        .flags = CLK_IS_CRITICAL,
>>>>>>> +    },
>>>>>>> +};
>>>>>>> +
>> [...]
>>>>>>> -- 
>>>>>>> 1.9.1
>>>>>>
>>>>>> .
>>>>>>
>>>>
>>>> .
>>>>
> 
> .
> 

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic

  reply	other threads:[~2019-10-14 13:43 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-25 11:44 [PATCH 0/2] add Amlogic A1 clock controller driver Jian Hu
2019-09-25 11:44 ` [PATCH 1/2] dt-bindings: clock: meson: add A1 clock controller bindings Jian Hu
2019-09-25 14:29   ` Jerome Brunet
2019-09-27  6:08     ` Jian Hu
2019-09-27 20:10   ` Rob Herring
2019-12-18  7:30     ` Jian Hu
2019-09-25 11:44 ` [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver Jian Hu
2019-09-25 13:12   ` Stephen Boyd
2019-09-27  3:11     ` Jian Hu
2019-09-27 12:56       ` Jerome Brunet
2019-09-29  6:15         ` Jian Hu
2019-09-25 15:09   ` Jerome Brunet
2019-09-27  9:52     ` Jian Hu
2019-09-27 13:32       ` Jerome Brunet
2019-09-29  9:38         ` Jian Hu
2019-10-08  8:03           ` Jian Hu
2019-10-11  7:39             ` Jerome Brunet
2019-10-14 13:42               ` Jian Hu [this message]
2019-10-14 14:55                 ` Jerome Brunet
2019-10-18  3:42                   ` Jian Hu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8ae988c2-68f5-603e-843b-9cd70e4d4349@amlogic.com \
    --to=jian.hu@amlogic.com \
    --cc=devicetree@vger.kernel.org \
    --cc=jbrunet@baylibre.com \
    --cc=jianxin.pan@amlogic.com \
    --cc=khilman@baylibre.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=mturquette@baylibre.com \
    --cc=narmstrong@baylibre.com \
    --cc=qiufang.dai@amlogic.com \
    --cc=sboyd@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).