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/ |
Attachment:
signature.asc
Description: PGP signature
