linux-kernel - Re: [PATCH 4/5] clk: eyeq5: add OSPI table-based divider clock

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: <897fe92cfe40a086832e0c85ef5358bc.sboyd@kernel.org>
Date: Tue, 19 Dec 2023 15:16:12 -0800
From: Stephen Boyd <sboyd@...nel.org>
To: Conor Dooley <conor+dt@...nel.org>, Gregory CLEMENT <gregory.clement@...tlin.com>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Michael Turquette <mturquette@...libre.com>, Rob Herring <robh+dt@...nel.org>, Thomas Bogendoerfer <tsbogend@...ha.franken.de>, Théo Lebrun <theo.lebrun@...tlin.com>
Cc: Vladimir Kondratiev <vladimir.kondratiev@...ileye.com>, linux-mips@...r.kernel.org, linux-clk@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, Thomas Petazzoni <thomas.petazzoni@...tlin.com>, Tawfik Bayouk <tawfik.bayouk@...ileye.com>, Théo Lebrun <theo.lebrun@...tlin.com>
Subject: Re: [PATCH 4/5] clk: eyeq5: add OSPI table-based divider clock

Quoting Théo Lebrun (2023-12-18 09:14:19)
> The driver supports PLLs on the platform. Add the single divider clock
> of the platform.
> 
> Helpers from include/linux/clk-provider.h could have been used if it was
> not for the use of regmap to access the register.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@...tlin.com>
> ---

This patch should be squashed with the previous one.

> diff --git a/drivers/clk/clk-eyeq5.c b/drivers/clk/clk-eyeq5.c
> index 74bcb8cec5c1..3382f4d870d7 100644
> --- a/drivers/clk/clk-eyeq5.c
> +++ b/drivers/clk/clk-eyeq5.c
> @@ -77,6 +78,8 @@ static const struct eq5c_pll {
[...]
> +
> +static int eq5c_ospi_div_set_rate(struct clk_hw *hw,
> +                                 unsigned long rate, unsigned long parent_rate)
> +{
> +       struct eq5c_ospi_div *div = clk_hw_to_ospi_priv(hw);
> +       unsigned int val;
> +       int value, ret;
> +
> +       value = divider_get_val(rate, parent_rate, eq5c_ospi_div_table,
> +                               OLB_OSPI_DIV_MASK_WIDTH, 0);
> +       if (value < 0)
> +               return value;
> +
> +       ret = regmap_read(div->olb, OLB_OSPI_REG, &val);
> +       if (ret) {
> +               pr_err("%s: regmap_read failed: %d\n", __func__, ret);
> +               return -ret;

Why negative ret?

> +       }
> +
> +       val &= ~OLB_OSPI_DIV_MASK;
> +       val |= FIELD_PREP(OLB_OSPI_DIV_MASK, value);
> +
> +       ret = regmap_write(div->olb, OLB_OSPI_REG, val);
> +       if (ret) {
> +               pr_err("%s: regmap_write failed: %d\n", __func__, ret);
> +               return -ret;

Why negative ret?

> +       }
> +
> +       return 0;
> +}
> +
> +const struct clk_ops eq5c_ospi_div_ops = {

static?

> +       .recalc_rate = eq5c_ospi_div_recalc_rate,
> +       .round_rate = eq5c_ospi_div_round_rate,
> +       .determine_rate = eq5c_ospi_div_determine_rate,
> +       .set_rate = eq5c_ospi_div_set_rate,
> +};
> +
> +static struct clk_hw *eq5c_init_ospi_div(const struct clk_hw *parent,
> +                                        struct regmap *olb)
> +{
> +       struct eq5c_ospi_div *div;
> +       int ret;
> +
> +       div = kzalloc(sizeof(*div), GFP_KERNEL);
> +       if (!div)
> +               return ERR_PTR(-ENOENT);
> +
> +       div->olb = olb;
> +       div->hw.init = CLK_HW_INIT_HW(EQ5C_OSPI_DIV_CLK_NAME, parent,
> +                                     &eq5c_ospi_div_ops, 0);
> +
> +       ret = clk_hw_register(NULL, &div->hw);
> +       if (ret) {
> +               pr_err("failed registering div_ospi: %d\n", ret);
> +               kfree(div);
> +               return ERR_PTR(-ENOENT);

return ERR_PTR(ret)

> +       }
> +
> +       return &div->hw;
> +}
> +
>  static void eq5c_init(struct device_node *np)
>  {
>         struct device_node *parent_np = of_get_parent(np);