| 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: <d36ff27b56b3e9c8ef490bfd9d24761d.sboyd@kernel.org>
Date: Wed, 10 Jul 2024 16:17:12 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Albert Ou <aou@...s.berkeley.edu>, Conor Dooley <conor+dt@...nel.org>, Drew Fustini <dfustini@...storrent.com>, Emil Renner Berthing <emil.renner.berthing@...onical.com>, Fu Wei <wefu@...hat.com>, Guo Ren <guoren@...nel.org>, Jisheng Zhang <jszhang@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Michael Turquette <mturquette@...libre.com>, Palmer Dabbelt <palmer@...belt.com>, Paul Walmsley <paul.walmsley@...ive.com>, Rob Herring <robh@...nel.org>, Thomas Bonnefille <thomas.bonnefille@...tlin.com>, Yangtao Li <frank.li@...o.com>
Cc: linux-riscv@...ts.infradead.org, linux-clk@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, Drew Fustini <dfustini@...storrent.com>
Subject: Re: [PATCH v2 2/7] clk: thead: Add support for T-Head TH1520 AP_SUBSYS clocks
Quoting Drew Fustini (2024-06-23 19:12:32)
> diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
> new file mode 100644
> index 000000000000..982d4d40f783
> --- /dev/null
> +++ b/drivers/clk/thead/clk-th1520-ap.c
> @@ -0,0 +1,1086 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023 Jisheng Zhang <jszhang@...nel.org>
> + * Copyright (C) 2023 Vivo Communication Technology Co. Ltd.
> + * Authors: Yangtao Li <frank.li@...o.com>
> + */
> +
> +#include <dt-bindings/clock/thead,th1520-clk-ap.h>
Preferably include dt-bindings after linux includes.
> +#include <linux/bitfield.h>
> +#include <linux/clk-provider.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/regmap.h>
> +
> +#define TH1520_PLL_POSTDIV2 GENMASK(26, 24)
> +#define TH1520_PLL_POSTDIV1 GENMASK(22, 20)
> +#define TH1520_PLL_FBDIV GENMASK(19, 8)
> +#define TH1520_PLL_REFDIV GENMASK(5, 0)
> +#define TH1520_PLL_BYPASS BIT(30)
> +#define TH1520_PLL_DSMPD BIT(24)
> +#define TH1520_PLL_FRAC GENMASK(23, 0)
> +#define TH1520_PLL_FRAC_BITS 24
[...]
> +
> +static unsigned long th1520_pll_vco_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct ccu_pll *pll = hw_to_ccu_pll(hw);
> + unsigned long div, mul, frac, rate = parent_rate;
> + unsigned int cfg0, cfg1;
> +
> + regmap_read(pll->common.map, pll->common.cfg0, &cfg0);
> + regmap_read(pll->common.map, pll->common.cfg1, &cfg1);
> +
> + mul = FIELD_GET(TH1520_PLL_FBDIV, cfg0);
> + div = FIELD_GET(TH1520_PLL_REFDIV, cfg0);
> + if (!(cfg1 & TH1520_PLL_DSMPD)) {
> + mul <<= TH1520_PLL_FRAC_BITS;
> + frac = FIELD_GET(TH1520_PLL_FRAC, cfg1);
> + mul += frac;
> + div <<= TH1520_PLL_FRAC_BITS;
> + }
> + rate = parent_rate * mul;
> + do_div(rate, div);
'rate' is only unsigned long, so do_div() isn't needed here. Perhaps if
'parent_rate * mul' can overflow 32-bits then 'rate' should be
u64.
> + return rate;
> +}
> +
> +static unsigned long th1520_pll_postdiv_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + struct ccu_pll *pll = hw_to_ccu_pll(hw);
> + unsigned long rate = parent_rate;
> + unsigned int cfg0, cfg1;
> +
> + regmap_read(pll->common.map, pll->common.cfg0, &cfg0);
> + regmap_read(pll->common.map, pll->common.cfg1, &cfg1);
> +
> + if (cfg1 & TH1520_PLL_BYPASS)
> + return rate;
> +
> + do_div(rate, FIELD_GET(TH1520_PLL_POSTDIV1, cfg0) *
Same, 'rate' is unsigned long. Did you get some compilation error
without this? How big is the divisor going to be? The fields are only
3-bits wide, so the multiplication would fit into a u32 just fine. Given
that 'rate' is unsigned long though I think you can just put the
multiplication result into a local variable that's also unsigned long
and then just write the divide with unsigned longs
div = FIELD_GET(...) * FIELD_GET(...);
return rate / div;
> + FIELD_GET(TH1520_PLL_POSTDIV2, cfg0));
> +
> + return rate;
> +}
> +
> +static unsigned long ccu_pll_recalc_rate(struct clk_hw *hw,
> + unsigned long parent_rate)
> +{
> + unsigned long rate = parent_rate;
> +
> + rate = th1520_pll_vco_recalc_rate(hw, rate);
> + rate = th1520_pll_postdiv_recalc_rate(hw, rate);
> +
> + return rate;
> +}
Please fold this in
----8<---
diff --git a/drivers/clk/thead/clk-th1520-ap.c b/drivers/clk/thead/clk-th1520-ap.c
index 982d4d40f783..0c3821d50765 100644
--- a/drivers/clk/thead/clk-th1520-ap.c
+++ b/drivers/clk/thead/clk-th1520-ap.c
@@ -411,7 +411,7 @@ static const struct clk_parent_data c910_i0_parents[] = {
{ .index = 0 }
};
-struct ccu_mux c910_i0_clk = {
+static struct ccu_mux c910_i0_clk = {
.mux = TH_CCU_ARG(1, 1),
.common = {
.clkid = CLK_C910_I0,
@@ -428,7 +428,7 @@ static const struct clk_parent_data c910_parents[] = {
{ .hw = &cpu_pll1_clk.common.hw }
};
-struct ccu_mux c910_clk = {
+static struct ccu_mux c910_clk = {
.mux = TH_CCU_ARG(0, 1),
.common = {
.clkid = CLK_C910,
@@ -845,7 +845,7 @@ static const struct clk_parent_data uart_sclk_parents[] = {
{ .index = 0 },
};
-struct ccu_mux uart_sclk = {
+static struct ccu_mux uart_sclk = {
.mux = TH_CCU_ARG(0, 1),
.common = {
.clkid = CLK_UART_SCLK,
Powered by blists - more mailing lists