Received: by 2002:a25:824b:0:0:0:0:0 with SMTP id d11csp5641176ybn; Sun, 29 Sep 2019 02:43:47 -0700 (PDT) X-Google-Smtp-Source: APXvYqzE+9fHbtEZ+uFbiMz0Ccuzsv4iW2KD9kGoNXesxEXA5t+/kP1Ec1cWa4tBZb1UycGE0kOo X-Received: by 2002:a05:6402:383:: with SMTP id o3mr13820559edv.205.1569750227168; Sun, 29 Sep 2019 02:43:47 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1569750227; cv=none; d=google.com; s=arc-20160816; b=fvs31pprMCWD254uy8hh2ZIO63viVxjqSWMuTDIz2yEdHxpAvNSHkZGLW7aYKcJp8f nuVA3r8Dx02vkWYILTWoeenKUNkXdjRBPG4r/Y6+Tii/gWQtt8h/W6kHn7NFMcU3FW7z pQ9ywY36YfRdaRX0lzk1NAVm6PyF2rm0xE/zjwxgYf4CqompZnZJKNjWjyerPUzdQisN 1bwufr+mW1TK7V0vLKr7/NczJcqcnliqoByXwMPzGHqwQpog2JIEcQggfO/0221RsBVT vV51PuMeBEi/inFk63vELN4B3ICXMxKvIFBTP/pj2kyS9L4Lahcm4GZJAGcCE8Ulsrsk drpA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject; bh=85ehfI5QIQw2btQH7mcfUX1ljhTyRLXThc4qJaycnpo=; b=KRBTxMjFKFS5hfpl2ZHIPgYlQzt2Fj9eWr1nnT6S1+jxORlfrmyIqovpzyKFWmbGur OC+i8Oj0WJwCeET4FX/dCIewUmexWPWklafOOfw07GrPpQjnL9i+jyOgXhUF3YumIZmS 10LcqyA3HpbddfRRrnckvDHBB5TeuHupjAJL1ruoMF7C050L/yALoEMLN37NmIJWloKX JMCXgGbgXKYxTfKrNIFaABRUIRcNo97lBqOC9ePUg3xcoPZWiBwUJ8QRDEwbsmwHWeBB 0LDXbqJ1Qx2AmqpGfEjQ8+1AgrN9sQ8bZ1ivb+6Ap3wcnJ2iPdx+E86dT8qq6ZJbALh7 f96w== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id x3si5111233eda.285.2019.09.29.02.43.21; Sun, 29 Sep 2019 02:43:47 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728920AbfI2JlJ (ORCPT + 99 others); Sun, 29 Sep 2019 05:41:09 -0400 Received: from mail-sz.amlogic.com ([211.162.65.117]:22986 "EHLO mail-sz.amlogic.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726018AbfI2JlJ (ORCPT ); Sun, 29 Sep 2019 05:41:09 -0400 Received: from [10.28.19.114] (10.28.19.114) by mail-sz.amlogic.com (10.28.11.5) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1591.10; Sun, 29 Sep 2019 17:38:37 +0800 Subject: Re: [PATCH 2/2] clk: meson: a1: add support for Amlogic A1 clock driver To: Jerome Brunet CC: Neil Armstrong , Jianxin Pan , Kevin Hilman , Rob Herring , Martin Blumenstingl , Michael Turquette , Stephen Boyd , Qiufang Dai , , , , , References: <1569411888-98116-1-git-send-email-jian.hu@amlogic.com> <1569411888-98116-3-git-send-email-jian.hu@amlogic.com> <1j1rw4mmzw.fsf@starbuckisacylon.baylibre.com> <1jimpd27cb.fsf@starbuckisacylon.baylibre.com> From: Jian Hu Message-ID: <5fd57563-0c34-be14-132a-74fd2c5a5275@amlogic.com> Date: Sun, 29 Sep 2019 17:38:38 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.3.3 MIME-Version: 1.0 In-Reply-To: <1jimpd27cb.fsf@starbuckisacylon.baylibre.com> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Originating-IP: [10.28.19.114] X-ClientProxiedBy: mail-sz.amlogic.com (10.28.11.5) To mail-sz.amlogic.com (10.28.11.5) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2019/9/27 21:32, Jerome Brunet wrote: > > On Fri 27 Sep 2019 at 11:52, Jian Hu 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 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 >>>> Signed-off-by: Jianxin Pan >>>> --- >>>> 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/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms >>>> index 16d7614..a48f67d 100644 >>>> --- a/arch/arm64/Kconfig.platforms >>>> +++ b/arch/arm64/Kconfig.platforms >>>> @@ -138,6 +138,7 @@ config ARCH_MESON >>>> select COMMON_CLK_AXG >>>> select COMMON_CLK_G12A >>>> select MESON_IRQ_GPIO >>>> + select COMMON_CLK_A1 >>> >>> This need to be separate patch addressed to Kevin once your driver is merged >>> >> ok, I will remove it in PATCH V2. And send it again after the driver is >> merged. >>>> help >>>> This enables support for the arm64 based Amlogic SoCs >>>> such as the s905, S905X/D, S912, A113X/D or S905X/D2 >>>> diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig >>>> index dabeb43..e6cb4c3 100644 >>>> --- a/drivers/clk/meson/Kconfig >>>> +++ b/drivers/clk/meson/Kconfig >>>> @@ -107,3 +107,13 @@ config COMMON_CLK_G12A >>>> help >>>> Support for the clock controller on Amlogic S905D2, S905X2 and S905Y2 >>>> devices, aka g12a. Say Y if you want peripherals to work. >>>> + >>>> +config COMMON_CLK_A1 >>>> + bool >>>> + depends on ARCH_MESON >>>> + select COMMON_CLK_MESON_REGMAP >>>> + select COMMON_CLK_MESON_DUALDIV >>>> + select COMMON_CLK_MESON_PLL >>>> + help >>>> + Support for the clock controller on Amlogic A113L device, >>>> + aka a1. Say Y if you want peripherals to work. >>>> diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile >>>> index 3939f21..6be3a8f 100644 >>>> --- a/drivers/clk/meson/Makefile >>>> +++ b/drivers/clk/meson/Makefile >>>> @@ -19,3 +19,4 @@ obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o >>>> obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o >>>> obj-$(CONFIG_COMMON_CLK_G12A) += g12a.o g12a-aoclk.o >>>> obj-$(CONFIG_COMMON_CLK_MESON8B) += meson8b.o >>>> +obj-$(CONFIG_COMMON_CLK_A1) += a1.o >>>> 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 >>>> +#include >>>> +#include >>>> +#include >>>> +#include >>>> +#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, >>>> + }, >>>> +}; >>>> + >>>> +/* sys clk = 64M */ >>> >>> Again this comment does not help understanding the driver code, please >>> remove >>> >> got it. >>>> +static u32 mux_table_sys_ab_clk[] = { 0, 1, 2, 3}; >>> >>> This table is useless >>> >>>> +static const struct clk_parent_data sys_ab_clk_parent_data[] = { >>>> + { .fw_name = "xtal", }, >>>> + { .hw = &a1_fclk_div2.hw }, >>>> + { .hw = &a1_fclk_div3.hw }, >>>> + { .hw = &a1_fclk_div5.hw }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_sys_b_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = SYS_CLK_CTRL0, >>>> + .mask = 0x7, >>>> + .shift = 26, >>>> + .table = mux_table_sys_ab_clk, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "sys_b_sel", >>>> + .ops = &clk_regmap_mux_ro_ops, >>>> + .parent_data = sys_ab_clk_parent_data, >>>> + .num_parents = ARRAY_SIZE(sys_ab_clk_parent_data), >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_sys_b_div = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = SYS_CLK_CTRL0, >>>> + .shift = 16, >>>> + .width = 10, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "sys_b_div", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_sys_b_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_sys_b = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = SYS_CLK_CTRL0, >>>> + .bit_idx = 29, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "sys_b", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_sys_b_div.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, >>> >>> Why does this need to be critical ? >>> The a1_sys_clk (leaf of the block) could possibly be critical (if you >>> explain why) ... but the flag should not be needed in the middle of the >>> block >>> >> ok, the a1_sys_clk should not be changed at runtime, it is the same with >> the clk81 clock in G12A. Just use another name in A1. > > This does not justify the clock to be critical. > > #1 Why can't it be changed ? > #2 CLK_IS_CRITICAL does not say that clock can't change, it says it > can't gate ... two different things > > DDR clock's parent clcok is fclk_div2, It can not be gated.I will add add commits. >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_sys_a_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = SYS_CLK_CTRL0, >>>> + .mask = 0x7, >>>> + .shift = 10, >>>> + .table = mux_table_sys_ab_clk, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "sys_a_sel", >>>> + .ops = &clk_regmap_mux_ro_ops, >>>> + .parent_data = sys_ab_clk_parent_data, >>>> + .num_parents = ARRAY_SIZE(sys_ab_clk_parent_data), >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_sys_a_div = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = SYS_CLK_CTRL0, >>>> + .shift = 0, >>>> + .width = 10, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "sys_a_div", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_sys_a_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_sys_a = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = SYS_CLK_CTRL0, >>>> + .bit_idx = 13, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "sys_a", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_sys_a_div.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IS_CRITICAL, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_sys_clk = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = SYS_CLK_CTRL0, >>>> + .mask = 0x1, >>>> + .shift = 31, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "sys_clk", >>>> + .ops = &clk_regmap_mux_ro_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_sys_a.hw, &a1_sys_b.hw, >>>> + }, >>>> + .num_parents = 2, >>> >>> SET_RATE_PARENT ? >>> >> the ops is RO, Maybe no flag is better. > > Is RO for now but setting the rate propagation properly fromt the > beginning is preferred. > > In general, you should have SET_RATE_PARENT everywhere unless there is a > reason to stop the rate propagation. > OK, keep SET_RATE_PARENT here >>>> + }, >>>> +}; >>>> + >>>> +/* rtc 32k clock in */ >>>> +static struct clk_regmap a1_rtc_32k_clkin = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = RTC_BY_OSCIN_CTRL0, >>>> + .bit_idx = 31, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "rtc_32k_clkin", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_data = &(const struct clk_parent_data) { >>>> + .fw_name = "xtal", >>>> + }, >>>> + .num_parents = 1, >>>> + }, >>>> +}; >>>> + >>>> +static const struct meson_clk_dualdiv_param a1_32k_div_table[] = { >>>> + { >>>> + .dual = 1, >>>> + .n1 = 733, >>>> + .m1 = 8, >>>> + .n2 = 732, >>>> + .m2 = 11, >>>> + }, {} >>>> +}; >>>> + >>>> +static struct clk_regmap a1_rtc_32k_div = { >>>> + .data = &(struct meson_clk_dualdiv_data){ >>>> + .n1 = { >>>> + .reg_off = RTC_BY_OSCIN_CTRL0, >>>> + .shift = 0, >>>> + .width = 12, >>>> + }, >>>> + .n2 = { >>>> + .reg_off = RTC_BY_OSCIN_CTRL0, >>>> + .shift = 12, >>>> + .width = 12, >>>> + }, >>>> + .m1 = { >>>> + .reg_off = RTC_BY_OSCIN_CTRL1, >>>> + .shift = 0, >>>> + .width = 12, >>>> + }, >>>> + .m2 = { >>>> + .reg_off = RTC_BY_OSCIN_CTRL1, >>>> + .shift = 12, >>>> + .width = 12, >>>> + }, >>>> + .dual = { >>>> + .reg_off = RTC_BY_OSCIN_CTRL0, >>>> + .shift = 28, >>>> + .width = 1, >>>> + }, >>>> + .table = a1_32k_div_table, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "rtc_32k_div", >>>> + .ops = &meson_clk_dualdiv_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_rtc_32k_clkin.hw >>>> + }, >>>> + .num_parents = 1, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_rtc_32k_xtal = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = RTC_BY_OSCIN_CTRL1, >>>> + .bit_idx = 24, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "rtc_32k_xtal", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_rtc_32k_clkin.hw >>>> + }, >>>> + .num_parents = 1, >>>> + }, >>>> +}; >>>> + >>>> +static u32 rtc_32k_sel[] = { 0, 1 }; >>> >>> useless >>> >> ok, I will delete the table. >>>> + >>>> +static struct clk_regmap a1_rtc_32k_sel = { >>>> + .data = &(struct clk_regmap_mux_data) { >>>> + .offset = RTC_CTRL, >>>> + .mask = 0x3, >>>> + .shift = 0, >>>> + .table = rtc_32k_sel, >>>> + .flags = CLK_MUX_ROUND_CLOSEST, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "rtc_32k_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_rtc_32k_xtal.hw, >>>> + &a1_rtc_32k_div.hw, >>>> + }, >>>> + .num_parents = 2, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +struct clk_regmap a1_rtc_clk = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = RTC_BY_OSCIN_CTRL0, >>>> + .bit_idx = 30, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "rtc_clk", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_rtc_32k_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +/* dsp a clk */ >>>> +static u32 mux_table_dsp_ab[] = { 0, 1, 2, 3, 4, 7 }; >>>> +static const struct clk_parent_data dsp_ab_clk_parent_data[] = { >>>> + { .fw_name = "xtal", }, >>>> + { .hw = &a1_fclk_div2.hw }, >>>> + { .hw = &a1_fclk_div3.hw }, >>>> + { .hw = &a1_fclk_div5.hw }, >>>> + { .hw = &a1_hifi_pll.hw }, >>>> + { .hw = &a1_rtc_clk.hw }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspa_a_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = DSPA_CLK_CTRL0, >>>> + .mask = 0x7, >>>> + .shift = 10, >>>> + .table = mux_table_dsp_ab, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dspa_a_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_data = dsp_ab_clk_parent_data, >>>> + .num_parents = ARRAY_SIZE(dsp_ab_clk_parent_data), >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspa_a_div = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = DSPA_CLK_CTRL0, >>>> + .shift = 0, >>>> + .width = 10, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dspa_a_div", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspa_a_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspa_a = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = DSPA_CLK_CTRL0, >>>> + .bit_idx = 13, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "dspa_a", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspa_a_div.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>> >>> This CLK_IGNORE_UNUSED, if justified, needs to be explained >>> >> ok, DSP cowork withe ARM in A1, dsp clock shoud not be changed at >> runtime. > > #1: UNUSED only guarantee that the clock won't be gated in the late init > if. No guarantee whatsoever after that. > #2: If you have a DSP that need a clock active at particular rate, write > a driver for it and use the clock API. Don't throw your use case > constraints in the tree def. > > Please review all occurrences of CRITICAL, RO, IGNORE_UNUSED before > sending a v2. > Yes, Dsp related driver should enable the clock itsself, remove CLK_IGNORE_UNUSED >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspa_b_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = DSPA_CLK_CTRL0, >>>> + .mask = 0x7, >>>> + .shift = 26, >>>> + .table = mux_table_dsp_ab, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dspa_b_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_data = dsp_ab_clk_parent_data, >>>> + .num_parents = ARRAY_SIZE(dsp_ab_clk_parent_data), >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspa_b_div = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = DSPA_CLK_CTRL0, >>>> + .shift = 16, >>>> + .width = 10, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dspa_b_div", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspa_b_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspa_b = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = DSPA_CLK_CTRL0, >>>> + .bit_idx = 29, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "dspa_b", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspa_b_div.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspa_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = DSPA_CLK_CTRL0, >>>> + .mask = 0x1, >>>> + .shift = 15, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dspa_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_data = (const struct clk_parent_data []) { >>>> + { .hw = &a1_dspa_a.hw }, >>>> + { .hw = &a1_dspa_b.hw }, >>>> + }, >>>> + .num_parents = 2, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspa_en_dspa = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = DSPA_CLK_EN, >>>> + .bit_idx = 1, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "dspa_en_dspa", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspa_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspa_en_nic = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = DSPA_CLK_EN, >>>> + .bit_idx = 0, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "dspa_en_nic", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspa_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>>> + }, >>>> +}; >>>> + >>>> +/* dsp b clk */ >>>> +static struct clk_regmap a1_dspb_a_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = DSPB_CLK_CTRL0, >>>> + .mask = 0x7, >>>> + .shift = 10, >>>> + .table = mux_table_dsp_ab, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dspb_a_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_data = dsp_ab_clk_parent_data, >>>> + .num_parents = ARRAY_SIZE(dsp_ab_clk_parent_data), >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspb_a_div = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = DSPB_CLK_CTRL0, >>>> + .shift = 0, >>>> + .width = 10, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dspb_a_div", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspb_a_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspb_a = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = DSPB_CLK_CTRL0, >>>> + .bit_idx = 13, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "dspb_a", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspb_a_div.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspb_b_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = DSPB_CLK_CTRL0, >>>> + .mask = 0x7, >>>> + .shift = 26, >>>> + .table = mux_table_dsp_ab, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dspb_b_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_data = dsp_ab_clk_parent_data, >>>> + .num_parents = ARRAY_SIZE(dsp_ab_clk_parent_data), >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspb_b_div = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = DSPB_CLK_CTRL0, >>>> + .shift = 16, >>>> + .width = 10, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dspb_b_div", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspb_b_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspb_b = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = DSPB_CLK_CTRL0, >>>> + .bit_idx = 29, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "dspb_b", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspb_b_div.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspb_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = DSPB_CLK_CTRL0, >>>> + .mask = 0x1, >>>> + .shift = 15, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dspb_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspb_a.hw, &a1_dspb_b.hw, >>>> + }, >>>> + .num_parents = 2, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspb_en_dspb = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = DSPB_CLK_EN, >>>> + .bit_idx = 1, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "dspb_en_dspb", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspb_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dspb_en_nic = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = DSPB_CLK_EN, >>>> + .bit_idx = 0, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "dspb_en_nic", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dspb_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>>> + }, >>>> +}; >>>> + >>>> +/* 12M/24M clock */ >>>> +static struct clk_regmap a1_24m = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = CLK12_24_CTRL, >>>> + .bit_idx = 11, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "24m", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_data = &(const struct clk_parent_data) { >>>> + .fw_name = "xtal", >>>> + }, >>>> + .num_parents = 1, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_fixed_factor a1_24m_div2 = { >>>> + .mult = 1, >>>> + .div = 2, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "24m_div2", >>>> + .ops = &clk_fixed_factor_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_24m.hw >>>> + }, >>>> + .num_parents = 1, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_12m = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = CLK12_24_CTRL, >>>> + .bit_idx = 10, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "12m", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_24m_div2.hw >>>> + }, >>>> + .num_parents = 1, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_fclk_div2_divn_pre = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = CLK12_24_CTRL, >>>> + .shift = 0, >>>> + .width = 8, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "fclk_div2_divn_pre", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_fclk_div2.hw >>>> + }, >>>> + .num_parents = 1, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_fclk_div2_divn = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = CLK12_24_CTRL, >>>> + .bit_idx = 12, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "fclk_div2_divn", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_fclk_div2_divn_pre.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +/* gen clk */ >>> >>> Either put something helpful or don't put anything >>> For example explaining why there is a hole in the table bellow and what >>> is parent 4 (if any) would be helpful. >> ok, parent 2 is sys_pll_div16, sys pll and related clock are not >> completeed, parent 4 is one of clock measurement source, it relies on >> the clock measurement register configuration. >> gen clock is a debug and monitor clock. >>> >>>> +static u32 gen_clk_table[] = { 0, 1, 3, 5, 6, 7, 8 }; >>>> +static const struct clk_parent_data gen_clk_parent_data[] = { >>>> + { .fw_name = "xtal", }, >>>> + { .hw = &a1_rtc_clk.hw }, >>>> + { .hw = &a1_hifi_pll.hw }, >>>> + { .hw = &a1_fclk_div2.hw }, >>>> + { .hw = &a1_fclk_div3.hw }, >>>> + { .hw = &a1_fclk_div5.hw }, >>>> + { .hw = &a1_fclk_div7.hw }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_gen_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = GEN_CLK_CTRL, >>>> + .mask = 0xf, >>>> + .shift = 12, >>>> + .table = gen_clk_table, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "gen_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_data = gen_clk_parent_data, >>>> + .num_parents = ARRAY_SIZE(gen_clk_parent_data), >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_gen_div = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = GEN_CLK_CTRL, >>>> + .shift = 0, >>>> + .width = 11, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "gen_div", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_gen_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_gen = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = GEN_CLK_CTRL, >>>> + .bit_idx = 11, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "gen", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_gen_div.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_saradc_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = SAR_ADC_CLK_CTRL, >>>> + .mask = 0x1, >>>> + .shift = 9, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "saradc_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_data = (const struct clk_parent_data []) { >>>> + { .fw_name = "xtal", }, >>>> + { .hw = &a1_sys_clk.hw, }, >>>> + }, >>>> + .num_parents = 2, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_saradc_div = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = SAR_ADC_CLK_CTRL, >>>> + .shift = 0, >>>> + .width = 8, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "saradc_div", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_saradc_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_saradc_clk = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = SAR_ADC_CLK_CTRL, >>>> + .bit_idx = 8, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "saradc_clk", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_saradc_div.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +/* pwm a/b/c/d parent data */ >>>> +static const struct clk_parent_data pwm_parent_data[] = { >>>> + { .fw_name = "xtal", }, >>>> + { .hw = &a1_sys_clk.hw }, >>>> +}; >>>> + >>>> +/* >>>> + * add CLK_IGNORE_UNUSED flag for pwm controller GATE >>>> + * clk core will disable unused clock, it may disable >>>> + * vddcore voltage which contrlled by one pwm in bl21. >>> ^ >>> typo >>> >>>> + * add the flag to avoid changing cpu voltage. >>> >>> 80 char per line is ok >>> This comment would be better placed next to the corresponding flag >>> >>> Also you could explain that flag could be removed when CCF has some >>> hand-off mechanism (similar comments you can copy in other driver) >>> >> ok, contrlled --> controlled >> I will move the comment next to the flag >> I have gotten the similar comments in g12a.c >>>> + */ >>>> +/* pwm a clk */ >>>> +static struct clk_regmap a1_pwm_a_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = PWM_CLK_AB_CTRL, >>>> + .mask = 0x1, >>>> + .shift = 9, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "pwm_a_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_data = pwm_parent_data, >>>> + .num_parents = ARRAY_SIZE(pwm_parent_data), >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_pwm_a_div = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = PWM_CLK_AB_CTRL, >>>> + .shift = 0, >>>> + .width = 8, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "pwm_a_div", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_pwm_a_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_pwm_a = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = PWM_CLK_AB_CTRL, >>>> + .bit_idx = 8, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "pwm_a", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_pwm_a_div.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>>> + }, >>>> +}; >>>> + >> >> [...] >> >>>> +/* usb bus clk */ >>>> +static const struct clk_parent_data usb_bus_parent_data[] = { >>>> + { .fw_name = "xtal", }, >>>> + { .hw = &a1_sys_clk.hw }, >>>> + { .hw = &a1_fclk_div3.hw }, >>>> + { .hw = &a1_fclk_div5.hw }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_usb_bus_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = USB_BUSCLK_CTRL, >>>> + .mask = 0x3, >>>> + .shift = 9, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "usb_bus_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_data = usb_bus_parent_data, >>>> + .num_parents = ARRAY_SIZE(usb_bus_parent_data), >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>> >>> Again, every unused flag need to be properly explained >>> >> ok, I will confirm this flag >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_usb_bus_div = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = USB_BUSCLK_CTRL, >>>> + .shift = 0, >>>> + .width = 8, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "usb_bus_div", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_usb_bus_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_usb_bus = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = USB_BUSCLK_CTRL, >>>> + .bit_idx = 8, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "usb_bus", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_usb_bus_div.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>>> + }, >>>> +}; >>>> + >> >> [...] >> >>>> + >>>> +/* dmc clk */ >>>> +static const struct clk_hw *dmc_parent_hws[] = { >>>> + &a1_fclk_div2.hw, >>>> + &a1_fclk_div3.hw, >>>> + &a1_fclk_div5.hw, >>>> + &a1_hifi_pll.hw, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dmc_sel = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = DMC_CLK_CTRL, >>>> + .mask = 0x3, >>>> + .shift = 9, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dmc_sel", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_hws = dmc_parent_hws, >>>> + .num_parents = ARRAY_SIZE(dmc_parent_hws), >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dmc_div = { >>>> + .data = &(struct clk_regmap_div_data){ >>>> + .offset = DMC_CLK_CTRL, >>>> + .shift = 0, >>>> + .width = 8, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dmc_div", >>>> + .ops = &clk_regmap_divider_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dmc_sel.hw >>>> + }, >>>> + .num_parents = 1, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dmc_sel2 = { >>>> + .data = &(struct clk_regmap_mux_data){ >>>> + .offset = DMC_CLK_CTRL, >>>> + .mask = 0x1, >>>> + .shift = 15, >>>> + }, >>>> + .hw.init = &(struct clk_init_data){ >>>> + .name = "dmc_sel2", >>>> + .ops = &clk_regmap_mux_ops, >>>> + .parent_data = (const struct clk_parent_data []) { >>>> + { .hw = &a1_dmc_div.hw }, >>>> + { .fw_name = "xtal", }, >>>> + }, >>>> + .num_parents = 2, >>>> + .flags = CLK_SET_RATE_PARENT, >>>> + }, >>>> +}; >>>> + >>>> +static struct clk_regmap a1_dmc = { >>>> + .data = &(struct clk_regmap_gate_data){ >>>> + .offset = DMC_CLK_CTRL, >>>> + .bit_idx = 8, >>>> + }, >>>> + .hw.init = &(struct clk_init_data) { >>>> + .name = "dmc", >>>> + .ops = &clk_regmap_gate_ops, >>>> + .parent_hws = (const struct clk_hw *[]) { >>>> + &a1_dmc_sel2.hw >>>> + }, >>>> + .num_parents = 1, >>>> + /* >>>> + * add CLK_IGNORE_UNUSED to avoid hangup >>>> + * DDR clock should not change at runtime >>>> + */ >>> >>> CRITICAL ? RO ops maybe ? >> RO ops is better >>> >>>> + .flags = CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED, >>>> + }, >>>> +}; >>>> + >> >> [...] >> >>>> + >>>> +#define MESON_GATE(_name, _reg, _bit) \ >>>> + MESON_PCLK(_name, _reg, _bit, &a1_sys_clk.hw) >>>> + >>>> +/* CLKTREE_SYS_CLK_EN0 */ >>> >>> Full caps is not ok ... and once again, try to add comment which helps >> ok, I will add some useful comment. >>> >>>> +static MESON_GATE(a1_clk_tree, SYS_CLK_EN0, 0); >>>> +static MESON_GATE(a1_reset_ctrl, SYS_CLK_EN0, 1); >>>> +static MESON_GATE(a1_analog_ctrl, SYS_CLK_EN0, 2); >>>> +static MESON_GATE(a1_pwr_ctrl, SYS_CLK_EN0, 3); >>>> +static MESON_GATE(a1_pad_ctrl, SYS_CLK_EN0, 4); >>>> +static MESON_GATE(a1_sys_ctrl, SYS_CLK_EN0, 5); >>>> +static MESON_GATE(a1_temp_sensor, SYS_CLK_EN0, 6); >>>> +static MESON_GATE(a1_am2axi_dev, SYS_CLK_EN0, 7); >>>> +static MESON_GATE(a1_spicc_b, SYS_CLK_EN0, 8); >>>> +static MESON_GATE(a1_spicc_a, SYS_CLK_EN0, 9); >>>> +static MESON_GATE(a1_clk_msr, SYS_CLK_EN0, 10); >>>> +static MESON_GATE(a1_audio, SYS_CLK_EN0, 11); >>>> +static MESON_GATE(a1_jtag_ctrl, SYS_CLK_EN0, 12); >>>> +static MESON_GATE(a1_saradc, SYS_CLK_EN0, 13); >>>> +static MESON_GATE(a1_pwm_ef, SYS_CLK_EN0, 14); >>>> +static MESON_GATE(a1_pwm_cd, SYS_CLK_EN0, 15); >>>> +static MESON_GATE(a1_pwm_ab, SYS_CLK_EN0, 16); >>>> +static MESON_GATE(a1_cec, SYS_CLK_EN0, 17); >>>> +static MESON_GATE(a1_i2c_s, SYS_CLK_EN0, 18); >>>> +static MESON_GATE(a1_ir_ctrl, SYS_CLK_EN0, 19); >>>> +static MESON_GATE(a1_i2c_m_d, SYS_CLK_EN0, 20); >>>> +static MESON_GATE(a1_i2c_m_c, SYS_CLK_EN0, 21); >>>> +static MESON_GATE(a1_i2c_m_b, SYS_CLK_EN0, 22); >>>> +static MESON_GATE(a1_i2c_m_a, SYS_CLK_EN0, 23); >>>> +static MESON_GATE(a1_acodec, SYS_CLK_EN0, 24); >>>> +static MESON_GATE(a1_otp, SYS_CLK_EN0, 25); >>>> +static MESON_GATE(a1_sd_emmc_a, SYS_CLK_EN0, 26); >>>> +static MESON_GATE(a1_usb_phy, SYS_CLK_EN0, 27); >>>> +static MESON_GATE(a1_usb_ctrl, SYS_CLK_EN0, 28); >>>> +static MESON_GATE(a1_sys_dspb, SYS_CLK_EN0, 29); >>>> +static MESON_GATE(a1_sys_dspa, SYS_CLK_EN0, 30); >>>> +static MESON_GATE(a1_dma, SYS_CLK_EN0, 31); >>>> +/* CLKTREE_SYS_CLK_EN1 */ >>>> +static MESON_GATE(a1_irq_ctrl, SYS_CLK_EN1, 0); >>>> +static MESON_GATE(a1_nic, SYS_CLK_EN1, 1); >>>> +static MESON_GATE(a1_gic, SYS_CLK_EN1, 2); >>>> +static MESON_GATE(a1_uart_c, SYS_CLK_EN1, 3); >>>> +static MESON_GATE(a1_uart_b, SYS_CLK_EN1, 4); >>>> +static MESON_GATE(a1_uart_a, SYS_CLK_EN1, 5); >>>> +static MESON_GATE(a1_sys_psram, SYS_CLK_EN1, 6); >>>> +static MESON_GATE(a1_rsa, SYS_CLK_EN1, 8); >>>> +static MESON_GATE(a1_coresight, SYS_CLK_EN1, 9); >>>> +/* CLKTREE_AXI_CLK_EN */ >>>> +static MESON_GATE(a1_am2axi_vad, AXI_CLK_EN, 0); >>>> +static MESON_GATE(a1_audio_vad, AXI_CLK_EN, 1); >>>> +static MESON_GATE(a1_axi_dmc, AXI_CLK_EN, 3); >>>> +static MESON_GATE(a1_axi_psram, AXI_CLK_EN, 4); >>>> +static MESON_GATE(a1_ramb, AXI_CLK_EN, 5); >>>> +static MESON_GATE(a1_rama, AXI_CLK_EN, 6); >>>> +static MESON_GATE(a1_axi_spifc, AXI_CLK_EN, 7); >>>> +static MESON_GATE(a1_axi_nic, AXI_CLK_EN, 8); >>>> +static MESON_GATE(a1_axi_dma, AXI_CLK_EN, 9); >>>> +static MESON_GATE(a1_cpu_ctrl, AXI_CLK_EN, 10); >>>> +static MESON_GATE(a1_rom, AXI_CLK_EN, 11); >>>> +static MESON_GATE(a1_prod_i2c, AXI_CLK_EN, 12); >>>> + >> >> [...] >> >>>> + >>>> +/* Convenience table to populate regmap in .probe >>>> + * Peripheral clock register base address is 0xfe000800 >>> >>> this info will be in DT >> I will remove the address commit >>> >>>> + */ >> >> [...] >> >>>> + >>>> +/* >>>> + * cpu clock register base address is 0xfd000080 >>>> + */ >>>> +static struct clk_regmap *const a1_cpu_clk_regmaps[] = { >>>> + /* TODO */ >>>> +}; >>>> + >>>> +/* >>>> + * pll clock register base address is 0xfe007c00 >>>> + */ >>>> +static struct clk_regmap *const a1_pll_clk_regmaps[] = { >>>> + &a1_fixed_pll_dco, >>>> + &a1_fixed_pll, >>>> + &a1_hifi_pll, >>>> + &a1_fclk_div2, >>>> + &a1_fclk_div3, >>>> + &a1_fclk_div5, >>>> + &a1_fclk_div7, >>>> +}; >>>> + >>>> +static struct regmap_config clkc_regmap_config = { >>>> + .reg_bits = 32, >>>> + .val_bits = 32, >>>> + .reg_stride = 4, >>>> +}; >>>> + >>>> +static struct regmap *a1_regmap_resource(struct device *dev, char *name) >>>> +{ >>>> + struct resource res; >>>> + void __iomem *base; >>>> + int i; >>>> + struct device_node *node = dev->of_node; >>>> + >>>> + i = of_property_match_string(node, "reg-names", name); >>>> + if (of_address_to_resource(node, i, &res)) >>>> + return ERR_PTR(-ENOENT); >>>> + >>>> + base = devm_ioremap_resource(dev, &res); >>>> + if (IS_ERR(base)) >>>> + return ERR_CAST(base); >>>> + >>>> + clkc_regmap_config.max_register = resource_size(&res) - 4; >>>> + clkc_regmap_config.name = devm_kasprintf(dev, GFP_KERNEL, >>>> + "%s-%s", node->name, >>>> + name); >>>> + if (!clkc_regmap_config.name) >>>> + return ERR_PTR(-ENOMEM); >>>> + >>>> + return devm_regmap_init_mmio(dev, base, &clkc_regmap_config); >>>> +} >>>> + >>>> +static int a1_clkc_probe(struct platform_device *pdev) >>>> +{ >>>> + struct device *dev = &pdev->dev; >>>> + struct regmap *peripheral_map, *pll_map, *cpu_map; >>>> + int ret, i; >>>> + >>>> + /* Get regmap for different clock area */ >>>> + peripheral_map = a1_regmap_resource(dev, "peripheral"); >>>> + if (IS_ERR(peripheral_map)) { >>>> + dev_err(dev, "peripheral clk registers not found\n"); >>>> + return PTR_ERR(peripheral_map); >>>> + } >>>> + >>>> + pll_map = a1_regmap_resource(dev, "pll"); >>>> + if (IS_ERR(pll_map)) { >>>> + dev_err(dev, "pll clk registers not found\n"); >>>> + return PTR_ERR(pll_map); >>>> + } >>>> + >>>> + cpu_map = a1_regmap_resource(dev, "cpu"); >>>> + if (IS_ERR(cpu_map)) { >>>> + dev_err(dev, "cpu clk registers not found\n"); >>>> + return PTR_ERR(cpu_map); >>>> + } >>>> + >>>> + /* Populate regmap for the regmap backed clocks */ >>>> + for (i = 0; i < ARRAY_SIZE(a1_clk_regmaps); i++) >>>> + a1_clk_regmaps[i]->map = peripheral_map; >>>> + >>>> + for (i = 0; i < ARRAY_SIZE(a1_cpu_clk_regmaps); i++) >>>> + a1_cpu_clk_regmaps[i]->map = cpu_map; >>>> + >>>> + for (i = 0; i < ARRAY_SIZE(a1_pll_clk_regmaps); i++) >>>> + a1_pll_clk_regmaps[i]->map = pll_map; >>> >>> Unless you have a rock solid reason why you absolutely need to make >>> single controller out of this 3 regions, Nack. >>> >> As comment replying in >> Documentation/devicetree/bindings/clock/amlogic,a1-clkc.yaml, I will >> implement three clock drivers. >>>> + >>>> + for (i = 0; i < a1_hw_onecell_data.num; i++) { >>>> + /* array might be sparse */ >>>> + if (!a1_hw_onecell_data.hws[i]) >>>> + continue; >>>> + >>>> + ret = devm_clk_hw_register(dev, a1_hw_onecell_data.hws[i]); >>>> + if (ret) { >>>> + dev_err(dev, "Clock registration failed\n"); >>>> + return ret; >>>> + } >>>> + } >>>> + >>>> + return devm_of_clk_add_hw_provider(dev, of_clk_hw_onecell_get, >>>> + &a1_hw_onecell_data); >>>> +} >>>> + >>>> +static const struct of_device_id clkc_match_table[] = { >>>> + { .compatible = "amlogic,a1-clkc" }, >>>> + { /* sentinel */ } >>>> +}; >>>> + >>>> +static struct platform_driver a1_driver = { >>>> + .probe = a1_clkc_probe, >>>> + .driver = { >>>> + .name = "a1-clkc", >>>> + .of_match_table = clkc_match_table, >>>> + }, >>>> +}; >>>> + >>>> +builtin_platform_driver(a1_driver); >>>> diff --git a/drivers/clk/meson/a1.h b/drivers/clk/meson/a1.h >>>> new file mode 100644 >>>> index 0000000..f2291c4 >>>> --- /dev/null >>>> +++ b/drivers/clk/meson/a1.h >>>> @@ -0,0 +1,172 @@ >>>> +/* SPDX-License-Identifier: (GPL-2.0+ OR MIT) */ >>>> +/* >>>> + * Copyright (c) 2019 Amlogic, Inc. All rights reserved. >>>> + */ >>>> + >>>> +#ifndef __A1_H >>>> +#define __A1_H >>>> + >>>> +/* >>>> + * peripheral clock controller register address >>>> + * APB_BASE: APB0_BASE_ADDR = 0xfe000800 >>> >>> Again this info is DT and might become wrong if your controller is ever >>> compatible with another SoC >>> >> I will remove the address comment, the right address is 0xfe000080. >>>> + */ >>>> +#define SYS_OSCIN_CTRL 0x0 >>>> +#define RTC_BY_OSCIN_CTRL0 0x4 >>>> +#define RTC_BY_OSCIN_CTRL1 0x8 >>>> +#define RTC_CTRL 0xc >>>> +#define SYS_CLK_CTRL0 0x10 >>>> +#define AXI_CLK_CTRL0 0x14 >>>> +#define SYS_CLK_EN0 0x1c >>>> +#define SYS_CLK_EN1 0x20 >>>> +#define AXI_CLK_EN 0x24 >>>> +#define DSPA_CLK_EN 0x28 >>>> +#define DSPB_CLK_EN 0x2c >>>> +#define DSPA_CLK_CTRL0 0x30 >>>> +#define DSPB_CLK_CTRL0 0x34 >>>> +#define CLK12_24_CTRL 0x38 >>>> +#define GEN_CLK_CTRL 0x3c >>>> +#define TIMESTAMP_CTRL0 0x40 >>>> +#define TIMESTAMP_CTRL1 0x44 >>>> +#define TIMESTAMP_CTRL2 0x48 >>>> +#define TIMESTAMP_VAL0 0x4c >>>> +#define TIMESTAMP_VAL1 0x50 >>>> +#define TIMEBASE_CTRL0 0x54 >>>> +#define TIMEBASE_CTRL1 0x58 >>>> +#define SAR_ADC_CLK_CTRL 0xc0 >>>> +#define PWM_CLK_AB_CTRL 0xc4 >>>> +#define PWM_CLK_CD_CTRL 0xc8 >>>> +#define PWM_CLK_EF_CTRL 0xcc >>>> +#define SPICC_CLK_CTRL 0xd0 >>>> +#define TS_CLK_CTRL 0xd4 >>>> +#define SPIFC_CLK_CTRL 0xd8 >>>> +#define USB_BUSCLK_CTRL 0xdc >>>> +#define SD_EMMC_CLK_CTRL 0xe0 >>>> +#define CECA_CLK_CTRL0 0xe4 >>>> +#define CECA_CLK_CTRL1 0xe8 >>>> +#define CECB_CLK_CTRL0 0xec >>>> +#define CECB_CLK_CTRL1 0xf0 >>>> +#define PSRAM_CLK_CTRL 0xf4 >>>> +#define DMC_CLK_CTRL 0xf8 >>>> +#define FCLK_DIV1_SEL 0xfc >>>> +#define TST_CTRL 0x100 >>>> +/* >>>> + * For PLL register offset >>>> + * APB_BASE: APB0_BASE_ADDR = 0xfe007c00 >>>> + */ >>>> +#define ANACTRL_FIXPLL_CTRL0 0x80 >>>> +#define ANACTRL_FIXPLL_CTRL1 0x84 >>>> +#define ANACTRL_FIXPLL_CTRL2 0x88 >>>> +#define ANACTRL_FIXPLL_CTRL3 0x8c >>>> +#define ANACTRL_FIXPLL_CTRL4 0x90 >>>> +#define ANACTRL_FIXPLL_STS 0x94 >>>> +#define ANACTRL_SYSPLL_CTRL0 0x100 >>>> +#define ANACTRL_SYSPLL_CTRL1 0x104 >>>> +#define ANACTRL_SYSPLL_CTRL2 0x108 >>>> +#define ANACTRL_SYSPLL_CTRL3 0x10c >>>> +#define ANACTRL_SYSPLL_CTRL4 0x110 >>>> +#define ANACTRL_SYSPLL_STS 0x114 >>>> +#define ANACTRL_HIFIPLL_CTRL0 0x140 >>>> +#define ANACTRL_HIFIPLL_CTRL1 0x144 >>>> +#define ANACTRL_HIFIPLL_CTRL2 0x148 >>>> +#define ANACTRL_HIFIPLL_CTRL3 0x14c >>>> +#define ANACTRL_HIFIPLL_CTRL4 0x150 >>>> +#define ANACTRL_HIFIPLL_STS 0x154 >>>> +#define ANACTRL_AUDDDS_CTRL0 0x180 >>>> +#define ANACTRL_AUDDDS_CTRL1 0x184 >>>> +#define ANACTRL_AUDDDS_CTRL2 0x188 >>>> +#define ANACTRL_AUDDDS_CTRL3 0x18c >>>> +#define ANACTRL_AUDDDS_CTRL4 0x190 >>>> +#define ANACTRL_AUDDDS_STS 0x194 >>>> +#define ANACTRL_MISCTOP_CTRL0 0x1c0 >>>> +#define ANACTRL_POR_CNTL 0x208 >>>> +/* >>>> + * CPU clok register offset >>>> + * APB_BASE: APB1_BASE_ADDR = 0xfd000080 >>>> + */ >>>> +#define CPUCTRL_CLK_CTRL0 0x00 >>>> +#define CPUCTRL_CLK_CTRL1 0x04 >>>> +#define CPUCTRL_CLK_CTRL2 0x08 >>>> +#define CPUCTRL_CLK_CTRL3 0x0c >>>> +#define CPUCTRL_CLK_CTRL4 0x10 >>>> +#define CPUCTRL_CLK_CTRL5 0x14 >>>> +#define CPUCTRL_CLK_CTRL6 0x18 >>>> +#define CPUCTRL_CLK_CTRL7 0x1c >>>> + >>>> +#define CLKID_FIXED_PLL_DCO 0 >>>> +#define CLKID_SYS_A_SEL 10 >>>> +#define CLKID_SYS_A_DIV 11 >>>> +#define CLKID_SYS_A 12 >>>> +#define CLKID_SYS_B_SEL 13 >>>> +#define CLKID_SYS_B_DIV 14 >>>> +#define CLKID_SYS_B 15 >>>> +#define CLKID_XTAL_CLKTREE 18 >>>> +#define CLKID_XTAL_FIXPLL 19 >>>> +#define CLKID_XTAL_USB_PHY 20 >>>> +#define CLKID_XTAL_USB_CTRL 21 >>>> +#define CLKID_XTAL_HIFIPLL 22 >>>> +#define CLKID_XTAL_SYSPLL 23 >>>> +#define CLKID_XTAL_DDS 24 >>>> +#define CLKID_DSPA_A_SEL 78 >>>> +#define CLKID_DSPA_A_DIV 79 >>>> +#define CLKID_DSPA_A 80 >>>> +#define CLKID_DSPA_B_SEL 81 >>>> +#define CLKID_DSPA_B_DIV 82 >>>> +#define CLKID_DSPA_B 83 >>>> +#define CLKID_DSPB_A_SEL 85 >>>> +#define CLKID_DSPB_A_DIV 86 >>>> +#define CLKID_DSPB_A 87 >>>> +#define CLKID_DSPB_B_SEL 88 >>>> +#define CLKID_DSPB_B_DIV 89 >>>> +#define CLKID_DSPB_B 90 >>>> +#define CLKID_RTC_32K_CLKIN 96 >>>> +#define CLKID_RTC_32K_DIV 97 >>>> +#define CLKID_RTC_32K_XTAL 98 >>>> +#define CLKID_RTC_32K_SEL 99 >>>> +#define CLKID_CECB_32K_CLKIN 106 >>>> +#define CLKID_CECB_32K_DIV 107 >>>> +#define CLKID_CECB_32K_SEL_PRE 108 >>>> +#define CLKID_CECB_32K_SEL 109 >>>> +#define CLKID_CECA_32K_CLKIN 101 >>>> +#define CLKID_CECA_32K_DIV 102 >>>> +#define CLKID_CECA_32K_SEL_PRE 103 >>>> +#define CLKID_CECA_32K_SEL 104 >>>> +#define CLKID_DIV2_PRE 113 >>>> +#define CLKID_24M_DIV2 115 >>>> +#define CLKID_GEN_SEL 116 >>>> +#define CLKID_GEN_DIV 117 >>>> +#define CLKID_SARADC_DIV 120 >>>> +#define CLKID_PWM_A_SEL 122 >>>> +#define CLKID_PWM_A_DIV 123 >>>> +#define CLKID_PWM_B_SEL 125 >>>> +#define CLKID_PWM_B_DIV 126 >>>> +#define CLKID_PWM_C_SEL 128 >>>> +#define CLKID_PWM_C_DIV 129 >>>> +#define CLKID_PWM_D_SEL 131 >>>> +#define CLKID_PWM_D_DIV 132 >>>> +#define CLKID_PWM_E_SEL 134 >>>> +#define CLKID_PWM_E_DIV 135 >>>> +#define CLKID_PWM_F_SEL 137 >>>> +#define CLKID_PWM_F_DIV 138 >>>> +#define CLKID_SPICC_SEL 140 >>>> +#define CLKID_SPICC_DIV 141 >>>> +#define CLKID_SPICC_SEL2 142 >>>> +#define CLKID_TS_DIV 144 >>>> +#define CLKID_SPIFC_SEL 146 >>>> +#define CLKID_SPIFC_DIV 147 >>>> +#define CLKID_SPIFC_SEL2 148 >>>> +#define CLKID_USB_BUS_SEL 150 >>>> +#define CLKID_USB_BUS_DIV 151 >>>> +#define CLKID_SD_EMMC_SEL 153 >>>> +#define CLKID_SD_EMMC_DIV 154 >>>> +#define CLKID_SD_EMMC_SEL2 155 >>>> +#define CLKID_PSRAM_SEL 157 >>>> +#define CLKID_PSRAM_DIV 158 >>>> +#define CLKID_PSRAM_SEL2 159 >>>> +#define CLKID_DMC_SEL 161 >>>> +#define CLKID_DMC_DIV 162 >>>> +#define CLKID_DMC_SEL2 163 >>>> +#define NR_CLKS 165 >>>> + >>>> +#include >>>> + >>>> +#endif /* __A1_H */ >>>> -- >>>> 1.9.1 >>> >>> . >>> > > . >