Re: [PATCH 05/10] clk: qcom: add Global Clock controller (GCC) driver for IPQ5332 SoC

[Stephen Boyd]

Quoting Kathiravan Thirumoorthy (2023-01-25 02:45:15)
> diff --git a/drivers/clk/qcom/gcc-ipq5332.c b/drivers/clk/qcom/gcc-ipq5332.c
> new file mode 100644
> index 000000000000..8351096a4d32
> --- /dev/null
> +++ b/drivers/clk/qcom/gcc-ipq5332.c
> @@ -0,0 +1,3954 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/regmap.h>
[...]
> +
> +static const struct freq_tbl ftbl_gcc_pcnoc_bfdcd_clk_src[] = {
> +       F(24000000, P_XO, 1, 0, 0),
> +       F(50000000, P_GPLL0_OUT_MAIN, 16, 0, 0),
> +       F(100000000, P_GPLL0_OUT_MAIN, 8, 0, 0),
> +       { }
> +};
> +
> +static struct clk_rcg2 gcc_pcnoc_bfdcd_clk_src = {
> +       .cmd_rcgr = 0x31004,
> +       .mnd_width = 0,
> +       .hid_width = 5,
> +       .parent_map = gcc_parent_map_0,
> +       .freq_tbl = ftbl_gcc_pcnoc_bfdcd_clk_src,
> +       .clkr.hw.init = &(const struct clk_init_data){
> +               .name = "gcc_pcnoc_bfdcd_clk_src",
> +               .parent_data = gcc_parent_data_0,
> +               .num_parents = ARRAY_SIZE(gcc_parent_data_0),
> +               .ops = &clk_rcg2_ops,
> +               .flags = CLK_IS_CRITICAL,

Why not just turn these clks on in probe and never register them with
the framework? That saves some memory for clks that there is no desire
to control from linux. This is an RCG, so in theory the frequency can
change, but does it really? Usually bus clks are controlled by the
interconnect driver.