Re: [PATCH v2 6/6] clk: qcom: Add video clock controller driver for SC7280

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Taniya Das (2021-04-27 03:33:10)
> diff --git a/drivers/clk/qcom/videocc-sc7280.c b/drivers/clk/qcom/videocc-sc7280.c
> new file mode 100644
> index 0000000..3387154
> --- /dev/null
> +++ b/drivers/clk/qcom/videocc-sc7280.c
> @@ -0,0 +1,372 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2021, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#include <dt-bindings/clock/qcom,videocc-sc7280.h>
> +
> +#include "clk-alpha-pll.h"
> +#include "clk-branch.h"
> +#include "clk-rcg.h"
> +#include "common.h"
> +#include "reset.h"
> +#include "gdsc.h"
> +
> +enum {
> +       P_BI_TCXO,
> +       P_SLEEP_CLK,
> +       P_VIDEO_PLL0_OUT_EVEN,
> +};
> +
> +static struct pll_vco lucid_vco[] = {

const?

> +       { 249600000, 2000000000, 0 },
> +};
> +
[...]
> +
> +static const struct parent_map video_cc_parent_map_0[] = {
> +       { P_BI_TCXO, 0 },
> +       { P_VIDEO_PLL0_OUT_EVEN, 3 },
> +};
> +
> +static const struct clk_parent_data video_cc_parent_data_0[] = {
> +       { .fw_name = "bi_tcxo" },
> +       { .hw = &video_pll0.clkr.hw },
> +};
> +
> +static const struct parent_map video_cc_parent_map_1[] = {
> +       { P_SLEEP_CLK, 0 },
> +};
> +
> +static const struct clk_parent_data video_cc_parent_data_1[] = {
> +       { .fw_name = "sleep_clk" },
> +};
> +
> +static const struct parent_map video_cc_parent_map_2[] = {
> +       { P_BI_TCXO, 0 },
> +};
> +
> +static const struct clk_parent_data video_cc_parent_data_2_ao[] = {
> +       { .fw_name = "bi_tcxo_ao" },

This is new. Why would we want the video clk parent state to turn off
when the CPU is off? Does the video engine keep XO enabled for itself?
Can you please add some comment into the code explaining why it's ok to
use the ao clk here?

> +};
> +
> +static const struct freq_tbl ftbl_video_cc_iris_clk_src[] = {
> +       F(133333333, P_VIDEO_PLL0_OUT_EVEN, 3, 0, 0),
> +       F(240000000, P_VIDEO_PLL0_OUT_EVEN, 2, 0, 0),
> +       F(335000000, P_VIDEO_PLL0_OUT_EVEN, 2, 0, 0),
> +       F(424000000, P_VIDEO_PLL0_OUT_EVEN, 2, 0, 0),
> +       F(460000000, P_VIDEO_PLL0_OUT_EVEN, 2, 0, 0),
> +       { }
> +};
[...]
> +
> +static struct clk_branch video_cc_xo_clk = {
> +       .halt_reg = 0x7018,
> +       .halt_check = BRANCH_HALT,
> +       .clkr = {
> +               .enable_reg = 0x7018,
> +               .enable_mask = BIT(0),
> +               .hw.init = &(struct clk_init_data){
> +                       .name = "video_cc_xo_clk",
> +                       .parent_hws = (const struct clk_hw*[]){
> +                               &video_cc_xo_clk_src.clkr.hw,
> +                       },
> +                       .num_parents = 1,
> +                       .flags = CLK_IS_CRITICAL | CLK_SET_RATE_PARENT,

Please add a comment why it is critical.

> +                       .ops = &clk_branch2_ops,
> +               },
> +       },
> +};
> +




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux