diff mbox series

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

Message ID 20260629-a733-rtc-v2-4-7b72112784f8@baylibre.com (mailing list archive)
State New
Headers
Series clk: sun6i-rtc: Add support for Allwinner A733 SoC |

Commit Message

Jerome Brunet June 29, 2026, 12:42 p.m. UTC 
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 | 11 +++++++++++
 drivers/clk/sunxi-ng/ccu_div.h |  1 +
 2 files changed, 12 insertions(+)

Comments

sashiko-bot@kernel.org June 29, 2026, 1:03 p.m. UTC | #1 
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] `ccu_rodiv_ops` omits the `.determine_rate` callback, which breaks rate rounding, reparenting, and proper rate propagation for clocks using `CLK_SET_RATE_PARENT`.
--

commit 97a1d0e7adc68eb81524d76b0c667da1b5f1be50
Author: Jerome Brunet <jbrunet@baylibre.com>

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

Add 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..0e3d9e6e30fb0 100644
> --- a/drivers/clk/sunxi-ng/ccu_div.c
> +++ b/drivers/clk/sunxi-ng/ccu_div.c
> @@ -143,3 +143,14 @@ 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,
> +
> +	.recalc_rate	= ccu_div_recalc_rate,

[Severity: Medium]
Does this struct need a .determine_rate callback to prevent breaking rate
rounding and parent rate propagation?

If a driver registers a clock using these exported operations and sets the
CLK_SET_RATE_PARENT flag, the Common Clock Framework's
clk_core_round_rate_nolock() falls back to a pass-through behavior when
.determine_rate is missing. This causes it to forward the child's requested
rate unmodified to the parent, ignoring the divider scaling, and also causes
mux reparenting to fail.

> +};
> +EXPORT_SYMBOL_NS_GPL(ccu_rodiv_ops, "SUNXI_CCU");
Jerome Brunet June 29, 2026, 1:58 p.m. UTC | #2 
On lun. 29 juin 2026 at 13:03, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] `ccu_rodiv_ops` omits the `.determine_rate` callback, which breaks rate rounding, reparenting, and proper rate propagation for clocks using `CLK_SET_RATE_PARENT`.
> --
>
> commit 97a1d0e7adc68eb81524d76b0c667da1b5f1be50
> Author: Jerome Brunet <jbrunet@baylibre.com>
>
> clk: sunxi-ng: div: add read-only operation support
>
> Add 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..0e3d9e6e30fb0 100644
>> --- a/drivers/clk/sunxi-ng/ccu_div.c
>> +++ b/drivers/clk/sunxi-ng/ccu_div.c
>> @@ -143,3 +143,14 @@ 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,
>> +
>> +	.recalc_rate	= ccu_div_recalc_rate,
>
> [Severity: Medium]
> Does this struct need a .determine_rate callback to prevent breaking rate
> rounding and parent rate propagation?

I don't think is a problem in the specific context of this series since
not parent on the RO clock may adjust but it is indeed something to fix
in the next revision

>
> If a driver registers a clock using these exported operations and sets the
> CLK_SET_RATE_PARENT flag, the Common Clock Framework's
> clk_core_round_rate_nolock() falls back to a pass-through behavior when
> .determine_rate is missing. This causes it to forward the child's requested
> rate unmodified to the parent, ignoring the divider scaling, and also causes
> mux reparenting to fail.
>
>> +};
>> +EXPORT_SYMBOL_NS_GPL(ccu_rodiv_ops, "SUNXI_CCU");
diff mbox series

Patch

diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c
index 62d680ccb524..0e3d9e6e30fb 100644
--- a/drivers/clk/sunxi-ng/ccu_div.c
+++ b/drivers/clk/sunxi-ng/ccu_div.c
@@ -143,3 +143,14 @@  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,
+
+	.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_ */