linux-kernel - Re: [PATCH V3 5/9] clk: qcom: add Global Clock controller (GCC) driver for IPQ5332 SoC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <019bf6ae-cdc4-8fa7-4d76-c39403d93b34@linaro.org>
Date:   Mon, 6 Feb 2023 11:56:56 +0200
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Kathiravan T <quic_kathirav@...cinc.com>,
        krzysztof.kozlowski@...aro.org, agross@...nel.org,
        andersson@...nel.org, konrad.dybcio@...aro.org, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, mturquette@...libre.com,
        sboyd@...nel.org, linus.walleij@...aro.org,
        catalin.marinas@....com, will@...nel.org, shawnguo@...nel.org,
        arnd@...db.de, marcel.ziswiler@...adex.com,
        nfraprado@...labora.com, robimarko@...il.com,
        quic_gurus@...cinc.com, linux-arm-msm@...r.kernel.org,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-clk@...r.kernel.org, linux-gpio@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Cc:     quic_varada@...cinc.com, quic_srichara@...cinc.com
Subject: Re: [PATCH V3 5/9] clk: qcom: add Global Clock controller (GCC)
 driver for IPQ5332 SoC

On 06/02/2023 09:12, Kathiravan T wrote:
 > Add support for the global clock controller found on IPQ5332 SoC.
 >
 > Signed-off-by: Kathiravan T <quic_kathirav@...cinc.com>
 > ---
 > Changes in V3:
 >     - As I mentined the bindings, changes need to be done in V2 got
 >       missed out and same has been done in V3, to call out
 >       specifically dropped the CLK_IS_CRITICAL and dropped the
 >       gcc_apss_ahb_clk, its source clock and gcc_apss_axi_clk
 >     - Used gcc_parent_data_xo wherever applicable and dropped the
 >       duplicate entries
 >     - dropped the unused parent_map_10 and parent_data_10
 >     - Used qcom_cc_probe instead of qcom_cc_really_probe
 > Changes in V2:
 >     - Added the 'dependes on' for Kconfig symbol
 >     - Dropped the CLK_IS_CRITICAL flag throughout the file
 >     - Dropped the gcc_apss_ahb_clk and gcc_apss_axi_clk as these are
 >       managed by bootloaders


[skipped]

 > +static const struct freq_tbl ftbl_gcc_pcie_aux_clk_src[] = {
 > +    F(2000000, P_XO, 12, 0, 0),
 > +    { }
 > +};
 > +
 > +static struct clk_rcg2 gcc_pcie_aux_clk_src = {
 > +    .cmd_rcgr = 0x28004,
 > +    .mnd_width = 16,
 > +    .hid_width = 5,
 > +    .parent_map = gcc_parent_map_6,
 > +    .freq_tbl = ftbl_gcc_pcie_aux_clk_src,
 > +    .clkr.hw.init = &(const struct clk_init_data){
 > +        .name = "gcc_pcie_aux_clk_src",
 > +        .parent_data = gcc_parent_data_6,
 > +        .num_parents = ARRAY_SIZE(gcc_parent_data_6),
 > +        .ops = &clk_rcg2_ops,
 > +    },
 > +};
 > +
 > +static struct clk_regmap_mux pcie3x2_pipe_clk_src = {
 > +    .reg = 0x28064,
 > +    .shift = 8,
 > +    .width = 2,
 > +    .parent_map = gcc_parent_map_14,
 > +    .clkr = {
 > +        .hw.init = &(struct clk_init_data){
 > +            .name = "pcie3x2_phy_pipe_clk_src",
 > +            .parent_data = gcc_parent_data_14,
 > +            .num_parents = ARRAY_SIZE(gcc_parent_data_14),
 > +            .ops = &clk_regmap_mux_closest_ops,

Should we use clk_regmap_phy_mux_ops here instead?

 > +            .flags = CLK_SET_RATE_PARENT,
 > +        },
 > +    },
 > +};
 > +
 > +static struct clk_regmap_mux pcie3x1_0_pipe_clk_src = {
 > +    .reg = 0x29064,
 > +    .shift = 8,
 > +    .width = 2,
 > +    .parent_map = gcc_parent_map_15,
 > +    .clkr = {
 > +        .hw.init = &(struct clk_init_data){
 > +            .name = "pcie3x1_0_phy_pipe_clk_src",
 > +            .parent_data = gcc_parent_data_15,
 > +            .num_parents = ARRAY_SIZE(gcc_parent_data_15),
 > +            .ops = &clk_regmap_mux_closest_ops,

And clk_regmap_phy_mux_ops here too?

 > +            .flags = CLK_SET_RATE_PARENT,
 > +        },
 > +    },
 > +};
 > +
 > +static struct clk_regmap_mux pcie3x1_1_pipe_clk_src = {
 > +    .reg = 0x2A064,
 > +    .shift = 8,
 > +    .width = 2,
 > +    .parent_map = gcc_parent_map_16,
 > +    .clkr = {
 > +        .hw.init = &(struct clk_init_data){
 > +            .name = "pcie3x1_1_phy_pipe_clk_src",
 > +            .parent_data = gcc_parent_data_16,
 > +            .num_parents = ARRAY_SIZE(gcc_parent_data_16),
 > +            .ops = &clk_regmap_mux_closest_ops,

And here?

 > +            .flags = CLK_SET_RATE_PARENT,
 > +        },
 > +    },
 > +};
 > +

[skipped]


 > +
 > +static struct clk_branch gcc_pcie3x1_0_pipe_clk = {
 > +    .halt_reg = 0x29068,
 > +    .halt_check = BRANCH_HALT_DELAY,
 > +    .clkr = {
 > +        .enable_reg = 0x29068,
 > +        .enable_mask = BIT(0),
 > +        .hw.init = &(const struct clk_init_data){
 > +            .name = "gcc_pcie3x1_0_pipe_clk",
 > +            .parent_names = (const char *[]){
 > +                "pcie3x1_0_pipe_clk_src"
 > +            },

Nooo. No parent_names please. You have several of them in the driver

 > +            .num_parents = 1,
 > +            .flags = CLK_SET_RATE_PARENT,
 > +            .ops = &clk_branch2_ops,
 > +        },
 > +    },
 > +};
 > +

-- 
With best wishes
Dmitry