Quoting Maxime Ripard (2022-02-25 06:26:06) > Hi Stephen, > > On Thu, Feb 24, 2022 at 02:54:20PM -0800, Stephen Boyd wrote: > > Quoting Daniel Latypov (2022-02-23 14:50:59) > > > On Wed, Feb 23, 2022 at 2:56 AM Maxime Ripard <maxime@xxxxxxxxxx> wrote: > > > Incremental coverage for 3/9 files in --diff_file > > > Total incremental: 99.29% coverage (281/283 lines) > > > drivers/clk/clk.c: 84.62% coverage (11/13 lines) > > > drivers/clk/clk_test.c: 100.00% coverage (269/269 lines) > > > include/linux/clk.h: 100.00% coverage (1/1 lines) > > > > > > Missing lines are drivers/clk/clk.c:2397-8, i.e. this part of the diff: > > > + if (ret) { > > > + /* rollback the changes */ > > > + clk->min_rate = old_min; <- 2397 > > > + clk->max_rate = old_max; <- 2398 > > > > > > These are from before and were just moved around. > > > > We could trigger a failure in the provider when the rate is set, and > > then we could call round_rate() again and make sure the boundaries from > > before are maintained. > > I tried to do that, and it turns out we can't, since we ignore the > set_rate return code: > > https://elixir.bootlin.com/linux/latest/source/drivers/clk/clk.c#L2107 > > We could make determine_rate fail, but then clk_round_rate would fail as > well and wouldn't allow us to test whether the boundaries are still in > place. > The test could still do it at a high level right? And when/if we decide to bubble up the set_rate failure then we would be testing these lines. Seems like a good idea to implement it with a TODO note that clk.c is ignoring the set_rate clk_op returning failure.