Follow up from Linux Plumbers about my clk rate change talk (was Re: [PATCH RFC v4 00/12] clk: add support for v1 / v2 clock rate negotiation and kunit tests)

[Brian Masney <bmasney@redhat.com>]

Hi all,

On Fri, Nov 21, 2025 at 03:35:10PM -0500, Brian Masney wrote:
> On Fri, Nov 21, 2025 at 11:09 AM Maxime Ripard <mripard@kernel.org> wrote:
> > > I'm giving a talk at Linux Plumbers in Tokyo next month:
> > >
> > >     Fixing Clock Tree Propagation in the Common Clk Framework
> > >     https://lpc.events/event/19/contributions/2152/
> > >
> > > Stephen will be there as well, and hopefully we can reach consensus
> > > about an acceptable approach to fix this.
> >
> > Yeah, discussing it at plumbers would probably be a good idea, and maybe
> > try to record / transcribe the meeting so we can have the minutes too
> > somewhere?
> 
> The talk will be recorded, plus I'm sure there will be discussion
> after my talk. I'll post a summary to this thread with the next steps
> after Plumbers.

I presented a talk at Linux Plumbers in Tokyo last week about this patch
set:

    Fixing Clock Tree Propagation in the Common Clk Framework
    https://www.youtube.com/watch?v=R8TytDzlcFs
    https://lpc.events/event/19/contributions/2152/
    My slide deck is on the second link. I didn't present all of the
    slides.

In summary, I made the scope of this patch set too big, and this is an
intractable problem the way I currently have this setup. Some boards
are currently unknowingly dependent on the existing behavior, and we
need to keep things the way they currently are. We can detect when rates
are unknowingly changed by a sibling, and log a warning, however some
systems are configured to panic the system on warning, so we don't want
to go that route.

Most clock rates are fine to change with the existing behavior. It's
just some clocks that are dependent on a precise clock rate, such as DRM
and sound, that are affected by this.

The way forward on this is to leave the existing behavior in the clock
core, and fix this in the clock providers themselves. When a rate change
occurs, it can walk down that portion of the subtree inside the clk
provider via some new helpers that will be added to the clk core, and
ensure that the clocks that need precise rates will have their rates,
and their parent (if needed), updated as necessary. An array of rate
changes can be added to struct clk_rate_request, and the clk core can
update the clocks in that order. So it'll be up to the clk providers to
ensure that the array is populated in the correct order.

I'm going to get this working first in kunit, and I'll post a new
version of this patch set with these changes. Once that's done, I'll
work with Maxime and some other folks to find a board that has this
problem, and I'll ensure my new clk patch set is able to fix the issue.

Thank you to everyone that attended and provided feedback. Please reply
here if I missed something.


Separately, I talked to Stephen about ways that I can help him with clk
maintenance. Here's some information from my notes:

- I converted from round_rate to determine_rate() across most of the
  kernel tree in over 200 patches. However, the only remaining patch
  set is to the phy subsystem. The patches have received Reviewed-by's,
  however I haven't been able to get the phy maintainer to pick up the
  patches. Stephen mentioned he can pick them up. There was a merge
  conflict against the latest linux-next, so I posted a new version that
  addresses the merge conflict.

  https://lore.kernel.org/linux-clk/20251212-phy-clk-round-rate-v3-0-beae3962f767@redhat.com/T/#t

  Here's the patch set that actually removes round_rate() from the clk
  core.

  https://lore.kernel.org/linux-clk/20251212-clk-remove-round-rate-v1-0-5c3d5f3edc78@redhat.com/

  We still occasionally get people that try to add new round_rate
  implementations. I try to catch them when the patches are posted,
  however I miss some across the tree and will post a patch when it hits
  linux-next. 

  Someone two days ago posted a patch that adds a new round_rate(), so
  it'd ideally be nice to get my two patch sets above into linux-next to
  put a stop to this.

  https://lore.kernel.org/linux-clk/20251216-dr1v90-cru-v3-3-52cc938d1db0@pigmoral.tech/

  I commented on that patch to drop the round_rate.

- There are maybe a dozen or so determine_rate() implementations where
  it just returns 0, and it lets the firmware deal with setting the
  appropriate rate. Stephen suggested that we add a new flag to the
  clk core so that we can drop these empty determine_rate()
  implementations. We didn't talk about a name, but at the moment I'm
  leaning towards CLK_IS_FW_MANAGED. If that's set, then the clk core
  will not allow determine_rate to be set. I'm open to other
  suggestions for a name.

- I will continue to help review changes to the clk core. As for the
  clk providers themselves, I don't have access to the data sheets, and
  I can't really review those in detail. However, I think it would be
  nice if we add some extra clk-specific checks that we can run against
  patches. Look for common issues that come up in review. Look for cases
  where some of the helpers in clk-provider.h can possibly be used.
  Chen-Yu Tsai pointed out that's may not be entirely accurate in all
  cases, but we can at least warn about it.

  Another case I thought of is if someone posts a patch set where the
  clocks referenced in the dt bindings don't match what's actually in
  the clock provider itself.

  Make this as a clk-specific check script, and ideally see if we can
  hook that script into checkpatch.pl for clk-specific patches. My
  perl days are long over though, and I'd like to make the clk check
  script in python.

  Anyways, with a pre flight check like this, I could help review clk
  drivers themselves to look for anything else that's out of the
  ordinary.

It was good to meet so many people in person at Japan.

Brian