Devicetree
 help / color / mirror / Atom feed
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

  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