| 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: <50ce7c9b-bef1-d570-c8b3-74914fbb43d6@collabora.com>
Date: Tue, 7 Mar 2023 10:38:15 +0100
From: AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
To: Chen-Yu Tsai <wenst@...omium.org>
Cc: sboyd@...nel.org, mturquette@...libre.com, robh+dt@...nel.org,
krzysztof.kozlowski+dt@...aro.org, matthias.bgg@...il.com,
edward-jw.yang@...iatek.com, johnson.wang@...iatek.com,
miles.chen@...iatek.com, chun-jie.chen@...iatek.com,
rex-bc.chen@...iatek.com, jose.exposito89@...il.com,
linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org,
kernel@...labora.com
Subject: Re: [PATCH v3 7/7] clk: mediatek: mt8195: Add support for frequency
hopping through FHCTL
Il 07/03/23 10:29, Chen-Yu Tsai ha scritto:
> On Tue, Mar 7, 2023 at 5:27 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@...labora.com> wrote:
>>
>> Il 07/03/23 05:43, Chen-Yu Tsai ha scritto:
>>> On Mon, Feb 6, 2023 at 6:01 PM AngeloGioacchino Del Regno
>>> <angelogioacchino.delregno@...labora.com> wrote:
>>>>
>>>> Add FHCTL parameters and register PLLs through FHCTL to add support
>>>> for frequency hopping and SSC. FHCTL will be enabled only on PLLs
>>>> specified in devicetree.
>>>>
>>>> This commit brings functional changes only upon addition of
>>>> devicetree configuration.
>>>>
>>>> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
>>>> ---
>>>> drivers/clk/mediatek/clk-mt8195-apmixedsys.c | 69 +++++++++++++++++++-
>>>> 1 file changed, 66 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/mediatek/clk-mt8195-apmixedsys.c b/drivers/clk/mediatek/clk-mt8195-apmixedsys.c
>>>> index 1bc917f2667e..c0db31ce0741 100644
>>>> --- a/drivers/clk/mediatek/clk-mt8195-apmixedsys.c
>>>> +++ b/drivers/clk/mediatek/clk-mt8195-apmixedsys.c
>>>> @@ -3,9 +3,11 @@
>>>> // Copyright (c) 2021 MediaTek Inc.
>>>> // Author: Chun-Jie Chen <chun-jie.chen@...iatek.com>
>>>>
>>>> +#include "clk-fhctl.h"
>>>> #include "clk-gate.h"
>>>> #include "clk-mtk.h"
>>>> #include "clk-pll.h"
>>>> +#include "clk-pllfh.h"
>>>>
>>>> #include <dt-bindings/clock/mt8195-clk.h>
>>>> #include <linux/of_device.h>
>>>> @@ -105,6 +107,61 @@ static const struct mtk_pll_data plls[] = {
>>>> 0, 0, 22, 0x0158, 24, 0, 0, 0, 0x0158, 0, 0x0158, 0, 9),
>>>> };
>>>>
>>>> +enum fh_pll_id {
>>>> + FH_ARMPLL_LL,
>>>> + FH_ARMPLL_BL,
>>>> + FH_MEMPLL,
>>>> + FH_ADSPPLL,
>>>> + FH_NNAPLL,
>>>> + FH_CCIPLL,
>>>> + FH_MFGPLL,
>>>> + FH_TVDPLL2,
>>>> + FH_MPLL,
>>>> + FH_MMPLL,
>>>> + FH_MAINPLL,
>>>> + FH_MSDCPLL,
>>>> + FH_IMGPLL,
>>>> + FH_VDECPLL,
>>>> + FH_TVDPLL1,
>>>> + FH_NR_FH,
>>>> +};
>>>> +
>>>> +#define FH(_pllid, _fhid, _offset) { \
>>>> + .data = { \
>>>> + .pll_id = _pllid, \
>>>> + .fh_id = _fhid, \
>>>> + .fh_ver = FHCTL_PLLFH_V2, \
>>>> + .fhx_offset = _offset, \
>>>> + .dds_mask = GENMASK(21, 0), \
>>>
>>>> + .slope0_value = 0x6003c97, \
>>>> + .slope1_value = 0x6003c97, \
>>>
>>> Are these
>>>
>>>> + .sfstrx_en = BIT(2), \
>>>> + .frddsx_en = BIT(1), \
>>>> + .fhctlx_en = BIT(0), \
>>>> + .tgl_org = BIT(31), \
>>>> + .dvfs_tri = BIT(31), \
>>>> + .pcwchg = BIT(31), \
>>>
>>>> + .dt_val = 0x0, \
>>>> + .df_val = 0x9, \
>>>
>>> and these just copied from MT8186?
>>
>> Yes, and that's because they're really the same.
>
> Just to be safe, I asked MediaTek to take a look at the parameters.
>
Cool. Let's see what they say then!
>>>
>>>> + .updnlmt_shft = 16, \
>>>> + .msk_frddsx_dys = GENMASK(23, 20), \
>>>> + .msk_frddsx_dts = GENMASK(19, 16), \
>>>> + }, \
>>>> + }
>>>> +
>>>> +static struct mtk_pllfh_data pllfhs[] = {
>>>> + FH(CLK_APMIXED_ADSPPLL, FH_ADSPPLL, 0x78),
>>>> + FH(CLK_APMIXED_NNAPLL, FH_NNAPLL, 0x8c),
>>>> + FH(CLK_APMIXED_MFGPLL, FH_MFGPLL, 0xb4),
>>>> + FH(CLK_APMIXED_TVDPLL2, FH_TVDPLL2, 0xc8),
>>>> + FH(CLK_APMIXED_MMPLL, FH_MMPLL, 0xf0),
>>>> + FH(CLK_APMIXED_MAINPLL, FH_MAINPLL, 0x104),
>>>> + FH(CLK_APMIXED_MSDCPLL, FH_MSDCPLL, 0x118),
>>>> + FH(CLK_APMIXED_IMGPLL, FH_IMGPLL, 0x12c),
>>>> + FH(CLK_APMIXED_VDECPLL, FH_VDECPLL, 0x140),
>>>> + FH(CLK_APMIXED_TVDPLL2, FH_TVDPLL1, 0x154),
>>>> +};
>>>> +
>>>> static const struct of_device_id of_match_clk_mt8195_apmixed[] = {
>>>> { .compatible = "mediatek,mt8195-apmixedsys", },
>>>> {}
>>>> @@ -114,13 +171,17 @@ static int clk_mt8195_apmixed_probe(struct platform_device *pdev)
>>>> {
>>>> struct clk_hw_onecell_data *clk_data;
>>>> struct device_node *node = pdev->dev.of_node;
>>>> + const u8 *fhctl_node = "mediatek,mt8195-fhctl";
>>>> int r;
>>>>
>>>> clk_data = mtk_alloc_clk_data(CLK_APMIXED_NR_CLK);
>>>> if (!clk_data)
>>>> return -ENOMEM;
>>>>
>>>> - r = mtk_clk_register_plls(node, plls, ARRAY_SIZE(plls), clk_data);
>>>> + fhctl_parse_dt(fhctl_node, pllfhs, ARRAY_SIZE(pllfhs));
>>>> +
>>>> + r = mtk_clk_register_pllfhs(node, plls, ARRAY_SIZE(plls),
>>>> + pllfhs, ARRAY_SIZE(pllfhs), clk_data);
>>>> if (r)
>>>> goto free_apmixed_data;
>>>>
>>>> @@ -140,7 +201,8 @@ static int clk_mt8195_apmixed_probe(struct platform_device *pdev)
>>>> unregister_gates:
>>>> mtk_clk_unregister_gates(apmixed_clks, ARRAY_SIZE(apmixed_clks), clk_data);
>>>> unregister_plls:
>>>> - mtk_clk_unregister_plls(plls, ARRAY_SIZE(plls), clk_data);
>>>> + mtk_clk_unregister_pllfhs(plls, ARRAY_SIZE(plls), pllfhs,
>>>> + ARRAY_SIZE(pllfhs), clk_data);
>>>
>>> Nit: I think this could be squeezed into one line.
>>>
>>>> free_apmixed_data:
>>>> mtk_free_clk_data(clk_data);
>>>> return r;
>>>> @@ -153,7 +215,8 @@ static int clk_mt8195_apmixed_remove(struct platform_device *pdev)
>>>>
>>>> of_clk_del_provider(node);
>>>> mtk_clk_unregister_gates(apmixed_clks, ARRAY_SIZE(apmixed_clks), clk_data);
>>>> - mtk_clk_unregister_plls(plls, ARRAY_SIZE(plls), clk_data);
>>>> + mtk_clk_unregister_pllfhs(plls, ARRAY_SIZE(plls), pllfhs,
>>>> + ARRAY_SIZE(pllfhs), clk_data);
>>>
>>> Same here.
>>>
>>
>> That's the same on the others as well, but if I compress those lines I will have
>> to rebase the clocks cleanup series again and send the 54 patches again.. I'd like
>> to avoid that noise.
>>
>> If you really want though, I can do that... what should I do?
>
> I see. Let's keep it the way it is then.
>
Great. We can always do a small cleanup later, if needed.
Angelo
Powered by blists - more mailing lists