linux-kernel - [RFC PATCH] clk: fractional-divider: Correct max_{m,n} handed over to rational_best

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210714064129.1321277-1-victor.liu@nxp.com>
Date:   Wed, 14 Jul 2021 14:41:29 +0800
From:   Liu Ying <victor.liu@....com>
To:     linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Dong Aisheng <aisheng.dong@....com>,
        NXP Linux Team <linux-imx@....com>,
        Jacky Bai <ping.bai@....com>
Subject: [RFC PATCH] clk: fractional-divider: Correct max_{m,n} handed over to rational_best_approximation()

If a fractional divider clock has the flag
CLK_FRAC_DIVIDER_ZERO_BASED set, the maximum
numerator and denominator handed over to
rational_best_approximation(), in this case
max_m and max_n, should be increased by one
comparing to those have the flag unset.  Without
this patch, a zero based fractional divider
with 1-bit mwidth and 3-bit nwidth would wrongly
generate 96MHz clock rate if the parent clock
rate is 288MHz, while the expected clock rate
is 115.2MHz with m = 2 and n = 5.

Fixes: e983da27f70e ("clk: fractional-divider: add CLK_FRAC_DIVIDER_ZERO_BASED flag support")
Cc: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: Michael Turquette <mturquette@...libre.com>
Cc: Stephen Boyd <sboyd@...nel.org>
Cc: Dong Aisheng <aisheng.dong@....com>
Cc: NXP Linux Team <linux-imx@....com>
Signed-off-by: Liu Ying <victor.liu@....com>
Signed-off-by: Jacky Bai <ping.bai@....com>
---
The patch is RFC, because the rationale behind the below snippet in
clk_fd_general_approximation() is unclear to Jacky and me and we are
not sure if there is any room to improve this patch due to the snippet.
Maybe, Andy may help shed some light here.  Thanks.

-----------------------------------8<---------------------------------
/*
 * Get rate closer to *parent_rate to guarantee there is no overflow
 * for m and n. In the result it will be the nearest rate left shifted
 * by (scale - fd->nwidth) bits.
 */
scale = fls_long(*parent_rate / rate - 1);
if (scale > fd->nwidth)
	rate <<= scale - fd->nwidth;
-----------------------------------8<---------------------------------

Jacky helped test this patch on i.MX7ulp EVK platform.

 drivers/clk/clk-fractional-divider.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c
index b1e556f20911..86e985e14468 100644
--- a/drivers/clk/clk-fractional-divider.c
+++ b/drivers/clk/clk-fractional-divider.c
@@ -30,6 +30,19 @@ static inline void clk_fd_writel(struct clk_fractional_divider *fd, u32 val)
 		writel(val, fd->reg);
 }
 
+static inline void clk_fd_get_max_m_n(struct clk_fractional_divider *fd,
+				      unsigned long *max_m,
+				      unsigned long *max_n)
+{
+	*max_m = GENMASK(fd->mwidth - 1, 0);
+	*max_n = GENMASK(fd->nwidth - 1, 0);
+
+	if (fd->flags & CLK_FRAC_DIVIDER_ZERO_BASED) {
+		(*max_m)++;
+		(*max_n)++;
+	}
+}
+
 static unsigned long clk_fd_recalc_rate(struct clk_hw *hw,
 					unsigned long parent_rate)
 {
@@ -73,7 +86,7 @@ static void clk_fd_general_approximation(struct clk_hw *hw, unsigned long rate,
 					 unsigned long *m, unsigned long *n)
 {
 	struct clk_fractional_divider *fd = to_clk_fd(hw);
-	unsigned long scale;
+	unsigned long scale, max_m, max_n;
 
 	/*
 	 * Get rate closer to *parent_rate to guarantee there is no overflow
@@ -84,9 +97,9 @@ static void clk_fd_general_approximation(struct clk_hw *hw, unsigned long rate,
 	if (scale > fd->nwidth)
 		rate <<= scale - fd->nwidth;
 
-	rational_best_approximation(rate, *parent_rate,
-			GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0),
-			m, n);
+	clk_fd_get_max_m_n(fd, &max_m, &max_n);
+
+	rational_best_approximation(rate, *parent_rate, max_m, max_n, m, n);
 }
 
 static long clk_fd_round_rate(struct clk_hw *hw, unsigned long rate,
@@ -115,12 +128,13 @@ static int clk_fd_set_rate(struct clk_hw *hw, unsigned long rate,
 {
 	struct clk_fractional_divider *fd = to_clk_fd(hw);
 	unsigned long flags = 0;
+	unsigned long max_m, max_n;
 	unsigned long m, n;
 	u32 val;
 
-	rational_best_approximation(rate, parent_rate,
-			GENMASK(fd->mwidth - 1, 0), GENMASK(fd->nwidth - 1, 0),
-			&m, &n);
+	clk_fd_get_max_m_n(fd, &max_m, &max_n);
+
+	rational_best_approximation(rate, parent_rate, max_m, max_n, &m, &n);
 
 	if (fd->flags & CLK_FRAC_DIVIDER_ZERO_BASED) {
 		m--;
-- 
2.25.1