[v3,6/8] 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.

Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
---
 drivers/clk/sunxi-ng/ccu_div.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 drivers/clk/sunxi-ng/ccu_div.h |  1 +
 drivers/clk/sunxi-ng/ccu_mux.c |  3 ++-
 drivers/clk/sunxi-ng/ccu_mux.h |  4 ++++
 4 files changed, 49 insertions(+), 1 deletion(-)
  

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The existing ccu_div_determine_rate_helper incorrectly asks the hardware parent for a non-predivided rate when CLK_SET_RATE_PARENT is set, resulting in unvalidated parent rate requests.
--

commit 20e3c89e1fcae37e3eaf3a9be6d0c4e28094d07d
Author: Jerome Brunet <jbrunet@baylibre.com>

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

This commit adds support for sunxi-ng read-only dividers to be used
by 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..d1c8c7baa12dc 100644
> --- a/drivers/clk/sunxi-ng/ccu_div.c
> +++ b/drivers/clk/sunxi-ng/ccu_div.c
> @@ -84,6 +84,36 @@ static int ccu_div_determine_rate(struct clk_hw *hw,
>  					     req, ccu_div_determine_rate_helper, cd);
>  }

[Severity: High]
This is a pre-existing issue, but does ccu_div_determine_rate_helper
incorrectly ask the hardware parent for a non-predivided rate when
CLK_SET_RATE_PARENT is set?

When ccu_div_determine_rate is called on a clock with a pre-divider, it
delegates to ccu_mux_helper_determine_rate, which calls
ccu_div_determine_rate_helper. That helper then directly invokes
divider_determine_rate() which queries req->best_parent_hw for
req->rate * div.

Could this cause the clock core to query the upstream parent for a target
rate without accounting for the pre-divider? If the core subsequently sets
the parent rate to an unvalidated higher rate, it seems the parent PLL may
fail or clamp because it was asked for an achievable rate that it cannot
actually sustain when multiplied by the pre-divider.

[ ... ]
  

On Thu, Jul 2, 2026 at 4:11 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.
>
> Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> ---
>  drivers/clk/sunxi-ng/ccu_div.c | 42 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/clk/sunxi-ng/ccu_div.h |  1 +
>  drivers/clk/sunxi-ng/ccu_mux.c |  3 ++-
>  drivers/clk/sunxi-ng/ccu_mux.h |  4 ++++
>  4 files changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c
> index 62d680ccb524..d1c8c7baa12d 100644
> --- a/drivers/clk/sunxi-ng/ccu_div.c
> +++ b/drivers/clk/sunxi-ng/ccu_div.c
> @@ -84,6 +84,36 @@ static int ccu_div_determine_rate(struct clk_hw *hw,
>                                              req, ccu_div_determine_rate_helper, cd);
>  }
>
> +static int ccu_rodiv_determine_rate(struct clk_hw *hw,
> +                                   struct clk_rate_request *req)
> +{
> +       struct ccu_div *cd = hw_to_ccu_div(hw);
> +       unsigned long val;
> +       u32 reg;
> +       int ret;
> +
> +       reg = readl(cd->common.base + cd->common.reg);
> +       val = reg >> cd->div.shift;
> +       val &= (1 << cd->div.width) - 1;
> +
> +       req->rate = ccu_mux_helper_unapply_prediv(&cd->common, &cd->mux, -1,
> +                                                 req->rate);
> +
> +       if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV)
> +               req->rate *= cd->fixed_post_div;
> +
> +       ret = divider_ro_determine_rate(hw, req, cd->div.table,
> +                                       cd->div.width, cd->div.flags, val);
> +
> +       if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV)
> +               req->rate /= cd->fixed_post_div;
> +
> +       req->rate = ccu_mux_helper_apply_prediv(&cd->common, &cd->mux, -1,
> +                                               req->rate);
> +
> +       return ret;
> +}
> +
>  static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate,
>                            unsigned long parent_rate)
>  {
> @@ -143,3 +173,15 @@ 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,
> +
> +       .determine_rate = ccu_rodiv_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 766f27cff748..e2d6833a6d33 100644
> --- a/drivers/clk/sunxi-ng/ccu_mux.c
> +++ b/drivers/clk/sunxi-ng/ccu_mux.c
> @@ -68,13 +68,14 @@ unsigned long ccu_mux_helper_apply_prediv(struct ccu_common *common,
>  }
>  EXPORT_SYMBOL_NS_GPL(ccu_mux_helper_apply_prediv, "SUNXI_CCU");
>
> -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)
>  {
>         return parent_rate * ccu_mux_get_prediv(common, cm, parent_index);
>  }
> +EXPORT_SYMBOL_NS_GPL(ccu_mux_helper_unapply_prediv, "SUNXI_CCU");

This does not need to be exported since all the base clocks build into
one module. And maybe it shouldn't as we probably don't want individual
clock drivers implementing ops.


Otherwise,

Reviewed-by: Chen-Yu Tsai <wens@kernel.org>

>  int ccu_mux_helper_determine_rate(struct ccu_common *common,
>                                   struct ccu_mux_internal *cm,
> 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
>
  

On sam. 04 juil. 2026 at 16:25, Chen-Yu Tsai <wens@kernel.org> wrote:

>>
>>  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 766f27cff748..e2d6833a6d33 100644
>> --- a/drivers/clk/sunxi-ng/ccu_mux.c
>> +++ b/drivers/clk/sunxi-ng/ccu_mux.c
>> @@ -68,13 +68,14 @@ unsigned long ccu_mux_helper_apply_prediv(struct ccu_common *common,
>>  }
>>  EXPORT_SYMBOL_NS_GPL(ccu_mux_helper_apply_prediv, "SUNXI_CCU");
>>
>> -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)
>>  {
>>         return parent_rate * ccu_mux_get_prediv(common, cm, parent_index);
>>  }
>> +EXPORT_SYMBOL_NS_GPL(ccu_mux_helper_unapply_prediv, "SUNXI_CCU");
>
> This does not need to be exported since all the base clocks build into
> one module. And maybe it shouldn't as we probably don't want individual
> clock drivers implementing ops.

Indeed, I'll add a patch to remove the export of
ccu_mux_helper_apply_prediv() as well, for consistency. It does not
appear to used out of that module either.

>
>
> Otherwise,
>
> Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
>