From: Jerome Brunet <jbrunet@baylibre.com>
To: Chen-Yu Tsai <wens@kernel.org>
Cc: sashiko-reviews@lists.linux.dev, robh@kernel.org,
linux-rtc@vger.kernel.org, conor+dt@kernel.org,
devicetree@vger.kernel.org, linux-sunxi@lists.linux.dev,
Alexandre Belloni <alexandre.belloni@bootlin.com>,
linux-clk <linux-clk@vger.kernel.org>
Subject: Re: [PATCH v5 2/4] clk: sunxi-ng: div: add read-only operation support
Date: Sun, 19 Jul 2026 15:29:43 +0200 [thread overview]
Message-ID: <1jqzkzds54.fsf@starbuckisacylon.baylibre.com> (raw)
In-Reply-To: <CAGb2v65PmMnm=gusbVRJqwHkMFAdKpFyk8coXK=pf0-dBgv8kw@mail.gmail.com> (Chen-Yu Tsai's message of "Sun, 19 Jul 2026 18:59:33 +0800")
On dim. 19 juil. 2026 at 18:59, Chen-Yu Tsai <wens@kernel.org> wrote:
>
>> > +
>> > + } else {
>>
>> [ ... ]
>>
>> > @@ -143,3 +157,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_div_determine_rate,
>> > + .recalc_rate = ccu_div_recalc_rate,
>> > +};
>>
>> [Severity: High]
>> Could the missing .set_parent callback in ccu_rodiv_ops lead to a silent
>> desynchronization between the software clock tree and the hardware?
>>
>> The struct provides reparenting capabilities via .get_parent and
>> .determine_rate, but omits .set_parent. If a rate request causes
>> ccu_mux_helper_determine_rate() to evaluate and select a new parent, the
>> Common Clock Framework will apply this new parent in software but skip
>> updating the hardware since .set_parent is NULL.
>
> This is an issue. I'm not sure if what Sashiko says actually happens.
I think it could. I've defenitely made a mistake here.
> But 1. this is "read-only divider", not "read-only mux & divider",
The correct way to choose between the 2 is CLK_SET_RATE_NO_REPARENT I think.
> so the .set_parent callback should be provided. And 2. this is using
> the ccu_mux_determine_rate_helper, so it's possible a clk_set_rate()
> call is going to cause a reparent.
>
> I can add it while applying if there are no other issues.
As you prefer, I don't mind sending another version in a few days
(giving some review time to patch #1)
>
> ChenYu
>
>> This would leave all downstream clock rate calculations incorrect.
>>
>> --
>> Sashiko AI review · https://sashiko.dev/#/patchset/20260717-a733-rtc-v5-0-3874cc26abf7@baylibre.com?part=2
>>
--
Jerome
next prev parent reply other threads:[~2026-07-19 13:29 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-17 15:24 [PATCH v5 0/4] clk: sun6i-rtc: Add support for Allwinner A733 SoC Jerome Brunet
2026-07-17 15:24 ` [PATCH v5 1/4] clk: sunxi-ng: mux: fix determine helper rate propagation Jerome Brunet
2026-07-17 15:49 ` sashiko-bot
2026-07-19 10:53 ` Chen-Yu Tsai
2026-07-17 15:24 ` [PATCH v5 2/4] clk: sunxi-ng: div: add read-only operation support Jerome Brunet
2026-07-17 15:40 ` sashiko-bot
2026-07-19 10:59 ` Chen-Yu Tsai
2026-07-19 13:29 ` Jerome Brunet [this message]
2026-07-19 15:12 ` Chen-Yu Tsai
2026-07-17 15:24 ` [PATCH v5 3/4] clk: sunxi-ng: sun6i-rtc: split main oscillator div and gate Jerome Brunet
2026-07-17 15:33 ` sashiko-bot
2026-07-17 15:24 ` [PATCH v5 4/4] clk: sunxi-ng: sun6i-rtc: add a733 support Jerome Brunet
2026-07-17 15:40 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1jqzkzds54.fsf@starbuckisacylon.baylibre.com \
--to=jbrunet@baylibre.com \
--cc=alexandre.belloni@bootlin.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-rtc@vger.kernel.org \
--cc=linux-sunxi@lists.linux.dev \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=wens@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox