From: Stephen Boyd <sboyd@kernel.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
mturquette@baylibre.com, robh+dt@kernel.org
Cc: bjorn.andersson@linaro.org, vkoul@kernel.org,
linux-arm-msm@vger.kernel.org, linux-clk@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Naveen Yadav <naveenky@codeaurora.org>,
Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Subject: Re: [PATCH 2/4] clk: qcom: Add SDX55 GCC support
Date: Wed, 04 Nov 2020 18:23:37 -0800 [thread overview]
Message-ID: <160454301723.3965362.9504622582275389041@swboyd.mtv.corp.google.com> (raw)
In-Reply-To: <20201028074232.22922-3-manivannan.sadhasivam@linaro.org>
Quoting Manivannan Sadhasivam (2020-10-28 00:42:30)
> From: Naveen Yadav <naveenky@codeaurora.org>
>
> Add Global Clock Controller (GCC) support for SDX55 SoCs from Qualcomm.
>
> Signed-off-by: Naveen Yadav <naveenky@codeaurora.org>
> [mani: converted to parent_data, commented critical clocks, cleanups]
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
> drivers/clk/qcom/Kconfig | 8 +
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/gcc-sdx55.c | 1667 ++++++++++++++++++++++++++++++++++
> 3 files changed, 1676 insertions(+)
> create mode 100644 drivers/clk/qcom/gcc-sdx55.c
>
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 3a965bd326d5..dbca8debc06f 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -502,4 +502,12 @@ config KRAITCC
> Support for the Krait CPU clocks on Qualcomm devices.
> Say Y if you want to support CPU frequency scaling.
>
> +config GCC_SDX55
Please sort instead of add at end.
> + tristate "SDX55 Global Clock Controller"
> + depends on ARM
Why?
> + help
> + Support for the global clock controller on SDX55 devices.
> + Say Y if you want to use peripheral devices such as UART,
> + SPI, I2C, USB, SD/UFS, PCIe etc.
> +
> endif
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 11ae86febe87..3e27d67f95aa 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -75,3 +75,4 @@ obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o
> obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o
> obj-$(CONFIG_QCOM_HFPLL) += hfpll.o
> obj-$(CONFIG_KRAITCC) += krait-cc.o
> +obj-$(CONFIG_GCC_SDX55) += gcc-sdx55.o
Please sort this instead of add at end.
> diff --git a/drivers/clk/qcom/gcc-sdx55.c b/drivers/clk/qcom/gcc-sdx55.c
> new file mode 100644
> index 000000000000..75831c829202
> --- /dev/null
> +++ b/drivers/clk/qcom/gcc-sdx55.c
> @@ -0,0 +1,1667 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2018-2019, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2020, Linaro Ltd.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/clock/qcom,gcc-sdx55.h>
> +
> +#include "common.h"
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
> +#include "clk-pll.h"
> +#include "clk-rcg.h"
> +#include "clk-regmap.h"
> +#include "reset.h"
> +
> +enum {
> + P_BI_TCXO,
> + P_CORE_BI_PLL_TEST_SE,
> + P_GPLL0_OUT_EVEN,
> + P_GPLL0_OUT_MAIN,
> + P_GPLL4_OUT_EVEN,
> + P_GPLL5_OUT_MAIN,
> + P_SLEEP_CLK,
> +};
> +
> +static struct pll_vco lucid_vco[] = {
const
> + { 249600000, 2000000000, 0 },
> +};
> +
> +static struct clk_alpha_pll gpll0 = {
> + .offset = 0x0,
> + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID],
> + .vco_table = lucid_vco,
> + .num_vco = ARRAY_SIZE(lucid_vco),
> + .clkr = {
> + .enable_reg = 0x6d000,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "gpll0",
> + .parent_data = &(const struct clk_parent_data){
> + .fw_name = "bi_tcxo",
> + },
> + .num_parents = 1,
> + .ops = &clk_alpha_pll_fixed_lucid_ops,
> + },
> + },
> +};
> +
> +static const struct clk_div_table post_div_table_lucid_even[] = {
> + { 0x0, 1 },
> + { 0x1, 2 },
> + { 0x3, 4 },
> + { 0x7, 8 },
> + { }
> +};
I think this table is common to all lucid plls? Maybe we can push it
into the clk_ops somehow and stop duplicating it here?
> +
> +static struct clk_alpha_pll_postdiv gpll0_out_even = {
> + .offset = 0x0,
> + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID],
> + .post_div_shift = 8,
> + .post_div_table = post_div_table_lucid_even,
> + .num_post_div = ARRAY_SIZE(post_div_table_lucid_even),
> + .width = 4,
> + .clkr.hw.init = &(struct clk_init_data){
> + .name = "gpll0_out_even",
> + .parent_data = &(const struct clk_parent_data){
> + .fw_name = "gpll0",
> + },
If this is gpll0 in this file, then this should be a clk_hws pointer
instead and directly pointing to the parent.
> + .num_parents = 1,
> + .ops = &clk_alpha_pll_postdiv_lucid_ops,
> + },
> +};
> +
> +static struct clk_alpha_pll gpll4 = {
> + .offset = 0x76000,
> + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID],
> + .vco_table = lucid_vco,
> + .num_vco = ARRAY_SIZE(lucid_vco),
> + .clkr = {
> + .enable_reg = 0x6d000,
> + .enable_mask = BIT(4),
> + .hw.init = &(struct clk_init_data){
> + .name = "gpll4",
> + .parent_data = &(const struct clk_parent_data){
> + .fw_name = "bi_tcxo",
> + },
> + .num_parents = 1,
> + .ops = &clk_alpha_pll_fixed_lucid_ops,
> + },
> + },
> +};
> +
> +static struct clk_alpha_pll_postdiv gpll4_out_even = {
> + .offset = 0x76000,
> + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID],
> + .post_div_shift = 8,
> + .post_div_table = post_div_table_lucid_even,
> + .num_post_div = ARRAY_SIZE(post_div_table_lucid_even),
> + .width = 4,
> + .clkr.hw.init = &(struct clk_init_data){
> + .name = "gpll4_out_even",
> + .parent_data = &(const struct clk_parent_data){
> + .fw_name = "gpll4",
If this is gpll4 in this file, then this should be a clk_hws pointer
instead and directly pointing to the parent.
> + },
> + .num_parents = 1,
> + .ops = &clk_alpha_pll_postdiv_lucid_ops,
> + },
> +};
> +
> +static struct clk_alpha_pll gpll5 = {
> + .offset = 0x74000,
> + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID],
> + .vco_table = lucid_vco,
> + .num_vco = ARRAY_SIZE(lucid_vco),
[...]
> + .name = "gcc_sdcc1_ahb_clk",
> + .ops = &clk_branch2_ops,
> + },
> + },
> +};
> +
> +static struct clk_branch gcc_sdcc1_apps_clk = {
> + .halt_reg = 0xf004,
> + .halt_check = BRANCH_HALT,
> + .clkr = {
> + .enable_reg = 0xf004,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "gcc_sdcc1_apps_clk",
> + .parent_hws = (const struct clk_hw *[]){
> + &gcc_sdcc1_apps_clk_src.clkr.hw },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + .ops = &clk_branch2_ops,
> + },
> + },
> +};
> +
> +/* For CPUSS functionality the SYS NOC clock needs to be left enabled */
> +static struct clk_branch gcc_sys_noc_cpuss_ahb_clk = {
> + .halt_reg = 0x4010,
> + .halt_check = BRANCH_HALT_VOTED,
> + .clkr = {
> + .enable_reg = 0x6d008,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "gcc_sys_noc_cpuss_ahb_clk",
> + .parent_hws = (const struct clk_hw *[]){
> + &gcc_cpuss_ahb_clk_src.clkr.hw },
> + .num_parents = 1,
> + .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,
These CLK_IS_CRITICAL clks can't be set once at driver probe time and
forgotten about? It would be nice to not allocate memory for things that
never matter.
> + .ops = &clk_branch2_ops,
> + },
> + },
> +};
> +
> +static struct clk_branch gcc_usb30_master_clk = {
> + .halt_reg = 0xb010,
> + .halt_check = BRANCH_HALT,
> + .clkr = {
> + .enable_reg = 0xb010,
> + .enable_mask = BIT(0),
> + .hw.init = &(struct clk_init_data){
> + .name = "gcc_usb30_master_clk",
> + .parent_hws = (const struct clk_hw *[]){
> + &gcc_usb30_master_clk_src.clkr.hw },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + .ops = &clk_branch2_ops,
[...]
> +
> +static const struct qcom_cc_desc gcc_sdx55_desc = {
> + .config = &gcc_sdx55_regmap_config,
> + .clks = gcc_sdx55_clocks,
> + .num_clks = ARRAY_SIZE(gcc_sdx55_clocks),
> + .resets = gcc_sdx55_resets,
> + .num_resets = ARRAY_SIZE(gcc_sdx55_resets),
No gdscs?
> +};
> +
> +static const struct of_device_id gcc_sdx55_match_table[] = {
> + { .compatible = "qcom,gcc-sdx55" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, gcc_sdx55_match_table);
> +
> +static int gcc_sdx55_probe(struct platform_device *pdev)
> +{
> + return qcom_cc_probe(pdev, &gcc_sdx55_desc);
Wow haven't seen this in some time. There isn't some sort of PLL that
needs configuring or some clks that need to be slammed on with a couple
register writes?
> +}
> +
next prev parent reply other threads:[~2020-11-05 2:23 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-28 7:42 [PATCH 0/4] Add GCC and RPMh clock support for SDX55 Manivannan Sadhasivam
2020-10-28 7:42 ` [PATCH 1/4] dt-bindings: clock: Add SDX55 GCC clock bindings Manivannan Sadhasivam
2020-10-30 19:22 ` Rob Herring
2020-10-31 3:29 ` Manivannan Sadhasivam
2020-11-05 2:03 ` Stephen Boyd
2020-10-28 7:42 ` [PATCH 2/4] clk: qcom: Add SDX55 GCC support Manivannan Sadhasivam
2020-11-05 2:23 ` Stephen Boyd [this message]
2020-11-05 8:51 ` Manivannan Sadhasivam
2020-11-13 7:58 ` Stephen Boyd
2020-10-28 7:42 ` [PATCH 3/4] dt-bindings: clock: Introduce RPMHCC bindings for SDX55 Manivannan Sadhasivam
2020-11-03 17:38 ` Bjorn Andersson
2020-10-28 7:42 ` [PATCH 4/4] clk: qcom: Add support for SDX55 RPMh clocks Manivannan Sadhasivam
2020-11-03 17:53 ` Bjorn Andersson
2020-11-05 2:25 ` Stephen Boyd
2020-10-28 17:08 ` [PATCH 0/4] Add GCC and RPMh clock support for SDX55 Manivannan Sadhasivam
2020-11-03 17:34 ` Bjorn Andersson
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=160454301723.3965362.9504622582275389041@swboyd.mtv.corp.google.com \
--to=sboyd@kernel.org \
--cc=bjorn.andersson@linaro.org \
--cc=devicetree@vger.kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=manivannan.sadhasivam@linaro.org \
--cc=mturquette@baylibre.com \
--cc=naveenky@codeaurora.org \
--cc=robh+dt@kernel.org \
--cc=vkoul@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.