linux-kernel - Re: [RFC 3/9] clk: qcom: rcg: Add macros to collapse definition

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]

Date:   Tue, 26 Jul 2022 19:39:40 +0300
From:   Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To:     Abel Vesa <abel.vesa@...aro.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Mike Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, linux-arm-msm@...r.kernel.org,
        linux-clk@...r.kernel.org,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC 3/9] clk: qcom: rcg: Add macros to collapse definition

On Tue, 26 Jul 2022 at 17:23, Abel Vesa <abel.vesa@...aro.org> wrote:
>
> Add macros for a visually more compact rcg clocks definition,
> one for each type of rcg2 ops struct. These are only the ones
> used by gcc-sdm845 driver. More will be added later on.
>
> Signed-off-by: Abel Vesa <abel.vesa@...aro.org>
> ---
>  drivers/clk/qcom/clk-rcg.h | 40 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index 012e745794fd..e856d472a14e 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -180,6 +180,46 @@ struct clk_rcg_dfs_data {
>         struct clk_init_data *init;
>  };
>
> +#define __DEFINE_QCOM_CC_CLK_RCG2(_name, _cmd_rcgr, _mnd_width,                \
> +                               _hid_width, _parent_map, _freq_tbl,     \
> +                               _parent_data, _ops, _flags)             \
> +       static struct clk_init_data _name##_init = {                    \
> +               .name = #_name,                                         \
> +               .parent_data = _parent_data,                            \

I must admit, I do not see beauty here. I'd prefer to be able to use
either parent_data or parent_hws.

> +               .num_parents = ARRAY_SIZE(_parent_data),                \
> +               .ops = _ops,                                            \
> +       };                                                              \
> +                                                                       \
> +       static struct clk_rcg2 _name = {                                \
> +               .cmd_rcgr = _cmd_rcgr,                                  \
> +               .mnd_width = _mnd_width,                                \
> +               .hid_width = _hid_width,                                \
> +               .parent_map = _parent_map,                              \
> +               .freq_tbl = _freq_tbl,                                  \
> +               .clkr.hw.init = &_name##_init,                          \
> +       }
> +
> +#define DEFINE_QCOM_CC_CLK_RCG2(_name, _cmd_rcgr, _mnd_width,          \
> +                               _hid_width, _parent_map, _freq_tbl,     \
> +                               _parent_data)                           \
> +       __DEFINE_QCOM_CC_CLK_RCG2(_name, _cmd_rcgr, _mnd_width,         \
> +                               _hid_width, _parent_map, _freq_tbl,     \
> +                               _parent_data, &clk_rcg2_ops, 0)
> +
> +#define DEFINE_QCOM_CC_CLK_RCG2_SHARED(_name, _cmd_rcgr, _mnd_width,           \
> +                               _hid_width, _parent_map, _freq_tbl,     \
> +                               _parent_data)                           \
> +       __DEFINE_QCOM_CC_CLK_RCG2(_name, _cmd_rcgr, _mnd_width,         \
> +                               _hid_width, _parent_map, _freq_tbl,     \
> +                               _parent_data, &clk_rcg2_shared_ops, 0)
> +
> +#define DEFINE_QCOM_CC_CLK_RCG2_FLOOR(_name, _cmd_rcgr, _mnd_width,            \
> +                               _hid_width, _parent_map, _freq_tbl,     \
> +                               _parent_data)                           \
> +       __DEFINE_QCOM_CC_CLK_RCG2(_name, _cmd_rcgr, _mnd_width,         \
> +                               _hid_width, _parent_map, _freq_tbl,     \
> +                               _parent_data, &clk_rcg2_floor_ops, 0)

Too many variants. I'd suggest making the default one (&clk_rcg2_ops)
and the extensible (with variable ops).

> +
>  #define DEFINE_RCG_DFS(r) \
>         { .rcg = &r, .init = &r##_init }
>
> --
> 2.34.3
>


-- 
With best wishes
Dmitry