Re: [PATCH v1 4/4] clk: qcom: lpass: Add support for LPASS clock controller for SC7280

From: Stephen Boyd
Date: Tue Nov 02 2021 - 18:19:18 EST

Next message: Paul Cercueil: "Re: [PATCH 1/9] Remove unused headers <linux/jz4740-adc.h> and <linux/power/jz4740-battery.h>"
Previous message: Pali Rohár: "Re: [PATCH v15 04/13] PCI: kirin: Add support for bridge slot DT schema"
In reply to: Taniya Das: "[PATCH v1 4/4] clk: qcom: lpass: Add support for LPASS clock controller for SC7280"
Next in thread: Taniya Das: "[PATCH v1 2/4] clk: qcom: gdsc: Add support for clock voting from GDSC"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Quoting Taniya Das (2021-11-02 02:56:53)
> The Low Power Audio subsystem core and audio clocks are required for
> Audio client to be able to request for the clocks and power domains.
>
> Signed-off-by: Taniya Das <tdas@xxxxxxxxxxxxxx>
> ---
> drivers/clk/qcom/Kconfig | 9 +
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/lpassaudiocc-sc7280.c | 831 +++++++++++++++++++++++++++++++++
> drivers/clk/qcom/lpasscorecc-sc7280.c | 491 +++++++++++++++++++
> 4 files changed, 1332 insertions(+)
> create mode 100644 drivers/clk/qcom/lpassaudiocc-sc7280.c
> create mode 100644 drivers/clk/qcom/lpasscorecc-sc7280.c
>
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 74efc82..4dcabc6 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -435,6 +435,15 @@ config SC_LPASS_CORECC_7180
> Say Y if you want to use LPASS clocks and power domains of the LPASS
> core clock controller.
>
> +config SC_LPASS_CORECC_7280
> + tristate "SC7280 LPASS Core & Audio Clock Controller"
> + select SC_GCC_7280

select GDSC too

> + help
> + Support for the LPASS(Low Power Audio Subsystem) core and audio clock
> + controller on SC7280 devices.
> + Say Y if you want to use LPASS clocks and power domains of the LPASS
> + core clock controller.
> +
> config SC_MSS_7180
> tristate "SC7180 Modem Clock Controller"
> select SC_GCC_7180
> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
> index 1718c34..9e4b8e1 100644
> --- a/drivers/clk/qcom/Makefile
> +++ b/drivers/clk/qcom/Makefile
> @@ -69,6 +69,7 @@ obj-$(CONFIG_SC_GPUCC_7180) += gpucc-sc7180.o
> obj-$(CONFIG_SC_GPUCC_7280) += gpucc-sc7280.o
> obj-$(CONFIG_SC_LPASSCC_7280) += lpasscc-sc7280.o
> obj-$(CONFIG_SC_LPASS_CORECC_7180) += lpasscorecc-sc7180.o
> +obj-$(CONFIG_SC_LPASS_CORECC_7280) += lpasscorecc-sc7280.o lpassaudiocc-sc7280.o
> obj-$(CONFIG_SC_MSS_7180) += mss-sc7180.o
> obj-$(CONFIG_SC_VIDEOCC_7180) += videocc-sc7180.o
> obj-$(CONFIG_SC_VIDEOCC_7280) += videocc-sc7280.o
> diff --git a/drivers/clk/qcom/lpassaudiocc-sc7280.c b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> new file mode 100644
> index 0000000..52e7f8e
> --- /dev/null
> +++ b/drivers/clk/qcom/lpassaudiocc-sc7280.c
> @@ -0,0 +1,831 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +#include <linux/pm_clock.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/clock/qcom,lpassaudiocc-sc7280.h>
> +
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
> +#include "clk-rcg.h"
> +#include "clk-regmap.h"
> +#include "clk-regmap-divider.h"
> +#include "clk-regmap-mux.h"
> +#include "common.h"
> +#include "gdsc.h"
> +
> +enum {
> + P_BI_TCXO,
> + P_LPASS_AON_CC_PLL_OUT_EVEN,
> + P_LPASS_AON_CC_PLL_OUT_MAIN,
> + P_LPASS_AON_CC_PLL_OUT_MAIN_CDIV_DIV_CLK_SRC,
> + P_LPASS_AON_CC_PLL_OUT_ODD,
> + P_LPASS_AUDIO_CC_PLL_OUT_AUX,
> + P_LPASS_AUDIO_CC_PLL_OUT_AUX2_DIV_CLK_SRC,
> + P_LPASS_AUDIO_CC_PLL_MAIN_DIV_CLK,
> +};
> +
> +static struct pll_vco zonda_vco[] = {

const

> + { 595200000, 3600000000, 0 },
> +};
> +
> +/* 1128.96MHz configuration */
> +static const struct alpha_pll_config lpass_audio_cc_pll_config = {
> + .l = 0x3a,
> + .alpha = 0xcccc,
> + .config_ctl_val = 0x08200920,
> + .config_ctl_hi_val = 0x05002001,
> + .config_ctl_hi1_val = 0x00000000,
> + .user_ctl_val = 0x03000101,
> +};
> +
> +static struct clk_alpha_pll lpass_audio_cc_pll = {
> + .offset = 0x0,
> + .vco_table = zonda_vco,
> + .num_vco = ARRAY_SIZE(zonda_vco),
> + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_ZONDA],
> + .clkr = {
> + .hw.init = &(struct clk_init_data){

const struct clk_init_data

BTW, we should do that for all the qcom clk drivers.

> + .name = "lpass_audio_cc_pll",
> + .parent_data = &(const struct clk_parent_data){
> + .fw_name = "bi_tcxo",

To make the kernel smaller we should consider using integers in the
.index field instead of .fw_name so that we don't have to store around
strings.

> + },
> + .num_parents = 1,
> + .ops = &clk_alpha_pll_zonda_ops,
> + },
> + },
> +};
> +
> +static const struct clk_div_table post_div_table_lpass_audio_cc_pll_out_aux2[] = {
> + { 0x1, 2 },
> + { }
> +};
> +
> +static struct clk_alpha_pll_postdiv lpass_audio_cc_pll_out_aux2 = {
> + .offset = 0x0,
> + .post_div_shift = 8,
> + .post_div_table = post_div_table_lpass_audio_cc_pll_out_aux2,
> + .num_post_div = ARRAY_SIZE(post_div_table_lpass_audio_cc_pll_out_aux2),
> + .width = 2,
> + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_ZONDA],
> + .clkr.hw.init = &(struct clk_init_data){
> + .name = "lpass_audio_cc_pll_out_aux2",
> + .parent_hws = (const struct clk_hw*[]){
> + &lpass_audio_cc_pll.clkr.hw,
> + },
> + .num_parents = 1,
> + .ops = &clk_alpha_pll_postdiv_zonda_ops,
> + },
> +};
> +
> +static struct pll_vco lucid_vco[] = {

const

> + { 249600000, 2000000000, 0 },
> +};
> +
> diff --git a/drivers/clk/qcom/lpasscorecc-sc7280.c b/drivers/clk/qcom/lpasscorecc-sc7280.c
> new file mode 100644
> index 0000000..812d9bd
> --- /dev/null
> +++ b/drivers/clk/qcom/lpasscorecc-sc7280.c
> @@ -0,0 +1,491 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of.h>
> +#include <linux/pm_clock.h>
> +#include <linux/pm_runtime.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/clock/qcom,lpasscorecc-sc7280.h>
> +
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
> +#include "clk-rcg.h"
> +#include "clk-regmap.h"
[...]
> + .hw.init = &(struct clk_init_data){
> + .name = "lpass_core_cc_sysnoc_mport_core_clk",
> + .parent_hws = (const struct clk_hw*[]){
> + &lpass_core_cc_core_clk_src.clkr.hw,
> + },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + .ops = &clk_branch2_ops,
> + },
> + },
> +};
> +
> +static struct gdsc lpass_core_cc_lpass_core_hm_gdsc = {

Only one space after gdsc please

> + .gdscr = 0x0,
> + .pd = {
> + .name = "lpass_core_cc_lpass_core_hm_gdsc",
> + },
> + .pwrsts = PWRSTS_OFF_ON,
> + .flags = RETAIN_FF_ENABLE,
> + .clk_name = "iface",
> +};
> +
> +static struct clk_regmap *lpass_core_cc_sc7280_clocks[] = {
> + [LPASS_CORE_CC_CORE_CLK] = &lpass_core_cc_core_clk.clkr,
> + [LPASS_CORE_CC_CORE_CLK_SRC] = &lpass_core_cc_core_clk_src.clkr,
> + [LPASS_CORE_CC_DIG_PLL] = &lpass_core_cc_dig_pll.clkr,
> + [LPASS_CORE_CC_DIG_PLL_OUT_MAIN_DIV_CLK_SRC] =
> + &lpass_core_cc_dig_pll_out_main_div_clk_src.clkr,
> + [LPASS_CORE_CC_DIG_PLL_OUT_ODD] = &lpass_core_cc_dig_pll_out_odd.clkr,
> + [LPASS_CORE_CC_EXT_IF0_CLK_SRC] = &lpass_core_cc_ext_if0_clk_src.clkr,
> + [LPASS_CORE_CC_EXT_IF0_IBIT_CLK] = &lpass_core_cc_ext_if0_ibit_clk.clkr,
> + [LPASS_CORE_CC_EXT_IF1_CLK_SRC] = &lpass_core_cc_ext_if1_clk_src.clkr,
> + [LPASS_CORE_CC_EXT_IF1_IBIT_CLK] = &lpass_core_cc_ext_if1_ibit_clk.clkr,
> + [LPASS_CORE_CC_LPM_CORE_CLK] = &lpass_core_cc_lpm_core_clk.clkr,
> + [LPASS_CORE_CC_LPM_MEM0_CORE_CLK] = &lpass_core_cc_lpm_mem0_core_clk.clkr,
> + [LPASS_CORE_CC_SYSNOC_MPORT_CORE_CLK] = &lpass_core_cc_sysnoc_mport_core_clk.clkr,
> +};
> +
> +static struct regmap_config lpass_core_cc_sc7280_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .fast_io = true,
> +};
> +
> +static const struct qcom_cc_desc lpass_core_cc_sc7280_desc = {
> + .config = &lpass_core_cc_sc7280_regmap_config,
> + .clks = lpass_core_cc_sc7280_clocks,
> + .num_clks = ARRAY_SIZE(lpass_core_cc_sc7280_clocks),
> +};
> +
> +static const struct of_device_id lpass_core_cc_sc7280_match_table[] = {
> + { .compatible = "qcom,sc7280-lpasscorecc" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, lpass_core_cc_sc7280_match_table);
> +
> +static struct gdsc *lpass_core_hm_sc7280_gdscs[] = {
> + [LPASS_CORE_CC_LPASS_CORE_HM_GDSC] = &lpass_core_cc_lpass_core_hm_gdsc,
> +};
> +
> +static const struct qcom_cc_desc lpass_core_hm_sc7280_desc = {
> + .config = &lpass_core_cc_sc7280_regmap_config,
> + .gdscs = lpass_core_hm_sc7280_gdscs,
> + .num_gdscs = ARRAY_SIZE(lpass_core_hm_sc7280_gdscs),
> +};
> +
> +static void lpass_pm_runtime_disable(void *data)
> +{
> + pm_runtime_disable(data);
> +}
> +
> +static void lpass_pm_clk_destroy(void *data)

Please make the symbols unique. This is the same function name twice.

> +{
> + pm_clk_destroy(data);
> +}
> +
> +static int lpass_create_pm_clks(struct platform_device *pdev)
> +{
> + int ret;
> +
> + pm_runtime_use_autosuspend(&pdev->dev);
> + pm_runtime_set_autosuspend_delay(&pdev->dev, 100);
> + pm_runtime_enable(&pdev->dev);
> +
> + ret = devm_add_action_or_reset(&pdev->dev, lpass_pm_runtime_disable, &pdev->dev);
> + if (ret)
> + return ret;
> +
> + ret = pm_clk_create(&pdev->dev);
> + if (ret)
> + return ret;
> + ret = devm_add_action_or_reset(&pdev->dev, lpass_pm_clk_destroy, &pdev->dev);
> + if (ret)
> + return ret;
> +
> + ret = pm_clk_add(&pdev->dev, "iface");
> + if (ret < 0)
> + dev_err(&pdev->dev, "failed to acquire iface clock\n");
> +
> + return ret;
> +}
> +
> +static int lpass_core_cc_sc7280_probe(struct platform_device *pdev)
> +{
> + struct regmap *regmap;
> + int ret;
> +
> + ret = lpass_create_pm_clks(pdev);
> + if (ret)
> + return ret;
> +
> + regmap = qcom_cc_map(pdev, &lpass_core_cc_sc7280_desc);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + clk_lucid_pll_configure(&lpass_core_cc_dig_pll, regmap, &lpass_core_cc_dig_pll_config);
> +
> + ret = qcom_cc_really_probe(pdev, &lpass_core_cc_sc7280_desc, regmap);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to register LPASS CORE CC clocks\n");
> + return ret;
> + }
> +
> + pm_runtime_mark_last_busy(&pdev->dev);
> + pm_runtime_put_autosuspend(&pdev->dev);
> +
> + return ret;
> +}
> +
> +static const struct dev_pm_ops lpass_core_cc_pm_ops = {
> + SET_RUNTIME_PM_OPS(pm_clk_suspend, pm_clk_resume, NULL)
> +};
> +
> +static struct platform_driver lpass_core_cc_sc7280_driver = {
> + .probe = lpass_core_cc_sc7280_probe,
> + .driver = {
> + .name = "lpass_core_cc-sc7280",
> + .of_match_table = lpass_core_cc_sc7280_match_table,
> + .pm = &lpass_core_cc_pm_ops,
> + },
> +};
> +
> +static int lpass_hm_core_probe(struct platform_device *pdev)
> +{
> + const struct qcom_cc_desc *desc;
> + int ret;
> +
> + lpass_core_cc_sc7280_regmap_config.name = "lpass_hm_core";
> + desc = &lpass_core_hm_sc7280_desc;
> +
> + ret = qcom_cc_probe_by_index(pdev, 0, desc);
> + if (ret)
> + goto destroy_clk;
> +
> + return 0;
> +
> +destroy_clk:
> + return ret;

return qcom_cc_probe_by_index(...);

is equivalent.

> +}
> +
> +static const struct of_device_id lpass_hm_sc7280_match_table[] = {
> + {
> + .compatible = "qcom,sc7280-lpasshm",
> + },

Please reduce to one line instead of three.

> + { }
> +};
> +MODULE_DEVICE_TABLE(of, lpass_hm_sc7280_match_table);
> +

Next message: Paul Cercueil: "Re: [PATCH 1/9] Remove unused headers <linux/jz4740-adc.h> and <linux/power/jz4740-battery.h>"
Previous message: Pali Rohár: "Re: [PATCH v15 04/13] PCI: kirin: Add support for bridge slot DT schema"
In reply to: Taniya Das: "[PATCH v1 4/4] clk: qcom: lpass: Add support for LPASS clock controller for SC7280"
Next in thread: Taniya Das: "[PATCH v1 2/4] clk: qcom: gdsc: Add support for clock voting from GDSC"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]