[PATCH 0/5] clk: Make clk_rate_exclusive_get() return void

[Uwe Kleine-König]

Hello Maxime,

On Wed, Dec 13, 2023 at 08:16:04AM +0100, Maxime Ripard wrote:
> On Tue, Dec 12, 2023 at 06:26:37PM +0100, Uwe Kleine-König wrote:
> > clk_rate_exclusive_get() returns zero unconditionally. Most users "know"
> > that and don't check the return value. This series fixes the four users
> > that do error checking on the returned value and then makes function
> > return void.
> > 
> > Given that the changes to the drivers are simple and so merge conflicts
> > (if any) should be easy to handle, I suggest to merge this complete
> > series via the clk tree.
> 
> I don't think it's the right way to go about it.
> 
> clk_rate_exclusive_get() should be expected to fail. For example if
> there's another user getting an exclusive rate on the same clock.
> 
> If we're not checking for it right now, then it should probably be
> fixed, but the callers checking for the error are right to do so if they
> rely on an exclusive rate. It's the ones that don't that should be
> modified.

If some other consumer has already "locked" a clock that I call
clk_rate_exclusive_get() for, this isn't an error. In my bubble I call
this function because I don't want the rate to change e.g. because I
setup some registers in the consuming device to provide a fixed UART
baud rate or i2c bus frequency (and that works as expected). In this
case I won't be able to change the rate of the clock, but that is
signalled by clk_set_rate() failing (iff and when I need awother rate)
which also seems the right place to fail to me.

It's like that since clk_rate_exclusive_get() was introduced in 2017
(commit 55e9b8b7b806ec3f9a8817e13596682a5981c19c).

BTW, I just noticed that my assertion "Most users \"know\" that
[clk_rate_exclusive_get() returns zero unconditionally]" is wrong. As of
next-20231213 there are 3 callers ignoring the return value of
clk_rate_exclusive_get() and 4 that handle (imaginary) returned errors.
I expected this function to be used more extensively. (In fact I think
it should be used more as several drivers rely on the clk rate not
changing.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-amlogic/attachments/20231213/583b0b68/attachment.sig>