diff mbox series

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

Message ID 20260722-a733-rtc-v6-2-631cca28cc9a@baylibre.com (mailing list archive)
State New
Headers
Series clk: sun6i-rtc: Add support for Allwinner A733 SoC |

Commit Message

Jerome Brunet July 22, 2026, 2:48 p.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 +
 drivers/clk/sunxi-ng/ccu_mux.c |  2 +-
 drivers/clk/sunxi-ng/ccu_mux.h |  4 ++++
 4 files changed, 35 insertions(+), 3 deletions(-)

Comments

sashiko-bot@kernel.org July 22, 2026, 3 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Potential division by zero due to unvalidated hardware register value propagating to `divider_ro_determine_rate`.
- [Low] Undocumented API exposure and dead code due to exporting `ccu_mux_helper_unapply_prediv`.
--

--- Patch [2]: [PATCH v6 2/4] clk: sunxi-ng: div: add read-only operation support ---
commit 14ce0824b93b836a325dbcab101b0319dddbe86a
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
> index 62d680ccb524b..c385e0160f13b 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);

[Severity: High]
Can this lead to a division by zero in divider_ro_determine_rate()?

When ccu_div_determine_rate_helper() reads the hardware register and passes
the unvalidated value to divider_ro_determine_rate(), _get_div() could
return 0 if the register value is 0 (with CLK_DIVIDER_ONE_BASED) or if the
value is not found in the table.

Unlike divider_recalc_rate(), divider_ro_determine_rate() does not have an
if (!div) check:

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);
    ...
}

Will this cause a panic if the hardware is in an unexpected reset state or
misconfigured?

[ ... ]

> diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c
> index 4503c9780c39f..12bfc99c91721 100644
> --- a/drivers/clk/sunxi-ng/ccu_mux.c
> +++ b/drivers/clk/sunxi-ng/ccu_mux.c
> @@ -67,7 +67,7 @@ unsigned long ccu_mux_helper_apply_prediv(struct ccu_common *common,
>  	return parent_rate / ccu_mux_get_prediv(common, cm, parent_index);
>  }
>  
> -static unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
> +unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
>  					    struct ccu_mux_internal *cm,
>  					    int parent_index,
>  					    unsigned long parent_rate)

[Severity: Low]
Is this function intended to be exported?

The commit message only mentions adding support for read-only operation in
sunxi-ng dividers and provides no justification for modifying mux helpers.

Additionally, ccu_mux_helper_unapply_prediv appears to be completely unused
outside of ccu_mux.c in this patch series. Could this change be an
undocumented API export that accidentally leaked from earlier development?
Chen-Yu Tsai July 22, 2026, 3:40 p.m. UTC | #2 
On Wed, Jul 22, 2026 at 10:48 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
> 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 +

>  drivers/clk/sunxi-ng/ccu_mux.c |  2 +-
>  drivers/clk/sunxi-ng/ccu_mux.h |  4 ++++

Left over stuff?


>  4 files changed, 35 insertions(+), 3 deletions(-)
>
> 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_ */
> diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c
> index 4503c9780c39..12bfc99c9172 100644
> --- a/drivers/clk/sunxi-ng/ccu_mux.c
> +++ b/drivers/clk/sunxi-ng/ccu_mux.c
> @@ -67,7 +67,7 @@ unsigned long ccu_mux_helper_apply_prediv(struct ccu_common *common,
>         return parent_rate / ccu_mux_get_prediv(common, cm, parent_index);
>  }
>
> -static unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
> +unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
>                                             struct ccu_mux_internal *cm,
>                                             int parent_index,
>                                             unsigned long parent_rate)
> diff --git a/drivers/clk/sunxi-ng/ccu_mux.h b/drivers/clk/sunxi-ng/ccu_mux.h
> index c94a4bde5d01..272a2c36a8f2 100644
> --- a/drivers/clk/sunxi-ng/ccu_mux.h
> +++ b/drivers/clk/sunxi-ng/ccu_mux.h
> @@ -134,6 +134,10 @@ unsigned long ccu_mux_helper_apply_prediv(struct ccu_common *common,
>                                           struct ccu_mux_internal *cm,
>                                           int parent_index,
>                                           unsigned long parent_rate);
> +unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
> +                                           struct ccu_mux_internal *cm,
> +                                           int parent_index,
> +                                           unsigned long parent_rate);
>  int ccu_mux_helper_determine_rate(struct ccu_common *common,
>                                   struct ccu_mux_internal *cm,
>                                   struct clk_rate_request *req,
>
> --
> 2.47.3
>
Jerome Brunet July 22, 2026, 4:28 p.m. UTC | #3 
On mer. 22 juil. 2026 at 23:40, Chen-Yu Tsai <wens@kernel.org> wrote:

> On Wed, Jul 22, 2026 at 10:48 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>>
>> 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 +
>
>>  drivers/clk/sunxi-ng/ccu_mux.c |  2 +-
>>  drivers/clk/sunxi-ng/ccu_mux.h |  4 ++++
>
> Left over stuff?
>

Argh indeed, that's no longer necessary 

>
>>  4 files changed, 35 insertions(+), 3 deletions(-)
>>
>> 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_ */
>> diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c
>> index 4503c9780c39..12bfc99c9172 100644
>> --- a/drivers/clk/sunxi-ng/ccu_mux.c
>> +++ b/drivers/clk/sunxi-ng/ccu_mux.c
>> @@ -67,7 +67,7 @@ unsigned long ccu_mux_helper_apply_prediv(struct ccu_common *common,
>>         return parent_rate / ccu_mux_get_prediv(common, cm, parent_index);
>>  }
>>
>> -static unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
>> +unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
>>                                             struct ccu_mux_internal *cm,
>>                                             int parent_index,
>>                                             unsigned long parent_rate)
>> diff --git a/drivers/clk/sunxi-ng/ccu_mux.h b/drivers/clk/sunxi-ng/ccu_mux.h
>> index c94a4bde5d01..272a2c36a8f2 100644
>> --- a/drivers/clk/sunxi-ng/ccu_mux.h
>> +++ b/drivers/clk/sunxi-ng/ccu_mux.h
>> @@ -134,6 +134,10 @@ unsigned long ccu_mux_helper_apply_prediv(struct ccu_common *common,
>>                                           struct ccu_mux_internal *cm,
>>                                           int parent_index,
>>                                           unsigned long parent_rate);
>> +unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
>> +                                           struct ccu_mux_internal *cm,
>> +                                           int parent_index,
>> +                                           unsigned long parent_rate);
>>  int ccu_mux_helper_determine_rate(struct ccu_common *common,
>>                                   struct ccu_mux_internal *cm,
>>                                   struct clk_rate_request *req,
>>
>> --
>> 2.47.3
>>
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_ */
diff --git a/drivers/clk/sunxi-ng/ccu_mux.c b/drivers/clk/sunxi-ng/ccu_mux.c
index 4503c9780c39..12bfc99c9172 100644
--- a/drivers/clk/sunxi-ng/ccu_mux.c
+++ b/drivers/clk/sunxi-ng/ccu_mux.c
@@ -67,7 +67,7 @@  unsigned long ccu_mux_helper_apply_prediv(struct ccu_common *common,
 	return parent_rate / ccu_mux_get_prediv(common, cm, parent_index);
 }
 
-static unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
+unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
 					    struct ccu_mux_internal *cm,
 					    int parent_index,
 					    unsigned long parent_rate)
diff --git a/drivers/clk/sunxi-ng/ccu_mux.h b/drivers/clk/sunxi-ng/ccu_mux.h
index c94a4bde5d01..272a2c36a8f2 100644
--- a/drivers/clk/sunxi-ng/ccu_mux.h
+++ b/drivers/clk/sunxi-ng/ccu_mux.h
@@ -134,6 +134,10 @@  unsigned long ccu_mux_helper_apply_prediv(struct ccu_common *common,
 					  struct ccu_mux_internal *cm,
 					  int parent_index,
 					  unsigned long parent_rate);
+unsigned long ccu_mux_helper_unapply_prediv(struct ccu_common *common,
+					    struct ccu_mux_internal *cm,
+					    int parent_index,
+					    unsigned long parent_rate);
 int ccu_mux_helper_determine_rate(struct ccu_common *common,
 				  struct ccu_mux_internal *cm,
 				  struct clk_rate_request *req,