Re: [PATCH 07/13] clk: detect unintended rate changes

[Maxime Ripard]

Hi,

On Mon, Sep 18, 2023 at 12:40:03AM +0200, Benjamin Bara wrote:
> From: Benjamin Bara <benjamin.bara@xxxxxxxxxxx>
> 
> As we now keep track of the clocks which are allowed to change - namely
> the ones which are along the ancestor line between the rate trigger and
> the top-most changed clock

I'm not sure that the fact that only a clock between the clock
triggering the rate change and the top-most changed clock was allowed to
change has ever been a thing.

This puts a fairly big pressure on the tree propagation code, and
whether or not that can be done is completely context-dependent.

Devices like UART, I2C or audio devices are rate change sensitive, and
yet usually have internal dividers to accomodate for their parent rate
so don't usually care as long as they are notified.

Similarly, all the non-rate-sensitive devices (like pretty much all
bus/registers clocks) don't care at all about what the rate is, so that
requirement is making a rate change going through less likely, without a
particular reason only for a handful of devices (display in this case).

Also, this rules out any child of a clock from being allowed to change?
That looks wild to me :)

> we can run through the subtree of changes and look for unexpected
> ones.

Again, I'm not sure we can say that those changes were unexpected. They
very much were expected to change to the CCF so far.

> Shared parents must set their rate in a way, that all
> consumer-configured rates are respected.

Again, that entirely depends on the context: the clock tree topology,
the devices involved, what their drivers support, etc. I'm sure it's
true in your case, but I'm not sure we can make that a generic
statement.

> As this is sometimes not possible and clocks sometime doesn't require
> the *exact* rate, we might have to find a way to find out if it is
> *exact enough*. Then we could fix it in the core.

And "exact enough" entirely depends on the context again, so really, I'm
not sure we can (and should!) fix that at the framework level.

Maxime

Attachment: signature.asc
Description: PGP signature