diff mbox series

[v7,2/4] clk: sunxi-ng: div: add read-only operation support

Message ID 20260723-a733-rtc-v7-2-8fd68aab94ae@baylibre.com (mailing list archive)
State New
Headers
Series clk: sun6i-rtc: Add support for Allwinner A733 SoC |

Commit Message

Jerome Brunet July 23, 2026, 9:39 a.m. UTC 
Add support for sunxi-ng read-only dividers. This will be
useful to the a733 oscillator detection logic.

Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/sunxi-ng/ccu_div.c | 31 +++++++++++++++++++++++++++++--
 drivers/clk/sunxi-ng/ccu_div.h |  1 +
 2 files changed, 30 insertions(+), 2 deletions(-)

Comments

sashiko-bot@kernel.org July 23, 2026, 9:50 a.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential division-by-zero kernel panic in `divider_ro_determine_rate` if the hardware register contains an invalid or unmapped value.
--

--- Patch [2]: [PATCH v7 2/4] clk: sunxi-ng: div: add read-only operation support ---
commit 3b6cfd40ff69176dfa1e8841c3c6d40584ab1369
Author: Jerome Brunet <jbrunet@baylibre.com>

clk: sunxi-ng: div: add read-only operation support

Add support for sunxi-ng read-only dividers. This will be
useful to the a733 oscillator detection logic.

> diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c
> --- a/drivers/clk/sunxi-ng/ccu_div.c
> +++ b/drivers/clk/sunxi-ng/ccu_div.c
> @@ -20,8 +20,22 @@ static int ccu_div_determine_rate_helper(struct ccu_mux_internal *mux,
>  	if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV)
>  		req->rate *= cd->fixed_post_div;
>  
> -	ret = divider_determine_rate(&cd->common.hw, req, cd->div.table,
> -				     cd->div.width, cd->div.flags);
> +	if (cd->div.flags & CLK_DIVIDER_READ_ONLY) {
> +		unsigned long val;
> +		u32 reg;
> +
> +		reg = readl(cd->common.base + cd->common.reg);
> +		val = reg >> cd->div.shift;
> +		val &= (1 << cd->div.width) - 1;
> +
> +		ret = divider_ro_determine_rate(&cd->common.hw, req, cd->div.table,
> +						cd->div.width, cd->div.flags, val);

[Severity: High]
This is a pre-existing issue, but could this expose a division by zero in
divider_ro_determine_rate() if the hardware register yields an invalid value?

The new code reads a raw divider value from the hardware register and passes
it directly into divider_ro_determine_rate(). If the register is in an
uninitialized or corrupted state containing a value not present in the
associated table, the internal _get_div() helper can return 0.

Unlike divider_recalc_rate() which safely checks the divisor,
divider_ro_determine_rate() directly evaluates DIV_ROUND_UP_ULL(..., div):

drivers/clk/clk-divider.c:divider_ro_determine_rate() {
    ...
    div = _get_div(table, val, flags, width);
    ...
    req->rate = DIV_ROUND_UP_ULL((u64)req->best_parent_rate, div);
    ...
}

Since this macro performs a division, a divisor of 0 triggers a
divide-by-zero. Should the hardware value or the resulting divisor be
validated to prevent this regression during clock initialization?

> +
> +	} else {
> +		ret = divider_determine_rate(&cd->common.hw, req, cd->div.table,
> +					     cd->div.width, cd->div.flags);
> +	}
> +
diff mbox series

Patch

diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c
index 62d680ccb524..c385e0160f13 100644
--- a/drivers/clk/sunxi-ng/ccu_div.c
+++ b/drivers/clk/sunxi-ng/ccu_div.c
@@ -20,8 +20,22 @@  static int ccu_div_determine_rate_helper(struct ccu_mux_internal *mux,
 	if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV)
 		req->rate *= cd->fixed_post_div;
 
-	ret = divider_determine_rate(&cd->common.hw, req, cd->div.table,
-				     cd->div.width, cd->div.flags);
+	if (cd->div.flags & CLK_DIVIDER_READ_ONLY) {
+		unsigned long val;
+		u32 reg;
+
+		reg = readl(cd->common.base + cd->common.reg);
+		val = reg >> cd->div.shift;
+		val &= (1 << cd->div.width) - 1;
+
+		ret = divider_ro_determine_rate(&cd->common.hw, req, cd->div.table,
+						cd->div.width, cd->div.flags, val);
+
+	} else {
+		ret = divider_determine_rate(&cd->common.hw, req, cd->div.table,
+					     cd->div.width, cd->div.flags);
+	}
+
 	if (ret)
 		return ret;
 
@@ -143,3 +157,16 @@  const struct clk_ops ccu_div_ops = {
 	.set_rate	= ccu_div_set_rate,
 };
 EXPORT_SYMBOL_NS_GPL(ccu_div_ops, "SUNXI_CCU");
+
+const struct clk_ops ccu_rodiv_ops = {
+	.disable	= ccu_div_disable,
+	.enable		= ccu_div_enable,
+	.is_enabled	= ccu_div_is_enabled,
+
+	.get_parent	= ccu_div_get_parent,
+	.set_parent	= ccu_div_set_parent,
+
+	.determine_rate	= ccu_div_determine_rate,
+	.recalc_rate	= ccu_div_recalc_rate,
+};
+EXPORT_SYMBOL_NS_GPL(ccu_rodiv_ops, "SUNXI_CCU");
diff --git a/drivers/clk/sunxi-ng/ccu_div.h b/drivers/clk/sunxi-ng/ccu_div.h
index be00b3277e97..a30a92780a05 100644
--- a/drivers/clk/sunxi-ng/ccu_div.h
+++ b/drivers/clk/sunxi-ng/ccu_div.h
@@ -300,5 +300,6 @@  static inline struct ccu_div *hw_to_ccu_div(struct clk_hw *hw)
 }
 
 extern const struct clk_ops ccu_div_ops;
+extern const struct clk_ops ccu_rodiv_ops;
 
 #endif /* _CCU_DIV_H_ */