Re: [PATCH v3 2/2] clk: qcom: Add display clock controller driver for QCM2290

[Stephen Boyd <sboyd@kernel.org>]

Quoting Loic Poulain (2021-12-15 09:09:40)
> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
> index 74efc82..b136cd2 100644
> --- a/drivers/clk/qcom/Kconfig
> +++ b/drivers/clk/qcom/Kconfig
> @@ -332,6 +332,14 @@ config QCM_GCC_2290
>           Say Y if you want to use multimedia devices or peripheral
>           devices such as UART, SPI, I2C, USB, SD/eMMC etc.
>  
> +config QCM_DISPCC_2290
> +       tristate "QCM2290 Display Clock Controller"

Select QCM_GCC_2290 here so that this driver being builtin forces GCC to
be builtin.

> +       help
> +         Support for the display clock controller on Qualcomm Technologies, Inc
> +         QCM2290 devices.
> +         Say Y if you want to support display devices and functionality such as
> +         splash screen.
> +
>  config QCS_GCC_404
>         tristate "QCS404 Global Clock Controller"
>         help
> diff --git a/drivers/clk/qcom/dispcc-qcm2290.c b/drivers/clk/qcom/dispcc-qcm2290.c
> new file mode 100644
> index 00000000..6854371
> --- /dev/null
> +++ b/drivers/clk/qcom/dispcc-qcm2290.c
> @@ -0,0 +1,602 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2021, Linaro Ltd.
> + */
> +
> +#include <linux/clk.h>

Same comments apply on this version.

> +#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/regmap.h>
> +
> +#include <dt-bindings/clock/qcom,dispcc-qcm2290.h>
> +
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
> +#include "clk-rcg.h"
> +#include "clk-regmap.h"
> +#include "clk-regmap-divider.h"
> +#include "common.h"
> +#include "reset.h"
> +#include "gdsc.h"

Is there any reset or gdsc? Please include what is used instead of
copy/pasting includes from somewhere else.

> +
> +enum {
> +       P_BI_TCXO,
> +       P_CORE_BI_PLL_TEST_SE,
> +       P_DISP_CC_PLL0_OUT_MAIN,
> +       P_DSI0_PHY_PLL_OUT_BYTECLK,
> +       P_DSI0_PHY_PLL_OUT_DSICLK,
> +       P_DSI1_PHY_PLL_OUT_DSICLK,
> +       P_GPLL0_OUT_MAIN,
> +       P_SLEEP_CLK,
> +};
> +
> +static struct pll_vco spark_vco[] = {
> +       { 500000000, 1000000000, 2 },
> +};
> +
> +/* 768MHz configuration */
> +static const struct alpha_pll_config disp_cc_pll0_config = {
> +       .l = 0x28,
> +       .alpha = 0x0,
> +       .alpha_en_mask = BIT(24),
> +       .vco_val = 0x2 << 20,
> +       .vco_mask = GENMASK(21, 20),
> +       .main_output_mask = BIT(0),
> +       .config_ctl_val = 0x4001055B,
> +};
> +
[...]
> +
> +static int disp_cc_qcm2290_probe(struct platform_device *pdev)
> +{
> +       struct regmap *regmap;
> +       int ret;
> +
> +       regmap = qcom_cc_map(pdev, &disp_cc_qcm2290_desc);
> +       if (IS_ERR(regmap))
> +               return PTR_ERR(regmap);
> +
> +       clk_alpha_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
> +
> +       ret = qcom_cc_really_probe(pdev, &disp_cc_qcm2290_desc, regmap);
> +       if (ret) {
> +               dev_err(&pdev->dev, "Failed to register DISP CC clocks\n");
> +               return ret;
> +       }
> +
> +       dev_info(&pdev->dev, "Registered DISP CC clocks\n");

Remove this.

> +
> +       return ret;
> +}
> +