| 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
| ||
|
Message-ID: <a10cdae8-3f18-7ffc-8db7-ab21b47b91ca@linaro.org>
Date: Mon, 26 Jun 2023 16:40:09 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Jagadeesh Kona <quic_jkona@...cinc.com>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Andy Gross <agross@...nel.org>,
Michael Turquette <mturquette@...libre.com>,
Stephen Boyd <sboyd@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>,
Vladimir Zapolskiy <vladimir.zapolskiy@...aro.org>,
linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
Taniya Das <quic_tdas@...cinc.com>,
Satya Priya Kakitapalli <quic_skakitap@...cinc.com>,
Imran Shaik <quic_imrashai@...cinc.com>,
Ajit Pandey <quic_ajipan@...cinc.com>
Subject: Re: [PATCH V5 4/5] clk: qcom: camcc-sm8550: Add support for qdss,
sleep and xo clocks
On 26/06/2023 14:59, Jagadeesh Kona wrote:
>
>
> On 6/24/2023 5:49 PM, Konrad Dybcio wrote:
>> On 23.06.2023 18:46, Jagadeesh Kona wrote:
>>> Add support for camera qdss, sleep and xo clocks.
>>>
>>> Co-developed-by: Taniya Das <quic_tdas@...cinc.com>
>>> Signed-off-by: Taniya Das <quic_tdas@...cinc.com>
>>> Signed-off-by: Jagadeesh Kona <quic_jkona@...cinc.com>
>>> ---
>>> Changes since v4:
>>> - No changes.
>>> Changes since v3:
>>> - No changes.
>>> Changes since v2:
>>> - No changes.
>>> Changes since v1:
>>> - Newly added.
>>>
>>> drivers/clk/qcom/camcc-sm8550.c | 180 ++++++++++++++++++++++++++++++++
>>> 1 file changed, 180 insertions(+)
>>>
>>> diff --git a/drivers/clk/qcom/camcc-sm8550.c
>>> b/drivers/clk/qcom/camcc-sm8550.c
>>> index 075bea32087c..7b4882444d58 100644
>>> --- a/drivers/clk/qcom/camcc-sm8550.c
>>> +++ b/drivers/clk/qcom/camcc-sm8550.c
>>> @@ -22,6 +22,8 @@
>>> enum {
>>> DT_IFACE,
>>> DT_BI_TCXO,
>>> + DT_BI_TCXO_AO,
>>> + DT_SLEEP_CLK,
>>> };
>>> enum {
>>> @@ -43,6 +45,7 @@ enum {
>>> P_CAM_CC_PLL10_OUT_EVEN,
>>> P_CAM_CC_PLL11_OUT_EVEN,
>>> P_CAM_CC_PLL12_OUT_EVEN,
>>> + P_SLEEP_CLK,
>>> };
>>> static const struct pll_vco lucid_ole_vco[] = {
>>> @@ -881,6 +884,22 @@ static const struct clk_parent_data
>>> cam_cc_parent_data_11[] = {
>>> { .hw = &cam_cc_pll7_out_even.clkr.hw },
>>> };
>>> +static const struct parent_map cam_cc_parent_map_12[] = {
>>> + { P_SLEEP_CLK, 0 },
>>> +};
>>> +
>>> +static const struct clk_parent_data cam_cc_parent_data_12[] = {
>>> + { .index = DT_SLEEP_CLK },
>>> +};
>>> +
>>> +static const struct parent_map cam_cc_parent_map_13[] = {
>>> + { P_BI_TCXO, 0 },
>>> +};
>>> +
>>> +static const struct clk_parent_data cam_cc_parent_data_13_ao[] = {
>>> + { .index = DT_BI_TCXO_AO },
>>> +};
>>> +
>>> static const struct freq_tbl ftbl_cam_cc_bps_clk_src[] = {
>>> F(19200000, P_BI_TCXO, 1, 0, 0),
>>> F(200000000, P_CAM_CC_PLL8_OUT_EVEN, 1, 0, 0),
>>> @@ -1565,6 +1584,29 @@ static struct clk_rcg2 cam_cc_mclk7_clk_src = {
>>> },
>>> };
>>> +static const struct freq_tbl ftbl_cam_cc_qdss_debug_clk_src[] = {
>>> + F(19200000, P_BI_TCXO, 1, 0, 0),
>>> + F(75000000, P_CAM_CC_PLL0_OUT_EVEN, 8, 0, 0),
>>> + F(150000000, P_CAM_CC_PLL0_OUT_EVEN, 4, 0, 0),
>>> + F(300000000, P_CAM_CC_PLL0_OUT_MAIN, 4, 0, 0),
>>> + { }
>>> +};
>>> +
>>> +static struct clk_rcg2 cam_cc_qdss_debug_clk_src = {
>>> + .cmd_rcgr = 0x13f24,
>>> + .mnd_width = 0,
>>> + .hid_width = 5,
>>> + .parent_map = cam_cc_parent_map_0,
>>> + .freq_tbl = ftbl_cam_cc_qdss_debug_clk_src,
>>> + .clkr.hw.init = &(const struct clk_init_data) {
>>> + .name = "cam_cc_qdss_debug_clk_src",
>>> + .parent_data = cam_cc_parent_data_0,
>>> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_0),
>>> + .flags = CLK_SET_RATE_PARENT,
>>> + .ops = &clk_rcg2_shared_ops,
>>> + },
>>> +};
>>> +
>>> static const struct freq_tbl ftbl_cam_cc_sfe_0_clk_src[] = {
>>> F(466000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0),
>>> F(594000000, P_CAM_CC_PLL6_OUT_EVEN, 1, 0, 0),
>>> @@ -1611,6 +1653,26 @@ static struct clk_rcg2 cam_cc_sfe_1_clk_src = {
>>> },
>>> };
>>> +static const struct freq_tbl ftbl_cam_cc_sleep_clk_src[] = {
>>> + F(32000, P_SLEEP_CLK, 1, 0, 0),
>>> + { }
>>> +};
>>> +
>>> +static struct clk_rcg2 cam_cc_sleep_clk_src = {
>>> + .cmd_rcgr = 0x141a0,
>>> + .mnd_width = 0,
>>> + .hid_width = 5,
>>> + .parent_map = cam_cc_parent_map_12,
>>> + .freq_tbl = ftbl_cam_cc_sleep_clk_src,
>>> + .clkr.hw.init = &(const struct clk_init_data) {
>>> + .name = "cam_cc_sleep_clk_src",
>>> + .parent_data = cam_cc_parent_data_12,
>>> + .num_parents = ARRAY_SIZE(cam_cc_parent_data_12),
>>> + .flags = CLK_SET_RATE_PARENT,
>>> + .ops = &clk_rcg2_shared_ops,
>>> + },
>>> +};
>>> +
>>> static const struct freq_tbl ftbl_cam_cc_slow_ahb_clk_src[] = {
>>> F(19200000, P_BI_TCXO, 1, 0, 0),
>>> F(80000000, P_CAM_CC_PLL0_OUT_EVEN, 7.5, 0, 0),
>>> @@ -1632,6 +1694,26 @@ static struct clk_rcg2 cam_cc_slow_ahb_clk_src
>>> = {
>>> },
>>> };
>>> +static const struct freq_tbl ftbl_cam_cc_xo_clk_src[] = {
>>> + F(19200000, P_BI_TCXO, 1, 0, 0),
>> You're overloading P_BI_TCXO with a different parent clock (XO_A).
>>
>
> This RCG just requires active only voting, hence using XO_A as its parent.
>
> Both XO and XO_A are same clock in HW (BI_TCXO), hence we can reuse
> P_BI_TCXO in frequency table for XO_A parent as well.
Please don't do such things, it complicates understanding the driver.
The reviewer could have thought that here the driver was really
referencing to the BI_TCXO rather than BI_TCXO_AO.
>
> Thanks,
> Jagadeesh
>
>> The rest lgtm
>>
>> Konrad
[skipped the rest]
--
With best wishes
Dmitry
Powered by blists - more mailing lists