linux-kernel - Re: [RFC PATCH] clk: qcom: rpmcc: Add support to XO buffered clocks

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: <152113633637.107923.1559162375425599621@swboyd.mtv.corp.google.com>
Date:   Thu, 15 Mar 2018 10:52:16 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     andy.gross@...aro.org, linux-arm-msm@...r.kernel.org,
        linux-clk@...r.kernel.org, srinivas.kandagatla@...aro.org
Cc:     david.brown@...aro.org, mturquette@...libre.com,
        robh+dt@...nel.org, linux-soc@...r.kernel.org,
        linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
        georgi.djakov@...aro.org,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Subject: Re: [RFC PATCH] clk: qcom: rpmcc: Add support to XO buffered clocks

Quoting srinivas.kandagatla@...aro.org (2018-03-15 07:37:24)
> diff --git a/drivers/clk/qcom/clk-rpm.c b/drivers/clk/qcom/clk-rpm.c
> index c60f61b10c7f..261f5505e714 100644
> --- a/drivers/clk/qcom/clk-rpm.c
> +++ b/drivers/clk/qcom/clk-rpm.c
> @@ -56,6 +57,19 @@
>                 },                                                            \
>         }
>  
> +#define DEFINE_CLK_RPM_XO_BUFFER(_platform, _name, _active, offset)          \
> +       static struct clk_rpm _platform##_##_name = {                         \
> +               .rpm_clk_id = QCOM_RPM_CXO_BUFFERS,                           \
> +               .xo_offset = (offset),                                        \
> +               .rate = 19200000,                                             \

Shouldn't these rates also come from parent, i.e. pxo_board? So drop
this line?

> +               .hw.init = &(struct clk_init_data){                           \
> +                       .ops = &clk_rpm_fixed_ops,                            \
> +                       .name = #_name,                                       \
> +                       .parent_names = (const char *[]){ "pxo_board" },      \
> +                       .num_parents = 1,                                     \
> +               },                                                            \
> +       }
> +
>  #define DEFINE_CLK_RPM_FIXED(_platform, _name, _active, r_id, r)             \
>         static struct clk_rpm _platform##_##_name = {                         \
>                 .rpm_clk_id = (r_id),                                         \
> @@ -128,6 +142,8 @@
>  
>  struct clk_rpm {
>         const int rpm_clk_id;
> +       const int xo_offset;
> +       u32 xo_buffer_value;
>         const bool active_only;
>         unsigned long rate;
>         bool enabled;
> @@ -308,6 +330,11 @@ static void clk_rpm_fixed_unprepare(struct clk_hw *hw)
>         u32 value = 0;
>         int ret;
>  
> +       if (r->rpm_clk_id == QCOM_RPM_CXO_BUFFERS) {
> +               r->xo_buffer_value &= ~(QCOM_RPM_XO_MODE_ON << r->xo_offset);
> +               value = r->xo_buffer_value;
> +       }
> +

I seem to recall that the xo buffers are within the same "word" or
something like that for each of the buffers. So we would need to make
sure we don't overwrite the bits in there between clks that are
enabling/disabling stuff. Maybe make some sort of shared variable they
all point to and then put a mutex around it? May also be worth making
more clk_ops at that point to not conflate with the simpler on/off code.