| 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: <374f4284-2c30-4485-a77c-154cde19c885@altera.com>
Date: Mon, 6 Oct 2025 01:39:23 +0000
From: "Romli, Khairul Anuar" <khairul.anuar.romli@...era.com>
To: Dinh Nguyen <dinguyen@...nel.og>, Dinh Nguyen <dinguyen@...nel.org>,
Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>,
"open list:COMMON CLK FRAMEWORK" <linux-clk@...r.kernel.org>, open list
<linux-kernel@...r.kernel.org>, "Ang, Tien Sung" <tien.sung.ang@...era.com>,
"Romli, Khairul Anuar" <khairul.anuar.romli@...era.com>
Subject: Re: [PATCH 1/1] clk: socfpga: agilex5: add clock driver for Agilex5
> On 10/3/25 04:19, Khairul Anuar Romli wrote:
>> Add the new Clock manager driver to support new Agilex5 platform. The new
>> driver got rid of the clk_parent_data structures as there are no
>> 'clock-names'
>> property in the DT bindings and use parent_names internally. This is
>> based on
>> the previous feedback from the maintainer.
>>
>> Signed-off-by: Ang Tien Sung <tiensung.ang@...era.com>
>> Signed-off-by: Khairul Anuar Romli <khairul.anuar.romli@...era.com>
>> ---
>> drivers/clk/socfpga/clk-agilex5.c | 563 +++++++++++++++++++++++++++
>> drivers/clk/socfpga/clk-gate-s10.c | 53 +++
>> drivers/clk/socfpga/clk-periph-s10.c | 41 ++
>> drivers/clk/socfpga/clk-pll-s10.c | 38 +-
>> drivers/clk/socfpga/stratix10-clk.h | 43 ++
>> 5 files changed, 737 insertions(+), 1 deletion(-)
>
> Do you want to add it a Makefile to build it?
I will add it to a Makefile in V2.
>
>> create mode 100644 drivers/clk/socfpga/clk-agilex5.c
>>
>> diff --git a/drivers/clk/socfpga/clk-agilex5.c b/drivers/clk/socfpga/
>> clk-agilex5.c
>> new file mode 100644
>> index 000000000000..0013fab81357
>> --- /dev/null
>> +++ b/drivers/clk/socfpga/clk-agilex5.c
>> @@ -0,0 +1,563 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Copyright (C) 2022-2024, Intel Corporation
>> + * Copyright (C) 2025, Altera Corporation
>> + */
>> +#include <linux/slab.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/of_device.h>
>> +#include <linux/of_address.h>
>
> Remove of_device.h and of_address and just use of.h.
Will use of.h as suggested.
>> +#include <linux/platform_device.h>
>> +#include <dt-bindings/clock/intel,agilex5-clkmgr.h>
>> +#include "stratix10-clk.h"
>> +#include "clk.h"
>> +
>
> <snip>
Sorry, what is this snip?
>
>
>> +
>> +static int
>> +agilex5_clk_register_c_perip(const struct stratix10_perip_c_clock *clks,
>> + int nums, struct stratix10_clock_data *data)
>> +{
>> + struct clk_hw *hw_clk;
>> + void __iomem *base = data->base;
>> + int i;
>> +
>> + for (i = 0; i < nums; i++) {
>> + hw_clk = s10_register_periph(&clks[i], base);
>> + if (IS_ERR(hw_clk)) {
>> + pr_err("%s: failed to register clock %s\n",
>> __func__,
>> + clks[i].name);
>> + continue;
>> + }
>> + data->clk_data.hws[clks[i].id] = hw_clk;
>> + }
>> + return 0;
>> +}
>> +
>> +static int
>> +agilex5_clk_register_cnt_perip(const struct agilex5_perip_cnt_clock
>> *clks,
>> + int nums, struct stratix10_clock_data *data)
>> +{
>> + struct clk_hw *hw_clk;
>> + void __iomem *base = data->base;
>> + int i;
>> +
>> + for (i = 0; i < nums; i++) {
>> + hw_clk = agilex5_register_cnt_periph(&clks[i], base);
>> + if (IS_ERR(hw_clk)) {
>> + pr_err("%s: failed to register clock %s\n",
>> __func__,
>> + clks[i].name);
>> + continue;
>> + }
>> + data->clk_data.hws[clks[i].id] = hw_clk;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int agilex5_clk_register_gate(const struct agilex5_gate_clock
>> *clks,
>> + int nums,
>> + struct stratix10_clock_data *data)
>> +{
>> + struct clk_hw *hw_clk;
>> + void __iomem *base = data->base;
>> + int i;
>> +
>> + for (i = 0; i < nums; i++) {
>> + hw_clk = agilex5_register_gate(&clks[i], base);
>> + if (IS_ERR(hw_clk)) {
>> + pr_err("%s: failed to register clock %s\n",
>> __func__,
>> + clks[i].name);
>> + continue;
>> + }
>> + data->clk_data.hws[clks[i].id] = hw_clk;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int agilex5_clk_register_pll(const struct agilex5_pll_clock
>> *clks,
>> + int nums, struct
>> stratix10_clock_data *data)
>> +{
>> + struct clk_hw *hw_clk;
>> + void __iomem *base = data->base;
>> + int i;
>> +
>> + for (i = 0; i < nums; i++) {
>> + hw_clk = agilex5_register_pll(&clks[i], base);
>> + if (IS_ERR(hw_clk)) {
>> + pr_err("%s: failed to register clock %s\n",
>> __func__,
>> + clks[i].name);
>> + continue;
>> + }
>> + data->clk_data.hws[clks[i].id] = hw_clk;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int agilex5_clkmgr_init(struct platform_device *pdev)
>> +{
>> + struct device_node *np = pdev->dev.of_node;
>> + struct device *dev = &pdev->dev;
>> + struct stratix10_clock_data *clk_data;
>> + struct resource *res;
>> + void __iomem *base;
>> + int i, num_clks;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + base = devm_ioremap_resource(dev, res);
>
> Use devm_platform_ioremap_resource() for simpler code.
Will use devm_platform_ioremap_resource as suggested.
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + num_clks = AGILEX5_NUM_CLKS;
>> +
>> + clk_data = devm_kzalloc(dev, struct_size(clk_data, clk_data.hws,
>> num_clks), GFP_KERNEL);
>> + if (!clk_data)
>> + return -ENOMEM;
>
> Please take a look at commit "65f9e1becb55 clk: socfpga: agilex: Add
> bounds-checking coverage for struct stratix10_clock_data"
>
Will use the similar changes as implemented in the aforementioned commit.
>
>> +
>> + for (i = 0; i < num_clks; i++)
>> + clk_data->clk_data.hws[i] = ERR_PTR(-ENOENT);
>> +
>> + clk_data->base = base;
>> + clk_data->clk_data.num = num_clks;
>> +
>> + agilex5_clk_register_pll(agilex5_pll_clks,
>> ARRAY_SIZE(agilex5_pll_clks),
>> + clk_data);
>> +
>> + /* mainPLL C0, C1, C2, C3 and periph PLL C0, C1, C2, C3*/
>> + agilex5_clk_register_c_perip(agilex5_main_perip_c_clks,
>> + ARRAY_SIZE(agilex5_main_perip_c_clks),
>> + clk_data);
>> +
>> + agilex5_clk_register_cnt_perip(agilex5_main_perip_cnt_clks,
>> +
>> ARRAY_SIZE(agilex5_main_perip_cnt_clks),
>> + clk_data);
>> +
>> + agilex5_clk_register_gate(agilex5_gate_clks,
>> + ARRAY_SIZE(agilex5_gate_clks), clk_data);
>> +
>> + of_clk_add_hw_provider(np, of_clk_hw_onecell_get, &clk_data-
>> >clk_data);
>> + return 0;
>> +}
>> +
>> +static int agilex5_clkmgr_probe(struct platform_device *pdev)
>> +{
>> + int (*probe_func)(struct platform_device *init_func);
>> +
>> + probe_func = of_device_get_match_data(&pdev->dev);
>> + if (!probe_func)
>> + return -ENODEV;
>> + return probe_func(pdev);
>> +}
>> +
>> +static const struct of_device_id agilex5_clkmgr_match_table[] = {
>> + { .compatible = "intel,agilex5-clkmgr", .data =
>> agilex5_clkmgr_init },
>> + {}
>> +};
>> +
>> +static struct platform_driver agilex5_clkmgr_driver = {
>> + .probe = agilex5_clkmgr_probe,
>> + .driver = {
>> + .name = "agilex5-clkmgr",
>> + .suppress_bind_attrs = true,
>> + .of_match_table = agilex5_clkmgr_match_table,
>> + },
>> +};
>> +
>> +static int __init agilex5_clk_init(void)
>> +{
>> + return platform_driver_register(&agilex5_clkmgr_driver);
>> +}
>> +core_initcall(agilex5_clk_init);
>> diff --git a/drivers/clk/socfpga/clk-gate-s10.c b/drivers/clk/socfpga/
>> clk-gate-s10.c
>> index 3930d922efb4..dce3ef137bf3 100644
>> --- a/drivers/clk/socfpga/clk-gate-s10.c
>> +++ b/drivers/clk/socfpga/clk-gate-s10.c
>> @@ -239,3 +239,56 @@ struct clk_hw *agilex_register_gate(const struct
>> stratix10_gate_clock *clks, voi
>> }
>> return hw_clk;
>> }
>> +
>> +struct clk_hw *agilex5_register_gate(const struct agilex5_gate_clock
>> *clks, void __iomem *regbase)
>> +{
>> + struct clk_hw *hw_clk;
>> + struct socfpga_gate_clk *socfpga_clk;
>> + struct clk_init_data init;
>> + int ret;
>> +
>> + socfpga_clk = kzalloc(sizeof(*socfpga_clk), GFP_KERNEL);
>> + if (!socfpga_clk)
>> + return NULL;
>> +
>> + socfpga_clk->hw.reg = regbase + clks->gate_reg;
>> + socfpga_clk->hw.bit_idx = clks->gate_idx;
>> +
>> + gateclk_ops.enable = clk_gate_ops.enable;
>> + gateclk_ops.disable = clk_gate_ops.disable;
>> +
>> + socfpga_clk->fixed_div = clks->fixed_div;
>> +
>> + if (clks->div_reg)
>> + socfpga_clk->div_reg = regbase + clks->div_reg;
>> + else
>> + socfpga_clk->div_reg = NULL;
>> +
>> + socfpga_clk->width = clks->div_width;
>> + socfpga_clk->shift = clks->div_offset;
>> +
>> + if (clks->bypass_reg)
>> + socfpga_clk->bypass_reg = regbase + clks->bypass_reg;
>> + else
>> + socfpga_clk->bypass_reg = NULL;
>> + socfpga_clk->bypass_shift = clks->bypass_shift;
>> +
>> + if (streq(clks->name, "cs_pdbg_clk"))
>> + init.ops = &dbgclk_ops;
>> + else
>> + init.ops = &agilex_gateclk_ops;
>> +
>> + init.name = clks->name;
>> + init.flags = clks->flags;
>> + init.num_parents = clks->num_parents;
>> + init.parent_names = clks->parent_names;
>> + socfpga_clk->hw.hw.init = &init;
>> + hw_clk = &socfpga_clk->hw.hw;
>> +
>> + ret = clk_hw_register(NULL, &socfpga_clk->hw.hw);
>> + if (ret) {
>> + kfree(socfpga_clk);
>> + return ERR_PTR(ret);
>> + }
>> + return hw_clk;
>> +}
>> diff --git a/drivers/clk/socfpga/clk-periph-s10.c b/drivers/clk/
>> socfpga/clk-periph-s10.c
>> index f5c1ca42b668..f12ca43ffe7c 100644
>> --- a/drivers/clk/socfpga/clk-periph-s10.c
>> +++ b/drivers/clk/socfpga/clk-periph-s10.c
>> @@ -214,3 +214,44 @@ struct clk_hw *s10_register_cnt_periph(const
>> struct stratix10_perip_cnt_clock *c
>> }
>> return hw_clk;
>> }
>> +
>> +struct clk_hw *agilex5_register_cnt_periph(const struct
>> agilex5_perip_cnt_clock *clks,
>> + void __iomem *regbase)
>> +{
>> + struct clk_hw *hw_clk;
>> + struct socfpga_periph_clk *periph_clk;
>> + struct clk_init_data init;
>> + const char *name = clks->name;
>> + int ret;
>> +
>> + periph_clk = kzalloc(sizeof(*periph_clk), GFP_KERNEL);
>> + if (WARN_ON(!periph_clk))
>> + return NULL;
>> +
>> + if (clks->offset)
>> + periph_clk->hw.reg = regbase + clks->offset;
>> + else
>> + periph_clk->hw.reg = NULL;
>> +
>> + if (clks->bypass_reg)
>> + periph_clk->bypass_reg = regbase + clks->bypass_reg;
>> + else
>> + periph_clk->bypass_reg = NULL;
>> + periph_clk->bypass_shift = clks->bypass_shift;
>> + periph_clk->fixed_div = clks->fixed_divider;
>> +
>> + init.name = name;
>> + init.ops = &peri_cnt_clk_ops;
>> + init.flags = clks->flags;
>> + init.num_parents = clks->num_parents;
>> + init.parent_names = clks->parent_names;
>> + periph_clk->hw.hw.init = &init;
>> + hw_clk = &periph_clk->hw.hw;
>> +
>> + ret = clk_hw_register(NULL, hw_clk);
>> + if (ret) {
>> + kfree(periph_clk);
>> + return ERR_PTR(ret);
>> + }
>> + return hw_clk;
>> +}
>> diff --git a/drivers/clk/socfpga/clk-pll-s10.c b/drivers/clk/socfpga/
>> clk-pll-s10.c
>> index a88c212bda12..ae80814cfa92 100644
>> --- a/drivers/clk/socfpga/clk-pll-s10.c
>> +++ b/drivers/clk/socfpga/clk-pll-s10.c
>> @@ -182,7 +182,7 @@ static const struct clk_ops clk_pll_ops = {
>> };
>>
>> static const struct clk_ops clk_boot_ops = {
>> - .recalc_rate = clk_boot_clk_recalc_rate,
>> + .recalc_rate = clk_boot_clk_recalc_rate, /* TODO this is wrong*/
>
> Do you know what's wrong with it? Maybe fix it first?
clk_boot_clk_recalc_rate fix will be in the separate patch.
>
> Dinh
Powered by blists - more mailing lists